refactor(platform): update file upload components#344
Conversation
📝 WalkthroughWalkthroughThis PR refactors the file upload system by introducing a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@services/platform/app/components/ui/forms/file-upload.tsx`:
- Around line 141-153: The DropZone uses a div with role="button" (in
services/platform/app/components/ui/forms/file-upload.tsx) which is both
semantically wrong and causes clicks on child elements to trigger file
selection; either replace the outer div with a real <button> (ensure it supports
disabled, aria-label and keyboard handlers) or modify the handleClick handler to
ignore clicks coming from child elements by checking event.target vs the
container ref and calling event.stopPropagation()/return early when they differ;
update related handlers (handleKeyDown, onClick) and tabIndex/aria-disabled
logic to match the chosen approach.
- Around line 113-117: The handleClick function currently queries the DOM by id
(inputId) which causes collisions when multiple DropZones share the default
'file-upload' id; replace this with a local React ref: create an inputRef via
useRef<HTMLInputElement | null>(null), attach it to the file <input> element,
and change handleClick (and its dependency array) to call
inputRef.current?.click() instead of document.getElementById(inputId); you can
then remove reliance on the default inputId or keep it only for accessibility,
ensuring each DropZone uses its own ref.
In `@services/platform/app/features/chat/hooks/use-convex-file-upload.ts`:
- Around line 94-102: The JSON parsing of the upload response can throw if the
server returns non-JSON (HTML/error page); wrap the await result.json() call in
a try/catch inside useConvexFileUpload (where result and storageId are handled)
and throw a new Error that includes the HTTP status and either the parsed JSON
error message or the raw response text; keep the existing checks for result.ok
and storageId but ensure parsing errors produce a clear error message instead of
an uncaught exception.
In
`@services/platform/app/features/customers/components/customer-import-form.tsx`:
- Around line 139-147: The FileUpload.DropZone usage (component
FileUpload.DropZone with props onFilesSelected, accept, inputId, className) is
missing an aria-label for screen readers; add an aria-label prop that uses the
shared localization key (e.g. aria.dropzone) — for example
aria-label={t('aria.dropzone')} or the app's localization helper — so the drop
zone is announced properly to assistive technologies.
- Around line 49-62: The file type check in handleFilesSelected is too fragile
(file.type.includes('sheet')) and incorrectly rejects .xls; update the
validation to use a whitelist of allowed MIME types (e.g.,
application/vnd.ms-excel,
application/vnd.openxmlformats-officedocument.spreadsheetml.sheet, text/csv)
and/or allowed extensions ('.xls', '.xlsx', '.csv') derived from file.name as a
fallback, then call setValue('file', file) when the file matches and show the
toast otherwise; reference handleFilesSelected, setValue, and the toast
invocation when making the change.
In
`@services/platform/app/features/documents/components/document-upload-dialog.tsx`:
- Around line 194-196: In DocumentUploadDialog (the span rendering {file.name}),
replace the invalid Tailwind class "wrap-anywhere" with a valid utility such as
"break-words" (or "break-all"/"overflow-wrap" variant) so the filename wraps
correctly; update the className on the span that currently reads text-sm
wrap-anywhere flex-1 min-w-0 to use text-sm break-words flex-1 min-w-0 (or your
chosen valid Tailwind utility).
In `@services/platform/app/features/products/components/product-import-form.tsx`:
- Around line 34-47: The MIME check in handleFilesSelected is fragile (e.g.,
misses .xls) — replace the inline detection with a shared utility (e.g.,
isSpreadsheetFile) that exports SPREADSHEET_TYPES and SPREADSHEET_EXTENSIONS and
checks both file.type and file.name extensions; update handleFilesSelected to
call isSpreadsheetFile(file) and keep the existing setValue('file', file) and
toast(...) behavior for invalid files so all import forms share consistent
validation.
In `@services/platform/app/features/vendors/components/vendor-import-form.tsx`:
- Around line 47-60: The file-type check in handleFilesSelected is duplicated
and fragile; create a shared utility function isSpreadsheetFile(file: File):
boolean (or in a shared utils module) that encapsulates the logic (check MIME
types, extensions like .csv, .xls, .xlsx) and export it for reuse, then replace
the inline condition in handleFilesSelected with if (isSpreadsheetFile(file)) {
setValue('file', file); } else { toast(...) } to ensure consistent validation
across vendor-import-form and other import forms.
In `@services/platform/messages/en.json`:
- Around line 67-68: The localization key "pickADate" is semantically misplaced
in the "upload" section next to "dropFilesHere"; move the "pickADate" entry out
of the upload group and into the "datePicker" section (or into a new
date-related section) so date strings are grouped together, ensure the JSON
remains valid (remove trailing commas or adjust commas accordingly) and search
for any code references to "pickADate" to confirm no path/name changes are
required after the move.
…attern - Refactor FileUpload to use Root/DropZone/Overlay compound pattern - Update chat-input, automation-assistant, document-upload-dialog - Simplify drag-and-drop handling with shared context
- Add storageId validation in use-convex-file-upload hook - Add cleanup effect to revoke object URLs on unmount - Upload valid files even when some files are invalid - Add toast notifications for unsupported file types in import forms - Add unsupportedFileType translation key
- Add PowerPoint MIME types to chat file upload config - Fix fragile MIME detection in import forms using extension checking
4f53b1c to
ef99bc0
Compare
Summary
useConvexFileUploadhook for chat file attachmentsChanges
unsupportedFileTypevalidation messageTest plan
Summary by CodeRabbit