SheetForm dirty-state confirm + wire isDirty into all form consumers (R4.5)
SheetForm gains an isDirty prop. When true, any close attempt (Cancel button, header X, Radix overlay click, Escape) opens a 'Discard changes?' ConfirmDialog instead of discarding unsaved edits. Radix dismiss callbacks (onEscapeKeyDown, onPointerDownOutside) are intercepted when dirty so the guard applies uniformly. All four form consumers now compute and pass isDirty: - ServicePage: name/enabled/config differ from the persisted instance. - Settings machine editor: field-by-field draft vs editingMachine (create mode is always dirty; secret write-only fields excluded). - Message compose: subject non-empty, body differs from default, or attachments present. - WidgetConfigDialog: draft !== null (only draft mode is guarded; list mode has nothing to discard). Tests: 3 new SheetForm dirty-guard cases (prompt on cancel, abort discard, clean close when not dirty) + one focused dirty-guard test per consumer. 122 tests pass; lint/build green. Refs openspec/changes/mobile-responsive-parity/verify-report.md residual risk #1.
This commit is contained in:
@@ -93,4 +93,76 @@ describe("SheetForm", () => {
|
||||
await userEvent.click(screen.getByRole("button", { name: "Close" }));
|
||||
expect(onCancel).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
describe("dirty-state confirm (R4.5)", () => {
|
||||
it("prompts before discarding via Cancel when isDirty", async () => {
|
||||
const onCancel = vi.fn();
|
||||
render(
|
||||
<SheetForm
|
||||
open
|
||||
onOpenChange={() => {}}
|
||||
title="Edit"
|
||||
onSave={() => {}}
|
||||
onCancel={onCancel}
|
||||
isDirty
|
||||
>
|
||||
<div />
|
||||
</SheetForm>,
|
||||
);
|
||||
|
||||
// Cancel does not immediately close; a confirm opens.
|
||||
await userEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
expect(onCancel).not.toHaveBeenCalled();
|
||||
expect(
|
||||
screen.getByRole("heading", { name: "Discard changes?" }),
|
||||
).toBeInTheDocument();
|
||||
|
||||
// Confirm discard -> actually closes.
|
||||
await userEvent.click(screen.getByRole("button", { name: "Discard" }));
|
||||
expect(onCancel).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("closing the confirm without discarding keeps the form open", async () => {
|
||||
const onCancel = vi.fn();
|
||||
render(
|
||||
<SheetForm
|
||||
open
|
||||
onOpenChange={() => {}}
|
||||
title="Edit"
|
||||
onSave={() => {}}
|
||||
onCancel={onCancel}
|
||||
isDirty
|
||||
>
|
||||
<div />
|
||||
</SheetForm>,
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
// Two Cancel buttons now exist: the SheetForm footer and the confirm dialog.
|
||||
const cancelButtons = screen.getAllByRole("button", { name: "Cancel" });
|
||||
await userEvent.click(cancelButtons[cancelButtons.length - 1]);
|
||||
expect(onCancel).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("closes immediately when not dirty", async () => {
|
||||
const onCancel = vi.fn();
|
||||
render(
|
||||
<SheetForm
|
||||
open
|
||||
onOpenChange={() => {}}
|
||||
title="Edit"
|
||||
onSave={() => {}}
|
||||
onCancel={onCancel}
|
||||
>
|
||||
<div />
|
||||
</SheetForm>,
|
||||
);
|
||||
|
||||
await userEvent.click(screen.getByRole("button", { name: "Cancel" }));
|
||||
expect(onCancel).toHaveBeenCalledTimes(1);
|
||||
expect(
|
||||
screen.queryByRole("heading", { name: "Discard changes?" }),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -4,6 +4,7 @@ import { Loader2, XIcon } from "lucide-react";
|
||||
import { cn } from "@/lib/utils";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Sheet, SheetContent, SheetTitle } from "@/components/ui/sheet";
|
||||
import { ConfirmDialog } from "@/components/ConfirmDialog";
|
||||
|
||||
export interface SheetFormProps {
|
||||
open: boolean;
|
||||
@@ -17,6 +18,12 @@ export interface SheetFormProps {
|
||||
saveLabel?: string;
|
||||
/** Disable the Save button (e.g. when required fields are empty). */
|
||||
saveDisabled?: boolean;
|
||||
/**
|
||||
* When true, any close attempt (Cancel button, header X, overlay click,
|
||||
* Escape) prompts a discard-confirmation instead of immediately closing.
|
||||
* Spec R4.5.
|
||||
*/
|
||||
isDirty?: boolean;
|
||||
children: React.ReactNode;
|
||||
/** Optional className applied to the scrolling body. */
|
||||
bodyClassName?: string;
|
||||
@@ -43,15 +50,53 @@ export function SheetForm({
|
||||
isPending = false,
|
||||
saveDisabled = false,
|
||||
saveLabel = "Save",
|
||||
isDirty = false,
|
||||
children,
|
||||
bodyClassName,
|
||||
}: SheetFormProps) {
|
||||
const [confirmDiscardOpen, setConfirmDiscardOpen] = React.useState(false);
|
||||
|
||||
// Route every close path (Cancel, header X, Radix overlay/Escape) through one
|
||||
// guard so the dirty-confirm is applied uniformly (spec R4.5).
|
||||
const attemptClose = React.useCallback(() => {
|
||||
if (isDirty) {
|
||||
setConfirmDiscardOpen(true);
|
||||
} else {
|
||||
onCancel();
|
||||
}
|
||||
}, [isDirty, onCancel]);
|
||||
|
||||
const handleOpenChange = React.useCallback(
|
||||
(next: boolean) => {
|
||||
if (!next) {
|
||||
attemptClose();
|
||||
} else {
|
||||
onOpenChange(next);
|
||||
}
|
||||
},
|
||||
[attemptClose, onOpenChange],
|
||||
);
|
||||
|
||||
return (
|
||||
<Sheet open={open} onOpenChange={onOpenChange}>
|
||||
<Sheet open={open} onOpenChange={handleOpenChange}>
|
||||
<SheetContent
|
||||
side="bottom"
|
||||
showCloseButton={false}
|
||||
className="flex h-[100dvh] w-full flex-col gap-0 p-0 sm:max-w-full"
|
||||
onEscapeKeyDown={(e) => {
|
||||
// Prevent Radix's default Escape close so our guard runs instead.
|
||||
if (isDirty) {
|
||||
e.preventDefault();
|
||||
attemptClose();
|
||||
}
|
||||
}}
|
||||
onPointerDownOutside={(e) => {
|
||||
// Prevent overlay-click close so our guard runs instead.
|
||||
if (isDirty) {
|
||||
e.preventDefault();
|
||||
attemptClose();
|
||||
}
|
||||
}}
|
||||
>
|
||||
{/* Header — fixed at top */}
|
||||
<div className="flex h-14 shrink-0 items-center justify-between border-b border-border px-4">
|
||||
@@ -62,7 +107,7 @@ export function SheetForm({
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
aria-label="Close"
|
||||
onClick={onCancel}
|
||||
onClick={attemptClose}
|
||||
>
|
||||
<XIcon />
|
||||
</Button>
|
||||
@@ -75,7 +120,7 @@ export function SheetForm({
|
||||
|
||||
{/* Footer — fixed at bottom */}
|
||||
<div className="flex shrink-0 items-center justify-end gap-2 border-t border-border bg-muted/50 p-4">
|
||||
<Button variant="outline" onClick={onCancel} disabled={isPending}>
|
||||
<Button variant="outline" onClick={attemptClose} disabled={isPending}>
|
||||
Cancel
|
||||
</Button>
|
||||
<Button onClick={onSave} disabled={isPending || saveDisabled}>
|
||||
@@ -90,6 +135,18 @@ export function SheetForm({
|
||||
</Button>
|
||||
</div>
|
||||
</SheetContent>
|
||||
|
||||
<ConfirmDialog
|
||||
open={confirmDiscardOpen}
|
||||
title="Discard changes?"
|
||||
message="You have unsaved changes. Discard them and close?"
|
||||
confirmLabel="Discard"
|
||||
onCancel={() => setConfirmDiscardOpen(false)}
|
||||
onConfirm={() => {
|
||||
setConfirmDiscardOpen(false);
|
||||
onCancel();
|
||||
}}
|
||||
/>
|
||||
</Sheet>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user