feat(dropzone): create dropzone component#1720
Open
AlexDev404 wants to merge 5 commits intounovue:devfrom
Open
feat(dropzone): create dropzone component#1720AlexDev404 wants to merge 5 commits intounovue:devfrom
AlexDev404 wants to merge 5 commits intounovue:devfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a complete Dropzone feature: a typed composable (useDropzoneUpload), a set of context-driven Vue UI components (provider, area, trigger, file list/items, messages, remove/retry, progress), demo pages, docs, registry entries, and a runtime dependency on Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User"
participant Trigger as "DropzoneTrigger"
participant Composable as "useDropzoneUpload"
participant Uploader as "AsyncUploader"
participant FileItem as "DropzoneFileListItem"
participant UI as "UI (Message/List)"
User->>Trigger: drop/select files
Trigger->>Composable: submit files (getInputProps)
Composable->>Composable: validate files (type/size/count)
alt validation fails
Composable->>UI: set rootError -> display
else validation passes
Composable->>FileItem: create FileStatus (pending)
Composable->>Uploader: call onDropFile(file)
Uploader-->>Composable: {status: success|error, result|error}
Composable->>FileItem: update fileStatus
FileItem->>UI: render progress/thumbnail/message
alt file error
User->>FileItem: click retry
FileItem->>Composable: onRetry -> re-call Uploader
end
User->>FileItem: click remove
FileItem->>Composable: onRemoveFile -> remove status
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. 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: 10
🧹 Nitpick comments (6)
apps/v4/components/demo/DropzoneMultiFile.vue (1)
17-33: Make the mock upload flow deterministic.Random delay and random failure make this docs demo flaky and make the retry state hard to reproduce on demand. A fixed rule or explicit toggle would showcase the same states without occasionally making the example look broken.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/components/demo/DropzoneMultiFile.vue` around lines 17 - 33, The demo's onDropFile handler in useDropzoneUpload uses Math.random for delay and failure which makes the demo flaky; replace the random behavior with deterministic logic (e.g., fixed timeout like 1000ms and a deterministic failure rule such as failing when the file name contains a specific token, when file.size === 0, or driven by a new prop/toggle like forceFail/retryScenario) so retry and error states are reproducible; update the DropzoneMultiFile.vue example to accept the toggle or deterministic rule and use that inside the onDropFile implementation to return the same status/result consistently.apps/v4/public/r/styles/new-york-v4/dropzone.json (1)
67-71: Consider scoping the animation styles in InfiniteProgress.The
<style>block inInfiniteProgress.vueis not scoped, meaning the.animate-infinite-progressclass and@keyframes infinite-progresswill be global. While this likely won't cause conflicts in most cases, consider addingscopedto prevent potential style collisions if another library or component defines the same animation name.♻️ Suggested change
In the InfiniteProgress.vue content, change:
-<style> +<style scoped> `@keyframes` infinite-progress {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/public/r/styles/new-york-v4/dropzone.json` around lines 67 - 71, The style rules in InfiniteProgress.vue (the .animate-infinite-progress class and `@keyframes` infinite-progress) are global; scope them to the component by adding the scoped attribute to the <style> tag (i.e., <style scoped>) and, if you want extra safety, rename the keyframe and class to a component-prefixed name (e.g., `@keyframes` infinite-progress-infinite and .animate-infinite-progress-infinite) and update references in the template and CSS accordingly so the animation only affects this component.apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts (4)
63-73: Consider improving fallback error messages.The fallback messages
"infinite?"and"negative?"(lines 66, 72) could be confusing to users if they ever appear. These edge cases should be rare (limits undefined when error triggered), but clearer fallbacks would improve debugging.♻️ Suggested improvement
case "file-too-large": { const maxMb = limits.maxSize ? (limits.maxSize / (1024 * 1024)).toFixed(2) - : "infinite?" + : "unknown" return `max size is ${maxMb}MB` } case "file-too-small": { const roundedMinSize = limits.minSize ? (limits.minSize / (1024 * 1024)).toFixed(2) - : "negative?" + : "unknown" return `min size is ${roundedMinSize}MB` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 63 - 73, The fallback error messages for file size in useDropzoneUpload (the switch cases referencing limits.maxSize and limits.minSize) are unclear; update the fallbacks to user-friendly, unambiguous text (e.g., use "unlimited" or "not specified" instead of "infinite?" and "negative?") so the returned strings become `max size is <value>MB` or `max size is unlimited` and `min size is <value>MB` or `min size is not specified`; locate the cases for "file-too-large" and "file-too-small" in the function and replace the fallback literals accordingly while keeping the same rounding logic for numeric limits.
154-157: crypto.randomUUID() requires modern browser/Node.js.
crypto.randomUUID()is used for generating unique IDs. This API requires:
- Browsers: Chrome 92+, Firefox 95+, Safari 15.4+
- Node.js: 19+ (or 14.17+ with
--experimental-global-webcrypto)- Secure context (HTTPS) in browsers
This should be fine for most modern Vue 3 projects, but worth noting in documentation if targeting older environments.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 154 - 157, The code uses crypto.randomUUID() to build inputId/rootMessageId/rootDescriptionId which may not exist in older browsers or Node versions; replace that direct call with a cross-environment helper (e.g., a new getUniqueId() or useDropzoneUniqueId()) that first tries crypto.randomUUID(), falls back to crypto.getRandomValues-based UUID or a lightweight UUID library (like uuid.v4), and returns the short slice used now; update the inputId assignment in useDropzoneUpload to call this helper so inputId, rootMessageId, and rootDescriptionId remain stable across environments.
233-239: Clarify behavior: unlimited retries when maxRetryCount is undefined.When
maxRetryCountis undefined,canRetrywill always returntruefor errored files (sincetries < Infinityis always true). This allows unlimited manual retries, which may be intended behavior but could surprise users expecting some default limit.Consider documenting this or providing a reasonable default (e.g., 3 retries).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 233 - 239, The canRetry function currently treats undefined maxRetryCount as Infinity, allowing unlimited retries; change it to use a sensible default (e.g., 3) by computing an effective max like const effectiveMax = maxRetryCount ?? 3 and then return fileStatus?.status === "error" && fileStatus.tries < effectiveMax; update any related docs/comments if you prefer to keep the unlimited behavior instead of the default.
176-226: Consider extracting array update logic to reduce duplication.The pattern of finding an index, slicing, and reconstructing the array is repeated multiple times (lines 185-189, 201-205, 217-221, 254-258). This is functionally correct but verbose and error-prone.
♻️ Suggested helper function
const updateFileStatus = ( id: string, updater: (file: FileStatus<TUploadRes, TUploadError>) => Partial<FileStatus<TUploadRes, TUploadError>> ) => { const index = fileStatuses.value.findIndex(f => f.id === id) const currentFile = fileStatuses.value[index] if (index !== -1 && currentFile) { fileStatuses.value = [ ...fileStatuses.value.slice(0, index), { ...currentFile, ...updater(currentFile) }, ...fileStatuses.value.slice(index + 1), ] as FileStatus<TUploadRes, TUploadError>[] } } // Usage: updateFileStatus(id, file => ({ status: "pending", tries: file.tries + 1 }))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 176 - 226, The repeated find-slice-rebuild logic in uploadFile should be extracted into a single helper (e.g., updateFileStatus) that takes the id and an updater function and performs the index lookup and array replacement on fileStatuses.value; replace the four duplicated blocks inside uploadFile with calls to that helper (use it to set status: "pending" and increment tries, set status: "error" and attach shapedError, and set status: "success" and attach result), preserving existing uses of pShapeUploadError, pOnFileUploadError and pOnFileUploaded and keeping the helper's return/type signature generic to FileStatus<TUploadRes, TUploadError>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/v4/components/demo/DropzoneMultiImage.vue`:
- Around line 79-89: The remove button (DropzoneRemoveFile) is hidden by default
with opacity-0 and only shown on hover, which prevents keyboard and touch users
from seeing it; update the element’s class list on DropzoneRemoveFile to also
reveal it on keyboard focus and in non-hover/touch contexts by adding
focus:opacity-100 and focus-visible:opacity-100 (and/or
focus-within:opacity-100) and consider enabling a persistent visible state for
touch by using an appropriate responsive utility (e.g., sm:opacity-100 or a
touch media query class) while keeping transition-opacity and the positioning
classes intact so keyboard, screen-reader and touch users can discover and
activate the control.
- Around line 14-27: The demo currently creates preview blob URLs in the
onDropFile callback using URL.createObjectURL(file) (inside useDropzoneUpload ->
dropzone) but never revokes them; modify the flow so every created blob URL is
revoked when its file is removed. Concretely, keep returning the blob URL from
onDropFile (the current result value) but also register a cleanup that calls
URL.revokeObjectURL(blobUrl) when that item is removed (e.g., implement an
onRemoveFile / cleanup handler on the dropzone instance or attach a disposer to
the returned upload result), ensuring you reference the
dropzone/useDropzoneUpload instance and the exact blobUrl created by
URL.createObjectURL(file) so no preview URLs leak.
In `@apps/v4/components/demo/DropzoneSingleFile.vue`:
- Around line 13-27: The preview blob URL created in the onDropFile handler
(result: URL.createObjectURL(file)) is never revoked, causing memory leaks when
shiftOnMaxFiles replaces the avatar; store the current preview URL in a
component-scoped variable or ref and when creating a new preview call
URL.revokeObjectURL(oldUrl) before overwriting it, and also revoke the last
preview inside the component unmount hook (e.g., beforeUnmount/unmounted) to
ensure cleanup; update the useDropzoneUpload onDropFile flow and add the unmount
cleanup to the component lifecycle.
In `@apps/v4/content/docs/components/dropzone.md`:
- Around line 344-348: The documentation snippet's validation settings don't
match the live demo (DropzoneMultiImage component); update the validation object
in the "Multiple Images Upload" snippet (and the duplicate occurrence later) to
mirror the demo: set maxFiles to 4 and maxSize to 5 * 1024 * 1024 (5MB), keeping
the accept types unchanged so the example behavior matches the live
DropzoneMultiImage.vue preview.
In `@apps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileMessage.vue`:
- Around line 18-25: The template always applies the destructive text class;
update the class binding on the <p> so 'text-destructive' is only included when
context.fileStatus.value.status === 'error' (e.g., modify the cn(...) call used
on the <p> to conditionally add 'text-destructive' based on
context.fileStatus.value.status), ensuring other classes ('h-5',
'text-[0.8rem]', 'font-medium') and props.class remain unchanged; locate this in
DropzoneFileMessage.vue by the <p> with :id="context.messageId" and the cn(...)
class binding.
In `@apps/v4/registry/new-york-v4/ui/dropzone/DropzoneMessage.vue`:
- Around line 18-26: The paragraph showing validation feedback (the <p> that
uses :id="context.rootMessageId" and displays context.rootError.value or slot)
must be made a live region so screen readers announce new errors; add ARIA
attributes such as aria-live="polite" and aria-atomic="true" (or role="status")
to that <p> element so updates to context.rootError are announced as they
change.
In `@apps/v4/registry/new-york-v4/ui/dropzone/DropzoneRetryFile.vue`:
- Around line 24-31: Replace the current aria-only disabling with a real
disabled state: bind the native disabled attribute on the Button to
!context.canRetry.value (e.g., :disabled="!context.canRetry.value"), keep
aria-disabled for accessibility (aria-disabled="!context.canRetry.value"), and
remove any reliance on CSS-only blocking like
'aria-disabled:pointer-events-none'; also ensure the click handler
(context.onRetry) is not invoked when disabled by either relying on the native
disabled behavior or wrapping the `@click` handler to no-op when
context.canRetry.value is false.
In `@apps/v4/registry/new-york-v4/ui/dropzone/DropzoneTrigger.vue`:
- Around line 31-33: The computed ariaDescribedBy currently omits
context.rootDescriptionId and when invalid replaces the description with message
IDs; update the ariaDescribedBy computed (the const ariaDescribedBy) to always
include context.rootDescriptionId (if present) and when context.isInvalid.value
is true append context.rootMessageId and fileMessageIds.value to any existing
description instead of replacing it so DropzoneDescription is announced along
with error messages; reference the computed ariaDescribedBy,
context.rootDescriptionId, context.rootMessageId, and fileMessageIds in your
change.
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 285-301: The concurrent map over slicedNewFiles causes race
conditions and wrong removals because each async iteration reads fileCount and
uses the loop index to pick existingFile; instead, in useDropzoneUpload compute
how many existing files must be removed up-front (e.g. removalsNeeded = max(0,
fileCount + slicedNewFiles.length - maxNewFiles)) and remove that many oldest
files by calling onRemoveFile sequentially (await each) before adding new items,
or process slicedNewFiles sequentially with a for..of that checks current
fileStatuses.value length and awaits onRemoveFile and uploadFile per iteration;
avoid using the captured index to reference fileStatuses.value inside the
concurrent Promise.all branch (refer to onDropFilePromises, slicedNewFiles,
onRemoveFile, fileStatuses.value, uploadFile, shiftOnMaxFiles, maxNewFiles,
fileCount).
In `@apps/v4/registry/registry-ui.ts`:
- Around line 843-848: Update the source registry entry for the UI component
with name "dropzone" (the object containing name: "dropzone", type:
"registry:ui", registryDependencies) to include a dependencies array that lists
"vue3-dropzone" so the source manifest matches the published manifest; ensure
the same dependency string used by the import in useDropzoneUpload.ts is added
to the dependencies array to provide correct install metadata.
---
Nitpick comments:
In `@apps/v4/components/demo/DropzoneMultiFile.vue`:
- Around line 17-33: The demo's onDropFile handler in useDropzoneUpload uses
Math.random for delay and failure which makes the demo flaky; replace the random
behavior with deterministic logic (e.g., fixed timeout like 1000ms and a
deterministic failure rule such as failing when the file name contains a
specific token, when file.size === 0, or driven by a new prop/toggle like
forceFail/retryScenario) so retry and error states are reproducible; update the
DropzoneMultiFile.vue example to accept the toggle or deterministic rule and use
that inside the onDropFile implementation to return the same status/result
consistently.
In `@apps/v4/public/r/styles/new-york-v4/dropzone.json`:
- Around line 67-71: The style rules in InfiniteProgress.vue (the
.animate-infinite-progress class and `@keyframes` infinite-progress) are global;
scope them to the component by adding the scoped attribute to the <style> tag
(i.e., <style scoped>) and, if you want extra safety, rename the keyframe and
class to a component-prefixed name (e.g., `@keyframes` infinite-progress-infinite
and .animate-infinite-progress-infinite) and update references in the template
and CSS accordingly so the animation only affects this component.
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 63-73: The fallback error messages for file size in
useDropzoneUpload (the switch cases referencing limits.maxSize and
limits.minSize) are unclear; update the fallbacks to user-friendly, unambiguous
text (e.g., use "unlimited" or "not specified" instead of "infinite?" and
"negative?") so the returned strings become `max size is <value>MB` or `max size
is unlimited` and `min size is <value>MB` or `min size is not specified`; locate
the cases for "file-too-large" and "file-too-small" in the function and replace
the fallback literals accordingly while keeping the same rounding logic for
numeric limits.
- Around line 154-157: The code uses crypto.randomUUID() to build
inputId/rootMessageId/rootDescriptionId which may not exist in older browsers or
Node versions; replace that direct call with a cross-environment helper (e.g., a
new getUniqueId() or useDropzoneUniqueId()) that first tries
crypto.randomUUID(), falls back to crypto.getRandomValues-based UUID or a
lightweight UUID library (like uuid.v4), and returns the short slice used now;
update the inputId assignment in useDropzoneUpload to call this helper so
inputId, rootMessageId, and rootDescriptionId remain stable across environments.
- Around line 233-239: The canRetry function currently treats undefined
maxRetryCount as Infinity, allowing unlimited retries; change it to use a
sensible default (e.g., 3) by computing an effective max like const effectiveMax
= maxRetryCount ?? 3 and then return fileStatus?.status === "error" &&
fileStatus.tries < effectiveMax; update any related docs/comments if you prefer
to keep the unlimited behavior instead of the default.
- Around line 176-226: The repeated find-slice-rebuild logic in uploadFile
should be extracted into a single helper (e.g., updateFileStatus) that takes the
id and an updater function and performs the index lookup and array replacement
on fileStatuses.value; replace the four duplicated blocks inside uploadFile with
calls to that helper (use it to set status: "pending" and increment tries, set
status: "error" and attach shapedError, and set status: "success" and attach
result), preserving existing uses of pShapeUploadError, pOnFileUploadError and
pOnFileUploaded and keeping the helper's return/type signature generic to
FileStatus<TUploadRes, TUploadError>.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1ead36aa-6166-4a84-9d66-787cc173dbd5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
apps/v4/components/demo/DropzoneDemo.vueapps/v4/components/demo/DropzoneMultiFile.vueapps/v4/components/demo/DropzoneMultiImage.vueapps/v4/components/demo/DropzoneSingleFile.vueapps/v4/components/demo/index.tsapps/v4/content/docs/components/dropzone.mdapps/v4/package.jsonapps/v4/public/r/index.jsonapps/v4/public/r/styles/new-york-v4/dropzone.jsonapps/v4/public/r/styles/new-york-v4/registry.jsonapps/v4/registry/__index__.tsapps/v4/registry/new-york-v4/ui/dropzone/Dropzone.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneArea.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneDescription.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileList.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileListItem.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileMessage.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneMessage.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneRemoveFile.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneRetryFile.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneTrigger.vueapps/v4/registry/new-york-v4/ui/dropzone/InfiniteProgress.vueapps/v4/registry/new-york-v4/ui/dropzone/index.tsapps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.tsapps/v4/registry/registry-ui.ts
…ng and validation
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
apps/v4/content/docs/components/dropzone.md (1)
208-238:⚠️ Potential issue | 🟡 MinorSync the “Multiple Files Upload” snippet with the live demo.
The preview for
DropzoneMultiFileenforces 4 files at 5MB, but this snippet still documents 10 files and omits the size cap. Anyone copying the example gets different behavior than the demo shown on the page.Suggested doc fix
validation: { - maxFiles: 10, + maxFiles: 4, + maxSize: 5 * 1024 * 1024, }, @@ - Please select up to 10 files + Please select up to 4 files (max 5MB)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/content/docs/components/dropzone.md` around lines 208 - 238, The example snippet shown uses useDropzoneUpload and currently sets validation.maxFiles = 10 and omits maxSize, which is out of sync with the live DropzoneMultiFile demo; update the useDropzoneUpload call to set validation.maxFiles to 4 and add validation.maxSize to 5 * 1024 * 1024 (5MB), and update the visible text inside DropzoneDescription to reflect "Please select up to 4 files (max 5MB each)" so the documented behavior (useDropzoneUpload, DropzoneDescription, and the DropzoneMultiFile preview) matches the live demo.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/v4/content/docs/components/dropzone.md`:
- Around line 429-441: The docs table incorrectly lists maxRetryCount default as
Infinity; update the Option row for `maxRetryCount` to show the actual default
used by the implementation (change from `Infinity` to `3`) and optionally note
that `useDropzoneUpload` supplies this fallback; ensure the `maxRetryCount`
description references the `useDropzoneUpload` fallback behavior so readers know
the composable defaults to 3 when not provided.
- Around line 38-55: The manual install instructions for the dropzone are
missing the required "button" component, causing unresolved imports for
DropzoneRemoveFile and DropzoneRetryFile; update the manual steps to either (a)
instruct the user to also install/copy the "button" component (or its package)
referenced by the dropzone registry entry, or (b) show the exact imports to
replace for DropzoneRemoveFile and DropzoneRetryFile with local equivalents, and
include a note to update import paths for the "button" component so the
dropzone's dependencies resolve correctly.
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 245-251: canRetry currently treats fileStatus.tries (which starts
at 1 on the initial upload) as a zero-based retry count, so with maxRetryCount =
3 users only get two manual retries; update the logic in the canRetry function
(and the similar check around line ~319) to compare using <= effectiveMax (or
otherwise compute remaining retries as effectiveMax - fileStatus.tries + 1) so
that a tries value equal to maxRetryCount still allows the final allowed retry
while preserving the "status === 'error'" check on fileStatuses.value.
- Around line 187-223: uploadFile currently assumes pOnDropFile resolves to a
result object, so if pOnDropFile throws the file remains pending and the batch
halts; wrap the await pOnDropFile(file) call in a try/catch inside uploadFile,
and in the catch treat the thrown exception the same as result.status ===
"error": compute effectiveMax and handle autoRetry by updating fileStatuses
(find by id, set status to "pending" and increment tries) and recursively retry,
otherwise shape the error via pShapeUploadError (if provided), update
fileStatuses entry to status "error" with the shaped error, and invoke
pOnFileUploadError(resultError) if defined — use the same index/currentFile
lookup and update logic as the existing error path so behavior is identical for
thrown exceptions.
- Around line 311-326: The pOnAllUploaded callback is called unconditionally
after iterating slicedNewFiles; change it to only run when the batch had at
least one file and all files finished with status "success". While creating
newFileStatus inside the loop (symbols: slicedNewFiles, newFileStatus, id,
fileStatuses, uploadFile), collect the generated ids into a local array (e.g.,
batchIds), await uploadFile for each as you already do, then after the loop
check that batchIds.length > 0 and batchIds.every(id =>
fileStatuses.value.find(f => f.id === id)?.status === "success") before invoking
pOnAllUploaded().
---
Duplicate comments:
In `@apps/v4/content/docs/components/dropzone.md`:
- Around line 208-238: The example snippet shown uses useDropzoneUpload and
currently sets validation.maxFiles = 10 and omits maxSize, which is out of sync
with the live DropzoneMultiFile demo; update the useDropzoneUpload call to set
validation.maxFiles to 4 and add validation.maxSize to 5 * 1024 * 1024 (5MB),
and update the visible text inside DropzoneDescription to reflect "Please select
up to 4 files (max 5MB each)" so the documented behavior (useDropzoneUpload,
DropzoneDescription, and the DropzoneMultiFile preview) matches the live demo.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8a72b49-fe2b-4520-be8b-f26bbfad91c4
📒 Files selected for processing (11)
apps/v4/components/demo/DropzoneMultiFile.vueapps/v4/components/demo/DropzoneMultiImage.vueapps/v4/components/demo/DropzoneSingleFile.vueapps/v4/content/docs/components/dropzone.mdapps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileMessage.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneMessage.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneRetryFile.vueapps/v4/registry/new-york-v4/ui/dropzone/DropzoneTrigger.vueapps/v4/registry/new-york-v4/ui/dropzone/InfiniteProgress.vueapps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.tsapps/v4/registry/registry-ui.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/v4/registry/new-york-v4/ui/dropzone/DropzoneFileMessage.vue
- apps/v4/registry/new-york-v4/ui/dropzone/DropzoneRetryFile.vue
- apps/v4/registry/new-york-v4/ui/dropzone/InfiniteProgress.vue
- apps/v4/registry/new-york-v4/ui/dropzone/DropzoneMessage.vue
- apps/v4/components/demo/DropzoneSingleFile.vue
…andling in useDropzoneUpload
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/v4/content/docs/components/dropzone.md (1)
137-142: Revoke preview object URLs in these examples.Both snippets create blob URLs but never release them. Replacing/removing images repeatedly will keep those blobs alive until page unload, so the examples are teaching a memory leak. Consider revoking previous URLs on file removal/replacement and in
onUnmounted.Also applies to: 337-342
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/content/docs/components/dropzone.md` around lines 137 - 142, The example's onDropFile handler uses URL.createObjectURL(file) but never revokes blob URLs; update the component to track the current preview URL (created in onDropFile) and call URL.revokeObjectURL(prevUrl) before replacing it and when removing the file, and also revoke the current URL inside onUnmounted; specifically modify the onDropFile flow and add cleanup logic (tracking a previewUrl variable and revoking it) and an onUnmounted cleanup call so URLs are released.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 214-229: The callbacks pOnFileUploadError and pOnFileUploaded are
being invoked even when the file was removed, so first re-check that the file
still exists before calling callbacks: locate the block that computes index via
fileStatuses.value.findIndex(f => f.id === id) and the variable currentFile, and
wrap the calls to pOnFileUploadError and pOnFileUploaded in the same conditional
that verifies index !== -1 && currentFile (or perform a fresh existence check
via findIndex) so callbacks are skipped for files that no longer exist; apply
the same guard to the analogous block around lines 233-245.
- Around line 303-316: When shiftOnMaxFiles is true the code sets slicedNewFiles
= newFiles (not clamped), so uploads can exceed validation.maxFiles; before
removing/adding files compute remainingCapacity = Math.max(0,
validation.maxFiles - fileStatuses.value.length) and then clamp slicedNewFiles
to at most remainingCapacity (or to validation.maxFiles when you're replacing
the entire list) so the loop that calls onRemoveFile(oldestFile.id) and
subsequent upload logic only processes the allowed number of new files; update
references to slicedNewFiles, fileStatuses, validation.maxFiles, and
onRemoveFile to use that clamped array/amount.
- Around line 114-115: The code assigns the return of shapeUploadError (string |
void) to FileStatus.error via an unsafe cast, violating the TUploadError type;
change the assignment so FileStatus.error keeps the original result.error
(preserving the TUploadError value) and store the shaped message separately
(e.g., shapedErrorMessage or uploadErrorMessage) using the
shapeUploadError(result.error) return, update any consumers to read the shaped
message field for UI text while preserving result.error for type-correct
programmatic error handling; locate the logic around shapeUploadError,
result.error and FileStatus.error in useDropzoneUpload.ts (the code that
currently does shapedError as TUploadError) and modify it accordingly.
---
Nitpick comments:
In `@apps/v4/content/docs/components/dropzone.md`:
- Around line 137-142: The example's onDropFile handler uses
URL.createObjectURL(file) but never revokes blob URLs; update the component to
track the current preview URL (created in onDropFile) and call
URL.revokeObjectURL(prevUrl) before replacing it and when removing the file, and
also revoke the current URL inside onUnmounted; specifically modify the
onDropFile flow and add cleanup logic (tracking a previewUrl variable and
revoking it) and an onUnmounted cleanup call so URLs are released.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30fbb391-1c56-4468-83da-1b79eb39fb7f
📒 Files selected for processing (2)
apps/v4/content/docs/components/dropzone.mdapps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts (1)
305-323:⚠️ Potential issue | 🟠 Major
shiftOnMaxFilesstill allows exceedingmaxFileswhen dropzone is empty.When
shiftOnMaxFilesistrueand the dropzone has no existing files, line 306 setsslicedNewFiles = newFiles(unclamped), and line 308's conditionfileStatuses.value.length > 0causes the clamping logic to be skipped entirely.Scenario:
maxFiles=4,shiftOnMaxFiles=true, empty dropzone, user drops 10 files → all 10 are added.🐛 Suggested fix: Always clamp when shiftOnMaxFiles is enabled
let slicedNewFiles - = shiftOnMaxFiles === true ? newFiles : newFiles.slice(0, maxNewFiles) + = shiftOnMaxFiles === true + ? newFiles.slice(0, validation?.maxFiles ?? Infinity) + : newFiles.slice(0, maxNewFiles) - if (shiftOnMaxFiles === true && fileStatuses.value.length > 0 && validation?.maxFiles !== undefined) { + if (shiftOnMaxFiles === true && validation?.maxFiles !== undefined && fileStatuses.value.length > 0) { // Calculate how many files need to be removed - const removalsNeeded = Math.max(0, fileStatuses.value.length + slicedNewFiles.length - validation.maxFiles) + const removalsNeeded = Math.max(0, fileStatuses.value.length + newFiles.length - validation.maxFiles) // Remove oldest files sequentially for (let i = 0; i < removalsNeeded; i++) { const oldestFile = fileStatuses.value[0] if (oldestFile) { await onRemoveFile(oldestFile.id) } } // Recalculate remaining capacity after removals const remainingCapacity = Math.max(0, validation.maxFiles - fileStatuses.value.length) slicedNewFiles = newFiles.slice(0, remainingCapacity) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 305 - 323, The current logic lets shiftOnMaxFiles=true add all dropped files when the dropzone is empty because slicedNewFiles is set to newFiles and the removal block is skipped by checking fileStatuses.value.length > 0; fix by always clamping when shiftOnMaxFiles is true: set slicedNewFiles to a sliced subset based on the allowed capacity (use validation.maxFiles or maxNewFiles as fallback) up front when shiftOnMaxFiles is true, remove the guard that requires fileStatuses.value.length > 0 so removalsNeeded is computed and applied regardless of existing files, and keep using onRemoveFile(oldestFile.id) and the recalculation of remainingCapacity to finally slice newFiles to the actual space available; refer to variables shiftOnMaxFiles, slicedNewFiles, newFiles, maxNewFiles, validation.maxFiles, fileStatuses.value, onRemoveFile, removalsNeeded, and remainingCapacity when making the change.
🧹 Nitpick comments (1)
apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts (1)
218-223:shapedErroris computed but never used.The
shapedErrorvariable is calculated on lines 218-220, but line 223 storesresult.error(the original error) instead. This leavesshapedErroras dead code.If error shaping is intended to affect the stored error message, use
shapedErrorin the assignment. Otherwise, remove the unused computation.Option A: Remove dead code if shaping is only for display elsewhere
- const shapedError = pShapeUploadError !== undefined - ? pShapeUploadError(result.error) - : result.error fileStatuses.value = [ ...fileStatuses.value.slice(0, index), { ...currentFile, status: "error" as const, error: result.error },Option B: Add a `shapedError` field if shaping should be persisted
const shapedError = pShapeUploadError !== undefined ? pShapeUploadError(result.error) : result.error fileStatuses.value = [ ...fileStatuses.value.slice(0, index), - { ...currentFile, status: "error" as const, error: result.error }, + { ...currentFile, status: "error" as const, error: result.error, shapedError }, ...fileStatuses.value.slice(index + 1), ] as FileStatus<TUploadRes, TUploadError>[]This would require adding
shapedError?: stringto theFileStatusinterface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts` around lines 218 - 223, The computed shapedError (from pShapeUploadError and result.error) is never used; either replace the stored error with shapedError by assigning error: shapedError in the fileStatuses update (symbols: shapedError, pShapeUploadError, fileStatuses, currentFile, result.error), or remove the dead computation entirely if shaping is not meant to be persisted; if you want to persist shaped text add shapedError?: string to the FileStatus type and store shapedError in the new field instead of overwriting the original error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/v4/content/docs/components/dropzone.md`:
- Around line 250-253: Update the documentation snippet so its validation limits
match the demo's DropzoneMultiFile.vue: change the validation block values from
maxFiles: 10 to maxFiles: 4 and add maxSize: 5 * 1024 * 1024; also update the
user-facing description text to "Select up to 4 files (max 5MB)" so the docs
reflect the live demo's maxFiles and maxSize settings.
- Around line 361-374: The multi-image dropzone example leaks blob URLs because
URL.createObjectURL(file) is returned but never revoked; update the
useDropzoneUpload flow (the onDropFile handler and the component using its
returned files) to store the created object URLs and call
URL.revokeObjectURL(...) when a file is removed and in the component
cleanup/unmount; ensure the logic around the returned value from onDropFile (in
useDropzoneUpload) ties each created URL to the file entry so it can be revoked
on removal and on unmount.
---
Duplicate comments:
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 305-323: The current logic lets shiftOnMaxFiles=true add all
dropped files when the dropzone is empty because slicedNewFiles is set to
newFiles and the removal block is skipped by checking fileStatuses.value.length
> 0; fix by always clamping when shiftOnMaxFiles is true: set slicedNewFiles to
a sliced subset based on the allowed capacity (use validation.maxFiles or
maxNewFiles as fallback) up front when shiftOnMaxFiles is true, remove the guard
that requires fileStatuses.value.length > 0 so removalsNeeded is computed and
applied regardless of existing files, and keep using onRemoveFile(oldestFile.id)
and the recalculation of remainingCapacity to finally slice newFiles to the
actual space available; refer to variables shiftOnMaxFiles, slicedNewFiles,
newFiles, maxNewFiles, validation.maxFiles, fileStatuses.value, onRemoveFile,
removalsNeeded, and remainingCapacity when making the change.
---
Nitpick comments:
In `@apps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts`:
- Around line 218-223: The computed shapedError (from pShapeUploadError and
result.error) is never used; either replace the stored error with shapedError by
assigning error: shapedError in the fileStatuses update (symbols: shapedError,
pShapeUploadError, fileStatuses, currentFile, result.error), or remove the dead
computation entirely if shaping is not meant to be persisted; if you want to
persist shaped text add shapedError?: string to the FileStatus type and store
shapedError in the new field instead of overwriting the original error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf5a6a72-3d50-47ec-9333-ce986689a688
📒 Files selected for processing (2)
apps/v4/content/docs/components/dropzone.mdapps/v4/registry/new-york-v4/ui/dropzone/useDropzoneUpload.ts
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
1 participant
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.
🔗 Linked issue
#1719
❓ Type of change
📚 Description
This is a drag-and-drop file upload component with support for multiple files, validation, and upload progress.
react-dropzonewithvue3-dropzone.This component is useful in file uploads - forms, profile settings, and other use-cases or when you would like to show a preview of the file you're uploading to the user.
Credit
📸 Screenshots (if appropriate)
📝 Checklist
Summary by CodeRabbit
New Features
Documentation
Chores