[pratikbuilds]

Attachments renders files as a stack above the input, tiles in a message, or a list, with preview, info, remove and more parts. The host owns the list: useAttachments applies the count, size and duplicate limits, builds previews and revokes every object URL it made once its attachment leaves the list. useFileDrop and AttachmentDropZone accept files and whole folders, and a stray drop elsewhere never replaces the page.

Stacked on #123: review only this commit. Part of #94.

Notes for review

• Hosts call release(ids) before moving attachments into a sent message, so the message keeps its previews (otherwise their object URLs are revoked).
• Files from a dropped folder are named by their path ("project/src/index.ts") so same-sized files in different folders aren't refused as duplicates. Folder walks have an entry budget (default 500) and skip hidden entries.
• "Show less" and "Remove all" are host buttons, not library parts; no text excerpts in previews.
• Drag-over state uses an enter/leave depth counter (WebKit omits relatedTarget on drag events). When the last attachment is removed, focus goes to returnFocusTo.

Verification

• This commit passes bun run typecheck and bun run lint on its own (the stack is checked commit by commit).
• At the top of the stack: bun run check, bun run build, bun run dep-guard, bun run contrast-test and bun run test:e2e (Playwright specs and the Storybook axe sweep in light and dark) pass.

Closes #104

[corbits-triage]

Triage: Fail

Asking a maintainer about:

• pull request is over the lines limit: 3443 lines changed (max 3000)

Stacked on #123: only this pull request's own changes were checked.

Notes:

• added code matches a code pattern: e2e/attachments.spec.ts:87 Promise chain; use async/await with try/catch., src/hooks/use-file-drop.test.ts:77 Promise chain; use async/await with try/catch., src/hooks/use-file-drop.ts:158 Immediately invoked function; name the function., src/hooks/use-file-drop.ts:374 Promise chain; use async/await with try/catch., src/ui/attachments.tsx:122 Nested ternary; use if statements or a lookup., src/ui/attachments.tsx:124 Nested ternary; use if statements or a lookup., src/ui/attachments.tsx:126 Nested ternary; use if statements or a lookup., src/ui/attachments.tsx:347 Nested ternary; use if statements or a lookup.
• Risk: medium (3443 lines)

Changed since the last verdict (Unsure at a3e9b62): 86 new commits.

[TheGreatAxios]

Automated multi-agent review: independent review + second-agent validation; only confirmed items listed (the top finding verified by an independently re-run repro).

• minor (VERIFIED by repro) src/hooks/use-attachments.ts:99-110 (with :89-97, 112-142) — a <Activity mode="hidden"> hide spanning more than one tick revokes every preview URL while the host's list still holds the items, and nothing rebuilds: the mount effect's cleanup forks a microtask that revokes all owned URLs (Activity unmounts effects but preserves state), and on re-show the per-commit effect only prunes and add() mints URLs for new files only. Result: re-showing leaves dead blob URLs behind every held attachment, silently degraded to type tiles by AttachmentPreview's onError. Smallest fix: recreate object URLs from the held item.file for held ids whose URL was revoked (or skip revocation when the hide leaves the root mounted), with a regression test: hide → a tick → show → previews still fetchable.
• minor a11y src/ui/attachment-drop-zone.tsx:62-73 — drag-over state is invisible to assistive tech: the overlay (including the "Drop files to attach" label) is aria-hidden, and the zone is a bare div with no role/name and no live-region twin of data-over. The pointer-only design is deliberate and documented, but an sr-only role="status" twin of the label plus a way for the host to name the zone would stop the total silence.
• minor (test gap) src/hooks/use-file-drop.ts:314-321 — the disabled option is implemented but never tested (mountTarget has no disabled parameter; the only "disabled" test covers usePreventStrayDrops(false)). One enter/over/leave/drop test with disabled: true, asserting over === false, onFiles uncalled, no preventDefault.
• nits confirmed — AttachmentPreview swallows a host aria-label (attachments.tsx:503-514; deliberate but asymmetric with AttachmentRemove's label prop); focusAfterExpand assumes AttachmentMore renders last (:686-694; document the contract); both Provider value={{…}} objects are fresh every render (:234-243, :350 — wrap in useMemo); the uncontrolled-expanded half (AttachmentsState, defaultExpanded, children-as-function) is public surface with zero consumers at the series tip (make private or exercise it); four use-file-drop exports (carriesFiles, DroppedFiles, FileDropTargetProps, UseFileDropResult) are public with zero consumers (un-export or defer).

[pratikbuilds]

Thanks for the review. Here's what happened to each point.

Fixed

1. Hiding attachments inside <Activity> for more than a moment broke their previews (minor)

• The fix: the hook can't tell a hide from an unmount, so instead each Attachment rebuilds its picture from the original file when its URL fails to load. It then owns that URL and cleans it up. AttachmentData gains an optional file for this. The docs explain that the URL in the host's own list does stay dead after such a hide.
• Tests: two new tests, both of which failed before the fix.

2. disabled on useFileDrop had no tests (test gap)

• The fix: covered every drag event while disabled, and disabling in the middle of a drag.

3. The drop zone gives screen readers nothing (minor)

• The fix: a host can already pass role and aria-label, and the docs now show how to make it a named region. I didn't add a live region for drag-over: it's a pointer-only gesture, and keyboard users attach files through the file input.

4. Nits

• AttachmentPreview now takes a label, matching AttachmentRemove.
• The docs say AttachmentMore must render last.
• The uncontrolled expand/collapse now has tests.
• carriesFiles is no longer exported.

Not changed, and why

• The other use-file-drop type exports: they name the return types of exported functions, so hosts need them.
• useMemo on the context values: they change whenever the state changes anyway, so it wouldn't save anything.

Commit: fix(attachments): rebuild revoked previews and cover the disabled option

[TheGreatAxios]

Review · Request changes

Adds attachment chips and a file drop zone, with count, size, and duplicate limits.

Findings

• src/hooks/use-attachments.ts:138 — Nameless files skip the duplicate check, then collide with a real Untitled. checkAttachments keys on the raw file.name (attachment-limits.ts:28) before add stores "" as "Untitled". Two empty-named files of the same size are both accepted, and a later file that is actually named Untitled with that size is refused. Run the limit check on the name that will be stored.

• src/hooks/use-file-drop.ts:344 — A drop on a nested picker is taken twice. FileInput handles drop and calls onFiles without stopping the bubble (file-input.tsx:55-60). The zone then reads the same files and calls onFiles again, so the story's picker path (attachments.stories.tsx:304-307) accepts the file and then reports duplicate. Skip a drop whose defaultPrevented is already set.

• src/hooks/use-file-drop.ts:281 — A folder walk that started while the zone was enabled still delivers after disabled flips. takes checks disabled only on the event (:318-319). latest does not keep disabled, and the readDroppedFiles callback (:353-358) delivers unconditionally. Re-check disabled before deliver.

• src/hooks/use-file-drop.ts:353 — A folder walk that finishes after unmount leaks the object URL. Unmount revokes URLs in a microtask (use-attachments.ts:109-112) and add never checks mounted. If readEntries resolves after that microtask, add calls URL.createObjectURL (use-attachments.ts:134) and nothing revokes it. Do not call onFiles once the drop hook has unmounted, or have add refuse work when mounted is false.

[pratikbuilds]

Thanks for the review. All four points are fixed.

1. Nameless files skipped the duplicate check

• The fix: add checks each file under the name it will be stored as, so an empty name is checked as "Untitled". rejected[].file is still the original File.
• Test: "a nameless file is checked as the Untitled it is held as". It checks that a second nameless file and a real Untitled of the same size are both refused as duplicates. It failed before the fix.

2. A drop on a picker inside the zone was taken twice

• The fix: the zone still clears its hover state. Then, if a child has already handled the drop (isDefaultPrevented()), the zone leaves it to that child. The docs for useFileDrop and AttachmentDropZone say that a child that takes its own drops keeps them.
• Tests:
  • The unit test "a drop a child took itself is left to it".
  • The Playwright case "a drop on the picker inside the zone is taken once". The attachments fixture now nests a FileInput in the zone. Before the fix, this case got the same "duplicate" refusal as the story.
  • Both failed before the fix.

3. A folder walk still delivered after the zone was disabled

• The fix: disabled is now kept with the hook's latest options. A read that finishes once the zone is disabled is dropped, errors included, and the disabled docs say so. The latest options are now written in a layout effect instead of during render.
• Test: "a drop still being read when the target is disabled is dropped". It failed before the fix.

4. A folder walk that finished after unmount leaked an object URL

• The fix: both suggestions are done.
  • useFileDrop drops a read that finishes after unmount. Hiding the zone in <Activity> counts as an unmount too, which matches how useAttachments already treats a hide.
  • useAttachments makes no object URL for an add that comes after it has unmounted. The item keeps its file, and Attachment already rebuilds the preview from it.
• Tests: "a drop still being read when the target unmounts is dropped" and "an add after unmounting makes no preview URL". Both failed before the fix.

Commit: fix(attachments): take a nested picker's drop once and drop late reads

[pratikbuilds]

Round 3 fixes

A fresh review pass over the stack turned up the points below; each is fixed in a new commit at the end of this branch, and the stack is rebased onto it.

• 7a1d69f: A folder dropped on the FileInput inside the zone was left to the picker and arrived as an unreadable entry. The zone now reads drops in the capture phase, so the folder is walked and reaches onFiles once. FileInput skips a drop something around it already handled (standalone use is unchanged).
• e2ede9b: An item added after unmount or a long hide had a file but no url and never showed its picture. The preview now builds its URL from the file when there is none (images and video only) and revokes it on unmount.
• c555eab: useAttachments wrote its latest-options ref during render; it is now written in a layout effect, so add checks against the committed list only.
• b243d48: The StrictMode comment is back above the test it describes.
• 35e8e05: React 18 refs: components that read ref from props only receive it on React 19, while the peer range allows ^18.2.0. They are now wrapped in forwardRef, so a host ref reaches the element on both; the peer range is unchanged. Covers Attachments, Attachment and AttachmentPreview.

bun run check and dep-guard pass on every branch of the stack, and the full e2e suite passes at the top of the stack (163/163, including the stories axe sweep). Each behaviour fix comes with a test that fails without it; the forwardRef tests can only guard, since the suite runs on React 19.