[P3] Keyboard focus falls to <body> after closing the summary opened from 'Dive complete', after the first Import dialog close, and after accepting Reset to Defaults #118
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Priority: P3
Type: bug
Source: full codebase review requested 2026-10-31 (independently reproduced against
2cac7cf)Location:
src/App.tsx:3090-3098,src/App.tsx:956-960,src/replay/DiveSummary.tsx:74-77,src/App.tsx:70-75,src/App.tsx:84-105Problem and evidence
The app relies on native
focus restoration, which returns focus to whichever element had focus when showModal() was called. In three paths that element no longer exists or can no longer be focused, so focus drops to :(1) 'View dive stats' (App.tsx:3090-3098) sets completionOpen=false and summaryOpen=true in the same commit. The already-loaded lazy DiveSummary is a child, so its showModal() effect (DiveSummary.tsx:74-77) runs before App's completion-dialog close effect (App.tsx:956-960). The summary therefore records the 'View dive stats' button inside the completion dialog as the element to restore. The completion dialog then closes, and when the summary closes, focus goes to a button inside a closed dialog and lands on .
(2) DiveImportLoader (App.tsx:70-75) unmounts the clicked 'Import dive file' button and lazy-mounts DiveImport with initiallyOpen. The DiveImport showModal() runs after the trigger is gone, so the first close restores focus to . Later opens restore correctly because DiveImport renders its own trigger button.
(3) Accepting Reset to Defaults bumps
key={version}on DiveApp (App.tsx:84-105), which remounts the whole app with the modal open, so focus ends on . This is the least surprising of the three because the whole UI is rebuilt, but nothing moves focus to a sensible place afterward.I ran Playwright (Chromium) against the HEAD production build served from /tmp/divesim-review-dist-SqbiNx (scratch script at /tmp/divesim-verify-gs1GKf/t.mjs):
(1) Dive to 65 ft, +1:00 x4, Surface, then wait for 'Dive complete'. Focus 'View dive stats' and press Enter: the summary opens with focus on its Close button. Pressing Escape left activeElement = BODY. Pressing Close with Enter also left BODY. Control case: focusing 'View last dive summary', pressing Enter, then Escape returned focus to 'View last dive summary'.
(2) Focus 'Import dive file' and press Enter: the dialog opens with focus on Close. Pressing Escape left activeElement = BODY. The second open and Escape returned focus to 'Import dive file'.
(3) Focus 'Reset to Defaults', press Enter, then press Enter on 'Accept · reset to defaults': activeElement = BODY, and the next Tab goes to the top 'DiveSim' link.
I also traced the code. React runs child passive effects before parent effects in the same commit, which accounts for (1). The unmount-then-lazy-mount order accounts for (2). The key remount accounts for (3).
Suggested correction
(1) In the 'View dive stats' click handler, call completionDialog.current?.close() synchronously before setSummaryOpen(true), so the summary records a focus target outside the closing dialog. Alternatively, have the summary's onClose focus the 'View last dive summary' button (keep a ref to it).
(2) In DiveImport, when initiallyOpen is set, focus its own
.import-dive-buttontrigger before the first showModal(), or explicitly focus that trigger in the close handler.(3) After the version bump, focus a stable element in the remounted app: the new 'Reset to Defaults' button or the main heading (with tabIndex=-1). For example, pass a flag or use an effect keyed on version > 0.
Acceptance criteria
Add a browser check (for example in tests/browser/check-review-browser.mjs) for each flow, run with the keyboard only:
(1) After surfacing, press Enter on 'View dive stats', then close the summary with Escape and with its Close button. document.activeElement must be a visible, connected element outside any closed dialog, preferably 'View last dive summary', and not BODY.
(2) On a fresh page, press Enter on 'Import dive file' and then Escape. Focus must be on the 'Import dive file' button on the first close, not only the second.
(3) Press Enter on Reset to Defaults and then on Accept. Focus must be on a defined element (the Reset to Defaults button or the main heading), not BODY.
The existing 'View last dive summary' restore must keep passing. pnpm verify passes.
Scope: maintenance of the educational simulator. Preserve historical R1 acceptance and operational-validation limits.