Skip to content

Commit 6cbbac0

Browse files
committed
fix(studio): stop a paused save queue reporting a position edit as saved
Two more commits that could not tell a caller they had failed. `useDomEditPositionPatchCommit` swallowed `DomEditSaveQueueOpenError` and resolved. The intent was right, a paused save queue already puts a banner on screen and one toast per blocked edit is noise, but swallowing it also skipped the caller's revert: `useDomGeometryCommits` only restores the optimistic offset, size or rotation from its `.catch`. So once the breaker opened, a drag left the element where the user dropped it while nothing reached the file, and the next reload snapped it back. It now rejects without toasting. The banner still does the telling; the caller gets to revert. `handleDomEditElementsDelete` caught everything and only toasted, so an unpatchable target and a completed delete were indistinguishable to a caller. It now returns an outcome, with `no-project` and `no-selection` separated from a failed write rather than all three sharing an early `return`. Adds the first test for `useDomEditPositionPatchCommit`, covering the paused queue, an ordinary failure, and success.
1 parent a6bb75d commit 6cbbac0

4 files changed

Lines changed: 130 additions & 3 deletions

File tree

packages/studio/src/hooks/domEditCommitRunner.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promi
6666
* programmatic caller has no screen, so it has to be told.
6767
*/
6868
export type DomEditCommitDeclineReason =
69+
| "no-project"
6970
| "no-selection"
7071
| "geometry-property"
7172
| "styles-not-editable"
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// @vitest-environment jsdom
2+
import { act } from "react";
3+
import { afterEach, describe, expect, it, vi } from "vitest";
4+
import type { DomEditSelection } from "../components/editor/domEditing";
5+
import { DomEditSaveQueueOpenError } from "../utils/domEditSaveQueue";
6+
import { mountReactHarness } from "./domSelectionTestHarness";
7+
import { useDomEditPositionPatchCommit } from "./useDomEditPositionPatchCommit";
8+
9+
Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true);
10+
11+
let cleanup: (() => void) | null = null;
12+
13+
function selectionStub(): DomEditSelection {
14+
const element = document.createElement("div");
15+
element.id = "card";
16+
return {
17+
id: "card",
18+
element,
19+
label: "Card",
20+
tagName: "div",
21+
sourceFile: "index.html",
22+
compositionPath: "index.html",
23+
isCompositionHost: false,
24+
isInsideLockedComposition: false,
25+
boundingBox: { x: 0, y: 0, width: 100, height: 100 },
26+
textContent: null,
27+
dataAttributes: {},
28+
inlineStyles: {},
29+
computedStyles: {},
30+
textFields: [],
31+
capabilities: {
32+
canSelect: true,
33+
canEditStyles: true,
34+
canCrop: true,
35+
canMove: true,
36+
canResize: true,
37+
canApplyManualOffset: true,
38+
canApplyManualSize: true,
39+
canApplyManualRotation: true,
40+
},
41+
};
42+
}
43+
44+
function renderCommit(params: Parameters<typeof useDomEditPositionPatchCommit>[0]) {
45+
const captured: { commit: ReturnType<typeof useDomEditPositionPatchCommit> | null } = {
46+
commit: null,
47+
};
48+
function Probe() {
49+
captured.commit = useDomEditPositionPatchCommit(params);
50+
return null;
51+
}
52+
const root = mountReactHarness(<Probe />);
53+
cleanup = () => act(() => root.unmount());
54+
if (!captured.commit) throw new Error("hook did not initialize");
55+
return captured.commit;
56+
}
57+
58+
function paramsWith(queueDomEditSave: (save: () => Promise<void>) => Promise<void>) {
59+
const showToast = vi.fn();
60+
return {
61+
showToast,
62+
params: {
63+
activeCompPath: "index.html",
64+
persistDomEditOperations: vi.fn().mockResolvedValue(undefined),
65+
queueDomEditSave,
66+
showToast,
67+
},
68+
};
69+
}
70+
71+
const options = { label: "Move layer", coalesceKey: "path-offset:card" };
72+
73+
afterEach(() => {
74+
cleanup?.();
75+
cleanup = null;
76+
vi.restoreAllMocks();
77+
});
78+
79+
describe("useDomEditPositionPatchCommit", () => {
80+
it("rejects when the save queue is paused, so the caller can revert its optimistic change", async () => {
81+
const { showToast, params } = paramsWith(() => Promise.reject(new DomEditSaveQueueOpenError()));
82+
const commit = renderCommit(params);
83+
84+
await act(async () => {
85+
await expect(commit(selectionStub(), [], options)).rejects.toBeInstanceOf(
86+
DomEditSaveQueueOpenError,
87+
);
88+
});
89+
90+
// No toast: the paused-save banner already tells the human, and one toast per
91+
// blocked edit is what the original swallow existed to prevent.
92+
expect(showToast).not.toHaveBeenCalled();
93+
});
94+
95+
it("toasts and rejects on an ordinary save failure", async () => {
96+
const { showToast, params } = paramsWith(() => Promise.reject(new Error("server said no")));
97+
const commit = renderCommit(params);
98+
99+
await act(async () => {
100+
await expect(commit(selectionStub(), [], options)).rejects.toThrow("server said no");
101+
});
102+
103+
expect(showToast).toHaveBeenCalledWith("server said no");
104+
});
105+
106+
it("resolves when the write lands", async () => {
107+
const { showToast, params } = paramsWith((save) => save());
108+
const commit = renderCommit(params);
109+
110+
await act(async () => {
111+
await expect(commit(selectionStub(), [], options)).resolves.toBeUndefined();
112+
});
113+
114+
expect(showToast).not.toHaveBeenCalled();
115+
});
116+
});

packages/studio/src/hooks/useDomEditPositionPatchCommit.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,12 @@ export function useDomEditPositionPatchCommit({
3535
skipRefresh: options.skipRefresh ?? true,
3636
});
3737
}).catch((error) => {
38-
if (error instanceof DomEditSaveQueueOpenError) return;
38+
// A paused save queue is not worth a toast: the paused-save banner is
39+
// already on screen, and one toast per blocked edit is what this branch
40+
// exists to prevent. It still has to REJECT, though. Swallowing it
41+
// resolved the commit, which skipped the caller's revert, so the element
42+
// stayed where the drag put it while nothing reached the file.
43+
if (error instanceof DomEditSaveQueueOpenError) throw error;
3944
showToast(error instanceof Error ? error.message : "Failed to save position");
4045
trackStudioSaveFailure({
4146
source: "dom_edit",

packages/studio/src/hooks/useElementLifecycleOps.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
type LayerRevealCommitOwnership,
2121
} from "../components/editor/useLayerRevealOverride";
2222
import type { CommitDomEditPatchBatches, DomEditPatchBatch } from "./domEditCommitTypes";
23+
import { domEditCommitDeclined } from "./domEditCommitRunner";
2324
import { cutoverCommittedOrThrow, type CutoverResult } from "../utils/sdkCutover";
2425
import { studioWriteHeaders } from "../utils/studioFileVersion";
2526

@@ -89,9 +90,9 @@ export function useElementLifecycleOps({
8990
// fallow-ignore-next-line complexity
9091
async (selections: DomEditSelection[]) => {
9192
const pid = projectIdRef.current;
92-
if (!pid) return;
93+
if (!pid) return domEditCommitDeclined("no-project");
9394
const [selection] = selections;
94-
if (!selection) return;
95+
if (!selection) return domEditCommitDeclined("no-selection");
9596
const label =
9697
selections.length === 1
9798
? selection.label || selection.id || selection.selector || selection.tagName
@@ -208,9 +209,13 @@ export function useElementLifecycleOps({
208209
`Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`,
209210
"info",
210211
);
212+
return { ok: true } as const;
211213
} catch (error) {
212214
const message = error instanceof Error ? error.message : "Failed to delete element";
213215
showToast(message);
216+
// The toast is what tells the human. The returned outcome is what tells
217+
// a caller that has no screen to read.
218+
return domEditCommitDeclined("persist-failed");
214219
}
215220
},
216221
[

0 commit comments

Comments
 (0)