fix(248): focus-trap the shared modal overlay so Tab can't escape a dialog #561

2026-07-22T18:56:16+02:00

timothy commented

2026-07-22 18:56:16 +02:00

Fixes #248.

What

Adds a keyboard focus trap to the shared useOverlayBehavior hook in web/src/components/overlay.tsx, so Tab / Shift+Tab cycle only among the focusable elements inside the open panel and can never reach controls on the background screen. Because both Dialog and SlideOver (and therefore ConfirmDialog) share this hook, the fix closes the a11y gap for every SPA dialog at once.

How

Folded the Tab handling into the existing document keydown effect that already handles Escape (keyed on open), matching the file's idiom; deps become [open, panelRef].
Focusable set is recomputed on each Tab via panel.querySelectorAll with the standard selector, so dynamic dialog contents are handled; the panel's own tabIndex={-1} excludes it from the stops.
Boundary-only interception: Shift+Tab at the first stop (or focus off the stops) → last; Tab at the last stop (or off the stops) → first; middle moves left to the browser. Focus sitting outside the panel is redirected back in (index === -1 case). Empty-focusable edge refocuses the panel to avoid a dead Tab.
No new dependency.

Test

Extended web/src/components/overlay.test.tsx (dependency-free fireEvent, matching existing tests) with a trap test rendering <Dialog open> (two body buttons + Close) alongside an outside button: two forward cycles stay inside and never hit the outside control, forward wrap last→first, Shift+Tab first→last, and focus parked outside is redirected back in. Given a per-test { timeout: 15000 } per the heavy-render web-test convention.

Verification

npm run typecheck clean; npm run build OK; eslint clean on touched files.
Overlay suite: 13 passed. Full web suite: 922 passed (102 files).
Pure-SPA leaf, no server-state effect → independent cross-model review skipped per the review rubric; author cold-review done.

Docs

No doc update required: no route/endpoint/convention change (shared component internals only).

Fixes #248. ## What Adds a keyboard focus trap to the shared `useOverlayBehavior` hook in `web/src/components/overlay.tsx`, so Tab / Shift+Tab cycle only among the focusable elements inside the open panel and can never reach controls on the background screen. Because both `Dialog` and `SlideOver` (and therefore `ConfirmDialog`) share this hook, the fix closes the a11y gap for **every** SPA dialog at once. ## How - Folded the Tab handling into the existing document `keydown` effect that already handles Escape (keyed on `open`), matching the file's idiom; deps become `[open, panelRef]`. - Focusable set is recomputed on each Tab via `panel.querySelectorAll` with the standard selector, so dynamic dialog contents are handled; the panel's own `tabIndex={-1}` excludes it from the stops. - Boundary-only interception: Shift+Tab at the first stop (or focus off the stops) → last; Tab at the last stop (or off the stops) → first; middle moves left to the browser. Focus sitting outside the panel is redirected back in (`index === -1` case). Empty-focusable edge refocuses the panel to avoid a dead Tab. - No new dependency. ## Test Extended `web/src/components/overlay.test.tsx` (dependency-free `fireEvent`, matching existing tests) with a trap test rendering `<Dialog open>` (two body buttons + Close) alongside an outside button: two forward cycles stay inside and never hit the outside control, forward wrap last→first, Shift+Tab first→last, and focus parked outside is redirected back in. Given a per-test `{ timeout: 15000 }` per the heavy-render web-test convention. ## Verification - `npm run typecheck` clean; `npm run build` OK; eslint clean on touched files. - Overlay suite: 13 passed. Full web suite: **922 passed (102 files)**. - Pure-SPA leaf, no server-state effect → independent cross-model review skipped per the review rubric; author cold-review done. ## Docs No doc update required: no route/endpoint/convention change (shared component internals only).

timothy added the bug priority: low frontend labels 2026-07-22 18:56:16 +02:00

timothy added 1 commit 2026-07-22 18:56:16 +02:00

fix(248): focus-trap the shared modal overlay so Tab can't escape a dialog

PR Gates / CI image pin matches docker/ci (pull_request) Successful in 11s

Details

PR Gates / Docs update reminder (pull_request) Successful in 12s

Details

PR Gates / decisions lifecycle (pull_request) Successful in 21s

Details

Build ErsatzTV Image / Functional E2E (curl contracts) (pull_request) Successful in 5m42s

Details

Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 8s

Details

Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 7s

Details

Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 20m21s

Details

Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 20m28s

Details

Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped

Details

ae4408c6b2

focus trap added to shared useOverlayBehavior (covers Dialog + SlideOver);
Tab/Shift+Tab now cycle within the panel; test added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

timothy referenced this pull request

2026-07-22 18:57:09 +02:00

SPA dialogs (overlay.tsx) are not focus-trapped / background-inerted #248

timothy commented

2026-07-22 18:57:27 +02:00

Cold author review of the focus-trap diff (pure-SPA leaf, shared useOverlayBehavior internals only — cross-model review skipped per the independent-review rubric). Boundary interception is correct (Tab last→first / off-stops→first; Shift+Tab first→last / off-stops→last; middle left to the browser; empty-focusable refocuses panel to avoid a dead Tab). Escape + scroll-lock behavior preserved; deps [open, panelRef] correct. Full web suite green (922). Noted minor out-of-scope edge: stacked modals each attach a document keydown listener — not a regression, not exercised by current UI.

Review-verdict: MERGEABLE @ ae4408c6

Cold author review of the focus-trap diff (pure-SPA leaf, shared `useOverlayBehavior` internals only — cross-model review skipped per the independent-review rubric). Boundary interception is correct (Tab last→first / off-stops→first; Shift+Tab first→last / off-stops→last; middle left to the browser; empty-focusable refocuses panel to avoid a dead Tab). Escape + scroll-lock behavior preserved; deps `[open, panelRef]` correct. Full web suite green (922). Noted minor out-of-scope edge: stacked modals each attach a document keydown listener — not a regression, not exercised by current UI. Review-verdict: MERGEABLE @ ae4408c6

timothy merged commit 5c73c7a4bd into main

2026-07-22 19:30:15 +02:00

timothy referenced this issue from a commit

2026-07-22 19:30:15 +02:00

Merge pull request 'fix(248): focus-trap the shared modal overlay so Tab can't escape a dialog' (#561) from issue-248-overlay-focus-trap into main

timothy deleted branch issue-248-overlay-focus-trap

2026-07-22 19:30:16 +02:00

timothy referenced this pull request

2026-07-22 19:30:48 +02:00

SPA dialogs (overlay.tsx) are not focus-trapped / background-inerted #248

Sign in to join this conversation.