Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new UploadInputV3 component (MUI-based) and a shared DropzoneJS wrapper to support v2/v3 upload inputs, along with dependency/build wiring for distribution.
Changes:
- Add new
UploadInputV3component + styles + tests. - Introduce shared
src/components/inputs/dropzone/implementation and updateUploadInputV2to use it. - Expose/build the new entrypoint (webpack entry + package exports) and add MUI icons dependency.
Reviewed changes
Copilot reviewed 8 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds lock entries for new dependencies (MUI icons + transitive Babel runtime). |
| package.json | Bumps version to 4.2.25-beta.1 and adds @mui/icons-material to deps/peerDeps. |
| webpack.common.js | Adds build entry for components/inputs/upload-input-v3. |
| src/components/index.js | Exports UploadInputV3 from the library root. |
| src/components/inputs/upload-input-v3/index.js | New MUI-based UploadInput v3 implementation with upload/error state UI. |
| src/components/inputs/upload-input-v3/index.less | Styles for the v3 dropzone container. |
| src/components/inputs/upload-input-v3/dropzone-v3.js | Wrapper around shared DropzoneJS to expose lifecycle callbacks. |
| src/components/inputs/upload-input-v3/tests/upload-input-v3.test.js | Enzyme tests covering render, states, and error handling. |
| src/components/inputs/upload-input-v2/index.js | Updates Dropzone import path to the new shared component. |
| src/components/inputs/dropzone/index.js | New shared DropzoneJS implementation (Dropzone.js integration + auth/md5). |
| src/components/inputs/dropzone/icon.js | Adds a simple icon component used by DropzoneJS. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📝 WalkthroughWalkthroughAdds UploadInputV3 (component, DropzoneV3 wrapper, styles, tests), updates exports and webpack entries, adjusts an import in upload-input-v2, and bumps package version with MUI icons dependency additions. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Parent as Parent Component
participant Upload as UploadInputV3
participant DZ3 as DropzoneV3
participant DZjs as DropzoneJS
User->>DZjs: Drag & drop file / select file
DZjs->>DZ3: addedfile event
DZ3->>Upload: onAddedFile(file)
Upload->>Upload: add file to uploadingFiles state
DZjs->>DZ3: uploadprogress(bytesSent, bytesTotal)
DZ3->>Upload: onUploadProgress(fileId, percentage)
Upload->>Upload: update file progress
Upload->>Parent: UI updates (progress)
alt upload success
DZjs->>DZ3: success(response)
DZ3->>Upload: onFileCompleted(file, response)
Upload->>Upload: mark complete, sync to value
Upload->>Parent: onUploadComplete callback
else upload failure
DZjs->>DZ3: error(errorMsg)
DZ3->>Upload: onFileError(file, errorMsg)
Upload->>Upload: add to errorFiles state
Upload->>Parent: display error
end
User->>Upload: Click delete on uploaded file
Upload->>DZjs: removeFile(file)
DZjs->>DZ3: removedfile event
DZ3->>Upload: onFileRemoved(file)
Upload->>Parent: onRemove callback
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/components/inputs/upload-input-v3/index.js (3)
283-286: Consider using stable keys instead of array indices.Using array index as
keycan cause subtle issues when items are removed from the middle of the list. A composite key based onfile.nameandfile.sizewould be more stable.💡 Suggested improvement
- <Box - key={`uploading-${index}`} + <Box + key={`uploading-${file.name}-${file.size}`}Apply similarly to
errorFiles(line 332) andvaluefiles (line 372).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/inputs/upload-input-v3/index.js` around lines 283 - 286, Replace unstable index-based keys in the map renderers with a stable composite key derived from the file object: where you currently have uploadingFiles.map((file, index) => <Box key={`uploading-${index}`} ...), use a stable key like `${file.name}-${file.size}` (or `${file.name}-${file.lastModified}` if available) instead; apply the same change to the errorFiles.map and the value (selected files) .map renderers so each <Box> (and any other mapped element) uses the composite file-based key rather than the array index to ensure stable identity when items are added/removed.
160-166: MissinguploadingFilesin useEffect dependency array.The effect reads
uploadingFilesbut doesn't include it in the dependency array, which is flagged by React's exhaustive-deps rule. However, since the effect filters based on currentuploadingFilesstate and only triggers onvaluechanges, this is intentional to avoid infinite loops. Consider adding a comment to clarify this decision for future maintainers.💡 Suggested clarification
+ // Intentionally omitting uploadingFiles from deps to avoid infinite loop; + // we only want to sync when the parent updates `value`. useEffect(() => { if (uploadingFiles.length === 0 || value.length === 0) return; setUploadingFiles(prev => prev.filter(f => { if (!f.complete) return true; return !value.some(v => v.filename === f.name); })); }, [value]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/inputs/upload-input-v3/index.js` around lines 160 - 166, The effect uses uploadingFiles, value, and setUploadingFiles but intentionally only depends on value to avoid a loop; add a brief inline comment above the useEffect explaining this deliberate omission and why uploadingFiles is excluded (or alternatively add an explicit eslint-disable-next-line react-hooks/exhaustive-deps with a concise rationale) so future maintainers understand that useEffect, uploadingFiles, value, and setUploadingFiles are intentionally handled this way.
114-117: Magic default value'100kb'for missing file size.Returning
'100kb'when the file size is falsy could be misleading to users. Consider displaying "Unknown" or a dash instead for clarity.💡 Alternative approach
const formatFileSize = useCallback((bytes) => { - if (!bytes) return '100kb'; + if (!bytes) return 'Unknown'; return `${Math.round(bytes / 1024)}kb`; }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/inputs/upload-input-v3/index.js` around lines 114 - 117, The formatFileSize callback currently returns a misleading magic default '100kb' when bytes is falsy; update formatFileSize to treat only null/undefined as unknown (e.g., return 'Unknown' or '—') and handle 0 explicitly (return '0kb'), otherwise compute `${Math.round(bytes / 1024)}kb`; modify the check in formatFileSize so it uses a null/undefined guard (bytes == null) instead of a falsy check to avoid treating 0 as missing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 25: DevDependency for `@mui/icons-material` is pinned to ^7.3.9 while
peerDependencies expect ^5.15.20 (incompatible with `@mui/material`@5.15.20);
update the devDependency entry for "@mui/icons-material" to ^5.15.20 so
devDependencies and peerDependencies match the installed `@mui/material` major
version, and run install/update lockfile accordingly (verify package.json
devDependencies and peerDependencies entries for "@mui/icons-material" and
"@mui/material" for consistency).
In `@src/components/inputs/upload-input-v3/dropzone-v3.js`:
- Around line 46-48: The uploadprogress handler currently computes bytesSent /
file.size * 100 which can produce Infinity/NaN when file.size is 0; update the
uploadprogress logic (the uploadprogress callback that calls onUploadProgress
and eventHandlers.uploadprogress) to guard against zero/invalid file.size by
computing a safe percentage: if file.size is a positive number use (bytesSent /
file.size * 100) (optionally clamped to 0–100), otherwise pass 0 (or a sensible
default) to onUploadProgress and still call eventHandlers.uploadprogress with
the original progress and bytesSent; ensure you check file.size is finite and >
0 before dividing.
In `@src/components/inputs/upload-input-v3/index.js`:
- Around line 62-64: getDefaultMaxSize currently divides mediaType.max_size by
1024 (KB) but the UI displays maxSize as "MB"; update getDefaultMaxSize to
return the size in megabytes (divide by 1024*1024 and round/ceil as desired) so
the displayed `${maxSize}MB` is correct (e.g., change getDefaultMaxSize to
return mediaType ? Math.round(mediaType.max_size / (1024 * 1024)) : 100). Also
ensure any other usages around the same block that expect the value (references
to maxSize in the display at lines noted) are consistent with MB units.
---
Nitpick comments:
In `@src/components/inputs/upload-input-v3/index.js`:
- Around line 283-286: Replace unstable index-based keys in the map renderers
with a stable composite key derived from the file object: where you currently
have uploadingFiles.map((file, index) => <Box key={`uploading-${index}`} ...),
use a stable key like `${file.name}-${file.size}` (or
`${file.name}-${file.lastModified}` if available) instead; apply the same change
to the errorFiles.map and the value (selected files) .map renderers so each
<Box> (and any other mapped element) uses the composite file-based key rather
than the array index to ensure stable identity when items are added/removed.
- Around line 160-166: The effect uses uploadingFiles, value, and
setUploadingFiles but intentionally only depends on value to avoid a loop; add a
brief inline comment above the useEffect explaining this deliberate omission and
why uploadingFiles is excluded (or alternatively add an explicit
eslint-disable-next-line react-hooks/exhaustive-deps with a concise rationale)
so future maintainers understand that useEffect, uploadingFiles, value, and
setUploadingFiles are intentionally handled this way.
- Around line 114-117: The formatFileSize callback currently returns a
misleading magic default '100kb' when bytes is falsy; update formatFileSize to
treat only null/undefined as unknown (e.g., return 'Unknown' or '—') and handle
0 explicitly (return '0kb'), otherwise compute `${Math.round(bytes / 1024)}kb`;
modify the check in formatFileSize so it uses a null/undefined guard (bytes ==
null) instead of a falsy check to avoid treating 0 as missing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 450cf651-53b4-4c57-bbe1-e3358ee6bf7c
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (10)
package.jsonsrc/components/index.jssrc/components/inputs/dropzone/icon.jssrc/components/inputs/dropzone/index.jssrc/components/inputs/upload-input-v2/index.jssrc/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.jssrc/components/inputs/upload-input-v3/dropzone-v3.jssrc/components/inputs/upload-input-v3/index.jssrc/components/inputs/upload-input-v3/index.lesswebpack.common.js
| "@babel/runtime": "^7.20.7", | ||
| "@emotion/react": "^11.11.4", | ||
| "@emotion/styled": "^11.11.5", | ||
| "@mui/icons-material": "^7.3.9", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the latest version of @mui/icons-materialand is it compatible with@mui/material 5.15.20?
💡 Result:
- Latest
@mui/icons-material: v7.3.9. (intel.aikido.dev) - Compatible with
@mui/material@5.15.20? No. Recent@mui/icons-materialreleases track@mui/materialas a peer dependency on the same major line (e.g., v7 icons peer on@mui/materialv7). (github.com) - What to use with
@mui/material@5.15.20: install@mui/icons-material@5.15.20(or another 5.15.x) to match. (newreleases.io)
Citations:
- 1: https://intel.aikido.dev/packages/npm/%40mui/icons-material?utm_source=openai
- 2: Extend support to @mui/material and @mui/icons-material v7 KevinVandy/material-react-table#1401
- 3: https://newreleases.io/project/npm/%40mui/icons-material/release/5.15.20?utm_source=openai
Fix @mui/icons-material version mismatch between devDependencies and peerDependencies.
The devDependency specifies ^7.3.9 but the peerDependency specifies ^5.15.20, and @mui/icons-material@7 is incompatible with @mui/material@5.15.20. Update the devDependency to ^5.15.20 to match @mui/material.
Proposed fix
- "@mui/icons-material": "^7.3.9",
+ "@mui/icons-material": "^5.15.20",Also applies to: 109-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 25, DevDependency for `@mui/icons-material` is pinned to
^7.3.9 while peerDependencies expect ^5.15.20 (incompatible with
`@mui/material`@5.15.20); update the devDependency entry for "@mui/icons-material"
to ^5.15.20 so devDependencies and peerDependencies match the installed
`@mui/material` major version, and run install/update lockfile accordingly (verify
package.json devDependencies and peerDependencies entries for
"@mui/icons-material" and "@mui/material" for consistency).
smarcet
requested changes
Mar 18, 2026
Collaborator
smarcet
left a comment
smarcet
left a comment
There was a problem hiding this comment.
@santipalenque please review comments ( marked 2 rabbit ai comments for review )
many thanks
Contributor
Author
|
done @smarcet |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/inputs/upload-input-v3/dropzone-v3.js`:
- Around line 50-56: The success and error Dropzone callback wrappers (success,
error) currently only accept specific args (file, file+message) and drop
additional Dropzone parameters; change both wrapper signatures to use rest
parameters (e.g., (...args)) and forward all arguments to the provided handlers
(call onFileCompleted and eventHandlers.success with ...args; likewise call
onFileError and eventHandlers.error with ...args) to match the existing
uploadprogress pattern and preserve the server response and extra data.
In `@src/components/inputs/upload-input-v3/index.js`:
- Around line 314-320: The IconButton instances rendering file actions (e.g.,
the delete button that calls handleDeleteUploading(file) and the other
IconButton usages at the other locations) are missing accessible names; update
each IconButton to include an aria-label that concisely describes the action and
includes the file name (for example "Delete {file.name}" for the delete button),
ensuring you use the current file variable passed into the handler so screen
readers receive the action plus filename; apply the same pattern to the other
IconButton usages at the other action spots.
- Around line 139-191: The code uses file.name and file.size as the upload
identity across handleAddedFile, handleUploadProgress, handleFileRemoved,
handleFileCompleted, handleFileError, handleDismissError, handleDeleteUploading
and the useEffect cleanup, which is unsafe; generate and persist a unique
per-upload id (e.g., uuid or incrementing id) when adding a file in
handleAddedFile, attach it to the Dropzone file object (e.g., file.uploadId) and
store that id in uploadingFiles/errorFiles instead of relying on name/size, then
update all comparisons (in handleUploadProgress, handleFileRemoved,
handleFileCompleted, handleFileError, handleDismissError, handleDeleteUploading
and the useEffect filter) to match by that upload id; also ensure the parent
update that removes uploads communicates or maps the created upload id so the
useEffect can remove completed entries by id rather than filename.
- Around line 86-88: The Dropzone event handler currently wires the parent
onRemove directly to Dropzone's removedfile, causing the parent to receive
internal cleanup removals (from removeFile) for files that were never added to
value; change eventHandlers/useMemo to register an internal removedfile handler
(e.g., handleRemovedFile) that always performs internal cleanup by calling
removeFile(file) / dismissing errors and then only calls the parent
onRemove(file) if the file exists in the component's current value (or has the
component's identifier/flag that marks it as user-added). Replace direct uses of
onRemove in eventHandlers and the other locations (the blocks around the other
noted ranges) with this internal handler to keep parent callbacks separate from
Dropzone internal removals.
- Around line 15-28: Your project is importing components from '@mui/material'
and icons from '@mui/icons-material' (e.g., UploadFileIcon, DeleteIcon,
CheckCircleIcon) but the installed versions are incompatible; resolve by
aligning versions: either downgrade '@mui/icons-material' to a v5 release
matching '@mui/material@^5.15.20' or upgrade '@mui/material' to v7 to match
'@mui/icons-material@^7.3.9'. Update package.json accordingly, run the
installer, and if you choose to upgrade material to v7, follow the MUI v7
migration steps and adjust any breaking API changes for the components/imports
used in this file (Box, Typography, IconButton, Alert, LinearProgress and the
listed icon names).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e340b7a-10b9-48b7-b761-3808c8e09f22
📒 Files selected for processing (4)
package.jsonsrc/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.jssrc/components/inputs/upload-input-v3/dropzone-v3.jssrc/components/inputs/upload-input-v3/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
Comment on lines
+50
to
+56
| success: (file) => { | ||
| if (onFileCompleted) onFileCompleted(file); | ||
| if (eventHandlers.success) eventHandlers.success(file); | ||
| }, | ||
| error: (file, message) => { | ||
| if (onFileError) onFileError(file, message); | ||
| if (eventHandlers.error) eventHandlers.error(file, message); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see the actual current implementation
cat -n src/components/inputs/upload-input-v3/dropzone-v3.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 2727
🏁 Script executed:
# Search for Dropzone documentation or references in the codebase
rg -i "dropzone" --type js -A 3 -B 3 | head -100Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 7044
🏁 Script executed:
# Check if there are handlers in the codebase that might expect additional arguments
rg -i "onFileCompleted|onFileError" --type js -A 3 -B 3Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 6175
🏁 Script executed:
# Find the DropzoneJS component
find . -name "dropzone*" -type f | grep -v node_modules | grep -v testRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 138
🏁 Script executed:
# Check the dropzone component implementation
cat -n src/components/inputs/dropzone/index.js | head -150Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 6301
🌐 Web query:
Dropzone.js success callback signature parameters documentation
💡 Result:
In Dropzone.js, the success callback/event handler is invoked with three arguments:
success(file, responseText, e) { ... }file: the uploaded File objectresponseText(often calledresponse): the server response (string, or parsed JSON whenContent-Type: application/json)e: an extra value passed through from the XHR handler (e.g., a JSON-parse error object in the invalid-JSON case) [1]
This matches the common usage shown in the Dropzone FAQ:
this.on("success", function(file, responseText) { ... })[2]
Related: when uploadMultiple: true, successmultiple(files, responseText, e) is emitted once for the batch [1].
Sources:
[1] Dropzone source (_finished(files, responseText, e) emits success) https://raw.githubusercontent.com/dropzone/dropzone/refs/heads/main/src/dropzone.js
[2] Dropzone FAQ (example success handler) https://github.com/dropzone/dropzone/wiki/faq
🏁 Script executed:
# Verify the uploadprogress handler uses multiple args as reference
sed -n '46,49p' src/components/inputs/upload-input-v3/dropzone-v3.jsRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 341
Forward all Dropzone callback arguments to event handlers in success and error wrappers.
Dropzone's success callback receives three arguments: file, responseText (server response), and an extra error object. The current wrapper only accepts file and discards the response, making this component lossy for callers that need the server payload. Use rest parameters ...args to forward all arguments, matching the pattern already used in uploadprogress (line 46).
🐛 Proposed fix
success: (file) => {
if (onFileCompleted) onFileCompleted(file);
if (eventHandlers.success) eventHandlers.success(file);
},
error: (file, message) => {
if (onFileError) onFileError(file, message);
if (eventHandlers.error) eventHandlers.error(file, message);
},
+ success: (...args) => {
+ if (onFileCompleted) onFileCompleted(...args);
+ if (eventHandlers.success) eventHandlers.success(...args);
+ },
+ error: (...args) => {
+ if (onFileError) onFileError(...args);
+ if (eventHandlers.error) eventHandlers.error(...args);
+ },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/upload-input-v3/dropzone-v3.js` around lines 50 - 56,
The success and error Dropzone callback wrappers (success, error) currently only
accept specific args (file, file+message) and drop additional Dropzone
parameters; change both wrapper signatures to use rest parameters (e.g.,
(...args)) and forward all arguments to the provided handlers (call
onFileCompleted and eventHandlers.success with ...args; likewise call
onFileError and eventHandlers.error with ...args) to match the existing
uploadprogress pattern and preserve the server response and extra data.
Comment on lines
+15
to
+28
| import { | ||
| Box, | ||
| Typography, | ||
| IconButton, | ||
| Alert, | ||
| LinearProgress, | ||
| } from '@mui/material'; | ||
| import { | ||
| UploadFile as UploadFileIcon, | ||
| Delete as DeleteIcon, | ||
| CheckCircle as CheckCircleIcon, | ||
| ErrorOutline as ErrorOutlineIcon, | ||
| Close as CloseIcon, | ||
| } from '@mui/icons-material'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find package manifest files
find . -maxdepth 2 -type f \( -name "package.json" -o -name "package-lock.json" -o -name "yarn.lock" \)Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 110
🏁 Script executed:
# Read package.json to check MUI dependencies
cat package.json | grep -A 50 '"dependencies"' | grep -E '@mui/(material|icons-material)'Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 66
🏁 Script executed:
# Read package.json and extract MUI versions
cat package.json | grep -E '"@mui/(material|icons-material)"'Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 225
🏁 Script executed:
# Read the full package.json to understand dependency sections
cat -n package.json | head -100Repository: OpenStackweb/openstack-uicore-foundation
Length of output: 4069
🏁 Script executed:
# Check if there's a dependencies section after devDependencies
tail -50 package.jsonRepository: OpenStackweb/openstack-uicore-foundation
Length of output: 1329
🌐 Web query:
MUI @mui/icons-materialv7 compatibility with@mui/material v5 requirements
💡 Result:
@mui/icons-material v7 is not compatible with @mui/material v5 under normal npm/yarn/pnpm dependency resolution, because @mui/icons-material@7.x declares a peer dependency on @mui/material@^7.x (example shown for @mui/icons-material@7.0.1 → peer @mui/material@^7.0.1). [1]
What to do:
- If you must stay on Material UI v5, install matching icons v5:
npm i@mui/material@^5@mui/icons-material@^5
- If you want icons v7, you need to upgrade
@mui/materialto v7 (and follow the v7 upgrade guide). [2]
You can bypass the peer-dep conflict with --force / --legacy-peer-deps, but npm explicitly warns that this may produce a broken install. [1]
Sources: [1], [2]
Downgrade @mui/icons-material to v5 or upgrade @mui/material to v7—versions are incompatible.
The dependency configuration has @mui/icons-material@^7.3.9 paired with @mui/material@^5.15.20. Material-UI v7 icons declares a peer dependency on @mui/material@^7.x, making this combination unsupported. Choose one path:
- Downgrade icons to
@mui/icons-material@^5.15.20(keep v5 stack), or - Upgrade material to
@mui/material@^7.x(adopt v7 stack and follow the migration guide).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/upload-input-v3/index.js` around lines 15 - 28, Your
project is importing components from '@mui/material' and icons from
'@mui/icons-material' (e.g., UploadFileIcon, DeleteIcon, CheckCircleIcon) but
the installed versions are incompatible; resolve by aligning versions: either
downgrade '@mui/icons-material' to a v5 release matching
'@mui/material@^5.15.20' or upgrade '@mui/material' to v7 to match
'@mui/icons-material@^7.3.9'. Update package.json accordingly, run the
installer, and if you choose to upgrade material to v7, follow the MUI v7
migration steps and adjust any breaking API changes for the components/imports
used in this file (Box, Typography, IconButton, Alert, LinearProgress and the
listed icon names).
Comment on lines
+86
to
+88
| const eventHandlers = useMemo(() => { | ||
| return onRemove ? { removedfile: onRemove } : {}; | ||
| }, [onRemove]); |
There was a problem hiding this comment.
Keep parent onRemove separate from Dropzone's removedfile event.
removeFile() is used here for canceling uploads and dismissing errors. Because onRemove is also wired into Dropzone's removedfile, those internal actions will call the parent with a Dropzone file that was never added to value.
🐛 Proposed fix
- const eventHandlers = useMemo(() => {
- return onRemove ? { removedfile: onRemove } : {};
- }, [onRemove]);
...
- eventHandlers={eventHandlers}Also applies to: 174-192, 217-221
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/upload-input-v3/index.js` around lines 86 - 88, The
Dropzone event handler currently wires the parent onRemove directly to
Dropzone's removedfile, causing the parent to receive internal cleanup removals
(from removeFile) for files that were never added to value; change
eventHandlers/useMemo to register an internal removedfile handler (e.g.,
handleRemovedFile) that always performs internal cleanup by calling
removeFile(file) / dismissing errors and then only calls the parent
onRemove(file) if the file exists in the component's current value (or has the
component's identifier/flag that marks it as user-added). Replace direct uses of
onRemove in eventHandlers and the other locations (the blocks around the other
noted ranges) with this internal handler to keep parent callbacks separate from
Dropzone internal removals.
Comment on lines
+139
to
+191
| const handleAddedFile = useCallback((file) => { | ||
| setUploadingFiles(prev => [...prev, { name: file.name, size: file.size, progress: 0, complete: false }]); | ||
| }, []); | ||
|
|
||
| const handleUploadProgress = useCallback((file, progress) => { | ||
| setUploadingFiles(prev => prev.map(f => | ||
| f.name === file.name && f.size === file.size ? { ...f, progress } : f | ||
| )); | ||
| }, []); | ||
|
|
||
| const handleFileRemoved = useCallback((file) => { | ||
| setUploadingFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); | ||
| }, []); | ||
|
|
||
| // Mark as complete instead of removing — keep it visible until value is updated by the parent | ||
| const handleFileCompleted = useCallback((file) => { | ||
| setUploadingFiles(prev => prev.map(f => | ||
| f.name === file.name && f.size === file.size ? { ...f, progress: 100, complete: true } : f | ||
| )); | ||
| }, []); | ||
|
|
||
| // Once the parent updates value, remove the matching completed file from uploadingFiles | ||
| useEffect(() => { | ||
| if (uploadingFiles.length === 0 || value.length === 0) return; | ||
| setUploadingFiles(prev => prev.filter(f => { | ||
| if (!f.complete) return true; | ||
| return !value.some(v => v.filename === f.name); | ||
| })); | ||
| }, [value]); | ||
|
|
||
| const handleFileError = useCallback((file, message) => { | ||
| setUploadingFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); | ||
| setErrorFiles(prev => [...prev, { name: file.name, size: file.size, message }]); | ||
| }, []); | ||
|
|
||
| const handleDismissError = useCallback((file) => { | ||
| if (dropzoneInstanceRef.current) { | ||
| const dzFile = dropzoneInstanceRef.current.files?.find( | ||
| f => f.name === file.name && f.size === file.size | ||
| ); | ||
| if (dzFile) dropzoneInstanceRef.current.removeFile(dzFile); | ||
| } | ||
| setErrorFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); | ||
| }, []); | ||
|
|
||
| const handleDeleteUploading = useCallback((file) => { | ||
| if (dropzoneInstanceRef.current) { | ||
| const dzFile = dropzoneInstanceRef.current.files?.find( | ||
| f => f.name === file.name && f.size === file.size | ||
| ); | ||
| if (dzFile) dropzoneInstanceRef.current.removeFile(dzFile); | ||
| } | ||
| setUploadingFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size))); |
There was a problem hiding this comment.
name + size is not a unique upload identity.
If the same file is added twice, or two files share the same name and size, every state transition here can hit both rows. The reconciliation effect is even weaker because it removes completed rows by filename only. Persist a real per-upload key instead of matching on display fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/upload-input-v3/index.js` around lines 139 - 191, The
code uses file.name and file.size as the upload identity across handleAddedFile,
handleUploadProgress, handleFileRemoved, handleFileCompleted, handleFileError,
handleDismissError, handleDeleteUploading and the useEffect cleanup, which is
unsafe; generate and persist a unique per-upload id (e.g., uuid or incrementing
id) when adding a file in handleAddedFile, attach it to the Dropzone file object
(e.g., file.uploadId) and store that id in uploadingFiles/errorFiles instead of
relying on name/size, then update all comparisons (in handleUploadProgress,
handleFileRemoved, handleFileCompleted, handleFileError, handleDismissError,
handleDeleteUploading and the useEffect filter) to match by that upload id; also
ensure the parent update that removes uploads communicates or maps the created
upload id so the useEffect can remove completed entries by id rather than
filename.
Comment on lines
+314
to
+320
| <IconButton | ||
| size="small" | ||
| onClick={() => handleDeleteUploading(file)} | ||
| sx={{ color: 'text.secondary', '&:hover': { color: 'error.main' } }} | ||
| > | ||
| <DeleteIcon /> | ||
| </IconButton> |
There was a problem hiding this comment.
Add accessible names to the icon-only buttons.
All three actions render as unlabeled IconButtons, so assistive tech only gets unnamed buttons. Please add an aria-label that includes the action and filename.
♿ Proposed fix
<IconButton
size="small"
onClick={() => handleDeleteUploading(file)}
+ aria-label={`Cancel upload for ${file.name}`}
sx={{ color: 'text.secondary', '&:hover': { color: 'error.main' } }}
>
<DeleteIcon />
</IconButton>
...
<IconButton
size="small"
onClick={() => handleDismissError(file)}
+ aria-label={`Dismiss error for ${file.name}`}
sx={{ color: 'error.main' }}
>
<CloseIcon fontSize="small" />
</IconButton>
...
<IconButton
size="small"
onClick={handleRemove(file)}
+ aria-label={`Delete ${filename}`}
sx={{ color: 'text.secondary', '&:hover': { color: 'error.main' } }}
>
<DeleteIcon />
</IconButton>Also applies to: 354-360, 396-402
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/inputs/upload-input-v3/index.js` around lines 314 - 320, The
IconButton instances rendering file actions (e.g., the delete button that calls
handleDeleteUploading(file) and the other IconButton usages at the other
locations) are missing accessible names; update each IconButton to include an
aria-label that concisely describes the action and includes the file name (for
example "Delete {file.name}" for the delete button), ensuring you use the
current file variable passed into the handler so screen readers receive the
action plus filename; apply the same pattern to the other IconButton usages at
the other action spots.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://app.clickup.com/t/86b8uj5dx
New UploadInputV3
version 4.2.25-beta.1
Summary by CodeRabbit
New Features
Style
Tests
Chores