-
Notifications
You must be signed in to change notification settings - Fork 62
chore(island-ui): Refactor InputFileUpload component #18801
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis change introduces a new deprecated file upload component ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FileUploadComponent
participant AppState
User->>FileUploadComponent: Selects or drags file(s)
FileUploadComponent->>AppState: Calls onChange with file list (deprecated or current type)
AppState->>FileUploadComponent: Renders files with status (enum or string)
User->>FileUploadComponent: Clicks remove/retry/open
FileUploadComponent->>AppState: Calls onRemove/onRetry/onOpenFile callbacks
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (48)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
View your CI Pipeline Execution ↗ for commit de9000e.
☁️ Nx Cloud last updated this comment at |
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 9 Passed Test ServicesThis report shows up to 10 services
|
…r-input-file-upload
…/island.is into ui/refactor-input-file-upload
…island.is into ui/refactor-input-file-upload
…or-input-file-upload
…island.is into ui/refactor-input-file-upload
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
libs/island-ui/core/src/lib/InputFileUpload/InputFileUpload.tsx (1)
101-111
: ImprovedtruncateMiddle
function no longer depends on external hooksThis implementation is more efficient as it no longer relies on the
useMeasure
hook as mentioned in the PR objectives. However, the maxLength is hardcoded which might not be ideal for all scenarios.Consider making the
maxLength
configurable through props to handle different UI contexts:- const truncateMiddle = (fileName: string) => { + const truncateMiddle = (fileName: string, maxLength = 40) => { - const maxLength = 40 if (fileName.length <= maxLength) return fileName
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/judicial-system/web/src/components/AccordionItems/IndictmentsCaseFilesAccordionItem/IndictmentsCaseFilesAccordionItem.spec.tsx
(4 hunks)libs/island-ui/core/src/lib/InputFileUpload/InputFileUpload.tsx
(8 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/judicial-system/web/src/components/AccordionItems/IndictmentsCaseFilesAccordionItem/IndictmentsCaseFilesAccordionItem.spec.tsx
🧰 Additional context used
📓 Path-based instructions (1)
`libs/**/*`: "Confirm that the code adheres to the following: - Reusability of components and hooks across different NextJS apps. - TypeScript usage for defining props and exportin...
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/island-ui/core/src/lib/InputFileUpload/InputFileUpload.tsx
🔇 Additional comments (9)
libs/island-ui/core/src/lib/InputFileUpload/InputFileUpload.tsx (9)
59-64
: Improved icon management with a dedicated interfaceThis new
Icons
interface provides a centralized way to manage and customize icons, making the component more flexible and maintainable. This is a good replacement for the previousdoneIcon
prop mentioned in the PR objectives.
76-90
: Good use of default values for iconsThe component now provides sensible defaults for icons while allowing consumers to override any individual icon if needed. This implementation aligns well with the PR objective of making the component more flexible.
92-99
: Well-designed event handler abstractionThis
handleClick
function effectively reduces code duplication by centralizing event handling logic. It properly handles event propagation and conditionally calls the appropriate handler.
113-127
: Enhanced file size formatting with appropriate unitsThe new
formatFileSize
function is a significant improvement as stated in the PR objectives. It now correctly handles different size units (B, KB, MB, GB) with appropriate precision.
184-186
: Improved file name and size displayGood implementation of the truncated file name and size formatting. The condition properly checks that file.size is a number before attempting to format it.
190-190
: Space between icon and textThis intentional space between the icon and text improves readability, as confirmed in the previous review comments.
229-248
: Improved props interface with better naming and organizationThe props interface has been significantly improved:
- Added required
name
prop for better accessibility- Renamed
fileList
tofiles
andheader
totitle
for clarity- Added comments clarifying the units for
maxSize
- Included the new
hideIcons
prop for more flexibilityThese changes align with the PR objective of simplifying the component's props.
316-316
: Fixed style prop usageThe style is now correctly passed to
getRootProps
, addressing the issue identified in previous review comments.
344-344
: Improved accessibility with proper input attributesUsing the
name
prop for bothid
andname
attributes on the input element improves form handling and accessibility, as it ensures the input can be properly associated with its label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cbcbc05
to
b1962c8
Compare
…or-input-file-upload # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Core files lgtm
…r-input-file-upload
…island.is into ui/refactor-input-file-upload
…r-input-file-upload
…island.is into ui/refactor-input-file-upload
…r-input-file-upload
…r-input-file-upload
Refactor InputFileUpload component
Asana
What
This is a re-write of the
InputFileUpload
component. What this re-write accomplishes isUploadFileStatus
andFileUploadStatus
which contained the same values.doneIcon
prop and this pr accepts aicons
prop where consumers can control each individual icon, if they want.truncateMiddle
andformatFileSize
functions.truncateMiddle
now no longer relies on auseMeasure
hook andformatFileSize
displays B, KB, MB, GB as appropriate.Why
The old component was becoming bloated and in need of a rewrite.
Checklist:
Summary by CodeRabbit
Refactor
fileList
tofiles
,header
totitle
, and addedname
props where applicable.showFileSize
prop in some file upload components.Chores