fix(forms): Add Contact closes on save, New Template keeps its error (QA FORM-01, FORM-06)

app/components/contacts/ContactModal.test.tsx

@@ -8,8 +8,8 @@
  * edits into a stated preference, and a stated preference is the only thing the
  * column is evidence of.
  */
-import { describe, it, expect } from "vitest";
-import { render, within } from "@testing-library/react";
+import { describe, it, expect, vi } from "vitest";
+import { render, within, fireEvent, waitFor } from "@testing-library/react";
 import { createRoutesStub } from "react-router";
 
 import { ContactModal } from "./ContactModal";
@@ -70,3 +70,25 @@ describe("ContactModal — preferred language", () => {
     expect(select.value).toBe("");
   });
 });
+
+describe("ContactModal - closing after a save (FORM-01)", () => {
+  it("closes when the first save comes back ok, and not before", async () => {
+    const onClose = vi.fn();
+    const Stub = createRoutesStub([
+      {
+        path: "/contacts",
+        Component: () => <ContactModal open onClose={onClose} contact={null} />,
+        action: async () => ({ ok: true }),
+      },
+    ]);
+    const { findByLabelText, getByRole } = render(<Stub initialEntries={["/contacts"]} />);
+
+    fireEvent.change(await findByLabelText(/^Full Name/), { target: { value: "Ada Lovelace" } });
+    expect(onClose).not.toHaveBeenCalled();
+    fireEvent.click(getByRole("button", { name: "Save" }));
+
+    // The pre-fix modal read the PREVIOUS result at click time (none), so this
+    // never fired on a first save and the dialog sat open after a 200.
+    await waitFor(() => expect(onClose).toHaveBeenCalledTimes(1));
+  });
+});

app/components/contacts/ContactModal.tsx

@@ -1,3 +1,4 @@
+import { useEffect, useRef } from "react";
 import { useFetcher } from "react-router";
 import { useForm, type SubmissionResult } from "@conform-to/react";
 import { parseWithZod } from "@conform-to/zod/v4";
@@ -40,8 +41,18 @@ export function ContactModal({
     shouldRevalidate: "onInput",
   });
 
-  // Close after a successful submission (fetcher.data has ok:true).
+  // Close when THIS submission settles ok. The old onSubmit read `fetcherOk`
+  // from the render the click happened in, i.e. the previous result, so a
+  // successful first save never closed the dialog (QA FORM-01). The modal
+  // stays mounted, so ok:true from an earlier save is still in fetcher.data:
+  // only a busy -> idle transition counts, not the value alone.
   const fetcherOk = (fetcher.data as { ok?: boolean } | undefined)?.ok;
+  const wasBusy = useRef(false);
+  useEffect(() => {
+    const busy = fetcher.state !== "idle";
+    if (wasBusy.current && !busy && fetcherOk) onClose();
+    wasBusy.current = busy;
+  }, [fetcher.state, fetcherOk, onClose]);
 
   return (
     <Modal
@@ -59,10 +70,7 @@ export function ContactModal({
       <fetcher.Form
         method="post"
         id={form.id}
-        onSubmit={(e) => {
-          form.onSubmit(e);
-          if (fetcherOk) setTimeout(onClose, 200);
-        }}
+        onSubmit={form.onSubmit}
         noValidate
         className="space-y-4"
       >

app/components/templates/CreateTemplateModal.tsx

@@ -44,6 +44,8 @@ export function CreateTemplateModal({
         <input
           ref={nameRef}
           value={newName}
+          // The API refuses a name over 100 characters (templates.schema).
+          maxLength={100}
           onChange={(e) => setNewName(e.target.value)}
           onKeyDown={(e) => e.key === "Enter" && handleCreate()}
           placeholder={m.templates_create_name_placeholder()}

app/routes/templates.test.tsx

@@ -44,7 +44,9 @@ const A_TEMPLATE: TemplateFixture = {
     updatedAt: "2026-08-01T10:00:00.000Z",
 };
 
-function renderPage(opts: { templates?: TemplateFixture[]; url?: string } = {}) {
+function renderPage(
+    opts: { templates?: TemplateFixture[]; url?: string; action?: () => unknown } = {},
+) {
     const templates = opts.templates ?? [];
     const context = {
         branding: { defaultLocale: "en-US", defaultTimezone: "UTC" },
@@ -68,6 +70,7 @@ function renderPage(opts: { templates?: TemplateFixture[]; url?: string } = {})
                 {
                     path: "templates",
                     Component: TemplatesPage,
+                    action: opts.action,
                     loader: () => ({
                         templates,
                         meta: {
@@ -186,3 +189,23 @@ describe("/templates — one front door", () => {
         expect(screen.queryByRole("link", { name: /New Template/i })).toBeNull();
     });
 });
+
+describe("/templates - a refused create stays visible (FORM-06)", () => {
+    it("keeps the dialog open and shows the server's reason", async () => {
+        renderPage({
+            url: "/templates?new=1",
+            action: () => ({ error: "Name must be 100 characters or fewer" }),
+        });
+
+        const dialog = await screen.findByRole("dialog");
+        const input = within(dialog).getByRole("textbox");
+        // The field itself stops a typed or pasted name at the API's limit.
+        expect(input).toHaveAttribute("maxlength", "100");
+        fireEvent.change(input, { target: { value: "Residential" } });
+        fireEvent.click(within(dialog).getByRole("button", { name: /create/i }));
+
+        // Pre-fix the dialog closed on click, so the refusal had nowhere to show.
+        expect(await screen.findByText("Name must be 100 characters or fewer")).toBeVisible();
+        expect(screen.getByRole("dialog")).toBeInTheDocument();
+    });
+});

app/routes/templates.tsx

@@ -188,9 +188,11 @@ export default function TemplatesPage() {
   /* ---- Actions ---- */
   const handleCreate = () => {
     if (!newName.trim()) return;
-    if (!submit({ intent: "create", name: newName.trim() }, { method: "post" })) return;
-    setCreateOpen(false);
-    setNewName("");
+    // Stays open until the action answers: a success navigates to the editor,
+    // a failure renders `error` inside the dialog. Closing here hid the
+    // refusal of an over-long name behind a dialog that was already gone
+    // (QA FORM-06).
+    submit({ intent: "create", name: newName.trim() }, { method: "post" });
   };
 
   const handleDuplicate = (t: Template) => {

docs/develop/fork-log.md

@@ -332,3 +332,5 @@ deliberate departure rather than work upstream simply has not done yet.
   looked" are the same silence otherwise.
 - Update the snapshot when you fetch, not when you remember.
 | `agreement` merge fields filled at send, placeholders block the send (2026-10-13 QA sweep DELIV-01, Blocker; no upstream fix) | `AgreementService.findOrCreate` (`server/services/agreement/envelope-legacy.ts`) runs the template through `fillAgreementTokens` (`server/lib/agreement-tokens.ts`) before it freezes `content_snapshot`: `[INSPECTOR_NAME]` (assigned inspector, else the company's legal or company name), `[CUSTOMER_NAME]`, `[PROPERTY_ADDRESS]`, `[INSPECTION_DATE]` (long civil day) and `$[AMOUNT]` (`getEffectivePriceCents`: invoice, then service lines, then `inspections.price`); `[CLIENT_SIGNATURE_BLOCK]` and `[SIGN_DATE]` become "(signature captured below)" and "(recorded at signing)", values HTML-escaped. If any other bracketed ALL-CAPS token remains, or the starter's "Review before sending to real customers" notice is still in the text, it throws 422 naming them and creates no envelope. `SendAgreementModal` takes `serverError` and shows it inside the dialog (the page-level line it replaces sat under the scrim, so Send looked dead). Specs: `tests/unit/agreements/agreement-send-tokens.spec.ts`, `app/components/agreements/SendAgreementModal.test.tsx` | The finding: a fresh company's only template could be sent and signed with every merge field still in brackets and the admin caveat in the permanent record. `git log main..upstream/main` by keyword (placeholder, merge field, starter agreement, agreement) and on the fixture file finds nothing; the upstream plan lists it under its PR 5 but upstream has not landed it. The starter text is unchanged, so an untouched starter is refused (notice and the liability clause both stay) until the company edits it in Settings; that is the intended state of a legal document, not a gap. **Known ceilings:** the match is `[A-Z][A-Z0-9_]{2,}`-style brackets only, so a custom template with lowercase or `{{x}}` fields is sent as written; the check runs only when an envelope is created (reusing a live envelope does not re-check); an inspection with no price blocks on `[AMOUNT]` rather than printing $0.00; the signing page itself still shows what the snapshot holds, so envelopes sent before this change keep their placeholders |
+| `ContactModal` closes after a save (2026-10-13 QA sweep FORM-01, Major; no upstream fix) | `app/components/contacts/ContactModal.tsx` closes on the fetcher's busy to idle transition with `ok:true`, in an effect, instead of reading `fetcherOk` inside `onSubmit`. Spec: `ContactModal.test.tsx` (proven red without the change) | The finding: the 200 created the contact but the dialog sat open with the form refilled. The old handler read the PREVIOUS result at click time, so it could only close on a second save. `upstream/main` (`435b24ec`) still has that line: its `793a4161` added the busy latch around it and left it. The transition (not the value) is the trigger because the modal stays mounted and an earlier `ok:true` is still in `fetcher.data`. No "Contact added" toast: the new row in the list is the confirmation, and a toast is a new string in both locales for this fork alone |
+| `/templates` create dialog stays open until the action answers (2026-10-13 QA sweep FORM-06, Major; no upstream fix) | `handleCreate` in `app/routes/templates.tsx` no longer closes the dialog and clears the name on click; success navigates to the editor, failure renders `error` in the dialog (it was already wired, to a dialog that had been closed). `CreateTemplateModal` gets `maxLength={100}`, the API's limit. Spec: `app/routes/templates.test.tsx` | The finding: a ~330-character name did nothing visible, because the API refused it after the dialog had closed. Checked `upstream/main` for the route and the modal: same close-on-click. **Known ceilings:** no character counter (a new string in both locales), so a typed name just stops at 100; the server's 400 for an over-long name is the generic "could not create" text if it ever gets past `maxLength` (the zod message is not read) |