Fix file drop functionality in the farm creation wizard shapefile upload#319
Conversation
🦋 Changeset detectedLatest commit: b0fb323 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughReplaces label-wrapped file inputs with hidden Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Dropzone as Dropzone (label)
participant HiddenInput as Hidden input[type="file"]
participant Component as Upload component logic
Note over User,Dropzone: User clicks, presses Enter/Space, or drops files
User->>Dropzone: click / key / drop
Dropzone->>Component: onClick / onKeyDown / onDrop
Component->>Component: validate/process files, update internal file list & UI status
Component->>HiddenInput: build DataTransfer & set input.files
HiddenInput->>Component: input.onchange (file picker) → handleFileChange
Component->>Dropzone: render updated status (idle → uploading → success/error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
206-237: Refactor to use the field'sonChangecallback for consistency.
handleDropusesform.setValuedirectly (line 233), whilehandleFileChangeuses the field'sonChangecallback (line 201 inhandleFileChange). This inconsistency may cause issues if the field'sonChangehas side effects that should execute.Refactor
handleDropto accept theonChangecallback as a parameter:-const handleDrop = async (e: React.DragEvent<HTMLLabelElement>) => { +const handleDrop = async ( + e: React.DragEvent<HTMLLabelElement>, + onChange: (files: File[]) => void, +) => { e.preventDefault() if (e.dataTransfer.files && e.dataTransfer.files.length > 0) { const newFiles = Array.from(e.dataTransfer.files) // ... validation logic ... const currentFiles = form.getValues("shapefile") || [] const updatedFiles = mergeShapefileParts( currentFiles, validNewFiles, requiredExtensions, ) - form.setValue("shapefile", updatedFiles, { shouldValidate: true }) + onChange(updatedFiles) await handleFilesSet(updatedFiles) e.dataTransfer.clearData() } }Then update the label's
onDropprop to pass theonChangecallback:- onDrop={handleDrop} + onDrop={async (e) => await handleDrop(e, onChange)}
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
461-464: Remove commented-out code.The commented-out file extensions should be removed if they're no longer needed, or uncommented if they should be displayed.
- <div className="text-xs text-muted-foreground mt-1"> - {/* .shp, - .shx, - .dbf, - .prj */} - </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/new-hornets-hope.md(1 hunks)fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(3 hunks)
🔇 Additional comments (1)
.changeset/new-hornets-hope.md (1)
1-5: LGTM!The changelog entry is clear and properly formatted. It accurately describes the UI improvement that allows users to drop files onto the entire shapefile upload area.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #319 +/- ##
============================================
Coverage 93.31% 93.31%
============================================
Files 96 96
Lines 15325 15325
Branches 1557 1557
============================================
Hits 14301 14301
Misses 1022 1022
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
391-414: AddtabIndex={0}androle="button"for keyboard accessibility.The label is not keyboard-focusable because the associated file input is hidden. This was flagged in a previous review but hasn't been addressed yet. Without
tabIndex={0}, keyboard users cannot tab to this element, and withoutrole="button", screen readers won't identify it as an interactive control.Apply this diff to make the label keyboard-accessible:
<label - className="relative block" + className="relative block focus:outline-none focus:ring-2 focus:ring-offset-2 focus:ring-primary" htmlFor="file-upload" + tabIndex={0} + role="button" aria-label="Upload shapefile files by clicking or dragging and dropping" onKeyDown={(e) => {
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
450-450: Consider movingcursor-pointerto the label.The
cursor-pointerclass on the inner div is somewhat redundant since the label itself is the interactive element. Consider moving it to the label's className (line 392) for clarity, or rely on the browser's default cursor behavior for labels.Apply this diff if you prefer to make it explicit on the label:
<label - className="relative block" + className="relative block cursor-pointer" htmlFor="file-upload"Then remove from the inner div:
-<div className="flex flex-col items-center justify-center w-full h-full cursor-pointer"> +<div className="flex flex-col items-center justify-center w-full h-full">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.changeset/free-donkeys-greet.md(1 hunks)fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/free-donkeys-greet.md
🧰 Additional context used
🧬 Code graph analysis (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
fdm-app/app/components/ui/input.tsx (1)
Input(25-25)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(4 hunks)
🧰 Additional context used
🪛 Biome (2.1.2)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
[error] 402-402: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 402-402: The HTML element label is non-interactive and should not have an interactive role.
Replace label with a div or a span.
Unsafe fix: Remove the role attribute.
(lint/a11y/noNoninteractiveElementToInteractiveRole)
🔇 Additional comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
234-244: LGTM! Type safety issue resolved.The code now properly types the file input element and includes a null check before accessing its properties, addressing the previous review concern.
BoraIneviNMI
changed the title
coderabbitai
Bot
added
the
branch:development
2 participants
Enhancements
Summary by CodeRabbit