584d082b6b959fcbc394e8281116ba7d4da2221a
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
f8a31d48cf |
fix(ui): a dialog keeps the focus it was given, and hands it back where it took it from
Round 5 extracted `ui/dialogFocus` so no shell would hand-roll the focus contract, then introduced four ways to lose the focus anyway — all four measured in jsdom against the real components. **Ownership was narrower than the trap.** `ui/Modal` traps Tab over the whole dialog on purpose, so a toast's Undo or ✕ stays reachable; ownership was tracked over the panel alone. Tabbing to that ✕ therefore read as "the user left the dialog", and since removing a toast fires no `focusin` to re-arm, closing afterwards skipped the restore and left the focus on <body>. Before this hook existed the restore was unconditional, so it was a regression. **A dialog stacked above us is still ours.** A nested `ui/Modal` portals into its own host, so it is outside the outer dialog's root however wide that scope is drawn — widening alone could not fix it. Both closing in one commit left the focus on <body>: the outer skipped its restore, and the child's own target (a control inside the outer panel) was already detached, because passive cleanups run parent-first. Ownership now means "inside my dialog, or inside a modal layer above it", which is what the rule always intended. **The wizard disagreed with `ui/Modal` on its first line.** `trapTab(e, panel)`, with neither the dialog-root scope (so its own toast host was outside the cycle — the connection notice never auto-dismisses, and its ✕ was unreachable from the keyboard) nor the `container` argument the JSDoc exists for: `panel.contains( panel)` is true, so focus sitting ON the tabIndex=-1 panel matched no branch and a Shift+Tab walked into the app behind the scrim. Both call sites now pass the same three arguments. **The toast handover was a render-phase read-modify-write.** A `useMemo` read a module global that the next line overwrote; React re-invokes a component body and every `useMemo` factory in it on a StrictMode double render, and the LAST invocation commits — so the second pass re-read what the first had just written, the takeover host classified its OWN new toast as inherited, and the dev build behaved exactly like the code round 5 replaced. Snapshotted through a ref keyed on `isTop`, so a repeated invocation of one render is a no-op. And the fourth `aria-modal` shell, `Welcome`'s screenshot lightbox, is finally swept into the same contract: it declared itself a modal dialog while leaving the focus on the thumbnail behind an 80%-black scrim and letting Tab walk the landing page underneath. The headline fix of the previous commit — StrictMode's re-mount taking the focus off an `autoFocus` field — had no test at all; deleting it left all 1009 green. It has one now. 9 tests, mutation-checked against 8 mutations, each red on the test that names it. |
||
|
|
41b652f8fd |
fix(ui): a dialog opens focused where it said it would, and the wizard is a dialog
Deep-review round 5, over round 4's own fixes. - Round 4 gave `ui/Modal` a persistent portal host so a fullscreen transition would stop tearing the dialog down. The host was attached in a `useLayoutEffect` — one commit phase AFTER React runs `autoFocus` in `commitMount`, and `focus()` on a disconnected node is a no-op. Every autoFocus in every dialog was dead: Mod+K opened the command palette with the focus on the ✕, so typing searched nothing and Enter closed it; the same for the download rename field and every non-danger confirm. The host is attached in a `useInsertionEffect` now, a whole phase earlier, and the whole re-parent — including the focus save/restore across the move, which `appendChild` needs because it is remove-then-insert — lives in one `useFullscreenPortalHost`. `ui/AnchoredPopover` had a second copy of that re-parent WITHOUT the focus restore, and a comment claiming focus survived it; both now come from the hook. - `ui/Toaster` classified a toast raised in the SAME commit as a host takeover among the ones it inherits, because it read the store live at the takeover render. A dialog that notifies and closes in one handler (UploadDialog) therefore put its toast up with no role, into a region still `aria-live="off"` — announced by neither path. "Inherited" is now what the outgoing host last had on screen, and a toast that is NOT inherited is announced even inside the unarmed window. - The onboarding wizard renders its own shell, and Escape ordering was all of the dialog contract it had: no role, no label, no initial focus, no trap, no restore. A keyboard user tabbed out of the first-login wizard into the app behind the scrim. The focus half of the contract is now one shared `ui/dialogFocus` that both `ui/Modal` and the wizard use. Every fix has a regression test, each mutation-checked (6 mutations, all red on the assertion they are named after). |