feat: add external file drag-and-drop from Finder/Explorer#78
Conversation
Files dragged from the OS file manager can now be dropped onto: - The file browser area (uploads to current folder) - Folder rows in the file list (uploads to that folder) - Breadcrumb segments (uploads to that folder) Internal drag-and-drop (move operations) continues to work as before. External drags show a 'copy' cursor and visual feedback on the content area. A new useDropUpload hook extracts reusable upload-on-drop logic. https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
|
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:
WalkthroughAdds OS-level external file drag-and-drop support and a centralized upload hook: new useDropUpload (MAX_FILE_SIZE, isExternalFileDrag, handleFileDrop, isUploading) and wiring in FileBrowser, Breadcrumbs, FileList, FileListItem, UploadZone, and EmptyState to accept folder-targeted external drops; also adds DetailsDialog and desktop PublishCoordinator. Changes
Sequence DiagramsequenceDiagram
participant User as User/OS
participant FB as FileBrowser
participant UI as Breadcrumbs/FileList/FileListItem
participant Hook as useDropUpload
participant Upload as Upload Service
participant Folder as Folder API
participant Store as Upload/Quota Store
User->>FB: Drag files over
FB->>FB: detect external drag (isExternalFileDrag) / set isDraggingExternal
FB->>UI: highlight drop targets
User->>UI: Drop files on target folder
UI->>FB: forward drop (files, targetFolderId)
FB->>Hook: handleFileDrop(files, targetFolderId)
Hook->>Hook: filter oversized, check quotas, detect duplicates
Hook->>Store: report errors / update upload state
Hook->>Upload: upload files
Upload-->>Hook: return file hashes/metadata
Hook->>Folder: register files in target folder
Folder-->>Hook: confirm registration
Hook->>Store: refresh quota / mark success
Hook-->>FB: resolve completion
FB->>FB: clear drag state / remove UI highlight
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Adds support for dragging files from the OS (Finder/Explorer) into the web file browser, enabling upload targets across the main content area, folder rows, and breadcrumb segments while keeping internal drag-to-move behavior unchanged.
Changes:
- Introduces a reusable
useDropUploadhook to validate and upload files dropped from the OS. - Extends folder rows and breadcrumbs to accept external file drops (copy semantics) in addition to internal move drops.
- Adds visual feedback for active external drags over the file browser content area.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/web/src/styles/file-browser.css | Adds an external-drag active styling state for the content area. |
| apps/web/src/hooks/useDropUpload.ts | New hook encapsulating upload-on-drop validation, upload, registration, and cleanup. |
| apps/web/src/components/file-browser/FileListItem.tsx | Allows dropping external files onto folder rows (copy) while preserving internal move drops. |
| apps/web/src/components/file-browser/FileList.tsx | Wires an optional external-file-drop callback down to folder items. |
| apps/web/src/components/file-browser/FileBrowser.tsx | Adds content-area external drag/drop handling and routes drops to current/target folders via the new hook. |
| apps/web/src/components/file-browser/Breadcrumbs.tsx | Enables dropping external files onto breadcrumb segments to upload into that folder. |
…base (#79) * docs: close TEE critical bugs todo (C1/C2/H1) — already fixed in codebase Security review findings C1 (missing epoch fields), C2 (hex/base64 mismatch), and H1 (health response shape) were all resolved during implementation. Marked as RESOLVED in security review and moved todo to done. https://claude.ai/code/session_01H4skah3QYgLKWJDpxDUFps * Update .planning/security/REVIEW-2026-02-07-tee-phase8.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * fix: prevent IPNS sequence rollback with PublishCoordinator Add PublishCoordinator to FUSE layer with: - Per-IPNS-name monotonic sequence cache (never decreases) - Per-folder tokio::sync::Mutex to serialize concurrent publishes - resolve_sequence() returns cached value on resolve failure, or errors if no cache exists (never falls back to seq=0) Updated all 3 publish paths: - spawn_metadata_publish (rename/delete/move operations) - File upload release (write operations) - mkdir parent folder publish Fixes security review L-6 (REVIEW-2026-02-08-phase9-desktop.md) and LOW-SEVERITY-BACKLOG.md item 18. https://claude.ai/code/session_01H4skah3QYgLKWJDpxDUFps --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* fix: fetch storage quota from backend on mount The quota display stayed at zero after re-login because fetchQuota() was only called during upload flows. The zustand store initialized with usedBytes: 0 and nothing triggered a backend sync on app load. Added a useEffect to StorageQuota that calls fetchQuota() on mount so the display always reflects persisted usage. https://claude.ai/code/session_01F11Fe3ZAvUTbt4FHg4V5Ke * fix: guard fetchQuota with auth check to prevent e2e logout race After page reload, StorageQuota's unconditional fetchQuota() fired before session restoration completed, sending an unauthenticated request that triggered the 401 → refresh token flow. With server-side refresh token rotation, this raced with restoreSession's own refresh call — one would fail, and the interceptor's catch handler called logout(), destroying the session for all subsequent e2e tests. Guard fetchQuota() with isAuthenticated so it only fires once the session is fully restored. https://claude.ai/code/session_01F11Fe3ZAvUTbt4FHg4V5Ke * chore: add debug session notes for storage quota investigation https://claude.ai/code/session_01F11Fe3ZAvUTbt4FHg4V5Ke * test: add e2e assertions for storage quota after upload and reload Add two new e2e tests: - 3.6.1: Verifies quota display shows non-zero usage after uploading 13+ files during the session - 3.7.1: Verifies quota persists after page reload (regression test for the quota-stuck-at-zero bug where fetchQuota was never called on mount) https://claude.ai/code/session_01F11Fe3ZAvUTbt4FHg4V5Ke --------- Co-authored-by: Claude <noreply@anthropic.com>
Address PR review feedback: 1. Extract isExternalFileDrag(dataTransfer) helper into useDropUpload to replace repeated Files/application-json type checks across FileBrowser, FileListItem, and Breadcrumbs. 2. Refactor UploadZone and EmptyState to delegate upload logic to useDropUpload hook, eliminating triple-duplicated validation, quota checking, duplicate detection, and IPFS cleanup code. 3. Export MAX_FILE_SIZE from useDropUpload as single source of truth. https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/components/file-browser/UploadZone.tsx`:
- Around line 54-59: handleFileDrop currently swallows errors so await
handleFileDrop(...) always resolves and onUploadComplete fires even on failures;
update the call site in UploadZone.tsx to only invoke onUploadComplete when the
upload truly succeeded by either (a) changing useDropUpload.handleFileDrop to
return a boolean (or re-throw) and awaiting that result here before calling
onUploadComplete, or (b) after awaiting handleFileDrop, read the upload store
status/error (from the hook/store used by useDropUpload) and only call
onUploadComplete when the status indicates success and no error is present;
reference handleFileDrop, useDropUpload, onUploadComplete and the upload store
status getter when implementing the guard.
🧹 Nitpick comments (7)
apps/web/src/components/file-browser/EmptyState.tsx (2)
21-22: Redundant hook call —useDropUploadalready returnsisUploading.
useDropUpload()internally usesuseFileUpload()and re-exportsisUploading(seeuseDropUpload.tsline 109). CallinguseFileUpload()separately here creates a redundant store subscription.♻️ Simplify to single hook
- const { handleFileDrop } = useDropUpload(); - const { isUploading } = useFileUpload(); + const { handleFileDrop, isUploading } = useDropUpload();And remove the unused import:
-import { useDropUpload, MAX_FILE_SIZE } from '../../hooks/useDropUpload'; -import { useFileUpload } from '../../hooks/useFileUpload'; +import { useDropUpload, MAX_FILE_SIZE } from '../../hooks/useDropUpload';
43-47: Catch block is effectively dead code.
handleFileDropinuseDropUploadcatches all errors internally (lines 93–103 ofuseDropUpload.ts) and never re-throws. Thistry/catchwill never enter the catch branch under normal operation, so the localerrorstate on line 46 won't be set from this path.This is a minor concern — the defensive catch is harmless — but it means the local
errorstate only serves therejectedFilespath (lines 29–38). Consider whether the local error display is still needed or if the upload store's error display (viaUploadModal) is sufficient.apps/web/src/components/file-browser/FileListItem.tsx (1)
238-256: Minor: redundantgetData('application/json')call for internal drops.For internal drag operations,
getData('application/json')is called on line 242 to check if it's an external drop, then called again on line 255 for the actual parsing. This is harmless (idempotent) but could be simplified by reusingjsonData.♻️ Reuse the already-fetched data
// Check for external file drop first if (e.dataTransfer.files && e.dataTransfer.files.length > 0) { const jsonData = e.dataTransfer.getData('application/json'); // No JSON data means this is an external file drag from OS if (!jsonData && onExternalFileDrop) { const files = Array.from(e.dataTransfer.files); onExternalFileDrop(files, item.id); return; } } // Internal move operation if (!onDrop) return; try { - const data = e.dataTransfer.getData('application/json'); + const data = e.dataTransfer.getData('application/json'); // already fetched above as jsonData if (!data) return;A cleaner approach would be to hoist
jsonDatato the top of the function and reuse it in both branches, but this is a small nit.apps/web/src/components/file-browser/FileBrowser.tsx (1)
143-175: Drag counter pattern is well-implemented, with one edge case to be aware of.The enter/leave counter approach correctly handles nested elements. However, if a user drags an external file over the content area and then moves the cursor outside the browser window (abandoning the drag without dropping), the
dragleaveevent sequence may not fully decrement the counter to zero on all browsers, leavingisDraggingExternalstuck astrue.A common safeguard is to also reset state on the
dropevent at the document/window level, or reset on the nextdragenter. YourhandleContentDropalready resets the counter (line 184), andhandleExternalFileDrop(line 204) covers child-intercepted drops. The remaining gap is the "drag away from window" scenario. In practice this is low-impact — the stale green outline is cosmetic and clears on the next interaction — but worth noting.apps/web/src/components/file-browser/Breadcrumbs.tsx (1)
119-153: Duplicated external-vs-internal drop detection pattern.The drop detection logic (lines 119–127) is virtually identical to
FileListItem.tsx(lines 241–248). Both check forfiles.length > 0, readgetData('application/json'), and branch on presence of JSON data. This is a good candidate for a shared helper inuseDropUpload.ts, e.g.:export function routeDropEvent( e: DragEvent, onExternal: (files: File[]) => void, onInternal: (jsonData: string) => void ): void { ... }Not blocking — the current approach is readable and the duplication is bounded. Just a thought for future cleanup.
apps/web/src/hooks/useDropUpload.ts (1)
98-102: Static analysis:forEachcallback implicitly returns a promise value.Biome correctly flags this. The arrow function
(f) => unpinFromIpfs(f.cid).catch(() => {})implicitly returns the Promise, whichforEachdiscards. Usevoidto make the fire-and-forget intent explicit:♻️ Silence the implicit return
- uploadedFiles.forEach((f) => unpinFromIpfs(f.cid).catch(() => {})); + uploadedFiles.forEach((f) => { unpinFromIpfs(f.cid).catch(() => {}); });apps/web/src/components/file-browser/UploadZone.tsx (1)
31-32: RedundantuseFileUploadcall —useDropUploadalready exposesisUploading.
useDropUpload()returns{ handleFileDrop, isUploading }(seeuseDropUpload.tsLine 108). InstantiatinguseFileUploadseparately creates a second subscription to the same underlying store for no benefit.Proposed fix
- const { handleFileDrop } = useDropUpload(); - const { isUploading } = useFileUpload(); + const { handleFileDrop, isUploading } = useDropUpload();And remove the unused import on line 4:
-import { useFileUpload } from '../../hooks/useFileUpload';
Critical:
- handleFileDrop now returns boolean success indicator so
UploadZone.onUploadComplete only fires after successful upload,
not after validation failures or errors.
Nitpicks:
- Remove redundant useFileUpload() in UploadZone and EmptyState;
get isUploading from useDropUpload instead.
- Remove dead catch blocks in UploadZone/EmptyState since
handleFileDrop handles all errors internally.
- Consolidate double getData('application/json') call in
FileListItem and Breadcrumbs drop handlers into single read.
- Add window dragend/dragleave/drop listeners in FileBrowser
to reset drag counter when user abandons drag outside window.
https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/web/src/hooks/useDropUpload.ts`:
- Around line 99-103: The Biome lint error is caused by the arrow callback in
uploadedFiles.forEach((f) => unpinFromIpfs(f.cid).catch(() => {})) implicitly
returning a Promise; change the cleanup to avoid returning a value by either
prefixing the call with void (e.g., uploadedFiles.forEach((f) => { void
unpinFromIpfs(f.cid).catch(() => {}); })) or replacing the forEach with a
for...of loop (e.g., for (const f of uploadedFiles) { void
unpinFromIpfs(f.cid).catch(() => {}); }) and keep the subsequent
useQuotaStore.getState().fetchQuota() call unchanged.
- Around line 64-73: The intra-batch duplicate check is currently inside the if
(folder) block so it’s skipped when folder is undefined; move the block that
creates batchNames and iterates over files (the Set<string> batchNames loop that
calls useUploadStore.getState().setError on duplicates) to run before or outside
the if (folder) guard so it always executes regardless of folder/cache state;
keep the same error message and early return behavior when a duplicate is found
and leave any folder-specific checks inside the if (folder) block.
🧹 Nitpick comments (1)
apps/web/src/components/file-browser/FileBrowser.tsx (1)
209-223:handleFileDropresult is discarded — intentional?Both
handleContentDrop(line 220) andhandleExternalFileDrop(line 233) callhandleFileDropwithoutawait-ing or inspecting the returnedPromise<boolean>. SincehandleFileDropmanages errors internally via the upload store, this works, but any unhandled rejection (e.g., an unexpected throw fromaddFiles) would surface as an unhandled promise rejection rather than being caught here.Consider
await-ing or at minimum attaching a.catchto avoid unhandled-rejection warnings in stricter environments.Example
const handleContentDrop = useCallback( (e: DragEvent) => { dragCounterRef.current = 0; setIsDraggingExternal(false); if (!e.dataTransfer.files || e.dataTransfer.files.length === 0) return; if (!isExternalFileDrag(e.dataTransfer)) return; e.preventDefault(); const files = Array.from(e.dataTransfer.files); - handleFileDrop(files, currentFolderId); + void handleFileDrop(files, currentFolderId); }, [handleFileDrop, currentFolderId] );Also applies to: 229-236
…(Pt 2) (#81) * docs(quick-005): align Pencil design with deployed staging app Reorganized cipher-box-design.pen from scattered exploration frames into a structured reference matching the actual staging deployment. - Deleted 7 old exploration artifacts (Option A/B, Toolbar Options, etc.) - Created 3 labeled sections: Desktop Screens, Mobile Screens, Components - Rebuilt 5 desktop screens showing full E2E flow (login → file browser → context menu) - Rebuilt 4 mobile screens with responsive layout differences (2-col, no sidebar) - Fixed 8+ discrepancies between design and staging (text, colors, layout) - No app code changes needed — staging implementation is accurate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(quick-005): add modal, DnD, scroll, and breadcrumb screens to Pencil design Add 14 new screens (8 desktop + 6 mobile) covering interaction states missing from the initial design alignment: Desktop (screens 6-13): - Rename dialog with green-glow input field - Move dialog with folder tree and selection indicator - Upload in progress with per-file progress bars - DnD drag-to-folder with row/breadcrumb drop targets - DnD upload drop zone with active glow effect - Delete confirmation with destructive red action - Scrollable file list (forward-looking) with scroll indicator - Breadcrumb truncation (forward-looking) with collapsed "..." pill Mobile (screens 5-10): - Rename, Move, Upload, Delete modals (358px width) - Scrollable file list with scroll track - Breadcrumb truncation with aggressive collapse (root/…/current) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add Pencil MCP multi-screen design learnings Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…82) * feat: add file/folder details modal with CID and encryption metadata Add a Details option to the context menu that opens a modal showing technical metadata for files and folders: Files: content CID, folder metadata CID, encryption mode, file IV, wrapped file key (redacted), size, timestamps. Folders: IPNS name, metadata CID (live-resolved), sequence number, wrapped folder key and IPNS key (redacted), child count, timestamps. CID values are copyable to clipboard. Terminal aesthetic maintained. https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * docs(quick-006): add file/folder details modal Quick task completed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * style: fix prettier formatting in DetailsDialog https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * chore: add package-lock.json https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * chore: remove erroneous package-lock.json Project uses pnpm, not npm. https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * docs(learnings): quick-006 - use pnpm not npm https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * docs(quick-006): add Pencil design update spec for details modal https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * fix: address PR review feedback on DetailsDialog - Wrap JSX comment text in braces to fix Biome noCommentText rule - Add setTimeout cleanup via useRef to prevent state updates after unmount - Add :focus-visible style on copy button for keyboard accessibility - Reset metadataLoading on early return when IPNS name is missing - Use null for unloaded folder childCount instead of misleading 0 - Add aria-label and aria-pressed to copy button for screen readers - Apply base details-value class alongside modifier classes (--dim, --redacted) https://claude.ai/code/session_01BACpg8j4E2GzGFJBCnGV9j * feat(design): add file/folder details modal screens and context menu item Add screens 14 (File Details Modal) and 15 (Folder Details Modal) with full overlay pattern, and add "ⓘ Details" to the context menu. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(e2e): add details dialog coverage and todo for flaky reload tests Add DetailsDialogPage page object and tests 3.65/3.66 verifying file and folder details modal (labels, badges, sections, copy buttons, redacted values). Add clickDetails() to ContextMenuPage. Also captures todo for pre-existing flaky tests 3.8/3.10 (IPNS cold-load timing after page reload). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: reset metadataCid when ipnsName is missing on item change Prevents stale CID from a previously viewed item being displayed when the dialog item changes without closing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(learnings): pr review comment triage workflow Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: remove completed pencil design update spec Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
- Move intra-batch duplicate filename check outside the `if (folder)` block so it runs even when the target folder isn't cached yet - Replace forEach with for...of + void to avoid Biome lint error (useIterableCallbackReturn) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/hooks/useDropUpload.ts`:
- Around line 50-63: The duplicate-name check currently skips when
useFolderStore.getState().folders[folderId] is undefined, causing uploads to
proceed then fail server-side; modify useDropUpload.ts to prefetch folder
metadata for folderId before running the duplicate validation: if the folder is
missing, call the existing folder-loading routine (or dispatch the same fetch
used elsewhere) to populate useFolderStore (so folder =
useFolderStore.getState().folders[folderId] is available), then re-run the
existing duplicate-detection logic (existingNames / duplicates) and call
useUploadStore.getState().setError on duplicates (same message) returning false;
ensure this happens before calling addFilesToFolder() so the client fails fast.
🧹 Nitpick comments (1)
apps/web/src/hooks/useDropUpload.ts (1)
94-108:(err as Error).messagemay beundefinedfor non-Error throws.If
uploadoraddFilesrejects with a non-Errorvalue (e.g., a string or an Axios error wrapper), casting toErrorand accessing.messagecould yieldundefined, which would then be passed tosetError(undefined).A small defensive guard would be safer:
Suggested fix
} catch (err) { - const message = (err as Error).message; + const message = err instanceof Error ? err.message : String(err); if (message !== 'Upload cancelled by user') {
Files dragged from the OS file manager can now be dropped onto: - The file browser area (uploads to current folder) - Folder rows in the file list (uploads to that folder) - Breadcrumb segments (uploads to that folder) Internal drag-and-drop (move operations) continues to work as before. External drags show a 'copy' cursor and visual feedback on the content area. A new useDropUpload hook extracts reusable upload-on-drop logic. https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
Address PR review feedback: 1. Extract isExternalFileDrag(dataTransfer) helper into useDropUpload to replace repeated Files/application-json type checks across FileBrowser, FileListItem, and Breadcrumbs. 2. Refactor UploadZone and EmptyState to delegate upload logic to useDropUpload hook, eliminating triple-duplicated validation, quota checking, duplicate detection, and IPFS cleanup code. 3. Export MAX_FILE_SIZE from useDropUpload as single source of truth. https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
Critical:
- handleFileDrop now returns boolean success indicator so
UploadZone.onUploadComplete only fires after successful upload,
not after validation failures or errors.
Nitpicks:
- Remove redundant useFileUpload() in UploadZone and EmptyState;
get isUploading from useDropUpload instead.
- Remove dead catch blocks in UploadZone/EmptyState since
handleFileDrop handles all errors internally.
- Consolidate double getData('application/json') call in
FileListItem and Breadcrumbs drop handlers into single read.
- Add window dragend/dragleave/drop listeners in FileBrowser
to reset drag counter when user abandons drag outside window.
https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
- Move intra-batch duplicate filename check outside the `if (folder)` block so it runs even when the target folder isn't cached yet - Replace forEach with for...of + void to avoid Biome lint error (useIterableCallbackReturn) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…/FSM1/cipher-box into claude/add-drag-drop-files-1cAdm
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)
apps/web/src/components/layout/StorageQuota.tsx (1)
8-18:⚠️ Potential issue | 🟡 Minor
formatBytesdoesn't handle negative input gracefully.If
usedByteswere ever negative (e.g., due to a staleremoveUsagecall),Math.log(negative)yieldsNaN, and the function would return"NaN undefined". A minor guard would prevent a confusing UI.Proposed fix
function formatBytes(bytes: number): string { if (bytes === 0) return '0 B'; + if (bytes < 0) return '0 B'; const units = ['B', 'KB', 'MB', 'GB', 'TB'];
🧹 Nitpick comments (8)
tests/e2e/page-objects/file-browser/context-menu.page.ts (1)
19-31: Pre-existing JSDoc mismatch: comments don't match methods.Line 20 says "Get the Download menu option" but the method is
renameOption(). Line 28 says "Get the Rename menu option" but the method isdeleteOption(). Not introduced by this PR, but worth fixing while you're here.📝 Suggested fix
/** - * Get the Download menu option (files only). + * Get the Rename menu option. */ renameOption(): Locator { return this.menu().locator('button[role="menuitem"]', { hasText: 'Rename' }); } /** - * Get the Rename menu option. + * Get the Delete menu option. */ deleteOption(): Locator { return this.menu().locator('button[role="menuitem"]', { hasText: 'Delete' }); }tests/e2e/tests/full-workflow.spec.ts (1)
374-383: Storage quota assertion is a good regression test.One consideration: the regex
/^0 B\s*\//assumes a specific format from the quota component. If the format changes (e.g., "0 Bytes"), this test would silently pass even with zero usage. Consider adding a positive match as well (e.g., checking that the text contains a non-zero number).apps/web/src/components/file-browser/DetailsDialog.tsx (1)
249-319: Consider resettingmetadataLoadingwhenopenbecomes false.When the dialog closes (
opengoes false),metadataCidis reset tonullon line 264, butmetadataLoadingis only reset in theipnsNameabsence branch (line 271). If the dialog is closed while IPNS is still resolving,metadataLoadingcould remaintruewhen reopened for a brief moment before the new effect runs. This is a very minor timing edge case since the effect re-runs immediately on reopen.🔧 Optional: reset loading state on close
if (!open || !item) { setMetadataCid(null); + setMetadataLoading(false); return; }apps/web/src/components/file-browser/FileBrowser.tsx (2)
210-224: Fire-and-forgethandleFileDrop— consider awaiting for error visibility.
handleFileDropreturnsPromise<boolean>but the result is silently discarded here (and inhandleExternalFileDropat Line 234). SinceuseDropUploadmanages errors internally viauseUploadStore, this works in practice. However, the floating promise means an unhandled rejection from an unexpected throw path would be swallowed silently.Consider adding
voidannotation to make the intent explicit, orawait/.catchfor safety:Suggested minimal change
- handleFileDrop(files, currentFolderId); + void handleFileDrop(files, currentFolderId);
432-446: Content area drop zone lacks an accessible hint for assistive technology.The
divis now a drop target but has noroleoraria-*attribute indicating it accepts dropped files. SinceUploadZoneprovides an accessible file-input fallback this isn't a blocker, but adding a subtlearia-labelor visually-hidden instruction whenisDraggingExternalis true would improve the experience for assistive-tech users.apps/desktop/src-tauri/src/fuse/operations.rs (1)
1146-1148: Metadata snapshot staleness under concurrent uploads to the same folder.The publish lock serializes IPNS sequence numbers, but the metadata snapshot (
self.build_folder_metadata(parent_ino)at Line 1128) is captured on the main FUSE thread before the background thread runs. If two files are written to the same folder concurrently, the second publish may overwrite the first file's metadata entry (since its snapshot was taken before the first upload completed and updated the CID).This is a pre-existing architectural limitation (not introduced by this PR), and the next
drain_refresh_completionscycle will eventually re-sync from IPNS. Just noting for awareness.apps/desktop/src-tauri/src/fuse/mod.rs (2)
100-107:publish_locksmap grows unbounded (one entry per IPNS name, never pruned).Each unique IPNS name adds an
Arc<tokio::sync::Mutex<()>>that is never removed. In practice this is bounded by the number of folders (likely small), so it's not a memory concern today. Just noting for future awareness if the folder count grows significantly.
185-254: Lock held across network I/O inspawn_metadata_publish— correct but could release before unpin.The per-folder lock is intentionally held across the full publish sequence (encrypt → resolve → upload → sign → publish → record), which is necessary for correctness. However, the lock is also held during
unpin_content(Line 242-244), which is fire-and-forget and doesn't affect sequencing. Releasing the lock before unpinning would reduce contention marginally.Optional: release lock before unpin
// Record successful publish in coordinator cache coordinator.record_publish(&ipns_name, new_seq); + // Release publish lock before fire-and-forget cleanup + drop(_guard); + // Unpin old metadata CID if let Some(old) = old_metadata_cid { let _ = crate::api::ipfs::unpin_content(&api, &old).await; }
The UI keyword heuristic was missing interaction terms (drag, drop, hover, animation, etc.) causing tasks like DnD to skip the design workflow. Unified and expanded the keyword list across all 3 locations: quick.md, discuss-phase.md, and gsd-verifier.md. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 4 new Pencil design screens (15-18) showing DnD visual feedback: - Drop on content area: green border + overlay prompt - Drop on folder row: row highlight with left accent - Drop on breadcrumb: segment highlight box - Drop on empty state: border + prominent upload CTA Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace silent unwrap_or(0) with explicit match that logs a warning when the API returns a non-numeric sequence number. Prevents silent sequence rollback on cold cache. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Math.log(negative) yields NaN, causing "NaN undefined" display. Treat negative values the same as zero. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Files dragged from the OS file manager can now be dropped onto:
Internal drag-and-drop (move operations) continues to work as before.
External drags show a 'copy' cursor and visual feedback on the content
area. A new useDropUpload hook extracts reusable upload-on-drop logic.
https://claude.ai/code/session_016z77d1zrWcRefW1Lhogprh
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests