Improved Custom Field Saving and Display

frontend/src/components/InlineEditCell.test.tsx

@@ -0,0 +1,57 @@
+import { describe, it, expect, vi } from "vitest";
+import { render, fireEvent } from "@solidjs/testing-library";
+import { createSignal } from "solid-js";
+
+vi.mock("./Toast", () => ({ showToast: vi.fn() }));
+
+import InlineEditCell from "./InlineEditCell";
+
+function flush() {
+  return new Promise((r) => setTimeout(r, 0));
+}
+
+describe("InlineEditCell boolean", () => {
+  it("keeps the new checked state after save resolves even if props.value is stale", async () => {
+    // Parent does not refetch after save, so props.value stays at the old value.
+    const onSave = vi.fn().mockResolvedValue(undefined);
+    const { container } = render(() => (
+      <InlineEditCell columnType="boolean" value="false" onSave={onSave} />
+    ));
+    const box = container.querySelector("input[type=checkbox]") as HTMLInputElement;
+    expect(box.checked).toBe(false);
+
+    fireEvent.click(box);
+    await flush();
+
+    expect(onSave).toHaveBeenCalledWith("true");
+    // Bug: setSaving(false) re-ran the sync effect with the stale prop and
+    // reverted the checkbox to unchecked.
+    expect(box.checked).toBe(true);
+  });
+
+  it("still syncs when props.value actually changes from outside", async () => {
+    const [value, setValue] = createSignal<string | undefined>("false");
+    const { container } = render(() => (
+      <InlineEditCell columnType="boolean" value={value()} onSave={vi.fn()} />
+    ));
+    const box = container.querySelector("input[type=checkbox]") as HTMLInputElement;
+    expect(box.checked).toBe(false);
+
+    setValue("true");
+    await flush();
+    expect(box.checked).toBe(true);
+  });
+
+  it("reverts to the previous value when save fails", async () => {
+    const onSave = vi.fn().mockRejectedValue(new Error("boom"));
+    const { container } = render(() => (
+      <InlineEditCell columnType="boolean" value="false" onSave={onSave} />
+    ));
+    const box = container.querySelector("input[type=checkbox]") as HTMLInputElement;
+
+    fireEvent.click(box);
+    await flush();
+
+    expect(box.checked).toBe(false);
+  });
+});

frontend/src/components/InlineEditCell.tsx

@@ -1,4 +1,4 @@
-import { createSignal, Show, For, createEffect } from "solid-js";
+import { createSignal, Show, For, createEffect, on, untrack } from "solid-js";
 import { showToast } from "./Toast";
 import { getErrorMessage } from "../utils/errors";
 
@@ -16,11 +16,15 @@ export default function InlineEditCell(props: Props) {
   const [saving, setSaving] = createSignal(false);
 
   // Sync from props when external data changes (e.g. refetch), but never
-  // stomp on an in-flight save that has not yet resolved.
-  createEffect(() => {
-    const incoming = props.value;
-    if (!saving()) setLocalValue(incoming);
-  });
+  // stomp on an in-flight save that has not yet resolved. Track only
+  // props.value: tracking saving() too would re-run this when a save
+  // finishes and clobber the just-committed value with a stale prop.
+  createEffect(on(
+    () => props.value,
+    (incoming) => {
+      if (!untrack(saving)) setLocalValue(incoming);
+    },
+  ));
 
   // Confirm-then-commit: keep the old value visible (with a spinner) until the
   // save resolves. On success show the new value; on failure keep the old value
@@ -56,12 +60,9 @@ export default function InlineEditCell(props: Props) {
     if (e.key === "Escape") setEditing(false);
   }
 
-  const Spinner = () => (
-    <span
-      class="inline-block w-3 h-3 ml-1 align-middle border border-theme-border border-t-theme-accent rounded-full animate-spin"
-      aria-label="Saving"
-    />
-  );
+  // Saving feedback is the greyed-out disabled control itself; anything that
+  // mounts next to it (spinner) flashes on fast LAN saves and reads as a
+  // rendering artifact.
 
   // Boolean: simple checkbox
   if (props.columnType === "boolean") {
@@ -71,10 +72,19 @@ export default function InlineEditCell(props: Props) {
           type="checkbox"
           checked={localValue() === "true"}
           disabled={saving()}
-          onChange={(e) => void save(e.currentTarget.checked ? "true" : "false")}
-          class="cursor-pointer disabled:opacity-50"
+          onChange={(e) => {
+            const el = e.currentTarget;
+            const want = el.checked ? "true" : "false";
+            // Confirm-then-commit: snap the DOM back to the committed value;
+            // a successful save flips localValue and re-checks it. Without
+            // this a failed save leaves the DOM out of sync, since the
+            // unchanged signal never rewrites the checked attribute.
+            el.checked = localValue() === "true";
+            void save(want);
+          }}
+          class="cursor-pointer disabled:opacity-40"
+          title={saving() ? "Saving..." : undefined}
         />
-        <Show when={saving()}><Spinner /></Show>
       </span>
     );
   }
@@ -90,14 +100,14 @@ export default function InlineEditCell(props: Props) {
             const val = e.currentTarget.value;
             if (val) void save(val);
           }}
-          class="px-1 py-0.5 rounded border border-theme-border bg-theme-input text-theme-text-primary text-sm disabled:opacity-50"
+          class="px-1 py-0.5 rounded border border-theme-border bg-theme-input text-theme-text-primary text-sm disabled:opacity-40"
+          title={saving() ? "Saving..." : undefined}
         >
           <option value="">-</option>
           <For each={props.dropdownOptions ?? []}>
             {(opt) => <option value={opt}>{opt}</option>}
           </For>
         </select>
-        <Show when={saving()}><Spinner /></Show>
       </span>
     );
   }
@@ -110,11 +120,10 @@ export default function InlineEditCell(props: Props) {
         <span
           onClick={startEdit}
           class="cursor-pointer min-w-[2rem] inline-block hover:bg-theme-hover rounded px-1"
-          classList={{ "opacity-60 cursor-wait": saving() }}
+          classList={{ "opacity-40 cursor-wait": saving() }}
           title={saving() ? "Saving..." : "Click to edit"}
         >
           {localValue() || "-"}
-          <Show when={saving()}><Spinner /></Show>
         </span>
       }
     >