refactor(ui): adopt the dialog close confirmation - #9441
Closed
maxyinger wants to merge 18 commits into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 0a2373f The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
maxyinger
force-pushed
the
max/adopt-close-confirmation
branch
from
August 13, 2026 17:35
3a0462c to
0be4aff
Compare
maxyinger
added a commit
that referenced
this pull request
Aug 13, 2026
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) <noreply@anthropic.com>
maxyinger
force-pushed
the
max/adopt-close-confirmation
branch
from
August 13, 2026 19:59
0be4aff to
677fdd2
Compare
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/electron
@clerk/electron-passkeys
@clerk/eslint-plugin
@clerk/expo
@clerk/expo-google-signin
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/hono
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/react
@clerk/react-router
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/ui
@clerk/upgrade
@clerk/vue
commit: |
Contributor
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
maxyinger
added a commit
that referenced
this pull request
Aug 13, 2026
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) <noreply@anthropic.com>
maxyinger
force-pushed
the
max/adopt-close-confirmation
branch
from
August 13, 2026 22:24
677fdd2 to
86ed2d0
Compare
This was referenced Aug 13, 2026
maxyinger
added a commit
that referenced
this pull request
Aug 13, 2026
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) <noreply@anthropic.com>
maxyinger
force-pushed
the
max/adopt-close-confirmation
branch
from
August 13, 2026 23:08
86ed2d0 to
40d7533
Compare
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.
`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.
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) <noreply@anthropic.com>
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.
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.
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.
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.
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.
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.
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.
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) <noreply@anthropic.com>
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".
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.
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) <noreply@anthropic.com>
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 `<AlertDialog.Confirm>` 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.
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 `<AlertDialog.Confirm>` unmounts rather than poisoning the handle. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
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) <noreply@anthropic.com>
maxyinger
force-pushed
the
max/adopt-close-confirmation
branch
from
August 14, 2026 15:01
40d7533 to
0a2373f
Compare
Collaborator
Author
|
Closing: #9452 removed the Emotion slot-recipe engine from Mosaic, deleting |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
dataChanged, exported ashasUnsavedEditsDialog.Close— was a baresend({ type: 'CANCEL' })that bypassedonOpenChangeDestructivegets no confirmation — a delete in flight can't be called backclosedBy='none'while deleting,closerequestotherwisePreview
Swingset links tbd
/organization/organization-profile-profile-section— real machine, fake 800ms update/organization/organization-profile-delete-section— same for the deleteChecklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change