From e805c624b273f62fa7253f9f84c9d9f18b317e2f Mon Sep 17 00:00:00 2001 From: Developer Date: Fri, 26 Jun 2026 13:56:27 +0000 Subject: [PATCH] Mobile Settings: machine editor SheetForm (Slice 7) Below md, the machine editor Dialog renders as a SheetForm (triggered by the same Edit/Add buttons via machineDialogOpen state). The shared MachineEditor body (fields + SSH validate button) renders inside the sheet; the ConfirmDialog is a sibling outside. Desktop Dialog is byte-for-byte identical. No navigation needed on close -- the Settings page content (tabbed cards, machine list) is always visible behind the sheet, so there is no stranding risk (unlike ServicePage where the sheet was the whole page). Added saveDisabled prop to SheetForm (additive, default false) so the machine editor can gate Save on required fields (name + host for SSH mode), matching the desktop DialogFooter confirmDisabled semantics. Scope note: SSHKeyManager is an inline two-panel layout (SelectionRailCard + SectionCard), not a dialog, and already stacks responsively via grid-cols-1 md:grid-cols-[...]. Wrapping it in SheetForm would break its always-visible selection rail. Left as-is. Tests: 3 new mobile cases (SheetForm render, save payload, cancel closes) + desktop unchanged. 113 tests pass; lint/build green. Refs openspec/changes/mobile-responsive-parity/ (spec R4, tasks slice 7). --- frontend/src/components/ui/sheet-form.tsx | 5 +- frontend/src/pages/Settings.tsx | 136 ++++++++++++------ .../src/pages/__tests__/Settings.test.tsx | 87 ++++++++++- 3 files changed, 180 insertions(+), 48 deletions(-) diff --git a/frontend/src/components/ui/sheet-form.tsx b/frontend/src/components/ui/sheet-form.tsx index 471b802..c19e1db 100644 --- a/frontend/src/components/ui/sheet-form.tsx +++ b/frontend/src/components/ui/sheet-form.tsx @@ -15,6 +15,8 @@ export interface SheetFormProps { isPending?: boolean; /** Override the Save button label (default "Save"). */ saveLabel?: string; + /** Disable the Save button (e.g. when required fields are empty). */ + saveDisabled?: boolean; children: React.ReactNode; /** Optional className applied to the scrolling body. */ bodyClassName?: string; @@ -39,6 +41,7 @@ export function SheetForm({ onSave, onCancel, isPending = false, + saveDisabled = false, saveLabel = "Save", children, bodyClassName, @@ -75,7 +78,7 @@ export function SheetForm({ - - ) : undefined - } - /> - - + {machineDraft.id ? ( + + ) : null} + + ) : ( + { + if (!open) closeMachineDialog(); + }} + > + + + + {machineDraft.id ? "Edit machine" : "Create machine"} + + + {machineDraft.mode === "local" + ? "Local API host" + : "SSH target"} + + + + { + void saveMachineDraft(machineDraft); + }} + confirmLabel={machineDraft.id ? "Save machine" : "Create machine"} + confirmDisabled={ + !machineDraft.name || + (machineDraft.mode === "ssh" && !machineDraft.host.trim()) + } + secondaryAction={ + machineDraft.id ? ( + + ) : undefined + } + /> + + + )} { expect(deleteMachineMutate).toHaveBeenCalledWith("m1"); }); }); + +// jsdom has no window.matchMedia; default to desktop so existing tests are +// unaffected. +function setMatchMedia(matches: boolean) { + window.matchMedia = ((query: string) => ({ + matches: query.includes("768") ? matches : false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, + })) as unknown as typeof window.matchMedia; +} + +describe("Settings (mobile SheetForm — slice 7)", () => { + beforeEach(() => setMatchMedia(true)); + + it("opens the machine editor in a SheetForm below md", async () => { + machines = [localMachine()]; + render(); + + // Open the editor via the detail-pane Edit button (visible text). + const detailEdit = screen + .getAllByRole("button", { name: "Edit" }) + .find((button) => button.textContent === "Edit") as HTMLButtonElement; + await userEvent.click(detailEdit); + + // SheetForm renders a dialog; the DialogTitle shows the editor title. + expect(screen.getByText("Edit machine")).toBeInTheDocument(); + expect(screen.getByRole("dialog")).toBeInTheDocument(); + // Desktop DialogDescription text is not rendered as a dialog description + // on mobile (the MachineEditor has its own hint labels, which is fine). + expect( + screen.queryByRole("heading", { name: "Create machine" }), + ).not.toBeInTheDocument(); + }); + + it("saves a machine via the SheetForm on mobile", async () => { + machines = [localMachine()]; + render(); + + const detailEdit = screen + .getAllByRole("button", { name: "Edit" }) + .find((button) => button.textContent === "Edit") as HTMLButtonElement; + await userEvent.click(detailEdit); + + const nameInput = screen.getByLabelText("Name"); + await userEvent.clear(nameInput); + await userEvent.type(nameInput, "Renamed node"); + + await userEvent.click(screen.getByRole("button", { name: "Save machine" })); + + expect(saveMachineMutate).toHaveBeenCalledTimes(1); + const saved = saveMachineMutate.mock.calls[0][0]; + expect(saved.name).toBe("Renamed node"); + expect(saved.mode).toBe("local"); + }); + + it("cancel closes the SheetForm on mobile", async () => { + machines = [localMachine()]; + render(); + + const detailEdit = screen + .getAllByRole("button", { name: "Edit" }) + .find((button) => button.textContent === "Edit") as HTMLButtonElement; + await userEvent.click(detailEdit); + + expect(screen.getByRole("dialog")).toBeInTheDocument(); + + await userEvent.click(screen.getByRole("button", { name: "Cancel" })); + // The sheet is now closed — the dialog role should no longer be present. + // (The page content itself is still rendered; only the sheet unmounts.) + expect(screen.queryByText("Edit machine")).not.toBeInTheDocument(); + }); +});