diff --git a/.changeset/mosaic-dialog-drop-prompt.md b/.changeset/mosaic-dialog-drop-prompt.md new file mode 100644 index 00000000000..a845151cc84 --- /dev/null +++ b/.changeset/mosaic-dialog-drop-prompt.md @@ -0,0 +1,2 @@ +--- +--- diff --git a/.claude/skills/mosaic/SKILL.md b/.claude/skills/mosaic/SKILL.md index c4108823c44..5142aeb49c4 100644 --- a/.claude/skills/mosaic/SKILL.md +++ b/.claude/skills/mosaic/SKILL.md @@ -78,3 +78,38 @@ layer. Copy from it. The migration workflow (`migration.md`) ties the flow references together: it treats the legacy component as the spec and drives you through the model, controller, and view layers, then verifies parity with `parity-audit.md`. + +## Documenting a component in swingset + +`packages/swingset/CLAUDE.md` is the house style — archetypes, required section +order, `meta` conventions. One rule on top of it, because it is the one agents +get wrong: + +**The docs describe the API as it is. They do not carry the reasoning that +produced it.** No "not a `size`, because…", no rejected alternatives, no history +of what the prop used to be. A reader is there to learn what the thing does, and +every sentence of rationale is a sentence they have to skim past to find it. +State the behaviour plainly and briefly, then stop. + +```mdx + + +### Variant + +Which surface the dialog holds, and so the geometry it is given. Not a `size`, +because these are different surfaces rather than one surface at two widths — a +second card width would be a size of the `card` variant, with nowhere to sit on +this axis. + + + +### Variant + +Which surface the dialog holds, and the geometry that comes with it. +``` + +This is the opposite of the rule for **code**, where a comment earns its place by +explaining what the code cannot say for itself — the cascade fight behind a +`null`, the measured reason a duration is what it is. Keep that reasoning where a +future maintainer will hit it, which is the source file. The trade you rejected +belongs in a code comment or the PR description; the docs get the conclusion. diff --git a/packages/swingset/CLAUDE.md b/packages/swingset/CLAUDE.md index 4f127488724..920c73cf521 100644 --- a/packages/swingset/CLAUDE.md +++ b/packages/swingset/CLAUDE.md @@ -273,6 +273,7 @@ import * as UserButtonStories from './user-button.stories'; - Keep the intro to one short, present-tense paragraph: what the thing is and what it's for. For primitives, say explicitly that it's headless and ships no styles. - Prose should add what a demo can't — behavior, accessibility, when to reach for it — not restate prop names already in the table. +- Describe the API as it is; leave the reasoning that produced it out. No "not a `size`, because…", no rejected alternatives, no history of what a prop used to be — that belongs in a code comment or the PR. The docs get the conclusion, stated plainly and briefly. - Lead every page with the heading hierarchy its archetype prescribes; don't invent new top-level sections or reorder them. Consistency across pages is the goal. ### Before you finish diff --git a/packages/swingset/src/stories/confirmation.mdx b/packages/swingset/src/stories/confirmation.mdx index 04c12f681c0..c5aeb1a7ef3 100644 --- a/packages/swingset/src/stories/confirmation.mdx +++ b/packages/swingset/src/stories/confirmation.mdx @@ -19,6 +19,8 @@ import * as Stories from './confirmation.stories'; A confirmation for a destructive action that is worth a second look but not worth making the user type for. Removing a connected account, revoking a session, signing out everywhere. For the actions that do warrant typing, use [Destructive](/components/destructive). +It is an `alertdialog`: it announces as an interruption, an outside press cannot dismiss it, and the card carries no corner dismiss. Escape and the footer's Cancel action dismiss it. Under the phone band it arrives as a bottom sheet. + The block holds nothing of its own. Everything that decides what the dialog does next belongs to the caller. `open` closes it, `isConfirming` marks it busy, `errorMessage` explains a failure. ```tsx @@ -94,18 +96,18 @@ Opened from a menu item, focus returns to that menu's trigger when the dialog cl Controlled: -| Prop | Type | Default | Description | -| -------------- | ------------------------- | ------------ | ------------------------------------------------------------------------------ | -| `open` | `boolean` | — (required) | Whether the confirmation is showing. Controlled, the way any dialog is. | -| `onOpenChange` | `(open: boolean) => void` | — (required) | Asks to open or close. Fired by the trigger, Cancel, Escape, and the backdrop. | -| `trigger` | `ReactNode` | — | The button that asks to open the dialog. | -| `title` | `string` | — (required) | Names what is about to happen. | -| `description` | `ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | -| `actionLabel` | `string` | — (required) | The destructive button's label. | -| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | -| `onConfirm` | `() => void` | — (required) | Asks the caller to run the action. | -| `isConfirming` | `boolean` | `false` | Renders the action pending and ignores further presses. | -| `errorMessage` | `string` | — | Renders as a negative banner above the actions. | +| Prop | Type | Default | Description | +| -------------- | ------------------------- | ------------ | ------------------------------------------------------------------------------------------ | +| `open` | `boolean` | — (required) | Whether the confirmation is showing. Controlled, the way any dialog is. | +| `onOpenChange` | `(open: boolean) => void` | — (required) | Asks to open or close. Fired by the trigger, Escape, and the `Dialog.Close` Cancel action. | +| `trigger` | `ReactNode` | — | The button that asks to open the dialog. | +| `title` | `string` | — (required) | Names what is about to happen. | +| `description` | `ReactNode` | — (required) | Spells out what it means. Takes markup, for a name to emphasise. | +| `actionLabel` | `string` | — (required) | The destructive button's label. | +| `cancelLabel` | `string` | `'Cancel'` | The cancel button's label. | +| `onConfirm` | `() => void` | — (required) | Asks the caller to run the action. | +| `isConfirming` | `boolean` | `false` | Renders the action pending and ignores further presses. | +| `errorMessage` | `string` | — | Renders as a negative banner above the actions. | With a handle: diff --git a/packages/swingset/src/stories/dialog.component.mdx b/packages/swingset/src/stories/dialog.component.mdx index 40a65eb0a4a..2fa62c39776 100644 --- a/packages/swingset/src/stories/dialog.component.mdx +++ b/packages/swingset/src/stories/dialog.component.mdx @@ -2,12 +2,6 @@ import * as DialogStories from './dialog.component.stories'; # Dialog -The Mosaic `Dialog` — a modal surface built on the `@clerk/headless` dialog primitive. Three parts: -`Dialog.Root` owns the state, `Dialog.Trigger` opens it, `Dialog.Popup` is the surface. Focus -trapping, scroll lock and ARIA wiring come from the primitive. - -## Playground - void' }, { name: 'modal', type: 'boolean', default: 'true' }, - { name: 'closedBy', type: "'any' | 'closerequest' | 'none'", default: "'any'" }, ]} /> -`size` belongs to `Dialog.Popup`; the rest belong to `Dialog.Root`. +`variant` and `compactPlacement` belong to `Dialog.Popup`; the rest belong to `Dialog.Root`. ## Usage ```tsx import { Button } from '@clerk/ui/mosaic/components/button'; +import { Card } from '@clerk/ui/mosaic/components/card'; import { Dialog } from '@clerk/ui/mosaic/components/dialog'; }>Open dialog - - Confirm action - Are you sure you want to proceed? - }>Cancel + + + Confirm action + Are you sure you want to proceed? + + + }>Cancel + + + ; ``` @@ -64,32 +63,28 @@ const [open, setOpen] = useState(false); Every close the dialog owns — Escape, outside press, `Dialog.Close`, `Dialog.CloseButton`, `handle.close()` — goes through `onOpenChange`, so a controlled consumer can veto one by not -committing. Your own `setOpen(false)` skips that; use `Dialog.Close` when a close might need vetoing. +committing. -### Size +### Variant -| Value | Width | For | -| --------- | ---------------------------------------- | ----------------------------------------------------- | -| `prompt` | `23.75rem`, height from content | One question or one field (default) | -| `card` | `25rem`, height from content | Sign-in / sign-up | -| `profile` | Width from its surface, fills the height | Account profile and settings — a surface you navigate | +| Value | Width | For | +| --------- | ---------------------------------------- | -------------------------------------------------------------------- | +| `card` | `25rem`, height from content | Everything a `Card` holds — a flow, a confirmation, a form (default) | +| `profile` | Width from its surface, fills the height | Account profile and settings — a surface you navigate | -### `card` and `profile` bring their own surface +### Every dialog brings its own surface -Only `prompt` paints itself. A `card` holds a `Card`, and a `profile` holds a `Profile` (or +The dialog **paints nothing**. A `card` holds a `Card`, and a `profile` holds a `Profile` (or `UserProfileView`): the dialog positions and animates the popup, and the surface inside paints it. -Both surfaces read `DialogContext` and are self-contained — `Card.Title` and the profile's label name -the dialog, and `Card.Header` and `Profile.Root` carry the dismiss — so `Dialog.CloseButton` -and `Dialog.Title` are only for a `prompt`. + +A popup with no surface inside renders an unpainted box. -A `profile` composes the same way, with the user page in place of the card. The page names the -dialog, carries the dismiss, scrolls its own content column, and collapses its own sidebar — the -full example is under [A profile](#a-profile): +A `profile` composes the same way, with the user page in place of the card. ```tsx import { Button } from '@clerk/ui/mosaic/components/button'; @@ -98,7 +93,7 @@ import { UserProfileView } from '@clerk/ui/mosaic/features/user-profile/user-pro }>Manage account - + ; ``` -### Alert dialogs +### Compact placement -`role='alertdialog'` on the root makes a dialog that interrupts to ask for a decision — a -destructive confirmation, a "discard changes?". +`compactPlacement='sheet'` bottom-anchors the surface in the compact container query and slides it up from the +edge instead of centring it. -What follows from the role: - -- Announced as an interruption. -- No outside-press dismissal. `closedBy` narrows to `closerequest` (default) or `none`; Escape still - cancels. -- Always a `prompt`. `size` is ignored (warns in dev). -- A `Title` **and** a `Description` are required; both warn in dev when missing. -- `Dialog.CloseButton` warns: the cancel in `Dialog.Actions` is the way out. +| Value | Compact band (`< 48rem`) | Above it | +| -------- | ------------------------------------------------------------- | -------- | +| `center` | Centred (default) | Centred | +| `sheet` | Bottom-anchored, slides up; full width until the surface caps | Centred | -`Dialog.Actions` is the button row for any prompt, not only alerts — it splits the width evenly. -Render the cancel first so it takes focus on open. The action usually starts -work, so close from your own state when it resolves rather than with `Dialog.Close`. Pass -`finalFocus` when there is no trigger to return focus to. +A `profile` takes no placement and warns in dev if given one. -### Confirming a discard +### Alert dialogs -Guard a dialog's close behind a confirmation with a handle, a hook, and `Dialog.Confirm`: +`role='alertdialog'` on the root makes a dialog that interrupts to ask for a decision — a +destructive confirmation, a "discard changes?". -```tsx -import { createConfirmHandle, Dialog, useConfirmedClose } from '@clerk/ui/mosaic/components/dialog'; - -const confirm = React.useMemo(() => createConfirmHandle(), []); - -const onOpenChange = useConfirmedClose({ - handle: confirm, - when: () => value !== '', - onOpenChange: setOpen, - confirm: { title: 'Discard changes?', description: '…', actionLabel: 'Discard', destructive: true }, -}); - - - - … - - -; -``` +What follows from the role: -- Render `Dialog.Confirm` **inside** the popup it guards, so the two share a floating tree. -- The guarded dialog must be controlled — a veto is the absence of a commit. -- `confirm.show(options)` asks the same question directly and resolves `true`/`false`. It needs the - `Dialog.Confirm` mounted, and returns the in-flight promise if one is already showing. +- Announced as an interruption. +- No outside-press dismissal; Escape still cancels. +- A name **and** a description are required — `Card.Title` and `Card.Description`; both warn in dev + when missing. +- `Card.Header` withholds its corner dismiss, and `Dialog.CloseButton` warns. -### Inline +### Confirming a discard -`inline` on the root renders the dialog in its host instead of over the page: no portal, scrim, -scroll lock or focus trap, and nothing dismisses it. For the account profile mounted in a page slot. +Press Cancel to open a discard confirmation above the profile form. Keep editing returns to the form with the draft intact; Discard clears the draft and closes both dialogs. The dialogs share one scrim, and the form beneath dims and recedes. -- `open`, `modal`, `closedBy` are implied; `onOpenChange` is never called. -- No initial focus on mount. `Dialog.CloseButton` renders nothing; `Card.Header` carries no dismiss. -- Fills the host edge to edge. How the surface looks there is the surface's call — the page - paints the same frame modal or inline. -- Dialogs opened from inside it are normal modal dialogs over the page. - ### Responsive behaviour The gap to the screen edge is a fixed inset, not a percentage: @@ -193,76 +158,62 @@ The gap to the screen edge is a fixed inset, not a percentage: | `>= 90rem` | `3rem` | `3rem` | The bands are **container queries** against the dialog's viewport element (named `cl-dialog`), not -media queries. For a modal dialog that is the window, so nothing differs; for an `inline` dialog the -bands follow the host's width. `Profile` does the same against its own width (`cl-profile`), which is -what collapses its navigation inside a narrow host. +media queries. -Below `48rem` a `prompt` becomes a bottom sheet: it pins to the bottom, slides up, and lifts its -width cap. `card` and `profile` are unchanged. Under `prefers-reduced-motion: reduce` only the fade -runs. +Below `48rem` a `compactPlacement='sheet'` dialog pins to the bottom and slides up. ### Close button -`Dialog.CloseButton` is the styled corner X for a `prompt`, absolutely positioned so it can sit -anywhere in the children. Pass `aria-label` to localise it. A `card` or `profile` surface carries its -own. `Dialog.Close` is the unstyled alternative for a footer "Cancel". +The surface carries the dismiss: `Card.Header` renders one in flow, and `Profile` anchors its own +through `Dialog.CloseButton`. -Focus opens on the first tabbable element, so a `CloseButton` rendered first takes it. Point -`initialFocus` on `Dialog.Popup` at the field that should have it instead. +Focus opens on the first tabbable element, which is why `Card.Header` renders its dismiss first in +the DOM. Point `initialFocus` on `Dialog.Popup` at the field that should have it instead. ### Dismissal -`closedBy` mirrors the native `` attribute: +`role` decides what closes the dialog. There is no prop for it. -| Value | Escape | Outside press | Programmatic | Use for | -| -------------- | ------ | ------------- | ------------ | ------------------------------------------- | -| `any` | ✅ | ✅ | ✅ | Read-and-dismiss content (default) | -| `closerequest` | ✅ | ❌ | ✅ | Anything holding input; alert dialogs | -| `none` | ❌ | ❌ | ✅ | Flows the user must complete or acknowledge | +| Role | Escape | Outside press | Programmatic | +| ------------- | ------ | ------------- | ------------ | +| `dialog` | ✅ | ✅ | ✅ | +| `alertdialog` | ✅ | ❌ | ✅ | -```tsx -… -``` +The two map to the native [``](https://developer.mozilla.org/en-US/docs/Web/API/HTMLDialogElement/closedBy) values `any` and `closerequest`. ### Keyboard | Key | Does | | ------------------ | ----------------------------------------------------------------- | -| Enter (in a field) | Submits the prompt's form — the primary action | -| Escape | Cancels, per `closedBy` | +| Enter (in a field) | Submits the dialog's form — the primary action | +| Escape | Cancels | | Tab | Moves through the popup in visual order: field → Cancel → confirm | -Tab follows the layout (WCAG 2.4.3), so a prompt with a field should be a form with the primary -action as its submit — that is how Enter reaches it: +Tab follows the layout (WCAG 2.4.3), so a dialog with a field should be a form with the primary +action as its submit. ```tsx -
+}> - - }>Cancel - - - +
+ + }>Cancel + + ``` -### Exit animations - -The popup's contents hold their last frame while it fades out, so state that resets on close (a -machine returning to idle) does not flash through the exit. - ## Parts -| Part | Slot | Description | -| -------------------- | --------------------- | ------------------------------------------------------------------- | -| `Dialog.Root` | — | State: open/close, `role`, `inline`, `modal`, `closedBy`, `handle`. | -| `Dialog.Trigger` | — | Opens the dialog; `render`, and `handle` + `payload` when detached. | -| `Dialog.Popup` | `dialog-popup` | The surface; `size`, `initialFocus`, `finalFocus`. | -| `Dialog.Title` | — | Wired to `aria-labelledby`. Unstyled — render through `Heading`. | -| `Dialog.Description` | — | Wired to `aria-describedby`. Unstyled — render through `Text`. | -| `Dialog.Close` | — | Unstyled dismiss; accepts `render`. | -| `Dialog.CloseButton` | `dialog-close-button` | The styled corner X. | -| `Dialog.Actions` | `dialog-actions` | An alert dialog's response row. Cancel first. | -| `Dialog.Confirm` | `dialog-popup` | A confirmation rendered from `confirm.show()`. | +| Part | Slot | Description | +| -------------------- | --------------------- | ----------------------------------------------------------------------------------------- | +| `Dialog.Root` | — | State: open/close, `role`, `modal`, `handle`. | +| `Dialog.Trigger` | — | Opens the dialog; `render`, and `handle` + `payload` when detached. | +| `Dialog.Popup` | `dialog-popup` | The box the surface sits in; `variant`, `compactPlacement`, `initialFocus`, `finalFocus`. | +| `Dialog.Close` | — | Unstyled dismiss; accepts `render`. | +| `Dialog.CloseButton` | `dialog-close-button` | The styled corner X, for a surface that anchors its own dismiss. | + +The name, the description and the button row come from the surface — `Card.Title`, +`Card.Description` and `Card.Footer` — not from the dialog. `Dialog.Popup` also renders the scrim (`dialog-backdrop`), the viewport (`dialog-viewport`, the `cl-dialog` container) and the centering track inside it (`dialog-track`, which carries the inset). @@ -275,7 +226,7 @@ Override by targeting the `.cl-*` slot from a layer that wins over `@clerk/ui/st @import '@clerk/ui/styles.css' layer(components); @layer overrides { - .cl-dialog-popup[data-size='profile'] { + .cl-dialog-popup[data-variant='profile'] { max-width: 60rem; } } @@ -287,23 +238,22 @@ State attributes: | ------------------------------------------- | ---------------------------------- | ------------------------------------- | | `data-open` / `data-closed` | Trigger, Backdrop, Viewport, Popup | Open state (`closed` during the exit) | | `data-starting-style` / `data-ending-style` | Backdrop, Viewport, Popup | Entering frame / exit animation | -| `data-size` | Viewport, Track, Popup | Resolved size | -| `data-inline` | Viewport, Track, Popup | Root is `inline` | +| `data-variant` | Viewport, Track, Popup | Resolved variant | | `data-nested` | Backdrop, Viewport, Popup | Opened inside another overlay | | `data-stacked` / `data-stack-base` | Popup | On top of / beneath another dialog | ### Nested dialogs and stacks -A dialog opened inside another is one of two relationships, decided by the size beneath it: +A dialog opened inside another is one of two relationships, decided by the variant beneath it: | | Nested | Stack | | ------------------- | -------------------------- | ---------------------------------- | -| Opens over | a `profile` or `card` | a `prompt` | +| Opens over | a `profile` | a `card` | | Its own scrim | Yes, lighter than the base | None — one scrim serves the stack | | The surface beneath | Unchanged | Dims and recedes | | Attributes | `data-nested` | `data-stacked` / `data-stack-base` | -A `profile` never opens inside another dialog — it warns in dev. A `prompt` or a `card` (a confirmation holding a `Card`) is what opens over one. +A `profile` never opens inside another dialog — it warns in dev. A `card` is what opens over one. --- @@ -311,32 +261,13 @@ A `profile` never opens inside another dialog — it warns in dev. A `prompt` or ### A profile -The account profile as a `profile`: the page inside paints the frame, names the dialog, scrolls its -own content column, collapses its own sidebar, and carries the dismiss. The dialog only positions it. - -Add an email address, type into the field and try to close it: the prompt opens over the profile -(nested, with its own lighter scrim), and a confirmation stacks on the prompt (no second scrim; the -prompt recedes). The same stack on its own: - - - -Nest by rendering a `Dialog.Root` inside another popup's children. Dismissal reaches only the top -dialog, scroll stays locked until the last one closes, and focus returns down the stack. - ### Scrolling a tall card -A `prompt` or `card` taller than the screen scrolls the whole dialog inside the viewport, inset and -all. Nothing to opt into. (A phone-band `prompt` sheet is clipped instead — use `card` for a tall -surface on a phone.) - ; +// A stand-in for a surface, painted by the story rather than by a component, so the playground +// shows the DIALOG's own box — what each `variant` measures and where `compactPlacement` puts it. +// Every real dialog holds a `Card` or a `Profile` instead; see the examples below. +const surface: React.CSSProperties = { + background: 'var(--cl-color-card)', + border: '1px dashed var(--cl-color-border)', + borderRadius: 'var(--cl-radius-xl)', + color: 'var(--cl-color-card-foreground)', + display: 'flex', + flexDirection: 'column', + gap: '0.5rem', + height: '100%', + padding: '1.5rem', +}; + +/** + * The dialog paints nothing: it brings the geometry, the motion and the floating tree, and the + * SURFACE comes from what is rendered inside it. This example draws its own box so the two are + * visible apart — the dashed edge is the popup, and `variant` is what it measures. + * + * Every real dialog holds a `Card` (see [the surface section](#every-dialog-brings-its-own-surface)) + * or a `Profile` (see [A profile](#a-profile)), which is also where the name and the dismiss come + * from — hence the `aria-label` and the corner `Dialog.CloseButton` standing in for them here. + */ export function Default(args: Record) { - const { size } = args as { size?: DialogSize }; + const { variant, compactPlacement } = args as { + variant?: DialogVariant; + compactPlacement?: DialogCompactPlacement; + }; return ( - + - Confirm action - Are you sure you want to proceed? This action cannot be undone. - }>Cancel +
+ The surface + + The dashed edge is Dialog.Popup. A real dialog fills it with a Card or a Profile, which paints + the frame, names the dialog and carries the dismiss. + +
); @@ -59,8 +96,10 @@ const deleteTrigger = (props: RenderProps) => ( ); /** - * `role='alertdialog'` is the whole difference: it announces as an interruption, an outside press - * cannot dismiss it, and it is always a `prompt`. `Dialog.Actions` holds the answer, cancel first. + * `role='alertdialog'` is the whole difference: it announces as an interruption, and an outside + * press cannot dismiss it — a question that needs an answer must not be answerable by clicking + * next to it. `Card.Header` withholds its corner dismiss for the same reason, and Escape still + * closes it. Cancel first, so it takes the opening focus. */ export function Alert() { const [open, setOpen] = React.useState(false); @@ -72,21 +111,38 @@ export function Alert() { > - }>Delete Acme Inc? - }> - The organization and everything in it will be permanently removed. This cannot be undone. - - - }>Cancel - {/* Not a `Dialog.Close`: the action is where the work happens, so the caller closes - once it resolves rather than the button closing on press. */} - - + + + Delete Acme Inc? + + The organization and everything in it will be permanently removed. This cannot be undone. + + + + + } + > + Cancel + + {/* Not a `Dialog.Close`: the action is where the work happens, so the caller closes + once it resolves rather than the button closing on press. */} + + + ); @@ -94,87 +150,212 @@ export function Alert() { const addEmailTrigger = (props: RenderProps) => ; -// `useConfirmedClose` wraps the dialog's own `onOpenChange`, so every close it owns — Escape, the -// corner X, `Dialog.Close` — is guarded by one hook and a veto is just the absence of a commit. -// `Dialog.Confirm` renders INSIDE the dialog it guards so the two share a floating tree, which -// escape ordering, the stacking styles and the refcounted scroll lock all depend on. `finalFocus` is -// optional but wanted here: a confirmation raised by a close request has no trigger to return to. -export function DiscardChanges() { - const confirm = React.useMemo(() => createConfirmHandle(), []); +/** + * `compactPlacement='sheet'` bottom-anchors the surface in the compact band — the dialog viewport + * under `48rem` — and slides it up from the edge, instead of centring it. Above the band nothing + * changes, so narrow the window to see it. + * + * For a dialog that asks one thing and returns: a confirmation, or a single-field form like this + * one, where the answer belongs within thumb's reach. + */ +export function Sheet() { const [open, setOpen] = React.useState(false); const [value, setValue] = React.useState(''); const inputRef = React.useRef(null); - const onOpenChange = useConfirmedClose({ - handle: confirm, - when: () => value.trim() !== '', - onOpenChange: next => { - setOpen(next); - if (!next) { - setValue(''); - } - }, - confirm: { - title: 'Discard changes?', - description: 'You have not finished adding this address. It will not be saved.', - actionLabel: 'Discard', - cancelLabel: 'Keep editing', - destructive: true, - }, - }); - return ( { + setOpen(next); + if (!next) { + setValue(''); + } + }} > - - - }>Add email address - }> - You will need to verify this address before it can be used. - - {/* A form, so Enter in the field is the primary action; Tab stays in visual order. Adding - is the one close that must not be questioned, so it goes straight to `setOpen`, past - the guard. */} -
{ - event.preventDefault(); - setValue(''); - setOpen(false); - }} + + - setValue(event.target.value)} - /> - - }>Cancel - - - - - + + Add email address + You will need to verify this address before it can be used. + + {/* A form, so Enter in the field is the primary action; Tab stays in visual order. */} + { + event.preventDefault(); + setValue(''); + setOpen(false); + }} + /> + } + > + setValue(event.target.value)} + /> + + + + } + > + Cancel + + + +
); } -const accountTrigger = (props: RenderProps) => ; +const editProfileTrigger = (props: RenderProps) => ; /** - * The "add email address" prompt the account profile opens, driven by `open` rather than a trigger. - * Closing it with a value typed asks first — `profile -> prompt -> prompt`. + * A dialog stacked on a dialog — the shape a close confirmation takes. Press Cancel: the alert + * arrives over the form, paints no scrim of its own, and the surface beneath recedes and dims + * instead. One scrim serves the whole stack, so depth never becomes a function of stack count. */ +export function Stacked() { + const [open, setOpen] = React.useState(false); + const [savedName, setSavedName] = React.useState('Ada Lovelace'); + const [name, setName] = React.useState(savedName); + const nameRef = React.useRef(null); + + const close = () => { + setName(savedName); + setOpen(false); + }; + + return ( + { + if (next) { + setOpen(true); + } else { + close(); + } + }} + > + + + + + Update profile + Change the name people see on your account. + + { + event.preventDefault(); + setSavedName(name); + setOpen(false); + }} + /> + } + > + setName(event.target.value)} + /> + + + {/* The confirmation lives INSIDE the dialog it guards, so the two share one floating + tree — which is what escape ordering, the stacking styles and the refcounted + scroll lock all read. */} + + + } + > + Cancel + + + + + Discard changes? + Your edits will be lost. + + + + } + > + Keep editing + + + + + + + + + + + + ); +} + +const accountTrigger = (props: RenderProps) => ; + +/** The "add email address" dialog the account profile opens, driven by `open` rather than a trigger. */ function AddEmailDialog({ open, onOpenChange, @@ -184,66 +365,74 @@ function AddEmailDialog({ onOpenChange: (open: boolean) => void; onAdd: (value: string) => void; }) { - const confirm = React.useMemo(() => createConfirmHandle(), []); const [value, setValue] = React.useState(''); const inputRef = React.useRef(null); - const guardedOpenChange = useConfirmedClose({ - handle: confirm, - when: () => value.trim() !== '', - onOpenChange: next => { - onOpenChange(next); - if (!next) { - setValue(''); - } - }, - confirm: { - title: 'Discard changes?', - description: 'You have not finished adding this address. It will not be saved.', - actionLabel: 'Discard', - cancelLabel: 'Keep editing', - destructive: true, - }, - }); - return ( { + onOpenChange(next); + if (!next) { + setValue(''); + } + }} > - - - }>Add email address - }>A verification code will be sent to this address. - {/* Straight to the parent's setter on submit: adding is the one close that must not be - questioned. A form, so Enter in the field adds. */} -
{ - event.preventDefault(); - onAdd(value.trim()); - setValue(''); - onOpenChange(false); - }} + + - setValue(event.target.value)} - /> - - }>Cancel - - - - + + Add email address + A verification code will be sent to this address. + + { + event.preventDefault(); + onAdd(value.trim()); + setValue(''); + onOpenChange(false); + }} + /> + } + > + setValue(event.target.value)} + /> + + + + } + > + Cancel + + + +
); @@ -253,7 +442,7 @@ function AddEmailDialog({ * The real user page inside a `profile` dialog. The dialog positions it and the page paints * itself — the same composition as a `Card` inside a `card` dialog — so the page names the * dialog, scrolls its own content column, collapses its own sidebar, and carries the dismiss. - * Adding an email opens a prompt over the profile; the danger zone's delete confirmation is the + * Adding an email opens a card over the profile; the danger zone's delete confirmation is the * page's own. */ export function Nested() { @@ -264,7 +453,7 @@ export function Nested() { return ( - + setAddEmailOpen(true), @@ -307,95 +496,20 @@ export function Inline() { width: '52rem', }} > - - - - - - + + ); } -const editProfileTrigger = (props: RenderProps) => ; - -/** - * A prompt stacked on a prompt — the shape a close confirmation takes. Edit the name and press - * Cancel: the second prompt paints no scrim of its own, and the one beneath it recedes instead. - */ -export function StackedPrompts() { - const confirm = React.useMemo(() => createConfirmHandle(), []); - const [open, setOpen] = React.useState(false); - const [savedName, setSavedName] = React.useState('Ada Lovelace'); - const [name, setName] = React.useState(savedName); - const nameRef = React.useRef(null); - - const onOpenChange = useConfirmedClose({ - handle: confirm, - when: () => name !== savedName, - onOpenChange: next => { - setOpen(next); - if (!next) { - setName(savedName); - } - }, - confirm: { - title: 'Discard changes?', - description: 'Your edits will be lost.', - actionLabel: 'Discard', - cancelLabel: 'Keep editing', - destructive: true, - }, - }); - - return ( - - - - - }>Update profile - }>Change the name people see on your account. - {/* Saving goes straight to `setOpen`, past the guard. A form, so Enter in the field saves. */} -
{ - event.preventDefault(); - setSavedName(name); - setOpen(false); - }} - > - setName(event.target.value)} - /> - - }>Cancel - - -
- - - - ); -} - /** * A handle at module scope: the trigger and the root only share it, not a JSX ancestor. * The same handle also has imperative `open()` / `close()` for opens with no trigger at all. @@ -411,9 +525,15 @@ export function DetachedTrigger() { /> - - }>Notifications - }>You are all caught up. Good job! + + + Notifications + You are all caught up. Good job! + + @@ -453,11 +573,15 @@ export function MultipleTriggers() { {({ payload }) => ( - - }>{payload?.name} - }> - {payload ? `${payload.role} of this organization.` : null} - + + + {payload?.name} + {payload ? `${payload.role} of this organization.` : null} + + )} @@ -465,19 +589,22 @@ export function MultipleTriggers() { ); } -/** `size='card'` paints nothing itself — the `Card` inside supplies the surface. */ +/** The sign-in / sign-up surface: the same composition, at the width the legacy card matches. */ export function CardSurface() { return ( } /> - + Sign in Continue to your account. - + } /> - + Terms of service @@ -549,23 +676,35 @@ export function OutsideScroll() { ); } -/** `initialFocus` skips past the close button and the name field; `finalFocus` is left default. */ +/** `initialFocus` skips past the card's dismiss and the name field; `finalFocus` is left default. */ export function CustomFocus() { const feedbackRef = React.useRef(null); return ( } /> - - }>Feedback - }> - The feedback field takes focus on open — past the close button and the name field. - - - + + + Feedback + + The feedback field takes focus on open — past the dismiss and the name field. + + + + + + + ); diff --git a/packages/swingset/src/stories/drawer.component.stories.tsx b/packages/swingset/src/stories/drawer.component.stories.tsx index aa8d8b5a8a8..7df605dc9a5 100644 --- a/packages/swingset/src/stories/drawer.component.stories.tsx +++ b/packages/swingset/src/stories/drawer.component.stories.tsx @@ -54,7 +54,7 @@ export function InsideProfile() { return ( }>Manage account - + }>Manage account - + { }); it("merges a render source's className/style out of the incoming prop bag", () => { - // `Dialog.Title render={}` hands Heading the title's merged pair through its + // `Card.Title render={}` hands Heading the title's merged pair through its // props; passing that bag last must merge them rather than clobber the part's own class. const rest = { className: 'cl-dialog-title', style: { color: 'red' }, id: 'title' }; const merged = mergeStyleProps({ className: 'cl-heading' }, { className: 'x1', style: { margin: 0 } }, rest); diff --git a/packages/ui/src/mosaic/blocks/confirmation/confirmation.test.tsx b/packages/ui/src/mosaic/blocks/confirmation/confirmation.test.tsx index d63c261ca3c..e46f6b571af 100644 --- a/packages/ui/src/mosaic/blocks/confirmation/confirmation.test.tsx +++ b/packages/ui/src/mosaic/blocks/confirmation/confirmation.test.tsx @@ -29,7 +29,7 @@ describe('Confirmation', () => { it('renders nothing until the caller opens it', () => { renderBlock({ open: false }); - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); }); it('asks to open from the trigger', async () => { @@ -61,7 +61,7 @@ describe('Confirmation', () => { ), }); - expect(screen.getByRole('dialog')).toHaveAccessibleDescription( + expect(screen.getByRole('alertdialog')).toHaveAccessibleDescription( 'preston@clerk.dev will be removed from this account.', ); expect(screen.getByText('preston@clerk.dev').tagName).toBe('STRONG'); @@ -127,7 +127,7 @@ describe('Confirmation with a handle', () => { it('renders nothing until opened through the handle', () => { renderWithHandle(); - expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); }); it('opens with a payload and renders the copy from it', () => { @@ -137,8 +137,8 @@ describe('Confirmation with a handle', () => { handle.open(preston); }); - expect(screen.getByRole('dialog')).toHaveAccessibleName('Remove member'); - expect(screen.getByRole('dialog')).toHaveAccessibleDescription( + expect(screen.getByRole('alertdialog')).toHaveAccessibleName('Remove member'); + expect(screen.getByRole('alertdialog')).toHaveAccessibleDescription( 'Preston Booth will be removed from the organization.', ); expect(screen.getByText('Preston Booth').tagName).toBe('STRONG'); @@ -156,7 +156,7 @@ describe('Confirmation with a handle', () => { await user.click(removeButton()); expect(onConfirm).toHaveBeenCalledWith(preston); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); }); it('stays inert and open while the action is pending', async () => { @@ -170,7 +170,7 @@ describe('Confirmation with a handle', () => { expect(removeButton()).toHaveAttribute('aria-busy', 'true'); await user.keyboard('{Escape}'); - expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); }); it('keeps the dialog open and explains a failed attempt', async () => { @@ -183,7 +183,7 @@ describe('Confirmation with a handle', () => { await user.click(removeButton()); await waitFor(() => expect(screen.getByRole('alert')).toHaveTextContent('Preston Booth is the last admin.')); - expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); expect(removeButton()).not.toHaveAttribute('aria-busy', 'true'); }); @@ -197,12 +197,12 @@ describe('Confirmation with a handle', () => { await waitFor(() => expect(screen.getByRole('alert')).toBeInTheDocument()); await user.click(screen.getByRole('button', { name: 'Cancel' })); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); act(() => { handle.open(preston); }); - expect(screen.getByRole('dialog')).toBeInTheDocument(); + expect(screen.getByRole('alertdialog')).toBeInTheDocument(); expect(screen.queryByRole('alert')).not.toBeInTheDocument(); }); @@ -217,7 +217,7 @@ describe('Confirmation with a handle', () => { handle.close(); }); - await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); + await waitFor(() => expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument()); expect(handle.isOpen).toBe(false); }); }); diff --git a/packages/ui/src/mosaic/blocks/confirmation/confirmation.tsx b/packages/ui/src/mosaic/blocks/confirmation/confirmation.tsx index f3d1f86789c..43ff3ce7cbc 100644 --- a/packages/ui/src/mosaic/blocks/confirmation/confirmation.tsx +++ b/packages/ui/src/mosaic/blocks/confirmation/confirmation.tsx @@ -27,7 +27,7 @@ function ConfirmationCard({ errorMessage, }: ConfirmationCardProps) { return ( - + @@ -176,7 +176,7 @@ function HandleConfirmation({ return ( = ConfirmationControlledProps | * Confirmation dialog for a destructive action that is worth a second look but not worth * making the user type for. Use `Destructive` for the actions that are. * + * An `alertdialog`: it announces as an interruption, an outside press cannot answer it, and the + * card withholds its corner dismiss. Escape still closes it, the way the cancel action does. Under + * the phone band it arrives as a bottom sheet, within reach of the thumb that has to answer it. + * * Two forms. Controlled: the caller owns `open`, `isConfirming`, and `errorMessage`, and the * block holds nothing of its own. With a `handle`: the block owns all three. Mount it once, * open it from anywhere with `handle.open(payload)`, and derive the copy and the action from diff --git a/packages/ui/src/mosaic/blocks/destructive/destructive.tsx b/packages/ui/src/mosaic/blocks/destructive/destructive.tsx index 16b74876a8d..670910a29c3 100644 --- a/packages/ui/src/mosaic/blocks/destructive/destructive.tsx +++ b/packages/ui/src/mosaic/blocks/destructive/destructive.tsx @@ -97,12 +97,11 @@ export function Destructive({ return ( {trigger ? : null} - + { Open - Review terms + + + Review terms + + , ); @@ -361,11 +368,11 @@ describe('Mosaic Card', () => { await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument()); }); - it('carries no dismiss button in an inline dialog, which nothing closes', () => { + it('carries a dismiss button inside a dialog, which is what it closes', () => { render( - - - + + + Account @@ -374,7 +381,7 @@ describe('Mosaic Card', () => { , ); - expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument(); expect(screen.getByRole('dialog')).toHaveAccessibleName('Account'); }); diff --git a/packages/ui/src/mosaic/components/card/card.tsx b/packages/ui/src/mosaic/components/card/card.tsx index 8b4c6a0f1ac..c8163530fb9 100644 --- a/packages/ui/src/mosaic/components/card/card.tsx +++ b/packages/ui/src/mosaic/components/card/card.tsx @@ -7,7 +7,7 @@ import { mergeStyleProps, themeProps } from '../../props'; import { reset } from '../../utils/reset.styles'; import { Branding } from '../branding'; import { Button } from '../button'; -import { Dialog, DialogContext, isOverlayDialog } from '../dialog'; +import { Dialog, DialogContext, isInDialog } from '../dialog'; import { Icon } from '../icon'; import { cardContentMarker } from './card.markers.stylex'; import * as slots from './card.styles'; @@ -66,8 +66,8 @@ const Root = React.forwardRef(function CardRoot( /** * Dismisses the dialog from inside the header, in flow, so the header reserves the width it takes - * and a long title cannot run under it. `Dialog.CloseButton` stays the corner affordance, for - * dialogs that hold no card. + * and a long title cannot run under it. `Dialog.CloseButton` stays the corner affordance, for the + * `Profile`, which anchors its own. */ function HeaderCloseButton() { return ( @@ -102,8 +102,10 @@ const Header = React.forwardRef>(fun <> {/* First in the DOM, so it is the first tabbable element and takes the dialog's opening focus — the same reason `Dialog.CloseButton` is a part rather than a popup flag. - Not for an inline dialog, which nothing closes. */} - {isOverlayDialog(dialog) ? : null} + Not outside a dialog, where there is nothing to close, and not in an alert dialog. */} + {/* An alert dialog interrupts to ask for a decision, and a corner X is a way out + without answering one. The cancel action in the footer is the way out. */} + {isInDialog(dialog) && dialog.role !== 'alertdialog' ? : null}
{children}
diff --git a/packages/ui/src/mosaic/components/dialog/alert-dialog.test.tsx b/packages/ui/src/mosaic/components/dialog/alert-dialog.test.tsx index 3783da325af..91bbe33279a 100644 --- a/packages/ui/src/mosaic/components/dialog/alert-dialog.test.tsx +++ b/packages/ui/src/mosaic/components/dialog/alert-dialog.test.tsx @@ -1,9 +1,9 @@ -import * as stylex from '@stylexjs/stylex'; 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 { Card } from '../card'; import type { DialogRootProps } from './dialog'; import { Dialog } from './dialog'; @@ -24,19 +24,23 @@ function Confirm({ onOpenChange, ...rest }: Partial = {}) { onOpenChange={onOpenChange} > - Discard changes? - This address has not been saved. - - Keep editing - - + + + Discard changes? + This address has not been saved. + + + Keep editing + + + ); } describe('role="alertdialog"', () => { - it('renders as an alertdialog, named and described by its parts', () => { + it('renders as an alertdialog, named and described by the card inside it', () => { render(); const popup = screen.getByRole('alertdialog', { name: 'Discard changes?' }); @@ -50,8 +54,12 @@ describe('role="alertdialog"', () => { role='alertdialog' > - Discard changes? - This address has not been saved. + + + Discard changes? + This address has not been saved. + + , ); @@ -66,26 +74,38 @@ describe('role="alertdialog"', () => { 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-dialog-actions')).toBeInTheDocument(); }); - it('is always the prompt size, and warns when asked for another', () => { + // The role decides the dismissal policy, and nothing else: an alert is a `Card` at the card + // size like every other dialog, and may be asked for any size and placement. + it('takes the size it is given, like any other dialog', () => { const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + render(); + + expect(document.querySelector('.cl-dialog-popup')).toHaveAttribute('data-variant', 'card'); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + // A corner X answers the question by leaving, which is the one thing an alert must not offer. + it('renders no dismiss in the card header, unlike a plain dialog', () => { + const { unmount } = render(); + expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument(); + unmount(); + render( - - - Discard changes? - This address has not been saved. + + + + + Add email address + + , ); - expect(document.querySelector('.cl-dialog-popup')).toHaveAttribute('data-size', 'prompt'); - expect(warn).toHaveBeenCalledWith(expect.stringContaining('size="profile"')); - warn.mockRestore(); + expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument(); }); it('opens from a trigger', async () => { @@ -94,8 +114,12 @@ describe('role="alertdialog"', () => { Delete - Delete this key? - Applications using it stop working. + + + Delete this key? + Applications using it stop working. + + , ); @@ -107,7 +131,9 @@ describe('role="alertdialog"', () => { expect(screen.getByRole('alertdialog')).toBeInTheDocument(); }); - it('opens focused on the cancel button, as the first element in the actions row', async () => { + // With no corner dismiss to take it, the opening focus lands on the first button in the footer + // — which is why the cancel is rendered first. + it('opens focused on the cancel button, as the first element in the footer', async () => { render(); // `FloatingFocusManager` moves focus asynchronously after mount, so this waits rather than @@ -148,11 +174,15 @@ describe('focus', () => { onOpenChange={setConfirmOpen} > - Discard changes? - This address has not been saved. - - Keep editing - + + + Discard changes? + This address has not been saved. + + + Keep editing + + @@ -187,15 +217,6 @@ describe('dismissal', () => { expect(screen.queryByRole('alertdialog')).not.toBeInTheDocument(); }); - it('keeps Escape out too under closedBy="none"', async () => { - const user = userEvent.setup(); - render(); - - await user.keyboard('{Escape}'); - - expect(screen.getByRole('alertdialog')).toBeInTheDocument(); - }); - it('lets a controlled consumer decline a close', async () => { const user = userEvent.setup(); @@ -212,11 +233,15 @@ describe('dismissal', () => { }} > - Discard changes? - This address has not been saved. - - Keep editing - + + + Discard changes? + This address has not been saved. + + + Keep editing + + ); @@ -239,7 +264,11 @@ describe('dev warnings', () => { role='alertdialog' > - Discard changes? + + + Discard changes? + + , ); @@ -247,7 +276,7 @@ describe('dev warnings', () => { await settle(); expect(warn).toHaveBeenCalledWith(expect.stringContaining('no description')); - expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); warn.mockRestore(); }); @@ -256,7 +285,11 @@ describe('dev warnings', () => { render( - Notifications + + + Notifications + + , ); @@ -277,7 +310,11 @@ describe('dev warnings', () => { role='alertdialog' > - This address has not been saved. + + + This address has not been saved. + + , ); @@ -285,7 +322,7 @@ describe('dev warnings', () => { await settle(); expect(warn).toHaveBeenCalledWith(expect.stringContaining('no accessible name')); - expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); + expect(warn).toHaveBeenCalledWith(expect.stringContaining('')); warn.mockRestore(); }); @@ -300,90 +337,28 @@ describe('dev warnings', () => { }); }); -describe('Dialog.Actions', () => { - it('composes consumer xstyle onto the row', () => { - const caller = stylex.create({ actions: { marginBlockStart: '2rem' } }); - render( - - - Discard changes? - This address has not been saved. - - Keep editing - - - , - ); - - expect(screen.getByTestId('actions')).toHaveClass( - 'cl-dialog-actions', - stylex.props(caller.actions).className ?? '', - ); - }); - - it('merges the className a render source hands the row', () => { - render( - - - Discard changes? - This address has not been saved. - } - data-testid='actions' - > - Keep editing - - - , - ); - - expect(screen.getByTestId('actions')).toHaveClass('cl-dialog-actions', 'from-render'); - }); - - it('renders as another element through render', () => { - render( - - - Discard changes? - This address has not been saved. -