feat(components): give alert-dialog's confirm button a destructive variant (objectui#8978) - #9199
Merged
os-tesla merged 2 commits intoSep 11, 2026
Merged
Conversation
…variant (objectui#8978) `AlertDialogSchema` declares `actionVariant?: 'default' | 'destructive'` and the renderer reads it onto the confirm button, so the red destructive confirm decision batch #70 granted on 2026-09-07 is finally authorable. ⛔ NOT under the spelling that ruling named. `confirmVariant` is retired with a tombstone (objectui#7963) because it was measured inert — it reached no DOM node at all — and re-adding a published spelling that reds today would be exactly the churn the maintainer flagged on 2026-09-10. It still reds, with the same code; only its MESSAGE moved, from "no replacement" to naming `actionVariant`. `actionVariant` is the `action*` dialect this node already uses for that button (`actionText`, `onAction`, `AlertDialogAction`), and the dialect the retirement itself pointed `confirmLabel` at. Two values and not `ButtonSchema.variant`'s six, by measurement rather than by deference: `packages/components/src/ui/**` is a No-Touch zone (AGENTS.md #7) and `AlertDialogAction` bakes in `cn(buttonVariants(), className)` with no variant prop, so the variant arrives as a className OVERRIDE. An override displaces only a baked-in class in its own tailwind-merge group, and rendered through the real renderer `outline` / `ghost` / `link` leave the primitive's background and/or text colour showing underneath while `default` / `destructive` land clean. Declaring a value the node cannot render is the retired key's disease one level down. The pin reads the confirm button's own `class` off the rendered DOM, never that a prop was passed, with the expected tokens computed FROM `buttonVariants` instead of typed in. Two legs so a probe on the wrong node cannot read clean — the destructive dialog carries the tokens and loses the ones they displace, the default dialog carries neither — plus an `UNTOUCHED` control proving a document without the key renders a byte-identical dialog, which is the blast-radius answer for every `AlertDialogAction` call site in the tree. The two schema-catalog fixtures whose confirm button authored `variant: "destructive"` before PR #7962 stripped it get it back. Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ Co-Authored-By: Claude <noreply@anthropic.com>
os-tesla
added
domain:ui
Claude
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
…eted (objectui#8978) Found by the ablation, ⛔ not by review. With the renderer's `actionVariant` wiring removed, the three `MECHANISM-` legs (`outline` / `ghost` / `link`) stayed GREEN: each asserted only that the primitive's baked-in classes are still on the node, and "still there" is trivially true of a renderer that applies nothing at all. An assertion that cannot fail through the very ablation that proves the feature is the exact shape the retired key got away with for months, so it is fixed rather than noted: each leg now asserts ARRIVAL first — the tokens the variant itself emits ARE on the node — and only then the loss. Ablation matrix after the fix: 7 failed | 10 passed, against 4 failed | 13 passed before it. Every leg that names the wiring reds; the four controls, the default-path legs and the authoring-face legs stay green, which is what they are for. Also widens the two class-oracle helpers to the full variant union so `MECHANISM+` can ask about `default`, whose displaced set is correctly empty (`tsc -p packages/components/tsconfig.test.json` caught the narrow parameter). Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
Collaborator
Author
PM review — ACCEPT, contract review PASS, armed SQUASHRead against R2 — the tombstone holds, verified at source. R5 — the brief's own suggested route was wrong, and the seat caught it. My dispatch inherited the card's suggestion to edit ⭐ Three things I want on the record, because each is a habit worth keeping:
Contract review: additive Generated by Claude Code |
os-tesla
deleted the
claude/issue-8978-alert-dialog-destructive-confirm
branch
September 11, 2026 17:52
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
2 participants
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.
Fixes #8978
alert-dialogcan paint the red destructive confirm.AlertDialogSchemadeclaresactionVariant?: 'default' | 'destructive', the renderer reads it onto the confirmbutton, and a DOM pin with a firing control holds it there.
The shape, and the argument against the tombstone
R2 required this to be argued rather than assumed, so here it is in full.
The spelling is
actionVariant, ⛔ notconfirmVariant. The retired key staysretired: a document that reds on
confirmVarianttoday still reds on it, with thesame
invalid_typecode. Re-adding a published spelling that refuses an authortoday and accepts one tomorrow is precisely
「协议不应该改来改去啊，否则元数据应用怎么办」 (maintainer, 2026-09-10). What did move
is one published string — that tombstone's MESSAGE, from "it has NO surviving
spelling … a separate question that needs its own card" to naming
actionVariant.This card is that card, so the message was stale the moment it landed here.
actionVariantis not a new dialect. This node already calls that buttonactionin three places —
actionText,onAction,AlertDialogAction— and the sameretirement pointed
confirmLabelatactionTextfor exactly that reason. Usingconfirm*for the variant whileconfirm*for the label is tombstoned would havepublished two prefixes for one button.
The value domain is two, ⛔ not
ButtonSchema.variant's six — by measurement.See M1/M5 below. Declaring a value this node cannot actually render would be the
confirmVariantdisease one level down, at the value instead of the key, and thepin is built so that widening the union without widening the mechanism reds.
M1 — which variants does
buttonVariantsadmit, and isdestructiveamong themPrinted from
packages/components/src/ui/button.tsx, not assumed. Six:defaultdestructiveoutlinesecondaryghostlink, plus asizeaxis,with
defaultVariants: { variant: 'default', size: 'default' }.destructiveemits
bg-destructive text-destructive-foreground hover:bg-destructive/90; thedefaultthe confirm button is pinned to emitsbg-primary text-primary-foreground hover:bg-primary/90. ⇒ yes,destructiveis among them, and
buttonVariants()===buttonVariants({ variant: undefined })byte-for-byte (measured), which is what makes the wiring free of blast radius.
⭐ The pin never types those class names in. It computes the expected token set
from
buttonVariantsat run time, so an upstream rename follows instead of goingstale, and an
ORACLEcontrol fails if the two variants ever stop differing.M2 — blast radius of
AlertDialogAction's defaultThe default is NOT changed, so the radius is zero — and that is a DOM reading,
not an argument. Every
AlertDialogActioncall site in the tree, enumerated bygit grep:apps/console/.../ApprovalsInboxPage.tsx(4),packages/app-shell/.../InvitationsPage.tsx,MembersPage.tsx,SettingsPage.tsx(2),
views/ActionConfirmDialog.tsx,packages/components/.../NotificationAlerts.tsx(2),
packages/plugin-detail/src/RelatedList.tsx,packages/plugin-form/src/DrawerForm.tsx,ModalForm.tsx,packages/plugin-gantt/src/ObjectGantt.tsx, and the SDUI renderer. Every one ofthem passes no variant. Under this change the renderer passes
className=undefinedwhen the key is unauthored, and
cn(buttonVariants(), undefined)is byte-identicalto
cn(buttonVariants())— asserted as the pin'sUNTOUCHEDcontrol, whichcompares two whole rendered dialogs.
packages/components/src/ui/alert-dialog.tsxto pass a variant — is forbidden byAGENTS.md Commandment #7, a No-Touch zone whose files are regenerated by
pnpm shadcn:update, andalert-dialogis one of the tracked components(
pnpm shadcn:list). The sanctioned escape hatch is a declared patch inscripts/shadcn-local-patches.mjs, whose own header says to keep the payload outof
src/ui/. So the variant is applied as a className OVERRIDE from the renderer —which is not an invention:
NotificationAlerts.tsxalready expresses a footervariant on this very button the same way, with a comment saying why.
M3 — the "authored it three times and had to drop it" catalog sites
Found. Commit
ed5fee9be(PR #7962) converted four fixtures underexamples/schema-catalog/src/schemas/components-overlay-alert-dialog/off aphantom
actionsarray, and its own message says "the per-buttonvariantisDROPPED". The dropped confirm-button variants:
basic-alert-dialogconfirmation-dialogcustom-actionsdestructive-actionreading that yields exactly three is a different denominator: the dropped variants
the primitives could not reproduce for free — two destructive confirms plus the
ghostcancel inconfirmation-dialog(the twooutlinecancels and the twodefaultconfirms are what the primitives already emit). That is a plausiblereconstruction of the ruling's sentence, ⛔ not a fact I measured it against. The
two destructive confirms are restored here; the
ghostcancel is not, and wouldneed a
cancelVariantkey that nothing has asked for — noted below, not filed.M4 — the card's anchors, re-measured by TEXT
The card read
2c208d5bb; this branch is based onfe9e0d054. Located by text,never by line number, and all three are still exactly as the card describes:
import { buttonVariants } from "./button", the confirm button'sclassName={cn(buttonVariants(), className)}with no argument, the cancelbutton's
buttonVariants({ variant: "outline" }), and the renderer's{schema.actionText && …AlertDialogAction onClick={schema.onAction}…}. Thepremise holds; only the line numbers moved.
M5 — does an authorable key need to be added at all
Yes, and the sweep is what says so. The repo has a strong idiom for authoring a
button variant — a key spelled
variant, typed atbuttonVariants' domain, on thenode that owns the button:
ButtonSchema.variantinpackages/types/src/form.tsread at
renderers/form/button.tsx, plus the same spelling on theaction:*family,
badge,alert,toggle-groupand more (18 registrations declare avariantinput; 20-odd renderers readschema.variant).⛔ But no existing idiom reaches this button.
AlertDialogSchemahas novariantkey and no footer slot; the footer is generated from two strings, and the
contentslot renders above the footer and is not anAlertDialogAction. Thefixtures in M3 prove the point from the author's side: they reached for
type: 'button'nodes withvariant— the idiom — and the renderer carried noneof it. So the idiom governs the spelling and value domain, not whether a key is
needed.
And then the value domain, measured through the real renderer rather than inherited
from the idiom:
defaultdestructivesecondaryoutlinetext-primary-foregroundghostbg-primary text-primary-foregroundlinkbg-primary hover:bg-primary/90An override can only displace a baked-in class that shares its tailwind-merge
group, and those three set no background and/or no text colour, so the primitive's
own survives underneath. The
MECHANISM-legs of the pin render each of the threethrough the renderer and assert the leftovers are still on the node — so this table
is re-derived on every run, not recorded.
The pin, and the ablation matrix
packages/components/src/__tests__/alert-dialog-action-variant-8978.test.tsxrenders through the real
SchemaRendererand the real registry and reads theconfirm button's
classattribute. ⛔ No leg asserts that a prop was passed, that acomponent received a value, or that a source file contains a spelling.
Ablation: the wiring line in the renderer replaced by the pre-fix
{schema.actionText && …AlertDialogAction onClick=…}, proven on disk by grepbefore and after, restored by
git checkout HEAD --and proven restored by anempty
git diff HEAD.Ran twice, because the FIRST run caught a bad assertion of my own.
WIRED/ORACLE/INSTRUMENT/UNTOUCHED(4 controls)MECHANISM+defaultMECHANISM+destructiveMECHANISM-outline/ghost/link⭐ The
MECHANISM-row is the finding. With the wiring deleted those threelegs stayed GREEN, because each asserted only that the primitive's baked-in
classes were still on the node — and "still there" is trivially true of a
renderer that applies nothing at all. That is an assertion that cannot fail
through the very ablation that proves the feature, i.e. exactly the shape
confirmVariantgot away with for months. ⛔ Not noted and left: each leg nowasserts ARRIVAL first (the tokens the variant itself emits ARE on the node) and
only then the loss, and all three red under ablation. Second commit on this
branch.
The legs that still stay green under ablation are the ones that do not name the
wiring: the four controls, the default path (unchanged by construction — which
is leg 2's whole job), and the authoring-face legs, which are schema facts.
On-disk proof, both runs:
HEADblob hash printed, anchorgrep -c1 before /0 after, post-mutation blob hash differs from
HEAD's, restore bygit checkout HEAD -- <absolute path>from anEXIT INT TERMtrap, restorationproven by an empty
git diff HEAD, not by an exit code.Acceptance notes
Found and deliberately NOT fixed, none of them filed:
noted, not filed— theghostcancel variant.confirmation-dialog.jsonauthored
variant: 'ghost'on its cancel button before PR fix(examples): author the four alert-dialog fixtures in the dialect the renderer reads (objectui#7693) #7962 stripped it.AlertDialogCancelhard-codesoutline, and there is nocancelVariantkey.This card is the destructive confirm; a cancel variant is a different
capability with no ruling behind it, and the same override channel would carry
neither
ghostnorlinkanyway. Handler if it is ever wanted: whoever widensthe override mechanism.
noted, not filed—NotificationAlerts.tsx'sALERT_ACTION_CLASSES. Ahand-written map of override classes for this exact button, including an explicit
bg-transparentto neutralise the baked-in default — i.e. the repo already knewabout the leak measured in M5. It is internal React props, not authorable
metadata, so it is not in any filing category; it is cited here because it is the
precedent the renderer's approach follows. Handler: whoever widens the union.
noted, not filed— the card's:15/:115/:128line citations. Drifted, asthe card itself warned. Nothing to fix; recorded so M4's re-measurement is not
read as a correction to the card.
Verification
All at
4a617ceb8.pnpm --filter @object-ui/types test— 177 files, 3460 tests, all passedpnpm --filter @object-ui/components test— 265 files, 2430 tests, all passedvitest run examples/schema-catalog/— 31 files, 2151 tests, all passed(the two edited fixtures render through the real registry there)
pnpm --filter @object-ui/types --filter @object-ui/components run type-check—Done on both,
tsconfig.test.jsonincluded, which is what compiles the@ts-expect-errorandEquallegspnpm --filter @object-ui/types --filter @object-ui/components run lint—0 errors on both (279 / 953 pre-existing warnings), which is the whole
population
turbo run lintcovers for the two packages this diff touches. Thenarrowing is measurable: type-aware linting is NOT enabled (the flat config
extends
tseslint.configs.recommended, andlanguageOptionscarries onlyecmaVersion+globals— noproject, noprojectService), so this diffcannot move a verdict on a file it does not contain.
changeset:check·check:changeset-claims·check-changeset-presence·check:doc-types·check:doc-fences·check:doc-example-ids·check:new-line-citations·check:handler-key-reads·check:control-bytes·check:side-effects-array·check:unreferenced-sources·check:spec-symbols·check:element-data-source-declaration·check:prompt-keys·check:vi-mock-specifiersartefacts a whole-tree build produces and the container's foreground cap does
not allow:
check:sdui-registration-pins(exit 2, "No console build to weighat apps/console/dist/assets — this is exit 2, not a pass"),
check:doc-snippetsandcheck:doc-examples(both exit 2 on the sameprerequisite). ⛔ None of the three is recorded as green. The cheap sibling of
the first,
check:side-effects-array, IS green, and this diff adds no moduleand no registration — only an
inputsentry on one that already existed.pnpm shadcn:checknot run: it is a weekly scheduled workflow needing thenetwork, and this diff deliberately touches no file under
packages/components/src/ui/.Two existing pins were re-derived rather than deleted, both on the other side of a
question they had answered honestly at the time: the
confirmVariantrefusalmessage leg (it asserted the message names NO survivor; it now asserts the message
names
actionVariant, while the sentence ruling the two LABEL keys out is kept andstill asserted), and the read-dialect pin's comment that said the block names no
substitute.
Generated by Claude Code