feat(a11y): modal focus-trap + restore + role=dialog, with @a11y tests#94
Merged
Conversation
…11y) Adds hooks/useModalA11y: marks a modal as role="dialog" aria-modal with an accessible name, moves focus into it on open, traps Tab/Shift+Tab within it, and restores focus to the trigger on close. Applied to CreateWorkItemModal, CreateGraphModal, and WorkItemDetailsModal (the details modal keeps its own initial container focus; fixed its aria-labelledby→aria-label so the name isn't the title input's value). Pairs with useDialog (Escape/click-outside). New @a11y suite (tests/e2e/a11y-focus.spec.ts) asserts role=dialog, focus enters the modal, Tab is trapped, and focus returns to the trigger on close — behavior that was previously untested and mostly absent (you could Tab out of a modal into the graph behind it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
Adversarial review fixes: - useModalA11y: prefer the first form field for initial focus (not the icon-only Close button); restore focus only when it dropped to <body> so a deliberate post-close focus move isn't overridden; sharper isVisible (getClientRects + computed visibility); honest docstring (Tab-trap + aria-modal hint, not a background-inert); include [contenteditable] in the focusable set. - CreateWorkItemModal / CreateGraphModal: aria-label="Close" on the X buttons. - WorkItemDetailsModal: drop the bespoke capture-phase Escape handler so Escape is owned by the dialog-manager (defers while typing, keeps the stack coherent); keep only Ctrl/Cmd+S. - a11y-focus.spec: test the trap AT THE BOUNDARY (Tab from the last focusable must wrap inside; Shift+Tab from the first) — the previous fixed 14 Tabs never reached the edge on modals with many focusables, so it passed even without the trap. Restore test now focuses the trigger first and asserts focus left it while open. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🧪 Comprehensive Test Suite
Full-stack smoke gate runs in the CI workflow. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Implements the keyboard-accessibility behavior modals were missing, plus the
@a11ytest category that locks it in.hooks/useModalA11y(new shared hook): marks a modalrole="dialog" aria-modal="true"with an accessible name, moves focus into it on open, traps Tab/Shift+Tab within it, and restores focus to the trigger on close. Pairs withuseDialog(Escape / click-outside).Applied to CreateWorkItemModal, CreateGraphModal, and WorkItemDetailsModal (the details modal keeps its own container focus; fixed its
aria-labelledby→aria-labelso the accessible name isn't the title input's value).Why
Discovery (a parallel audit of every modal) found nearly all of them had no
role=dialog, no focus trap, and no focus restore — you could Tab straight out of an open modal into the graph behind it, and keyboard focus never came back to where you were. Real keyboard friction.@a11ysuite —tests/e2e/a11y-focus.spec.tsrole=dialog+ accessible name + Tab trappedrole=dialog+ Tab trappedVerification
@a11y3/3 pass; typecheck clean.@dismissal+@zorder25/25 (same modals).🤖 Generated with Claude Code