fix: allow dropping image into image tab #2965
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughIntroduces an optional onUpload callback into the image drag-and-drop flow. The hook now handles drop events and invokes onUpload with dropped files. ImageGrid accepts and forwards onUpload to the hook and binds onDrop. ImagesTab passes its existing handleUpload down to ImageGrid. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant IG as ImageGrid
participant H as useImageDragDrop
participant CB as onUpload (callback)
U->>IG: Drag files over container
IG->>H: handleDragEnter/Over
H-->>IG: Set dragging state
U->>IG: Drop files
IG->>H: handleDrop(event)
H->>H: Prevent default, clear drag state
H->>CB: onUpload(FileList)?
alt onUpload provided
CB-->>H: Promise resolved/rejected
else
H-->>IG: No-op
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
|
|
||
| const files = e.dataTransfer.files; | ||
| if (files.length > 0 && onUpload) { | ||
| void onUpload(files); |
There was a problem hiding this comment.
Consider adding error handling (e.g., try/catch) around the onUpload(files) call in handleDrop to cover upload failures.
![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
🧹 Nitpick comments (1)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx (1)
95-98: Consider validating dropped files are images.While
handleDragStateChangechecks MIME types during drag, the actual dropped files aren't validated. This defensive check would catch edge cases where non‑images slip through.const files = e.dataTransfer.files; -if (files.length > 0 && onUpload) { +const imageFiles = Array.from(files).filter(file => file.type.startsWith('image/')); +if (imageFiles.length > 0 && onUpload) { - onUpload(files).catch((error) => { + const fileList = Object.assign(imageFiles, { item: (i: number) => imageFiles[i] }) as FileList; + onUpload(fileList).catch((error) => { console.error('Upload failed:', error); // TODO: Show user-facing error notification }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx(2 hunks)apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsxapps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
🧬 Code graph analysis (1)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx (1)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx (1)
useImageDragDrop(6-112)
🔇 Additional comments (5)
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/index.tsx (1)
86-86: LGTM!The prop wiring correctly enables drag-and-drop uploads by passing
handleUploadtoImageGrid.apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/image-grid.tsx (2)
14-21: LGTM!The integration correctly wires the upload callback through the hook and binds the drop handler to the container.
31-31: LGTM!The drop handler binding completes the drag-and-drop upload flow.
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx (2)
6-6: LGTM!The optional
onUploadparameter correctly enables external upload handling.
106-106: LGTM!Correctly exposes
handleDropfor binding in consumer components.
| const handleDrop = useCallback((e: React.DragEvent<HTMLDivElement>) => { | ||
| e.preventDefault(); | ||
| setIsDragging(false); | ||
| e.currentTarget.removeAttribute('data-dragging-image'); | ||
|
|
||
| const files = e.dataTransfer.files; | ||
| if (files.length > 0 && onUpload) { | ||
| void onUpload(files); | ||
| } | ||
| }, [onUpload]); |
There was a problem hiding this comment.
Add error handling for upload failures.
The void onUpload(files) call explicitly ignores the promise, meaning upload errors won't be caught or displayed. Users won't know if their drop upload failed.
Consider handling errors:
const handleDrop = useCallback((e: React.DragEvent<HTMLDivElement>) => {
e.preventDefault();
setIsDragging(false);
e.currentTarget.removeAttribute('data-dragging-image');
const files = e.dataTransfer.files;
if (files.length > 0 && onUpload) {
- void onUpload(files);
+ onUpload(files).catch((error) => {
+ console.error('Upload failed:', error);
+ // TODO: Show user-facing error notification
+ });
}
}, [onUpload]);🤖 Prompt for AI Agents
In
apps/web/client/src/app/project/[id]/_components/left-panel/image-tab/hooks/use-image-drag-drop.tsx
around lines 90–99, the drop handler currently calls void onUpload(files) which
ignores the returned promise and swallows upload errors; change the handler to
await the onUpload call inside a try/catch (or call onUpload(files).catch(...))
so upload failures are caught, set an error state or invoke an existing error
callback (or show a user-facing toast/message) with the error, and ensure the
dependency array reflects any new state or callbacks used; keep preventing
default and clearing dragging attributes as-is.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Adds drag-and-drop image upload functionality to the image tab, updating
use-image-drag-drop.tsx,image-grid.tsx, andindex.tsx.handleDropfunction inuse-image-drag-drop.tsxprocesses dropped files and callsonUpload.ImageGridcomponent updated to acceptonUploadprop and handleonDropevent.ImagesTabcomponent passeshandleUploadtoImageGrid.This description was created by
for a50209e. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit