Repository navigation
fix(core): one modality call per component, reading the component's own document - #9148
Conversation
…wn document useInteractionModalityTracking now takes the component's element ref: it tracks that element's current owner document when it is not the page's (ref-counted, re-targeted when the node moves, released on unmount) and returns a reader for that document. Dialog's own useEffect and its trackInteractionModalityIn import are gone; trackInteractionModalityIn is module-private. Every reader (Slider, MultiSelector, DropdownMenu and its submenu and sheet list, ChatComposer, CommandPaletteInput, PanelSearchInput, useTouchTrigger, useFocusReturnVisibility, useMenuPress, BottomSheet, BreadcrumbItem) reads its element's own document, and focusForLastInput defaults to the element's. Listening sits behind one internal function. On the page nothing changes. Builds on vjeux's per-document fix for Dialog (#8381).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC |
|---|---|---|
| Pagination | 88.3 kB (+393 B) · 15.7 kB CSS (0) | — |
| Selector | 79.8 kB (+383 B) · 15.4 kB CSS (0) | — |
| MultiSelector · Storybook | 75.8 kB (+364 B) · 15.0 kB CSS (0) | 2,704 (+3) |
132 more components
| Component | Size | LoC |
|---|---|---|
| ChatComposer | 63.6 kB (+344 B) · 11.1 kB CSS (0) | — |
| DateInput | 103.3 kB (+334 B) · 15.1 kB CSS (0) | — |
| MoreMenu | 76.0 kB (+330 B) · 15.4 kB CSS (0) | — |
| Table | 87.5 kB (+326 B) · 15.9 kB CSS (0) | — |
| TableHeaderCell | 78.5 kB (+323 B) · 15.4 kB CSS (0) | — |
| DropdownMenu | 75.7 kB (+322 B) · 15.4 kB CSS (0) | — |
| lab/Schedule | 62.8 kB (+318 B) · 3.9 kB CSS (0) | — |
| BreadcrumbItem | 69.9 kB (+317 B) · 11.4 kB CSS (0) | — |
| lab/ChatEmojiPicker | 68.3 kB (+317 B) · 773 B CSS (0) | — |
| Slider · Storybook | 50.7 kB (+316 B) · 8.8 kB CSS (0) | 1,048 (−3) |
| lab/CommentThread | 220.5 kB (+311 B) · 2.4 kB CSS (0) | — |
| CommandPaletteInput | 33.6 kB (+309 B) · 8.4 kB CSS (0) | — |
| Popover | 52.3 kB (+306 B) · 12.4 kB CSS (0) | — |
| Markdown | 148.0 kB (+305 B) · 18.2 kB CSS (0) | — |
| DateRangeInput | 72.7 kB (+300 B) · 11.5 kB CSS (0) | — |
| lab/CommentComposer | 97.5 kB (+298 B) · 1.9 kB CSS (0) | — |
| lab/TourStep | 57.6 kB (+295 B) · 590 B CSS (0) | — |
| lab/TransferList | 70.2 kB (+295 B) · 1.7 kB CSS (0) | — |
| lab/ChatReactionBar | 68.9 kB (+294 B) · 1.1 kB CSS (0) | — |
| Carousel | 53.1 kB (+287 B) · 10.5 kB CSS (0) | — |
| DateTimeInput | 114.7 kB (+286 B) · 15.6 kB CSS (0) | — |
| lab/TransferListSelector | 75.9 kB (+286 B) · 1.9 kB CSS (0) | — |
| TableCell | 78.8 kB (+279 B) · 15.7 kB CSS (0) | — |
| lab/ListInput | 78.1 kB (+278 B) · 1.6 kB CSS (0) | — |
| BottomSheet | 34.1 kB (+275 B) · 7.4 kB CSS (0) | — |
| ContextMenu | 77.6 kB (+273 B) · 15.2 kB CSS (0) | — |
| TimeInput | 91.6 kB (+273 B) · 14.1 kB CSS (0) | — |
| richtext/RichTextEditorToolbar | 216.2 kB (+258 B) · 454 B CSS (0) | — |
| PowerSearch | 172.0 kB (+255 B) · 20.3 kB CSS (0) | — |
| lab/Comments | 225.0 kB (+231 B) · 2.5 kB CSS (0) | — |
| charts/ChartTooltip | 55.5 kB (+204 B) · 20 B CSS (0) | — |
| Tokenizer | 71.8 kB (+202 B) · 16.9 kB CSS (0) | — |
| charts/Chart | 66.1 kB (+201 B) · 20 B CSS (0) | — |
| SegmentedControl | 40.0 kB (+184 B) · 6.5 kB CSS (0) | — |
| lab/InfoTip | 22.7 kB (+183 B) · 458 B CSS (0) | — |
| charts/ChartLegend | 55.5 kB (+181 B) · 20 B CSS (0) | — |
| lab/CommentPin | 49.8 kB (+180 B) · 1.2 kB CSS (0) | — |
| ContextMenuItem | 55.5 kB (+175 B) · 9.9 kB CSS (0) | — |
| lab/MobilePowerSearch | 158.4 kB (+173 B) · 1.2 kB CSS (0) | — |
| TopNavHeading | 60.5 kB (+171 B) · 10.3 kB CSS (0) | — |
| DrawerHeader | 51.5 kB (+170 B) · 11.5 kB CSS (0) | — |
| lab/EmojiPicker | 66.1 kB (+168 B) · 722 B CSS (0) | — |
| CommandPalette | 54.0 kB (+167 B) · 12.1 kB CSS (0) | — |
| FieldLabel | 39.8 kB (+166 B) · 5.9 kB CSS (0) | — |
| TopNavMenu | 60.1 kB (+165 B) · 10.5 kB CSS (0) | — |
| AppShell | 71.2 kB (+163 B) · 12.0 kB CSS (0) | — |
| SideNavHeading | 61.1 kB (+163 B) · 10.5 kB CSS (0) | — |
| Toast | 62.6 kB (+162 B) · 10.0 kB CSS (0) | — |
| lab/MobileTokenizer | 95.7 kB (+162 B) · 880 B CSS (0) | — |
| BaseTypeahead | 57.1 kB (+161 B) · 10.2 kB CSS (0) | — |
| SideNavCollapseButton | 49.2 kB (+161 B) · 9.8 kB CSS (0) | — |
| TopNavMegaMenu | 56.9 kB (+161 B) · 11.1 kB CSS (0) | — |
| AlertDialog | 59.4 kB (+160 B) · 12.7 kB CSS (0) | — |
| DialogHeader | 51.4 kB (+160 B) · 10.1 kB CSS (0) | — |
| StatusDot | 19.8 kB (+160 B) · 6.3 kB CSS (0) | — |
| MobileNav | 53.8 kB (+159 B) · 10.4 kB CSS (0) | — |
| MobileNavToggle | 48.8 kB (+159 B) · 9.7 kB CSS (0) | — |
| TabMenu | 57.8 kB (+159 B) · 10.4 kB CSS (0) | — |
| richtext/RichTextView | 253.2 kB (+159 B) · 2.3 kB CSS (0) | — |
| ChatLayoutScrollButton | 49.0 kB (+158 B) · 9.9 kB CSS (0) | — |
| Stepper | 50.0 kB (+158 B) · 9.9 kB CSS (0) | — |
| AvatarGroupOverflow | 42.1 kB (+157 B) · 6.5 kB CSS (0) | — |
| ChatDictationButton | 49.0 kB (+157 B) · 9.8 kB CSS (0) | — |
| NavHeadingMenu | 13.2 kB (+157 B) · 3.4 kB CSS (0) | — |
| SideNav | 66.9 kB (+157 B) · 10.9 kB CSS (0) | — |
| Switch | 51.0 kB (+157 B) · 9.7 kB CSS (0) | — |
| AvatarGroup | 41.1 kB (+156 B) · 6.6 kB CSS (0) | — |
| Banner | 50.3 kB (+156 B) · 10.2 kB CSS (0) | — |
| CheckboxListItem | 57.2 kB (+156 B) · 11.0 kB CSS (0) | — |
| Field | 46.0 kB (+156 B) · 8.3 kB CSS (0) | — |
| FileInput | 57.1 kB (+156 B) · 10.7 kB CSS (0) | — |
| Lightbox | 54.1 kB (+156 B) · 10.1 kB CSS (0) | — |
| richtext/RichTextEditor | 272.8 kB (+156 B) · 2.6 kB CSS (0) | — |
| HoverCard | 17.9 kB (+155 B) · 5.0 kB CSS (0) | — |
| ListItem | 56.7 kB (+155 B) · 11.0 kB CSS (0) | — |
| TextArea | 51.2 kB (+155 B) · 9.7 kB CSS (0) | — |
| ToggleButton | 46.4 kB (+154 B) · 9.5 kB CSS (0) | — |
| Calendar | 57.7 kB (+153 B) · 10.3 kB CSS (0) | — |
| ChatComposerInput | 58.2 kB (+153 B) · 10.3 kB CSS (0) | — |
| CodeBlock | 62.7 kB (+153 B) · 11.0 kB CSS (0) | — |
| Link | 46.1 kB (+153 B) · 7.9 kB CSS (0) | — |
| SideNavItem | 57.2 kB (+153 B) · 10.4 kB CSS (0) | — |
| Typeahead | 64.6 kB (+153 B) · 11.6 kB CSS (0) | — |
| ChatComposerTokenElement | 58.2 kB (+152 B) · 10.3 kB CSS (0) | — |
| ChatLayout | 51.5 kB (+151 B) · 10.2 kB CSS (0) | — |
| ChatSendButton | 48.2 kB (+151 B) · 9.4 kB CSS (0) | — |
| ComplexSelector | 59.1 kB (+151 B) · 11.0 kB CSS (0) | — |
| LayerProvider | 66.7 kB (+151 B) · 10.4 kB CSS (0) | — |
| TextInput | 56.4 kB (+151 B) · 10.8 kB CSS (0) | — |
| TopNav | 54.7 kB (+151 B) · 10.7 kB CSS (0) | — |
| CheckboxInput | 52.7 kB (+150 B) · 9.6 kB CSS (0) | — |
| DropdownMenuSubMenu | 61.0 kB (+149 B) · 9.9 kB CSS (0) | — |
| NumberInput | 58.6 kB (+149 B) · 11.0 kB CSS (0) | — |
| Avatar | 38.7 kB (+148 B) · 6.5 kB CSS (0) | — |
| RadioListItem | 55.2 kB (+148 B) · 10.0 kB CSS (0) | — |
| Thumbnail | 53.3 kB (+148 B) · 11.3 kB CSS (0) | — |
| IconButton | 45.6 kB (+147 B) · 9.4 kB CSS (0) | — |
| Tooltip | 17.5 kB (+146 B) · 5.0 kB CSS (0) | — |
| Button | 45.5 kB (+145 B) · 9.4 kB CSS (0) | — |
| RadioList | 47.4 kB (+145 B) · 8.3 kB CSS (0) | — |
| CheckboxList | 47.8 kB (+143 B) · 8.5 kB CSS (0) | — |
| InputGroup | 46.6 kB (+143 B) · 8.3 kB CSS (0) | — |
| TableSelectionToolbar | 54.7 kB (+142 B) · 11.4 kB CSS (0) | — |
| Dialog | 15.9 kB (+134 B) · 6.6 kB CSS (0) | — |
| DropdownMenuCheckboxItem | 17.6 kB (+18 B) · 5.9 kB CSS (0) | — |
| DropdownMenuRadioItem | 17.7 kB (+16 B) · 5.9 kB CSS (0) | — |
| DropdownMenuItem | 16.9 kB (+13 B) · 5.5 kB CSS (0) | — |
| BottomSheetSwitcher | 14.9 kB (+9 B) · 3.4 kB CSS (0) | — |
| Timer | 9.5 kB (+1 B) · 6.9 kB CSS (0) | — |
| Breadcrumbs | 22.9 kB (0) · 3.4 kB CSS (0) | — |
| ChatComposerDrawer | 24.5 kB (0) · 4.7 kB CSS (0) | — |
| ChatMessage | 23.6 kB (0) · 3.7 kB CSS (0) | — |
| ChatMessageBubble | 7.1 kB (0) · 3.7 kB CSS (0) | — |
| ChatMessageList | 29.8 kB (0) · 7.9 kB CSS (0) | — |
| ChatMessageMetadata | 26.5 kB (0) · 4.1 kB CSS (0) | — |
| ChatSystemMessage | 6.8 kB (0) · 3.8 kB CSS (0) | — |
| ChatTokenizedText | 7.1 kB (0) · 4.0 kB CSS (0) | — |
| CommandPaletteEmpty | 6.2 kB (0) · 3.5 kB CSS (0) | — |
| CommandPaletteFooter | 23.8 kB (0) · 3.7 kB CSS (0) | — |
| CommandPaletteGroup | 3.5 kB (0) · 3.5 kB CSS (0) | — |
| CommandPaletteItem | 9.5 kB (0) · 3.6 kB CSS (0) | — |
| CommandPaletteList | 22.9 kB (0) · 3.4 kB CSS (0) | — |
| DropdownMenuDivider | 5.2 kB (0) · 548 B CSS (0) | — |
| DropdownMenuGroup | 6.2 kB (0) · 3.4 kB CSS (0) | — |
| DropdownMenuRadioGroup | 6.1 kB (0) · 3.3 kB CSS (0) | — |
| Heading | 7.9 kB (0) · 6.0 kB CSS (0) | — |
| ProgressBar | 7.9 kB (0) · 6.9 kB CSS (0) | — |
| SelectorOption | 15.9 kB (0) · 5.4 kB CSS (0) | — |
| Spinner | 25.5 kB (0) · 6.7 kB CSS (0) | — |
| Text | 7.4 kB (0) · 6.0 kB CSS (0) | — |
| Timestamp | 28.2 kB (0) · 9.9 kB CSS (0) | — |
| ChatToolCalls | 32.8 kB (−1 B) · 9.2 kB CSS (0) | — |
Package size: @astryxdesign/core 6.52 MB packed (+3.8 kB); no new dependencies.
Not run on this PR: add ci:run-visual, ci:run-a11y or ci:run-rtl to run them.
Generated by PR Enrichment workflow | View full report
|
AI review status for this pull request.
|
![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 per-document tracking works for the readers it moves, and the new iframe and focus-return tests fail on main and pass here. Two things need fixing before approval:
- The changeset says MultiSelector's returned focus and every reader now follow their own document, but
useSelectorPresentationstill callsuseFocusReturnVisibility()without a ref (line 87); passtriggerRefthere with a second-document test, or narrow the changeset. - The second-document hold is taken in a dependency-less effect in
useInteractionModalityTracking; a callback ref that tracks on attach and releases in its cleanup would give attach, detach and replacement one owner (architecture:react-component-runtimeINV3 and INV6), with a StrictMode replay case to cover it.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Components rendered into a same-origin iframe or a Document Picture-in-Picture window now follow the last input in their own document: keyboard focus rings, keyboard highlights and returned focus (Slider, MultiSelector, DropdownMenu, ChatComposer, CommandPalette, panel search fields, menus, BottomSheet, Dialog) no longer read the page's last input. Each component tracks its document through its one modality call, with no effect of its own. On the page, behavior is unchanged. vjeux wrote the original per-document fix, for Dialog (#8381); this extends it to every reader. |
There was a problem hiding this comment.
Selector's and MultiSelector's returned focus still reads the page: useSelectorPresentation.ts:87 calls useFocusReturnVisibility() without a ref, so "returned focus (… MultiSelector …)" and "every reader" overstate this change. Passing triggerRef there (with a second-document test) would make the note true; otherwise narrow it.
| useEffect(() => { | ||
| const ownerDocument = elementRef?.current?.ownerDocument ?? null; | ||
| const secondDocument = | ||
| ownerDocument != null && | ||
| (typeof document === 'undefined' || ownerDocument !== document) | ||
| ? ownerDocument | ||
| : null; | ||
| if (secondDocument === trackedDocumentRef.current) { | ||
| return; | ||
| } | ||
| releaseRef.current?.(); | ||
| releaseRef.current = | ||
| secondDocument == null | ||
| ? null | ||
| : trackInteractionModalityIn(secondDocument); | ||
| trackedDocumentRef.current = secondDocument; | ||
| }); |
There was a problem hiding this comment.
This takes the per-document hold in an effect with no dependency list and no cleanup of its own (release lives in the unmount-only effect below), and it sees a node only on this component's commits. architecture:react-component-runtime INV6 gives attach, null detach and replacement to a callback ref; returning one from this hook, acquiring on attach and releasing in its cleanup, would remove both effects and satisfy INV3's "cleanup MUST release that resource".
… trigger's document The hook's hold on a second document is one idempotent owner that handles attachment, null detachment and replacement, and unmount releases it; a StrictMode replay leaves one hold. useSelectorPresentation passes its trigger to useFocusReturnVisibility.
There was a problem hiding this comment.
Changes requested: keyboard rings and highlights now follow the iframe's own input, and the new iframe and focus-return tests fail on main and pass here. Two things remain before approval:
- In a same-origin iframe, DropdownMenu still rings its trigger after a pointer pick, exactly as on
main, because its hide path compares the page'sdocument.activeElement(the iframe element) and returns before the new per-document focus return (DropdownMenu.tsx:1032; the same read sits inuseSelectorPresentation.ts:99,useMenuHover.ts:281andBottomSheet.tsx:537), so either read the trigger's own document there with an iframe test or narrow the changeset's "returned focus" claim. - The second-document hold in
useInteractionModalityTrackingis still acquired in a dependency-less effect with no cleanup, so it follows only the caller's commits (architecture:react-component-runtimeINV3, INV6); a callback ref that holds on attach and releases in its cleanup would replace both effects.
[Automated review]
| useEffect(() => { | ||
| hold(elementRef?.current?.ownerDocument ?? null); | ||
| }); | ||
| useEffect(() => () => hold(null), [hold]); |
There was a problem hiding this comment.
This still acquires the second-document hold in an effect with no cleanup of its own (release is the unmount-only effect on the next line), and it only sees a node on this component's commits: a node another component attaches in its own commit stays untracked until this caller re-renders. architecture:react-component-runtime INV3 asks the acquiring cleanup to release, and INV6 gives attach, null detach and replacement to a callback ref; returning one from this hook (hold on attach, release in its cleanup) and passing it through the callback refs at Dialog.tsx:516, DropdownMenu.tsx:1680, MultiSelector.tsx:1192 and PanelSearchInput.tsx:222 would remove both effects.
There was a problem hiding this comment.
Changes requested: keyboard rings and highlights in a same-origin iframe now follow the iframe's own last input, and the new iframe and focus-return tests fail on main and pass here. One thing remains: the second-document hold in useInteractionModalityTracking is acquired in an Effect with no cleanup and re-targeted only on the caller's commits (interactionModality.ts:189-192), which architecture:react-component-runtime INV3 and INV6 do not allow. Having the hook return a callback ref that holds the node's document on attach and releases it in its cleanup, passed through the components' existing refs, would replace both Effects; only the public useMenuPress and useTouchTrigger, which receive a consumer's object ref, still need an Effect, and there its own cleanup should release the hold.
[Automated review]
… ref
useInteractionModalityTracking() now returns {ref, readModality}: a component
attaches the callback ref to its element, which holds the element's second
document on attach and releases it on detach or replacement (React's ref
cleanup or a null call, never twice), so an unchanged commit touches no
listeners. Only useMenuPress and useTouchTrigger, which receive a consumer's
object ref, use useInteractionModalityTrackingFor(ref), whose Effect cleanup
releases the hold; their public signatures are unchanged.
useFocusReturnVisibility takes the component's tracking, and DropdownMenu,
Selector and MultiSelector attach it to their triggers.
There was a problem hiding this comment.
Changes requested: components in a same-origin iframe now read their own document's last input, and the new iframe and focus-return tests fail on main and pass here. Two things remain:
- A bottom-sheet DropdownMenu attaches its tracking ref to the sheet's action list, which unmounts on close, so in an iframe a keyboard user who tabs back to the trigger after a tapped pick gets no focus ring at all; attaching
modality.refto the sheet trigger, as the popover does, fixes it (DropdownMenu.tsx:634). useTouchTriggeris internal and both of its callers already own the trigger through a callback ref (useTooltip.tsx:479,useHoverCard.tsx:528), so they can make the oneuseInteractionModalityTracking()call and passreadModalitydown, leaving the per-commit Effect inuseInteractionModalityTrackingFor(interactionModality.ts:243) to the publicuseMenuPressalone.
[Automated review]
| const actionListRef = useRef<HTMLDivElement>(null); | ||
| const openModalityRef = useRef<'keyboard' | 'pointer'>('pointer'); | ||
| const modality = useInteractionModalityTracking(); | ||
| const actionListComposedRef = useMergedRefs(actionListRef, modality.ref); |
There was a problem hiding this comment.
This ref sits on the sheet's action list, which exists only while the sheet is open (MenuBottomSheet renders nothing at rest), but readModality also serves the trigger's focus (onFocusReturnTargetFocus at lines 812 and 838). After a tapped pick closes the sheet in an iframe, that document is released and its keys are no longer heard, so a keyboard user who tabs back to the trigger keeps focusOutlineStyles.suppressed: no ring, whatever the page's last input (in Chromium, main drew a 2px outline there when the page had seen no pointer input). Attaching modality.ref to the sheet trigger (triggerRef above and sheetTriggerRef), as the popover does, keeps the trigger's document tracked while the menu is mounted; with that change an iframe probe rings and the DropdownMenu suites pass.
| useEffect(() => { | ||
| const ownerDocument = elementRef.current?.ownerDocument; | ||
| if ( | ||
| ownerDocument == null || | ||
| (typeof document !== 'undefined' && ownerDocument === document) | ||
| ) { | ||
| return undefined; | ||
| } | ||
| return trackInteractionModalityIn(ownerDocument); | ||
| }); |
There was a problem hiding this comment.
useTouchTrigger is the other caller of this per-commit Effect, but it is internal and both of its callers own the trigger node through a callback ref (interactionRef in useTooltip.tsx:479 and useHoverCard.tsx:528), so architecture:react-component-runtime INV6 puts the hold there: they can make the one useInteractionModalityTracking() call, call its ref from interactionRef, and pass readModality down. That leaves this form to the public useMenuPress alone.
…; the reader forgets a detached node The sheet variant attached its tracking to the action list, which unmounts on close, so focus returning to the trigger in a second document read a released document. It now sits on the trigger, as in the popover variant. The tracking ref's cleanup also forgets its node, so the reader never answers from a document whose listeners are gone, and DropdownMenu's own trigger callback calls it without returning the cleanup, so React keeps calling the popover's ref with null.
There was a problem hiding this comment.
Changes requested: keyboard rings in a same-origin iframe now follow the iframe's own input for the components this moves (in Chromium the Slider thumb and MultiSelector's action cell ring after keyboard input inside the iframe), and both earlier points are resolved. One thing remains: MultiSelector's renderTrigger branch builds its own mergeRefs without triggerModality.ref (MultiSelector.tsx:2535-2547), so with a caller-rendered trigger the action-cell ring still follows the page's last input, although the changeset lists MultiSelector as fixed. Passing triggerModality.ref to that mergeRefs fixes it with MultiSelector's tests still passing, and an iframe case for that mode would pin it.
[Automated review]
| const readModality = triggerModality.readModality; | ||
| const triggerButtonRef = useMergedRefs<HTMLElement>( | ||
| triggerRef, | ||
| triggerModality.ref, |
There was a problem hiding this comment.
This ref reaches only the built-in trigger (triggerButtonRef, attached at :2663). The renderTrigger branch builds its own mergeRefs at :2535-2547 without triggerModality.ref, so with a caller-rendered trigger readModality reads getInteractionModality(null), the page: in a same-origin iframe, arrowing onto an option's action cell after a click on the page draws no ring. Adding triggerModality.ref to that mergeRefs makes it ring, and MultiSelector's tests still pass.
…acking too The renderTrigger branch built its own merged ref without the tracking ref, so in a second document its action-cell ring still followed the page's input.
There was a problem hiding this comment.
This looks ready to approve: components rendered into a same-origin iframe or a Document Picture-in-Picture window now read their own document's last input, and the caller-rendered MultiSelector trigger from the last round is fixed and pinned by an iframe test that fails without it. In Chromium, a Slider thumb and a panel search field inside an iframe now take their keyboard ring after a Tab there even when the page's last input was a click, and Dialog's second-document browser spec still passes in the iframe and Picture-in-Picture cases. One note on the description: DialogSecondDocument.a11y.browser.spec.ts runs in CI only with the ci:run-a11y label, so it did not run on this push.
[Automated review]
…dalityTrackingFor #9148 added the document-scoped hook that useTouchTrigger and useMenuPress call; a change to those calls now routes interaction-modality INV1-INV3 and react-component-runtime INV16 like the other modality names.
User impact
A keyboard user working in a component that an app renders into a same-origin iframe or a Document Picture-in-Picture window gets the page's last input instead of their own for keyboard rings and highlights. If their last click was on the page:
focusForLastInput) gets the ring of the page's last input.Where an overlay returns focus on close (the opener is recorded from the page's document in DropdownMenu, Selector, useMenuHover and BottomSheet) is a separate defect, owned by the planned focus-return helper; this PR doesn't change it.
Only Dialog read its own document (#8381), and it needed an extra effect next to its hook call to do it.
Problem and solution fit
getInteractionModality(), andfocusForLastInput's default) asked the page.architecture:interaction-modalityINV1 says the ring follows the last input. Per the new one-call rule, a concern Core owns in a shared hook is fixed there, and a component makes one call with no effect of its own. Dialog's hook-plus-effect pattern would otherwise spread to every component that needs a second document.The fix moves per-document tracking into the hook. A component passes its element ref and makes one call that both tracks and reads its own document.
Expected behavior and authority
architecture:interaction-modality(current) INV1: the focus ring follows the last input.architecture:react-component-runtime(current) INV2/INV3: no effect for work a handler can do, and every effect names the external system it syncs. The one-call rule is proposed in spec(react-component-runtime): a shared concern is extended in its hook, and components call it once #9139.component:DialogFR4 andcomponent:PopoverAR4: focus returns to the opener, with the ring its document's last input calls for.Change
Before (Dialog, the only per-document reader):
After (every component):
utils/interactionModality.ts:useInteractionModalityTracking()keeps the page's document-lifetime listeners and returns{ref, readModality}.refis a callback ref the component attaches to its element. On attach it holds the element's document when that isn't the page's; detach or replacement releases it first, through React's ref cleanup or anullcall, never twice.readModalityanswers for that element's document.useInteractionModalityTrackingFor(objectRef)serves only useMenuPress and useTouchTrigger, which receive a consumer's object ref. Both are public: useMenuPress from the package root, useTouchTrigger from the@astryxdesign/core/Layersubpath export. So their signatures stay unchanged. After every commit it holds the ref's current owner document, and the same Effect's cleanup releases it.getInteractionModality(target?)accepts a node or a document.setListening, so a later move onto the shared input stream (fix(core): one click guard for the menu press model and the gesture host #8360) touches one place.trackInteractionModalityInis no longer exported; a test-only alias remains.utils/focusReturn.ts:focusForLastInputdefaults to the element's own document. All 11 callers get the right document with no callsite change.useFocusReturnVisibilitynow takes the component's tracking, so a component still makes one call.menuRef) and useTouchTrigger.trackInteractionModalityInimport are deleted; one call remains.useFocusReturnhelper.Original per-document fix by @vjeux (#8381).
Ownership (react-component-runtime INV3, INV5, INV6)
Evidence
interactionModality.test.ts, callback ref (real components rendered in a same-origin iframe):nullcall release only once, and React's cleanup forgets the node so the reader answers for the page again.interactionModality.test.ts, object-ref form: it holds and releases with its last consumer, re-targets a moved node, releases onnulldetach, holds one under StrictMode, and keeps a page element's document-lifetime listeners.useFocusReturnVisibility.test.ts: the returned-focus ring follows the trigger's own document, not the page's.MultiSelector.test.tsx: with a caller-rendered trigger in a same-origin iframe, after a click on the page and a key in the iframe, the highlighted action cell takes the ring. Fails without the fix.PanelSearchInput.secondDocument.test.tsx(non-Dialog reader in a real iframe): rings after keyboard input in the iframe even when the page's last input was a pointer, and not the other way round. Both fail on main.focusReturn.test.ts:focusForLastInputfollows the element's own document. Fails on main.Dialog.test.tsxand the interactionModality tests.DialogSecondDocument.a11y.browser.spec.tsruns in CI only with theci:run-a11ylabel; CodeBunny's review ran it in Chromium on this head, and its iframe and Picture-in-Picture cases pass.tscand ESLint are clean.Scope
Testing
pnpm vitest run packages/core/src/utils packages/core/src/hooks packages/core/src/Dialog packages/core/src/DropdownMenu packages/core/src/Slider packages/core/src/MultiSelector packages/core/src/Selector packages/core/src/Chat packages/core/src/CommandPalette packages/core/src/Field packages/core/src/Layer packages/core/src/BottomSheet packages/core/src/Breadcrumbs;tsc --noEmit; ESLint on changed files.