Skip to content

Commit 345c66c

Browse files
committed
fix(react-headless-components-preview): move overlays onto the reworked useTopLayer options
- Popover passes the hook's onKeyDown to the surface; returnFocus targets the trigger - DialogSurface reports Escape through the hook as escapeKeyDown and other browser opens and closes as surfaceToggle; non-modal uses returnFocus: 'always' - Menu focuses its first item with initialFocus and re-applies focusgroup in onShow - Tooltip, Listbox and Toaster move to onOpenChange and returnFocus - spec: add the Consumers and Changes from master sections
1 parent affdffb commit 345c66c

14 files changed

Lines changed: 136 additions & 91 deletions

File tree

‎packages/react-components/react-headless-components-preview/library/docs/top-layer-spec.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,15 @@ This covers what the browser doesn't restore: `manual`, `hint`, a `<dialog>` sho
145145
- Showing a modal closes open `auto` popovers. Content that must sit above a modal, like toasts, has to be shown after it.
146146
- Popovers opened without user activation in between share a close watcher, so one Escape closes them all. Tests need trusted input (`cypress-real-events`).
147147

148+
## Consumers
149+
150+
- **Popover / TeachingPopover / InfoLabel.** `usePopover` drives `contentRef` with `mode: trapFocus ? 'modal' : mode` and `returnFocus: triggerRef`, routes `onOpenChange` into its open state, and passes the hook's `onKeyDown` to the surface, so a `manual` surface closes on Escape. A custom surface without `contentRef` is mounted only while open.
151+
- **Dialog / Drawer.** `DialogSurface` uses `'modal'`, or `'manual'` when non-modal, with `preventEscapeClose` because `onOpenChange` can veto. Escape through the hook's `onKeyDown` is reported as `escapeKeyDown`; Escape inside a nested layer, or while an `auto` or `hint` popover such as a Tooltip is open inside, is left to that layer, and Escape from content portaled out of the surface still closes it. Every other browser open or close (`popovertarget`, `commandfor`, `<form method="dialog">`, `dialog.close()`, a close request that never reached the surface) is reported as `surfaceToggle`. A non-modal surface uses `returnFocus: 'always'`, as before. Backdrop clicks stay in a React handler.
152+
- **Tooltip.** `mode: 'hint'`, which acts as `manual` where unsupported, so it never closes an open `auto` popover. Stays mounted so `aria-labelledby` / `aria-describedby` resolve.
153+
- **Menu.** `MenuPopover` stays mounted with `mode: 'auto'`, focuses the first enabled item through `initialFocus`, re-applies `focusgroup` in `onShow`, and leaves focus return to `useMenu`. Browser closes are reported as `clickOutside`.
154+
- **Combobox / Dropdown / TagPicker listbox.** `mode: 'auto'`, rendered only while open. Focus stays on the trigger. Browser opens and closes go through `setOpen`.
155+
- **Toaster.** Each toast position is a `manual` popover, shown while it has toasts.
156+
148157
## Decisions
149158

150159
**React owns `open`.** The hook reports every open and close the browser makes, and none made through `open`. A browser close is final: the element stays closed even if the consumer keeps `open`. Escape on a modal follows the platform: the browser closes it and `onOpenChange` reports it. `preventEscapeClose` keeps it open for consumers that can veto.
@@ -154,3 +163,20 @@ This covers what the browser doesn't restore: `manual`, `hint`, a `<dialog>` sho
154163
**Internal only.** A public hook would let consumers build overlays without roles, keyboard handling or focus behaviour. Custom overlays compose the components instead; where a component lacks a mode (e.g. `manual` or `hint` on Popover), the component gains it. The hook can become public later if a case comes up that composition cannot cover. Option shapes can change freely until then.
155164

156165
**Focus follows the platform.** The hook doesn't move focus on open unless asked, doesn't wrap Tab, and returns focus only where the browser doesn't, including after light dismiss, so keyboard and screen reader users don't lose their place.
166+
167+
## Changes from master
168+
169+
Visible differences in the components moved onto `useTopLayer`. Everything else is unchanged.
170+
171+
| Area | Master | Now |
172+
| ---------------------------------------------------------------------------- | ---------------------------------------------------------------------------- | -------------------------------------------------------------------------- |
173+
| Popover exit | unmounted once the browser reported the close | also waits for exit animations, like `DialogSurface` |
174+
| Exiting Popover and Dialog | — | `inert` and `aria-hidden="true"` while exiting |
175+
| Escape in a layer nested in a `DialogSurface`, or with a Tooltip open inside | the outer dialog closed too, unless the inner handler stopped the event | only the inner one closes |
176+
| `popover="hint"` on Tooltip | `hint`; browsers without it treat it as `manual` | unchanged |
177+
| Custom Popover surface | mounted only while open | unchanged |
178+
| Popover kept `open` after a browser dismiss | shown again | stays closed, with a dev warning; use `mode="manual"` to prevent dismissal |
179+
| Modal Dialog native close (`<form method="dialog">`) | closes, not reported, so React state goes out of sync | closes, reported as `surfaceToggle` |
180+
| Modal Dialog close request without a keydown on it (focus on `<body>`) | ignored | reported as `surfaceToggle` |
181+
| Popover `trapFocus` Escape | browser closes the dialog, `onOpenChange` `type: 'close'` | unchanged |
182+
| Native opens (`popovertarget`, `commandfor`, `showModal()`) | non-modal Dialog synced as `surfaceToggle`; Popover unreachable while closed | also modal Dialog, where dialogs fire `toggle` (Chromium 132+) |

‎packages/react-components/react-headless-components-preview/library/etc/popover.api.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ export const Popover: {
3434
};
3535

3636
// @public
37-
export type PopoverContextValue = Pick<PopoverState, 'open' | 'setOpen' | 'toggleOpen' | 'triggerRef' | 'contentRef' | 'arrowRef' | 'openOnHover' | 'openOnContext' | 'withArrow' | 'surfaceId' | 'trapFocus' | 'mode'> & {
37+
export type PopoverContextValue = Pick<PopoverState, 'open' | 'setOpen' | 'toggleOpen' | 'triggerRef' | 'contentRef' | 'arrowRef' | 'openOnHover' | 'openOnContext' | 'withArrow' | 'surfaceId' | 'trapFocus' | 'mode' | 'onSurfaceKeyDown'> & {
3838
positioning: {
3939
targetRef: React_2.RefCallback<HTMLElement>;
4040
containerRef: React_2.RefCallback<HTMLElement>;
@@ -80,6 +80,7 @@ export type PopoverState = Required<Pick<PopoverProps, 'open' | 'trapFocus' | 'm
8080
positioning: PositioningReturn;
8181
surfaceId: string;
8282
shouldRenderSurface: boolean;
83+
onSurfaceKeyDown: (event: React_2.KeyboardEvent<HTMLElement>) => void;
8384
};
8485

8586
// @public

‎packages/react-components/react-headless-components-preview/library/etc/teaching-popover.api.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,7 @@ export type TeachingPopoverState = Required<Pick<TeachingPopoverProps, 'open' |
260260
positioning: PositioningReturn;
261261
surfaceId: string;
262262
shouldRenderSurface: boolean;
263+
onSurfaceKeyDown: (event: React_2.KeyboardEvent<HTMLElement>) => void;
263264
};
264265

265266
// @public

‎packages/react-components/react-headless-components-preview/library/src/components/Dialog/Dialog.test.tsx‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,49 @@ describe('Dialog', () => {
125125
);
126126
});
127127

128+
it('reports Escape once as escapeKeyDown and keeps the dialog open for the consumer to decide', () => {
129+
const onOpenChange = jest.fn();
130+
const { getByRole, getByText } = render(
131+
<Dialog open onOpenChange={onOpenChange}>
132+
<DialogSurface aria-label="Dialog">
133+
<button>Inside</button>
134+
</DialogSurface>
135+
</Dialog>,
136+
);
137+
const surface = getByRole('dialog', { hidden: true });
138+
139+
fireEvent.keyDown(getByText('Inside'), { key: 'Escape' });
140+
const cancelEvent = new Event('cancel', { cancelable: true });
141+
fireEvent(surface, cancelEvent);
142+
143+
expect(cancelEvent.defaultPrevented).toBe(true);
144+
expect(onOpenChange).toHaveBeenCalledTimes(1);
145+
expect(onOpenChange).toHaveBeenCalledWith(
146+
expect.anything(),
147+
expect.objectContaining({ type: 'escapeKeyDown', open: false }),
148+
);
149+
});
150+
151+
it('reports a close request that never reached the surface as surfaceToggle', () => {
152+
const onOpenChange = jest.fn();
153+
const { getByRole } = render(
154+
<Dialog open onOpenChange={onOpenChange}>
155+
<DialogSurface aria-label="Dialog">Content</DialogSurface>
156+
</Dialog>,
157+
);
158+
const surface = getByRole('dialog', { hidden: true });
159+
160+
// Escape with focus on <body>, or the Android back button: `cancel` without a keydown on the surface.
161+
const cancelEvent = new Event('cancel', { cancelable: true });
162+
fireEvent(surface, cancelEvent);
163+
164+
expect(cancelEvent.defaultPrevented).toBe(true);
165+
expect(onOpenChange).toHaveBeenCalledWith(
166+
expect.anything(),
167+
expect.objectContaining({ type: 'surfaceToggle', open: false }),
168+
);
169+
});
170+
128171
it('returns focus to the element focused before opening a non-modal dialog, even if focus moved outside', () => {
129172
const Example = ({ open }: { open: boolean }) => (
130173
<>

‎packages/react-components/react-headless-components-preview/library/src/components/Dialog/DialogSurface/useDialogSurface.ts‎

Lines changed: 20 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import { useDialogContext } from '../dialogContext';
1313
import { toDataAttributeValue } from '../../../utils';
1414
import { lockDocumentScroll, unlockDocumentScroll } from '../utils/scroll';
1515
import { useTopLayer } from '../../../hooks/useTopLayer/useTopLayer';
16-
import { isEscapeForLayer } from '../../../utils/topLayer';
1716
import type { DialogSurfaceProps, DialogSurfaceState } from './DialogSurface.types';
1817

1918
/**
@@ -33,38 +32,30 @@ export const useDialogSurface = (props: DialogSurfaceProps, ref: React.Ref<HTMLD
3332

3433
const isNonModal = modalType === 'non-modal';
3534

36-
// Non-modal: always return focus to the element focused before opening. Declared before `useTopLayer` so the
37-
// snapshot is taken before the surface is shown.
38-
const previouslyFocusedElement = React.useRef<HTMLElement | null>(null);
39-
useIsomorphicLayoutEffect(() => {
40-
if (!isNonModal) {
41-
return;
42-
}
43-
if (open) {
44-
previouslyFocusedElement.current = targetDocument?.activeElement as HTMLElement | null;
45-
return;
46-
}
47-
const elementToFocus = previouslyFocusedElement.current;
48-
previouslyFocusedElement.current = null;
49-
if (elementToFocus && targetDocument?.contains(elementToFocus)) {
50-
elementToFocus.focus();
51-
}
52-
}, [open, isNonModal, targetDocument]);
53-
54-
// A `popovertarget` invoker can toggle a non-modal surface; it is reported as `surfaceToggle`.
55-
const { shouldRender, phase } = useTopLayer(dialogRef, {
35+
// Escape goes through the hook's `onKeyDown`, so it is reported as `escapeKeyDown` with the React event. A browser
36+
// open or close (`popovertarget`, `commandfor`, `<form method="dialog">`) is reported as `surfaceToggle`.
37+
const { shouldRender, phase, onKeyDown } = useTopLayer(dialogRef, {
5638
open,
5739
mode: isNonModal ? 'manual' : 'modal',
5840
unmountOnClose,
59-
restoreFocus: !isNonModal,
41+
// Non-modal: always return focus to the element focused before opening.
42+
returnFocus: isNonModal ? 'always' : true,
6043
// `onOpenChange` can veto Escape, so the browser must not close the dialog first.
6144
preventEscapeClose: true,
62-
onOpen: isNonModal ? event => requestOpenChange({ type: 'surfaceToggle', open: true, event }) : undefined,
63-
// Escape is reported by the keydown handler below.
64-
onClose: ({ type, event }) => {
65-
if (type !== 'escape') {
66-
requestOpenChange({ type: 'surfaceToggle', open: false, event });
45+
onOpenChange: (nextOpen, { type, event }) => {
46+
if (!nextOpen && type === 'escape' && 'nativeEvent' in event) {
47+
requestOpenChange({
48+
type: 'escapeKeyDown',
49+
open: false,
50+
event: event as React.KeyboardEvent<HTMLDialogElement>,
51+
});
52+
return;
6753
}
54+
requestOpenChange({
55+
type: 'surfaceToggle',
56+
open: nextOpen,
57+
event: 'nativeEvent' in event ? event.nativeEvent : event,
58+
});
6859
},
6960
});
7061

@@ -78,17 +69,8 @@ export const useDialogSurface = (props: DialogSurfaceProps, ref: React.Ref<HTMLD
7869

7970
const handleKeyDown = useEventCallback((event: React.KeyboardEvent<HTMLDialogElement>) => {
8071
props.onKeyDown?.(event);
81-
// Escape inside a nested popover or dialog closes that layer first.
82-
if (
83-
event.key === 'Escape' &&
84-
!event.isDefaultPrevented() &&
85-
isEscapeForLayer(event.currentTarget, event.target as Element)
86-
) {
87-
requestOpenChange({ type: 'escapeKeyDown', open: false, event });
88-
// Stop propagation to avoid closing parent dialogs simultaneously.
89-
event.stopPropagation();
90-
return;
91-
}
72+
// Escape, unless it belongs to a layer nested inside.
73+
onKeyDown(event);
9274

9375
// When the non-modal surface itself is focused, move Tab to the first
9476
// focusable descendant explicitly.

‎packages/react-components/react-headless-components-preview/library/src/components/Dropdown/useListboxSlot.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,8 @@ export function useListboxSlot(
106106
open,
107107
mode: 'auto',
108108
unmountOnClose: false,
109-
restoreFocus: false,
110-
onClose: ({ event }) => setOpen(event as unknown as DropdownOpenEvents, false),
111-
onOpen: event => setOpen(event as unknown as DropdownOpenEvents, true),
109+
returnFocus: false,
110+
onOpenChange: (nextOpen, { event }) => setOpen(event as unknown as DropdownOpenEvents, nextOpen),
112111
});
113112

114113
return listboxSlot;

‎packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuPopover/useMenuPopover.ts‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import type * as React from 'react';
44
import { useMenuPopoverBase_unstable } from '@fluentui/react-menu';
55
import type { MenuPopoverProps, MenuPopoverState } from '@fluentui/react-menu';
6-
import { useIsomorphicLayoutEffect } from '@fluentui/react-utilities';
76
import { useMenuContext } from '../menuContext';
87
import { useTopLayer } from '../../../hooks/useTopLayer';
98

@@ -22,35 +21,28 @@ export const useMenuPopover = (props: MenuPopoverProps, ref: React.Ref<HTMLEleme
2221
const setOpen = useMenuContext(ctx => ctx.setOpen);
2322
const menuPopoverRef = useMenuContext(ctx => ctx.menuPopoverRef);
2423

25-
// Before `useTopLayer` shows the popover, so the browser focuses the `autofocus` item.
26-
useIsomorphicLayoutEffect(() => {
27-
if (open) {
28-
menuPopoverRef.current?.querySelector<HTMLElement>(MENU_ITEM_SELECTOR)?.setAttribute('autofocus', '');
29-
}
30-
}, [menuPopoverRef, open]);
31-
32-
// Stays mounted; `useMenu` restores focus.
24+
// Stays mounted; `useMenu` restores focus. The first enabled item takes focus, and `focusgroup` is re-applied once
25+
// shown, so the polyfill sees the visible items.
3326
useTopLayer(menuPopoverRef as React.RefObject<HTMLElement | null>, {
3427
open,
3528
mode: 'auto',
3629
unmountOnClose: false,
37-
restoreFocus: false,
38-
onClose: ({ event }) =>
39-
setOpen(event as unknown as MouseEvent, {
40-
open: false,
41-
type: 'clickOutside',
42-
event: event as unknown as MouseEvent,
43-
}),
30+
returnFocus: false,
31+
initialFocus: element => element.querySelector<HTMLElement>(MENU_ITEM_SELECTOR),
32+
onShow: element => {
33+
const menuListEl = element.querySelector<HTMLElement>('[focusgroup]');
34+
menuListEl?.setAttribute('focusgroup', menuListEl.getAttribute('focusgroup') ?? '');
35+
},
36+
onOpenChange: (nextOpen, { event }) => {
37+
if (!nextOpen) {
38+
setOpen(event as unknown as MouseEvent, {
39+
open: false,
40+
type: 'clickOutside',
41+
event: event as unknown as MouseEvent,
42+
});
43+
}
44+
},
4445
});
4546

46-
// Re-apply `focusgroup` once shown, so the polyfill sees the visible items.
47-
useIsomorphicLayoutEffect(() => {
48-
if (!open) {
49-
return;
50-
}
51-
const menuListEl = menuPopoverRef.current?.querySelector<HTMLElement>('[focusgroup]');
52-
menuListEl?.setAttribute('focusgroup', menuListEl.getAttribute('focusgroup') ?? '');
53-
}, [menuPopoverRef, open]);
54-
5547
return state;
5648
};

‎packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.test.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,10 @@ describe('Popover', () => {
311311
const onRender = jest.fn();
312312
const MeasuredPopover = () => {
313313
const state = usePopover({ children: <div /> });
314-
onRender();
314+
// Count commits: `useTopLayer` adjusts state during render, which re-runs the body but doesn't commit twice.
315+
React.useEffect(() => {
316+
onRender();
317+
});
315318
return (
316319
<>
317320
<button aria-expanded={state.open} onClick={event => state.setOpen(event, true)}>

‎packages/react-components/react-headless-components-preview/library/src/components/Popover/Popover.types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ export type PopoverState = Required<Pick<PopoverProps, 'open' | 'trapFocus' | 'm
133133
surfaceId: string;
134134
/** Determines whether the surface is present in the DOM. */
135135
shouldRenderSurface: boolean;
136+
/** Merged into the surface's `onKeyDown`; closes a `manual` surface on Escape. */
137+
onSurfaceKeyDown: (event: React.KeyboardEvent<HTMLElement>) => void;
136138
};
137139

138140
/**
@@ -152,6 +154,7 @@ export type PopoverContextValue = Pick<
152154
| 'surfaceId'
153155
| 'trapFocus'
154156
| 'mode'
157+
| 'onSurfaceKeyDown'
155158
> & {
156159
positioning: {
157160
targetRef: React.RefCallback<HTMLElement>;

‎packages/react-components/react-headless-components-preview/library/src/components/Popover/PopoverSurface/usePopoverSurface.ts‎

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,9 @@
11
'use client';
22

33
import type * as React from 'react';
4-
import { Escape } from '@fluentui/keyboard-keys';
54
import { useMergedRefs, slot, useEventCallback } from '@fluentui/react-utilities';
65
import { usePopoverContext } from '../popoverContext';
76
import { toDataAttributeValue } from '../../../utils';
8-
import { isEscapeForLayer } from '../../../utils/topLayer';
97
import type { PopoverSurfaceProps, PopoverSurfaceState } from './PopoverSurface.types';
108

119
/**
@@ -25,6 +23,7 @@ export const usePopoverSurface = (
2523
const surfaceId = usePopoverContext(context => context.surfaceId);
2624
const trapFocus = usePopoverContext(context => context.trapFocus);
2725
const mode = usePopoverContext(context => context.mode);
26+
const onSurfaceKeyDown = usePopoverContext(context => context.onSurfaceKeyDown);
2827

2928
if (process.env.NODE_ENV !== 'production' && trapFocus && props.as === 'div') {
3029
// eslint-disable-next-line no-console
@@ -63,18 +62,10 @@ export const usePopoverSurface = (
6362
onKeyDown: onKeyDownOriginal,
6463
} = state.root;
6564

66-
// The browser ignores Escape on `manual` popovers (and on `hint` where unsupported); close it from React instead.
65+
// The browser ignores Escape on `manual` popovers (and on `hint` where unsupported); `useTopLayer` closes it.
6766
state.root.onKeyDown = useEventCallback((e: React.KeyboardEvent<HTMLDialogElement & HTMLDivElement>) => {
6867
onKeyDownOriginal?.(e);
69-
const popoverMode = (e.currentTarget as HTMLElement & { popover?: string | null }).popover;
70-
if (
71-
e.key === Escape &&
72-
!e.isDefaultPrevented() &&
73-
popoverMode === 'manual' &&
74-
isEscapeForLayer(e.currentTarget, e.target as Element)
75-
) {
76-
setOpen(e, false);
77-
}
68+
onSurfaceKeyDown(e);
7869
});
7970

8071
state.root.onMouseEnter = useEventCallback((e: React.MouseEvent<HTMLDialogElement & HTMLDivElement>) => {

0 commit comments

Comments
 (0)