From 9a8b95cfd0cffdcd87d841be74da12cd5f9cf9fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 21 Oct 2026 12:02:13 +0200 Subject: [PATCH] fix(ui): let ConfirmButton's dialog wear the destructive style ConfirmButton took a `destructive` prop and dropped it on the way to its dialog, so every delete, revoke and remove built on it confirmed with a neutral primary button while ConfirmDialog confirmed the same kind of act in red. The prop is forwarded now, and the call sites whose act takes something away pass it. A guard holds every confirmation whose slot names a destroying act to the destructive style; resets stay neutral. --- .../confirm-destructive-style-guard.test.ts | 104 ++++++++++++++++++ src/app/coach/plans/page.tsx | 1 + .../admin/ai-server-key-section.tsx | 1 + .../admin/central-codex-section.tsx | 1 + src/components/cycle/log-day-sheet.tsx | 2 + .../records/about-me-note-manager.tsx | 1 + src/components/records/audit-c-panel.tsx | 2 + .../settings/access/grants-given-card.tsx | 1 + .../settings/access/grants-received-card.tsx | 1 + .../settings/access/managed-profile-card.tsx | 1 + .../access/managed-profile-guardians.tsx | 1 + .../account-section/avatar-section.tsx | 1 + .../settings/ai/codex-provider-form.tsx | 1 + .../settings/coach-memory-section.tsx | 1 + .../settings/coach-reminders-section.tsx | 1 + .../settings/environment-section.tsx | 1 + .../settings/security-sessions-card.tsx | 2 + .../settings/trusted-devices-card.tsx | 2 + src/components/timeline/life-event-form.tsx | 1 + .../ui/__tests__/confirm-button.test.tsx | 53 +++++++++ src/components/ui/confirm-button.tsx | 2 + 21 files changed, 181 insertions(+) create mode 100644 src/__tests__/confirm-destructive-style-guard.test.ts diff --git a/src/__tests__/confirm-destructive-style-guard.test.ts b/src/__tests__/confirm-destructive-style-guard.test.ts new file mode 100644 index 000000000..44ae1df62 --- /dev/null +++ b/src/__tests__/confirm-destructive-style-guard.test.ts @@ -0,0 +1,104 @@ +/** + * A confirmation whose act takes something away wears the destructive style. + * + * `ConfirmButton` accepted a `destructive` prop and dropped it on the way to + * its dialog, so every delete, revoke and remove built on it asked its + * question over a neutral primary button — while the documents vault, which + * uses `ConfirmDialog`, asked the same question in red. The primitive forwards + * it now (`confirm-button.test.tsx` pins the wiring); this file pins that the + * call sites ask for it. + * + * The judgement "does this act destroy" is read off the `slot` the call site + * already names: a slot that says delete, remove, revoke, renounce, forget, + * clear or disconnect is that act. A reset is deliberately not in the list: + * it puts back a default the person can change again, and stays neutral. + * A slot without one of those words is not checked, which is the honest + * limit of a name-based rule; the slot is the one label every call site + * carries, and it is what the destructive-control registry is keyed on too. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import ts from "typescript"; +import { describe, expect, it } from "vitest"; + +import { walkSourceFiles } from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); +const DESTROYING = /(delete|remove|revoke|renounce|forget|clear|disconnect)/i; +const TAGS = new Set(["ConfirmButton", "ConfirmDialog"]); + +interface Site { + file: string; + slot: string; + destructive: boolean; +} + +function slotText(attr: ts.JsxAttribute): string | null { + const init = attr.initializer; + if (!init) return null; + if (ts.isStringLiteral(init)) return init.text; + if (ts.isJsxExpression(init) && init.expression) { + const expr = init.expression; + if (ts.isStringLiteral(expr) || ts.isNoSubstitutionTemplateLiteral(expr)) { + return expr.text; + } + if (ts.isTemplateExpression(expr)) return expr.head.text; + } + return null; +} + +function collect(): Site[] { + const sites: Site[] = []; + for (const rel of walkSourceFiles(SRC, { + floor: 1200, + extensions: [".tsx"], + })) { + if (rel.includes("__tests__/") || rel.startsWith("generated/")) continue; + const text = readFileSync(join(SRC, rel), "utf8"); + if (!text.includes(" { + const opening = ts.isJsxSelfClosingElement(node) + ? node + : ts.isJsxElement(node) + ? node.openingElement + : null; + if (opening && TAGS.has(opening.tagName.getText(sf))) { + let slot: string | null = null; + let destructive = false; + for (const prop of opening.attributes.properties) { + if (!ts.isJsxAttribute(prop)) continue; + const name = prop.name.getText(sf); + if (name === "slot") slot = slotText(prop); + if (name === "destructive") destructive = true; + } + if (slot !== null) sites.push({ file: rel, slot, destructive }); + } + ts.forEachChild(node, visit); + }; + visit(sf); + } + return sites; +} + +describe("destroying confirmations wear the destructive style", () => { + const sites = collect(); + + it("finds the confirmations it is meant to check", () => { + // A matcher that found nothing would agree with any tree. + expect(sites.filter((s) => DESTROYING.test(s.slot)).length).toBeGreaterThan( + 15, + ); + }); + + it("every one whose slot names a destroying act passes `destructive`", () => { + const missing = sites + .filter((s) => DESTROYING.test(s.slot) && !s.destructive) + .map((s) => `${s.file}: ${s.slot}`); + expect( + missing, + "Pass `destructive` so the confirm button is red, or rename the slot if the act does not take anything away.", + ).toEqual([]); + }); +}); diff --git a/src/app/coach/plans/page.tsx b/src/app/coach/plans/page.tsx index f60a3cfc4..cd646d1e5 100644 --- a/src/app/coach/plans/page.tsx +++ b/src/app/coach/plans/page.tsx @@ -191,6 +191,7 @@ function CoachPlansBody({ readOnly }: { readOnly: boolean }) { {(readOnly || group === "past") && ( } @@ -1471,6 +1472,7 @@ export function SymptomRow({ } variant="outline" diff --git a/src/components/records/audit-c-panel.tsx b/src/components/records/audit-c-panel.tsx index 6f040d9b4..c986cff7d 100644 --- a/src/components/records/audit-c-panel.tsx +++ b/src/components/records/audit-c-panel.tsx @@ -176,6 +176,7 @@ export function AuditCPanel({ className="text-destructive size-11 shrink-0 sm:size-9" icon={} slot={`audit-c-remove-${entry.id}`} + destructive /> ))} @@ -214,6 +215,7 @@ export function AuditCPanel({ className="text-destructive size-11 shrink-0 sm:size-9" icon={} slot="audit-c-legacy-remove" + destructive /> ) : null} diff --git a/src/components/settings/access/grants-given-card.tsx b/src/components/settings/access/grants-given-card.tsx index 71dc4d500..bf7a01b72 100644 --- a/src/components/settings/access/grants-given-card.tsx +++ b/src/components/settings/access/grants-given-card.tsx @@ -127,6 +127,7 @@ export function GrantsGivenCard() { return ( } diff --git a/src/components/settings/ai/codex-provider-form.tsx b/src/components/settings/ai/codex-provider-form.tsx index 78bfce68f..e8d0a18c1 100644 --- a/src/components/settings/ai/codex-provider-form.tsx +++ b/src/components/settings/ai/codex-provider-form.tsx @@ -210,6 +210,7 @@ export function CodexProviderForm({ ) : isConnected ? (