1615838f333a9d3d6d90aae6341eb41b4ee0151c
6
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 peter
|
569190480d |
refactor: hook dependencies name everything the hooks read
The 12 react-hooks/exhaustive-deps warnings the gate let through: - ShareDialog and YearFilter built a fresh [] per render while their query loaded, so their memos recomputed on every render; they share one empty list now. - VideoEditor's drag effect called a seek/moveBoundary recreated each render; both are stable callbacks in its deps. - WizardProvider re-checks onboarding on exactly the fields it reads (not the me object, whose 60 s refetch would reopen a closed wizard). - Channels, Feed and PlexPlayer list the stable values they already used. - The player's keydown effect is bound once on purpose (its helpers read refs): said so, with a disable, as the codebase's other deliberate sites do. - dialogFocus and the YouTube player's cleanup read refs React may already have let go of; they use what they captured when the effect ran. |
||
|
peter
|
b4aea7fa5b |
fix(ui): dialogs, menus and busy buttons keep the keyboard focus where it belongs
Measured in Chrome before each change: - A dialog opened from a menu lost its focus: the menu row ran its action and closed in one batch, and floating-ui's focus return (a microtask) moved the focus to the row BEHIND the confirm dialog's scrim 7 ms after it opened, where Enter activated the row. MenuItemList now closes first and runs the action a task later; the dialog also captures the row, not a dying menu item, as its restore target. - A side panel opened as a flyout collapsed the moment a dialog opened from inside it, unmounting the row the dialog hands the focus back to. useHoverFocusWithin holds while the focus is inside an aria-modal dialog. - ui/Button's loading state set native disabled; Chrome moved the focus of the pressed button to <body> and never gave it back. Loading is now aria-disabled with a click guard that also cancels implicit submission. - Escape/Enter in the playlist rail's rename field dropped the focus on <body>. The row refocuses from its ref callback when it re-attaches (a post-commit focus sticks; the earlier attempt ran before the button existed). - PlayerModal was the one aria-modal shell without the dialog contract: it is now named by its title, traps Tab, restores the focus to its opener and shares one counted body scroll lock with every dialog. |
||
|
peter
|
c7a377396b |
fix(ui): a closing dialog leaves the focus to the dialog standing above it
Round 6 widened dialog focus ownership to count focus inside ANY `aria-modal` subtree as ours, so a dialog and the dialog stacked on it closing together would still hand the focus back. That clause has no way to tell a layer closing WITH us from one that stays open, so a dialog closing on its own — an in-flight mutation's `onSuccess`, not a click, since a scrim covers it — under a modal raised above it pulled the focus onto its own pre-open trigger, behind that scrim, where Enter still activated it. Three verifiers confirmed the mechanism and two corrected the severity down: it self-heals on the next Tab, which is why this is a containment violation rather than a broken end state. `lib/layerStack` cannot refine it. Measured against the real hook: at the instant the lower dialog's focus cleanup runs, the both-closing case and the sibling-stays-open case give identical readings — `anyLayerAbove` true, the upper layer topmost — because passive cleanups run parent-first and `useDialogChrome` is declared before the effect that calls `removeLayer`. Gating on the stack turns round 6's own test red. What separates them is the DOM: React detaches a deleted subtree before passive cleanups, so a layer closing with us is already disconnected while one that stays open is not. Both tests are green, and each was mutation-checked against the other. Verified in Chrome on localdev: opening the cheat sheet from a sidebar button and closing it restores that button; with the command palette stacked over the cheat sheet, closing the top one leaves the focus neither on the background trigger nor on <body>, and closing both returns it to the trigger. Welcome's Lightbox got the trap and the restore in round 6 but not the layer membership the other shells have, so its Escape and Tab fired from a raw window listener whatever was on top — the shape the wizard's comment records being bitten by, unreachable today only by a mount ordering nothing enforces. It joins the stack. Its comment also read as though the four-shell sweep were finished: `PlayerModal` is still an `aria-modal` surface with no name, no trap and no restore, which is on ROADMAP.md now rather than implied to be done. |
||
|
peter
|
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. |
||
|
peter
|
1f81684b6e |
fix(ui,plex): a dialog keeps the focus its child asked for, and a hopeless segment fails at once
Both found by UAT in a real browser, after the tests for them were already green — which is the whole reason this repo settles focus and timing claims by running them. - **The autoFocus fix worked, and then the dialog took the focus back.** React 18's StrictMode runs every effect mount→cleanup→mount in development: the cleanup handed focus back to the trigger, and the second mount, finding the focus outside the dialog, moved it to the first focusable — off the `autoFocus` field a beat after the browser had put it there. Chrome measured it: INPUT (commitMount) → BODY (cleanup) → ✕ (re-mount). The effect is idempotent now — it remembers what the dialog was focused on and restores THAT on a re-mount — and the close-time restore is conditional on the dialog still holding the focus, which it tracks as it changes rather than computing at cleanup, where the panel is already out of the document. A dialog no longer yanks focus back from wherever the user has since moved it. - **Passing ffmpeg's handle to the segment wait only shortened the FINISHED case.** Measured against the running localdev API: a request for `seg_99999.ts` on a live session still took 20 195 ms, because that ffmpeg has not exited and the wait has no other reason to stop. ffmpeg writes segments in order, so a segment whose predecessor does not exist is not the next one and cannot arrive — and that is knowable at once. Same request now: 45 ms. Verified on localdev against the real library (Westworld S01E06, remux, multi-audio): Mod+K opens the command palette with the focus in its search field and typing reaches it; playback runs on the new session-scoped HLS URL; a request naming a stale session id is a 404; and a viewer paused for longer than the finished-session rule keeps their 2.4 GB session across two reaper passes — the progress checkpoint touches it — and resumes cleanly. vitest 1009, pytest 1127. |
||
|
peter
|
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). |