diff --git a/.changeset/headless-popover-menu.md b/.changeset/headless-popover-menu.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/headless-popover-menu.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/primitives/menu/README.md b/packages/headless/src/primitives/menu/README.md index ee2c9dda87e..4ac10b6dac2 100644 --- a/packages/headless/src/primitives/menu/README.md +++ b/packages/headless/src/primitives/menu/README.md @@ -139,7 +139,7 @@ Accepts all `FloatingArrow` props. `ref` and `context` are injected automaticall - Nested menus open on hover (75ms delay) with a `safePolygon` safe zone. - Only one sibling submenu can be open at a time. - Clicking any item with `closeOnClick={true}` (default) closes the entire menu tree via a tree event. -- `Escape` closes the innermost menu first, bubbling up through the tree. +- `Escape` closes one level: the innermost open menu, leaving its parent — a parent menu, or a `Popover` the menu is rendered inside — open. Pressing it again closes the next level up. An outside press is the opposite: it dismisses the whole stack at once. ## Important Notes diff --git a/packages/headless/src/primitives/menu/menu-root.tsx b/packages/headless/src/primitives/menu/menu-root.tsx index bf70205abb2..2afbed329be 100644 --- a/packages/headless/src/primitives/menu/menu-root.tsx +++ b/packages/headless/src/primitives/menu/menu-root.tsx @@ -114,7 +114,9 @@ function MenuInner(props: MenuProps) { delete reference.role; return { ...baseRole, reference }; }, [baseRole, isNested]); - const dismiss = useDismiss(floatingContext, { bubbles: true }); + // Escape must not bubble: it closes this menu and leaves whatever it sits inside — a parent menu, + // or a popover — open. An outside press is the opposite, and dismisses the whole stack. + const dismiss = useDismiss(floatingContext, { bubbles: { escapeKey: false, outsidePress: true } }); const listNavigation = useListNavigation(floatingContext, { listRef: elementsRef, activeIndex, diff --git a/packages/headless/src/primitives/menu/menu.test.tsx b/packages/headless/src/primitives/menu/menu.test.tsx index 0baf2917088..ec5349e579a 100644 --- a/packages/headless/src/primitives/menu/menu.test.tsx +++ b/packages/headless/src/primitives/menu/menu.test.tsx @@ -682,6 +682,80 @@ describe('Menu', () => { expect(onClick).toHaveBeenCalledTimes(1); }); + + it('Escape closes only the submenu', async () => { + const user = userEvent.setup(); + render( + + Actions + + + + Share + + + Email + + + + + + , + ); + + await user.click(screen.getByText('Actions')); + await new Promise(r => requestAnimationFrame(r)); + await user.keyboard('{ArrowDown}'); + await user.keyboard('{ArrowRight}'); + await user.keyboard('{Escape}'); + + expect(screen.getByText('Share')).toHaveAttribute('data-closed', ''); + expect(screen.getByText('Actions')).toHaveAttribute('data-open', ''); + }); + }); + + describe('inside a popover', () => { + function renderMenuInPopover() { + return render( + + Open popover + + + + Actions + + + Cut + + + + + + , + ); + } + + it('Escape closes only the menu', async () => { + const user = userEvent.setup(); + renderMenuInPopover(); + + await user.click(screen.getByText('Actions')); + await user.keyboard('{Escape}'); + + expect(screen.getByText('Actions')).toHaveAttribute('data-closed', ''); + expect(screen.getByText('Open popover')).toHaveAttribute('data-open', ''); + }); + + it('Escape closes the popover once the menu is closed', async () => { + const user = userEvent.setup(); + renderMenuInPopover(); + + await user.click(screen.getByText('Actions')); + await user.keyboard('{Escape}'); + await user.keyboard('{Escape}'); + + expect(screen.getByText('Open popover')).toHaveAttribute('data-closed', ''); + }); }); describe('positioner', () => { diff --git a/packages/headless/src/primitives/popover/README.md b/packages/headless/src/primitives/popover/README.md index 58ceb1d53aa..5e75920e4b1 100644 --- a/packages/headless/src/primitives/popover/README.md +++ b/packages/headless/src/primitives/popover/README.md @@ -63,14 +63,15 @@ const [open, setOpen] = useState(false); ### `Popover.Root` -| Prop | Type | Default | Description | -| -------------- | ------------------------- | ---------- | ---------------------------------- | -| `open` | `boolean` | — | Controlled open state | -| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | -| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | -| `placement` | `Placement` | `"bottom"` | Floating UI placement | -| `sideOffset` | `number` | `4` | Gap between trigger and popup (px) | -| `modal` | `boolean` | `false` | Traps focus within the popover | +| Prop | Type | Default | Description | +| -------------- | ------------------------- | ---------- | ----------------------------------- | +| `open` | `boolean` | — | Controlled open state | +| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | +| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | +| `placement` | `Placement` | `"bottom"` | Floating UI placement | +| `sideOffset` | `number` | `4` | Gap between trigger and popup (px) | +| `alignOffset` | `number` | `0` | Nudge along the alignment axis (px) | +| `modal` | `boolean` | `false` | Traps focus within the popover | ### `Popover.Trigger`, `Popover.Positioner`, `Popover.Popup`, `Popover.Title`, `Popover.Description`, `Popover.Close` @@ -104,6 +105,7 @@ Middleware stack: `offset` -> `flip` -> `shift` -> `arrow` -> CSS vars. The popu - **Title and Description are optional but recommended.** They wire `aria-labelledby` and `aria-describedby` to the positioner. If omitted, those attributes are simply absent. - **Non-modal by default.** Unlike Dialog, the page remains interactive behind the popover. Set `modal={true}` for a stricter focus trap. - **Nested popovers are supported.** The `FloatingTree` pattern handles nesting automatically. +- **Popup contents freeze while closing.** The popup outlives `open` by its exit animation, so its children are wrapped in `Freeze` (`@clerk/headless/utils`) and hold their last frame instead of re-rendering under the animation. The popup element itself keeps updating, so `data-closed` / `data-ending-style` still land. Freezing wraps the children in a `display: contents` element and detaches refs inside them until the popup reopens. ## ARIA diff --git a/packages/headless/src/primitives/popover/popover-popup.tsx b/packages/headless/src/primitives/popover/popover-popup.tsx index 8f8d5b5a13b..64e2bcbd1da 100644 --- a/packages/headless/src/primitives/popover/popover-popup.tsx +++ b/packages/headless/src/primitives/popover/popover-popup.tsx @@ -2,17 +2,22 @@ import React from 'react'; -import { type ComponentProps, mergeProps, useRender } from '../../utils'; +import { type ComponentProps, Freeze, mergeProps, useRender } from '../../utils'; import { usePopoverContext } from './popover-context'; export type PopoverPopupProps = ComponentProps<'div'>; export const PopoverPopup = React.forwardRef(function PopoverPopup(props, ref) { - const { render, ...otherProps } = props; - const { popupRef, transitionProps } = usePopoverContext(); + const { render, children, ...otherProps } = props; + const { open, popupRef, transitionProps } = usePopoverContext(); const defaultProps = { ...transitionProps, + // The popup outlives `open` by the length of its exit animation. Whatever closed it has + // usually changed the data behind it (switching account, picking an item), so the contents + // hold their last frame on the way out instead of swapping under the animation. The popup + // element itself stays live, so `data-closed` / `data-ending-style` still land. + children: {children}, }; return useRender({ diff --git a/packages/headless/src/primitives/popover/popover-root.tsx b/packages/headless/src/primitives/popover/popover-root.tsx index d9831ea4e16..212277c3e4c 100644 --- a/packages/headless/src/primitives/popover/popover-root.tsx +++ b/packages/headless/src/primitives/popover/popover-root.tsx @@ -31,6 +31,7 @@ export interface PopoverProps { onOpenChange?: (open: boolean) => void; placement?: Placement; sideOffset?: number; + alignOffset?: number; modal?: boolean; /** * Where focus lands when the popup opens. @@ -47,7 +48,14 @@ export interface PopoverProps { function PopoverInner(props: PopoverProps) { const nodeId = useFloatingNodeId(); - const { placement: placementProp = 'bottom', sideOffset = 4, modal = false, initialFocus = 'auto', children } = props; + const { + placement: placementProp = 'bottom', + sideOffset = 4, + alignOffset = 0, + modal = false, + initialFocus = 'auto', + children, + } = props; const [open, setOpen] = useControllableState(props.open, props.defaultOpen ?? false, props.onOpenChange); @@ -71,7 +79,7 @@ function PopoverInner(props: PopoverProps) { onOpenChange: setOpen, placement: placementProp, middleware: [ - offset(sideOffset), + offset({ mainAxis: sideOffset, alignmentAxis: alignOffset }), flip({ crossAxis: placementProp.includes('-'), fallbackAxisSideDirection: 'end', diff --git a/packages/headless/src/utils/freeze.test.tsx b/packages/headless/src/utils/freeze.test.tsx new file mode 100644 index 00000000000..b0fb5adf2cd --- /dev/null +++ b/packages/headless/src/utils/freeze.test.tsx @@ -0,0 +1,72 @@ +import { cleanup, render, screen } from '@testing-library/react'; +import { afterEach, describe, expect, it } from 'vitest'; + +import { Freeze } from './freeze'; + +afterEach(() => { + cleanup(); +}); + +describe('Freeze', () => { + it('renders children while not frozen', () => { + render(Acme); + + expect(screen.getByText('Acme')).toBeInTheDocument(); + }); + + it('holds the committed DOM when children change while frozen', () => { + const { rerender } = render(Acme); + + rerender(Globex); + + expect(screen.getByText('Acme')).toBeInTheDocument(); + expect(screen.queryByText('Globex')).toBeNull(); + }); + + it('keeps the held DOM visible', () => { + const { rerender } = render(Acme); + + rerender(Globex); + + expect(screen.getByText('Acme')).toBeVisible(); + }); + + it('keeps the held DOM visible across further updates while frozen', () => { + const { rerender } = render(Acme); + + rerender(Globex); + rerender(Initech); + + expect(screen.getByText('Acme')).toBeVisible(); + }); + + it('commits the pending children once unfrozen', () => { + const { rerender } = render(Acme); + + rerender(Globex); + rerender(Globex); + + expect(screen.getByText('Globex')).toBeInTheDocument(); + expect(screen.queryByText('Acme')).toBeNull(); + }); + + it('holds state updates raised from inside the frozen subtree', () => { + function Counter({ count }: { count: number }) { + return count: {count}; + } + + const { rerender } = render( + + + , + ); + + rerender( + + + , + ); + + expect(screen.getByText('count: 0')).toBeInTheDocument(); + }); +}); diff --git a/packages/headless/src/utils/freeze.tsx b/packages/headless/src/utils/freeze.tsx new file mode 100644 index 00000000000..bdb5469ebf6 --- /dev/null +++ b/packages/headless/src/utils/freeze.tsx @@ -0,0 +1,64 @@ +'use client'; + +import * as React from 'react'; + +/** + * Never settles. Throwing it suspends the enclosing boundary indefinitely: React keeps + * rendering the subtree but holds the commit, so the DOM keeps painting its last frame. + */ +const never = new Promise(() => {}); + +function Suspend(): null { + // eslint-disable-next-line @typescript-eslint/only-throw-error -- Suspending is React's thrown-thenable protocol, not an error. `React.use()` would say this more plainly but needs React 19.2; this package supports React 18. + throw never; +} + +export interface FreezeProps { + /** While `true`, the DOM below holds whatever it last committed. */ + frozen: boolean; + children?: React.ReactNode; +} + +/** + * Holds its subtree's DOM at the last committed frame while `frozen`. Renders keep + * happening, they just don't reach the DOM; the pending one commits when `frozen` flips + * back to `false`. + * + * Use it to stop content from visibly changing under an exit animation — a popover that + * closes because the thing it was showing changed would otherwise swap its contents on the + * way out. + */ +export function Freeze({ frozen, children }: FreezeProps) { + const contentRef = React.useRef(null); + + // Hold onto the node ourselves rather than reading a plain ref: hiding a boundary's children + // detaches their refs, so by the time the effect below runs a normal ref reads `null`. + const setContent = React.useCallback((node: HTMLDivElement | null) => { + if (node) { + contentRef.current = node; + } + }, []); + + // React hides a suspended boundary's host children with `display: none !important`, which is + // the opposite of what this is for. Undo it on the commit that applies it: insertion effects + // run after the boundary's mutation and before paint, so the held frame never blinks out. + // `display: contents` is also what the wrapper renders with, so React puts it back on unfreeze + // and the wrapper stays out of the layout it is spliced into. + React.useInsertionEffect(() => { + if (frozen) { + contentRef.current?.style.setProperty('display', 'contents'); + } + }, [frozen]); + + return ( + + {frozen ? : null} +
+ {children} +
+ + ); +} diff --git a/packages/headless/src/utils/index.ts b/packages/headless/src/utils/index.ts index 566a8adacfa..f54beed344e 100644 --- a/packages/headless/src/utils/index.ts +++ b/packages/headless/src/utils/index.ts @@ -1,4 +1,5 @@ export { cssVars } from './css-vars'; +export { Freeze, type FreezeProps } from './freeze'; export { isKeyboardEvent, isKeyboardOpen } from './interaction-modality'; export { resetLayoutStyles } from './reset-layout-styles'; export { diff --git a/packages/swingset/src/stories/popover.component.mdx b/packages/swingset/src/stories/popover.component.mdx index 9ce80bcbbb2..fde2fb75410 100644 --- a/packages/swingset/src/stories/popover.component.mdx +++ b/packages/swingset/src/stories/popover.component.mdx @@ -145,16 +145,37 @@ centering on it. Cross-axis flipping is only enabled for aligned placements, so `bottom-start` may become `bottom-end` near a viewport edge while a plain `bottom` will not. +`alignOffset` nudges the popup along that alignment axis, the way `sideOffset` does along the side. +Use it to cancel padding inside the popup so its content, rather than its edge, lines up with the +trigger — a negative value pulls a `-start` placement further left. + +```tsx + + Open + + + Pulled 8px left of the trigger's start edge. + + +; +``` + +It is a preference like placement is: `shift` still claws the popup back when the nudge would push +it out of view. + ## Parts -| Part | Slot | Description | -| --------------------- | --------------- | ----------------------------------------------------------------------- | -| `Popover.Root` | — | State provider; owns open/close, `placement`, `sideOffset`, `modal`. | -| `Popover.Trigger` | — | Anchor element; renders a `