Skip to content

Commit 72ffc81

Browse files
maxyingerclaude
andauthored
feat(ui): dialog close confirmation (#9439)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 41a5fb1 commit 72ffc81

14 files changed

Lines changed: 914 additions & 69 deletions

File tree

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
---
2+
---

packages/headless/src/primitives/dialog/README.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ const feedbackDialog = Dialog.createHandle();
6363
### Multiple triggers and payloads
6464

6565
Each trigger can carry an `id` and a `payload`. The root's children can be a function receiving
66-
the active trigger's payload, so one dialog renders per-trigger content. Type the payload through
66+
the active payload, so one dialog renders per-trigger content. Type the payload through
6767
the handle: `Dialog.createHandle<Payload>()`. The payload is captured when the trigger opens the
6868
dialog; for data that can change while it is open, carry an id and read live state inside.
6969

@@ -78,6 +78,17 @@ const detail = Dialog.createHandle<{ name: string }>();
7878
</Dialog.Root>
7979
```
8080

81+
An open with no trigger behind it can supply the payload directly: `handle.open(payload)` is the
82+
programmatic counterpart, for a dialog raised by something that happened rather than by an element
83+
— a confirmation that has to say what it is asking. A trigger-driven open supersedes it, since a
84+
trigger names its own payload.
85+
86+
```tsx
87+
const confirmation = Dialog.createHandle<{ question: string }>();
88+
89+
confirmation.open({ question: 'Discard changes?' });
90+
```
91+
8192
In controlled mode, track which trigger is active with `triggerId``onOpenChange`'s second
8293
argument reports the trigger behind each change:
8394

@@ -147,7 +158,7 @@ the close was pointer-driven, where focus is left where the pointer put it (see
147158
| `closedBy` | `'any' \| 'closerequest' \| 'none'` | `'any'` | Which gestures dismiss the dialog |
148159
| `handle` | `DialogHandle` || Connects detached triggers (see `Dialog.createHandle()`) |
149160
| `triggerId` | `string \| null` || Controls which trigger the open is attributed to |
150-
| `children` | `ReactNode \| ({ payload }) => ReactNode` || Content, or a render function of the active trigger's `payload` |
161+
| `children` | `ReactNode \| ({ payload }) => ReactNode` || Content, or a render function of the active `payload` |
151162

152163
#### `closedBy`
153164

packages/headless/src/primitives/dialog/dialog-handle.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ export interface DialogTriggerRegistration<Payload = unknown> {
2020
* requests made with no root attached are ignored, matching Base UI.
2121
* @internal
2222
*/
23-
export interface DialogRootController {
23+
export interface DialogRootController<Payload = unknown> {
2424
openFromTrigger: (id: string, event: Event) => void;
2525
closeFromTrigger: (id: string, event: Event) => void;
26-
setOpen: (open: boolean) => void;
26+
setOpen: (open: boolean, payload?: Payload) => void;
2727
}
2828

2929
/** The slice of root state a trigger renders from: its `data-open` / ARIA wiring. */
@@ -45,20 +45,34 @@ const CLOSED_STATE: DialogHandleState = { open: false, triggerId: null, popupId:
4545
* lets a `DialogHandle<Payload>` flow into contexts typed `DialogHandle<unknown>`.
4646
*/
4747
export interface DialogHandle<Payload = unknown> {
48-
/** Opens the attached root. Ignored while no root is mounted. */
49-
open(): void;
48+
/**
49+
* Opens the attached root. Ignored while no root is mounted.
50+
*
51+
* The optional `payload` is the programmatic counterpart of a trigger's: it reaches the root's
52+
* children-as-function as `{ payload }`, so an imperative open can carry the content the dialog
53+
* is about — what a confirmation is asking, which record is being deleted — without the caller
54+
* holding a second piece of state alongside `open`. A trigger-driven open supersedes it, since
55+
* a trigger names its own payload.
56+
*/
57+
open(payload?: Payload): void;
5058
/** Closes the attached root. Ignored while no root is mounted. */
5159
close(): void;
5260
/** Whether the attached root is open. `false` while no root is mounted. */
5361
readonly isOpen: boolean;
62+
/**
63+
* Whether a root is mounted and attached. `open`/`close` are ignored while this is `false`,
64+
* so a caller that must not silently no-op can check first.
65+
* @internal
66+
*/
67+
readonly hasRoot: boolean;
5468
/** @internal */
5569
registerTrigger(registration: DialogTriggerRegistration<Payload>): () => void;
5670
/** @internal */
5771
getTrigger(id: string): DialogTriggerRegistration<Payload> | undefined;
5872
/** @internal */
5973
getFirstTrigger(): DialogTriggerRegistration<Payload> | undefined;
6074
/** @internal */
61-
setRoot(controller: DialogRootController): () => void;
75+
setRoot(controller: DialogRootController<Payload>): () => void;
6276
/** @internal */
6377
requestOpen(id: string, event: Event): void;
6478
/** @internal */
@@ -79,21 +93,24 @@ export interface DialogHandle<Payload = unknown> {
7993
export function createDialogHandle<Payload = unknown>(): DialogHandle<Payload> {
8094
const triggers = new Map<string, DialogTriggerRegistration<Payload>>();
8195
const listeners = new Set<() => void>();
82-
let root: DialogRootController | null = null;
96+
let root: DialogRootController<Payload> | null = null;
8397
let state = CLOSED_STATE;
8498

8599
const notify = () => listeners.forEach(listener => listener());
86100

87101
return {
88-
open() {
89-
root?.setOpen(true);
102+
open(payload) {
103+
root?.setOpen(true, payload);
90104
},
91105
close() {
92106
root?.setOpen(false);
93107
},
94108
get isOpen() {
95109
return state.open;
96110
},
111+
get hasRoot() {
112+
return root !== null;
113+
},
97114
registerTrigger(registration) {
98115
triggers.set(registration.id, registration);
99116
notify();

packages/headless/src/primitives/dialog/dialog-root.tsx

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export interface DialogProps<Payload = unknown> {
7474
* open programmatically as if a given trigger had been activated.
7575
*/
7676
triggerId?: string | null;
77-
/** Content, or a function of `{ payload }` — the `payload` of the active trigger — for per-trigger content. */
77+
/** Content, or a function of `{ payload }` — the active trigger's, or the one `handle.open()` supplied. */
7878
children: ReactNode | ((ctx: { payload: Payload | undefined }) => ReactNode);
7979
}
8080

@@ -98,6 +98,10 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
9898
// consumed by the floating `onOpenChange` the request funnels into.
9999
const pendingDetailsRef = useRef<DialogOpenChangeDetails | null>(null);
100100

101+
// The payload of the most recent programmatic `handle.open(payload)`, kept so the registry
102+
// lookup below has something to fall back to when no trigger is involved.
103+
const directPayloadRef = useRef<Payload | undefined>(undefined);
104+
101105
// Every open/close funnels through `floatingContext.onOpenChange` — trigger activations,
102106
// dismissals, and programmatic `setOpen` alike. floating-ui emits its `openchange` event
103107
// synchronously before invoking this callback, which is what lets listeners (`useReturnFocus`,
@@ -119,6 +123,9 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
119123
openFromTrigger: (id, event) => {
120124
const registration = store.getTrigger(id);
121125
setActiveTriggerId(id);
126+
// A trigger names its own payload, so it supersedes anything a previous programmatic
127+
// open supplied — otherwise the stale one would resurface through the effect below.
128+
directPayloadRef.current = undefined;
122129
setActivePayload(registration?.getPayload());
123130
if (registration) {
124131
refs.setReference(registration.element);
@@ -131,7 +138,21 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
131138
pendingDetailsRef.current = { trigger: registration?.element ?? null, triggerId: id, event };
132139
floatingContext.onOpenChange(false, event, 'click');
133140
},
134-
setOpen: nextOpen => floatingContext.onOpenChange(nextOpen),
141+
setOpen: (nextOpen, payload) => {
142+
// Held in a ref as well as in state because the payload effect below re-runs on `open`
143+
// and would otherwise resolve a trigger-less open to `undefined`, wiping this a commit
144+
// after it was set. Cleared on close as well, so the next payload-less open does not
145+
// resurface this one.
146+
directPayloadRef.current = payload;
147+
// Published only on the way IN. A close carries no payload, and writing it would blank
148+
// the children-as-function while the popup is still mounted for its exit transition —
149+
// rendering the dialog empty as it leaves, or throwing in a consumer that dereferences
150+
// the payload. The next open sets it afresh.
151+
if (nextOpen) {
152+
setActivePayload(payload);
153+
}
154+
floatingContext.onOpenChange(nextOpen);
155+
},
135156
});
136157
// `floatingContext` is rebuilt on open/element changes; re-registering is an idempotent swap.
137158
}, [store, refs, floatingContext, setActiveTriggerId]);
@@ -155,9 +176,21 @@ function DialogInner<Payload>(props: DialogProps<Payload> & { isNested: boolean
155176
// `defaultOpen` — the payload is looked up from the registry once the dialog is open. Runs
156177
// after the children's layout effects, so triggers rendered inside the root are registered by
157178
// the time it reads, and the pre-paint re-render delivers their payload on the first frame.
179+
//
180+
// An explicit programmatic payload wins over the registry lookup: `activeTriggerId` is never
181+
// reset on close, so once any trigger has opened the dialog the lookup would otherwise overwrite
182+
// every later `handle.open(payload)` with that trigger's payload. The mirror already holds —
183+
// `openFromTrigger` clears `directPayloadRef`, so a trigger wins the other way.
158184
useLayoutEffect(() => {
159185
if (open) {
160-
setActivePayload(activeTriggerId != null ? store.getTrigger(activeTriggerId)?.getPayload() : undefined);
186+
const direct = directPayloadRef.current;
187+
setActivePayload(
188+
direct !== undefined
189+
? direct
190+
: activeTriggerId != null
191+
? store.getTrigger(activeTriggerId)?.getPayload()
192+
: undefined,
193+
);
161194
}
162195
}, [store, open, activeTriggerId]);
163196

packages/headless/src/primitives/dialog/dialog.test.tsx

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,109 @@ describe('Dialog', () => {
528528

529529
expect(screen.getByRole('dialog', { name: 'payload-b' })).toBeInTheDocument();
530530
});
531+
532+
// The programmatic counterpart of a trigger's payload, for an open that no element initiated —
533+
// a confirmation raised by a close request, say, which has to say what it is asking.
534+
describe('handle.open(payload)', () => {
535+
function renderDetached() {
536+
const handle = Dialog.createHandle<string>();
537+
render(
538+
<Dialog.Root handle={handle}>
539+
{({ payload }) => (
540+
<>
541+
<Dialog.Trigger id='trigger-a'>Open A</Dialog.Trigger>
542+
<Dialog.Trigger
543+
id='trigger-b'
544+
payload='from-trigger-b'
545+
>
546+
Open B
547+
</Dialog.Trigger>
548+
<Dialog.Portal>
549+
<Dialog.Viewport>
550+
<Dialog.Popup>
551+
<Dialog.Title>{payload ?? 'no payload'}</Dialog.Title>
552+
</Dialog.Popup>
553+
</Dialog.Viewport>
554+
</Dialog.Portal>
555+
</>
556+
)}
557+
</Dialog.Root>,
558+
);
559+
return handle;
560+
}
561+
562+
it('delivers it to the children render function', () => {
563+
const handle = renderDetached();
564+
565+
act(() => handle.open('from-handle'));
566+
567+
expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument();
568+
});
569+
570+
it('survives the registry lookup that runs once the dialog is open', async () => {
571+
const handle = renderDetached();
572+
573+
act(() => handle.open('from-handle'));
574+
// The lookup effect re-runs on `open`; without a fallback it would resolve to `undefined`
575+
// a commit later and blank the dialog.
576+
await act(async () => {
577+
await Promise.resolve();
578+
});
579+
580+
expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument();
581+
});
582+
583+
it('is superseded by a trigger, which names its own payload', async () => {
584+
const user = userEvent.setup();
585+
const handle = renderDetached();
586+
587+
act(() => handle.open('from-handle'));
588+
act(() => handle.close());
589+
await user.click(screen.getByRole('button', { name: 'Open A' }));
590+
591+
expect(screen.getByRole('dialog', { name: 'no payload' })).toBeInTheDocument();
592+
});
593+
594+
it('supersedes the trigger that opened the dialog last', async () => {
595+
const user = userEvent.setup();
596+
const handle = renderDetached();
597+
598+
// `activeTriggerId` is never reset on close, so without an explicit precedence the
599+
// registry lookup would hand this open the previous trigger's payload back.
600+
await user.click(screen.getByRole('button', { name: 'Open B' }));
601+
act(() => handle.close());
602+
act(() => handle.open('from-handle'));
603+
await act(async () => {
604+
await Promise.resolve();
605+
});
606+
607+
expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument();
608+
});
609+
610+
it('survives a `handle.close()` for the length of the exit transition', () => {
611+
// Keep an animation pending so the popup stays mounted after the close.
612+
const original = (Element.prototype as { getAnimations?: unknown }).getAnimations;
613+
(Element.prototype as { getAnimations?: unknown }).getAnimations = () => [
614+
{ finished: new Promise<void>(() => {}) },
615+
];
616+
try {
617+
const handle = renderDetached();
618+
619+
act(() => handle.open('from-handle'));
620+
act(() => handle.close());
621+
622+
// `close()` carries no payload; blanking it here would render the dialog empty on its
623+
// way out, or throw in a children function that dereferences it.
624+
expect(screen.getByRole('dialog', { name: 'from-handle' })).toBeInTheDocument();
625+
} finally {
626+
if (original) {
627+
(Element.prototype as { getAnimations?: unknown }).getAnimations = original;
628+
} else {
629+
delete (Element.prototype as { getAnimations?: unknown }).getAnimations;
630+
}
631+
}
632+
});
633+
});
531634
});
532635

533636
describe('initialFocus', () => {

packages/swingset/src/stories/alert-dialog.component.mdx

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,70 @@ the same reason: a corner X is a way out without answering.
8282
Every close request — Escape or `AlertDialog.Close` — routes through `onOpenChange`, so a controlled
8383
consumer can decline one by not committing the state.
8484

85+
### Confirming a close
86+
87+
A dialog holding unsaved work should ask before discarding it. That is three pieces: a handle, a
88+
hook that guards the close, and the confirmation itself.
89+
90+
```tsx
91+
import { AlertDialog, createConfirmHandle, useConfirmedClose } from '@clerk/ui/mosaic/components/alert-dialog';
92+
93+
const confirm = React.useMemo(() => createConfirmHandle(), []);
94+
95+
const onOpenChange = useConfirmedClose({
96+
handle: confirm,
97+
when: () => value !== '',
98+
onOpenChange: setOpen,
99+
confirm: {
100+
title: 'Discard changes?',
101+
description: 'You have not finished adding this address.',
102+
actionLabel: 'Discard',
103+
cancelLabel: 'Keep editing',
104+
destructive: true,
105+
},
106+
});
107+
108+
<Dialog open={open} onOpenChange={onOpenChange} closedBy='closerequest'>
109+
{/**/}
110+
<AlertDialog.Confirm handle={confirm} finalFocus={inputRef} />
111+
</Dialog>
112+
```
113+
114+
**Render `AlertDialog.Confirm` inside the dialog it guards** — anywhere in its children. That is
115+
what puts the two in one floating tree, and escape ordering, the stacking styles and the refcounted
116+
scroll lock all read that tree. A confirmation mounted app-globally would be a sibling of the dialog
117+
rather than a child of it, and all three would break.
118+
119+
**The guarded dialog must be controlled.** A veto is the absence of a commit, and an uncontrolled
120+
dialog has already committed by the time `onOpenChange` runs.
121+
122+
**What it covers is every close the dialog owns**: Escape, an outside press where `closedBy` allows
123+
one, `Dialog.CloseButton`, `Dialog.Close`, and the `close` the `Dialog` wrapper hands its children.
124+
A button wired to your own `setOpen(false)` never reaches the dialog, so it bypasses the question
125+
silently — route those through `Dialog.Close`.
126+
127+
`when()` is evaluated at each close request, so a close that no longer needs guarding (the form has
128+
just been submitted, the field cleared) passes straight through.
129+
130+
#### Asking without a close
131+
132+
`show()` is the same confirmation, awaited directly — for a decision that is not about closing:
133+
134+
```tsx
135+
if (await confirm.show({ title: 'Delete this key?', description: 'Applications using it stop working.' })) {
136+
await deleteKey();
137+
}
138+
```
139+
140+
It resolves `true` for the action and `false` for cancel or any dismissal. Calling it while a
141+
confirmation is already showing returns the in-flight promise rather than opening a second one, so
142+
repeated close requests ask once.
143+
144+
**`AlertDialog.Confirm` must be mounted when `show()` is called** — it is the thing that opens, and
145+
a `show()` with nothing mounted to answer it never resolves. Since the confirmation lives inside the
146+
dialog it guards, that means asking from inside that dialog, while it is open. A confirmation that
147+
unmounts with a question in flight answers `false` rather than leaving the `await` hanging.
148+
85149
## Parts
86150

87151
| Part | Slot | Description |
@@ -96,6 +160,7 @@ consumer can decline one by not committing the state.
96160
| `AlertDialog.Description` || Description; wired to the popup's `aria-describedby`. Required. |
97161
| `AlertDialog.Close` || Dismisses the alert; unstyled, accepts a `render` prop. |
98162
| `AlertDialog.Actions` | `alert-dialog-actions` | The response row. Cancel first. |
163+
| `AlertDialog.Confirm` | `dialog-popup` | A whole confirmation rendered from a `show()` call. See [Confirming a close](#confirming-a-close). |
99164

100165
Every part except `Popup` and `Actions` is `Dialog`'s own component, not a wrapper around it — one
101166
implementation, so the two cannot drift. `Title` and `Description` are unstyled passthroughs from the

0 commit comments

Comments
 (0)