Files
ersatztv/web
timothyandClaude Opus 5 9ba397e1dd fix(830): a write failure reports to a surface that outlives the dialog that started it
`AddItemsDialog.submit` POSTed to `/api/v1/collections/{id}/items` and reported failure into a
banner rendered from its OWN state. The dialog is dismissible mid-request through three paths that
never consult `adding` -- Escape and a backdrop click (both `useOverlayBehavior`) and the header
close button -- and the caller remounts it on `key={`add-${pickerOpen}`}`, so dismissal genuinely
unmounts it. Select 12 items, Add, press Escape, the request fails: nothing surfaces, the list
reloads unchanged, and the user believes 12 items were added.

drop deliberate rather than accidental. A deliberate drop is still a user who is told nothing.

The asymmetry is the finding: SUCCESS already outlived dismissal everywhere here, because it is
reported through a parent callback (`onAdded`/`onDone`, which the screens turn into a `Toast`).
Only failure died with the surface. So this is not a new notification system -- it routes failure
through the channel success already uses. `AddToMenu` had `onDone` and no counterpart at all.

`useDismissSafeError` (`web/src/hooks.ts`) renders the message INLINE while the surface is mounted
-- the better surface, since it keeps the user's selections and context -- and diverts to a
caller-supplied `onFailed` once it is gone. The surviving surface belongs to the parent and differs
per screen (a `role="alert"` banner on CollectionsScreen, `notice`+`Toast` on MediaBrowse/Search),
so it is a prop contract rather than a rendering decision. Gating dismissal on the busy flag was
considered and rejected: it traps the user behind an in-flight request with no cancel path, and
would not cancel the write anyway.

Applied to the A1 shape -- where the surface owns the error state and is really unmounted:
AddItemsDialog plus the four `web/src/media/addTo/` dialogs, whose failures previously could not
reach the screen Toast that already showed their successes.

Proofs, executed rather than described:
- deleting `reportRef.current(message)` alone reddens the new CollectionsScreen test on
  `Unable to find an element with the text: Request failed with status 500`
- `hooks.test.tsx` pins both branches directly, plus that the report goes through the LATEST
  callback rather than the one captured on first render
- `CollectionsScreen.guards.test.tsx`'s is-mounted read count moves 2 -> 1 because the catch's
  guard migrated into the hook (its `...actual` module mock cannot see the hook's internal
  `useIsMountedRef()`); removing the surviving `finally` guard takes it to 0 and reddens, so the
  anti-masking property that count was added for is intact

Scope is stated rather than implied. A sweep of all 68 Dialog/ConfirmDialog/SlideOver call sites
found three shapes; only A1 is fixed here. A2 -- error state that survives but whose render site is
gated by the same condition dismissal clears, mostly delete-confirm flows -- is left open in #877
because its right answer is probably a shared surface, not twenty prop threads.

fixes #830
refs #877, #740, #685

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XE2tF2aUasK2hWPmBRsrMY
2026-09-05 00:23:08 +02:00
..
2026-07-07 10:05:39 +02:00
2026-07-02 18:36:56 +02:00
2026-07-07 10:05:39 +02:00