a11y(2.4.3): inert-based focus containment + restore; drop trap from native dialogs#3598
Open
ardiewen wants to merge 8 commits into
Open
a11y(2.4.3): inert-based focus containment + restore; drop trap from native dialogs#3598ardiewen wants to merge 8 commits into
ardiewen wants to merge 8 commits into
Conversation
…native dialogs Replaces the keydown-wrapping focus trap with an `inert`-based approach and removes the trap entirely from the two `<dialog>`+`showModal()` consumers, which already get focus containment and restoration natively. - `focus-trap.ts`: `inertBackground()` walks node→<body> marking sibling content `inert` (clearing only what it set), and the action now captures the pre-activation focus and restores it on deactivate. This delegates Tab containment to the platform — and `inert` also blocks pointer + assistive-tech access to the background, which the old keydown trap (first/last Tab only) did not, notably for `drawer.svelte`, which has no overlay. - `holocene/modal.svelte`, `command-palette/modal.svelte`: removed `use:focusTrap`. Native `<dialog>.showModal()`/`close()` already traps and restores focus. - Drawer and Maximizable (non-dialog) keep the action, now inert-based. Verified in-browser: Modal containment + Esc-restore via native dialog; Drawer and Maximizable inert containment (real focus-blocking) + restore via the action. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
a11y
…-render) Maximizing re-renders the CodeMirror content, which blurs focus to <body> before the focus-trap action activates — so the action captured <body> and restored focus there on minimize instead of returning it to the trigger. Capture the focused element in maximizable.svelte before toggling `maximized`, and restore it after `tick()` on the minimize/Escape paths. Browser-verified: focus returns to the maximize button via both Escape and the minimize button; inert containment unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
9 tasks
laurakwhit
reviewed
Jun 25, 2026
Collaborator
There was a problem hiding this comment.
Could use:focusTrap potentially run before the portalElement is set? Maybe we could make portalElement $state() and add a null check to use:focusTrap just in case.
Contributor
Author
There was a problem hiding this comment.
Verified: yes.
Went another direction here: removed the $effect entirely: maximize() now portals to <body> synchronously before setting maximized = true, so the node is already in place when use:focusTrap activates in the same flush. No more $effect phase ordering, so there's nothing to null-check.
| releaseBackground(); | ||
| releaseBackground = () => {}; | ||
|
|
||
| if (previouslyFocused && document.body.contains(previouslyFocused)) { |
Collaborator
There was a problem hiding this comment.
Should we provide a way to pass a fallback or default here? That way if whatever opened the focus trap re-renders (e.g. a CodeBlock) while the user is in it they won't be kicked back to <body>.
The drawer is now a true modal (inert background), so clicking the main submit button while it is open no longer works. Values bind live to the form, so closing commits nothing. Also avoids a strict-mode double-match on the timeout error text (rendered in both the drawer and the form). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 participants
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.
Alternative to #3536. Fixes WCAG 2.4.3 (Focus Order) using platform primitives instead of the hand-rolled keydown trap.
focus-trap.tsdid two jobs — Tab containment + focus restoration — both of which have native equivalents:<dialog>+showModal()(holocene/modal,command-palette/modal) already traps and restores focus natively → removeduse:focusTrapentirely.drawer— no overlay,maximizable) needs help → the action now setsinerton background content (capturing/restoring focus around it).inertalso blocks pointer + assistive-tech access, which the old keydown trap (first/lastTabonly) did not.maximizableadditionally captures focus in the component before toggling, because maximizing re-renders CodeMirror and blurs to<body>before the action runs (the action alone — and a11y(2.4.3): focus-trap — capture and restore focus to the trigger element on close #3536 — would restore to<body>).Net: −44 lines of trap machinery (keydown handlers, MutationObserver, first/last tracking).
Testing
inert).pnpm checkclean, suite green.focusTrapremoved).inertblocks background focus; close → inert cleared + focus restored to trigger.inertcontainment; focus restored to the maximize button via both Esc and the minimize button.inertis Baseline (Chrome 102+/FF 112+/Safari 15.5+), same era asshowModal().A11y-Audit-Ref: 2.4.3-focus-restoration-on-close
🤖 Generated with Claude Code