From 38e7c1f31731a88c59914ca27294ac5cee20906f Mon Sep 17 00:00:00 2001 From: Max Yinger Date: Fri, 7 Aug 2026 15:15:36 -0600 Subject: [PATCH] feat(ui): add dialog closedBy dismissal policy Adds `closedBy: 'any' | 'closerequest' | 'none'` to the headless Dialog root, driving `escapeKey` and `outsidePress` on `useDismiss`. Defaults to `any`, so existing callers are unaffected. The five Mosaic dialogs now use `closerequest`, which stops a stray backdrop click from discarding the type-to-confirm input in `Destructive` or closing a dialog mid-request. `trigger` on the Mosaic `Dialog` becomes optional, so the machine-driven dialogs no longer render a button they don't use. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/lucky-donuts-invite.md | 7 +++ .../headless/src/primitives/dialog/README.md | 35 +++++++++-- .../src/primitives/dialog/dialog-root.tsx | 19 +++++- .../src/primitives/dialog/dialog.test.tsx | 60 +++++++++++++++++++ .../headless/src/primitives/dialog/index.ts | 1 + .../headless/src/primitives/dialog/parts.ts | 2 +- packages/swingset/src/stories/dialog.mdx | 35 +++++++++-- .../swingset/src/stories/dialog.stories.tsx | 5 +- packages/ui/src/mosaic/block/destructive.tsx | 1 + packages/ui/src/mosaic/components/dialog.tsx | 16 +++-- ...rofile-domains-section-add-verify.view.tsx | 1 + ...rofile-domains-section-enrollment.view.tsx | 1 + ...on-profile-domains-section-remove.view.tsx | 1 + ...anization-profile-profile-section.view.tsx | 1 + 14 files changed, 165 insertions(+), 20 deletions(-) create mode 100644 .changeset/lucky-donuts-invite.md diff --git a/.changeset/lucky-donuts-invite.md b/.changeset/lucky-donuts-invite.md new file mode 100644 index 00000000000..c45a0eafe26 --- /dev/null +++ b/.changeset/lucky-donuts-invite.md @@ -0,0 +1,7 @@ +--- +'@clerk/ui': patch +--- + +Add a `closedBy` prop to the Mosaic `Dialog`, controlling which gestures dismiss it: `any` (Escape and outside press, the default), `closerequest` (Escape only), or `none` (neither — the dialog closes only programmatically). Use `closerequest` for dialogs holding user input so a stray backdrop click cannot discard it. + +The `trigger` prop is now optional, so a dialog driven entirely by `open` no longer has to render a trigger button it does not need. diff --git a/packages/headless/src/primitives/dialog/README.md b/packages/headless/src/primitives/dialog/README.md index 43a3a6614e0..3ae2a9f1f76 100644 --- a/packages/headless/src/primitives/dialog/README.md +++ b/packages/headless/src/primitives/dialog/README.md @@ -63,12 +63,35 @@ const [open, setOpen] = useState(false); ### `Dialog.Root` -| Prop | Type | Default | Description | -| -------------- | ------------------------- | ------- | --------------------------------------- | -| `open` | `boolean` | — | Controlled open state | -| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | -| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | -| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | +| Prop | Type | Default | Description | +| -------------- | ----------------------------------- | ------- | --------------------------------------- | +| `open` | `boolean` | — | Controlled open state | +| `defaultOpen` | `boolean` | `false` | Initial open state (uncontrolled) | +| `onOpenChange` | `(open: boolean) => void` | — | Called when open state changes | +| `modal` | `boolean` | `true` | Traps focus and blocks page interaction | +| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog | + +#### `closedBy` + +Mirrors the native `` attribute. + +| Value | Escape | Outside press | Programmatic | +| -------------- | ------ | ------------- | ------------ | +| `any` | ✅ | ✅ | ✅ | +| `closerequest` | ✅ | ❌ | ✅ | +| `none` | ❌ | ❌ | ✅ | + +```tsx +// A form dialog: Escape backs out, a stray backdrop click doesn't discard input. +{/* ... */} +``` + +Reach for `closerequest` on anything holding user input or confirming a destructive action. +Reserve `none` for flows the user genuinely must complete or explicitly acknowledge — it removes +the keyboard exit, so it fails the usual expectation that Escape dismisses a modal. + +A single ordered enum rather than two booleans: it keeps the fourth combination — outside press +dismisses but Escape does not — unrepresentable. ### `Dialog.Portal` diff --git a/packages/headless/src/primitives/dialog/dialog-root.tsx b/packages/headless/src/primitives/dialog/dialog-root.tsx index 471029c7956..4b5ba8bdaa9 100644 --- a/packages/headless/src/primitives/dialog/dialog-root.tsx +++ b/packages/headless/src/primitives/dialog/dialog-root.tsx @@ -18,18 +18,33 @@ import { useReturnFocus } from '../../hooks/use-return-focus'; import { useTransition } from '../../hooks/use-transition'; import { DialogContext, type DialogContextValue } from './dialog-context'; +/** + * Which gestures dismiss the dialog, mirroring the native `` attribute. + * + * - `any` — Escape and outside press + * - `closerequest` — Escape only + * - `none` — neither; the dialog closes only programmatically + * + * A single ordered enum rather than two booleans, so the fourth combination — outside press + * dismisses but Escape does not — stays unrepresentable. Dismissing by pointer but not by + * keyboard is not something to offer. + */ +export type DialogClosedBy = 'any' | 'closerequest' | 'none'; + export interface DialogProps { open?: boolean; defaultOpen?: boolean; onOpenChange?: (open: boolean) => void; /** When true, the dialog traps focus and blocks interaction with the rest of the page. Default: true */ modal?: boolean; + /** Which gestures dismiss the dialog. Default: `any` */ + closedBy?: DialogClosedBy; children: ReactNode; } function DialogInner(props: DialogProps) { const nodeId = useFloatingNodeId(); - const { modal = true, children } = props; + const { modal = true, closedBy = 'any', children } = props; const [open, setOpen] = useControllableState(props.open, props.defaultOpen ?? false, props.onOpenChange); @@ -54,6 +69,8 @@ function DialogInner(props: DialogProps) { const click = useClick(floatingContext); const dismiss = useDismiss(floatingContext, { outsidePressEvent: 'mousedown', + escapeKey: closedBy !== 'none', + outsidePress: closedBy === 'any', }); const role = useRole(floatingContext); diff --git a/packages/headless/src/primitives/dialog/dialog.test.tsx b/packages/headless/src/primitives/dialog/dialog.test.tsx index 06011b50342..fc40964b7d5 100644 --- a/packages/headless/src/primitives/dialog/dialog.test.tsx +++ b/packages/headless/src/primitives/dialog/dialog.test.tsx @@ -271,6 +271,66 @@ describe('Dialog', () => { }); }); + describe('closedBy', () => { + // The viewport is the element outside the popup that a light dismiss lands on. + const pressOutside = async (user: ReturnType) => + user.click(screen.getByTestId('dialog-viewport')); + + it('defaults to dismissing on both Escape and outside press', async () => { + const user = userEvent.setup(); + renderDialog({ defaultOpen: true }); + + await pressOutside(user); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Open dialog' })); + await user.keyboard('{Escape}'); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('closerequest dismisses on Escape but not outside press', async () => { + const user = userEvent.setup(); + renderDialog({ defaultOpen: true, closedBy: 'closerequest' }); + + await pressOutside(user); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + + await user.keyboard('{Escape}'); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('none dismisses on neither', async () => { + const user = userEvent.setup(); + renderDialog({ defaultOpen: true, closedBy: 'none' }); + + await pressOutside(user); + await user.keyboard('{Escape}'); + + expect(screen.getByRole('dialog')).toBeInTheDocument(); + }); + + it('leaves the Close button working regardless of closedBy', async () => { + const user = userEvent.setup(); + renderDialog({ defaultOpen: true, closedBy: 'none' }); + + await user.click(screen.getByRole('button', { name: 'Close' })); + + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + }); + + it('leaves controlled open authoritative regardless of closedBy', async () => { + const onOpenChange = vi.fn(); + const user = userEvent.setup(); + renderDialog({ open: true, closedBy: 'none', onOpenChange }); + + await pressOutside(user); + await user.keyboard('{Escape}'); + + expect(onOpenChange).not.toHaveBeenCalled(); + expect(screen.getByRole('dialog')).toBeInTheDocument(); + }); + }); + describe('focus management', () => { it('moves focus into dialog on open', async () => { const user = userEvent.setup(); diff --git a/packages/headless/src/primitives/dialog/index.ts b/packages/headless/src/primitives/dialog/index.ts index 56c9fa1d5fa..7233056a816 100644 --- a/packages/headless/src/primitives/dialog/index.ts +++ b/packages/headless/src/primitives/dialog/index.ts @@ -5,6 +5,7 @@ export type { DialogContextValue } from './dialog-context'; export type { DialogBackdropProps, + DialogClosedBy, DialogCloseProps, DialogDescriptionProps, DialogPopupProps, diff --git a/packages/headless/src/primitives/dialog/parts.ts b/packages/headless/src/primitives/dialog/parts.ts index 7fc3352bcb9..527f8e4a357 100644 --- a/packages/headless/src/primitives/dialog/parts.ts +++ b/packages/headless/src/primitives/dialog/parts.ts @@ -1,4 +1,4 @@ -export { type DialogProps, DialogRoot as Root } from './dialog-root'; +export { type DialogClosedBy, type DialogProps, DialogRoot as Root } from './dialog-root'; export { type DialogTriggerProps, DialogTrigger as Trigger } from './dialog-trigger'; export { type DialogPortalProps, DialogPortal as Portal } from './dialog-portal'; export { type DialogBackdropProps, DialogBackdrop as Backdrop } from './dialog-backdrop'; diff --git a/packages/swingset/src/stories/dialog.mdx b/packages/swingset/src/stories/dialog.mdx index 23db48b9821..f30c9f4c96b 100644 --- a/packages/swingset/src/stories/dialog.mdx +++ b/packages/swingset/src/stories/dialog.mdx @@ -57,6 +57,17 @@ const [open, setOpen] = useState(false); {/* focus is not trapped and the rest of the page stays interactive */} ``` +### Controlling dismissal + +```tsx +// Escape still backs out, but a stray click on the backdrop won't discard the form. +{/* trigger + portal */} +``` + +Reach for `closerequest` on anything holding user input or confirming a destructive action. +Reserve `none` for flows the user must complete or explicitly acknowledge — it removes the +keyboard exit, so it breaks the usual expectation that Escape dismisses a modal. + ## Parts | Part | Default Element | Description | @@ -80,12 +91,24 @@ for centered, scroll-locked modal behavior nest `Dialog.Popup` inside `Dialog.Vi ### `Dialog.Root` -| Prop | Type | Default | Description | -| ------------------------- | ------------------------------------ | ------------------ | ---------------------------------------------- | -| open | boolean | — | Controlled open state | -| defaultOpen | boolean | false | Initial open state (uncontrolled) | -| onOpenChange | (open: boolean) => void | — | Called when the open state changes | -| modal | boolean | true | Trap focus and make the rest of the page inert | +| Prop | Type | Default | Description | +| ------------------------- | ---------------------------------------------- | ------------------ | ---------------------------------------------- | +| open | boolean | — | Controlled open state | +| defaultOpen | boolean | false | Initial open state (uncontrolled) | +| onOpenChange | (open: boolean) => void | — | Called when the open state changes | +| modal | boolean | true | Trap focus and make the rest of the page inert | +| closedBy | 'any' \| 'closerequest' \| 'none' | 'any' | Which gestures dismiss the dialog | + +closedBy mirrors the native <dialog closedby> attribute: + +| Value | Escape | Outside press | Programmatic | +| ------------------------- | ------ | ------------- | ------------ | +| any | yes | yes | yes | +| closerequest | yes | no | yes | +| none | no | no | yes | + +A single ordered enum rather than two booleans, so the fourth combination — outside press +dismisses but Escape does not — stays unrepresentable. ### `Dialog.Portal` diff --git a/packages/swingset/src/stories/dialog.stories.tsx b/packages/swingset/src/stories/dialog.stories.tsx index c1b07583877..e801c2fca64 100644 --- a/packages/swingset/src/stories/dialog.stories.tsx +++ b/packages/swingset/src/stories/dialog.stories.tsx @@ -20,7 +20,8 @@ export function Default() { Open dialog - + + Confirm action @@ -28,7 +29,7 @@ export function Default() { Cancel - + ); diff --git a/packages/ui/src/mosaic/block/destructive.tsx b/packages/ui/src/mosaic/block/destructive.tsx index 066d430041a..73a4d24f381 100644 --- a/packages/ui/src/mosaic/block/destructive.tsx +++ b/packages/ui/src/mosaic/block/destructive.tsx @@ -56,6 +56,7 @@ export function Destructive({ return ( (function Dialog ); }); -interface DialogProps extends Pick { - trigger: MosaicComponentProps<'button'>['render']; +interface DialogProps extends Pick< + HeadlessDialogProps, + 'open' | 'defaultOpen' | 'onOpenChange' | 'modal' | 'closedBy' +> { + /** + * Renders the button that opens the dialog. Omit for dialogs driven entirely by `open` — + * opened from a menu item, a route, or a state machine — where there is no trigger to render. + */ + trigger?: MosaicComponentProps<'button'>['render']; children: ReactNode | ((ctx: { close: () => void }) => ReactNode); size?: DialogVariantProps['size']; } @@ -134,7 +141,7 @@ function DialogContent({ children }: { children: DialogProps['children'] }) { return <>{children({ close: () => setOpen(false) })}; } -export function Dialog({ trigger, children, size, open, defaultOpen, onOpenChange, modal }: DialogProps) { +export function Dialog({ trigger, children, size, open, defaultOpen, onOpenChange, modal, closedBy }: DialogProps) { return ( - + {trigger ? : null} diff --git a/packages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsx index e043f91d91f..f33afe7c80a 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-domains-section-add-verify.view.tsx @@ -44,6 +44,7 @@ export function OrganizationProfileDomainsSectionAddVerifyView({ return ( { if (!open) { diff --git a/packages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsx index 7f8ac142af4..712bb10e969 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-domains-section-enrollment.view.tsx @@ -42,6 +42,7 @@ export function OrganizationProfileDomainsSectionEnrollmentView({ return ( { if (!open) { diff --git a/packages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsx b/packages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsx index c68d2ffaa54..44bd943fcc0 100644 --- a/packages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsx +++ b/packages/ui/src/mosaic/organization/organization-profile-domains-section-remove.view.tsx @@ -24,6 +24,7 @@ export function OrganizationProfileDomainsSectionRemoveView({ return ( { if (!open) { 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 e63d442cb02..0ae9df542f1 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 @@ -80,6 +80,7 @@ export function OrganizationProfileProfileSectionView({ )} send({ type: open ? 'OPEN' : 'CANCEL' })} trigger={props => (