diff --git a/web/src/lib/__tests__/unsavedChanges.test.tsx b/web/src/lib/__tests__/unsavedChanges.test.tsx new file mode 100644 index 0000000..77427f0 --- /dev/null +++ b/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()); +} + +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(); + }); +}); diff --git a/web/src/lib/unsavedChanges.ts b/web/src/lib/unsavedChanges.ts index 0ac475a..4ecb72f 100644 --- a/web/src/lib/unsavedChanges.ts +++ b/web/src/lib/unsavedChanges.ts @@ -91,7 +91,11 @@ export async function confirmLeaveUnsaved(): Promise { 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"), diff --git a/web/src/styles/app.css b/web/src/styles/app.css index c228195..0ccb704 100644 --- a/web/src/styles/app.css +++ b/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; } diff --git a/web/src/ui/dialog.tsx b/web/src/ui/dialog.tsx index 8931a5b..cedef34 100644 --- a/web/src/ui/dialog.tsx +++ b/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" && (
{req.choices?.map((c) => ( -