Highlight saving, not discarding, on the unsaved-changes guard

The guard shipped with "Discard changes" as the only choice carrying a
colour -- a filled red button, against a plain outlined "Save changes" --
which made losing the work the loudest thing in a dialog whose entire
purpose is to stop that. The emphasis belongs on the safe answer.

A dialog choice can now be marked `primary`, and Save is. Discard keeps its
`danger` flag, but a danger choice is drawn the way `.menu-item.danger`
already is: a red label on the ordinary surface. In a list of answers a
filled red button is not "this one is destructive", it is "this one is the
default", which is the opposite of what it meant here.

That rendering change reaches the other choice dialog too -- the calendar's
"this occurrence or the whole series", where both answers are marked danger
because both delete something. Two filled red buttons become two red labels
and nothing is highlighted, which is right: neither answer there is the safe
one, so neither should look like it.

Checked in the browser against the mock, in both themes. Light: #dc2626 on
white, 4.8:1. Dark: the theme's own --danger, which every palette already
tunes for contrast on this surface.

Reported on #175 by the reporter's colleague, who is right that the
non-destructive action is the one that normally gets the highlight.

web/src/lib/__tests__/unsavedChanges.test.tsx

@@ -0,0 +1,110 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import { act } from "react";
+import { createRoot, type Root } from "react-dom/client";
+
+/*
+ * The guard's answers, and which of them the dialog leans on.
+ *
+ * It shipped with "Discard changes" as the only choice carrying a colour, which
+ * made losing the work the easy thing to click on a dialog whose entire purpose
+ * is to stop that (#175). The emphasis belongs on the safe answer; the
+ * destructive one stays legible as destructive without being the loudest thing
+ * in the box.
+ */
+
+const choiceDialog = vi.fn();
+vi.mock("@/ui/dialog", () => ({ choiceDialog: (...args: unknown[]) => choiceDialog(...args) }));
+
+const { confirmLeaveUnsaved, useUnsavedChanges } = await import("@/lib/unsavedChanges");
+
+interface Choice { value: string; label: string; hint?: string; danger?: boolean; primary?: boolean }
+
+const save = vi.fn(async () => true);
+const discard = vi.fn();
+
+function Editor() {
+  useUnsavedChanges({ dirty: true, save, discard, message: "Your filters have unsaved changes." });
+  return null;
+}
+
+let root: Root | null = null;
+let host: HTMLDivElement | null = null;
+
+/** One dirty editor on screen, which is what registers anything at all. */
+function mountDirtyEditor() {
+  host = document.createElement("div");
+  document.body.appendChild(host);
+  root = createRoot(host);
+  act(() => root!.render(<Editor />));
+}
+
+const asked = () => choiceDialog.mock.calls[0]![0] as { choices: Choice[]; cancelLabel: string; title: string; message: string };
+
+beforeEach(() => {
+  (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+  choiceDialog.mockReset().mockResolvedValue(null);
+  save.mockClear().mockResolvedValue(true);
+  discard.mockClear();
+  mountDirtyEditor();
+});
+
+afterEach(() => {
+  act(() => root!.unmount());
+  host!.remove();
+  root = null;
+  host = null;
+});
+
+describe("the unsaved-changes guard", () => {
+  it("highlights saving, not discarding", async () => {
+    await confirmLeaveUnsaved();
+    const [saveChoice, discardChoice] = asked().choices;
+    expect(saveChoice!.primary).toBe(true);
+    expect(discardChoice!.primary).toBeFalsy();
+  });
+
+  it("still says which answer is the destructive one", async () => {
+    await confirmLeaveUnsaved();
+    const [saveChoice, discardChoice] = asked().choices;
+    expect(discardChoice!.danger).toBe(true);
+    expect(discardChoice!.hint).toBeTruthy();
+    expect(saveChoice!.danger).toBeFalsy();
+  });
+
+  it("offers saving first, and staying as the way out", async () => {
+    await confirmLeaveUnsaved();
+    expect(asked().choices.map((c) => c.value)).toEqual(["save", "discard"]);
+    expect(asked().cancelLabel).toBeTruthy();
+  });
+
+  it("says what is about to be lost, in the editor's own words", async () => {
+    await confirmLeaveUnsaved();
+    expect(asked().message).toBe("Your filters have unsaved changes.");
+  });
+
+  it("stays put when the dialog is dismissed, and loses nothing", async () => {
+    choiceDialog.mockResolvedValue(null);
+    await expect(confirmLeaveUnsaved()).resolves.toBe(false);
+    expect(save).not.toHaveBeenCalled();
+    expect(discard).not.toHaveBeenCalled();
+  });
+
+  it("saves and goes when saving is chosen", async () => {
+    choiceDialog.mockResolvedValue("save");
+    await expect(confirmLeaveUnsaved()).resolves.toBe(true);
+    expect(save).toHaveBeenCalledOnce();
+  });
+
+  it("holds you on the page when the save fails", async () => {
+    choiceDialog.mockResolvedValue("save");
+    save.mockResolvedValue(false);
+    await expect(confirmLeaveUnsaved()).resolves.toBe(false);
+  });
+
+  it("discards and goes when discarding is chosen", async () => {
+    choiceDialog.mockResolvedValue("discard");
+    await expect(confirmLeaveUnsaved()).resolves.toBe(true);
+    expect(discard).toHaveBeenCalledOnce();
+    expect(save).not.toHaveBeenCalled();
+  });
+});

web/src/lib/unsavedChanges.ts

@@ -91,7 +91,11 @@ export async function confirmLeaveUnsaved(): Promise<boolean> {
       title: t("Save your changes?"),
       message: pending.message,
       choices: [
-        { value: "save", label: t("Save changes") },
+        // Save is the highlighted one. Highlighting the destructive answer makes
+        // losing the work the easy thing to click, which is the failure this
+        // dialog exists to prevent -- #175, after the guard shipped with the
+        // emphasis the wrong way round.
+        { value: "save", label: t("Save changes"), primary: true },
         { value: "discard", label: t("Discard changes"), hint: t("What you changed here will be lost."), danger: true },
       ],
       cancelLabel: t("Stay here"),

web/src/styles/app.css

@@ -715,6 +715,10 @@ a.menu-item:hover { color: var(--fg); }
 .dialog-choices { display: flex; flex-direction: column; gap: 8px; }
 .dialog-choice { display: flex; flex-direction: column; align-items: flex-start; gap: 2px; width: 100%; text-align: left; padding: 10px 12px; height: auto; }
 .dialog-choice small { font-weight: 400; opacity: 0.75; }
+/* Destructive, the way .menu-item.danger is: a red label on the ordinary
+   surface. A filled red button in a list of answers is the loudest thing in the
+   dialog, which is wrong when it is the answer that loses your work. */
+.dialog-choice.danger { color: var(--danger); }
 
 /* Toasts ----------------------------------------------------------------- */
 .toast-host { position: fixed; left: 50%; bottom: 24px; transform: translateX(-50%); z-index: 3000; display: flex; flex-direction: column; gap: 8px; align-items: center; pointer-events: none; padding: 0 12px; width: 100%; max-width: 520px; }

web/src/ui/dialog.tsx

@@ -93,6 +93,17 @@ export interface DialogChoice {
   /** Shown under the label, for the choice that needs the caveat. */
   hint?: string;
   danger?: boolean;
+  /**
+   * The safe answer, given the weight a dialog's confirm button has.
+   *
+   * A list of choices has no default until one is said to be, and the
+   * destructive one must not become it by being the only thing with a colour --
+   * which is what "Discard changes" was, on a guard whose whole purpose is to
+   * stop you losing work ([#175]).
+   *
+   * [#175]: https://github.com/Coffey-Labs/ihasmail/issues/175
+   */
+  primary?: boolean;
 }
 
 interface ConfirmRequest {
@@ -178,7 +189,7 @@ export function ConfirmHost() {
       {req.kind === "choice" && (
         <div className="dialog-choices">
           {req.choices?.map((c) => (
-            <button key={c.value} className={`btn dialog-choice ${c.danger ? "btn-danger" : ""}`} onClick={() => done(c.value)}>
+            <button key={c.value} className={`btn dialog-choice ${c.primary ? "btn-primary" : ""} ${c.danger ? "danger" : ""}`} onClick={() => done(c.value)}>
               <span>{c.label}</span>
               {c.hint && <small>{c.hint}</small>}
             </button>