From 385c106ec5c5d56be462ea2d54223c84e7cc34b8 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 18:15:59 -0600 Subject: [PATCH 01/18] feat(headless): dialog stacking state and alertdialog role Adds the two signals the Mosaic stacking styles need, and the role an AlertDialog preset needs. `data-stacked` / `data-stack-base` describe dialog-on-dialog specifically, in both directions of the relationship. `data-nested` could not: it reports any floating ancestor, so a dialog opened from a menu item reads as nested while sitting on the bare page. Under the incoming rule that a stacked dialog paints no backdrop, styling off `data-nested` would leave that dialog with no scrim at all. The child registers with its parent while OPEN rather than while mounted, so a dialog beneath comes forward with its child's exit transition rather than after it. The count is of direct children only, which is enough for the single recede step that exists. --- .changeset/dialog-stacking-state.md | 2 + .../headless/src/primitives/dialog/README.md | 44 +++-- .../src/primitives/dialog/dialog-backdrop.tsx | 8 +- .../src/primitives/dialog/dialog-context.ts | 8 + .../src/primitives/dialog/dialog-nesting.ts | 85 +++++++++ .../src/primitives/dialog/dialog-popup.tsx | 7 + .../src/primitives/dialog/dialog-root.tsx | 23 ++- .../src/primitives/dialog/dialog.test.tsx | 161 ++++++++++++++++++ .../headless/src/primitives/dialog/index.ts | 1 + .../headless/src/primitives/dialog/parts.ts | 8 +- 10 files changed, 326 insertions(+), 21 deletions(-) create mode 100644 .changeset/dialog-stacking-state.md create mode 100644 packages/headless/src/primitives/dialog/dialog-nesting.ts diff --git a/.changeset/dialog-stacking-state.md b/.changeset/dialog-stacking-state.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/dialog-stacking-state.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/primitives/dialog/README.md b/packages/headless/src/primitives/dialog/README.md index 51ae61f9227..e160414777a 100644 --- a/packages/headless/src/primitives/dialog/README.md +++ b/packages/headless/src/primitives/dialog/README.md @@ -137,16 +137,17 @@ the close was pointer-driven, where focus is left where the pointer put it (see ### `Dialog.Root` -| Prop | Type | Default | Description | -| -------------- | ----------------------------------------------------------- | ------- | --------------------------------------------------------------------- | -| `open` | `boolean` | — | Controlled open state | -| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | -| `onOpenChange` | `(open: boolean, details: DialogOpenChangeDetails) => void` | — | Called when open state changes; `details` names the trigger behind it | -| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | -| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog | -| `handle` | `DialogHandle` | — | Connects detached triggers (see `Dialog.createHandle()`) | -| `triggerId` | `string \| null` | — | Controls which trigger the open is attributed to | -| `children` | `ReactNode \| ({ payload }) => ReactNode` | — | Content, or a render function of the active trigger's `payload` | +| Prop | Type | Default | Description | +| -------------- | ----------------------------------------------------------- | ---------- | --------------------------------------------------------------------- | +| `open` | `boolean` | — | Controlled open state | +| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | +| `onOpenChange` | `(open: boolean, details: DialogOpenChangeDetails) => void` | — | Called when open state changes; `details` names the trigger behind it | +| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | +| `role` | `'dialog' \| 'alertdialog'` | `'dialog'` | The popup's ARIA role | +| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog | +| `handle` | `DialogHandle` | — | Connects detached triggers (see `Dialog.createHandle()`) | +| `triggerId` | `string \| null` | — | Controls which trigger the open is attributed to | +| `children` | `ReactNode \| ({ payload }) => ReactNode` | — | Content, or a render function of the active trigger's `payload` | #### `closedBy` @@ -218,10 +219,23 @@ No additional props beyond standard HTML attributes and the `render` prop. | --------------------------- | ---------------------------------- | ------------------------------------------- | | `data-open` / `data-closed` | Trigger, Backdrop, Viewport, Popup | Open state | | `data-nested` | Backdrop, Viewport, Popup | Opened from inside another floating element | +| `data-stacked` | Backdrop, Popup | Layered over an open dialog | +| `data-stack-base` | Popup | Has an open dialog layered over it | -`data-nested` is what a stacked overlay styles itself from — chiefly so backdrops don't composite -into an ever-darker scrim as the stack grows. It reflects any floating ancestor, not strictly a -dialog one: the `FloatingTree` a Menu or Popover establishes counts too. +`data-nested` reflects any floating ancestor: the `FloatingTree` a Menu or Popover establishes +counts too. + +`data-stacked` and `data-stack-base` are narrower, and are what stacking styles should use. They +describe dialog-on-dialog specifically, in the two directions of the same relationship — the one +on top, and the one it covers. A dialog opened from a menu item is `data-nested` but not +`data-stacked`: it has a floating ancestor, yet it sits on the bare page and still owns its scrim. + +Both can be set at once, and that is the ordinary case rather than an edge — in a panel → prompt → +alert stack, the middle dialog is stacked on one surface while another is stacked on it. + +`data-stacked` exists chiefly so the stack shows one scrim: the dialog on top drops its own +backdrop instead of compositing a darker one per level. `data-stack-base` is for whatever the +surface underneath does to signal depth. The headless parts are unstyled. Target a part with your own className (or `render` prop) and combine it with the `data-*` state attributes above. @@ -229,7 +243,7 @@ The headless parts are unstyled. Target a part with your own className (or `rend - **`Dialog.Popup` should be a child of `Dialog.Viewport`** for centered, scroll-locked modal behavior. The viewport hosts the fixed overlay container; the popup alone does not handle positioning or scroll lock. - **Title and Description are optional but recommended.** If omitted, `aria-labelledby` / `aria-describedby` are simply absent from the popup. -- **Nested dialogs are supported**, and covered by tests. The `FloatingTree` pattern handles it: `useDismiss` blocks both Escape and outside-press on a parent while any child is open, and `FloatingOverlay`'s scroll lock is refcounted, so the body stays locked until the last dialog closes. +- **Nested dialogs are supported**, and covered by tests. The `FloatingTree` pattern handles it: `useDismiss` blocks both Escape and outside-press on a parent while any child is open, and `FloatingOverlay`'s scroll lock is refcounted, so the body stays locked until the last dialog closes. Style the stack with `data-stacked` / `data-stack-base`, not `data-nested`. - **No positioning middleware.** Dialogs are centered via CSS, not Floating UI positioning. ## Authoring rule for new primitives @@ -238,5 +252,5 @@ Each styleable surface = one part. Layout infrastructure (overlay, scroll lock, ## ARIA -- Popup: `role="dialog"`, `aria-labelledby` (from Title), `aria-describedby` (from Description) +- Popup: `role="dialog"` (or `"alertdialog"`, via the root's `role`), `aria-labelledby` (from Title), `aria-describedby` (from Description) - Trigger: `aria-expanded`, `aria-haspopup="dialog"`, `aria-controls` diff --git a/packages/headless/src/primitives/dialog/dialog-backdrop.tsx b/packages/headless/src/primitives/dialog/dialog-backdrop.tsx index c934d4bfe6d..91f22b5a35d 100644 --- a/packages/headless/src/primitives/dialog/dialog-backdrop.tsx +++ b/packages/headless/src/primitives/dialog/dialog-backdrop.tsx @@ -12,9 +12,12 @@ export type DialogBackdropProps = ComponentProps<'div'>; export const DialogBackdrop = React.forwardRef( function DialogBackdrop(props, ref) { const { render, ...otherProps } = props; - const { open, mounted, isNested, transitionProps } = useDialogContext(); + const { open, mounted, isNested, isStacked, transitionProps } = useDialogContext(); - const state = { open, nested: isNested }; + // No `stacked` counterpart to `data-stack-base` here: what a dialog beneath the stack does is + // recede, and that is the popup's business. The backdrop only needs to know to get out of the + // way when it is not the one scrim the stack shows. + const state = { open, nested: isNested, stacked: isStacked }; const defaultProps = { ...transitionProps, @@ -29,6 +32,7 @@ export const DialogBackdrop = React.forwardRef | null => (v ? { 'data-open': '' } : { 'data-closed': '' }), nested: (v: boolean): Record | null => (v ? { 'data-nested': '' } : null), + stacked: (v: boolean): Record | null => (v ? { 'data-stacked': '' } : null), }, props: mergeProps<'div'>(defaultProps, otherProps), }); diff --git a/packages/headless/src/primitives/dialog/dialog-context.ts b/packages/headless/src/primitives/dialog/dialog-context.ts index 50101526501..a20561a2e64 100644 --- a/packages/headless/src/primitives/dialog/dialog-context.ts +++ b/packages/headless/src/primitives/dialog/dialog-context.ts @@ -31,6 +31,14 @@ export interface DialogContextValue { * cases coincide in practice. */ isNested: boolean; + /** + * Whether this dialog is layered over an open DIALOG — the signal the stacking styles key on, + * where `isNested` is too broad to use. A stacked dialog drops its own backdrop so the stack + * shows one scrim rather than compositing a darker one per level. + */ + isStacked: boolean; + /** How many open dialogs are stacked directly on this one. See `useDialogNesting`. */ + stackedChildCount: number; labelId: string; descriptionId: string; mounted: boolean; diff --git a/packages/headless/src/primitives/dialog/dialog-nesting.ts b/packages/headless/src/primitives/dialog/dialog-nesting.ts new file mode 100644 index 00000000000..2fff5e4fae4 --- /dev/null +++ b/packages/headless/src/primitives/dialog/dialog-nesting.ts @@ -0,0 +1,85 @@ +'use client'; + +import { createContext, useCallback, useContext, useLayoutEffect, useMemo, useState } from 'react'; + +/** + * How a dialog root reaches the dialog root it renders inside, so the two can style the stack + * they form: the one on top drops its backdrop, the one beneath recedes behind it. + * + * Deliberately separate from `isNested`, which reports any FLOATING ancestor — a Menu or a + * Popover counts. Stacking styles cannot key on that: a dialog opened from a menu item has a + * floating ancestor but sits on the bare page, and must still paint its own scrim. + */ +export interface DialogNestingContextValue { + /** Whether the surrounding dialog is itself open. */ + open: boolean; + /** + * Called by a dialog rendered inside this one, for as long as it is open. Returns the release. + * Stable for the lifetime of the root, so registering never churns. + */ + registerStackedChild: () => () => void; +} + +export const DialogNestingContext = createContext(null); + +/** What a root learns about the stack it belongs to. */ +export interface DialogNesting { + /** Whether this dialog is layered over an open dialog. */ + isStacked: boolean; + /** + * How many open dialogs are stacked directly on this one. Counts DIRECT children only — a + * three-deep stack reports 1 at both lower levels rather than 2 and 1 — which is enough for + * the single recede step that exists today. Making it cumulative means propagating the count + * back up the chain, and getting that to settle when two levels mount in one commit. + */ + stackedChildCount: number; + /** Provided to this root's children, so a dialog inside it registers against this one. */ + context: DialogNestingContextValue; +} + +/** + * Joins a dialog root to the stack it belongs to, in both directions: up, to report itself to + * the dialog it renders inside, and down, to count the dialogs that render inside it. + */ +export function useDialogNesting(open: boolean): DialogNesting { + const parent = useContext(DialogNestingContext); + const [stackedChildCount, setStackedChildCount] = useState(0); + + const registerStackedChild = useCallback(() => { + setStackedChildCount(count => count + 1); + let released = false; + return () => { + if (released) { + return; + } + released = true; + setStackedChildCount(count => count - 1); + }; + }, []); + + const registerWithParent = parent?.registerStackedChild; + + // Gated on `open` rather than on being mounted: a closing dialog stays mounted for the length + // of its exit transition, and the surface beneath has to come forward WITH it rather than + // after it. Depends on the registration function, not the whole context value, so a parent + // opening or closing does not re-register. + useLayoutEffect(() => { + if (!open || !registerWithParent) { + return; + } + return registerWithParent(); + }, [open, registerWithParent]); + + const context = useMemo( + () => ({ open, registerStackedChild }), + [open, registerStackedChild], + ); + + return { + // A closed parent is not something to sit on top of: the child owns the scrim in that case, + // which is what a confirmation root mounted beside its dialog's portal relies on. + isStacked: parent !== null && parent.open, + stackedChildCount, + context, + }; +} diff --git a/packages/headless/src/primitives/dialog/dialog-popup.tsx b/packages/headless/src/primitives/dialog/dialog-popup.tsx index fcb6023e59e..587062ecf2b 100644 --- a/packages/headless/src/primitives/dialog/dialog-popup.tsx +++ b/packages/headless/src/primitives/dialog/dialog-popup.tsx @@ -137,6 +137,8 @@ export const DialogPopup = React.forwardRef(fu floatingContext, modal, isNested, + isStacked, + stackedChildCount, returnFocusRef, labelId, descriptionId, @@ -155,6 +157,11 @@ export const DialogPopup = React.forwardRef(fu const defaultProps = { ...ownProps, ...(isNested ? { 'data-nested': '' } : {}), + // Both can be set at once, and that is the ordinary case rather than an edge: in a + // panel -> prompt -> alert stack the middle dialog is stacked on one surface while another + // is stacked on it. + ...(isStacked ? { 'data-stacked': '' } : {}), + ...(stackedChildCount > 0 ? { 'data-stack-base': '' } : {}), ...getFloatingProps(), ...transitionProps, }; diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index 187ba660bc1..dd32d1ae976 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -17,6 +17,7 @@ import { useReturnFocus } from '../../hooks/use-return-focus'; import { useTransition } from '../../hooks/use-transition'; import { DialogContext, type DialogContextValue } from './dialog-context'; import { createDialogHandle, type DialogHandle } from './dialog-handle'; +import { DialogNestingContext, useDialogNesting } from './dialog-nesting'; /** * Which gestures dismiss the dialog, mirroring the native `` attribute. @@ -31,6 +32,12 @@ import { createDialogHandle, type DialogHandle } from './dialog-handle'; */ export type DialogClosedBy = 'any' | 'closerequest' | 'none'; +/** + * The popup's ARIA role. `alertdialog` is for a dialog interrupting the user to confirm or warn, + * which assistive technology announces more urgently; everything else is a `dialog`. + */ +export type DialogRole = 'dialog' | 'alertdialog'; + /** What accompanies an `onOpenChange` call, mirroring Base UI's event details. */ export interface DialogOpenChangeDetails { /** @@ -53,6 +60,8 @@ export interface DialogProps { modal?: boolean; /** Which gestures dismiss the dialog. Default: `any` */ closedBy?: DialogClosedBy; + /** The popup's ARIA role. Default: `dialog` */ + role?: DialogRole; /** * Connects this root to triggers rendered outside it. Create with `Dialog.createHandle()` * and pass the same handle to each `Dialog.Trigger`. @@ -71,7 +80,7 @@ export interface DialogProps { function DialogInner(props: DialogProps & { isNested: boolean }) { const nodeId = useFloatingNodeId(); - const { modal = true, closedBy = 'any', isNested, children, onOpenChange } = props; + const { modal = true, closedBy = 'any', role: ariaRole = 'dialog', isNested, children, onOpenChange } = props; const fallbackStore = useMemo(() => createDialogHandle(), []); const store = props.handle ?? fallbackStore; @@ -80,6 +89,8 @@ function DialogInner(props: DialogProps & { isNested: boolean const [activeTriggerId, setActiveTriggerId] = useControllableState(props.triggerId, null); const [activePayload, setActivePayload] = useState(undefined); + const nesting = useDialogNesting(open); + const labelId = useId(); const descriptionId = useId(); @@ -172,7 +183,7 @@ function DialogInner(props: DialogProps & { isNested: boolean escapeKey: closedBy !== 'none', outsidePress: closedBy === 'any', }); - const role = useRole(floatingContext); + const role = useRole(floatingContext, { role: ariaRole }); const { getFloatingProps } = useInteractions([dismiss, role]); @@ -193,6 +204,8 @@ function DialogInner(props: DialogProps & { isNested: boolean store, modal, isNested, + isStacked: nesting.isStacked, + stackedChildCount: nesting.stackedChildCount, labelId, descriptionId, mounted, @@ -208,6 +221,8 @@ function DialogInner(props: DialogProps & { isNested: boolean store, modal, isNested, + nesting.isStacked, + nesting.stackedChildCount, labelId, descriptionId, mounted, @@ -219,7 +234,9 @@ function DialogInner(props: DialogProps & { isNested: boolean return ( - {content} + + {content} + ); } diff --git a/packages/headless/src/primitives/dialog/dialog.test.tsx b/packages/headless/src/primitives/dialog/dialog.test.tsx index d0513bbbe43..33633af2448 100644 --- a/packages/headless/src/primitives/dialog/dialog.test.tsx +++ b/packages/headless/src/primitives/dialog/dialog.test.tsx @@ -4,6 +4,7 @@ import React from 'react'; import { afterEach, describe, expect, it, vi } from 'vitest'; import { axe } from '../../test-utils/axe'; +import { Popover } from '../popover'; import { Dialog } from './index'; afterEach(() => cleanup()); @@ -671,6 +672,166 @@ describe('Dialog', () => { }); }); + describe('role', () => { + it('renders role=alertdialog when asked', () => { + renderDialog({ defaultOpen: true, role: 'alertdialog' }); + + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + }); + + describe('stacking', () => { + // Two dialogs, the inner one rendered inside the outer's popup. `outer`/`inner` prefixes on + // the test ids because both levels render the same parts. + function renderStack({ outerOpen = true, innerOpen = true }: { outerOpen?: boolean; innerOpen?: boolean } = {}) { + return render( + + + + + Outer + + + + + Inner + + + + + + , + ); + } + + it('marks the dialog on top stacked and the one beneath a stack base', () => { + renderStack(); + + expect(screen.getByTestId('inner-popup')).toHaveAttribute('data-stacked', ''); + expect(screen.getByTestId('inner-popup')).not.toHaveAttribute('data-stack-base'); + expect(screen.getByTestId('outer-popup')).toHaveAttribute('data-stack-base', ''); + expect(screen.getByTestId('outer-popup')).not.toHaveAttribute('data-stacked'); + }); + + it('marks the stacked backdrop, so only one scrim in the stack paints', () => { + renderStack(); + + expect(screen.getByTestId('inner-backdrop')).toHaveAttribute('data-stacked', ''); + expect(screen.getByTestId('outer-backdrop')).not.toHaveAttribute('data-stacked'); + }); + + it('drops the stack base marking when the dialog on top closes', () => { + const { rerender } = renderStack(); + expect(screen.getByTestId('outer-popup')).toHaveAttribute('data-stack-base', ''); + + rerender( + + + + Outer + + + + Inner + + + + + + , + ); + + expect(screen.getByTestId('outer-popup')).not.toHaveAttribute('data-stack-base'); + }); + + it('is not stacked on a dialog that is closed', () => { + // A confirmation root mounted beside its dialog's portal is inside the root but outlives + // the open state; on its own it owns the scrim like any root-level dialog. + render( + + + + + + Inner + + + + , + ); + + expect(screen.getByTestId('inner-popup')).not.toHaveAttribute('data-stacked'); + expect(screen.getByTestId('inner-backdrop')).not.toHaveAttribute('data-stacked'); + }); + + it('marks the middle of a three-deep stack as both', () => { + render( + + + + Bottom + + + + Middle + + + + Top + + + + + + + + + , + ); + + const middle = screen.getByTestId('middle-popup'); + expect(middle).toHaveAttribute('data-stacked', ''); + expect(middle).toHaveAttribute('data-stack-base', ''); + expect(screen.getByTestId('bottom-popup')).not.toHaveAttribute('data-stacked'); + expect(screen.getByTestId('top-popup')).not.toHaveAttribute('data-stack-base'); + }); + + it('does not treat a floating but non-dialog ancestor as a stack', async () => { + // The distinction `data-nested` cannot make: a dialog opened from a popover has a floating + // ancestor, but it sits on the bare page and still owns its scrim. + const user = userEvent.setup(); + render( + + Open popover + + + + + Open dialog + + + + Dialog in a popover + + + + + + + , + ); + + await user.click(screen.getByRole('button', { name: 'Open popover' })); + await user.click(screen.getByRole('button', { name: 'Open dialog' })); + + const popup = screen.getByTestId('dialog-popup'); + expect(popup).toHaveAttribute('data-nested', ''); + expect(popup).not.toHaveAttribute('data-stacked'); + expect(screen.getByTestId('dialog-backdrop')).not.toHaveAttribute('data-stacked'); + }); + }); + describe('accessibility (axe)', () => { it('has no violations when closed', async () => { const { container } = renderDialog(); diff --git a/packages/headless/src/primitives/dialog/index.ts b/packages/headless/src/primitives/dialog/index.ts index c8e9789136d..e51ebc56558 100644 --- a/packages/headless/src/primitives/dialog/index.ts +++ b/packages/headless/src/primitives/dialog/index.ts @@ -14,6 +14,7 @@ export type { DialogPopupProps, DialogPortalProps, DialogProps, + DialogRole, DialogTitleProps, DialogTriggerProps, DialogViewportProps, diff --git a/packages/headless/src/primitives/dialog/parts.ts b/packages/headless/src/primitives/dialog/parts.ts index 3c9534100e6..a2af980ca40 100644 --- a/packages/headless/src/primitives/dialog/parts.ts +++ b/packages/headless/src/primitives/dialog/parts.ts @@ -1,4 +1,10 @@ -export { type DialogClosedBy, type DialogOpenChangeDetails, type DialogProps, DialogRoot as Root } from './dialog-root'; +export { + type DialogClosedBy, + type DialogOpenChangeDetails, + type DialogProps, + type DialogRole, + DialogRoot as Root, +} from './dialog-root'; export { type DialogTriggerProps, DialogTrigger as Trigger } from './dialog-trigger'; export { createDialogHandle as createHandle, type DialogHandle } from './dialog-handle'; export { type DialogPortalProps, DialogPortal as Portal } from './dialog-portal'; From b2757276773d5b6d4df511c61fd737a53a0c70a9 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 21:51:59 -0600 Subject: [PATCH 02/18] fix(headless): keep Drawer's context off the dialog stacking members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DrawerContextValue` inherits from `DialogContextValue`, so adding `isStacked` and `stackedChildCount` there made every drawer root fail to satisfy its own context — `tsc --noEmit` was red for the whole package, and for swingset, which typechecks headless from source. They belong in the same `Omit` as `store`. A drawer already tracks its nesting as `nestedOpenCount`, and `isStacked` asks a question about DIALOGS that a drawer has nothing to answer with. --- packages/headless/src/primitives/drawer/drawer-context.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/headless/src/primitives/drawer/drawer-context.ts b/packages/headless/src/primitives/drawer/drawer-context.ts index 0cdd7357352..30cec1dfafc 100644 --- a/packages/headless/src/primitives/drawer/drawer-context.ts +++ b/packages/headless/src/primitives/drawer/drawer-context.ts @@ -30,7 +30,12 @@ export interface NestedDrawerCallbacks { // The dialog-only members are dropped: the drawer has no trigger registry (its detached // triggers go through `DrawerHandle`), and its triggers still wire through floating-ui's // reference props, which the dialog's no longer do. -export interface DrawerContextValue extends Omit { +// +// The stacking pair goes with them. A drawer already counts its own nesting as +// `nestedOpenCount` / `onNested`, which is a different question from the dialog's: `isStacked` +// asks whether a DIALOG sits above, and a drawer's stacked-child styling has nothing to read it +// from. Inheriting them would oblige every drawer root to publish two values no drawer part uses. +export interface DrawerContextValue extends Omit { getReferenceProps: UseInteractionsReturn['getReferenceProps']; backdropRef: React.RefObject; drag: DrawerDrag; From fa3075222805edc5672a44a0e2642f295bc3e693 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 13:32:16 -0600 Subject: [PATCH 03/18] fix(ui): address review feedback on #9427 Publish "still covering" rather than the raw `open` flag from `DialogNestingContext`, so a stacked child keeps `data-stacked` for the length of the parent's exit instead of painting a second scrim over the parent's fading one. Co-Authored-By: Claude Opus 5 (1M context) --- .../headless/src/primitives/dialog/README.md | 3 ++ .../src/primitives/dialog/dialog-nesting.ts | 17 +++++--- .../src/primitives/dialog/dialog-root.tsx | 6 ++- .../src/primitives/dialog/dialog.test.tsx | 43 +++++++++++++++++++ 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/packages/headless/src/primitives/dialog/README.md b/packages/headless/src/primitives/dialog/README.md index e160414777a..1ef84c34301 100644 --- a/packages/headless/src/primitives/dialog/README.md +++ b/packages/headless/src/primitives/dialog/README.md @@ -237,6 +237,9 @@ alert stack, the middle dialog is stacked on one surface while another is stacke backdrop instead of compositing a darker one per level. `data-stack-base` is for whatever the surface underneath does to signal depth. +`data-stacked` holds for as long as the dialog underneath is on screen, exit transition included — +otherwise the one on top would paint a second scrim over the fading original. + The headless parts are unstyled. Target a part with your own className (or `render` prop) and combine it with the `data-*` state attributes above. ## Important Notes diff --git a/packages/headless/src/primitives/dialog/dialog-nesting.ts b/packages/headless/src/primitives/dialog/dialog-nesting.ts index 2fff5e4fae4..4521a78549a 100644 --- a/packages/headless/src/primitives/dialog/dialog-nesting.ts +++ b/packages/headless/src/primitives/dialog/dialog-nesting.ts @@ -11,7 +11,12 @@ import { createContext, useCallback, useContext, useLayoutEffect, useMemo, useSt * floating ancestor but sits on the bare page, and must still paint its own scrim. */ export interface DialogNestingContextValue { - /** Whether the surrounding dialog is itself open. */ + /** + * Whether the surrounding dialog is still covering the page — open, or closed but still + * mounted for its exit transition. Not the raw `open` flag: a child that un-suppressed its + * backdrop the instant the parent started closing would paint a second scrim over the + * parent's still-fading one. + */ open: boolean; /** * Called by a dialog rendered inside this one, for as long as it is open. Returns the release. @@ -41,7 +46,7 @@ export interface DialogNesting { * Joins a dialog root to the stack it belongs to, in both directions: up, to report itself to * the dialog it renders inside, and down, to count the dialogs that render inside it. */ -export function useDialogNesting(open: boolean): DialogNesting { +export function useDialogNesting(open: boolean, mounted: boolean): DialogNesting { const parent = useContext(DialogNestingContext); const [stackedChildCount, setStackedChildCount] = useState(0); @@ -70,13 +75,15 @@ export function useDialogNesting(open: boolean): DialogNesting { return registerWithParent(); }, [open, registerWithParent]); + const covering = open || mounted; + const context = useMemo( - () => ({ open, registerStackedChild }), - [open, registerStackedChild], + () => ({ open: covering, registerStackedChild }), + [covering, registerStackedChild], ); return { - // A closed parent is not something to sit on top of: the child owns the scrim in that case, + // A parent that is closed AND gone is not something to sit on top of: the child owns the scrim, // which is what a confirmation root mounted beside its dialog's portal relies on. isStacked: parent !== null && parent.open, stackedChildCount, diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index dd32d1ae976..66bb2fb0f72 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -89,8 +89,6 @@ function DialogInner(props: DialogProps & { isNested: boolean const [activeTriggerId, setActiveTriggerId] = useControllableState(props.triggerId, null); const [activePayload, setActivePayload] = useState(undefined); - const nesting = useDialogNesting(open); - const labelId = useId(); const descriptionId = useId(); @@ -178,6 +176,10 @@ function DialogInner(props: DialogProps & { isNested: boolean ref: popupRef, }); + // Below `useTransition` because it needs `mounted`: what a stacked child has to key off is + // whether this dialog is still on screen, not whether it is still open. + const nesting = useDialogNesting(open, mounted); + const dismiss = useDismiss(floatingContext, { outsidePressEvent: 'mousedown', escapeKey: closedBy !== 'none', diff --git a/packages/headless/src/primitives/dialog/dialog.test.tsx b/packages/headless/src/primitives/dialog/dialog.test.tsx index 33633af2448..578e8ba03b1 100644 --- a/packages/headless/src/primitives/dialog/dialog.test.tsx +++ b/packages/headless/src/primitives/dialog/dialog.test.tsx @@ -745,6 +745,49 @@ describe('Dialog', () => { expect(screen.getByTestId('outer-popup')).not.toHaveAttribute('data-stack-base'); }); + it('stays stacked while the dialog beneath is exiting', () => { + // Keep an animation pending so the one beneath stays mounted for its exit instead of + // unmounting in the same commit. + const original = (Element.prototype as { getAnimations?: unknown }).getAnimations; + (Element.prototype as { getAnimations?: unknown }).getAnimations = () => [ + { finished: new Promise(() => {}) }, + ]; + try { + const { rerender } = renderStack(); + + // The one beneath closes first. Its backdrop is still on screen for the length of the + // exit, so the one on top has to keep suppressing its own scrim rather than paint a + // second one over the fading original. + rerender( + + + + + Outer + + + + + Inner + + + + + + , + ); + + expect(screen.getByTestId('outer-backdrop')).toBeInTheDocument(); + expect(screen.getByTestId('inner-backdrop')).toHaveAttribute('data-stacked', ''); + } finally { + if (original) { + (Element.prototype as { getAnimations?: unknown }).getAnimations = original; + } else { + delete (Element.prototype as { getAnimations?: unknown }).getAnimations; + } + } + }); + it('is not stacked on a dialog that is closed', () => { // A confirmation root mounted beside its dialog's portal is inside the root but outlives // the open state; on its own it owns the scrim like any root-level dialog. From 40d0f5ecfee5ce87eee8e6ef578029674039a114 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 18:30:48 -0600 Subject: [PATCH 04/18] feat(ui): stacked dialog motion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One backdrop for the whole stack. A stacked dialog paints none, and the root-level dialog's scrim survives underneath it. The scrim it replaces was solved so that two levels composited to an intended total, which held only for the two-deep case: alpha over alpha compounds, so the three-deep stack this exists for went from an intended 0.68 to 0.83. Depth comes from the surface beneath receding instead — scale and a lift, with the radius divided by the same factor so the corners render unchanged, the correction `ENTER_SCALE` already documents. Only `prompt` recedes; `panel` and `card` are root-level surfaces and leave the stacked prompt's shadow to carry the separation. A dialog stacked inside another one warns in development if it is any size but `prompt`, which covers panel-in-panel and card-in-panel without enumerating what may host what. The recede stays live on the phone band where the entrance scale is pinned flat: those are different gestures, and a stacked sheet covers enough of what is beneath it that dropping the recede would leave that level with no cue at all. Keyed on `data-stacked`, not `data-nested` — the latter reports any floating ancestor, so a dialog opened from a menu item would have lost its only scrim. --- .changeset/dialog-stack-motion.md | 5 ++ .../swingset/src/stories/dialog.component.mdx | 29 +++++++- .../src/stories/dialog.component.stories.tsx | 69 +++++++++++++++++- .../mosaic/components/dialog/dialog.styles.ts | 71 ++++++++++++++++--- .../mosaic/components/dialog/dialog.test.tsx | 69 ++++++++++++++++-- .../src/mosaic/components/dialog/dialog.tsx | 24 +++++++ 6 files changed, 247 insertions(+), 20 deletions(-) create mode 100644 .changeset/dialog-stack-motion.md diff --git a/.changeset/dialog-stack-motion.md b/.changeset/dialog-stack-motion.md new file mode 100644 index 00000000000..8acfaa74312 --- /dev/null +++ b/.changeset/dialog-stack-motion.md @@ -0,0 +1,5 @@ +--- +'@clerk/ui': patch +--- + +Mosaic `Dialog`s stacked on one another now share a single backdrop instead of each painting its own, so the page no longer darkens further with every level. The dialog beneath a stacked `prompt` recedes slightly to signal the layering. Only `prompt` dialogs are meant to stack; opening a `panel` or `card` inside another dialog now warns in development. diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index ebe7b975a50..b6fa8818201 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -251,9 +251,22 @@ which also shrinks the visual viewport — is excluded. ### Stacked dialogs -A dialog opened from inside another one carries `data-nested` and paints its own, lighter scrim, so -each level reads as a step further from the page without the backdrops compounding toward an -opaque wall. +Only a `prompt` stacks. `panel` and `card` are root-level surfaces — they host a stack, they are +never the thing stacked — and a dialog opened inside another one warns in development if it is any +other size. + +A stacked dialog carries `data-stacked` and paints **no** scrim: one backdrop serves the whole +stack, so how dark the page goes never depends on how deep the stack is. The dialog beneath carries +`data-stack-base`, and if it is a `prompt` it recedes — scaling down slightly and lifting, with its +radius divided by the same factor so the corners render unchanged. That recede and the stacked +surface's own shadow are the entire depth cue, so a stacked prompt over a `panel` (which never +recedes) leans on the shadow alone. + +`data-stacked` is narrower than the older `data-nested`, which reports any floating ancestor: a +dialog opened from a menu item is nested but not stacked, and still owns its scrim. + +Under `prefers-reduced-motion: reduce` nothing recedes — the surface beneath holds still and the +stacked one simply appears over it. --- @@ -342,6 +355,16 @@ prompt. storyModule={DialogStories} /> +A `panel` never recedes, so what separates the two levels here is the prompt's own shadow. + +Stack a prompt on a prompt and the one beneath moves instead — the shape a close confirmation +takes: + + + Nest by rendering a `Dialog` inside another one's children. Nothing else is required — the inner dialog finds the outer through Floating UI's tree and wires up its own stacking: diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index 2d5e857a58d..640fcd51b9e 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -154,7 +154,7 @@ function AddValueDialog({ ); } -/** A `panel` account surface with `card` dialogs opened from inside it. */ +/** A `panel` account surface with `prompt` dialogs opened from inside it. */ export function Nested() { return ( ({ when: SESSION_TIMES[index % SESSION_TIMES.length], })); +const editProfileTrigger = (props: RenderProps) => ; + +const discardTrigger = (props: RenderProps) => ( + +); + +/** + * A prompt stacked on a prompt — the shape a close confirmation takes. The second prompt paints + * no scrim of its own; the one beneath it recedes instead. + */ +export function StackedPrompts() { + return ( + + {({ close }) => ( + <> + + }>Update profile + }>Change the name people see on your account. + +
+ + {({ close: closeConfirmation }) => ( + <> + }>Discard changes? + }>Your edits will be lost. +
+ + +
+ + )} + + +
+ + )} + + ); +} + /** The panel clips rather than scrolling, so the scroll region is composed inside it. */ export function PanelSidebar() { return ( diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index 181e0dc29f4..1e65e792c69 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -10,18 +10,24 @@ export const styles = stylex.create({ // Black in both schemes. A grey veil was tried for dark mode — lightening a dark page rather // than darkening it — and it read as haze over the page rather than as a surface lifting off it. // - // A stacked dialog paints its OWN scrim rather than deferring to the one beneath it, so each - // level reads as a step further from the page. It is lighter than the base because the two - // COMPOSITE: alpha over alpha is `1 − (1 − a)(1 − b)`, so the nested value is solved for the - // intended total rather than picked by eye — `1 − 0.32/0.6 = 0.4667` lands two levels on 0.68. - // Exact for a two-deep stack, which is the shape that exists; a third level would go darker - // still, and wants its own value rather than a third application of this one. - // `data-nested` comes from the headless layer. + // ONE scrim for the whole stack: a stacked dialog paints none, and the root-level dialog's + // survives underneath it. The stack reads as depth through the surface beneath receding + // (`popupMotion.prompt`), not through the page going darker. + // + // The alternative — each level painting a lighter scrim solved so the composite lands on an + // intended total — was here first, and worked only for the two-deep case it was solved for. + // Alpha over alpha is `1 − (1 − a)(1 − b)`, so every level compounds: a third took the same + // three-deep stack this feature exists for from an intended 0.68 to 0.83, and "how dark is the + // page" became a function of stack depth. + // + // Keyed on `data-stacked`, NOT `data-nested`: the latter reports any floating ancestor, so a + // dialog opened from a menu item would drop the only scrim it has. Both come from the headless + // layer. backdrop: { inset: 0, backgroundColor: { default: 'color-mix(in oklab, oklch(0 0 0) 40%, transparent)', - ':where([data-nested])': 'color-mix(in oklab, oklch(0 0 0) 46.67%, transparent)', + ':where([data-stacked])': 'transparent', }, position: 'fixed', }, @@ -424,6 +430,23 @@ export const backdropMotion = stylex.create({ const SHEET_EXIT_EASE = 'ease-out'; const ENTER_SCALE = 0.94; + +// How far a prompt recedes while another prompt is stacked on it, and the radius that survives +// that scale — the same `r/s` correction `ENTER_SCALE` documents above, for the same reason. +// +// Shallower than the entrance scale on purpose: the entrance is a surface arriving from nowhere, +// while this is a surface that stays legible the whole time and only has to read as further back. +// The lift is what separates it from the entrance rather than the depth of the scale — a surface +// that only shrinks reads as being pushed away, one that shrinks and rises reads as being layered +// over, which is the relationship this actually is. +// +// A single step rather than a `--cl-stack-index` formula: the headless layer counts DIRECT +// children, so a third level would report the same 1 as the second and every level below the top +// would recede identically anyway. The formula and the cumulative count belong in the same change, +// whenever a stack deep enough to need them turns up. +const STACK_SCALE = 0.96; +const STACK_LIFT = '-0.5rem'; + const popupRadius = radiusVars['--cl-radius-xl']; export const popupMotion = stylex.create({ @@ -437,15 +460,22 @@ export const popupMotion = stylex.create({ prompt: { borderRadius: { default: popupRadius, + // The recede is the one scale that survives the phone band, so unlike the entrance its + // radius correction is NOT pinned flat there — see `transform` below. + ':where([data-stack-base])': `calc(${popupRadius} / ${STACK_SCALE})`, ':where([data-starting-style], [data-ending-style])': `calc(${popupRadius} / ${ENTER_SCALE})`, '@media (max-width: 47.99rem)': { default: popupRadius, + ':where([data-stack-base])': `calc(${popupRadius} / ${STACK_SCALE})`, ':where([data-starting-style], [data-ending-style])': popupRadius, }, - // Both branches resolve to the same value, so their order relative to each other cannot - // matter: there is no scale to counteract in either case. + // Both entrance branches resolve to the same value, so their order relative to each other + // cannot matter: there is no scale to counteract in either case. The stack branch is here + // for the same reason it is on `transform` — under `reduce` nothing scales, so there is + // nothing to correct. '@media (prefers-reduced-motion: reduce)': { default: popupRadius, + ':where([data-stack-base])': popupRadius, ':where([data-starting-style], [data-ending-style])': popupRadius, }, }, @@ -481,13 +511,34 @@ export const popupMotion = stylex.create({ */ transform: { default: 'scale(1)', + /** + * The recede: what a prompt does while another prompt is stacked on it. There is no second + * scrim, so this and the stacked surface's own shadow are the entire depth cue. + * + * Kept ON the phone band, where the entrance scale is pinned flat. Those are different + * gestures and the reasoning does not carry over: the entrance pin exists because stacking a + * shrink on top of a full-height slide makes the sheet arrive small and settle. A sheet + * receding under another sheet is the familiar one — it is what vaul does — and on a phone, + * where a stacked sheet covers most of what is beneath it, dropping the recede would leave + * the level below with no depth cue at all. + * + * `@stylexjs/sort-keys` puts this branch before the entrance one, so an exit that somehow + * begins while a child is still open renders the exit scale rather than the recede. Nothing + * ordinary reaches that state — floating-ui blocks the parent's own dismissal while a child + * is open — and the exit scale is the better of the two to see if anything ever does. + */ + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, ':where([data-starting-style], [data-ending-style])': `scale(${ENTER_SCALE})`, '@media (max-width: 47.99rem)': { default: 'scale(1)', + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, '@media (prefers-reduced-motion: reduce)': { default: 'scale(1)', + // Reduced motion drops the recede entirely rather than snapping to it: the level below + // holds still and the stacked surface simply appears over it. + ':where([data-stack-base])': 'scale(1)', ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, }, diff --git a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx index 9b7ff5eb24e..43464782b69 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx @@ -245,27 +245,84 @@ describe('stacked backdrops', () => { ); - it('marks only the inner backdrop as nested, so the scrims do not compound', async () => { - const user = userEvent.setup(); - render( + function renderStack() { + return render( + Account
Outer body
+ Add email address
Inner body
 , ); + } - expect(document.querySelector('.cl-dialog-backdrop')).not.toHaveAttribute('data-nested'); + it('marks only the stacked backdrop, so one scrim paints for the whole stack', async () => { + const user = userEvent.setup(); + renderStack(); + + expect(document.querySelector('.cl-dialog-backdrop')).not.toHaveAttribute('data-stacked'); await user.click(screen.getByRole('button', { name: 'Add email' })); const backdrops = document.querySelectorAll('.cl-dialog-backdrop'); - expect(backdrops[0]).not.toHaveAttribute('data-nested'); - expect(backdrops[1]).toHaveAttribute('data-nested', ''); + expect(backdrops[0]).not.toHaveAttribute('data-stacked'); + expect(backdrops[1]).toHaveAttribute('data-stacked', ''); + }); + + it('marks the popup beneath as the stack base, so it can recede', async () => { + const user = userEvent.setup(); + renderStack(); + + const outerPopup = document.querySelector('.cl-dialog-popup'); + expect(outerPopup).not.toHaveAttribute('data-stack-base'); + + await user.click(screen.getByRole('button', { name: 'Add email' })); + + const popups = document.querySelectorAll('.cl-dialog-popup'); + expect(popups[0]).toHaveAttribute('data-stack-base', ''); + expect(popups[1]).not.toHaveAttribute('data-stack-base'); + }); + + it('warns when a stacked dialog is not a prompt', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const user = userEvent.setup(); + render( + + Account +
Outer body
+ + Add email address +
Inner body
+ + , + ); + + await user.click(screen.getByRole('button', { name: 'Add email' })); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('size="card"')); + warn.mockRestore(); + }); + + it('does not warn for a stacked prompt, or for a root-level panel', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const user = userEvent.setup(); + renderStack(); + + await user.click(screen.getByRole('button', { name: 'Add email' })); + + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); }); }); diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index 14d08dabf45..af5f84c39cf 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -206,16 +206,40 @@ const Viewport = React.forwardRef(function ); }); +/** + * Warns when a dialog stacked on another one is not a `prompt`. + * + * Only `prompt` stacks. `panel` and `card` are root-level surfaces: they host a stack, and the + * styles that make one work — dropping the scrim, receding behind the surface above — exist for + * `prompt` alone, so a `panel` opened inside a dialog silently renders with neither. + * + * One rule stated on the child covers every case, without having to enumerate which sizes may + * host what. + */ +function useStackedSizeWarning(isStacked: boolean, size: DialogSize) { + React.useEffect(() => { + if (process.env.NODE_ENV === 'production' || !isStacked || size === 'prompt') { + return; + } + console.warn( + `Mosaic: a Dialog opened inside another Dialog should be size="prompt", but this one is size="${size}". ` + + 'Only prompts are styled to stack — this dialog will paint no backdrop and the surface beneath it will not recede.', + ); + }, [isStacked, size]); +} + /** The dialog surface: `role="dialog"`, focus-trapped, and the element that paints. */ const Popup = React.forwardRef(function DialogPopup( { className, style, ...rest }, ref, ) { const size = React.useContext(DialogSizeContext); + const { isStacked } = useDialogContext(); // Observed through state rather than a plain ref, because the warning has to re-run when the // node arrives and a ref mutation does not re-render. const [node, setNode] = React.useState(null); useAccessibleNameWarning(node, 'Dialog'); + useStackedSizeWarning(isStacked, size); const mergedRef = React.useCallback( (element: HTMLDivElement | null) => { From 5cead4ae6e5084466a4dbf0a7ca5482a3e0b05e9 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 20:29:56 -0600 Subject: [PATCH 05/18] feat(ui): dim the surface beneath a stacked dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recede alone reads as smaller more than as further back. Veiling the contents toward the surface's own background separates the layers on its own — which is what `panel` and `card` get, since neither moves. A veil rather than `opacity` on the popup: fading the popup fades the surface with it, letting the scrim through, which reads as the dialog dissolving rather than as depth. --- .../swingset/src/stories/dialog.component.mdx | 15 ++++--- .../mosaic/components/dialog/dialog.styles.ts | 43 +++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index b6fa8818201..f5682649a7d 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -257,16 +257,16 @@ other size. A stacked dialog carries `data-stacked` and paints **no** scrim: one backdrop serves the whole stack, so how dark the page goes never depends on how deep the stack is. The dialog beneath carries -`data-stack-base`, and if it is a `prompt` it recedes — scaling down slightly and lifting, with its -radius divided by the same factor so the corners render unchanged. That recede and the stacked -surface's own shadow are the entire depth cue, so a stacked prompt over a `panel` (which never -recedes) leans on the shadow alone. +`data-stack-base`, and its contents dim toward its own background — enough on its own to read as a +layer further back, which is what a `panel` gets, since a panel never moves. A `prompt` also +recedes, scaling down slightly and lifting, with its radius divided by the same factor so the +corners render unchanged. `data-stacked` is narrower than the older `data-nested`, which reports any floating ancestor: a dialog opened from a menu item is nested but not stacked, and still owns its scrim. -Under `prefers-reduced-motion: reduce` nothing recedes — the surface beneath holds still and the -stacked one simply appears over it. +Under `prefers-reduced-motion: reduce` nothing recedes — the surface beneath holds still, keeping +the dimming as its only cue. --- @@ -355,7 +355,8 @@ prompt. storyModule={DialogStories} /> -A `panel` never recedes, so what separates the two levels here is the prompt's own shadow. +A `panel` never recedes, so what separates the two levels here is the dimming and the prompt's own +shadow. Stack a prompt on a prompt and the one beneath moves instead — the shape a close confirmation takes: diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index 1e65e792c69..6ea99c32b23 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -2,6 +2,11 @@ import * as stylex from '@stylexjs/stylex'; import { colorVars, durationVars, easingVars, radiusVars, space } from '../../tokens.stylex'; +// How far the surface beneath a stacked prompt is veiled toward its own background. Declared up +// here because `styles.popup` needs it; it belongs with `STACK_SCALE` / `STACK_LIFT` further down, +// which drive the other half of the same effect. +const STACK_VEIL_OPACITY = 0.4; + export const styles = stylex.create({ // The scrim. A black wash over `transparent` rather than a percentage of a neutral // token: it composites over whatever the host app renders, so the same value reads @@ -87,6 +92,30 @@ export const styles = stylex.create({ // raw content rather than a `Card` and the surface has to come from somewhere. `sizes.card` // nulls the painting properties back out — see the note there. popup: { + /** + * The other half of the recede: while a prompt is stacked on this surface, its contents dim + * toward the surface's own background, so the layer beneath reads as further back rather than + * merely smaller. + * + * A veil rather than `opacity` on the popup, because those are different effects. Fading the + * popup fades the SURFACE — its background and its shadow — and the scrim shows through, which + * reads as the dialog dissolving. Painting the background colour back over the contents leaves + * the surface at full strength and dims only what sits on it. + * + * Driven by a private custom property rather than by a state branch on the pseudo-element: + * a `:where()` nested inside a `::after` block would describe the pseudo-element's own state, + * not the popup's. Setting the variable on the popup — where the state actually lives — and + * reading it here is the only shape that says what is meant. + * + * `zIndex` so it also covers `Dialog.CloseButton`, which is positioned and would otherwise + * paint over it and stay undimmed. Never interactive: the whole subtree is inert while a + * stacked dialog holds focus, and `pointer-events: none` keeps it that way regardless. + * + * Kept under `prefers-reduced-motion: reduce`, where the recede is dropped. A cross-fade is + * not the kind of motion that setting is about, and without it that mode would have no depth + * cue at all. + */ + '--_cl-stack-veil': { default: 0, ':where([data-stack-base])': STACK_VEIL_OPACITY }, padding: space['6'], // Forced-colors mode discards `box-shadow` outright, and the ring above is the only thing // separating the surface from the page — so in HCM the dialog would float edgeless over its @@ -130,6 +159,20 @@ export const styles = stylex.create({ // The containing block for `Dialog.CloseButton`. position: 'relative', width: '100%', + '::after': { + inset: 0, + // Follows the popup's own radius, counter-scale included. + borderRadius: 'inherit', + backgroundColor: colorVars['--cl-color-card'], + content: '""', + opacity: 'var(--_cl-stack-veil)', + pointerEvents: 'none', + position: 'absolute', + transitionDuration: durationVars['--cl-duration-base'], + transitionProperty: 'opacity', + transitionTimingFunction: easingVars['--cl-ease-enter'], + zIndex: 1, + }, }, /** From e598c210116a872ac4cbc02d6b62a20a1fc7cabb Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 20:49:48 -0600 Subject: [PATCH 06/18] feat(ui): separate a stack from a nested dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dialog over a `panel` or a `card` keeps the scrim it had — that is a new surface over a page-like one, and its own scrim is what says so. Only successive prompts drop it, where a second scrim would darken the page for what is the same conversation one step further in. Whether it is a stack turns on the size of the dialog BENEATH, which the headless layer has no notion of, so the backdrop resolves it from the parent size carried in context and applies the style rather than keying on `data-stacked`. The veil moves to `sizes.prompt` for the same reason. Reduced motion keeps the recede and drops only its duration. Removing it outright left a stacked prompt sitting on an identical prompt with no scrim between them, which reads as a rendering fault rather than as a preference being honoured. Dismissing a stack in one action now staggers the exits off `data-stack-exiting`, so it unwinds rather than vanishing on a single frame. --- .changeset/dialog-stack-motion.md | 2 +- .../swingset/src/stories/dialog.component.mdx | 39 ++++---- .../mosaic/components/dialog/dialog.styles.ts | 94 ++++++++++++++----- .../mosaic/components/dialog/dialog.test.tsx | 37 ++++++-- .../src/mosaic/components/dialog/dialog.tsx | 58 ++++++++---- 5 files changed, 164 insertions(+), 66 deletions(-) diff --git a/.changeset/dialog-stack-motion.md b/.changeset/dialog-stack-motion.md index 8acfaa74312..b5ca2129e76 100644 --- a/.changeset/dialog-stack-motion.md +++ b/.changeset/dialog-stack-motion.md @@ -2,4 +2,4 @@ '@clerk/ui': patch --- -Mosaic `Dialog`s stacked on one another now share a single backdrop instead of each painting its own, so the page no longer darkens further with every level. The dialog beneath a stacked `prompt` recedes slightly to signal the layering. Only `prompt` dialogs are meant to stack; opening a `panel` or `card` inside another dialog now warns in development. +Mosaic `Dialog` now distinguishes a stack — successive `prompt` dialogs, such as a confirmation over the form it is confirming — from a dialog opened over a `panel` or `card`. A stacked prompt paints no backdrop of its own, so the page no longer darkens further with every level; the prompt beneath it dims and recedes instead, and holds briefly when a stack is dismissed all at once so the exits are staggered. Dialogs opened over a `panel` or `card` are unchanged. Opening a `panel` or `card` inside another dialog now warns in development. diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index f5682649a7d..cab87fe10cc 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -249,24 +249,29 @@ difference and adds it to its own bottom padding, which gives each size the righ A card taller than the remaining space aligns to its top rather than losing its head. Pinch-zoom — which also shrinks the visual viewport — is excluded. -### Stacked dialogs +### Nested dialogs and stacks -Only a `prompt` stacks. `panel` and `card` are root-level surfaces — they host a stack, they are -never the thing stacked — and a dialog opened inside another one warns in development if it is any -other size. +Two different relationships, which look different on purpose. -A stacked dialog carries `data-stacked` and paints **no** scrim: one backdrop serves the whole -stack, so how dark the page goes never depends on how deep the stack is. The dialog beneath carries -`data-stack-base`, and its contents dim toward its own background — enough on its own to read as a -layer further back, which is what a `panel` gets, since a panel never moves. A `prompt` also -recedes, scaling down slightly and lifting, with its radius divided by the same factor so the -corners render unchanged. +A **nested** dialog is one opened over a `panel` or a `card` — a new surface over a page-like one. +It paints its own scrim, lighter than the base so the two composite to the intended darkness rather +than doubling it. Nothing else changes. -`data-stacked` is narrower than the older `data-nested`, which reports any floating ancestor: a -dialog opened from a menu item is nested but not stacked, and still owns its scrim. +A **stack** is successive `prompt`s: the confirmation over the form it is confirming. The same +conversation, one step further in. A stacked prompt paints **no** scrim — one backdrop serves the +whole stack, so how dark the page goes never depends on how deep the stack is. Depth comes from the +prompt beneath instead: its contents dim toward its own background, and it recedes, scaling down +slightly and lifting, with its radius divided by the same factor so the corners render unchanged. -Under `prefers-reduced-motion: reduce` nothing recedes — the surface beneath holds still, keeping -the dimming as its only cue. +Whichever it is, the thing that opens is always a `prompt`. `panel` and `card` are root-level +surfaces — they host, they are never hosted — and a dialog opened inside another one warns in +development if it is any other size. + +Dismissing a stack in one action, as "discard" does, staggers the exits: the surface beneath holds +briefly so the stack unwinds rather than vanishing on one frame. + +Under `prefers-reduced-motion: reduce` the recede still happens, it just arrives in a single frame +with nothing interpolating — the setting asks for no animation, not for no distinction. --- @@ -355,10 +360,10 @@ prompt. storyModule={DialogStories} /> -A `panel` never recedes, so what separates the two levels here is the dimming and the prompt's own -shadow. +This is the nested case, not a stack: the prompt paints its own scrim over the panel, and the panel +neither dims nor recedes. -Stack a prompt on a prompt and the one beneath moves instead — the shape a close confirmation +Stack a prompt on a prompt and the relationship changes — the shape a close confirmation takes: prompt -> alert` this exists for would land on 0.83 against the 0.68 the + * nested value above was solved for. The stack reads through the surface beneath receding and + * dimming instead. + * + * Applied by `Dialog.Backdrop` rather than keyed on `data-stacked`, because whether this is a + * stack depends on the size of the dialog beneath — which the headless layer has no notion of. + * It rides in the same `stylex.props` call as `backdrop`, so this `backgroundColor` replaces + * that one outright rather than the two both emitting. + */ + backdropStacked: { + backgroundColor: 'transparent', + }, + // Centering track inside the headless `FloatingOverlay`, which owns the fixed positioning and // the scroll lock. Whether this box is a fixed height or grows with its content is the whole // outside-scroll question, and it differs per size — see `viewportSizes` below. @@ -111,11 +124,10 @@ export const styles = stylex.create({ * paint over it and stay undimmed. Never interactive: the whole subtree is inert while a * stacked dialog holds focus, and `pointer-events: none` keeps it that way regardless. * - * Kept under `prefers-reduced-motion: reduce`, where the recede is dropped. A cross-fade is - * not the kind of motion that setting is about, and without it that mode would have no depth - * cue at all. + * The variable itself is set per size — only `prompt` sets it, in `sizes` below — so this + * reads `0` on a `panel` or a `card`, which have a scrim of their own to separate them from + * what they host and would double up. */ - '--_cl-stack-veil': { default: 0, ':where([data-stack-base])': STACK_VEIL_OPACITY }, padding: space['6'], // Forced-colors mode discards `box-shadow` outright, and the ring above is the only thing // separating the surface from the page — so in HCM the dialog would float edgeless over its @@ -165,7 +177,7 @@ export const styles = stylex.create({ borderRadius: 'inherit', backgroundColor: colorVars['--cl-color-card'], content: '""', - opacity: 'var(--_cl-stack-veil)', + opacity: 'var(--_cl-stack-veil, 0)', pointerEvents: 'none', position: 'absolute', transitionDuration: durationVars['--cl-duration-base'], @@ -286,6 +298,10 @@ export const viewportSizes = stylex.create({ export const sizes = stylex.create({ prompt: { + // Read by the veil on `styles.popup`. Set here rather than there so it applies to `prompt` + // alone: a `panel` or a `card` hosting a dialog gets a scrim between the two instead, and + // would otherwise dim as well as darken. + '--_cl-stack-veil': { default: 0, ':where([data-stack-base])': STACK_VEIL_OPACITY }, // Tighter than the popup's default 1.5rem. A prompt asks one thing, so its content box is // small and a 1.5rem surround reads as a disproportionate frame around two lines of text. // Overrides `styles.popup` by position — `sizes[size]` is spread after it in the same @@ -490,6 +506,15 @@ const ENTER_SCALE = 0.94; const STACK_SCALE = 0.96; const STACK_LIFT = '-0.5rem'; +// How long a surface holds before starting its own exit while a dialog stacked on it is still +// leaving. Without it a stack dismissed in one action — "discard", which closes the confirmation +// and the form behind it together — leaves on a single frame, and the two surfaces read as one +// thing vanishing rather than as a stack unwinding. +// +// Half the child's exit rather than all of it: the two overlapping is the point. Fully sequenced, +// the dismissal takes twice as long and starts to feel like waiting. +const STACK_EXIT_STAGGER = `calc(${durationVars['--cl-duration-fast']} / 2)`; + const popupRadius = radiusVars['--cl-radius-xl']; export const popupMotion = stylex.create({ @@ -513,12 +538,12 @@ export const popupMotion = stylex.create({ ':where([data-starting-style], [data-ending-style])': popupRadius, }, // Both entrance branches resolve to the same value, so their order relative to each other - // cannot matter: there is no scale to counteract in either case. The stack branch is here - // for the same reason it is on `transform` — under `reduce` nothing scales, so there is - // nothing to correct. + // cannot matter: there is no scale to counteract in either case. The recede is the + // exception — it still applies under `reduce`, just without a duration — so its correction + // has to come with it. '@media (prefers-reduced-motion: reduce)': { default: popupRadius, - ':where([data-stack-base])': popupRadius, + ':where([data-stack-base])': `calc(${popupRadius} / ${STACK_SCALE})`, ':where([data-starting-style], [data-ending-style])': popupRadius, }, }, @@ -577,14 +602,33 @@ export const popupMotion = stylex.create({ ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, + // The recede is NOT dropped here, unlike the entrance scale. `reduce` asks for no + // ANIMATION, not for no distinction: `transitionProperty` below narrows to `opacity` in + // this mode, so the recede lands in one frame with nothing interpolating. Dropping it + // outright leaves a stacked prompt sitting on an identical prompt with no scrim between + // them, which reads as a rendering fault rather than as a preference being honoured. '@media (prefers-reduced-motion: reduce)': { default: 'scale(1)', - // Reduced motion drops the recede entirely rather than snapping to it: the level below - // holds still and the stacked surface simply appears over it. - ':where([data-stack-base])': 'scale(1)', + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, }, + /** + * Holds this surface's exit while a dialog stacked on it is still leaving, so a stack + * dismissed in one action unwinds instead of vanishing at once. + * + * Applies to every slot of `transitionProperty` rather than to the transform alone: what is + * being delayed is the whole departure. Under `reduce` that list narrows to `opacity`, so the + * stagger survives as a staggered fade. + * + * `data-stack-exiting` and not `data-stack-base`, and the difference is the whole reason the + * headless layer reports both: `data-stack-base` releases the moment its child closes, which + * is a frame before there is anything to stagger against. + */ + transitionDelay: { + default: null, + ':where([data-stack-exiting][data-ending-style])': STACK_EXIT_STAGGER, + }, // The sheet travels its OWN HEIGHT rather than the ~11px a scale does, so it runs longer than // anything else here: `slow` in, `base` out, a 1.67:1 ratio in line with the rest of Mosaic. // The dead-frame concern that caps long durations elsewhere does not apply — the delta is diff --git a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx index 43464782b69..8ff9af9dedb 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.test.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.test.tsx @@ -261,17 +261,40 @@ describe('stacked backdrops', () => { ); } - it('marks only the stacked backdrop, so one scrim paints for the whole stack', async () => { + // The backdrop's two cases differ by a style rather than by an attribute, so the assertion is + // that the same tree with only the hosting size changed produces different classes. Comparing + // rather than matching a class: StyleX names are content hashes and would pin the value. + async function innerBackdropClass(hostSize: DialogSize) { const user = userEvent.setup(); - renderStack(); + render( + + Host + + Add email address + + , + ); + await user.click(screen.getByRole('button', { name: 'Add email' })); + const className = document.querySelectorAll('.cl-dialog-backdrop')[1].className; + cleanup(); + return className; + } - expect(document.querySelector('.cl-dialog-backdrop')).not.toHaveAttribute('data-stacked'); + it('drops the scrim for a prompt over a prompt, and keeps it for one over a panel', async () => { + const overPrompt = await innerBackdropClass('prompt'); + const overPanel = await innerBackdropClass('panel'); - await user.click(screen.getByRole('button', { name: 'Add email' })); + expect(overPrompt).not.toBe(overPanel); + }); - const backdrops = document.querySelectorAll('.cl-dialog-backdrop'); - expect(backdrops[0]).not.toHaveAttribute('data-stacked'); - expect(backdrops[1]).toHaveAttribute('data-stacked', ''); + it('keeps a prompt over a card on the nested scrim, same as over a panel', async () => { + const overCard = await innerBackdropClass('card'); + const overPanel = await innerBackdropClass('panel'); + + expect(overCard).toBe(overPanel); }); it('marks the popup beneath as the stack base, so it can recede', async () => { diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index af5f84c39cf..c539aae84e4 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -29,6 +29,24 @@ export interface DialogRootProps extends HeadlessDialogProps< */ const DialogSizeContext = React.createContext('prompt'); +/** + * The size of the dialog this one was opened from, which is what decides whether the two form a + * STACK — successive prompts — or a nested dialog over a `panel` or `card`. The two want opposite + * backdrops, so the distinction has to be reachable from the parts. + * + * Read from `DialogSizeContext` before a root overwrites it with its own size. Meaningless on its + * own, since a root-level dialog reads the context default: pair it with the headless `isStacked`, + * which is what reports that there is a dialog above at all. + */ +const DialogParentSizeContext = React.createContext('prompt'); + +/** Whether this dialog is a prompt stacked on a prompt — see {@link DialogParentSizeContext}. */ +function useIsStacked() { + const { isStacked } = useDialogContext(); + const parentSize = React.useContext(DialogParentSizeContext); + return isStacked && parentSize === 'prompt'; +} + /** * The headless parts type their props (and the `render` callback's argument) against * the raw tag props, which carry the non-standard HTML `color` attribute typed @@ -74,10 +92,13 @@ export type DialogPopupProps = MosaicComponentProps<'div'> & { /** Owns the open state and the size both the backdrop and the popup read. */ function Root({ size = 'prompt', children, ...rest }: DialogRootProps) { + const parentSize = React.useContext(DialogSizeContext); return ( - - {...rest}>{children} - + + + {...rest}>{children} + + ); } @@ -167,12 +188,15 @@ const Backdrop = React.forwardRef(function ref, ) { const size = React.useContext(DialogSizeContext); + const isStacked = useIsStacked(); return ( (function }); /** - * Warns when a dialog stacked on another one is not a `prompt`. + * Warns when a dialog opened inside another dialog is not a `prompt`. * - * Only `prompt` stacks. `panel` and `card` are root-level surfaces: they host a stack, and the - * styles that make one work — dropping the scrim, receding behind the surface above — exist for - * `prompt` alone, so a `panel` opened inside a dialog silently renders with neither. + * `panel` and `card` are root-level surfaces: they host what opens over them and are never the + * thing that opens. A `panel` inside a dialog renders at a size that assumes it owns the viewport, + * over a surface it was meant to replace. * - * One rule stated on the child covers every case, without having to enumerate which sizes may - * host what. + * One rule stated on the child covers every case — panel-in-panel, card-in-panel — without having + * to enumerate which sizes may host what. */ -function useStackedSizeWarning(isStacked: boolean, size: DialogSize) { +function useNestedSizeWarning(isNestedInDialog: boolean, size: DialogSize) { React.useEffect(() => { - if (process.env.NODE_ENV === 'production' || !isStacked || size === 'prompt') { + if (process.env.NODE_ENV === 'production' || !isNestedInDialog || size === 'prompt') { return; } console.warn( `Mosaic: a Dialog opened inside another Dialog should be size="prompt", but this one is size="${size}". ` + - 'Only prompts are styled to stack — this dialog will paint no backdrop and the surface beneath it will not recede.', + 'Only prompts are meant to open over another dialog; the rest are root-level surfaces.', ); - }, [isStacked, size]); + }, [isNestedInDialog, size]); } /** The dialog surface: `role="dialog"`, focus-trapped, and the element that paints. */ @@ -234,12 +258,14 @@ const Popup = React.forwardRef(function Dialog ref, ) { const size = React.useContext(DialogSizeContext); - const { isStacked } = useDialogContext(); + // The headless flag, not `useIsStacked` — the rule is about opening a dialog inside ANY dialog, + // which is broader than the prompt-on-prompt case the stacking styles cover. + const { isStacked: isNestedInDialog } = useDialogContext(); // Observed through state rather than a plain ref, because the warning has to re-run when the // node arrives and a ref mutation does not re-render. const [node, setNode] = React.useState(null); useAccessibleNameWarning(node, 'Dialog'); - useStackedSizeWarning(isStacked, size); + useNestedSizeWarning(isNestedInDialog, size); const mergedRef = React.useCallback( (element: HTMLDivElement | null) => { From 8b3fbf41653201c89e8507bdad6f46d89811b698 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 20:58:27 -0600 Subject: [PATCH 07/18] feat(ui): hold the recede through a stacked dialog's exit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stagger was invisible, and mostly not because it was short. The veil and the lift were keyed on `data-stack-base`, which releases the instant the child closes — so the surface beneath un-dimmed and dropped forward WHILE fading out, moving toward the viewer at the moment it should have been receding away. Both now hold through `data-stack-exiting` too, and the lift rides a custom property so the exit branch carries it without having to know whether there was ever a stacked child. The delay goes to a full `fast`. Half of it was 50ms against a 100ms exit — about a frame and a half, which is nothing. --- .../mosaic/components/dialog/dialog.styles.ts | 53 ++++++++++++------- 1 file changed, 33 insertions(+), 20 deletions(-) diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index c54e3a57c12..bebc27e5144 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -2,10 +2,17 @@ import * as stylex from '@stylexjs/stylex'; import { colorVars, durationVars, easingVars, radiusVars, space } from '../../tokens.stylex'; -// How far the surface beneath a stacked prompt is veiled toward its own background. Declared up -// here because `styles.popup` needs it; it belongs with `STACK_SCALE` / `STACK_LIFT` further down, -// which drive the other half of the same effect. +// The stack state a surface beneath a stacked prompt takes on: how far its contents are veiled +// toward its own background, and how far it lifts. Declared up here rather than beside +// `STACK_SCALE` further down because `styles` and `sizes` read them, and StyleX requires a +// referenced constant to be declared before the `create()` call that reads it. const STACK_VEIL_OPACITY = 0.4; +const STACK_LIFT = '-0.5rem'; + +// The lift rides a custom property rather than being written into each `transform` value, because +// the exit branch has to carry it too and cannot know whether there was ever a stacked child. The +// variable is set per state in `sizes.prompt`; every `transform` below just composes it. +const STACK_TRANSLATE = 'translateY(var(--_cl-stack-lift, 0rem))'; export const styles = stylex.create({ // The scrim. A black wash over `transparent` rather than a percentage of a neutral @@ -301,7 +308,13 @@ export const sizes = stylex.create({ // Read by the veil on `styles.popup`. Set here rather than there so it applies to `prompt` // alone: a `panel` or a `card` hosting a dialog gets a scrim between the two instead, and // would otherwise dim as well as darken. - '--_cl-stack-veil': { default: 0, ':where([data-stack-base])': STACK_VEIL_OPACITY }, + // Both held through `data-stack-exiting` as well as `data-stack-base`, and that is the whole + // reason the stagger is visible. The base releases the instant its child closes, so keyed on + // it alone the surface beneath un-dims and drops forward WHILE it is fading out — moving + // toward the viewer at the moment it should be receding away, which reads as a glitch and + // buries any stagger under it. + '--_cl-stack-lift': { default: '0rem', ':where([data-stack-base], [data-stack-exiting])': STACK_LIFT }, + '--_cl-stack-veil': { default: 0, ':where([data-stack-base], [data-stack-exiting])': STACK_VEIL_OPACITY }, // Tighter than the popup's default 1.5rem. A prompt asks one thing, so its content box is // small and a 1.5rem surround reads as a disproportionate frame around two lines of text. // Overrides `styles.popup` by position — `sizes[size]` is spread after it in the same @@ -495,25 +508,25 @@ const ENTER_SCALE = 0.94; // // Shallower than the entrance scale on purpose: the entrance is a surface arriving from nowhere, // while this is a surface that stays legible the whole time and only has to read as further back. -// The lift is what separates it from the entrance rather than the depth of the scale — a surface -// that only shrinks reads as being pushed away, one that shrinks and rises reads as being layered -// over, which is the relationship this actually is. +// The lift (`STACK_LIFT`, at the top of this file) is what separates it from the entrance rather +// than the depth of the scale — a surface that only shrinks reads as being pushed away, one that +// shrinks and rises reads as being layered over, which is the relationship this actually is. // // A single step rather than a `--cl-stack-index` formula: the headless layer counts DIRECT // children, so a third level would report the same 1 as the second and every level below the top // would recede identically anyway. The formula and the cumulative count belong in the same change, // whenever a stack deep enough to need them turns up. const STACK_SCALE = 0.96; -const STACK_LIFT = '-0.5rem'; // How long a surface holds before starting its own exit while a dialog stacked on it is still // leaving. Without it a stack dismissed in one action — "discard", which closes the confirmation // and the form behind it together — leaves on a single frame, and the two surfaces read as one // thing vanishing rather than as a stack unwinding. // -// Half the child's exit rather than all of it: the two overlapping is the point. Fully sequenced, -// the dismissal takes twice as long and starts to feel like waiting. -const STACK_EXIT_STAGGER = `calc(${durationVars['--cl-duration-fast']} / 2)`; +// A full `fast` — the same length as the child's exit — so the surface beneath starts leaving as +// the one above finishes. Half of it was tried first and `fast` is only 0.1s, so the separation +// came to about a frame and a half and read as nothing at all. +const STACK_EXIT_STAGGER = durationVars['--cl-duration-fast']; const popupRadius = radiusVars['--cl-radius-xl']; @@ -578,7 +591,7 @@ export const popupMotion = stylex.create({ * free to reorder them. */ transform: { - default: 'scale(1)', + default: `scale(1) ${STACK_TRANSLATE}`, /** * The recede: what a prompt does while another prompt is stacked on it. There is no second * scrim, so this and the stacked surface's own shadow are the entire depth cue. @@ -595,12 +608,12 @@ export const popupMotion = stylex.create({ * ordinary reaches that state — floating-ui blocks the parent's own dismissal while a child * is open — and the exit scale is the better of the two to see if anything ever does. */ - ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, - ':where([data-starting-style], [data-ending-style])': `scale(${ENTER_SCALE})`, + ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, + ':where([data-starting-style], [data-ending-style])': `scale(${ENTER_SCALE}) ${STACK_TRANSLATE}`, '@media (max-width: 47.99rem)': { - default: 'scale(1)', - ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, - ':where([data-starting-style], [data-ending-style])': 'scale(1)', + default: `scale(1) ${STACK_TRANSLATE}`, + ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, + ':where([data-starting-style], [data-ending-style])': `scale(1) ${STACK_TRANSLATE}`, }, // The recede is NOT dropped here, unlike the entrance scale. `reduce` asks for no // ANIMATION, not for no distinction: `transitionProperty` below narrows to `opacity` in @@ -608,9 +621,9 @@ export const popupMotion = stylex.create({ // outright leaves a stacked prompt sitting on an identical prompt with no scrim between // them, which reads as a rendering fault rather than as a preference being honoured. '@media (prefers-reduced-motion: reduce)': { - default: 'scale(1)', - ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, - ':where([data-starting-style], [data-ending-style])': 'scale(1)', + default: `scale(1) ${STACK_TRANSLATE}`, + ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, + ':where([data-starting-style], [data-ending-style])': `scale(1) ${STACK_TRANSLATE}`, }, }, /** From 7bd0aa2b43b505bbd3d2d705b128640de0ecd7e0 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 21:03:13 -0600 Subject: [PATCH 08/18] feat(ui): drop the stacked-dialog exit stagger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A prompt's exit runs 0.1s, which leaves no room to separate two of them, and lengthening a dismissal to make the separation visible is the wrong trade — the dismissal is the part that should feel immediate. Takes the veil and lift back to `data-stack-base` alone. They briefly held through the exit as well, to stop the surface beneath un-dimming as it faded, but that too is below the threshold over 0.1s. --- .changeset/dialog-stack-motion.md | 2 +- .../swingset/src/stories/dialog.component.mdx | 3 - .../mosaic/components/dialog/dialog.styles.ts | 66 ++++--------------- 3 files changed, 15 insertions(+), 56 deletions(-) diff --git a/.changeset/dialog-stack-motion.md b/.changeset/dialog-stack-motion.md index b5ca2129e76..60a5b6ddef7 100644 --- a/.changeset/dialog-stack-motion.md +++ b/.changeset/dialog-stack-motion.md @@ -2,4 +2,4 @@ '@clerk/ui': patch --- -Mosaic `Dialog` now distinguishes a stack — successive `prompt` dialogs, such as a confirmation over the form it is confirming — from a dialog opened over a `panel` or `card`. A stacked prompt paints no backdrop of its own, so the page no longer darkens further with every level; the prompt beneath it dims and recedes instead, and holds briefly when a stack is dismissed all at once so the exits are staggered. Dialogs opened over a `panel` or `card` are unchanged. Opening a `panel` or `card` inside another dialog now warns in development. +Mosaic `Dialog` now distinguishes a stack — successive `prompt` dialogs, such as a confirmation over the form it is confirming — from a dialog opened over a `panel` or `card`. A stacked prompt paints no backdrop of its own, so the page no longer darkens further with every level; the prompt beneath it dims and recedes instead. Dialogs opened over a `panel` or `card` are unchanged. Opening a `panel` or `card` inside another dialog now warns in development. diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index cab87fe10cc..19a5793bf28 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -267,9 +267,6 @@ Whichever it is, the thing that opens is always a `prompt`. `panel` and `card` a surfaces — they host, they are never hosted — and a dialog opened inside another one warns in development if it is any other size. -Dismissing a stack in one action, as "discard" does, staggers the exits: the surface beneath holds -briefly so the stack unwinds rather than vanishing on one frame. - Under `prefers-reduced-motion: reduce` the recede still happens, it just arrives in a single frame with nothing interpolating — the setting asks for no animation, not for no distinction. diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index bebc27e5144..816839c9213 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -2,17 +2,10 @@ import * as stylex from '@stylexjs/stylex'; import { colorVars, durationVars, easingVars, radiusVars, space } from '../../tokens.stylex'; -// The stack state a surface beneath a stacked prompt takes on: how far its contents are veiled -// toward its own background, and how far it lifts. Declared up here rather than beside -// `STACK_SCALE` further down because `styles` and `sizes` read them, and StyleX requires a -// referenced constant to be declared before the `create()` call that reads it. +// How far the contents of a surface beneath a stacked prompt are veiled toward its own background. +// Declared up here rather than beside `STACK_SCALE` further down because `sizes` reads it, and +// StyleX requires a referenced constant to be declared before the `create()` call that reads it. const STACK_VEIL_OPACITY = 0.4; -const STACK_LIFT = '-0.5rem'; - -// The lift rides a custom property rather than being written into each `transform` value, because -// the exit branch has to carry it too and cannot know whether there was ever a stacked child. The -// variable is set per state in `sizes.prompt`; every `transform` below just composes it. -const STACK_TRANSLATE = 'translateY(var(--_cl-stack-lift, 0rem))'; export const styles = stylex.create({ // The scrim. A black wash over `transparent` rather than a percentage of a neutral @@ -308,13 +301,7 @@ export const sizes = stylex.create({ // Read by the veil on `styles.popup`. Set here rather than there so it applies to `prompt` // alone: a `panel` or a `card` hosting a dialog gets a scrim between the two instead, and // would otherwise dim as well as darken. - // Both held through `data-stack-exiting` as well as `data-stack-base`, and that is the whole - // reason the stagger is visible. The base releases the instant its child closes, so keyed on - // it alone the surface beneath un-dims and drops forward WHILE it is fading out — moving - // toward the viewer at the moment it should be receding away, which reads as a glitch and - // buries any stagger under it. - '--_cl-stack-lift': { default: '0rem', ':where([data-stack-base], [data-stack-exiting])': STACK_LIFT }, - '--_cl-stack-veil': { default: 0, ':where([data-stack-base], [data-stack-exiting])': STACK_VEIL_OPACITY }, + '--_cl-stack-veil': { default: 0, ':where([data-stack-base])': STACK_VEIL_OPACITY }, // Tighter than the popup's default 1.5rem. A prompt asks one thing, so its content box is // small and a 1.5rem surround reads as a disproportionate frame around two lines of text. // Overrides `styles.popup` by position — `sizes[size]` is spread after it in the same @@ -517,16 +504,7 @@ const ENTER_SCALE = 0.94; // would recede identically anyway. The formula and the cumulative count belong in the same change, // whenever a stack deep enough to need them turns up. const STACK_SCALE = 0.96; - -// How long a surface holds before starting its own exit while a dialog stacked on it is still -// leaving. Without it a stack dismissed in one action — "discard", which closes the confirmation -// and the form behind it together — leaves on a single frame, and the two surfaces read as one -// thing vanishing rather than as a stack unwinding. -// -// A full `fast` — the same length as the child's exit — so the surface beneath starts leaving as -// the one above finishes. Half of it was tried first and `fast` is only 0.1s, so the separation -// came to about a frame and a half and read as nothing at all. -const STACK_EXIT_STAGGER = durationVars['--cl-duration-fast']; +const STACK_LIFT = '-0.5rem'; const popupRadius = radiusVars['--cl-radius-xl']; @@ -591,7 +569,7 @@ export const popupMotion = stylex.create({ * free to reorder them. */ transform: { - default: `scale(1) ${STACK_TRANSLATE}`, + default: 'scale(1)', /** * The recede: what a prompt does while another prompt is stacked on it. There is no second * scrim, so this and the stacked surface's own shadow are the entire depth cue. @@ -608,12 +586,12 @@ export const popupMotion = stylex.create({ * ordinary reaches that state — floating-ui blocks the parent's own dismissal while a child * is open — and the exit scale is the better of the two to see if anything ever does. */ - ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, - ':where([data-starting-style], [data-ending-style])': `scale(${ENTER_SCALE}) ${STACK_TRANSLATE}`, + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, + ':where([data-starting-style], [data-ending-style])': `scale(${ENTER_SCALE})`, '@media (max-width: 47.99rem)': { - default: `scale(1) ${STACK_TRANSLATE}`, - ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, - ':where([data-starting-style], [data-ending-style])': `scale(1) ${STACK_TRANSLATE}`, + default: 'scale(1)', + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, + ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, // The recede is NOT dropped here, unlike the entrance scale. `reduce` asks for no // ANIMATION, not for no distinction: `transitionProperty` below narrows to `opacity` in @@ -621,27 +599,11 @@ export const popupMotion = stylex.create({ // outright leaves a stacked prompt sitting on an identical prompt with no scrim between // them, which reads as a rendering fault rather than as a preference being honoured. '@media (prefers-reduced-motion: reduce)': { - default: `scale(1) ${STACK_TRANSLATE}`, - ':where([data-stack-base])': `scale(${STACK_SCALE}) ${STACK_TRANSLATE}`, - ':where([data-starting-style], [data-ending-style])': `scale(1) ${STACK_TRANSLATE}`, + default: 'scale(1)', + ':where([data-stack-base])': `scale(${STACK_SCALE}) translateY(${STACK_LIFT})`, + ':where([data-starting-style], [data-ending-style])': 'scale(1)', }, }, - /** - * Holds this surface's exit while a dialog stacked on it is still leaving, so a stack - * dismissed in one action unwinds instead of vanishing at once. - * - * Applies to every slot of `transitionProperty` rather than to the transform alone: what is - * being delayed is the whole departure. Under `reduce` that list narrows to `opacity`, so the - * stagger survives as a staggered fade. - * - * `data-stack-exiting` and not `data-stack-base`, and the difference is the whole reason the - * headless layer reports both: `data-stack-base` releases the moment its child closes, which - * is a frame before there is anything to stagger against. - */ - transitionDelay: { - default: null, - ':where([data-stack-exiting][data-ending-style])': STACK_EXIT_STAGGER, - }, // The sheet travels its OWN HEIGHT rather than the ~11px a scale does, so it runs longer than // anything else here: `slow` in, `base` out, a 1.67:1 ratio in line with the rest of Mosaic. // The dead-frame concern that caps long durations elsewhere does not apply — the delta is From 19d0e8433c67abefdabab77057704e5440cf561f Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 21:07:10 -0600 Subject: [PATCH 09/18] feat(ui): shorten the sheet fade for a stacked prompt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a phone a prompt fades over the full length of its slide, which earns itself against the page — the fade gives the travel somewhere to resolve into. Over an opaque surface it does the opposite: for a quarter of a second the dialog underneath shows through the one arriving, and two stacked sheets read as one muddy surface. There is already a surface there, so the slide can carry the arrival alone and the fade goes back to the desktop `fast`. Keyed on being over any open dialog rather than on the narrower prompt-on-prompt stack: what makes the long fade wrong is arriving over something opaque, and a panel is as opaque as a prompt. --- .../swingset/src/stories/dialog.component.mdx | 5 +++++ .../mosaic/components/dialog/dialog.styles.ts | 17 +++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index 19a5793bf28..fccd3c71368 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -146,6 +146,11 @@ The sheet fades over the full length of its slide, while the backdrop keeps its the scrim answers the tap first, then the sheet arrives into an already-dimmed page. Under `prefers-reduced-motion: reduce` the sheet holds flat and only the fade runs. +A sheet arriving over another dialog takes the shorter desktop fade instead. The long one earns +itself against the page, where it gives the travel somewhere to resolve into; over an opaque +surface it just shows the dialog underneath through the one arriving, and the two read as one muddy +surface. The slide is unchanged, and carries the arrival on its own. + Drag-to-dismiss is deliberately absent — `Drawer` owns the drag engine, and a second one should not grow inside `Dialog`. diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index 816839c9213..2c59d2977d3 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -615,12 +615,29 @@ export const popupMotion = stylex.create({ // `fast` and lands with the scrim, since the scale it accompanies barely moves. The third slot // is inert under the phone band (no scale, so no radius counter-scale) but still has to be // filled — the list is positional. + // + // EXCEPT for a sheet arriving over another dialog, which takes the desktop `fast` fade back. + // The long fade earns itself on the first sheet, where it gives the travel somewhere to + // resolve into against the page. Over an opaque surface it does the opposite: for a quarter of + // a second the dialog underneath shows through the one arriving, and two stacked surfaces + // read as one muddy one. There is already a surface there, so the fade has nothing left to do + // and the slide can carry the arrival alone. + // + // Keyed on `data-stacked` — over any open dialog, panel included — rather than on the narrower + // prompt-on-prompt stack the backdrop cares about. What makes the long fade wrong here is + // arriving over something opaque, and a panel is as opaque as a prompt. + // + // The combined exiting branch restates `base` because `@stylexjs/sort-keys` puts it after the + // plain `data-stacked` one, which would otherwise hand a stacked sheet the four-value entrance + // list on its way out and slow its exit slide. transitionDuration: { default: `${durationVars['--cl-duration-fast']}, ${durationVars['--cl-duration-base']}, ${durationVars['--cl-duration-base']}, ${durationVars['--cl-duration-base']}`, ':where([data-ending-style])': durationVars['--cl-duration-fast'], '@media (max-width: 47.99rem)': { default: `${durationVars['--cl-duration-slow']}, ${durationVars['--cl-duration-slow']}, ${durationVars['--cl-duration-base']}, ${durationVars['--cl-duration-slow']}`, ':where([data-ending-style])': durationVars['--cl-duration-base'], + ':where([data-stacked])': `${durationVars['--cl-duration-fast']}, ${durationVars['--cl-duration-slow']}, ${durationVars['--cl-duration-base']}, ${durationVars['--cl-duration-slow']}`, + ':where([data-stacked][data-ending-style])': durationVars['--cl-duration-base'], }, }, transitionProperty: { From 8b7c0aa9a90f3fceb9becb7a72a7cdb933f30f0c Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 21:11:09 -0600 Subject: [PATCH 10/18] feat(ui): demo a vetoed close in the nested dialog story Makes the panel -> prompt -> prompt case reachable: typing into "add email address" and then trying to close it stacks a confirmation instead. Every close request routes through the controlled `onOpenChange`, so declining to commit there covers Escape, the corner X and Cancel at once. Hand-rolled, and meant to be replaced by the AlertDialog and close confirmation work rather than kept. --- .../swingset/src/stories/dialog.component.mdx | 5 ++ .../src/stories/dialog.component.stories.tsx | 85 ++++++++++++++++--- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index fccd3c71368..0128da7fd06 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -365,6 +365,11 @@ prompt. This is the nested case, not a stack: the prompt paints its own scrim over the panel, and the panel neither dims nor recedes. +Type into **Add email address** and then try to close it — Escape, the corner X, or Cancel — and a +confirmation stacks on top instead, making the panel → prompt → prompt case reachable. The veto is +a controlled `open` whose `onOpenChange` declines to commit; every close request routes through it, +so one check covers all of them. + Stack a prompt on a prompt and the relationship changes — the shape a close confirmation takes: diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index 640fcd51b9e..6dbb4001580 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -107,7 +107,14 @@ const sectionHeader = { justifyContent: 'space-between', } as const; -/** A `prompt` dialog opened from inside the `panel` — the shape the account profile uses. */ +/** + * A `prompt` dialog opened from inside the `panel` — the shape the account profile uses. + * + * With `confirmDiscard`, closing it while the field holds anything opens a confirmation stacked on + * top rather than closing: `panel -> prompt -> prompt`, and the veto is nothing more than a + * controlled `open` whose `onOpenChange` declines to commit. Hand-rolled here on purpose — it is + * what the `AlertDialog` and close-confirmation work is meant to replace. + */ function AddValueDialog({ trigger, title, @@ -115,6 +122,7 @@ function AddValueDialog({ placeholder, confirmLabel = 'Continue', confirmColor, + confirmDiscard = false, }: { trigger: (props: RenderProps) => React.ReactElement; title: string; @@ -122,34 +130,82 @@ function AddValueDialog({ placeholder: string; confirmLabel?: string; confirmColor?: 'negative'; + confirmDiscard?: boolean; }) { + const [open, setOpen] = React.useState(false); + const [discardOpen, setDiscardOpen] = React.useState(false); + const [value, setValue] = React.useState(''); + + const dismiss = () => { + setValue(''); + setOpen(false); + }; + return ( { + // The veto. Every close request lands here — Escape, the corner X, `Dialog.Close` — so + // declining to commit covers all of them at once. A footer button wired to a bare + // `setOpen(false)` would go around it, which is the argument for `Dialog.Close`. + if (!next && confirmDiscard && value.trim() !== '') { + setDiscardOpen(true); + return; + } + if (!next) { + setValue(''); + } + setOpen(next); + }} > - {({ close }) => ( - <> - - }>{title} - }>{description} - + + }>{title} + }>{description} + setValue(event.target.value)} + /> +
+ }>Cancel + +
+ {confirmDiscard ? ( + + }>Discard changes? + }> + You have not finished adding this address. It will not be saved. +
- - )} + + ) : null} ); } @@ -173,6 +229,7 @@ export function Nested() { title='Add email address' description="We'll send a verification code to this address." placeholder='you@example.com' + confirmDiscard /> From 64b4053d9bb526a2a45f0fd467ffb0e80e264b09 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 13:34:38 -0600 Subject: [PATCH 11/18] fix(ui): address review feedback on #9432 Empty the changeset (Mosaic has no consumer-visible surface), match the `[clerk] ` prefix the package's other dev warnings use, and give the stack veil the phone band's `slow` duration so it stays in step with the recede it accompanies. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/dialog-stack-motion.md | 3 --- packages/ui/src/mosaic/components/dialog/dialog.styles.ts | 8 +++++++- packages/ui/src/mosaic/components/dialog/dialog.tsx | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.changeset/dialog-stack-motion.md b/.changeset/dialog-stack-motion.md index 60a5b6ddef7..a845151cc84 100644 --- a/.changeset/dialog-stack-motion.md +++ b/.changeset/dialog-stack-motion.md @@ -1,5 +1,2 @@ --- -'@clerk/ui': patch --- - -Mosaic `Dialog` now distinguishes a stack — successive `prompt` dialogs, such as a confirmation over the form it is confirming — from a dialog opened over a `panel` or `card`. A stacked prompt paints no backdrop of its own, so the page no longer darkens further with every level; the prompt beneath it dims and recedes instead. Dialogs opened over a `panel` or `card` are unchanged. Opening a `panel` or `card` inside another dialog now warns in development. diff --git a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts index 2c59d2977d3..27c51dc0fc8 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.styles.ts +++ b/packages/ui/src/mosaic/components/dialog/dialog.styles.ts @@ -180,7 +180,13 @@ export const styles = stylex.create({ opacity: 'var(--_cl-stack-veil, 0)', pointerEvents: 'none', position: 'absolute', - transitionDuration: durationVars['--cl-duration-base'], + // Tracks the recede it accompanies rather than standing on its own: the two are halves of + // one gesture, and the phone band runs the transform at `slow`. Pinning the veil at `base` + // there finishes the dim 100ms before the surface stops moving, in both directions. + transitionDuration: { + default: durationVars['--cl-duration-base'], + '@media (max-width: 47.99rem)': durationVars['--cl-duration-slow'], + }, transitionProperty: 'opacity', transitionTimingFunction: easingVars['--cl-ease-enter'], zIndex: 1, diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index c539aae84e4..c235c14965d 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -246,7 +246,7 @@ function useNestedSizeWarning(isNestedInDialog: boolean, size: DialogSize) { return; } console.warn( - `Mosaic: a Dialog opened inside another Dialog should be size="prompt", but this one is size="${size}". ` + + `[clerk] a Dialog opened inside another Dialog should be size="prompt", but this one is size="${size}". ` + 'Only prompts are meant to open over another dialog; the rest are root-level surfaces.', ); }, [isNestedInDialog, size]); From 01653e053523a2384d617e400711fe3fea48056e Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Wed, 12 Aug 2026 21:45:34 -0600 Subject: [PATCH 12/18] feat(ui): add Mosaic AlertDialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Dialog that interrupts to ask for a decision and waits for one. Base UI's shape: the same parts as Dialog, with the three props that would let it stop being an alert dialog removed rather than defaulted — `role` is `alertdialog`, `closedBy` is `closerequest`, `size` is `prompt`. Everything else is Dialog's own component rather than a wrapper around it, so there is one implementation of each and no way for the two to drift. No `CloseButton` part, for the same reason an outside press cannot dismiss it: a corner X is a way out without answering. `AlertDialog.Actions` is the one addition — the response row, which is anatomy here in a way a dialog's footer is not. A grid rather than a flex row, because the phone layout is a property on the container instead of something every button has to be told: `grid-auto-columns` is `1fr` under the sheet band, so the buttons split the row and span it, and `auto` above it, where the tracks size to their labels and sit at the inline end. Full-width beats a right-aligned pair floating against one edge of a screen-wide sheet. The cancel goes first, which makes it the first tabbable element and therefore what the alert opens focused on — the least destructive choice, with no `initialFocus` plumbing, and with the keyboard order agreeing with the screen. Title and Description are both required, and both warn in development when missing: an alert dialog's description is announced with its name at the moment it interrupts, so without one the user is choosing between "Cancel" and "Delete" with nothing saying what is being deleted. The existing name warning skipped any role but `dialog`, which would have made it silently inert here, and it now names the component it is complaining about instead of always saying "Dialog". --- .changeset/mosaic-alert-dialog.md | 2 + .../swingset/src/components/DocsViewer.tsx | 1 + packages/swingset/src/lib/registry.ts | 12 + .../src/stories/alert-dialog.component.mdx | 131 +++++++ .../alert-dialog.component.stories.tsx | 136 +++++++ .../alert-dialog/alert-dialog.styles.ts | 38 ++ .../alert-dialog/alert-dialog.test.tsx | 335 ++++++++++++++++++ .../components/alert-dialog/alert-dialog.tsx | 230 ++++++++++++ .../mosaic/components/alert-dialog/index.ts | 13 + .../src/mosaic/components/dialog/dialog.tsx | 10 +- .../hooks/useAccessibleDescriptionWarning.ts | 47 +++ .../mosaic/hooks/useAccessibleNameWarning.ts | 7 +- packages/ui/src/mosaic/styles/index.ts | 13 + 13 files changed, 972 insertions(+), 3 deletions(-) create mode 100644 .changeset/mosaic-alert-dialog.md create mode 100644 packages/swingset/src/stories/alert-dialog.component.mdx create mode 100644 packages/swingset/src/stories/alert-dialog.component.stories.tsx create mode 100644 packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts create mode 100644 packages/ui/src/mosaic/components/alert-dialog/alert-dialog.test.tsx create mode 100644 packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx create mode 100644 packages/ui/src/mosaic/components/alert-dialog/index.ts create mode 100644 packages/ui/src/mosaic/hooks/useAccessibleDescriptionWarning.ts diff --git a/.changeset/mosaic-alert-dialog.md b/.changeset/mosaic-alert-dialog.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-alert-dialog.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/swingset/src/components/DocsViewer.tsx b/packages/swingset/src/components/DocsViewer.tsx index 3895b47ccfe..347a60ca40a 100644 --- a/packages/swingset/src/components/DocsViewer.tsx +++ b/packages/swingset/src/components/DocsViewer.tsx @@ -38,6 +38,7 @@ const docModules: Record> = { input: dynamic(() => import('../stories/input.mdx')), item: dynamic(() => import('../stories/item.mdx')), dialog: dynamic(() => import('../stories/dialog.component.mdx')), + 'alert-dialog': dynamic(() => import('../stories/alert-dialog.component.mdx')), heading: dynamic(() => import('../stories/heading.mdx')), icon: dynamic(() => import('../stories/icon.mdx')), menu: dynamic(() => import('../stories/menu.component.mdx')), diff --git a/packages/swingset/src/lib/registry.ts b/packages/swingset/src/lib/registry.ts index 9baf653c30b..48326de4769 100644 --- a/packages/swingset/src/lib/registry.ts +++ b/packages/swingset/src/lib/registry.ts @@ -1,5 +1,10 @@ // Import stories explicitly to control order and avoid type casting through unknown. import { meta as accordionMeta } from '../stories/accordion.stories'; +import { + Default as AlertDialogDefault, + DiscardChanges as AlertDialogDiscardChanges, + meta as alertDialogComponentMeta, +} from '../stories/alert-dialog.component.stories'; import { meta as autocompleteMeta } from '../stories/autocomplete.stories'; import { Fallback as AvatarFallbackStory, @@ -195,6 +200,12 @@ const inputModule: StoryModule = { meta: inputMeta, Default, Sizes: InputSizes, const dialogComponentModule: StoryModule = { meta: dialogComponentMeta, Default: DialogDefault }; +const alertDialogComponentModule: StoryModule = { + meta: alertDialogComponentMeta, + Default: AlertDialogDefault, + DiscardChanges: AlertDialogDiscardChanges, +}; + const popoverComponentModule: StoryModule = { meta: popoverComponentMeta, Default: PopoverComponentDefault, @@ -293,6 +304,7 @@ export const registry: StoryModule[] = [ inputModule, itemModule, dialogComponentModule, + alertDialogComponentModule, headingModule, iconModule, menuComponentModule, diff --git a/packages/swingset/src/stories/alert-dialog.component.mdx b/packages/swingset/src/stories/alert-dialog.component.mdx new file mode 100644 index 00000000000..d6605d8f66c --- /dev/null +++ b/packages/swingset/src/stories/alert-dialog.component.mdx @@ -0,0 +1,131 @@ +import * as AlertDialogStories from './alert-dialog.component.stories'; + +# AlertDialog + +The Mosaic `AlertDialog` — a `Dialog` that interrupts to ask for a decision, and waits for one. +Reach for it when continuing depends on the answer: confirming something destructive, or warning +that leaving loses work. Anything the user can read and dismiss is a `Dialog`. + +It is composed from the same parts as `Dialog`, so the surface, the motion, the stacking and the +scroll lock are all shared. What differs is fixed rather than configurable: it announces itself as +`role="alertdialog"`, an outside press cannot dismiss it, and it is always the `prompt` size. + +## Example + + + +## Usage + +```tsx +import { AlertDialog } from '@clerk/ui/mosaic/components/alert-dialog'; +import { Button } from '@clerk/ui/mosaic/components/button'; + + }> + {({ close }) => ( + <> + Delete Acme Inc? + This cannot be undone. + + }>Cancel + + + + )} + +``` + +`trigger` is optional, and usually absent — an alert is normally raised by something that already +happened rather than by a button that exists to raise it. Drive those with `open` and +`onOpenChange`. + +### A Title and a Description are both required + +An alert dialog is announced as an interruption, and its description is announced with its name at +that moment — so a title and two buttons leave the user choosing between "Cancel" and "Delete" with +nothing saying what is being deleted. Both are checked in development and warn when missing; neither +can be required in the type system, since parts arrive as children. + +### The cancel comes first + +Render the cancel as the first child of `AlertDialog.Actions`. It is the least destructive choice, +and being first makes it the first tabbable element — which is what the dialog opens focused on, with +no `initialFocus` needed. It is also the visual order in both layouts, so the keyboard order and the +screen agree. + +### The action does not close by itself + +`AlertDialog.Close` dismisses on press, which is what the cancel wants. The action usually starts +work, so close it when that work resolves rather than on the press — the render-prop `close` above, +or your own controlled state. That leaves room for a pending state on the button. + +### Returning focus + +`finalFocus` (and `initialFocus`) are accepted on the wrapper as well as on `AlertDialog.Popup`. +Pass one whenever the alert has no trigger: focus returns to the trigger by default, and an alert +raised by something that happened has none, so answering it would otherwise drop the user on the +body. A confirmation guarding a form wants the caret back in the field it asked about — see +[Confirming a discard](#confirming-a-discard) below. + +### Dismissal + +There is no `closedBy` prop. An outside press never dismisses an alert dialog: a question that needs +an answer must not be answerable by clicking next to it. Escape still closes — it is the keyboard's +equivalent of the cancel button, which is always present here. There is no `CloseButton` part for +the same reason: a corner X is a way out without answering. + +Every close request — Escape or `AlertDialog.Close` — routes through `onOpenChange`, so a controlled +consumer can decline one by not committing the state. + +## Parts + +| Part | Slot | Description | +| ------------------------- | ---------------------- | -------------------------------------------------------------------------------------------- | +| `AlertDialog.Root` | — | State provider; owns open/close, `modal`, `handle`. `role`, `closedBy` and `size` are fixed. | +| `AlertDialog.Trigger` | — | Opens the alert; accepts `render`, and `handle` + `payload` when detached. | +| `AlertDialog.Portal` | — | Portals the overlay out of the tree. | +| `AlertDialog.Backdrop` | `dialog-backdrop` | The scrim behind the alert. | +| `AlertDialog.Viewport` | `dialog-viewport` | Centering container; owns the scroll lock. | +| `AlertDialog.Popup` | `dialog-popup` | The surface (`role="alertdialog"`, focus-trapped); `initialFocus` / `finalFocus`. | +| `AlertDialog.Title` | — | Heading; wired to the popup's `aria-labelledby`. Required. | +| `AlertDialog.Description` | — | Description; wired to the popup's `aria-describedby`. Required. | +| `AlertDialog.Close` | — | Dismisses the alert; unstyled, accepts a `render` prop. | +| `AlertDialog.Actions` | `alert-dialog-actions` | The response row. Cancel first. | + +Every part except `Popup` and `Actions` is `Dialog`'s own component, not a wrapper around it — one +implementation, so the two cannot drift. `Title` and `Description` are unstyled passthroughs from the +headless layer; render them through your own typography (`Heading`, `Text`) via `render`. + +## Styling + +The alert dialog carries the same `.cl-dialog-*` slots as `Dialog`, and is themed the same way — see +the [Dialog](/components/dialog) page for the surface, the motion, the inset, and the state +attributes, all of which apply unchanged. Only the response row is its own: + +```css +@import '@clerk/ui/styles.css' layer(components); + +@layer overrides { + .cl-alert-dialog-actions { + margin-block-start: 1.5rem; + } +} +``` + +`AlertDialog.Actions` is a grid rather than a flex row, which is what lets the row change shape +without the buttons knowing. From `48rem` up the tracks size to their labels and sit at the inline +end. Below it — where a `prompt` is a bottom sheet spanning the screen — the tracks split the row +evenly, so the buttons are full width rather than a pair floating against one edge. + +--- + +## Examples + +### Confirming a discard + + diff --git a/packages/swingset/src/stories/alert-dialog.component.stories.tsx b/packages/swingset/src/stories/alert-dialog.component.stories.tsx new file mode 100644 index 00000000000..d50b499f0d6 --- /dev/null +++ b/packages/swingset/src/stories/alert-dialog.component.stories.tsx @@ -0,0 +1,136 @@ +/** @jsxImportSource @emotion/react */ +import type { RenderProps } from '@clerk/headless/utils'; +import { AlertDialog } from '@clerk/ui/mosaic/components/alert-dialog'; +import { Button } from '@clerk/ui/mosaic/components/button'; +import { Dialog } from '@clerk/ui/mosaic/components/dialog'; +import { Heading } from '@clerk/ui/mosaic/components/heading'; +import { Input } from '@clerk/ui/mosaic/components/input'; +import { Text } from '@clerk/ui/mosaic/components/text'; +import React from 'react'; + +import type { StoryMeta } from '@/lib/types'; + +// Exposes this file's own source (via the `?raw` webpack rule) so each `` example +// renders a code footer with its function's source. See `StoryModule.__source`. +export { default as __source } from './alert-dialog.component.stories?raw'; + +export const meta: StoryMeta = { + group: 'Components', + title: 'AlertDialog', + source: 'packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx', + styleEngine: 'stylex', +}; + +const deleteTrigger = (props: RenderProps) => ( + +); + +export function Default() { + return ( + + {({ close }) => ( + <> + }>Delete Acme Inc? + }> + The organization and everything in it will be permanently removed. This cannot be undone. + + + }>Cancel + {/* Not an `AlertDialog.Close`: the action is where the work happens, so the caller + closes once it resolves rather than the button closing on press. */} + + + + )} + + ); +} + +const addEmailTrigger = (props: RenderProps) => ; + +/** + * The case the stack was built for: a form prompt raising a confirmation over itself rather than + * discarding what was typed. + * + * The veto is a controlled `open` whose `onOpenChange` declines to commit — every close request + * lands there, so Escape, the corner X and `Dialog.Close` are all covered by the one branch. The + * `AlertDialog` is rendered inside the dialog it guards, which is what puts the two in the same + * floating tree: escape ordering, the stacking styles and the refcounted scroll lock all depend + * on it. + */ +export function DiscardChanges() { + const [open, setOpen] = React.useState(false); + const [confirmOpen, setConfirmOpen] = React.useState(false); + const [value, setValue] = React.useState(''); + const inputRef = React.useRef(null); + + const discard = () => { + setValue(''); + setConfirmOpen(false); + setOpen(false); + }; + + return ( + { + if (!next && value.trim() !== '') { + setConfirmOpen(true); + return; + } + setOpen(next); + }} + > + + }>Add email address + }> + You will need to verify this address before it can be used. + + setValue(event.target.value)} + /> +
+ }>Cancel + +
+ + {/* `finalFocus` puts the caret back in the field. Without it there is nowhere to return to — + this alert is raised by the veto rather than by a trigger — so keeping editing would + leave focus on the body, at the top of the page rather than where the work was. */} + + }>Discard changes? + }> + You have not finished adding this address. It will not be saved. + + + }>Keep editing + + + + + ); +} diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts new file mode 100644 index 00000000000..a19841dce8f --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts @@ -0,0 +1,38 @@ +import * as stylex from '@stylexjs/stylex'; + +import { space } from '../../tokens.stylex'; + +export const styles = stylex.create({ + /** + * The response row. An alert dialog exists to be answered, so its buttons are anatomy rather + * than content — the one part `Dialog` deliberately does not ship, because a dialog's footer is + * whatever the consumer composes and an alert dialog's is always the same two choices. + * + * A GRID rather than a flex row, and that is what buys the phone layout without touching the + * buttons. Full-width buttons need `flex: 1` on each CHILD, which a parent cannot set — StyleX + * has no child selector, and reaching into the children would mean every call site remembering + * to pass something. `grid-auto-flow: column` + `grid-auto-columns` moves the same decision onto + * the container: `1fr` gives every button an equal share of the row, `auto` sizes each to its + * label. One property, two layouts. + * + * Under the phone band the row therefore splits evenly and spans the sheet; from 48rem up the + * tracks shrink to their labels and `justify-content: end` puts them at the inline end. The + * buttons never sit hard against one edge on a phone, where the row is the width of the screen + * and a right-aligned pair reads as floating. + * + * DOM order is the visual order in both: the cancel comes first, which is also what makes it the + * first tabbable element and therefore what opens focused — the least destructive choice, with + * no `initialFocus` plumbing. Keep it first; reversing the row visually would leave the keyboard + * order disagreeing with the screen. + */ + actions: { + gap: space['2'], + display: 'grid', + gridAutoColumns: { default: '1fr', '@media (min-width: 48rem)': 'auto' }, + gridAutoFlow: 'column', + justifyContent: { default: null, '@media (min-width: 48rem)': 'end' }, + // On top of the popup's own `gap`, so the response separates from the question it answers + // rather than reading as a third paragraph. + marginBlockStart: space['2'], + }, +}); diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.test.tsx b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.test.tsx new file mode 100644 index 00000000000..e3ecd2cfbe1 --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.test.tsx @@ -0,0 +1,335 @@ +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { Dialog } from '../dialog'; +import { AlertDialog } from './alert-dialog'; + +afterEach(() => cleanup()); + +// Both dev warnings defer by a task, so the assertions have to let one elapse. +const settle = () => + act(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + }); + +function Confirm({ onOpenChange }: { onOpenChange?: (open: boolean) => void } = {}) { + return ( + + Discard changes? + This address has not been saved. + + Keep editing + + + + ); +} + +describe('Mosaic AlertDialog', () => { + it('renders as an alertdialog, named and described by its parts', () => { + render(); + + const popup = screen.getByRole('alertdialog', { name: 'Discard changes?' }); + expect(popup).toHaveAccessibleDescription('This address has not been saved.'); + }); + + it('carries the dialog slot classes, so it inherits the surface and its motion', () => { + render(); + + expect(document.querySelector('.cl-dialog-backdrop')).toBeInTheDocument(); + expect(document.querySelector('.cl-dialog-viewport')).toBeInTheDocument(); + expect(document.querySelector('.cl-dialog-popup')).toBeInTheDocument(); + expect(document.querySelector('.cl-alert-dialog-actions')).toBeInTheDocument(); + }); + + it('is always the prompt size', () => { + render(); + + expect(document.querySelector('.cl-dialog-popup')).toHaveAttribute('data-size', 'prompt'); + }); + + it('opens from a trigger', async () => { + const user = userEvent.setup(); + render( + ( + + )} + > + Delete this key? + Applications using it stop working. + , + ); + + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Delete' })); + + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + }); + + it('opens focused on the cancel button, as the first element in the actions row', async () => { + render(); + + // `FloatingFocusManager` moves focus asynchronously after mount, so this waits rather than + // letting a single task elapse — under a loaded run the one task is not always enough. + await waitFor(() => expect(screen.getByRole('button', { name: 'Keep editing' })).toHaveFocus()); + }); + + it('closes on AlertDialog.Close, reporting it through onOpenChange', async () => { + const user = userEvent.setup(); + const onOpenChange = vi.fn(); + render(); + + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + expect(onOpenChange).toHaveBeenCalledWith(false, expect.anything()); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + it('hands the render-prop form a close that routes through onOpenChange', async () => { + const user = userEvent.setup(); + const onOpenChange = vi.fn(); + render( + + {({ close }) => ( + <> + Discard changes? + This address has not been saved. + + + + + )} + , + ); + + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + expect(onOpenChange).toHaveBeenCalledWith(false, expect.anything()); + }); +}); + +// The dismissal policy is the behavioural half of what makes this an alert dialog: it cannot be +// answered by clicking next to it, but Escape — the keyboard's cancel — still works. +// An alert raised by a veto has no trigger, so without `finalFocus` there is nothing for focus to +// return to and answering the question drops the user on the body. +describe('focus', () => { + it('returns focus where finalFocus points when it closes', async () => { + const user = userEvent.setup(); + + function Guarded() { + const [confirmOpen, setConfirmOpen] = React.useState(true); + const inputRef = React.useRef(null); + return ( + <> + + + Discard changes? + This address has not been saved. + + Keep editing + + + + ); + } + render(); + + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Email address' })).toHaveFocus()); + }); +}); + +describe('dismissal', () => { + it('does not close on an outside press', async () => { + const user = userEvent.setup(); + render(); + + await user.click(document.querySelector('.cl-dialog-backdrop') as HTMLElement); + + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + }); + + it('closes on Escape', async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard('{Escape}'); + + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + it('lets a controlled consumer decline a close', async () => { + const user = userEvent.setup(); + + function Guarded() { + const [open, setOpen] = React.useState(true); + return ( + { + if (next) { + setOpen(true); + } + }} + > + Discard changes? + This address has not been saved. + + Keep editing + + + ); + } + render(); + + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); + }); +}); + +describe('dev warnings', () => { + it('warns when the alert dialog has no description', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render( + + Discard changes? + , + ); + + await settle(); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no description')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); + warn.mockRestore(); + }); + + // The name warning skipped any role but `dialog` before this component existed, which would have + // made it silently inert for every alert dialog. + it('warns when it has no accessible name, and names the alert dialog parts', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render( + + This address has not been saved. + , + ); + + await settle(); + + expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessible name')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); + warn.mockRestore(); + }); + + it('stays quiet when both are supplied', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render(); + + await settle(); + + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); + +describe('AlertDialog.Actions', () => { + it('merges consumer className and style', () => { + render( + + Discard changes? + This address has not been saved. + + Keep editing + + , + ); + + const actions = screen.getByTestId('actions'); + expect(actions).toHaveClass('cl-alert-dialog-actions'); + expect(actions).toHaveClass('custom'); + expect(actions).toHaveStyle({ marginBlockStart: '2rem' }); + }); + + it('renders as another element through render', () => { + render( + + Discard changes? + This address has not been saved. +
}> + Keep editing + + , + ); + + expect(document.querySelector('footer.cl-alert-dialog-actions')).toBeInTheDocument(); + }); +}); + +// An alert dialog is a `prompt`, which is the size that may stack — a form prompt raising a +// "discard changes?" over itself is the case the whole stack was built for. +describe('stacked on another dialog', () => { + it('stacks on a prompt without warning, and marks the surface beneath', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const user = userEvent.setup(); + render( + + Add email address + ( + + )} + > + Discard changes? + This address has not been saved. + + , + ); + + await user.click(screen.getByRole('button', { name: 'Discard' })); + await settle(); + + const popups = document.querySelectorAll('.cl-dialog-popup'); + expect(popups[0]).toHaveAttribute('data-stack-base', ''); + expect(popups[1]).toHaveAttribute('data-stacked', ''); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); +}); diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx new file mode 100644 index 00000000000..5fde39ca70b --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx @@ -0,0 +1,230 @@ +import { useDialogContext } from '@clerk/headless/dialog'; +import { useRender } from '@clerk/headless/utils'; +import * as stylex from '@stylexjs/stylex'; +import type { ReactNode } from 'react'; +import React from 'react'; + +import { useAccessibleDescriptionWarning } from '../../hooks/useAccessibleDescriptionWarning'; +import type { MosaicComponentProps } from '../../props'; +import { mergeStyleProps, themeProps } from '../../props'; +import type { + DialogBackdropProps, + DialogCloseProps, + DialogDescriptionProps, + DialogPopupProps, + DialogRootProps, + DialogTitleProps, + DialogTriggerProps, + DialogViewportProps, +} from '../dialog'; +import { Dialog } from '../dialog'; +// Deep import: the part-name context is how one Mosaic component wraps another and is +// deliberately absent from `../dialog`'s public surface. +import { DialogPartNameContext } from '../dialog/dialog'; +import { reset } from '../reset.styles'; +import { styles } from './alert-dialog.styles'; + +/** + * An alert dialog is a `Dialog` with three decisions already made, so the props that would make + * them are not offered: + * + * - `role` is `alertdialog`, which is the whole point — assistive technology announces it as an + * interruption rather than as a surface the user navigated to; + * - `closedBy` is `closerequest`, so an outside press cannot dismiss it. A dialog asking a + * question it needs an answer to must not be answerable by clicking next to it. Escape still + * closes, which is not negotiable either: it is the keyboard's equivalent of the cancel button, + * and the cancel button is always present here; + * - `size` is `prompt`, the size that means "asks one thing and returns". + */ +export type AlertDialogRootProps = Omit, 'closedBy' | 'role' | 'size'>; + +export type AlertDialogTriggerProps = DialogTriggerProps; +export type AlertDialogBackdropProps = DialogBackdropProps; +export type AlertDialogViewportProps = DialogViewportProps; +export type AlertDialogPopupProps = DialogPopupProps; +export type AlertDialogTitleProps = DialogTitleProps; +export type AlertDialogDescriptionProps = DialogDescriptionProps; +export type AlertDialogCloseProps = DialogCloseProps; +export type AlertDialogActionsProps = MosaicComponentProps<'div'>; + +/** Owns the open state, and pins the three props that make a dialog an alert dialog. */ +function Root({ children, ...rest }: AlertDialogRootProps) { + return ( + + + {...rest} + role='alertdialog' + closedBy='closerequest' + size='prompt' + > + {children} + + + ); +} + +/** + * The alert surface. Identical to `Dialog.Popup` — same styles, same focus trap, same stacking — + * plus the description check, which is a requirement here rather than a nicety. + * + * No `Dialog.CloseButton` counterpart, and that omission is the design: a corner X is a way out + * without answering, and an alert dialog has no such path. The cancel button is the way out. + */ +const Popup = React.forwardRef(function AlertDialogPopup(props, ref) { + // Observed through state rather than a plain ref, for the same reason `Dialog.Popup` does it: + // the warning has to re-run when the node arrives, and a ref mutation does not re-render. + const [node, setNode] = React.useState(null); + useAccessibleDescriptionWarning(node, 'AlertDialog'); + + const mergedRef = React.useCallback( + (element: HTMLDivElement | null) => { + setNode(element); + if (typeof ref === 'function') { + ref(element); + } else if (ref) { + ref.current = element; + } + }, + [ref], + ); + + return ( + + ); +}); + +/** + * The row holding the answer. Render the cancel first — see `alert-dialog.styles.ts` for why that + * ordering is what focuses it on open. + */ +const Actions = React.forwardRef(function AlertDialogActions( + { render, className, style, ...rest }, + ref, +) { + return useRender({ + defaultTagName: 'div', + render, + ref, + props: { + ...mergeStyleProps( + themeProps('alert-dialog-actions'), + stylex.props(reset.base, styles.actions), + className, + style, + ), + ...rest, + }, + }); +}); + +export interface AlertDialogProps + extends + Pick, + /** + * Focus, forwarded to the popup. `finalFocus` earns its place on the wrapper rather than only + * on the part: an alert is usually raised by something that happened rather than by a trigger, + * and with no trigger there is nothing for focus to return to when it closes. Answering + * "keep editing" should put the caret back in the field the question was about. + */ + Pick { + /** + * Renders the button that opens the alert. Omit for alerts driven entirely by `open` — the + * common case, since an alert is usually raised by something that already happened rather than + * by a button that exists to raise it. + */ + trigger?: MosaicComponentProps<'button'>['render']; + children: ReactNode | ((ctx: { close: () => void }) => ReactNode); +} + +function AlertDialogContent({ children }: { children: AlertDialogProps['children'] }) { + const { setOpen } = useDialogContext(); + if (typeof children !== 'function') { + return <>{children}; + } + // Routed through the primitive's close funnel, so a controlled consumer's `onOpenChange` sees + // this close the same as Escape does — and can decline it. + return <>{children({ close: () => setOpen(false) })}; +} + +/** + * Mosaic `AlertDialog` — a `Dialog` that interrupts to ask for a decision, and waits for one. + * + * Reach for it when continuing depends on the answer: confirming something destructive, or + * warning that leaving loses work. Anything the user can simply read and dismiss is a `Dialog`. + * + * Composed from the same parts, so everything true of `Dialog` is true here — the surface, the + * motion, the stacking over another dialog, the scroll lock. What differs is what it announces + * itself as, that an outside press does not dismiss it, and that it carries a `Title`, a + * `Description`, and an `Actions` row rather than arbitrary content. Both are checked in + * development; neither is enforceable in the type system, since parts arrive as children. + * + * Drop to the compound parts (`AlertDialog.Root` and friends) for layouts this wrapper does not + * cover. + * + * @example + * } + * > + * Delete this key? + * Applications using it will stop working immediately. + * + * }>Cancel + * + * + * + */ +export function AlertDialog({ + trigger, + children, + open, + defaultOpen, + onOpenChange, + modal, + initialFocus, + finalFocus, +}: AlertDialogProps) { + return ( + + {trigger ? : null} + + + + + {children} + + + + + ); +} + +/** + * Compound parts. The ones an alert dialog does not change are `Dialog`'s own — same components, + * not wrappers around them, so there is one implementation of each and no way for the two to + * drift. + */ +AlertDialog.Root = Root; +AlertDialog.Trigger = Dialog.Trigger; +/** Creates a handle linking detached `AlertDialog.Trigger`s to an `AlertDialog.Root` anywhere in the tree. */ +AlertDialog.createHandle = Dialog.createHandle; +AlertDialog.Portal = Dialog.Portal; +AlertDialog.Backdrop = Dialog.Backdrop; +AlertDialog.Viewport = Dialog.Viewport; +AlertDialog.Popup = Popup; +AlertDialog.Title = Dialog.Title; +AlertDialog.Description = Dialog.Description; +AlertDialog.Close = Dialog.Close; +AlertDialog.Actions = Actions; diff --git a/packages/ui/src/mosaic/components/alert-dialog/index.ts b/packages/ui/src/mosaic/components/alert-dialog/index.ts new file mode 100644 index 00000000000..4c16d774290 --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/index.ts @@ -0,0 +1,13 @@ +export { AlertDialog } from './alert-dialog'; +export type { + AlertDialogActionsProps, + AlertDialogBackdropProps, + AlertDialogCloseProps, + AlertDialogDescriptionProps, + AlertDialogPopupProps, + AlertDialogProps, + AlertDialogRootProps, + AlertDialogTitleProps, + AlertDialogTriggerProps, + AlertDialogViewportProps, +} from './alert-dialog'; diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index c235c14965d..033048d4a83 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -40,6 +40,14 @@ const DialogSizeContext = React.createContext('prompt'); */ const DialogParentSizeContext = React.createContext('prompt'); +/** + * The compound component the popup's dev warnings speak in. `AlertDialog` is composed from these + * same parts, so a message hardcoded to `Dialog` would name parts that do not exist at the call + * site it is complaining about. Not exported from the folder's `index.ts`: it is how one Mosaic + * component wraps another, not something a consumer sets. + */ +export const DialogPartNameContext = React.createContext('Dialog'); + /** Whether this dialog is a prompt stacked on a prompt — see {@link DialogParentSizeContext}. */ function useIsStacked() { const { isStacked } = useDialogContext(); @@ -264,7 +272,7 @@ const Popup = React.forwardRef(function Dialog // Observed through state rather than a plain ref, because the warning has to re-run when the // node arrives and a ref mutation does not re-render. const [node, setNode] = React.useState(null); - useAccessibleNameWarning(node, 'Dialog'); + useAccessibleNameWarning(node, React.useContext(DialogPartNameContext)); useNestedSizeWarning(isNestedInDialog, size); const mergedRef = React.useCallback( diff --git a/packages/ui/src/mosaic/hooks/useAccessibleDescriptionWarning.ts b/packages/ui/src/mosaic/hooks/useAccessibleDescriptionWarning.ts new file mode 100644 index 00000000000..57883cd1f47 --- /dev/null +++ b/packages/ui/src/mosaic/hooks/useAccessibleDescriptionWarning.ts @@ -0,0 +1,47 @@ +import { useEffect } from 'react'; + +/** + * Warns in development when an alert dialog has no accessible description. + * + * A name alone is enough for an ordinary dialog — the surface is there to be read, and what it + * contains describes itself. An alert dialog is the case where it is not: it interrupts to demand + * a decision, and a screen reader announces its description alongside its name at that moment, so + * a title and two buttons leave the user choosing between "Cancel" and "Delete" with nothing + * saying what is being deleted. + * + * Same shape as {@link useAccessibleNameWarning}, and for the same reasons — the description part + * reports itself through an effect, so the attribute is legitimately unresolved on the commit that + * mounts the surface and the check has to be both post-mount and deferred by a task. + * + * @param node - The element carrying the dialog role, once mounted. + * @param component - Compound component name, used to name the parts in the message. + */ +export function useAccessibleDescriptionWarning(node: HTMLElement | null, component: string): void { + useEffect(() => { + if (process.env.NODE_ENV === 'production' || !node) { + return; + } + + const timer = setTimeout(() => { + if (!node.isConnected) { + return; + } + // RESOLVED, not merely present: the primitive emits `aria-describedby` unconditionally, so + // with no `Description` part the attribute points at an id that is not in the document — + // which describes the dialog exactly as poorly as having no attribute at all. + const describedBy = node.getAttribute('aria-describedby'); + const described = describedBy + ?.split(/\s+/) + .filter(Boolean) + .some(id => node.ownerDocument.getElementById(id)?.textContent?.trim()); + if (described) { + return; + } + console.warn( + `[clerk] <${component}.Popup> renders an alert dialog with no description. Render a \`<${component}.Description>\` inside it — it is announced with the title, and is what says which decision is being asked for.`, + ); + }, 0); + + return () => clearTimeout(timer); + }, [node, component]); +} diff --git a/packages/ui/src/mosaic/hooks/useAccessibleNameWarning.ts b/packages/ui/src/mosaic/hooks/useAccessibleNameWarning.ts index 941d3006f6c..0fc7e1e03c2 100644 --- a/packages/ui/src/mosaic/hooks/useAccessibleNameWarning.ts +++ b/packages/ui/src/mosaic/hooks/useAccessibleNameWarning.ts @@ -12,7 +12,9 @@ import { useEffect } from 'react'; * - it has to be deferred by a task even then, for the same reason one commit later. * * `role` is checked rather than assumed because the part may be rendered as something else - * through `render`, and only a `dialog` needs a name badly enough to warn about. + * through `render`, and only a dialog needs a name badly enough to warn about. Both dialog roles + * count: `alertdialog` is the same surface asking more urgently, and an unnamed one is worse, not + * exempt. * * @param node - The element carrying `role="dialog"`, once mounted. * @param component - Compound component name, used to name the parts in the message. @@ -24,7 +26,8 @@ export function useAccessibleNameWarning(node: HTMLElement | null, component: st } const timer = setTimeout(() => { - if (!node.isConnected || node.getAttribute('role') !== 'dialog') { + const role = node.getAttribute('role'); + if (!node.isConnected || (role !== 'dialog' && role !== 'alertdialog')) { return; } if (node.getAttribute('aria-label')?.trim()) { diff --git a/packages/ui/src/mosaic/styles/index.ts b/packages/ui/src/mosaic/styles/index.ts index 1c306a7efe5..da5534b2bc2 100644 --- a/packages/ui/src/mosaic/styles/index.ts +++ b/packages/ui/src/mosaic/styles/index.ts @@ -6,6 +6,19 @@ export type { MosaicComponentProps, MosaicElementProps } from '../props'; +export { AlertDialog } from '../components/alert-dialog'; +export type { + AlertDialogActionsProps, + AlertDialogBackdropProps, + AlertDialogCloseProps, + AlertDialogDescriptionProps, + AlertDialogPopupProps, + AlertDialogProps, + AlertDialogRootProps, + AlertDialogTitleProps, + AlertDialogTriggerProps, + AlertDialogViewportProps, +} from '../components/alert-dialog'; export { Avatar } from '../components/avatar'; export type { AvatarProps, AvatarImageProps, AvatarFallbackProps, AvatarIconProps } from '../components/avatar'; export { Badge } from '../components/badge'; From 712759b535e32eabfa033503150dd09ec0c131b2 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 11:27:24 -0600 Subject: [PATCH 13/18] feat(ui): split a prompt's actions evenly at every width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The designs give every `prompt` the same footer, alert dialogs included: one full-width button, or two at even halves with a 0.75rem gap. Not a pair sized to their labels against the inline end, which is what this shipped as. So the media branch goes, and the row gets shorter rather than longer: `grid-auto-columns: 1fr` unconditionally already means one button fills the row and two split it, with nothing to switch on at 48rem and no edge case waiting for a third. The phone layout was this all along — it is now simply the layout. --- .../src/stories/alert-dialog.component.mdx | 9 +++--- .../alert-dialog/alert-dialog.styles.ts | 30 +++++++++---------- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/packages/swingset/src/stories/alert-dialog.component.mdx b/packages/swingset/src/stories/alert-dialog.component.mdx index d6605d8f66c..b1cbfe2c4cb 100644 --- a/packages/swingset/src/stories/alert-dialog.component.mdx +++ b/packages/swingset/src/stories/alert-dialog.component.mdx @@ -114,10 +114,11 @@ attributes, all of which apply unchanged. Only the response row is its own: } ``` -`AlertDialog.Actions` is a grid rather than a flex row, which is what lets the row change shape -without the buttons knowing. From `48rem` up the tracks size to their labels and sit at the inline -end. Below it — where a `prompt` is a bottom sheet spanning the screen — the tracks split the row -evenly, so the buttons are full width rather than a pair floating against one edge. +`AlertDialog.Actions` is a grid rather than a flex row, which is what lets one declaration cover +both cases without the buttons knowing anything. Every button takes an equal share of the row, so a +single action fills it and two split it in half, at every width — the convention for a `prompt` +generally, not a rule about alert dialogs. Nothing about it is media-scoped, so a third button +divides the same row into thirds rather than finding an edge case. --- diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts index a19841dce8f..7c56f170be2 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.styles.ts @@ -8,29 +8,27 @@ export const styles = stylex.create({ * than content — the one part `Dialog` deliberately does not ship, because a dialog's footer is * whatever the consumer composes and an alert dialog's is always the same two choices. * - * A GRID rather than a flex row, and that is what buys the phone layout without touching the - * buttons. Full-width buttons need `flex: 1` on each CHILD, which a parent cannot set — StyleX - * has no child selector, and reaching into the children would mean every call site remembering - * to pass something. `grid-auto-flow: column` + `grid-auto-columns` moves the same decision onto - * the container: `1fr` gives every button an equal share of the row, `auto` sizes each to its - * label. One property, two layouts. + * One layout at every width, because that is the convention for a `prompt` generally rather than + * a rule about alert dialogs: its buttons span the surface, one full width or two at even halves. + * A right-aligned pair sized to its labels was tried first and is what the designs do not do. * - * Under the phone band the row therefore splits evenly and spans the sheet; from 48rem up the - * tracks shrink to their labels and `justify-content: end` puts them at the inline end. The - * buttons never sit hard against one edge on a phone, where the row is the width of the screen - * and a right-aligned pair reads as floating. + * A GRID rather than a flex row, and that is what makes both cases the same declaration. Filling + * the row needs `flex: 1` on each CHILD, which a parent cannot set — StyleX has no child + * selector, and reaching into the children would mean every call site remembering to pass + * something. `grid-auto-flow: column` with `grid-auto-columns: 1fr` puts it on the container + * instead: every button takes an equal share of the row, so one fills it and two split it, with + * no branch and nothing for a third to break. * - * DOM order is the visual order in both: the cancel comes first, which is also what makes it the - * first tabbable element and therefore what opens focused — the least destructive choice, with - * no `initialFocus` plumbing. Keep it first; reversing the row visually would leave the keyboard + * DOM order is the visual order: the cancel comes first, which is also what makes it the first + * tabbable element and therefore what opens focused — the least destructive choice, with no + * `initialFocus` plumbing. Keep it first; reversing the row visually would leave the keyboard * order disagreeing with the screen. */ actions: { - gap: space['2'], + gap: space['3'], display: 'grid', - gridAutoColumns: { default: '1fr', '@media (min-width: 48rem)': 'auto' }, + gridAutoColumns: '1fr', gridAutoFlow: 'column', - justifyContent: { default: null, '@media (min-width: 48rem)': 'end' }, // On top of the popup's own `gap`, so the response separates from the question it answers // rather than reading as a third paragraph. marginBlockStart: space['2'], From 4d5635ee8c165a7cdf58de45e941b5a9d283c91e Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 13:36:52 -0600 Subject: [PATCH 14/18] fix(ui): address review feedback on #9433 Scope `DialogPartNameContext` to the alert's popup, so a plain `Dialog` nested inside one no longer inherits the name, and share `Dialog`'s content resolver instead of copying it. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/alert-dialog/alert-dialog.tsx | 50 ++++++++----------- .../src/mosaic/components/dialog/dialog.tsx | 10 +++- 2 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx index 5fde39ca70b..31ade3e65b9 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx @@ -1,4 +1,3 @@ -import { useDialogContext } from '@clerk/headless/dialog'; import { useRender } from '@clerk/headless/utils'; import * as stylex from '@stylexjs/stylex'; import type { ReactNode } from 'react'; @@ -18,9 +17,9 @@ import type { DialogViewportProps, } from '../dialog'; import { Dialog } from '../dialog'; -// Deep import: the part-name context is how one Mosaic component wraps another and is -// deliberately absent from `../dialog`'s public surface. -import { DialogPartNameContext } from '../dialog/dialog'; +// Deep import: the part-name context and the content resolver are how one Mosaic component wraps +// another and are deliberately absent from `../dialog`'s public surface. +import { DialogContent, DialogPartNameContext } from '../dialog/dialog'; import { reset } from '../reset.styles'; import { styles } from './alert-dialog.styles'; @@ -50,16 +49,14 @@ export type AlertDialogActionsProps = MosaicComponentProps<'div'>; /** Owns the open state, and pins the three props that make a dialog an alert dialog. */ function Root({ children, ...rest }: AlertDialogRootProps) { return ( - - - {...rest} - role='alertdialog' - closedBy='closerequest' - size='prompt' - > - {children} - - + + {...rest} + role='alertdialog' + closedBy='closerequest' + size='prompt' + > + {children} + ); } @@ -88,11 +85,16 @@ const Popup = React.forwardRef(function A [ref], ); + // Scoped to the popup rather than to the whole root: this is the only place the name is read, + // and a plain `Dialog` nested inside an alert would otherwise inherit it and have its own + // warnings name `AlertDialog` parts that do not exist at that call site. return ( - + + + ); }); @@ -139,16 +141,6 @@ export interface AlertDialogProps children: ReactNode | ((ctx: { close: () => void }) => ReactNode); } -function AlertDialogContent({ children }: { children: AlertDialogProps['children'] }) { - const { setOpen } = useDialogContext(); - if (typeof children !== 'function') { - return <>{children}; - } - // Routed through the primitive's close funnel, so a controlled consumer's `onOpenChange` sees - // this close the same as Escape does — and can decline it. - return <>{children({ close: () => setOpen(false) })}; -} - /** * Mosaic `AlertDialog` — a `Dialog` that interrupts to ask for a decision, and waits for one. * @@ -203,7 +195,7 @@ export function AlertDialog({ initialFocus={initialFocus} finalFocus={finalFocus} > - {children} + {children} diff --git a/packages/ui/src/mosaic/components/dialog/dialog.tsx b/packages/ui/src/mosaic/components/dialog/dialog.tsx index 033048d4a83..0e041a1e0e5 100644 --- a/packages/ui/src/mosaic/components/dialog/dialog.tsx +++ b/packages/ui/src/mosaic/components/dialog/dialog.tsx @@ -315,7 +315,15 @@ export interface DialogProps extends Pick< size?: DialogSize; } -function DialogContent({ children }: { children: DialogProps['children'] }) { +/** + * Resolves the render-prop form of `children`. Shared with `AlertDialog`, which offers the same + * `close` contract and would otherwise carry a second implementation of it. Not exported from the + * folder's `index.ts`, for the same reason as {@link DialogPartNameContext}. + * + * Routed through the primitive's close funnel, so a controlled consumer's `onOpenChange` sees this + * close the same as Escape does — and can decline it. + */ +export function DialogContent({ children }: { children: DialogProps['children'] }) { const { setOpen } = useDialogContext(); if (typeof children !== 'function') { return <>{children}; From 7d5914a1fdf844ef38b9b851c30d660c41fd3229 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 09:33:18 -0600 Subject: [PATCH 15/18] feat(ui): dialog close confirmation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dialog holding unsaved work should ask before discarding it. Three pieces, each with one job: `createConfirmHandle()` links a question to its answer, `useConfirmedClose` guards the close path, and `` is the dialog, rendered inside the one it guards so the two share a floating tree — escape ordering, the stacking styles and the refcounted scroll lock all read that tree, and a globally mounted confirmation would break every one of them. `show()` returns a promise resolving to the answer, so a confirmation reads as `if (await confirm.show({…}))` rather than as a pair of state variables and a callback. Calling it while one is already showing returns the IN-FLIGHT promise instead of opening a second: holding Escape against a guarded dialog would otherwise stack a confirmation per keypress. The veto is the absence of a commit. `useConfirmedClose` wraps the consumer's own `onOpenChange`, so it covers every close the dialog owns — Escape, outside press, `Dialog.CloseButton`, `Dialog.Close`, and the `close` the wrapper hands its children all funnel through it. A button wired to the consumer's own `setOpen(false)` never reaches the dialog and so bypasses the question; that is inherent, and both the hook's JSDoc and the docs page say so. Two ordering details that are load-bearing. The action settles `true` before closing, and `settle` is a no-op once a question is answered, so the close that follows cannot overwrite the answer with `false`. And the hook reads `when` and `onOpenChange` through a ref, so the callback identity is stable across the keystrokes of the very form whose dirtiness `when` reports on. Headless gains `handle.open(payload)` — the programmatic counterpart of a trigger's payload, which is how the confirmation's own text reaches it. The root holds it in a ref as well as in state: the registry lookup that runs once the dialog is open resolves a trigger-less open to `undefined`, and would otherwise blank the dialog a commit after it was filled. --- .changeset/dialog-close-confirmation.md | 2 + .../headless/src/primitives/dialog/README.md | 11 + .../src/primitives/dialog/dialog-handle.ts | 24 +- .../src/primitives/dialog/dialog-root.tsx | 20 +- .../src/primitives/dialog/dialog.test.tsx | 57 ++++ .../src/stories/alert-dialog.component.mdx | 60 ++++ .../alert-dialog.component.stories.tsx | 117 +++---- .../src/stories/dialog.component.stories.tsx | 5 +- .../components/alert-dialog/alert-dialog.tsx | 70 +++++ .../components/alert-dialog/confirm-handle.ts | 87 ++++++ .../components/alert-dialog/confirm.test.tsx | 291 ++++++++++++++++++ .../mosaic/components/alert-dialog/index.ts | 5 + .../alert-dialog/use-confirmed-close.ts | 76 +++++ packages/ui/src/mosaic/styles/index.ts | 6 +- 14 files changed, 762 insertions(+), 69 deletions(-) create mode 100644 .changeset/dialog-close-confirmation.md create mode 100644 packages/ui/src/mosaic/components/alert-dialog/confirm-handle.ts create mode 100644 packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx create mode 100644 packages/ui/src/mosaic/components/alert-dialog/use-confirmed-close.ts diff --git a/.changeset/dialog-close-confirmation.md b/.changeset/dialog-close-confirmation.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/dialog-close-confirmation.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/headless/src/primitives/dialog/README.md b/packages/headless/src/primitives/dialog/README.md index 1ef84c34301..c566d5bba4a 100644 --- a/packages/headless/src/primitives/dialog/README.md +++ b/packages/headless/src/primitives/dialog/README.md @@ -78,6 +78,17 @@ const detail = Dialog.createHandle<{ name: string }>(); ``` +An open with no trigger behind it can supply the payload directly: `handle.open(payload)` is the +programmatic counterpart, for a dialog raised by something that happened rather than by an element +— a confirmation that has to say what it is asking. A trigger-driven open supersedes it, since a +trigger names its own payload. + +```tsx +const confirmation = Dialog.createHandle<{ question: string }>(); + +confirmation.open({ question: 'Discard changes?' }); +``` + In controlled mode, track which trigger is active with `triggerId` — `onOpenChange`'s second argument reports the trigger behind each change: diff --git a/packages/headless/src/primitives/dialog/dialog-handle.ts b/packages/headless/src/primitives/dialog/dialog-handle.ts index 2ce8e89e9aa..739cec4364f 100644 --- a/packages/headless/src/primitives/dialog/dialog-handle.ts +++ b/packages/headless/src/primitives/dialog/dialog-handle.ts @@ -20,10 +20,10 @@ export interface DialogTriggerRegistration { * requests made with no root attached are ignored, matching Base UI. * @internal */ -export interface DialogRootController { +export interface DialogRootController { openFromTrigger: (id: string, event: Event) => void; closeFromTrigger: (id: string, event: Event) => void; - setOpen: (open: boolean) => void; + setOpen: (open: boolean, payload?: Payload) => void; } /** The slice of root state a trigger renders from: its `data-open` / ARIA wiring. */ @@ -45,8 +45,16 @@ const CLOSED_STATE: DialogHandleState = { open: false, triggerId: null, popupId: * lets a `DialogHandle` flow into contexts typed `DialogHandle`. */ export interface DialogHandle { - /** Opens the attached root. Ignored while no root is mounted. */ - open(): void; + /** + * Opens the attached root. Ignored while no root is mounted. + * + * The optional `payload` is the programmatic counterpart of a trigger's: it reaches the root's + * children-as-function as `{ payload }`, so an imperative open can carry the content the dialog + * is about — what a confirmation is asking, which record is being deleted — without the caller + * holding a second piece of state alongside `open`. A trigger-driven open supersedes it, since + * a trigger names its own payload. + */ + open(payload?: Payload): void; /** Closes the attached root. Ignored while no root is mounted. */ close(): void; /** Whether the attached root is open. `false` while no root is mounted. */ @@ -58,7 +66,7 @@ export interface DialogHandle { /** @internal */ getFirstTrigger(): DialogTriggerRegistration | undefined; /** @internal */ - setRoot(controller: DialogRootController): () => void; + setRoot(controller: DialogRootController): () => void; /** @internal */ requestOpen(id: string, event: Event): void; /** @internal */ @@ -79,14 +87,14 @@ export interface DialogHandle { export function createDialogHandle(): DialogHandle { const triggers = new Map>(); const listeners = new Set<() => void>(); - let root: DialogRootController | null = null; + let root: DialogRootController | null = null; let state = CLOSED_STATE; const notify = () => listeners.forEach(listener => listener()); return { - open() { - root?.setOpen(true); + open(payload) { + root?.setOpen(true, payload); }, close() { root?.setOpen(false); diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index 66bb2fb0f72..7ae9f947531 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -98,6 +98,10 @@ function DialogInner(props: DialogProps & { isNested: boolean // consumed by the floating `onOpenChange` the request funnels into. const pendingDetailsRef = useRef(null); + // The payload of the most recent programmatic `handle.open(payload)`, kept so the registry + // lookup below has something to fall back to when no trigger is involved. + const directPayloadRef = useRef(undefined); + // Every open/close funnels through `floatingContext.onOpenChange` — trigger activations, // dismissals, and programmatic `setOpen` alike. floating-ui emits its `openchange` event // synchronously before invoking this callback, which is what lets listeners (`useReturnFocus`, @@ -119,6 +123,9 @@ function DialogInner(props: DialogProps & { isNested: boolean openFromTrigger: (id, event) => { const registration = store.getTrigger(id); setActiveTriggerId(id); + // A trigger names its own payload, so it supersedes anything a previous programmatic + // open supplied — otherwise the stale one would resurface through the effect below. + directPayloadRef.current = undefined; setActivePayload(registration?.getPayload()); if (registration) { refs.setReference(registration.element); @@ -131,7 +138,14 @@ function DialogInner(props: DialogProps & { isNested: boolean pendingDetailsRef.current = { trigger: registration?.element ?? null, triggerId: id, event }; floatingContext.onOpenChange(false, event, 'click'); }, - setOpen: nextOpen => floatingContext.onOpenChange(nextOpen), + setOpen: (nextOpen, payload) => { + // Held in a ref as well as in state because the payload effect below re-runs on `open` + // and would otherwise resolve a trigger-less open to `undefined`, wiping this a commit + // after it was set. + directPayloadRef.current = payload; + setActivePayload(payload); + floatingContext.onOpenChange(nextOpen); + }, }); // `floatingContext` is rebuilt on open/element changes; re-registering is an idempotent swap. }, [store, refs, floatingContext, setActiveTriggerId]); @@ -157,7 +171,9 @@ function DialogInner(props: DialogProps & { isNested: boolean // the time it reads, and the pre-paint re-render delivers their payload on the first frame. useLayoutEffect(() => { if (open) { - setActivePayload(activeTriggerId != null ? store.getTrigger(activeTriggerId)?.getPayload() : undefined); + setActivePayload( + activeTriggerId != null ? store.getTrigger(activeTriggerId)?.getPayload() : directPayloadRef.current, + ); } }, [store, open, activeTriggerId]); diff --git a/packages/headless/src/primitives/dialog/dialog.test.tsx b/packages/headless/src/primitives/dialog/dialog.test.tsx index 578e8ba03b1..51f65891232 100644 --- a/packages/headless/src/primitives/dialog/dialog.test.tsx +++ b/packages/headless/src/primitives/dialog/dialog.test.tsx @@ -528,6 +528,63 @@ describe('Dialog', () => { expect(screen.getByRole('dialog', { name: 'payload-b' })).toBeInTheDocument(); }); + + // The programmatic counterpart of a trigger's payload, for an open that no element initiated — + // a confirmation raised by a close request, say, which has to say what it is asking. + describe('handle.open(payload)', () => { + function renderDetached() { + const handle = Dialog.createHandle(); + render( + + {({ payload }) => ( + <> + Open A + + + + {payload ?? 'no payload'} + + + + + )} + , + ); + return handle; + } + + it('delivers it to the children render function', () => { + const handle = renderDetached(); + + act(() => handle.open('from-handle')); + + expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument(); + }); + + it('survives the registry lookup that runs once the dialog is open', async () => { + const handle = renderDetached(); + + act(() => handle.open('from-handle')); + // The lookup effect re-runs on `open`; without a fallback it would resolve to `undefined` + // a commit later and blank the dialog. + await act(async () => { + await Promise.resolve(); + }); + + expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument(); + }); + + it('is superseded by a trigger, which names its own payload', async () => { + const user = userEvent.setup(); + const handle = renderDetached(); + + act(() => handle.open('from-handle')); + act(() => handle.close()); + await user.click(screen.getByRole('button', { name: 'Open A' })); + + expect(screen.getByRole('dialog', { name: 'no payload' })).toBeInTheDocument(); + }); + }); }); describe('initialFocus', () => { diff --git a/packages/swingset/src/stories/alert-dialog.component.mdx b/packages/swingset/src/stories/alert-dialog.component.mdx index b1cbfe2c4cb..3a25e811c76 100644 --- a/packages/swingset/src/stories/alert-dialog.component.mdx +++ b/packages/swingset/src/stories/alert-dialog.component.mdx @@ -79,6 +79,65 @@ the same reason: a corner X is a way out without answering. Every close request — Escape or `AlertDialog.Close` — routes through `onOpenChange`, so a controlled consumer can decline one by not committing the state. +## Confirming a close + +A dialog holding unsaved work should ask before discarding it. That is three pieces: a handle, a +hook that guards the close, and the confirmation itself. + +```tsx +import { AlertDialog, createConfirmHandle, useConfirmedClose } from '@clerk/ui/mosaic/components/alert-dialog'; + +const confirm = React.useMemo(() => createConfirmHandle(), []); + +const onOpenChange = useConfirmedClose({ + handle: confirm, + when: () => value !== '', + onOpenChange: setOpen, + confirm: { + title: 'Discard changes?', + description: 'You have not finished adding this address.', + actionLabel: 'Discard', + cancelLabel: 'Keep editing', + destructive: true, + }, +}); + + + {/* … */} + + +``` + +**Render `AlertDialog.Confirm` inside the dialog it guards** — anywhere in its children. That is +what puts the two in one floating tree, and escape ordering, the stacking styles and the refcounted +scroll lock all read that tree. A confirmation mounted app-globally would be a sibling of the dialog +rather than a child of it, and all three would break. + +**The guarded dialog must be controlled.** A veto is the absence of a commit, and an uncontrolled +dialog has already committed by the time `onOpenChange` runs. + +**What it covers is every close the dialog owns**: Escape, an outside press where `closedBy` allows +one, `Dialog.CloseButton`, `Dialog.Close`, and the `close` the `Dialog` wrapper hands its children. +A button wired to your own `setOpen(false)` never reaches the dialog, so it bypasses the question +silently — route those through `Dialog.Close`. + +`when()` is evaluated at each close request, so a close that no longer needs guarding (the form has +just been submitted, the field cleared) passes straight through. + +### Asking without a close + +`show()` is the same confirmation, awaited directly — for a decision that is not about closing: + +```tsx +if (await confirm.show({ title: 'Delete this key?', description: 'Applications using it stop working.' })) { + await deleteKey(); +} +``` + +It resolves `true` for the action and `false` for cancel or any dismissal. Calling it while a +confirmation is already showing returns the in-flight promise rather than opening a second one, so +repeated close requests ask once. + ## Parts | Part | Slot | Description | @@ -93,6 +152,7 @@ consumer can decline one by not committing the state. | `AlertDialog.Description` | — | Description; wired to the popup's `aria-describedby`. Required. | | `AlertDialog.Close` | — | Dismisses the alert; unstyled, accepts a `render` prop. | | `AlertDialog.Actions` | `alert-dialog-actions` | The response row. Cancel first. | +| `AlertDialog.Confirm` | `dialog-popup` | A whole confirmation rendered from a `show()` call. See below. | Every part except `Popup` and `Actions` is `Dialog`'s own component, not a wrapper around it — one implementation, so the two cannot drift. `Title` and `Description` are unstyled passthroughs from the diff --git a/packages/swingset/src/stories/alert-dialog.component.stories.tsx b/packages/swingset/src/stories/alert-dialog.component.stories.tsx index d50b499f0d6..dc246efa812 100644 --- a/packages/swingset/src/stories/alert-dialog.component.stories.tsx +++ b/packages/swingset/src/stories/alert-dialog.component.stories.tsx @@ -1,6 +1,6 @@ /** @jsxImportSource @emotion/react */ import type { RenderProps } from '@clerk/headless/utils'; -import { AlertDialog } from '@clerk/ui/mosaic/components/alert-dialog'; +import { AlertDialog, createConfirmHandle, useConfirmedClose } from '@clerk/ui/mosaic/components/alert-dialog'; import { Button } from '@clerk/ui/mosaic/components/button'; import { Dialog } from '@clerk/ui/mosaic/components/dialog'; import { Heading } from '@clerk/ui/mosaic/components/heading'; @@ -62,75 +62,80 @@ const addEmailTrigger = (props: RenderProps) => - + {({ close }) => ( + <> + + }>Add email address + }> + You will need to verify this address before it can be used. + + setValue(event.target.value)} + /> +
+ }>Cancel + {/* Adding is the one close that must NOT be questioned, so it clears the field the + guard reads before closing. */} + +
- {/* `finalFocus` puts the caret back in the field. Without it there is nowhere to return to — - this alert is raised by the veto rather than by a trigger — so keeping editing would - leave focus on the body, at the top of the page rather than where the work was. */} - - }>Discard changes? - }> - You have not finished adding this address. It will not be saved. - - - }>Keep editing - - - + + + )}
); } diff --git a/packages/swingset/src/stories/dialog.component.stories.tsx b/packages/swingset/src/stories/dialog.component.stories.tsx index 6dbb4001580..9ddac2b644c 100644 --- a/packages/swingset/src/stories/dialog.component.stories.tsx +++ b/packages/swingset/src/stories/dialog.component.stories.tsx @@ -112,8 +112,9 @@ const sectionHeader = { * * With `confirmDiscard`, closing it while the field holds anything opens a confirmation stacked on * top rather than closing: `panel -> prompt -> prompt`, and the veto is nothing more than a - * controlled `open` whose `onOpenChange` declines to commit. Hand-rolled here on purpose — it is - * what the `AlertDialog` and close-confirmation work is meant to replace. + * controlled `open` whose `onOpenChange` declines to commit. Hand-rolled here on purpose, to show + * that a veto needs no machinery; `AlertDialog`'s `useConfirmedClose` is the same thing packaged, + * and its page has the composed version. */ function AddValueDialog({ trigger, diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx index 31ade3e65b9..e7b53c55357 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx @@ -1,3 +1,5 @@ +import type { DialogFocusTarget } from '@clerk/headless/dialog'; +import { useDialogContext } from '@clerk/headless/dialog'; import { useRender } from '@clerk/headless/utils'; import * as stylex from '@stylexjs/stylex'; import type { ReactNode } from 'react'; @@ -6,6 +8,7 @@ import React from 'react'; import { useAccessibleDescriptionWarning } from '../../hooks/useAccessibleDescriptionWarning'; import type { MosaicComponentProps } from '../../props'; import { mergeStyleProps, themeProps } from '../../props'; +import { Button } from '../button'; import type { DialogBackdropProps, DialogCloseProps, @@ -20,8 +23,11 @@ import { Dialog } from '../dialog'; // Deep import: the part-name context and the content resolver are how one Mosaic component wraps // another and are deliberately absent from `../dialog`'s public surface. import { DialogContent, DialogPartNameContext } from '../dialog/dialog'; +import { Heading } from '../heading'; import { reset } from '../reset.styles'; +import { Text } from '../text'; import { styles } from './alert-dialog.styles'; +import { type ConfirmHandle, createConfirmHandle } from './confirm-handle'; /** * An alert dialog is a `Dialog` with three decisions already made, so the props that would make @@ -203,6 +209,67 @@ export function AlertDialog({ ); } +export interface AlertDialogConfirmProps { + /** Shared with the `show()` call, or with `useConfirmedClose`, that raises this confirmation. */ + handle: ConfirmHandle; + /** + * Where focus goes when the confirmation closes. Worth passing: the confirmation has no trigger, + * so by default there is nothing for focus to return to. Point it at the field the question was + * about and declining puts the caret back in it. + */ + finalFocus?: DialogFocusTarget; +} + +/** + * The dialog half of {@link createConfirmHandle} — an alert dialog rendered from whatever the + * `show()` call asked, and closed by answering it. + * + * Render it INSIDE the dialog it guards (anywhere in its children; outside its `Portal` is fine). + * That is what puts the two in one floating tree, which is what escape ordering, the stacking + * styles and the refcounted scroll lock all read. + */ +function Confirm({ handle, finalFocus }: AlertDialogConfirmProps) { + return ( + { + // Every close that is not the action lands here — cancel, Escape, a programmatic close — + // and they all mean no. The action settles `true` BEFORE closing, and `settle` is a no-op + // once the question is answered, so this cannot overwrite it. + if (!open) { + handle.settle(false); + } + }} + > + {({ payload }) => + payload ? ( + + + + + }>{payload.title} + }>{payload.description} + + }>{payload.cancelLabel ?? 'Cancel'} + + + + + + ) : null + } + + ); +} + /** * Compound parts. The ones an alert dialog does not change are `Dialog`'s own — same components, * not wrappers around them, so there is one implementation of each and no way for the two to @@ -220,3 +287,6 @@ AlertDialog.Title = Dialog.Title; AlertDialog.Description = Dialog.Description; AlertDialog.Close = Dialog.Close; AlertDialog.Actions = Actions; +AlertDialog.Confirm = Confirm; +/** Creates the handle pairing an awaitable `show()` with an ``. */ +AlertDialog.createConfirmHandle = createConfirmHandle; diff --git a/packages/ui/src/mosaic/components/alert-dialog/confirm-handle.ts b/packages/ui/src/mosaic/components/alert-dialog/confirm-handle.ts new file mode 100644 index 00000000000..d3e09c1b937 --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/confirm-handle.ts @@ -0,0 +1,87 @@ +import { Dialog as Primitive, type DialogHandle } from '@clerk/headless/dialog'; +import type { ReactNode } from 'react'; + +/** What a confirmation asks. Delivered to `` as the dialog's payload. */ +export interface ConfirmOptions { + title: ReactNode; + description: ReactNode; + /** Label on the confirming button. @default 'Confirm' */ + actionLabel?: ReactNode; + /** Label on the declining button. @default 'Cancel' */ + cancelLabel?: ReactNode; + /** Colours the action as destructive, for a confirmation that discards or deletes. */ + destructive?: boolean; +} + +/** + * Links a `show()` call to the `` that answers it. Create with + * {@link createConfirmHandle}; `show` is the whole public surface. + */ +export interface ConfirmHandle { + /** + * Opens the confirmation and resolves with the user's answer: `true` for the action, `false` + * for cancel or any dismissal. + * + * Calling it while a confirmation is already showing returns the IN-FLIGHT promise rather than + * opening a second one — repeated Escapes against a guarded dialog would otherwise stack + * confirmations, one per keypress. The options of the later call are ignored, since the + * question on screen is already the one being answered. + */ + show(options: ConfirmOptions): Promise; + /** The dialog handle `` mounts against. @internal */ + readonly dialog: DialogHandle; + /** + * Resolves the in-flight promise, if any. Idempotent per question: the second call for the same + * `show()` is a no-op, which is what lets the action settle `true` and then close through the + * ordinary path without the close settling `false` on top of it. @internal + */ + settle(confirmed: boolean): void; +} + +/** + * Creates a {@link ConfirmHandle}: an awaitable confirmation, in the shape of a promise rather + * than a pair of state variables and a callback. + * + * ```ts + * const confirm = createConfirmHandle(); + * if (await confirm.show({ title: 'Discard changes?', description: '…' })) { + * discard(); + * } + * ``` + * + * The dialog itself is still rendered as JSX — `` — and + * where it is rendered matters: it belongs inside the dialog it guards, so the two are in the same + * floating tree and escape ordering, the stacking styles and the refcounted scroll lock all apply. + * A confirmation mounted app-globally would be a sibling of the dialog rather than a child of it, + * and every one of those would break. + * + * Create one per guarded dialog, at module scope or in a `useMemo` — never inside the render body + * without one, since a new handle each render would orphan the promise a `show()` is waiting on. + */ +export function createConfirmHandle(): ConfirmHandle { + const dialog = Primitive.createHandle(); + let pending: { promise: Promise; resolve: (confirmed: boolean) => void } | null = null; + + return { + dialog, + show(options) { + if (pending) { + return pending.promise; + } + let resolve!: (confirmed: boolean) => void; + const promise = new Promise(res => { + resolve = res; + }); + pending = { promise, resolve }; + // The options ride along as the dialog's payload rather than being held in state out here, + // so what is on screen and what the promise resolves are the same object. + dialog.open(options); + return promise; + }, + settle(confirmed) { + const inFlight = pending; + pending = null; + inFlight?.resolve(confirmed); + }, + }; +} diff --git a/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx b/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx new file mode 100644 index 00000000000..00fbbe41262 --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx @@ -0,0 +1,291 @@ +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { Dialog } from '../dialog'; +import { AlertDialog } from './alert-dialog'; +import { createConfirmHandle } from './confirm-handle'; +import { useConfirmedClose } from './use-confirmed-close'; + +afterEach(() => cleanup()); + +const settle = () => + act(async () => { + await new Promise(resolve => setTimeout(resolve, 0)); + }); + +/** + * The target shape: a form prompt that will not close while its field holds anything, guarded by a + * confirmation stacked on it. + */ +function GuardedForm({ onClosed }: { onClosed?: () => void } = {}) { + const confirm = React.useMemo(() => createConfirmHandle(), []); + const [open, setOpen] = React.useState(true); + const [value, setValue] = React.useState(''); + const inputRef = React.useRef(null); + + const onOpenChange = useConfirmedClose({ + handle: confirm, + when: () => value.trim() !== '', + onOpenChange: next => { + setOpen(next); + if (!next) { + onClosed?.(); + } + }, + confirm: { + title: 'Discard changes?', + description: 'This address has not been saved.', + actionLabel: 'Discard', + cancelLabel: 'Keep editing', + destructive: true, + }, + }); + + return ( + + Add email address + setValue(event.target.value)} + /> + Cancel + + + ); +} + +describe('useConfirmedClose', () => { + it('closes without asking when the guard does not apply', async () => { + const user = userEvent.setup(); + render(); + + await user.keyboard('{Escape}'); + + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + it('asks instead of closing once the guard applies, and keeps the dialog open behind it', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'name@example.com'); + await user.keyboard('{Escape}'); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + // `hidden: true` because it has to be: a modal confirmation marks everything outside it inert + // and `aria-hidden`, the guarded dialog included, so the default accessible-tree query would + // report it missing when it is merely covered. That it is still mounted is the assertion. + expect(screen.getByRole('dialog', { name: 'Add email address', hidden: true })).toBeInTheDocument(); + }); + + it('renders the labels the caller asked for', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + + expect(screen.getByRole('button', { name: 'Keep editing' })).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Discard' })).toBeInTheDocument(); + }); + + it('commits the close when the action is taken', async () => { + const user = userEvent.setup(); + const onClosed = vi.fn(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Discard' })); + + await waitFor(() => expect(onClosed).toHaveBeenCalled()); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('leaves the dialog open when the confirmation is cancelled', async () => { + const user = userEvent.setup(); + const onClosed = vi.fn(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + await settle(); + + expect(onClosed).not.toHaveBeenCalled(); + expect(screen.getByRole('dialog', { name: 'Add email address' })).toBeInTheDocument(); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + // A veto still runs floating-ui's synchronous `openchange` emit, so the focus machinery fires on + // a close that never happened. Focus must not be dropped on the body. + it('keeps focus inside the dialog after a vetoed Escape', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await settle(); + + const alert = screen.getByRole('alertdialog'); + expect(alert.contains(document.activeElement)).toBe(true); + }); + + it('returns focus to the field when the confirmation is cancelled', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + await waitFor(() => expect(screen.getByRole('textbox', { name: 'Email address' })).toHaveFocus()); + }); + + // One question per answer: hammering Escape used to be able to stack a confirmation per keypress. + it('opens one confirmation for repeated close requests', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await user.keyboard('{Escape}'); + await user.keyboard('{Escape}'); + await settle(); + + expect(screen.getAllByRole('alertdialog')).toHaveLength(1); + }); + + it('covers Dialog.Close as well as Escape', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + expect(screen.getByRole('dialog', { name: 'Add email address', hidden: true })).toBeInTheDocument(); + }); + + it('can be asked again after an answer', async () => { + const user = userEvent.setup(); + render(); + const field = screen.getByRole('textbox', { name: 'Email address' }); + + await user.type(field, 'a'); + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + await settle(); + await user.keyboard('{Escape}'); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + }); +}); + +describe('createConfirmHandle', () => { + it('resolves true for the action and false for a cancel', async () => { + const user = userEvent.setup(); + const handle = createConfirmHandle(); + const answers: boolean[] = []; + + function Harness() { + return ( + + Host + + + + ); + } + render(); + + await user.click(screen.getByRole('button', { name: 'Ask' })); + await user.click(screen.getByRole('button', { name: 'Confirm' })); + await waitFor(() => expect(answers).toEqual([true])); + + await user.click(screen.getByRole('button', { name: 'Ask' })); + await user.click(screen.getByRole('button', { name: 'Cancel' })); + await waitFor(() => expect(answers).toEqual([true, false])); + }); + + it('resolves false when dismissed with Escape', async () => { + const user = userEvent.setup(); + const handle = createConfirmHandle(); + const answers: boolean[] = []; + + render( + + Host + + + , + ); + + await user.click(screen.getByRole('button', { name: 'Ask' })); + await user.keyboard('{Escape}'); + + await waitFor(() => expect(answers).toEqual([false])); + }); + + it('returns the in-flight promise rather than opening a second confirmation', async () => { + const handle = createConfirmHandle(); + render( + + Host + + , + ); + + let first!: Promise; + let second!: Promise; + await act(async () => { + first = handle.show({ title: 'First', description: 'One' }); + second = handle.show({ title: 'Second', description: 'Two' }); + }); + + expect(second).toBe(first); + expect(screen.getByRole('alertdialog', { name: 'First' })).toBeInTheDocument(); + }); + + it('stacks over the dialog it guards rather than replacing it', async () => { + const user = userEvent.setup(); + render(); + + await user.type(screen.getByRole('textbox', { name: 'Email address' }), 'a'); + await user.keyboard('{Escape}'); + await settle(); + + const popups = document.querySelectorAll('.cl-dialog-popup'); + expect(popups).toHaveLength(2); + expect(popups[0]).toHaveAttribute('data-stack-base', ''); + expect(popups[1]).toHaveAttribute('data-stacked', ''); + }); +}); diff --git a/packages/ui/src/mosaic/components/alert-dialog/index.ts b/packages/ui/src/mosaic/components/alert-dialog/index.ts index 4c16d774290..8acec07aa7e 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/index.ts +++ b/packages/ui/src/mosaic/components/alert-dialog/index.ts @@ -1,8 +1,13 @@ export { AlertDialog } from './alert-dialog'; +export { createConfirmHandle } from './confirm-handle'; +export type { ConfirmHandle, ConfirmOptions } from './confirm-handle'; +export { useConfirmedClose } from './use-confirmed-close'; +export type { UseConfirmedCloseOptions } from './use-confirmed-close'; export type { AlertDialogActionsProps, AlertDialogBackdropProps, AlertDialogCloseProps, + AlertDialogConfirmProps, AlertDialogDescriptionProps, AlertDialogPopupProps, AlertDialogProps, diff --git a/packages/ui/src/mosaic/components/alert-dialog/use-confirmed-close.ts b/packages/ui/src/mosaic/components/alert-dialog/use-confirmed-close.ts new file mode 100644 index 00000000000..f29f718e595 --- /dev/null +++ b/packages/ui/src/mosaic/components/alert-dialog/use-confirmed-close.ts @@ -0,0 +1,76 @@ +import type { DialogOpenChangeDetails } from '@clerk/headless/dialog'; +import React from 'react'; + +import type { ConfirmHandle, ConfirmOptions } from './confirm-handle'; + +export interface UseConfirmedCloseOptions { + /** The handle shared with the `` rendered inside the guarded dialog. */ + handle: ConfirmHandle; + /** + * Whether closing needs confirming, evaluated at the moment of each close request — typically + * "is this form dirty". Return `false` and the close commits immediately, with no interruption. + */ + when: () => boolean; + /** Commits the open state. The guarded dialog's own `onOpenChange`, usually a `setOpen`. */ + onOpenChange: (open: boolean, details: DialogOpenChangeDetails) => void; + /** What to ask when `when()` returns true. */ + confirm: ConfirmOptions; +} + +/** A close the user never asked for: the one this hook commits after the action is confirmed. */ +const PROGRAMMATIC_DETAILS: DialogOpenChangeDetails = { trigger: null, triggerId: null, event: undefined }; + +/** + * Guards a dialog's close behind a confirmation, returning the `onOpenChange` to hand it. + * + * ```tsx + * const onOpenChange = useConfirmedClose({ + * handle: confirm, + * when: () => value !== '', + * onOpenChange: setOpen, + * confirm: { title: 'Discard changes?', description: '…', actionLabel: 'Discard' }, + * }); + * + * + * … + * + * + * ``` + * + * **The dialog must be controlled.** A veto is the absence of a commit, and an uncontrolled dialog + * has already committed internally by the time `onOpenChange` runs — there would be nothing left to + * decline. + * + * **What it covers is every close the dialog itself owns**: Escape, an outside press where + * `closedBy` allows one, `Dialog.CloseButton`, `Dialog.Close`, and the `close` the `Dialog` wrapper + * hands its children. All of them funnel through `onOpenChange`, so one branch here answers them + * all. What it cannot cover is a button wired to your own `setOpen(false)` — that never reaches the + * dialog, so it bypasses the question silently. Route those through `Dialog.Close` instead. + */ +export function useConfirmedClose({ handle, when, onOpenChange, confirm }: UseConfirmedCloseOptions) { + // Read through a ref so the returned callback is stable across renders: it is handed to a + // dialog that would otherwise see a new `onOpenChange` identity on every keystroke of the very + // form whose dirtiness `when` is reporting on. + const latest = React.useRef({ when, onOpenChange, confirm }); + React.useLayoutEffect(() => { + latest.current = { when, onOpenChange, confirm }; + }); + + return React.useCallback( + (open: boolean, details: DialogOpenChangeDetails) => { + if (open || !latest.current.when()) { + latest.current.onOpenChange(open, details); + return; + } + // The veto: return without committing, so the dialog stays open behind the question. The + // close is re-issued from here only if the answer is yes — and it is a fresh close rather + // than the original, whose event belongs to an interaction that has long since finished. + void handle.show(latest.current.confirm).then(confirmed => { + if (confirmed) { + latest.current.onOpenChange(false, PROGRAMMATIC_DETAILS); + } + }); + }, + [handle], + ); +} diff --git a/packages/ui/src/mosaic/styles/index.ts b/packages/ui/src/mosaic/styles/index.ts index da5534b2bc2..07e1dbe5903 100644 --- a/packages/ui/src/mosaic/styles/index.ts +++ b/packages/ui/src/mosaic/styles/index.ts @@ -6,11 +6,15 @@ export type { MosaicComponentProps, MosaicElementProps } from '../props'; -export { AlertDialog } from '../components/alert-dialog'; +export { AlertDialog, createConfirmHandle, useConfirmedClose } from '../components/alert-dialog'; export type { + ConfirmHandle, + ConfirmOptions, + UseConfirmedCloseOptions, AlertDialogActionsProps, AlertDialogBackdropProps, AlertDialogCloseProps, + AlertDialogConfirmProps, AlertDialogDescriptionProps, AlertDialogPopupProps, AlertDialogProps, From e08c4836eebea750a5ad74719565e856372ec8e6 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 13:42:38 -0600 Subject: [PATCH 16/18] fix(ui): address review feedback on #9439 Let an explicit `handle.open(payload)` win over the stale `activeTriggerId` lookup, keep the payload published through the exit transition so a `handle.close()` no longer blanks children-as-function content, and settle an in-flight question `false` when `` unmounts rather than poisoning the handle. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/primitives/dialog/dialog-root.tsx | 23 ++++++++-- .../src/primitives/dialog/dialog.test.tsx | 46 +++++++++++++++++++ .../src/stories/alert-dialog.component.mdx | 5 ++ .../components/alert-dialog/alert-dialog.tsx | 7 ++- .../components/alert-dialog/confirm-handle.ts | 6 +++ .../components/alert-dialog/confirm.test.tsx | 30 ++++++++++++ 6 files changed, 113 insertions(+), 4 deletions(-) diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index 7ae9f947531..c3b1d077097 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -141,9 +141,16 @@ function DialogInner(props: DialogProps & { isNested: boolean setOpen: (nextOpen, payload) => { // Held in a ref as well as in state because the payload effect below re-runs on `open` // and would otherwise resolve a trigger-less open to `undefined`, wiping this a commit - // after it was set. + // after it was set. Cleared on close as well, so the next payload-less open does not + // resurface this one. directPayloadRef.current = payload; - setActivePayload(payload); + // Published only on the way IN. A close carries no payload, and writing it would blank + // the children-as-function while the popup is still mounted for its exit transition — + // rendering the dialog empty as it leaves, or throwing in a consumer that dereferences + // the payload. The next open sets it afresh. + if (nextOpen) { + setActivePayload(payload); + } floatingContext.onOpenChange(nextOpen); }, }); @@ -169,10 +176,20 @@ function DialogInner(props: DialogProps & { isNested: boolean // `defaultOpen` — the payload is looked up from the registry once the dialog is open. Runs // after the children's layout effects, so triggers rendered inside the root are registered by // the time it reads, and the pre-paint re-render delivers their payload on the first frame. + // + // An explicit programmatic payload wins over the registry lookup: `activeTriggerId` is never + // reset on close, so once any trigger has opened the dialog the lookup would otherwise overwrite + // every later `handle.open(payload)` with that trigger's payload. The mirror already holds — + // `openFromTrigger` clears `directPayloadRef`, so a trigger wins the other way. useLayoutEffect(() => { if (open) { + const direct = directPayloadRef.current; setActivePayload( - activeTriggerId != null ? store.getTrigger(activeTriggerId)?.getPayload() : directPayloadRef.current, + direct !== undefined + ? direct + : activeTriggerId != null + ? store.getTrigger(activeTriggerId)?.getPayload() + : undefined, ); } }, [store, open, activeTriggerId]); diff --git a/packages/headless/src/primitives/dialog/dialog.test.tsx b/packages/headless/src/primitives/dialog/dialog.test.tsx index 51f65891232..996a3bd05c5 100644 --- a/packages/headless/src/primitives/dialog/dialog.test.tsx +++ b/packages/headless/src/primitives/dialog/dialog.test.tsx @@ -539,6 +539,12 @@ describe('Dialog', () => { {({ payload }) => ( <> Open A + + Open B + @@ -584,6 +590,46 @@ describe('Dialog', () => { expect(screen.getByRole('dialog', { name: 'no payload' })).toBeInTheDocument(); }); + + it('supersedes the trigger that opened the dialog last', async () => { + const user = userEvent.setup(); + const handle = renderDetached(); + + // `activeTriggerId` is never reset on close, so without an explicit precedence the + // registry lookup would hand this open the previous trigger's payload back. + await user.click(screen.getByRole('button', { name: 'Open B' })); + act(() => handle.close()); + act(() => handle.open('from-handle')); + await act(async () => { + await Promise.resolve(); + }); + + expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument(); + }); + + it('survives a `handle.close()` for the length of the exit transition', () => { + // Keep an animation pending so the popup stays mounted after the close. + const original = (Element.prototype as { getAnimations?: unknown }).getAnimations; + (Element.prototype as { getAnimations?: unknown }).getAnimations = () => [ + { finished: new Promise(() => {}) }, + ]; + try { + const handle = renderDetached(); + + act(() => handle.open('from-handle')); + act(() => handle.close()); + + // `close()` carries no payload; blanking it here would render the dialog empty on its + // way out, or throw in a children function that dereferences it. + expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument(); + } finally { + if (original) { + (Element.prototype as { getAnimations?: unknown }).getAnimations = original; + } else { + delete (Element.prototype as { getAnimations?: unknown }).getAnimations; + } + } + }); }); }); diff --git a/packages/swingset/src/stories/alert-dialog.component.mdx b/packages/swingset/src/stories/alert-dialog.component.mdx index 3a25e811c76..03500a4ed12 100644 --- a/packages/swingset/src/stories/alert-dialog.component.mdx +++ b/packages/swingset/src/stories/alert-dialog.component.mdx @@ -138,6 +138,11 @@ It resolves `true` for the action and `false` for cancel or any dismissal. Calli confirmation is already showing returns the in-flight promise rather than opening a second one, so repeated close requests ask once. +**`AlertDialog.Confirm` must be mounted when `show()` is called** — it is the thing that opens, and +a `show()` with nothing mounted to answer it never resolves. Since the confirmation lives inside the +dialog it guards, that means asking from inside that dialog, while it is open. A confirmation that +unmounts with a question in flight answers `false` rather than leaving the `await` hanging. + ## Parts | Part | Slot | Description | diff --git a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx index e7b53c55357..ad4ae4889f4 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx +++ b/packages/ui/src/mosaic/components/alert-dialog/alert-dialog.tsx @@ -1,5 +1,4 @@ import type { DialogFocusTarget } from '@clerk/headless/dialog'; -import { useDialogContext } from '@clerk/headless/dialog'; import { useRender } from '@clerk/headless/utils'; import * as stylex from '@stylexjs/stylex'; import type { ReactNode } from 'react'; @@ -229,6 +228,12 @@ export interface AlertDialogConfirmProps { * styles and the refcounted scroll lock all read. */ function Confirm({ handle, finalFocus }: AlertDialogConfirmProps) { + // A question can only be answered while the thing that asks it is on screen. Going away with one + // in flight would leave the promise unresolved forever, and `show()` short-circuits on an + // in-flight question — so the handle would never open a confirmation again, and a guarded dialog + // whose closes route through one could no longer be closed at all. + React.useEffect(() => () => handle.settle(false), [handle]); + return ( ` must be MOUNTED when this is called. It is what opens, and a + * `dialog.open()` with no root attached is a no-op — the promise would never settle. Since the + * confirmation belongs inside the dialog it guards, that means asking only from inside that + * dialog while it is open. A confirmation that unmounts with a question in flight answers + * `false` rather than hanging. */ show(options: ConfirmOptions): Promise; /** The dialog handle `` mounts against. @internal */ diff --git a/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx b/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx index 00fbbe41262..4e242878a7d 100644 --- a/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx +++ b/packages/ui/src/mosaic/components/alert-dialog/confirm.test.tsx @@ -275,6 +275,36 @@ describe('createConfirmHandle', () => { expect(screen.getByRole('alertdialog', { name: 'First' })).toBeInTheDocument(); }); + // An unanswered question poisons the handle: `show()` short-circuits on the in-flight promise, + // so the confirmation would never open again and a guarded dialog could no longer be closed. + it('answers false when the confirmation unmounts with a question in flight', async () => { + const handle = createConfirmHandle(); + const answers: boolean[] = []; + + function Harness({ mounted }: { mounted: boolean }) { + return ( + + Host + {mounted ? : null} + + ); + } + const { rerender } = render(); + + await act(async () => { + void handle.show({ title: 'Sure?', description: 'No going back.' }).then(a => answers.push(a)); + }); + rerender(); + + await waitFor(() => expect(answers).toEqual([false])); + // And the handle is usable again rather than stuck on the dead promise. + rerender(); + await act(async () => { + void handle.show({ title: 'Again?', description: 'Still no going back.' }).then(a => answers.push(a)); + }); + expect(screen.getByRole('alertdialog', { name: 'Again?' })).toBeInTheDocument(); + }); + it('stacks over the dialog it guards rather than replacing it', async () => { const user = userEvent.setup(); render(); From 41b664226ce9a6db4f29f15d46795599d61e883c Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 10:03:48 -0600 Subject: [PATCH 17/18] refactor(ui): adopt the dialog close confirmation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two dialogs that could throw work away, treated differently, because they are different problems wearing the same description. The org-profile edit form now guards its close: edits raise "Discard changes?" instead of vanishing. The predicate is the machine's own `dataChanged`, exported as `hasUnsavedEdits` rather than restated in the view, so the guard and the SUBMIT guard cannot come to disagree about what an edit is. Typing a value and undoing it therefore closes without a question, since nothing changed. Its Cancel button was the live instance of the bypass the new hook's docs warn about: wired to a bare `send({ type: 'CANCEL' })`, it went around `onOpenChange` — so it was the one way out that discarded edits without asking, including after this change. It is now a `Dialog.Close`, which funnels. `Destructive` gets no confirmation, and that is the point. Its remaining bug is closing mid-delete, and a confirmation is the wrong shape for it: the request cannot be called back, so there is no answer that changes anything. It is simply not dismissible while the delete is in flight (`closedBy='none'`), and `closerequest` the rest of the time, as before. Asking "discard your typed text?" on the way out of a confirmation dialog would be a confirmation to escape a confirmation. Neither dialog asks while its request is in flight: `CANCEL` is not a transition the `saving` state accepts either, so a question there would be one whose answer changes nothing. --- .changeset/adopt-close-confirmation.md | 2 + packages/ui/src/mosaic/block/destructive.tsx | 6 +- ...ation-profile-delete-section.view.test.tsx | 32 +++++ ...tion-profile-profile-section.view.test.tsx | 136 ++++++++++++++++++ ...profile-profile-section-details.machine.ts | 7 + ...anization-profile-profile-section.view.tsx | 47 +++++- 6 files changed, 223 insertions(+), 7 deletions(-) create mode 100644 .changeset/adopt-close-confirmation.md create mode 100644 packages/ui/src/mosaic/organization/__tests__/organization-profile-profile-section.view.test.tsx diff --git a/.changeset/adopt-close-confirmation.md b/.changeset/adopt-close-confirmation.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/adopt-close-confirmation.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/packages/ui/src/mosaic/block/destructive.tsx b/packages/ui/src/mosaic/block/destructive.tsx index 73a4d24f381..f854cd983f8 100644 --- a/packages/ui/src/mosaic/block/destructive.tsx +++ b/packages/ui/src/mosaic/block/destructive.tsx @@ -56,7 +56,11 @@ export function Destructive({ return ( { expect(screen.getByText('Delete failed')).toBeInTheDocument(); }); + // The delete request cannot be called back, so nothing may dismiss the dialog while it is in + // flight — closing would leave it running behind a surface that is gone. Asserted on the event + // rather than on the dialog going away: `open` is controlled from the machine, which is mocked + // here, so the surface stays either way and only the request to close distinguishes them. + it('does not request a close while the delete is in flight', async () => { + const user = userEvent.setup(); + const { send } = renderView( + snapshot({ + value: 'deleting', + context: { + organizationName: 'Acme Inc', + confirmationValue: 'Acme Inc', + destroyOrganization: async () => {}, + error: null, + }, + }), + ); + + await user.keyboard('{Escape}'); + + expect(send).not.toHaveBeenCalledWith({ type: 'CANCEL' }); + }); + + it('still dismisses with Escape before the delete starts', async () => { + const user = userEvent.setup(); + const { send } = renderView(snapshot()); + + await user.keyboard('{Escape}'); + + expect(send).toHaveBeenCalledWith({ type: 'CANCEL' }); + }); }); diff --git a/packages/ui/src/mosaic/organization/__tests__/organization-profile-profile-section.view.test.tsx b/packages/ui/src/mosaic/organization/__tests__/organization-profile-profile-section.view.test.tsx new file mode 100644 index 00000000000..927e2588337 --- /dev/null +++ b/packages/ui/src/mosaic/organization/__tests__/organization-profile-profile-section.view.test.tsx @@ -0,0 +1,136 @@ +import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import type { Snapshot } from '../../machine/types'; +import { MosaicProvider } from '../../MosaicProvider'; +import { OrganizationProfileProfileSectionView } from '../organization-profile-profile-section.view'; +import type { OrganizationProfileProfileSectionDetailsContext } from '../organization-profile-profile-section-details.machine'; + +afterEach(() => cleanup()); + +function snapshot( + context: Partial = {}, + value = 'editing', +): Snapshot { + return { + value, + status: 'active', + context: { + committedName: 'Acme Inc', + committedSlug: 'acme', + slugEnabled: true, + draftName: null, + draftSlug: null, + error: null, + updateOrganization: async () => {}, + ...context, + }, + } as Snapshot; +} + +function renderView( + snap: Snapshot = snapshot(), + send = vi.fn(), + canSubmit = true, +) { + render( + + + , + ); + return { send }; +} + +const edited = snapshot({ draftName: 'Acme Incorporated' }); + +describe('OrganizationProfileProfileSectionView — discarding edits', () => { + it('closes without asking when nothing has been edited', async () => { + const user = userEvent.setup(); + const { send } = renderView(); + + await user.keyboard('{Escape}'); + + expect(send).toHaveBeenCalledWith({ type: 'CANCEL' }); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + it('asks before discarding edits, and does not close meanwhile', async () => { + const user = userEvent.setup(); + const { send } = renderView(edited); + + await user.keyboard('{Escape}'); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + expect(send).not.toHaveBeenCalledWith({ type: 'CANCEL' }); + }); + + it('discards once confirmed', async () => { + const user = userEvent.setup(); + const { send } = renderView(edited); + + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Discard' })); + + await waitFor(() => expect(send).toHaveBeenCalledWith({ type: 'CANCEL' })); + }); + + it('keeps the edits when the confirmation is declined', async () => { + const user = userEvent.setup(); + const { send } = renderView(edited); + + await user.keyboard('{Escape}'); + await user.click(screen.getByRole('button', { name: 'Keep editing' })); + + expect(send).not.toHaveBeenCalledWith({ type: 'CANCEL' }); + await waitFor(() => expect(screen.getByRole('dialog', { name: 'Update profile' })).toBeInTheDocument()); + }); + + // The Cancel button used to send `CANCEL` itself, which went around the guard entirely — the one + // way out that discarded the edits without asking. + it('asks when the Cancel button is pressed with edits pending', async () => { + const user = userEvent.setup(); + const { send } = renderView(edited); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + expect(send).not.toHaveBeenCalledWith({ type: 'CANCEL' }); + }); + + it('asks about a slug edit as well as a name edit', async () => { + const user = userEvent.setup(); + renderView(snapshot({ draftSlug: 'acme-inc' })); + + await user.keyboard('{Escape}'); + + expect(screen.getByRole('alertdialog', { name: 'Discard changes?' })).toBeInTheDocument(); + }); + + // Typing and then undoing leaves a draft that is no longer a change; the guard follows the + // machine's own definition of an edit rather than "has been touched". + it('does not ask when the draft matches what is committed', async () => { + const user = userEvent.setup(); + const { send } = renderView(snapshot({ draftName: 'Acme Inc' })); + + await user.keyboard('{Escape}'); + + expect(send).toHaveBeenCalledWith({ type: 'CANCEL' }); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); + + // `CANCEL` is not a transition `saving` accepts, so the dialog stays open regardless: a question + // whose answer changes nothing is worse than no question. + it('does not ask while saving', async () => { + const user = userEvent.setup(); + renderView(snapshot({ draftName: 'Acme Incorporated' }, 'saving')); + + await user.keyboard('{Escape}'); + + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); + }); +}); diff --git a/packages/ui/src/mosaic/organization/organization-profile-profile-section-details.machine.ts b/packages/ui/src/mosaic/organization/organization-profile-profile-section-details.machine.ts index 7837db23228..fba27ec058e 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-profile-section-details.machine.ts +++ b/packages/ui/src/mosaic/organization/organization-profile-profile-section-details.machine.ts @@ -35,6 +35,13 @@ const dataChanged = (context: OrganizationProfileProfileSectionDetailsContext): effectiveName(context) !== context.committedName || (context.slugEnabled && effectiveSlug(context) !== context.committedSlug); +/** + * Whether closing would throw away work. Exported so the view can ask before letting a close + * through, while the definition of "changed" stays here with the guard that already depends on it — + * the two must not be able to disagree about what an edit is. + */ +export const hasUnsavedEdits = dataChanged; + const canSave = (context: OrganizationProfileProfileSectionDetailsContext): boolean => dataChanged(context) && effectiveName(context).trim() !== ''; diff --git a/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx index 0ae9df542f1..80753fa91d1 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx @@ -1,5 +1,7 @@ import type { FormEvent } from 'react'; +import { useMemo, useRef } from 'react'; +import { AlertDialog, createConfirmHandle, useConfirmedClose } from '../components/alert-dialog'; import { Box } from '../components/box'; import { Button } from '../components/button'; import { Dialog } from '../components/dialog'; @@ -10,6 +12,7 @@ import type { OrganizationProfileProfileSectionDetailsContext, OrganizationProfileProfileSectionDetailsEvent, } from './organization-profile-profile-section-details.machine'; +import { hasUnsavedEdits } from './organization-profile-profile-section-details.machine'; interface OrganizationProfileProfileSectionViewProps { snapshot: Snapshot; @@ -30,6 +33,27 @@ export function OrganizationProfileProfileSectionView({ const nameValue = draftName ?? committedName; const slugValue = draftSlug ?? committedSlug; + const confirm = useMemo(() => createConfirmHandle(), []); + const nameInputRef = useRef(null); + + // Closing this form used to discard the edits silently — Escape, or the corner X, and the typing + // was gone. Every close the dialog owns funnels through here, so one guard covers them all. + // + // Not asked while saving: `CANCEL` is not a transition the `saving` state accepts, so the dialog + // stays open regardless, and a question whose answer changes nothing is worse than no question. + const onOpenChange = useConfirmedClose({ + handle: confirm, + when: () => !isSaving && hasUnsavedEdits(snapshot.context), + onOpenChange: open => send({ type: open ? 'OPEN' : 'CANCEL' }), + confirm: { + title: 'Discard changes?', + description: 'The edits to this profile have not been saved.', + actionLabel: 'Discard', + cancelLabel: 'Keep editing', + destructive: true, + }, + }); + const handleSubmit = (event: FormEvent) => { event.preventDefault(); if (canSubmit) { @@ -82,7 +106,7 @@ export function OrganizationProfileProfileSectionView({ send({ type: open ? 'OPEN' : 'CANCEL' })} + onOpenChange={onOpenChange} trigger={props => ( + From 0a2373f075d8b69c458de17ca9270bfc0c0df210 Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Thu, 13 Aug 2026 13:45:20 -0600 Subject: [PATCH 18/18] fix(ui): address review feedback on #9441 Disable `Destructive`'s Cancel while the delete is in flight, so the block enforces the no-dismissal invariant its `closedBy` comment states instead of relying on each consumer's machine to drop the event. Correct a comment that named a corner X this dialog does not render. Co-Authored-By: Claude Opus 5 (1M context) --- packages/ui/src/mosaic/block/destructive.tsx | 4 ++++ .../organization-profile-profile-section.view.tsx | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/ui/src/mosaic/block/destructive.tsx b/packages/ui/src/mosaic/block/destructive.tsx index f854cd983f8..9ccb0307017 100644 --- a/packages/ui/src/mosaic/block/destructive.tsx +++ b/packages/ui/src/mosaic/block/destructive.tsx @@ -108,6 +108,10 @@ export function Destructive({ diff --git a/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx index 80753fa91d1..46cd747b902 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-profile-section.view.tsx @@ -36,8 +36,8 @@ export function OrganizationProfileProfileSectionView({ const confirm = useMemo(() => createConfirmHandle(), []); const nameInputRef = useRef(null); - // Closing this form used to discard the edits silently — Escape, or the corner X, and the typing - // was gone. Every close the dialog owns funnels through here, so one guard covers them all. + // Closing this form used to discard the edits silently — Escape, or Cancel, and the typing was + // gone. Every close the dialog owns funnels through here, so one guard covers them all. // // Not asked while saving: `CANCEL` is not a transition the `saving` state accepts, so the dialog // stays open regardless, and a question whose answer changes nothing is worse than no question.