mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 04:32:42 +00:00
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.
This commit is contained in:
1 parent
dc704b5e63
commit
9a8b95cfd0
21 files changed
+181
No files matched your search
@@ -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("<Confirm")) continue;
|
||||
const sf = ts.createSourceFile(rel, text, ts.ScriptTarget.Latest, true);
|
||||
const visit = (node: ts.Node) => {
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
@@ -191,6 +191,7 @@ function CoachPlansBody({ readOnly }: { readOnly: boolean }) {
|
||||
{(readOnly || group === "past") && (
|
||||
<ConfirmButton
|
||||
slot="coach-plan-delete"
|
||||
destructive
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="min-h-9"
|
||||
|
||||
@@ -187,6 +187,7 @@ export function AiServerKeySection() {
|
||||
{configured && (
|
||||
<ConfirmButton
|
||||
slot="admin-ai-server-key-remove"
|
||||
destructive
|
||||
variant="outline"
|
||||
size="sm"
|
||||
label={t("admin.aiServerKey.remove")}
|
||||
|
||||
@@ -196,6 +196,7 @@ export function CentralCodexSection() {
|
||||
<SettingsCardActions>
|
||||
<ConfirmButton
|
||||
slot="admin-central-codex-disconnect"
|
||||
destructive
|
||||
variant="destructive"
|
||||
size="sm"
|
||||
className="min-h-11 shrink-0 sm:min-h-9"
|
||||
|
||||
@@ -880,6 +880,7 @@ export function LogDaySheet({
|
||||
{currentRowId ? (
|
||||
<ConfirmButton
|
||||
slot="cycle-day-delete"
|
||||
destructive
|
||||
variant="outline"
|
||||
className="text-destructive hover:text-destructive mr-auto"
|
||||
icon={<Trash2 className="h-4 w-4" />}
|
||||
@@ -1471,6 +1472,7 @@ export function SymptomRow({
|
||||
</button>
|
||||
<ConfirmDialog
|
||||
slot="cycle-custom-symptom-remove"
|
||||
destructive
|
||||
open={confirmRemove}
|
||||
onOpenChange={setConfirmRemove}
|
||||
title={t("cycle.symptom.custom.removeTitle")}
|
||||
|
||||
@@ -194,6 +194,7 @@ export function AboutMeNoteManager() {
|
||||
{hasNote && (
|
||||
<ConfirmButton
|
||||
slot="settings-about-me-clear"
|
||||
destructive
|
||||
label={t("settings.ai.aboutMe.clear")}
|
||||
icon={<Trash2 className="size-4" aria-hidden />}
|
||||
variant="outline"
|
||||
|
||||
@@ -176,6 +176,7 @@ export function AuditCPanel({
|
||||
className="text-destructive size-11 shrink-0 sm:size-9"
|
||||
icon={<Trash2 className="size-4" aria-hidden />}
|
||||
slot={`audit-c-remove-${entry.id}`}
|
||||
destructive
|
||||
/>
|
||||
</li>
|
||||
))}
|
||||
@@ -214,6 +215,7 @@ export function AuditCPanel({
|
||||
className="text-destructive size-11 shrink-0 sm:size-9"
|
||||
icon={<Trash2 className="size-4" aria-hidden />}
|
||||
slot="audit-c-legacy-remove"
|
||||
destructive
|
||||
/>
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
@@ -127,6 +127,7 @@ export function GrantsGivenCard() {
|
||||
return (
|
||||
<ConfirmButton
|
||||
slot="grant-revoke"
|
||||
destructive
|
||||
size="sm"
|
||||
label={
|
||||
grant.state === "PENDING"
|
||||
|
||||
@@ -229,6 +229,7 @@ export function GrantsReceivedCard() {
|
||||
</Button>
|
||||
<ConfirmButton
|
||||
slot="grant-renounce"
|
||||
destructive
|
||||
size="sm"
|
||||
label={t("recordSharing.received.renounce")}
|
||||
title={t("recordSharing.received.renounceTitle", { name })}
|
||||
|
||||
@@ -188,6 +188,7 @@ function ManagedProfileRow({
|
||||
{guardians.data && (
|
||||
<ConfirmButton
|
||||
slot="managed-profile-delete"
|
||||
destructive
|
||||
size="sm"
|
||||
className="min-h-11 sm:min-h-9"
|
||||
label={t("recordSharing.managed.delete")}
|
||||
|
||||
@@ -172,6 +172,7 @@ export function ManagedProfileGuardians({
|
||||
<div className="flex shrink-0 items-center">
|
||||
<ConfirmButton
|
||||
slot="managed-profile-guardian-remove"
|
||||
destructive
|
||||
size="sm"
|
||||
className="min-h-11 sm:min-h-9"
|
||||
label={t("recordSharing.managed.guardianRemove")}
|
||||
|
||||
@@ -154,6 +154,7 @@ export function AvatarSection() {
|
||||
{avatarUrl && (
|
||||
<ConfirmButton
|
||||
slot="settings-avatar-remove"
|
||||
destructive
|
||||
variant="ghost"
|
||||
className="text-destructive min-h-11 sm:min-h-9"
|
||||
icon={<Trash2 className="h-4 w-4" />}
|
||||
|
||||
@@ -210,6 +210,7 @@ export function CodexProviderForm({
|
||||
) : isConnected ? (
|
||||
<ConfirmButton
|
||||
slot="settings-codex-disconnect"
|
||||
destructive
|
||||
variant="outline"
|
||||
size="sm"
|
||||
className="text-destructive min-h-11 shrink-0 sm:min-h-9"
|
||||
|
||||
@@ -228,6 +228,7 @@ function FactRow({
|
||||
accessible name and the confirmation carry the words. */}
|
||||
<ConfirmButton
|
||||
slot="settings-coach-memory-forget"
|
||||
destructive
|
||||
variant="ghost"
|
||||
size="icon"
|
||||
className="size-11 sm:size-9"
|
||||
|
||||
@@ -147,6 +147,7 @@ export function CoachRemindersSection({
|
||||
)}
|
||||
<ConfirmButton
|
||||
slot="coach-reminder-delete"
|
||||
destructive
|
||||
variant="ghost"
|
||||
size="sm"
|
||||
className="text-muted-foreground hover:text-foreground min-h-9"
|
||||
|
||||
@@ -450,6 +450,7 @@ export function EnvironmentSection() {
|
||||
</span>
|
||||
<ConfirmButton
|
||||
slot="travel-remove"
|
||||
destructive
|
||||
size="icon"
|
||||
variant="ghost"
|
||||
className="min-h-11 min-w-11 sm:min-h-9 sm:min-w-9"
|
||||
|
||||
@@ -216,6 +216,7 @@ export function SecuritySessionsCard({
|
||||
{!s.isCurrent && (
|
||||
<ConfirmButton
|
||||
slot="revoke-session"
|
||||
destructive
|
||||
size="sm"
|
||||
className="min-h-9 shrink-0"
|
||||
label={t("settings.security.revokeSession")}
|
||||
@@ -237,6 +238,7 @@ export function SecuritySessionsCard({
|
||||
<SettingsCardActions>
|
||||
<ConfirmButton
|
||||
slot="sign-out-everywhere"
|
||||
destructive
|
||||
className="min-h-11 sm:min-h-9"
|
||||
label={t("settings.security.signOutEverywhere")}
|
||||
title={t("settings.security.signOutEverywhereConfirmTitle")}
|
||||
|
||||
@@ -137,6 +137,7 @@ export function TrustedDevicesCard({
|
||||
</div>
|
||||
<ConfirmButton
|
||||
slot="revoke-trusted-device"
|
||||
destructive
|
||||
size="sm"
|
||||
className="min-h-9 shrink-0"
|
||||
label={t("settings.security.trustedDevices.revoke")}
|
||||
@@ -157,6 +158,7 @@ export function TrustedDevicesCard({
|
||||
<SettingsCardActions>
|
||||
<ConfirmButton
|
||||
slot="revoke-all-trusted-devices"
|
||||
destructive
|
||||
className="min-h-11 sm:min-h-9"
|
||||
label={t("settings.security.trustedDevices.revokeAll")}
|
||||
title={t(
|
||||
|
||||
@@ -285,6 +285,7 @@ export function LifeEventForm({
|
||||
className="mr-auto min-h-11 sm:min-h-9"
|
||||
icon={<Trash2 className="size-4" aria-hidden="true" />}
|
||||
slot="life-event-delete"
|
||||
destructive
|
||||
/>
|
||||
)}
|
||||
{onCancel && (
|
||||
|
||||
@@ -160,6 +160,59 @@ describe("ConfirmBody — the action is the only path to the callback", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("the destructive style reaches the action from both shapes", () => {
|
||||
// `ConfirmButton` accepted `destructive` through its props type and dropped
|
||||
// it on the way to the dialog, so every delete confirmation built on it
|
||||
// offered a neutral primary where `ConfirmDialog` offered a red one.
|
||||
function actionVariant(element: ReactElement): unknown {
|
||||
const body = walk(element).find((el) => nameOf(el) === "ConfirmBody")!;
|
||||
const rendered = walk(
|
||||
(body.type as (p: unknown) => ReactElement)(body.props),
|
||||
);
|
||||
const action = rendered.find((el) => nameOf(el) === "AlertDialogAction")!;
|
||||
return (action.props as Record<string, unknown>).variant;
|
||||
}
|
||||
|
||||
it("ConfirmButton forwards destructive to the dialog's action", () => {
|
||||
expect(
|
||||
actionVariant(
|
||||
ConfirmButton({
|
||||
label: "Delete",
|
||||
onConfirm: vi.fn(),
|
||||
destructive: true,
|
||||
...COPY,
|
||||
}) as ReactElement,
|
||||
),
|
||||
).toBe("destructive");
|
||||
});
|
||||
|
||||
it("ConfirmButton stays neutral when the act does not destroy", () => {
|
||||
expect(
|
||||
actionVariant(
|
||||
ConfirmButton({
|
||||
label: "Reset",
|
||||
onConfirm: vi.fn(),
|
||||
...COPY,
|
||||
}) as ReactElement,
|
||||
),
|
||||
).toBe("default");
|
||||
});
|
||||
|
||||
it("ConfirmDialog forwards it the same way", () => {
|
||||
expect(
|
||||
actionVariant(
|
||||
ConfirmDialog({
|
||||
open: true,
|
||||
onOpenChange: vi.fn(),
|
||||
onConfirm: vi.fn(),
|
||||
destructive: true,
|
||||
...COPY,
|
||||
}) as ReactElement,
|
||||
),
|
||||
).toBe("destructive");
|
||||
});
|
||||
});
|
||||
|
||||
describe("<ConfirmDialog> — the trigger-less half", () => {
|
||||
const onConfirm = vi.fn();
|
||||
const tree = walk(
|
||||
|
||||
@@ -114,6 +114,7 @@ export function ConfirmButton({
|
||||
ariaLabel,
|
||||
slot,
|
||||
extra,
|
||||
destructive = false,
|
||||
}: ConfirmCopy & {
|
||||
/** The control's own label, before the dialog opens. */
|
||||
label: string;
|
||||
@@ -156,6 +157,7 @@ export function ConfirmButton({
|
||||
pending={pending}
|
||||
slot={slot}
|
||||
extra={extra}
|
||||
destructive={destructive}
|
||||
/>
|
||||
</AlertDialog>
|
||||
);
|
||||
|
||||
Reference in new issue
Block a user