Add a Dropzone component for shapefile and soil analysis uploads#323
Conversation
🦋 Changeset detectedLatest commit: 27e7cad 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 |
WalkthroughExtracts drag-and-drop and file-selection logic into a new reusable Dropzone component, updates two form-upload components (mijnpercelen and soil) to use Dropzone and derive selected files from form state, and adds a changeset documenting the patch release. (34 words) Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant Dropzone
participant Form as "React Hook Form"
participant DBFParser
Note over User,Dropzone: User selects files via drag/drop or file picker
User->>Dropzone: drag/drop or choose file(s)
Dropzone->>Dropzone: validate extensions & sizes
alt validation OK
Dropzone->>Form: onFilesChange(files) -> form.setValue(...)
Form->>DBFParser: trigger DBF parse if .dbf present
DBFParser-->>Form: parsed metadata
Form-->>User: update selectedFiles / status
else validation error
Dropzone-->>User: show validation error notification
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## development #323 +/- ##
============================================
Coverage 87.68% 87.68%
============================================
Files 69 69
Lines 3499 3499
Branches 973 973
============================================
Hits 3068 3068
Misses 431 431
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:
|
coderabbitai
Bot
added
branch:development
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.changeset/bright-pillows-wink.md (1)
5-5: Use hyphens for compound modifiers.The phrase "drag and drop handlers" should use a hyphenated compound modifier: "drag-and-drop handlers".
Apply this diff:
-Removed clutter from form-upload components by moving the drag and drop handlers into a new Dropzone component. +Removed clutter from form-upload components by moving the drag-and-drop handlers into a new Dropzone component.fdm-app/app/components/blocks/soil/form-upload.tsx (1)
78-78: Consider the stability ofform.resetin the dependency array.While including
form.resetin the dependency array is a common pattern withreact-hook-form(since the form object is stable), it can be confusing. Consider using the ESLint disable comment to clarify intent:- }, [isSubmitting, actionData, form.reset]) + }, [isSubmitting, actionData, form.reset]) // eslint-disable-line react-hooks/exhaustive-depsOr better yet, ensure
form.resetis stable by not including it if the form instance doesn't change.fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
339-348: UseselectedFilesinstead ofform.getValuesto avoid redundant reads.The
selectedFilesvariable already watches the"shapefile"form value (line 114), so callingform.getValues("shapefile")here is redundant and could impact performance.Apply this diff:
<RequiredFilesStatus files={ - form.getValues( - "shapefile", - ) || [] + selectedFiles } requiredExtensions={ requiredExtensions } />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
fdm-app/app/components/ui/dropzone.tsxis excluded by!fdm-app/app/components/ui/**
📒 Files selected for processing (3)
.changeset/bright-pillows-wink.md(1 hunks)fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(4 hunks)fdm-app/app/components/blocks/soil/form-upload.tsx(7 hunks)
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2024-11-25T12:42:32.783Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 6
File: fdm-app/vite.config.ts:5-9
Timestamp: 2024-11-25T12:42:32.783Z
Learning: In the `fdm-app` project, SvenVw is preparing for migration to Remix v3 and may include type declarations or configurations for v3 features in advance, such as in `vite.config.ts`.
Applied to files:
.changeset/bright-pillows-wink.md
📚 Learning: 2025-09-26T08:34:50.413Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 279
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.norms.tsx:277-283
Timestamp: 2025-09-26T08:34:50.413Z
Learning: In the fdm project, fdm-core and fdm-app are updated together as part of a monorepo structure, which eliminates legacy data concerns when new fields like b_isproductive are introduced. Both packages are synchronized, so there's no need for defensive coding against undefined values for newly introduced database fields.
Applied to files:
.changeset/bright-pillows-wink.md
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
.changeset/bright-pillows-wink.md
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
🧬 Code graph analysis (2)
fdm-app/app/components/blocks/soil/form-upload.tsx (1)
fdm-app/app/components/ui/dropzone.tsx (1)
Dropzone(49-230)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
fdm-app/app/components/ui/dropzone.tsx (1)
Dropzone(49-230)
🪛 LanguageTool
.changeset/bright-pillows-wink.md
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...orm-upload components by moving the drag and drop handlers into a new Dropzone co...
(QB_NEW_EN_HYPHEN)
[grammar] ~5-~5: Use a hyphen to join words.
Context: ...upload components by moving the drag and drop handlers into a new Dropzone compon...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (10)
fdm-app/app/components/blocks/soil/form-upload.tsx (4)
3-3: LGTM: Import additions are correct.The
useMemoandDropzoneimports are properly added and used in the component.Also applies to: 25-25
41-45: LGTM: Derived state implementation is correct.The
selectedFilewatch and memoizeddropzoneValuearray correctly track and transform the form state for the Dropzone component.
80-83: LGTM: File change handler correctly extracts single file.The
handleFilesChangefunction correctly takes an array (as required by Dropzone'sonFilesChangecallback) and extracts the first file for this single-file upload scenario.
113-194: LGTM: Dropzone integration is well-implemented.The Dropzone component is properly configured with all necessary props, and the UI feedback for different upload states is clear and user-friendly.
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (6)
32-32: LGTM: Dropzone import added correctly.
114-121: LGTM: Derived state implementation is efficient.The
selectedFileswatch and derivedhasAllRequiredFilesboolean correctly track shapefile completeness from form state.
129-150: LGTM: File handling with DBF parsing is well-implemented.The
handleFilesSetfunction correctly:
- Updates form state with the new files
- Resets upload state
- Parses the DBF file to extract field names
- Handles errors gracefully with user feedback
284-316: LGTM: Dropzone configuration is comprehensive and correct.The Dropzone is properly configured for multiple shapefile uploads with:
- Correct props (
value,accept,multiple,onFilesChange)- Custom
mergeFileslogic for intelligent shapefile part merging- Conditional styling based on upload state and file completeness
317-372: LGTM: Upload status UI provides clear user feedback.The UI correctly displays:
- Selected file names
- Required file status indicators
- Success and error states with appropriate icons and messages
502-537: LGTM: Shapefile merge logic is well-designed.The
mergeShapefilePartsfunction intelligently handles:
- Complete replacement when all required files are provided at once
- Incremental updates by extension when adding files one-by-one
- Deduplication by filename
This approach provides good UX for shapefile uploads.
SvenVw
left a comment
SvenVw
left a comment
There was a problem hiding this comment.
Thanks, very nice! Just some small comments
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/bright-pillows-wink.md(1 hunks)fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(5 hunks)fdm-app/app/components/blocks/soil/form-upload.tsx(6 hunks)fdm-app/app/components/custom/dropzone.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/bright-pillows-wink.md
🚧 Files skipped from review as they are similar to previous changes (2)
- fdm-app/app/components/blocks/soil/form-upload.tsx
- fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-10T15:05:38.163Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
🪛 Biome (2.1.2)
fdm-app/app/components/custom/dropzone.tsx
[error] 196-196: The HTML element label is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
fdm-app/app/components/custom/dropzone.tsx (1)
201-221: Critical accessibility violation and duplicate ref assignment.This segment has two major issues that were flagged in previous reviews but remain unaddressed:
- Accessibility: Placing
tabIndex={0}on a non-interactive<label>violates accessibility standards and confuses keyboard users.- Duplicate ref: Line 208 assigns the forwarded
refto the<label>, overriding the correct assignment to the<input>at lines 188-191. This breaks integrations likereact-hook-formthat expectrefto point to the input element.Replace the
<label>with an interactive<div>and removeref={ref}from it:- <label + <div + role="button" tabIndex={0} className={cn( "flex-col items-center justify-center w-full h-32 rounded-md border border-dashed border-muted-foreground/25 px-6 py-4 text-center transition-colors hover:bg-muted/25", disabled ? "hidden" : "flex", className, )} - ref={ref} - htmlFor={labelId} + onClick={() => inputRef.current?.click()} aria-label="Upload shapefile files by clicking or dragging and dropping" onKeyDown={(e) => { if (e.key === "Enter" || e.key === " ") { e.preventDefault() inputRef.current?.click() } }} onDragOver={handleDragOver} onDrop={handleDrop} > {children} - </label> + </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/custom/dropzone.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-06-10T15:05:38.163Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
🪛 Biome (2.1.2)
fdm-app/app/components/custom/dropzone.tsx
[error] 202-202: The HTML element label is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🔇 Additional comments (6)
fdm-app/app/components/custom/dropzone.tsx (6)
71-75: LGTM! Proper normalization of accept tokens.The token normalization correctly handles case variations and whitespace, addressing the previous review feedback.
112-123: LGTM! Clean file validation with user feedback.The validation correctly checks normalized extensions and provides non-blocking warnings via toast notifications, allowing valid files through while alerting users to invalid types.
180-200: LGTM! Proper ref forwarding to the input element.The ref callback correctly merges the internal
inputRefwith the forwardedrefon the actual<input>element, ensuring both the component's internal logic and external consumers (e.g.,react-hook-form) can access the input.
238-246: LGTM! Standard context hook pattern.The hook correctly enforces usage within a
Dropzoneprovider and provides clear error messages.
169-178: Verify the necessity of the Context.Providerkey.Using
key={JSON.stringify(fileNames)}forces the entire context and its children to remount whenever files change. This is expensive and should only be used if you need to reset internal state in child components.Typically, context consumers should reactively update when the
valueprop changes, making thekeyunnecessary.If remounting is intentional, please confirm the use case. Otherwise, consider removing the key:
<DropzoneContext.Provider - key={JSON.stringify(fileNames)} value={{ files, accept: acceptedFileExtensions,
1-1: Component correctly placed in custom/ directory.Per codebase conventions and previous feedback, this small reusable component is appropriately located in
~/components/custom/.Based on learnings.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
fdm-app/app/components/custom/dropzone.tsx (2)
171-171: Remove the unnecessary key prop on Context.Provider.
key={JSON.stringify(fileNames)}forces the Provider to unmount and remount whenever files change, destroying internal state in all consuming children. Context value changes already trigger re-renders in consumers without requiring a key.Apply this diff:
<DropzoneContext.Provider - key={JSON.stringify(fileNames)} value={{
126-167: Consider extracting shared file processing logic.
handleFileChangeandhandleDropcontain nearly identical logic (validation, merging, DataTransfer update). Extracting the common flow reduces duplication and improves maintainability.Example refactor:
const processFiles = async (newFiles: File[]) => { const validNewFiles = newFiles.filter(checkNewFile) if (validNewFiles.length === 0) return const finalFiles = await handleFilesSet(files, validNewFiles) if (inputRef.current) { const container = new DataTransfer() finalFiles.forEach((f) => container.items.add(f)) inputRef.current.files = container.files } } const handleFileChange = async (event: React.ChangeEvent<HTMLInputElement>) => { if (event.target.files) { await processFiles(Array.from(event.target.files)) } } const handleDrop = async (e: React.DragEvent<HTMLLabelElement>) => { e.preventDefault() if (e.dataTransfer.files?.length > 0) { await processFiles(Array.from(e.dataTransfer.files)) e.dataTransfer.clearData() } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/custom/dropzone.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-06-10T15:05:38.163Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
🪛 Biome (2.1.2)
fdm-app/app/components/custom/dropzone.tsx
[error] 203-203: The HTML element label is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🔇 Additional comments (5)
fdm-app/app/components/custom/dropzone.tsx (5)
22-25: LGTM—proper guard for files without extensions.The function correctly handles filenames without dots by checking
dotIndex === -1before slicing.
72-76: LGTM—accept tokens normalized correctly.Token normalization (trim + lowercase) prevents whitespace and case mismatches from causing false rejections.
83-86: LGTM—file deduplication by name using Map.The Map-based approach correctly deduplicates files by filename, replacing earlier files with later ones of the same name.
90-94: LGTM—input cleared with empty string.Correctly assigns
""instead ofnullto clear the input value.
189-192: LGTM—ref callback correctly merges internal and forwarded refs.The callback ref pattern properly assigns both
inputRef.currentand the forwardedrefto the input element.
| const files = | ||
| value ?? | ||
| (inputRef.current?.files ? Array.from(inputRef.current.files) : []) |
There was a problem hiding this comment.
Fix the controlled/uncontrolled state mixing.
The files derivation reads directly from inputRef.current?.files when value is undefined. This is non-reactive—changes to the input's FileList won't trigger re-renders, causing the component to show stale state when operating in uncontrolled mode.
Either make the component fully controlled (always require value and onFilesChange) or maintain internal state for uncontrolled usage.
If fully controlled is the intent, apply this diff:
- const files =
- value ??
- (inputRef.current?.files ? Array.from(inputRef.current.files) : [])
+ const files = value ?? []Then document that value and onFilesChange are required props. Otherwise, add internal state:
+ const [internalFiles, setInternalFiles] = useState<File[]>([])
const files =
value ??
- (inputRef.current?.files ? Array.from(inputRef.current.files) : [])
+ internalFilesAnd update handlers to call setInternalFiles when onFilesChange is absent.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const files = | |
| value ?? | |
| (inputRef.current?.files ? Array.from(inputRef.current.files) : []) | |
| const files = | |
| value ?? | |
| [] |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-app/app/components/custom/dropzone.tsx (1)
68-70: Fix controlled/uncontrolled state mixing (previously flagged, still unresolved).Reading from
inputRef.current?.fileswhenvalueis undefined is non-reactive—changes to the input's FileList won't trigger re-renders, causing stale UI in uncontrolled mode.Since both consumers (mijnpercelen and soil forms) pass
valueandonFilesChange, this component should be fully controlled.Apply this diff:
- const files = - value ?? - (inputRef.current?.files ? Array.from(inputRef.current.files) : []) + const files = value ?? []Then document in the component's JSDoc that
valueandonFilesChangeare required for proper operation.
🧹 Nitpick comments (1)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
337-341: Remove commented-out code.The commented extension labels serve no purpose and add clutter.
Apply this diff:
<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 (3)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx(5 hunks)fdm-app/app/components/blocks/soil/form-upload.tsx(6 hunks)fdm-app/app/components/custom/dropzone.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (12)
📚 Learning: 2025-11-04T13:46:46.396Z
Learnt from: BoraIneviNMI
Repo: SvenVw/fdm PR: 0
File: :0-0
Timestamp: 2025-11-04T13:46:46.396Z
Learning: In the FDM app, file upload validation uses a two-layer approach: client-side Zod schemas (e.g., in fdm-app/app/components/blocks/soil/form-upload.tsx) provide early user-facing error feedback before upload, while server-side upload handlers enforce the same limits for security.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/soil/form-upload.tsx
📚 Learning: 2025-05-09T14:53:44.578Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:53:44.578Z
Learning: In the context of this React Router v7 project, it's important to follow the pattern of importing only the types (like UseFormReturn) from "react-hook-form" while importing the Form component from "react-router" to avoid naming conflicts.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/soil/form-upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, not from "remix-run/react".
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/soil/form-upload.tsx
📚 Learning: 2025-05-09T14:41:43.484Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/fertilizer-applications/form.tsx:6-6
Timestamp: 2025-05-09T14:41:43.484Z
Learning: The project uses React Router v7 which exports a Form component directly from the "react-router" package, making importing from "remix-run/react" unnecessary.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/soil/form-upload.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsxfdm-app/app/components/blocks/soil/form-upload.tsxfdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-04-18T13:49:17.029Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 124
File: fdm-app/app/components/custom/farm/farm-title.tsx:3-3
Timestamp: 2025-04-18T13:49:17.029Z
Learning: In the fdm project, NavLink and other routing components can be imported from either "react-router" or "react-router-dom" as react-router-dom is included in react-router.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2024-12-16T10:56:07.561Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/app/routes/app.addfarm.$b_id_farm.cultivations.$b_lu_catalogue.fertilizers.tsx:1-1
Timestamp: 2024-12-16T10:56:07.561Z
Learning: The project uses `react-router` v7, and the `data` function is exported and used for error handling in loaders and actions.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2024-12-16T10:48:27.598Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 16
File: fdm-app/package.json:40-40
Timestamp: 2024-12-16T10:48:27.598Z
Learning: In the `fdm-app/package.json` file, `remix-toast` and `sonner` are not duplicate dependencies. `remix-toast` manages toast generation and handling within the Remix framework, while `sonner` provides the actual implementation and UI of the toast notifications. They serve different purposes and are intended to be used together in the project.
Applied to files:
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx
📚 Learning: 2025-06-10T15:05:38.163Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
🧬 Code graph analysis (2)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (1)
fdm-app/app/components/custom/dropzone.tsx (1)
Dropzone(50-258)
fdm-app/app/components/blocks/soil/form-upload.tsx (1)
fdm-app/app/components/custom/dropzone.tsx (1)
Dropzone(50-258)
🪛 Biome (2.1.2)
fdm-app/app/components/custom/dropzone.tsx
[error] 224-224: The HTML element label is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🔇 Additional comments (14)
fdm-app/app/components/blocks/mijnpercelen/form-upload.tsx (4)
16-16: LGTM! Import added for the new Dropzone component.The import follows the project's conventions for custom component paths.
114-121: LGTM! Clean derivation of file state from form.Deriving
selectedFilesandhasAllRequiredFilesfromform.watch("shapefile")is the correct pattern—form state is the single source of truth, eliminating the need for redundant local state.
129-150: LGTM! Simplified file handler delegates validation to Dropzone.The refactored
handleFilesSetnow accepts pre-validated files from the Dropzone component, reducing duplication. DBF parsing logic is correctly preserved.
284-318: LGTM! Dropzone integration with custom merge strategy.The FormField correctly wires the Dropzone component with:
- Controlled
valuefrom form stateonFilesChangecallback to update form- Custom
mergeShapefilePartsstrategy for handling partial shapefile uploads- Proper ref forwarding and validation props
The custom merge function intelligently handles both complete uploads (all required extensions at once) and incremental uploads (one extension at a time).
fdm-app/app/components/custom/dropzone.tsx (6)
22-25: LGTM! Correct edge case handling.The
getFileExtensionfunction now properly handles filenames without extensions by checking for-1before slicing.
27-48: LGTM! Well-typed props interface.The
DropzonePropstype correctly usesReact.Ref<HTMLInputElement>and provides comprehensive prop coverage for controlled usage, file validation, and custom merge strategies.
78-88: LGTM! Correct file deduplication by name.The default merge strategy now properly deduplicates by filename using a Map, avoiding the reference-equality issue from earlier versions.
90-94: LGTM! Correct input clearing.Using an empty string instead of
nullto clear the input value is correct.
113-144: LGTM! Comprehensive file validation.The
checkNewFilefunction validates extension, min size, and max size with clear user feedback via toast notifications.
260-268: LGTM! Proper context hook implementation.The
useDropzoneContexthook correctly enforces usage within a Dropzone provider with a clear error message.fdm-app/app/components/blocks/soil/form-upload.tsx (4)
8-8: LGTM! Import added for the new Dropzone component.Follows the project's conventions for custom component paths.
3-3: LGTM! Efficient memoization for single-file wrapper.The
useMemoprevents unnecessary re-creation of the array wrapper whenselectedFilehasn't changed, optimizing the controlled value prop passed to Dropzone.Also applies to: 41-45
80-83: LGTM! Clean single-file extraction.The simplified
handleFilesChangecorrectly extracts the first file from the Dropzone's file array and updates the form state.
113-195: LGTM! Clean Dropzone integration for single-file upload.The FormField correctly integrates Dropzone with:
- Controlled
valueviadropzoneValuememo- Single-file extraction via
handleFilesChange- Proper validation props (accept, maxSize, required)
- Status-based styling and feedback
The implementation is simpler than mijnpercelen because it doesn't need custom merge logic—Dropzone's default single-file behavior (replace) is appropriate here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
fdm-app/app/components/custom/dropzone.tsx (1)
68-70: Clarify the component contract: controlled vs. uncontrolled.The fallback to
inputRef.current?.fileswhenvalueis undefined creates a non-reactive path—changes to the input's FileList won't trigger re-renders, causing stale state if the component is used in uncontrolled mode.Since the component uses
onFilesChangefor state updates and relies onvaluebeing provided, it appears designed as a controlled component. Either:
- Make it fully controlled (recommended):
- const files = - value ?? - (inputRef.current?.files ? Array.from(inputRef.current.files) : []) + const files = value ?? []Then document that
valueandonFilesChangeare required for the component to function.
- Or add internal state for true uncontrolled support:
+ const [internalFiles, setInternalFiles] = useState<File[]>([]) const files = value ?? - (inputRef.current?.files ? Array.from(inputRef.current.files) : []) + internalFilesAnd update handlers to call
setInternalFileswhenonFilesChangeis absent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
fdm-app/app/components/custom/dropzone.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-06-10T15:05:38.163Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 163
File: fdm-app/app/routes/farm.$b_id_farm.$calendar.field._index.tsx:27-29
Timestamp: 2025-06-10T15:05:38.163Z
Learning: In the fdm-app codebase, the component directory structure is intentionally organized with `~/components/custom/` containing small reusable components that can be used in both blocks and pages, while `~/components/blocks/` contains larger block-level components. Not all components should be migrated from custom to blocks - the custom directory serves as a library of smaller utility components.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-13T11:05:40.105Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 237
File: fdm-data/src/cultivations/catalogues/brp.ts:65-68
Timestamp: 2025-08-13T11:05:40.105Z
Learning: In fdm-app Select components, using `undefined` instead of empty string (`""`) as fallback value prevents empty strings from being submitted as form values. This approach fixes the issue at the UI source rather than requiring backend validation.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-05-09T14:58:10.465Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 138
File: fdm-app/app/components/custom/combobox.tsx:34-37
Timestamp: 2025-05-09T14:58:10.465Z
Learning: When updating React components that use both react-hook-form and React Router v7, it's important to only import types (like UseFormReturn, FieldValues) from react-hook-form to avoid naming conflicts with React Router's Form component. Use `import type { ... } from 'react-hook-form'` syntax to ensure only types are imported.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-02-13T09:03:11.890Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 71
File: fdm-app/app/routes/farm.create.$b_id_farm.cultivations.$b_lu_catalogue.crop.harvest._index.tsx:111-135
Timestamp: 2025-02-13T09:03:11.890Z
Learning: When adding multiple harvests in fdm-app, use Promise.all instead of Promise.allSettled to ensure atomic behavior - if one harvest addition fails, all should fail and rollback to maintain data consistency.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
📚 Learning: 2025-08-11T12:24:32.200Z
Learnt from: SvenVw
Repo: SvenVw/fdm PR: 233
File: fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx:53-53
Timestamp: 2025-08-11T12:24:32.200Z
Learning: In `fdm-app/app/components/blocks/atlas-fields/cultivation-history.tsx`, the NMI API for cultivations guarantees that each year will be unique in the cultivation history data, so using `cultivation.year` as a React list key is safe and won't cause duplicate key warnings.
Applied to files:
fdm-app/app/components/custom/dropzone.tsx
🪛 Biome (2.1.2)
fdm-app/app/components/custom/dropzone.tsx
[error] 224-224: The HTML element label is non-interactive. Do not use tabIndex.
Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.
(lint/a11y/noNoninteractiveTabindex)
🔇 Additional comments (6)
fdm-app/app/components/custom/dropzone.tsx (6)
1-25: LGTM: Imports and helper function are correct.The imports follow React 19 patterns with proper type imports, and the
getFileExtensionhelper correctly handles filenames without extensions.
27-48: LGTM: Props are well-typed.The prop types are appropriate, and the
refprop is correctly typed asReact.Ref<HTMLInputElement>.
78-88: LGTM: File deduplication strategy is sound.The default
mergeFilesimplementation correctly deduplicates by filename using aMap, avoiding the previous Set-based reference equality issue.
90-188: LGTM: File validation and event handlers are well-implemented.The file validation logic correctly checks extensions and sizes, provides user feedback via toasts, and properly syncs the native input's FileList using DataTransfer after validation.
202-222: LGTM: Input element and ref forwarding are correct.The hidden file input properly merges the internal
inputRefwith the forwardedrefusing a callback ref (lines 210-212), ensuring both react-hook-form integration and internal input access work correctly.
190-201: LGTM: Context provider and hook are well-structured.The context provider correctly exposes file state to children, the conditional reset button respects the
allowResetprop, and theuseDropzoneContexthook properly enforces usage within a Dropzone component.Also applies to: 244-268
2 participants
Enhancements
Summary by CodeRabbit
New Features
Refactor
Bug Fixes