client: an aria-modal dialog never takes focus, so Tab walks the page behind the scrim (Settings, and five more) #2251

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: 1c93c3c8a21745e6878fb18999f399ef
Branch: dev2/fix/2251-aria-modal-dialog-focus
Base: 6f862cd
At: 2026-11-03T01:39:32Z

Written-by: FlyBot Lead
At: 2026-11-03

Every dialog that declares itself modal never takes focus, so the keyboard starts on the page behind the scrim. Settings is the one people hit first: open it, press Tab, and focus walks the sidebar buttons under the overlay before it ever reaches the dialog.

What the code does at live main 6f862cd0

core/src/client/settings.js:212 renders the dialog node with role: 'dialog', 'aria-modal': true, 'aria-label': t('FlyBot settings') — and nothing that can hold focus:

$ grep -c "tabIndex\|autoFocus\|\.focus(\|preventScroll" core/src/client/settings.js
0

The node has no tabIndex, no ref, no focus effect. core/src/client/overlays.js:55 then portals it to document.body (createPortal(h(SettingsDialog, …), document.body)), while the whole covered application stays in the DOM and stays focusable; .bt-settings-layer is position: fixed; …; z-index: 120 (core/src/client/styles.js:461-462) with a .bt-scrim — a visual cover, not an inert one.

So aria-modal: true claims "the rest is inert" while the code makes the covered half the only reachable region. The dialog is the ninth of the aria-modal sites and none of the six below takes focus.

The repo already has the correct shape

core/src/client/remote-bot-settings.js:33-41, written for #274, takes focus when the sheet opens and gives it back on close without scrolling:

const before = document.activeElement
dialog.current?.focus?.({ preventScroll: true })
// cleanup: before.focus({ preventScroll: true })

with tabIndex: -1, ref: dialog on the node (:45). palette.js:110 (autoFocus) and memory-panel.js:264/questions.js:33 (ref.focus()) are the same idea elsewhere. This is a convention the six dialogs were missed by, not a new requirement.

Same shape, not in this slice

grep -rn "aria-modal" core/src/client/*.js gives 10 sites in 9 files; the six with no focus code at all are settings.js:212, inbox.js:650, kanban.js:453, project-board.js:1109, workflows.js:618 (Rename) and :630 (Delete). Three of them get an incidental autoFocus on their close button or first input, which is not the same thing.

Reproduce it

1. Click the sidebar gear → Settings opens → press Tab: focus moves to the next sidebar button, under the scrim, and only after walking the page does it enter the dialog.
2. Press ⌘K, type Settings, Enter: the palette unmounts and focus drops to document.body (the palette is a body portal too), so Tab starts again from the top of the document.

Expected

• the dialog node carries tabIndex: -1 and keeps the rest of its attributes;
• on mount it takes focus exactly once with { preventScroll: true };
• on close it returns focus to the element that had it before opening (guarded by isConnected);
• Escape still closes exactly once — the existing useEscape (settings.js:176) is not replaced or doubled.

Scope

One shared hook useDialogFocus in core/src/client/overlay-hooks.js (next to useEscape), wired into the Settings dialog only — the other five keep working exactly as today and get their own follow-up. Plus the docs/subsystems/core-client.md line and a changelog.d/ fragment. No new endpoint, no server change, no change to the read-only decision.

Red first, already run

New core/test/dialog-focus.test.js on a clean tree at base 6f862cd0, with the source at base and only this test file added: 3 tests / 1 pass / 2 fail

not ok 1 - the dialog itself can hold focus so the keyboard starts inside it
  error: undefined !== -1            (no tabIndex today)
not ok 2 - the control that opened the dialog has focus back after it closes
  error: 0 !== 1                     (no focus given back today)
ok     3 - the counter-case: role, aria-label and aria-modal are unchanged  (green on base)

The fixture runs effects for real and attaches refs (a small hook runtime in the test file); core/test/settings-page-slot.test.js was deliberately not used, because its useEffect(){} stub cannot observe an effect-driven focus call and would report a false green.

Verification of the fix (local, before the issue)

overlay-hooks.js +13 (useDialogFocus), settings.js +4/−2 (tabIndex: -1, ref), the new test +150, one docs line, one changelog fragment, and the regenerated core/client.js / patches/ds-bot/{0003,0008,0014} / files/test/ mirror: 11 files +853/−383. After the change the test file is 3/3, core is 2678/2678/0, and TZ=UTC ARCH_DOCS_BASE=6f862cd0 npm run verify is exit 0, 4574/4574/0 with arch-docs: OK (11 changed file(s)), 10 patches apply cleanly in order, core/ equals ds-bot/ + patches/ds-bot/ and generate is a no-op.

The PR will be rebased onto live main at push time and the red re-captured there; it waits for the core/client.js lane (T1, opens as a draft).

Duplicate check (2026-11-03)

• GitHub search repo:ThinkFlowLab/FlyBot focus dialog aria-modal → 1 hit, #1265 (the delivery board, unrelated, closed); settings dialog focus → 16 hits, all closed and about other Settings problems (#421, #392, #1749, #1514, #1556, #173, #385, #423); tabIndex dialog → 3 hits, #1701/#1724/#1726 (Project board, closed).
• None of them is about a modal dialog not taking focus. #274 (closed) added the focus round-trip to the friend's-Bot sheet only.
• product-backlog.md: aria-modal appears only in this defect (P-453); focus/焦点 only in P-67 (a test-fixture gap) and this one.

So this is not a duplicate and not an enhancement: the declaration and the behaviour contradict each other today.