Repository navigation
fix(Dialog): an explicit autofocus request wins over a header's title, and Lab can read the Dialog context through an internal entry - #8798
Conversation
…, and Lab can read the Dialog context through an internal entry Dialog now honors only eligible initial-focus requests (component:Dialog FR2): a request inside a hidden or inert subtree is skipped, and a request that cannot take focus falls through to the next one. A header's default title request (data-autofocus="dialog-title") comes after every explicit request. The Dialog context gains syncDefaultName(), so a composed header whose title renders after mount can ask Dialog to check its default name again. @astryxdesign/core/internal/dialog-context re-exports useDialogContext for Astryx's own packages only (spec:AST-017 FR21), the same shape as internal/tokenizer-presentation. Part of meta#4182. Split out of meta#5105.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
community
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: No accessibility violations detected. Visual Regression57 of 504 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 37 more. Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: the new fall-through and title precedence match component:Dialog FR2 in an ordinary page, but two lines in the focus loop look outside the dialog. Please fix both and add a regression test for each case:
Dialog.tsx:604:closest('[hidden], [inert], [aria-hidden="true"]')walks past the<dialog>, so a Dialog rendered inside aninertregion skips its own focusable request; only count a match the dialog contains.Dialog.tsx:611:document.activeElementbelongs to the page's document, so in an iframe or Picture-in-Picture window the loop never stops and the last request (thedialog-titlerequest when present) wins; compare againsttarget.ownerDocument.activeElement.
The changesets could also mention the aria-hidden skip and syncDefaultName.
[Automated review]
| ), | ||
| ]; | ||
| for (const target of autofocusTargets) { | ||
| if (target.closest('[hidden], [inert], [aria-hidden="true"]')) { |
There was a problem hiding this comment.
This walks every ancestor, past the <dialog>. A modal dialog escapes an ancestor's inertness: Chromium focuses a button inside a modal <dialog> under <div inert>. A Dialog rendered in an inert region therefore skips its own eligible request and falls back to native focus, against component:Dialog FR2. Count a match only when it is inside the dialog:
const blocker = target.closest('[hidden], [inert], [aria-hidden="true"]');
if (blocker != null && dialog.contains(blocker)) {
continue;
}| // browser. If a request cannot land, try the next eligible request | ||
| // and finally the title; otherwise preserve native initial focus. | ||
| focusForLastInput(target); | ||
| if (document.activeElement === target) { |
There was a problem hiding this comment.
document is the page's document. For a Dialog rendered into an iframe or a Picture-in-Picture window this is never true, so the loop focuses every eligible request in turn and ends on the last one, which is the dialog-title request when there is one. Compare against the target's own document:
if (target.ownerDocument.activeElement === target) {
break;
}
|
A case for function Repro() {
const [isReady, setIsReady] = useState(false);
useEffect(() => {
const id = setTimeout(() => setIsReady(true), 10);
return () => clearTimeout(id);
}, []);
return (
<Dialog isOpen onOpenChange={() => {}}>
{isReady ? <DialogHeader title="Edit profile" /> : <p>Loading</p>}
</Dialog>
);
}In jsdom on 0.7.4 (unchanged on main at 14fc587), the Since this PR adds |
Part of #4182. Split out of #5105.
The review app asked to split #5105 because it has 2,146 authored lines, and the limit is 2,000. This PR holds only the Core portion. #5105 keeps the Lab
DialogHeroHeader. When this PR merges, I will mergemaininto #5105, and its diff will drop below the limit.What changes
1. Initial focus follows
component:DialogFR2. (packages/core/src/Dialog/Dialog.tsx:595)FR2 is
settled: "A request is eligible only when its rendered target is inside the active Dialog, is neither hidden nor inert, and can receive programmatic focus when selection runs. DialogHeader supplies its title as the default when no non-default eligible request is rendered."main[data-autofocus]is in ahiddenorinertsubtreedisabled)data-autofocus="dialog-title"comes after every explicit request2.
syncDefaultName()on the Dialog context. (Dialog.tsx:528)Dialog looks for its title in its callback ref, when it mounts. A composed header whose title element renders later, changes, or leaves calls
syncDefaultName(), and Dialog runs the same check again. The defaultaria-labelledbythen follows the title. A caller'saria-labeloraria-labelledbystill wins.3.
@astryxdesign/core/internal/dialog-context. (packages/core/src/internal/dialog-context.ts)This entry re-exports
useDialogContextby name and exports nothing else.spec:AST-017FR21 keeps a module without a public contract internal, and exports it only through Core's./internal/*subpaths. Those subpaths are reserved for Astryx's own packages, and lint limits them to Core and Lab (architecture:react-component-runtimeINV13). The entry has the same shape asinternal/tokenizer-presentation(#8735), and it is registered inFILE_MODULE_SUBPATH_EXPORTS. The public@astryxdesign/core/Dialogbarrel is unchanged, and a test asserts that it exports no context.Who uses this
No Core component uses the
dialog-titlemarker orsyncDefaultName()yet. Lab'sDialogHeroHeaderin #5105 uses both. Core'sDialogHeaderdoes not change: it still focuses its title from its own mount effect.Tests
mainDialog.test.tsx:211skips a hidden or inert request for the next eligible oneDialog.test.tsx:233falls through a request that cannot take focusDialog.test.tsx:248honors an explicit request before a header's title requestDialog.test.tsx:263focuses the title request when no explicit request is eligibleDialog.test.tsx:1291names itself by a header title that renders after mountinternal/dialog-context.test.tsre-exports the same hook by name; the public barrel exports no contextTo get the
maincolumn, I putmain'sDialog.tsxin place and ran the new tests.Local validation:
Dialog,AlertDialog,Drawer,BottomSheet,CommandPaletteandinternalsuites, and inscripts/sync-exports.test.mjs.pnpm -F @astryxdesign/core typecheck, eslint and prettier pass on the changed files.check:changesets,check:sync,check:package-boundariesand the full pre-commitcheck:repopass.changeset-coverage.mjs prpasses againstv0.7.4.Changesets
[fix]Dialog: a hidden, inert or unfocusable first request falls through to the next one, and a header's default title request comes after every explicit request.[chore]Internal: the new internal entry. Nothing that a consumer sees changes.