admin/src/definitions/DefinitionsTab.css

@@ -353,6 +353,16 @@
   color: var(--nq-brand-blue);
 }
 
+.defs-picker-option--locked {
+  color: var(--nq-a-ink-secondary);
+  cursor: default;
+}
+
+.defs-picker-note {
+  font-size: 12px;
+  font-weight: 600;
+}
+
 .defs-repeats {
   display: flex;
   gap: 8px;
@@ -493,6 +503,24 @@
   margin-top: 16px;
 }
 
+.defs-confirm {
+  margin-top: 12px;
+  padding: 10px 12px;
+  border-radius: var(--nq-a-density-radius);
+  background: var(--nq-a-danger-bg);
+  color: var(--nq-a-danger-strong);
+  font-size: 12.5px;
+  font-weight: 700;
+}
+
+.defs-confirm p {
+  margin: 0;
+}
+
+.defs-confirm .defs-sheet-footer {
+  margin-top: 10px;
+}
+
 .defs-cancel,
 .defs-save {
   padding: 13px 0;

admin/src/definitions/DefinitionsTab.test.tsx

@@ -152,6 +152,14 @@ function fakeApi(initial: QuestDefinition[]) {
               title: body.title ?? d.title,
               skip_on_away: body.skip_on_away ?? d.skip_on_away,
               rule: body.rule ?? d.rule,
+              // Like the API: a sent list replaces the whole set; an absent one keeps it.
+              assignees:
+                "assignee_child_ids" in body
+                  ? CHILDREN.filter((c) => body.assignee_child_ids.includes(c.id)).map((c) => ({
+                      id: c.id,
+                      display_name: c.display_name,
+                    }))
+                  : d.assignees,
             }
           : d,
       );
@@ -360,9 +368,13 @@ describe("DefinitionsTab", () => {
     expect((within(s).getByLabelText("Afternoon due time") as HTMLInputElement).value).toBe(
       "16:00",
     );
-    // The inactive child "Old" is not offered and not submitted.
+    // The inactive child "Old" is shown locked, not offered as a checkbox.
     fireEvent.click(within(s).getByRole("button", { name: /Assigned children/ }));
     expect(within(s).queryByRole("checkbox", { name: "Old" })).toBeNull();
+    const locked = within(s).getByTestId("inactive-assignee-4");
+    expect(locked.textContent).toContain("Old");
+    expect(locked.textContent).toContain("Inactive");
+    expect(locked.getAttribute("aria-disabled")).toBe("true");
     fireEvent.click(within(s).getByRole("checkbox", { name: "Maeve" }));
 
     fireEvent.change(within(s).getByRole("textbox", { name: "Title" }), {
@@ -382,8 +394,11 @@ describe("DefinitionsTab", () => {
     });
     fireEvent.click(within(s).getByRole("switch", { name: "Skip on away days" }));
 
+    // Changing the selection would drop "Old": confirm before sending.
+    fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    expect(api.writes()).toHaveLength(0);
     await act(async () => {
-      fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+      fireEvent.click(within(s).getByRole("button", { name: "Unassign and save" }));
     });
 
     expect(api.writes()).toEqual([
@@ -417,6 +432,125 @@ describe("DefinitionsTab", () => {
     expect(screen.queryByRole("dialog")).toBeNull();
   });
 
+  it("editing only the title omits assignee_child_ids and keeps an inactive assignee", async () => {
+    await renderReady();
+    fireEvent.click(screen.getByTestId("definition-11"));
+    const s = sheet();
+    expect(within(s).getByText(/Also assigned \(inactive\): Old\./)).toBeTruthy();
+    fireEvent.change(within(s).getByRole("textbox", { name: "Title" }), {
+      target: { value: "Bins out" },
+    });
+
+    await act(async () => {
+      fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    });
+
+    expect(screen.queryByTestId("unassign-confirm")).toBeNull();
+    expect(api.writes()).toHaveLength(1);
+    const body = api.writes()[0].body as Record<string, unknown>;
+    expect(api.writes()[0].method).toBe("PATCH");
+    expect(body.title).toBe("Bins out");
+    expect(body).not.toHaveProperty("assignee_child_ids");
+
+    await screen.findByText("Bins out");
+    expect(meta(screen.getByTestId("definition-11"))).toBe(
+      "Declan, Old · Every 2 weeks on Mon, Thu · Afternoon",
+    );
+  });
+
+  it("changing the active selection with an inactive assignee asks first", async () => {
+    await renderReady();
+    fireEvent.click(screen.getByTestId("definition-11"));
+    const s = sheet();
+    fireEvent.click(within(s).getByRole("button", { name: /Assigned children/ }));
+    fireEvent.click(within(s).getByRole("checkbox", { name: "Rory" }));
+
+    fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    const confirm = within(s).getByTestId("unassign-confirm");
+    expect(confirm.textContent).toContain("Saving will unassign Old.");
+    expect(document.activeElement).toBe(confirm);
+    expect(api.writes()).toHaveLength(0);
+
+    // Going back sends nothing and restores the normal footer.
+    fireEvent.click(within(confirm).getByRole("button", { name: "Go back" }));
+    expect(within(s).queryByTestId("unassign-confirm")).toBeNull();
+    expect(api.writes()).toHaveLength(0);
+
+    fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    await act(async () => {
+      fireEvent.click(within(s).getByRole("button", { name: "Unassign and save" }));
+    });
+    expect(api.writes()).toHaveLength(1);
+    expect((api.writes()[0].body as { assignee_child_ids: number[] }).assignee_child_ids).toEqual([
+      1, 3,
+    ]);
+  });
+
+  it("reverting the selection to the original omits assignee_child_ids without asking", async () => {
+    await renderReady();
+    fireEvent.click(screen.getByTestId("definition-11"));
+    const s = sheet();
+    fireEvent.click(within(s).getByRole("button", { name: /Assigned children/ }));
+    fireEvent.click(within(s).getByRole("checkbox", { name: "Rory" }));
+    fireEvent.click(within(s).getByRole("checkbox", { name: "Rory" }));
+
+    await act(async () => {
+      fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    });
+    expect(screen.queryByTestId("unassign-confirm")).toBeNull();
+    expect(api.writes()).toHaveLength(1);
+    expect(api.writes()[0].body).not.toHaveProperty("assignee_child_ids");
+  });
+
+  it("focus moves into the sheet on open and returns to the row on Escape", async () => {
+    await renderReady();
+    const row = screen.getByTestId("definition-10");
+    row.focus();
+    fireEvent.click(row);
+    const s = sheet();
+    expect(document.activeElement).toBe(within(s).getByRole("heading", { name: "Edit task" }));
+
+    fireEvent.keyDown(s, { key: "Escape" });
+    expect(screen.queryByRole("dialog")).toBeNull();
+    expect(document.activeElement).toBe(row);
+    expect(api.writes()).toHaveLength(0);
+  });
+
+  it("Tab and Shift+Tab wrap within the sheet", async () => {
+    await renderReady();
+    fireEvent.click(screen.getByTestId("definition-10"));
+    const s = sheet();
+    const title = within(s).getByRole("textbox", { name: "Title" });
+    const save = within(s).getByRole("button", { name: "Save changes" });
+
+    save.focus();
+    fireEvent.keyDown(s, { key: "Tab" });
+    expect(document.activeElement).toBe(title);
+
+    fireEvent.keyDown(s, { key: "Tab", shiftKey: true });
+    expect(document.activeElement).toBe(save);
+  });
+
+  it("Escape does not close the sheet while saving", async () => {
+    await renderReady();
+    fireEvent.click(screen.getByTestId("definition-10"));
+    let release: (response: Response) => void = () => {};
+    api.fetchMock.mockImplementationOnce(
+      () => new Promise<Response>((resolve) => (release = resolve)),
+    );
+    const s = sheet();
+    await act(async () => {
+      fireEvent.click(within(s).getByRole("button", { name: "Save changes" }));
+    });
+    fireEvent.keyDown(s, { key: "Escape" });
+    expect(screen.getByRole("dialog")).toBeTruthy();
+
+    await act(async () => {
+      release(jsonResponse(BRUSH));
+    });
+    await vi.waitFor(() => expect(screen.queryByRole("dialog")).toBeNull());
+  });
+
   it("the skip-on-away toggle reflects and persists a false value", async () => {
     await renderReady();
     fireEvent.click(screen.getByTestId("definition-11"));

admin/src/definitions/DefinitionsTab.tsx

@@ -4,7 +4,13 @@
  * The icon picker (§3.3) and the occurrence preview are separate tasks: an
  * existing definition's icon is carried through unchanged by never sending it.
  */
-import { useEffect, useRef, useState, type ReactNode } from "react";
+import {
+  useEffect,
+  useRef,
+  useState,
+  type KeyboardEvent as ReactKeyboardEvent,
+  type ReactNode,
+} from "react";
 import { ApiForbiddenError } from "../api/client";
 import {
   ApiRequestError,
@@ -14,6 +20,7 @@ import {
   updateDefinition,
   type AdminChild,
   type DefinitionCreateBody,
+  type DefinitionEditBody,
   type DefinitionRule,
   type QuestDefinition,
   type WindowEntry,
@@ -202,10 +209,14 @@ function buildRule(draft: Draft): DefinitionRule | string {
   }
 }
 
-function buildBody(draft: Draft): DefinitionCreateBody | string {
+/**
+ * `keepsAssignees`: an edit whose active selection is unchanged sends no
+ * assignee list, so an empty selection is fine when inactive children remain.
+ */
+function buildBody(draft: Draft, keepsAssignees = false): DefinitionCreateBody | string {
   const title = draft.title.trim();
   if (!title) return "Give the task a title.";
-  if (draft.assigneeIds.length === 0) return "Assign at least one child.";
+  if (draft.assigneeIds.length === 0 && !keepsAssignees) return "Assign at least one child.";
   if (draft.windows.length === 0) return "Pick at least one window.";
   const rule = buildRule(draft);
   if (typeof rule === "string") return rule;
@@ -231,6 +242,13 @@ function toggle<T>(list: T[], value: T): T[] {
   return list.includes(value) ? list.filter((item) => item !== value) : [...list, value];
 }
 
+function sameIds(a: number[], b: number[]): boolean {
+  return a.length === b.length && a.every((id) => b.includes(id));
+}
+
+const FOCUSABLE =
+  'button:not([disabled]), input:not([disabled]), select:not([disabled]), textarea:not([disabled]), [href], [tabindex]:not([tabindex="-1"])';
+
 /* ── List ───────────────────────────────────────────────────────────── */
 
 function FilterChips({
@@ -334,11 +352,58 @@ function EditSheet({
   const [draft, setDraft] = useState<Draft>(() =>
     target === "new" ? newDraft(activeChildren) : draftFrom(target, activeChildren),
   );
+  // The active selection as loaded: while it is unchanged, the PATCH omits
+  // assignee_child_ids, because the API replaces the whole set and inactive
+  // children cannot be sent back.
+  const [originalActiveIds] = useState(() => draft.assigneeIds);
+  const inactiveAssignees =
+    target === "new"
+      ? []
+      : target.assignees.filter((a) => !activeChildren.some((child) => child.id === a.id));
   const [pickerOpen, setPickerOpen] = useState(false);
   const [saving, setSaving] = useState(false);
+  const [confirming, setConfirming] = useState(false);
   const [error, setError] = useState<string | null>(null);
   // A ref, not state: two taps in one frame must not both see `saving === false`.
   const inFlight = useRef(false);
+  const sheetRef = useRef<HTMLDivElement>(null);
+  const titleRef = useRef<HTMLHeadingElement>(null);
+  const confirmRef = useRef<HTMLDivElement>(null);
+
+  // Modal focus: move in on open, hand back to the trigger on close.
+  useEffect(() => {
+    const trigger = document.activeElement as HTMLElement | null;
+    titleRef.current?.focus();
+    return () => {
+      if (trigger && trigger.isConnected) trigger.focus();
+    };
+  }, []);
+
+  useEffect(() => {
+    if (confirming) confirmRef.current?.focus();
+  }, [confirming]);
+
+  function onKeyDown(event: ReactKeyboardEvent<HTMLDivElement>) {
+    if (event.key === "Escape") {
+      event.preventDefault();
+      if (!inFlight.current) onCancel();
+      return;
+    }
+    if (event.key !== "Tab" || !sheetRef.current) return;
+    const focusable = Array.from(sheetRef.current.querySelectorAll<HTMLElement>(FOCUSABLE));
+    if (focusable.length === 0) return;
+    const first = focusable[0];
+    const last = focusable[focusable.length - 1];
+    const current = document.activeElement;
+    const outside = !focusable.includes(current as HTMLElement);
+    if (event.shiftKey && (current === first || outside)) {
+      event.preventDefault();
+      last.focus();
+    } else if (!event.shiftKey && (current === last || outside)) {
+      event.preventDefault();
+      first.focus();
+    }
+  }
 
   const update = (patch: Partial<Draft>) => setDraft((d) => ({ ...d, ...patch }));
 
@@ -347,19 +412,32 @@ function EditSheet({
     .map((child) => ({ id: child.id, display_name: child.display_name }));
   const orderedWindows = WINDOW_ORDER.filter((w) => draft.windows.includes(w));
 
-  async function save() {
+  async function save(confirmed = false) {
     if (inFlight.current) return;
-    const body = buildBody(draft);
+    const keepsAssignees = target !== "new" && sameIds(draft.assigneeIds, originalActiveIds);
+    const body = buildBody(draft, keepsAssignees);
     if (typeof body === "string") {
       setError(body);
+      setConfirming(false);
       return;
     }
+    if (target !== "new" && !keepsAssignees && inactiveAssignees.length > 0 && !confirmed) {
+      setError(null);
+      setConfirming(true);
+      return;
+    }
+    setConfirming(false);
     inFlight.current = true;
     setSaving(true);
     setError(null);
     try {
-      if (target === "new") await createDefinition(body);
-      else await updateDefinition(target.id, body);
+      if (target === "new") {
+        await createDefinition(body);
+      } else {
+        const edit: DefinitionEditBody = { ...body };
+        if (keepsAssignees) delete edit.assignee_child_ids;
+        await updateDefinition(target.id, edit);
+      }
       onSaved();
     } catch (err) {
       setError(saveErrorMessage(err));
@@ -378,13 +456,15 @@ function EditSheet({
         }}
       />
       <div
+        ref={sheetRef}
         className="defs-sheet"
         role="dialog"
         aria-modal="true"
         aria-labelledby="defs-sheet-title"
+        onKeyDown={onKeyDown}
       >
         <div className="defs-grabber" aria-hidden="true" />
-        <h3 className="defs-sheet-title" id="defs-sheet-title">
+        <h3 className="defs-sheet-title" id="defs-sheet-title" ref={titleRef} tabIndex={-1}>
           {target === "new" ? "New task" : "Edit task"}
         </h3>
         <div className="defs-fields">
@@ -431,8 +511,25 @@ function EditSheet({
                     </li>
                   );
                 })}
+                {inactiveAssignees.map((assignee) => (
+                  <li
+                    key={assignee.id}
+                    className="defs-picker-option defs-picker-option--locked"
+                    aria-disabled="true"
+                    data-testid={`inactive-assignee-${assignee.id}`}
+                  >
+                    <span>{assignee.display_name}</span>
+                    <span className="defs-picker-note">Inactive</span>
+                  </li>
+                ))}
               </ul>
             ) : null}
+            {inactiveAssignees.length > 0 ? (
+              <p className="defs-hint">
+                Also assigned (inactive): {inactiveAssignees.map((a) => a.display_name).join(", ")}.
+                Kept unless you change the selection.
+              </p>
+            ) : null}
           </Field>
 
           <Field label="Repeats" id="defs-label-repeats">
@@ -594,14 +691,39 @@ function EditSheet({
           </p>
         ) : null}
 
-        <div className="defs-sheet-footer">
-          <button type="button" className="defs-cancel" onClick={onCancel} disabled={saving}>
-            Cancel
-          </button>
-          <button type="button" className="defs-save" onClick={save} disabled={saving}>
-            {saving ? "Saving…" : "Save changes"}
-          </button>
-        </div>
+        {confirming ? (
+          <div
+            className="defs-confirm"
+            role="group"
+            aria-labelledby="defs-confirm-text"
+            data-testid="unassign-confirm"
+            ref={confirmRef}
+            tabIndex={-1}
+          >
+            <p id="defs-confirm-text">
+              Saving will unassign{" "}
+              {inactiveAssignees.map((a) => a.display_name).join(", ")}. Inactive children can't be
+              assigned again until they are reactivated.
+            </p>
+            <div className="defs-sheet-footer">
+              <button type="button" className="defs-cancel" onClick={() => setConfirming(false)}>
+                Go back
+              </button>
+              <button type="button" className="defs-save" onClick={() => save(true)}>
+                Unassign and save
+              </button>
+            </div>
+          </div>
+        ) : (
+          <div className="defs-sheet-footer">
+            <button type="button" className="defs-cancel" onClick={onCancel} disabled={saving}>
+              Cancel
+            </button>
+            <button type="button" className="defs-save" onClick={() => save()} disabled={saving}>
+              {saving ? "Saving…" : "Save changes"}
+            </button>
+          </div>
+        )}
       </div>
     </div>
   );