F131: move add/edit capture into accessible modal dialogs

specs/product/GLOSSARY.md

@@ -322,3 +322,15 @@ RFC 4180 (UTF-8 with BOM), dates ISO-8601, money as raw minor units with
 currency columns — a copy of what's stored, never a re-formatting. Cells
 that a spreadsheet would run as a formula are prefixed so they render as
 text. Scheduled backups belong to Phase 1.5 (spec §17.5).
+
+**Dialog (#131)** — the popup that adding and correcting happen in. A
+**New …** button sits beside the screen's title, and each row's **edit** or
+**correct** link opens the same form seeded with that row's values; a
+drill-down panel's actions (record a purchase, feed usage, a stock
+correction, a bird movement, a customer payment) work the same way. The list
+never shifts to make room for a form, and closing — **Cancel**, Escape, or a
+click outside — leaves the data untouched. What a dialog submits is exactly
+what the old inline form submitted: same fields, same validation, same
+idempotency and version guards. Screens whose whole job is capture keep their
+form on the page: **Daily entry**, **Water**, recording an expense, and
+adding lines to a draft order.

web/package.json

@@ -22,6 +22,7 @@
   "devDependencies": {
     "@testing-library/jest-dom": "^6.9.1",
     "@testing-library/react": "^16.3.2",
+    "@testing-library/user-event": "^14.6.1",
     "@types/node": "^22.10.2",
     "@types/react": "^19.1.0",
     "@types/react-dom": "^19.1.0",

web/src/components/Dialog.test.tsx

@@ -0,0 +1,256 @@
+import { describe, it, expect, vi } from "vitest";
+import { render, screen, waitFor } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { useState } from "react";
+import { Dialog } from "./Dialog";
+
+function Body() {
+  return (
+    <form>
+      <input aria-label="Name" />
+      <input aria-label="Note" />
+      <button type="submit">Save</button>
+    </form>
+  );
+}
+
+// A realistic host: a trigger button that opens the dialog, so focus return has
+// somewhere to go back to.
+function Host({ onClose }: { onClose?: () => void } = {}) {
+  const [open, setOpen] = useState(false);
+  return (
+    <>
+      <button onClick={() => setOpen(true)}>New grade</button>
+      <Dialog
+        open={open}
+        title="New grade"
+        onClose={() => { setOpen(false); onClose?.(); }}
+      >
+        <Body />
+      </Dialog>
+    </>
+  );
+}
+
+describe("Dialog", () => {
+  it("renders nothing until opened, then exposes an accessible modal", async () => {
+    const user = userEvent.setup();
+    render(<Host />);
+    expect(screen.queryByRole("dialog")).toBeNull();
+
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+
+    const dialog = screen.getByRole("dialog");
+    expect(dialog).toHaveAttribute("aria-modal", "true");
+    // The heading names the dialog via aria-labelledby, not a duplicated label.
+    expect(dialog).toHaveAccessibleName("New grade");
+  });
+
+  it("focuses the first field, not the close button", async () => {
+    const user = userEvent.setup();
+    render(<Host />);
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+    expect(screen.getByLabelText("Name")).toHaveFocus();
+  });
+
+  it("closes on Escape and returns focus to the trigger", async () => {
+    const user = userEvent.setup();
+    const onClose = vi.fn();
+    render(<Host onClose={onClose} />);
+    const trigger = screen.getByRole("button", { name: "New grade" });
+    await user.click(trigger);
+
+    await user.keyboard("{Escape}");
+
+    expect(onClose).toHaveBeenCalledTimes(1);
+    expect(screen.queryByRole("dialog")).toBeNull();
+    expect(trigger).toHaveFocus();
+  });
+
+  it("closes on the close button", async () => {
+    const user = userEvent.setup();
+    render(<Host />);
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+
+    await user.click(screen.getByRole("button", { name: "Close" }));
+
+    expect(screen.queryByRole("dialog")).toBeNull();
+  });
+
+  it("closes on a backdrop click but not on a click inside the panel", async () => {
+    const user = userEvent.setup();
+    render(<Host />);
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+
+    // A click that lands on the panel bubbles to the backdrop handler; it must
+    // not be mistaken for a dismiss.
+    await user.click(screen.getByRole("dialog"));
+    expect(screen.getByRole("dialog")).toBeInTheDocument();
+
+    await user.click(document.querySelector(".dialog-backdrop")!);
+    expect(screen.queryByRole("dialog")).toBeNull();
+  });
+
+  it("traps Tab inside the panel in both directions", async () => {
+    const user = userEvent.setup();
+    render(<Host />);
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+
+    const name = screen.getByLabelText("Name");
+    const close = screen.getByRole("button", { name: "Close" });
+    const save = screen.getByRole("button", { name: "Save" });
+
+    expect(name).toHaveFocus();
+    await user.tab();
+    expect(screen.getByLabelText("Note")).toHaveFocus();
+    await user.tab();
+    expect(save).toHaveFocus();
+    // Past the last control it wraps to the first — the close button, which is
+    // first in DOM order — rather than escaping to the page behind.
+    await user.tab();
+    expect(close).toHaveFocus();
+
+    await user.tab({ shift: true });
+    expect(save).toHaveFocus();
+  });
+
+  it("locks page scroll while open and restores it on close", async () => {
+    const user = userEvent.setup();
+    document.body.style.overflow = "visible";
+    render(<Host />);
+
+    await user.click(screen.getByRole("button", { name: "New grade" }));
+    expect(document.body.style.overflow).toBe("hidden");
+
+    await user.keyboard("{Escape}");
+    expect(document.body.style.overflow).toBe("visible");
+  });
+});
+
+// ---------------------------------------------------------------------------
+// Review follow-ups (codex + feature-dev on PR #132)
+// ---------------------------------------------------------------------------
+
+// Mirrors the real screens: saving closes the dialog while the write is still
+// in flight, and the row's trigger is `disabled={busy}` for one more render.
+// focus() is a no-op on a disabled control, so a naive restore drops focus to
+// <body> exactly when a keyboard user needs it most.
+function BusyHost() {
+  const [open, setOpen] = useState(false);
+  const [busy, setBusy] = useState(false);
+  async function save() {
+    setBusy(true);
+    setOpen(false); // closes while still busy — the trigger is disabled here
+    await Promise.resolve();
+    setBusy(false);
+  }
+  return (
+    <>
+      <button disabled={busy} onClick={() => setOpen(true)}>edit</button>
+      <Dialog open={open} title="Edit grade" onClose={() => setOpen(false)}>
+        <form onSubmit={(e) => { e.preventDefault(); void save(); }}>
+          <input aria-label="Name" />
+          <button type="submit">Save</button>
+        </form>
+      </Dialog>
+    </>
+  );
+}
+
+describe("Dialog focus return when the trigger is momentarily disabled", () => {
+  it("retries on the next frame instead of dropping focus to the page", async () => {
+    const user = userEvent.setup();
+    render(<BusyHost />);
+    const trigger = screen.getByRole("button", { name: "edit" });
+    await user.click(trigger);
+
+    await user.click(screen.getByRole("button", { name: "Save" }));
+
+    expect(screen.queryByRole("dialog")).toBeNull();
+    // The first restore attempt hit a disabled button; the retry lands it.
+    await waitFor(() => expect(trigger).toHaveFocus());
+  });
+});
+
+// A 409 swaps the server's newer record into the dialog while it stays open.
+function RebindHost() {
+  const [record, setRecord] = useState<{ id: string } | null>(null);
+  return (
+    <>
+      <button onClick={() => setRecord({ id: "v1" })}>correct</button>
+      <button onClick={() => setRecord({ id: "v2" })}>simulate conflict</button>
+      <Dialog
+        open={record !== null}
+        title="Correct"
+        onClose={() => setRecord(null)}
+        focusKey={record}
+      >
+        <form>
+          <input aria-label="Amount" />
+          <button type="submit">Save correction</button>
+        </form>
+      </Dialog>
+    </>
+  );
+}
+
+describe("Dialog focusKey", () => {
+  it("pulls focus back to the first field when the record is swapped underneath", async () => {
+    const user = userEvent.setup();
+    render(<RebindHost />);
+    await user.click(screen.getByRole("button", { name: "correct" }));
+    const amount = screen.getByLabelText("Amount");
+    expect(amount).toHaveFocus();
+
+    // Move focus off the first field, then rebind to a different record.
+    await user.click(screen.getByRole("button", { name: "Save correction" }));
+    expect(amount).not.toHaveFocus();
+    await user.click(screen.getByRole("button", { name: "simulate conflict" }));
+
+    // The form under the cursor is not the one being filled in any more.
+    expect(screen.getByLabelText("Amount")).toHaveFocus();
+  });
+
+  it("does not re-grab focus on an unrelated re-render", async () => {
+    const user = userEvent.setup();
+    render(<RebindHost />);
+    await user.click(screen.getByRole("button", { name: "correct" }));
+
+    await user.click(screen.getByRole("button", { name: "Save correction" }));
+    await user.keyboard("x"); // a re-render that does not change the record
+
+    expect(screen.getByLabelText("Amount")).not.toHaveFocus();
+  });
+});
+
+describe("Dialog focus trap skips controls the browser would not tab to", () => {
+  function SkipHost() {
+    return (
+      <Dialog open title="New item" onClose={() => {}}>
+        <form>
+          <input type="hidden" aria-label="Hidden field" />
+          <input aria-label="First real field" />
+          <button type="submit">Save</button>
+          <button type="button" tabIndex={-1} aria-label="Programmatic only">skip me</button>
+        </form>
+      </Dialog>
+    );
+  }
+
+  it("lands initial focus past a hidden input", () => {
+    render(<SkipHost />);
+    expect(screen.getByLabelText("First real field")).toHaveFocus();
+  });
+
+  it("treats the last TABBABLE control as the wrap boundary", async () => {
+    const user = userEvent.setup();
+    render(<SkipHost />);
+
+    await user.tab(); // First real field -> Save
+    expect(screen.getByRole("button", { name: "Save" })).toHaveFocus();
+    // Save is the last tabbable control: the tabindex="-1" button is not a
+    // boundary, so Tab must wrap inside rather than escape to the page.
+    await user.tab();
+    expect(screen.getByRole("button", { name: "Close" })).toHaveFocus();
+  });
+});