Skip to content

Commit d0dd779

Browse files
michaldudakclaude
michaldudak
and
claude
committed
[popups] Hand focus back when a closing popup would strand it
The previous commit made a closing popup `inert`, which blurs whatever it holds. Where nothing hands focus back, that left focus on `<body>` for the whole exit animation, restarting the tab order at the top of the document. - `FloatingFocusManager` gains an internal `returnFocusIfOrphaned` prop. When the popup closed while still holding focus and `returnFocus` opted out of moving it, focus goes to the default return element instead of nowhere. It hits the common Combobox layout, where the input sits outside the popup and `returnFocus` is `false` on the assumption focus never left the input. Closes that are already deliberate are skipped: an outside press, a focus-out, an explicit `finalFocus`, or a cleanup from a mid-life dependency change rather than a close. - `Popover.Positioner` no longer goes inert for a hover-opened popover, whose focus manager is disabled and so never registers a hand-off. This matches Tooltip and Preview card. The hover origin is latched while the popover is open, because `openReason` reads as the close reason once it closes. Tests pin the decisions that the suite could otherwise not see: the trigger focus guards outliving the close, and the positioners deliberately left non-inert in Navigation menu, Preview card and Tooltip. The keyboard navigation conformance test now asserts it is still inside the closing window, which it was not for Combobox. The animation handbook was wrong about Navigation menu being non-inert, about clicks passing through a backdrop, and about which popups return focus. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent aee9212 commit d0dd779

21 files changed

Lines changed: 506 additions & 44 deletions

File tree

‎docs/src/app/(docs)/react/components/combobox/page.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,7 @@ The typed filter still resets once the popup closes.
250250

251251
`<Combobox.Input>` can be rendered inside `<Combobox.Popup>` to create a searchable select popup.
252252

253-
In this layout, focus returns to the trigger as soon as the popup closes, so typing while the popup plays its exit animation no longer reopens it — those keystrokes reach the trigger and are dropped. Press <kbd>ArrowDown</kbd> to reopen. When the input sits outside the popup, focus never leaves it and typing during the exit animation still reopens and filters as before.
253+
In this layout, focus returns to the trigger as soon as the popup closes, so the input can't be typed in or clicked while the popup plays its exit animation. Press <kbd>ArrowDown</kbd> or <kbd>ArrowUp</kbd> to reopen with a cleared filter. When the input sits outside the popup, focus stays in it, and typing during the exit animation reopens the popup and filters.
254254

255255
import { DemoComboboxInputInsidePopup } from './demos/input-inside-popup';
256256

‎docs/src/app/(docs)/react/handbook/animation/page.mdx‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ Base UI components can be animated using CSS transitions, CSS animations, or Ja
77

88
## Behavior while animating out
99

10-
A component that stays in the DOM while closed is already closed. Base UI marks the closed subtree with the [`inert`](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/inert) attribute for the popups that move focus into themselves when they open: Dialog, Alert dialog, Drawer, Popover, Menu, Context menu, Menubar, Select, Combobox and Autocomplete. Those return focus when the component closes, rather than when the exit animation finishes, so a closing popup is never left holding focus or reachable by the Tab key. This applies whenever the popup is closed and still rendered, including a `keepMounted` popup that has never been opened.
10+
A component that stays in the DOM while closed is already closed. Base UI marks the closed subtree with the [`inert`](https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Global_attributes/inert) attribute, so a closing popup is neither exposed to assistive technology nor reachable by the Tab key. This applies whenever the popup is closed and still rendered, including a `keepMounted` popup that has never been opened.
1111

12-
Tooltip, Preview card and Navigation menu are not made inert, because they don't return focus on close. Navigation menu is the notable one: focus can be inside it, and it is only restored once the popup unmounts.
12+
An inert subtree can't hold focus, so this only applies where something hands focus back on close: [Dialog](/react/components/dialog), [Alert Dialog](/react/components/alert-dialog), [Drawer](/react/components/drawer), [Popover](/react/components/popover), [Menu](/react/components/menu), [Context Menu](/react/components/context-menu), [Menubar](/react/components/menubar), [Select](/react/components/select), [Combobox](/react/components/combobox) and [Autocomplete](/react/components/autocomplete). [Tooltip](/react/components/tooltip) and [Preview Card](/react/components/preview-card) never manage focus and are left as they are, as is a Popover opened by hover. [Navigation Menu](/react/components/navigation-menu) restores focus only once the popup unmounts, so its closing content goes inert only while focus is outside it.
1313

14-
An inert subtree also stops receiving pointer events, so a click during the exit animation passes through to whatever is behind the popup rather than doing nothing. Keep exit animations short if a stray click in that window would be disruptive.
14+
Focus is handed back when the component closes rather than when the exit animation finishes, so a popup closed while focus is inside it moves focus to its trigger, or to whatever was focused before it opened. Passing `finalFocus` opts out, whether it is `false` or a function returning `false`: focus is then yours to move, and if you do not move it, it drops to `<body>` for the rest of the exit animation.
15+
16+
An inert subtree also stops receiving pointer events, so a click during the exit animation passes through to whatever is behind the popup rather than doing nothing. A backdrop is not inert, so a modal Dialog or Drawer still absorbs that click. Keep exit animations short if a stray click in that window would be disruptive.
1517

1618
## CSS transitions
1719

‎docs/src/app/(private)/experiments/exit-animation-a11y.tsx‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,16 @@ export default function ExitAnimationA11y() {
8282
<div className={styles.Case}>
8383
<h2 className={styles.CaseTitle}>Combobox — input outside the popup</h2>
8484
<p className={styles.CaseText}>
85-
Unchanged: focus never enters the popup, so typing during the exit animation still
86-
reopens and filters.
85+
Typing during the exit animation still reopens and filters, because focus stays on the
86+
input. Click &ldquo;Create new&rdquo; first and then press Escape: focus was inside the
87+
popup, so it is handed back to the input rather than dropped on the page.
8788
</p>
8889
<Combobox.Root items={fruits}>
8990
<Combobox.Input
9091
className={styles.Input}
9192
placeholder="Search fruits"
9293
aria-label="Fruits"
94+
data-testid="outside-input"
9395
/>
9496
<Combobox.Portal>
9597
<Combobox.Positioner sideOffset={4}>
@@ -101,6 +103,9 @@ export default function ExitAnimationA11y() {
101103
</Combobox.Item>
102104
)}
103105
</Combobox.List>
106+
<button type="button" className={styles.Button} data-testid="popup-footer">
107+
Create new…
108+
</button>
104109
</Combobox.Popup>
105110
</Combobox.Positioner>
106111
</Combobox.Portal>

‎packages/react/src/combobox/popup/ComboboxPopup.test.tsx‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { expect } from 'vitest';
22
import * as React from 'react';
33
import { Combobox } from '@base-ui/react/combobox';
4-
import { createRenderer, describeConformance } from '#test-utils';
4+
import { createRenderer, describeConformance, isJSDOM } from '#test-utils';
55
import { fireEvent, screen, waitFor } from '@mui/internal-test-utils';
66

77
describe('<Combobox.Popup />', () => {
@@ -150,4 +150,102 @@ describe('<Combobox.Popup />', () => {
150150
expect(screen.getByRole('button', { name: 'final focus' })).toHaveFocus();
151151
});
152152
});
153+
154+
// `inert` is only implemented in a real browser; jsdom keeps focus inside the closing popup.
155+
describe.skipIf(isJSDOM)('exit animation', () => {
156+
const style = `
157+
@keyframes combobox-close-test {
158+
to {
159+
opacity: 0;
160+
}
161+
}
162+
163+
.animation-test-popup[data-ending-style] {
164+
animation: combobox-close-test 500ms linear;
165+
}
166+
`;
167+
168+
// The common layout: the input sits outside the popup, so the focus manager runs with
169+
// `returnFocus: false` on the assumption that focus never left the input. A control inside the
170+
// popup breaks that assumption.
171+
function AnimatedCombobox() {
172+
return (
173+
<React.Fragment>
174+
{/* eslint-disable-next-line react/no-danger */}
175+
<style dangerouslySetInnerHTML={{ __html: style }} />
176+
<Combobox.Root items={['a', 'b']}>
177+
<Combobox.Input data-testid="input" />
178+
<Combobox.Portal>
179+
<Combobox.Positioner>
180+
<Combobox.Popup data-testid="popup" className="animation-test-popup">
181+
<Combobox.List>
182+
{(item: string) => (
183+
<Combobox.Item key={item} value={item}>
184+
{item}
185+
</Combobox.Item>
186+
)}
187+
</Combobox.List>
188+
<button type="button" data-testid="inside">
189+
Create new
190+
</button>
191+
</Combobox.Popup>
192+
</Combobox.Positioner>
193+
</Combobox.Portal>
194+
</Combobox.Root>
195+
<button type="button" data-testid="after">
196+
After
197+
</button>
198+
</React.Fragment>
199+
);
200+
}
201+
202+
it('returns focus to the input when the closing popup held it', async ({ onTestFinished }) => {
203+
globalThis.BASE_UI_ANIMATIONS_DISABLED = false;
204+
onTestFinished(() => {
205+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
206+
});
207+
208+
const { user } = await render(<AnimatedCombobox />);
209+
await user.click(screen.getByTestId('input'));
210+
await user.click(await screen.findByTestId('inside'));
211+
expect(screen.getByTestId('inside')).toHaveFocus();
212+
213+
await user.keyboard('{Escape}');
214+
expect(screen.getByTestId('popup')).toHaveAttribute('data-ending-style');
215+
216+
// Making the closing popup inert blurs whatever it held, so without a fallback focus would
217+
// sit on `<body>` for the whole exit animation, restarting the tab order at the top of the
218+
// document.
219+
await waitFor(() => expect(screen.getByTestId('input')).toHaveFocus());
220+
221+
await user.keyboard('{Tab}');
222+
expect(screen.getByTestId('after')).toHaveFocus();
223+
});
224+
225+
it('leaves focus alone when an outside press moved it out of the popup', async ({
226+
onTestFinished,
227+
}) => {
228+
globalThis.BASE_UI_ANIMATIONS_DISABLED = false;
229+
onTestFinished(() => {
230+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
231+
});
232+
233+
const { user } = await render(
234+
<React.Fragment>
235+
<p data-testid="plain">Not focusable</p>
236+
<AnimatedCombobox />
237+
</React.Fragment>,
238+
);
239+
await user.click(screen.getByTestId('input'));
240+
await user.click(await screen.findByTestId('inside'));
241+
expect(screen.getByTestId('inside')).toHaveFocus();
242+
243+
// Pressing non-focusable content blurs to `<body>`. That is the user's own gesture rather
244+
// than `inert` stranding focus, so the popup must not claw it back to the input.
245+
await user.click(screen.getByTestId('plain'));
246+
expect(screen.getByTestId('popup')).toHaveAttribute('data-ending-style');
247+
248+
await waitFor(() => expect(screen.getByTestId('input')).not.toHaveFocus());
249+
});
250+
});
153251
});

‎packages/react/src/combobox/popup/ComboboxPopup.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ export const ComboboxPopup = React.forwardRef(function ComboboxPopup(
134134
openInteractionType={openMethod}
135135
initialFocus={resolvedInitialFocus}
136136
returnFocus={resolvedFinalFocus}
137+
returnFocusIfOrphaned={finalFocus == null}
137138
getInsideElements={() => [
138139
store.state.startDismissRef.current,
139140
store.state.endDismissRef.current,

‎packages/react/src/combobox/root/ComboboxRoot.test.tsx‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,9 @@ describe('<Combobox.Root />', () => {
534534
// parked inside the subtree that's animating out.
535535
expect(trigger).toHaveFocus();
536536

537-
// The closing popup is inert, so focus can't move back into it either.
537+
// The closing popup is inert, so focus can't move back into it either. Re-checked here
538+
// because an unmounted popup would satisfy the assertions below without testing anything.
539+
expect(popup).toHaveAttribute('data-ending-style');
538540
input.focus();
539541
expect(input).not.toHaveFocus();
540542
expect(trigger).toHaveFocus();

‎packages/react/src/dialog/popup/DialogPopup.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ export const DialogPopup = React.forwardRef(function DialogPopup(
106106
closeOnFocusOut={!disablePointerDismissal}
107107
initialFocus={resolvedInitialFocus}
108108
returnFocus={finalFocus}
109+
returnFocusIfOrphaned={finalFocus == null}
109110
modal={modal !== false}
110111
restoreFocus="popup"
111112
>

‎packages/react/src/dialog/root/DialogRoot.test.tsx‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2281,8 +2281,13 @@ describe('<Dialog.Root />', () => {
22812281
);
22822282
}
22832283

2284-
it('returns focus to the trigger and makes the popup inert while it animates out', async () => {
2284+
it('returns focus to the trigger and makes the popup inert while it animates out', async ({
2285+
onTestFinished,
2286+
}) => {
22852287
globalThis.BASE_UI_ANIMATIONS_DISABLED = false;
2288+
onTestFinished(() => {
2289+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
2290+
});
22862291

22872292
const { user } = await render(<AnimatedDialog />);
22882293

@@ -2307,8 +2312,11 @@ describe('<Dialog.Root />', () => {
23072312
expect(inside).not.toHaveFocus();
23082313
});
23092314

2310-
it('removes the focus guards while the popup animates out', async () => {
2315+
it('removes the focus guards while the popup animates out', async ({ onTestFinished }) => {
23112316
globalThis.BASE_UI_ANIMATIONS_DISABLED = false;
2317+
onTestFinished(() => {
2318+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
2319+
});
23122320

23132321
const { user } = await render(<AnimatedDialog />);
23142322
const guards = () => document.querySelectorAll('[data-base-ui-focus-guard]');

‎packages/react/src/drawer/popup/DrawerPopup.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ export const DrawerPopup = React.forwardRef(function DrawerPopup(
405405
closeOnFocusOut={!disablePointerDismissal}
406406
initialFocus={resolvedInitialFocus}
407407
returnFocus={finalFocus}
408+
returnFocusIfOrphaned={finalFocus == null}
408409
modal={modal !== false}
409410
restoreFocus="popup"
410411
>

0 commit comments

Comments
 (0)