From a6bb75dc1a17fb678098d0426b0540adb18d2721 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 26 Aug 2026 18:15:23 -0400 Subject: [PATCH 1/5] fix(studio): let a failed text or style commit report itself `runDomEditCommit` catches a persist failure, reverts, fires `onError` and then resolves. That contract is deliberate and its docstring says so: the human path learns the write failed from the toast `onError` puts on screen, so a rejection would be redundant. It also means a caller awaiting `handleDomTextCommit` or `handleDomStyleCommit` cannot tell a landed write from a reverted one, because both resolve with `undefined`. The runner already offers `onSettled` as the way out. Text and style were the two commits that never got it wired. Add `runReportedDomEditCommit`, which owns `onSettled` (forwarding to a caller-supplied one rather than dropping it) and returns whether the write landed. Both handlers now return a tagged outcome, so the three preconditions that previously returned early and silently are each distinguishable: no selection, a manual-geometry property the style path refuses, and a selection that cannot edit styles. Same for text: no selection versus not text-editable. Human-facing behaviour is unchanged and the tests assert that: the toast still fires and the optimistic DOM change is still reverted. The callback props that carry these handlers ignore the result, so their declared type widens from `Promise` to `Promise`. That type is hand-copied in fourteen places; consolidating it is worth its own change. `useDomEditTextCommits.ts` is now 593 lines against the 600-line cap. The next change to it needs a split. --- .../editor/DomEditCropHandles.test.tsx | 2 +- .../components/editor/DomEditCropHandles.tsx | 2 +- .../src/components/editor/DomEditOverlay.tsx | 2 +- .../editor/DomEditSelectionChrome.tsx | 2 +- .../editor/propertyPanelCommitField.tsx | 2 +- .../editor/propertyPanelFlatLayoutSection.tsx | 6 +- .../editor/propertyPanelFlatMaskInsetRows.tsx | 2 +- .../editor/propertyPanelFlatMediaSection.tsx | 2 +- .../editor/propertyPanelFlatPrimitives.tsx | 2 +- .../editor/propertyPanelFlatStyleSections.tsx | 16 +- .../editor/propertyPanelMediaSection.tsx | 2 +- .../editor/propertyPanelPrimitives.tsx | 2 +- .../editor/propertyPanelStyleSections.tsx | 2 +- .../components/editor/propertyPanelTypes.ts | 2 +- .../editor/useInspectorGestureTransaction.ts | 6 +- .../studio/src/hooks/domEditCommitRunner.ts | 44 ++++ .../src/hooks/useDomEditTextCommits.test.tsx | 196 ++++++++++++++++-- .../studio/src/hooks/useDomEditTextCommits.ts | 21 +- 18 files changed, 259 insertions(+), 54 deletions(-) diff --git a/packages/studio/src/components/editor/DomEditCropHandles.test.tsx b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx index 1ea0381352..a75fc39493 100644 --- a/packages/studio/src/components/editor/DomEditCropHandles.test.tsx +++ b/packages/studio/src/components/editor/DomEditCropHandles.test.tsx @@ -35,7 +35,7 @@ function makeEl(id: string, clip: string): HTMLElement { function render( el: HTMLElement, - onStyleCommit: (property: string, value: string) => Promise | void = () => undefined, + onStyleCommit: (property: string, value: string) => Promise | void = () => undefined, ): { root: Root; rerender: (next: HTMLElement) => void } { const host = document.createElement("div"); document.body.append(host); diff --git a/packages/studio/src/components/editor/DomEditCropHandles.tsx b/packages/studio/src/components/editor/DomEditCropHandles.tsx index 5dc5050c30..62ab287a70 100644 --- a/packages/studio/src/components/editor/DomEditCropHandles.tsx +++ b/packages/studio/src/components/editor/DomEditCropHandles.tsx @@ -28,7 +28,7 @@ interface CropGestureState { interface DomEditCropHandlesProps { selection: DomEditSelection; overlayRect: OverlayRect; - onStyleCommit?: (property: string, value: string) => Promise | void; + onStyleCommit?: (property: string, value: string) => Promise | void; } // Hit-strip size (px) for an edge crop handle: THICKNESS extends outward from diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 11cd1f0d4c..3e2bfa6010 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -83,7 +83,7 @@ interface DomEditOverlayProps { restore?: () => void, ) => Promise | void; onRotationCommit: (selection: DomEditSelection, next: { angle: number }) => Promise | void; - onStyleCommit?: (property: string, value: string) => Promise | void; + onStyleCommit?: (property: string, value: string) => Promise | void; gridVisible?: boolean; gridSpacing?: number; recordingState?: GestureRecordingState; diff --git a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx index 8046e8608f..720352e7ef 100644 --- a/packages/studio/src/components/editor/DomEditSelectionChrome.tsx +++ b/packages/studio/src/components/editor/DomEditSelectionChrome.tsx @@ -123,7 +123,7 @@ interface DomEditSelectionChromeProps { groupSelectionCount: number; blockedMoveRef: RefObject; gestures: GestureHandlers; - onStyleCommit?: (property: string, value: string) => Promise | void; + onStyleCommit?: (property: string, value: string) => Promise | void; onBoxMouseDown: (e: React.MouseEvent) => void; onBoxClick: (event: React.MouseEvent) => void; /** The canvas' text-editing session: what opens one, and whether one is open. */ diff --git a/packages/studio/src/components/editor/propertyPanelCommitField.tsx b/packages/studio/src/components/editor/propertyPanelCommitField.tsx index 015d25e15a..d50511ff87 100644 --- a/packages/studio/src/components/editor/propertyPanelCommitField.tsx +++ b/packages/studio/src/components/editor/propertyPanelCommitField.tsx @@ -21,7 +21,7 @@ export function CommitField({ liveCommit?: boolean; align?: "left" | "right"; onPreview?: (nextValue: string) => void; - onCommit: (nextValue: string) => void | Promise; + onCommit: (nextValue: string) => void | Promise; }) { const [draft, setDraft] = useState(value); const valueRef = useRef(value); diff --git a/packages/studio/src/components/editor/propertyPanelFlatLayoutSection.tsx b/packages/studio/src/components/editor/propertyPanelFlatLayoutSection.tsx index e59cde3351..e4f7160059 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatLayoutSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatLayoutSection.tsx @@ -181,7 +181,7 @@ export function LayoutZIndexRow({ onSetStyle, }: { styles: Record; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const zIndex = String(parseInt(styles["z-index"] || "auto", 10) || 0); return ( @@ -200,7 +200,7 @@ export function LayoutFlexBlock({ disabled, }: { styles: Record; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; disabled: boolean; }) { const isFlex = styles.display === "flex" || styles.display === "inline-flex"; @@ -337,7 +337,7 @@ interface FlatLayoutSectionProps > { element: DomEditSelection; styles: Record; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; disabled: boolean; } diff --git a/packages/studio/src/components/editor/propertyPanelFlatMaskInsetRows.tsx b/packages/studio/src/components/editor/propertyPanelFlatMaskInsetRows.tsx index 1dfb77cb71..c3ba415f69 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatMaskInsetRows.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatMaskInsetRows.tsx @@ -27,7 +27,7 @@ export function FlatMaskInsetRows({ clipPathValue: string; radiusValue: number; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const clipPathPreset = inferClipPathPreset(clipPathValue); const parsedClipInsets = parseInsetClipPathSides(clipPathValue); diff --git a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx index 6ba4dd095a..98650a31b9 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatMediaSection.tsx @@ -38,7 +38,7 @@ export function FlatMediaSection({ projectDir: string | null; element: DomEditSelection; styles: Record; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onSetAttribute: (attr: string, value: string) => void | Promise; onSetHtmlAttribute: (attr: string, value: string | null) => void | Promise; /** A volume lane in the timeline drives the level; the slider cannot. */ diff --git a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx index ce99e13f8e..b101a70337 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatPrimitives.tsx @@ -35,7 +35,7 @@ export function FlatRow({ /** Renders a trailing 10px caret-down, for select-backed rows. */ dropdown?: boolean; onPreview?: (nextValue: string) => void; - onCommit: (nextValue: string) => void | Promise; + onCommit: (nextValue: string) => void | Promise; onReset?: () => void; }) { const track = useTrackDesignInput(); diff --git a/packages/studio/src/components/editor/propertyPanelFlatStyleSections.tsx b/packages/studio/src/components/editor/propertyPanelFlatStyleSections.tsx index 362f938d52..ff207878a7 100644 --- a/packages/studio/src/components/editor/propertyPanelFlatStyleSections.tsx +++ b/packages/studio/src/components/editor/propertyPanelFlatStyleSections.tsx @@ -50,7 +50,7 @@ function FlatFillFields({ element: DomEditSelection; styles: Record; assets: string[]; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onPreviewStyle?: (prop: string, value: string) => void; onImportAssets?: (files: FileList) => Promise; }) { @@ -156,7 +156,7 @@ function FlatStrokeRow({ }: { styles: Record; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const borderWidthValue = parsePxMetricValue(styles["border-width"] ?? "") ?? @@ -236,7 +236,7 @@ function FlatRadiusRow({ styles: Record; gsapBorderRadius?: { tl: number; tr: number; br: number; bl: number } | null; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const radiusValue = parseNumericValue(styles["border-radius"]) ?? 0; const radiusTL = @@ -286,7 +286,7 @@ function FlatShadowBlendRows({ }: { styles: Record; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const boxShadowPreset = inferBoxShadowPreset(styles["box-shadow"]); const blendValue = styles["mix-blend-mode"] || "normal"; @@ -332,7 +332,7 @@ function FlatBlurSliders({ }: { styles: Record; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const filterBlurValue = getCssFilterFunctionPx(styles.filter, "blur"); const backdropBlurValue = getCssFilterFunctionPx(styles["backdrop-filter"], "blur"); @@ -378,7 +378,7 @@ function FlatOverflowMaskRows({ }: { styles: Record; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const radiusValue = parseNumericValue(styles["border-radius"]) ?? 0; const clipPathValue = styles["clip-path"] || "none"; @@ -432,7 +432,7 @@ function FlatOpacitySlider({ }: { styles: Record; disabled: boolean; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; }) { const opacityValue = Math.round((parseNumericValue(styles.opacity) ?? 1) * 100); @@ -464,7 +464,7 @@ export function FlatStyleSection({ element: DomEditSelection; styles: Record; assets: string[]; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onPreviewStyle?: (prop: string, value: string) => void; onImportAssets?: (files: FileList) => Promise; gsapBorderRadius?: { tl: number; tr: number; br: number; bl: number } | null; diff --git a/packages/studio/src/components/editor/propertyPanelMediaSection.tsx b/packages/studio/src/components/editor/propertyPanelMediaSection.tsx index c1599e3fba..be2606da75 100644 --- a/packages/studio/src/components/editor/propertyPanelMediaSection.tsx +++ b/packages/studio/src/components/editor/propertyPanelMediaSection.tsx @@ -35,7 +35,7 @@ export function MediaSection({ projectDir: string | null; element: DomEditSelection; styles: Record; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onSetAttribute: (attr: string, value: string) => void | Promise; onSetHtmlAttribute: (attr: string, value: string | null) => void | Promise; onRemoveBackground?: ( diff --git a/packages/studio/src/components/editor/propertyPanelPrimitives.tsx b/packages/studio/src/components/editor/propertyPanelPrimitives.tsx index 62d5b0984a..66d53066ec 100644 --- a/packages/studio/src/components/editor/propertyPanelPrimitives.tsx +++ b/packages/studio/src/components/editor/propertyPanelPrimitives.tsx @@ -29,7 +29,7 @@ export function MetricField({ scrub?: boolean; suffix?: string; tooltip?: string; - onCommit: (nextValue: string) => void | Promise; + onCommit: (nextValue: string) => void | Promise; }) { const track = useTrackDesignInput(); const scrubRef = useRef<{ startX: number; startValue: number; pointerId: number } | null>(null); diff --git a/packages/studio/src/components/editor/propertyPanelStyleSections.tsx b/packages/studio/src/components/editor/propertyPanelStyleSections.tsx index 1ced7c3664..1d73c7be9a 100644 --- a/packages/studio/src/components/editor/propertyPanelStyleSections.tsx +++ b/packages/studio/src/components/editor/propertyPanelStyleSections.tsx @@ -53,7 +53,7 @@ export function StyleSections({ element: DomEditSelection; styles: Record; assets: string[]; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onImportAssets?: (files: FileList) => Promise; gsapBorderRadius?: { tl: number; tr: number; br: number; bl: number } | null; // When true, the Flex `Section` is suppressed. The flat inspector renders diff --git a/packages/studio/src/components/editor/propertyPanelTypes.ts b/packages/studio/src/components/editor/propertyPanelTypes.ts index 0e40abd4e9..ac93927ccc 100644 --- a/packages/studio/src/components/editor/propertyPanelTypes.ts +++ b/packages/studio/src/components/editor/propertyPanelTypes.ts @@ -44,7 +44,7 @@ export interface PropertyPanelProps { copiedAgentPrompt: boolean; onClearSelection: () => void; onUngroup?: () => void; - onSetStyle: (prop: string, value: string) => void | Promise; + onSetStyle: (prop: string, value: string) => void | Promise; onPreviewStyle?: (prop: string, value: string) => void; onSetAttribute: (attr: string, value: string) => void | Promise; /** Commits several data-* attributes on the SAME element in ONE atomic diff --git a/packages/studio/src/components/editor/useInspectorGestureTransaction.ts b/packages/studio/src/components/editor/useInspectorGestureTransaction.ts index 51bc8ce6fd..93c2b6af2f 100644 --- a/packages/studio/src/components/editor/useInspectorGestureTransaction.ts +++ b/packages/studio/src/components/editor/useInspectorGestureTransaction.ts @@ -1,6 +1,6 @@ import { useCallback, useEffect, useRef, useState } from "react"; -function isPromiseCommit(result: void | Promise): result is Promise { +function isPromiseCommit(result: void | Promise): result is Promise { return Boolean(result && typeof result.then === "function"); } @@ -12,7 +12,7 @@ export function useInspectorGestureTransaction({ }: { sourceValue: T; onPreview: (value: T) => void; - onCommit: (value: T) => void | Promise; + onCommit: (value: T) => void | Promise; }) { const sourceRef = useRef(sourceValue); const activeRef = useRef<{ before: T; latest: T } | null>(null); @@ -122,7 +122,7 @@ export function useInspectorGestureDraft({ }: { sourceValue: T; onPreview: (value: T) => void; - onCommit: (value: T) => void | Promise; + onCommit: (value: T) => void | Promise; }) { const [draft, setDraft] = useState(sourceValue); const transaction = useInspectorGestureTransaction({ diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts index 516e6b4632..754037b8a1 100644 --- a/packages/studio/src/hooks/domEditCommitRunner.ts +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -55,3 +55,47 @@ export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promi if (!config.shouldResync()) return; await config.resync(); } + +/** + * Why a DOM edit commit did not change the file. + * + * `runDomEditCommit` resolves on persist failure by design (see its contract + * above), so a caller cannot learn whether the write landed by awaiting it — a + * failed commit and a successful one are indistinguishable. The human path does + * not need to ask, because `onError` already put a toast on screen. A + * programmatic caller has no screen, so it has to be told. + */ +export type DomEditCommitDeclineReason = + | "no-selection" + | "geometry-property" + | "styles-not-editable" + | "not-text-editable" + | "persist-failed"; + +export type DomEditCommitOutcome = { ok: true } | { ok: false; reason: DomEditCommitDeclineReason }; + +export function domEditCommitDeclined(reason: DomEditCommitDeclineReason): DomEditCommitOutcome { + return { ok: false, reason }; +} + +/** + * `runDomEditCommit`, reporting whether the write actually landed. + * + * Owns `onSettled` to do it, and forwards to a caller-supplied one rather than + * dropping it. `runDomEditCommit` calls `onSettled` exactly once on both the + * success and the failure path, so the flag is always set by the time it + * resolves. + */ +export async function runReportedDomEditCommit( + config: DomEditCommitRunnerConfig, +): Promise { + let landed = false; + await runDomEditCommit({ + ...config, + onSettled: (ok) => { + landed = ok; + config.onSettled?.(ok); + }, + }); + return landed ? { ok: true } : domEditCommitDeclined("persist-failed"); +} diff --git a/packages/studio/src/hooks/useDomEditTextCommits.test.tsx b/packages/studio/src/hooks/useDomEditTextCommits.test.tsx index 7632880dae..cc440823e7 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.test.tsx +++ b/packages/studio/src/hooks/useDomEditTextCommits.test.tsx @@ -66,6 +66,42 @@ function selectionFor(element: HTMLElement): DomEditSelection { }; } +/** A preview element inside a real iframe, which is where Studio's chrome expects to find it. */ +function previewElement( + html: string, + id: string, +): { iframe: HTMLIFrameElement; element: HTMLElement } { + const iframe = document.createElement("iframe"); + document.body.append(iframe); + const doc = iframe.contentDocument; + if (!doc) throw new Error("expected iframe document"); + doc.body.innerHTML = html; + const element = doc.getElementById(id); + const HTMLElementCtor = doc.defaultView?.HTMLElement; + if (!HTMLElementCtor || !(element instanceof HTMLElementCtor)) { + throw new Error("expected preview element"); + } + return { iframe, element }; +} + +/** Hook params with nothing selected and a writer that succeeds; override what the test is about. */ +function commitParams( + overrides: Partial = {}, +): UseDomEditTextCommitsParams { + return { + activeCompPath: "index.html", + previewIframeRef: { current: null }, + showToast: vi.fn(), + domEditSelection: null, + applyDomSelection: vi.fn(), + refreshDomEditSelectionFromPreview: vi.fn(), + buildDomSelectionFromTarget: vi.fn(async () => null), + persistDomEditOperations: vi.fn().mockResolvedValue(undefined), + resolveImportedFontAsset: () => null, + ...overrides, + }; +} + let cleanup: (() => void) | null = null; function renderTextCommitHook(params: UseDomEditTextCommitsParams) { @@ -89,16 +125,7 @@ afterEach(() => { describe("useDomEditTextCommits", () => { it("does not let a stale failed fields commit revert newer text", async () => { - const iframe = document.createElement("iframe"); - document.body.append(iframe); - const doc = iframe.contentDocument; - if (!doc) throw new Error("expected iframe document"); - doc.body.innerHTML = '
Original
'; - const element = doc.getElementById("card"); - const HTMLElementCtor = doc.defaultView?.HTMLElement; - if (!HTMLElementCtor || !(element instanceof HTMLElementCtor)) { - throw new Error("expected preview element"); - } + const { iframe, element } = previewElement("
Original
", "card"); vi.spyOn(console, "warn").mockImplementation(() => {}); const selection = selectionFor(element); const stalePersist = createDeferred(); @@ -106,17 +133,13 @@ describe("useDomEditTextCommits", () => { .fn() .mockImplementationOnce(() => stalePersist.promise) .mockResolvedValueOnce(undefined); - const hook = renderTextCommitHook({ - activeCompPath: "index.html", - previewIframeRef: { current: iframe }, - showToast: vi.fn(), - domEditSelection: selection, - applyDomSelection: vi.fn(), - refreshDomEditSelectionFromPreview: vi.fn(), - buildDomSelectionFromTarget: vi.fn(async () => null), - persistDomEditOperations, - resolveImportedFontAsset: () => null, - }); + const hook = renderTextCommitHook( + commitParams({ + previewIframeRef: { current: iframe }, + domEditSelection: selection, + persistDomEditOperations, + }), + ); let staleCommit: Promise | undefined; act(() => { @@ -132,4 +155,135 @@ describe("useDomEditTextCommits", () => { expect(element.innerHTML).toBe("Newest"); }); + + it("reports persist failure from a style commit instead of resolving silently", async () => { + const { iframe, element } = previewElement("
Original
", "card"); + const selection = selectionFor(element); + const showToast = vi.fn(); + const hook = renderTextCommitHook( + commitParams({ + previewIframeRef: { current: iframe }, + showToast, + domEditSelection: selection, + persistDomEditOperations: vi.fn().mockRejectedValue(new Error("server said no")), + }), + ); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomStyleCommit("color", "red"); + }); + + expect(outcome).toEqual({ ok: false, reason: "persist-failed" }); + // The human-facing behaviour must be unchanged: still toasts, still reverts. + expect(showToast).toHaveBeenCalled(); + expect(element.style.getPropertyValue("color")).toBe(""); + }); + + it("reports a successful style commit", async () => { + const { iframe, element } = previewElement("
Original
", "card"); + const selection = selectionFor(element); + const hook = renderTextCommitHook( + commitParams({ previewIframeRef: { current: iframe }, domEditSelection: selection }), + ); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomStyleCommit("color", "red"); + }); + + expect(outcome).toEqual({ ok: true }); + }); + + it("declines a style commit with no selection, without reaching the writer", async () => { + const persistDomEditOperations = vi.fn().mockResolvedValue(undefined); + const hook = renderTextCommitHook( + commitParams({ + domEditSelection: null, + persistDomEditOperations, + }), + ); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomStyleCommit("color", "red"); + }); + + expect(outcome).toEqual({ ok: false, reason: "no-selection" }); + expect(persistDomEditOperations).not.toHaveBeenCalled(); + }); + + it("declines a style commit for a manual-geometry property", async () => { + const persistDomEditOperations = vi.fn().mockResolvedValue(undefined); + const { element } = previewElement("
Original
", "card"); + const hook = renderTextCommitHook( + commitParams({ + domEditSelection: selectionFor(element), + persistDomEditOperations, + }), + ); + + let outcome: unknown; + await act(async () => { + // `left` is a manual-geometry property the style path deliberately refuses. + outcome = await hook.handleDomStyleCommit("left", "10px"); + }); + + expect(outcome).toEqual({ ok: false, reason: "geometry-property" }); + expect(persistDomEditOperations).not.toHaveBeenCalled(); + }); + + it("declines a style commit when the selection cannot edit styles", async () => { + const persistDomEditOperations = vi.fn().mockResolvedValue(undefined); + const { element } = previewElement("
Original
", "card"); + const locked = selectionFor(element); + locked.capabilities = { ...locked.capabilities, canEditStyles: false }; + const hook = renderTextCommitHook( + commitParams({ + domEditSelection: locked, + persistDomEditOperations, + }), + ); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomStyleCommit("color", "red"); + }); + + expect(outcome).toEqual({ ok: false, reason: "styles-not-editable" }); + expect(persistDomEditOperations).not.toHaveBeenCalled(); + }); + + it("reports persist failure from a text commit instead of resolving silently", async () => { + const { iframe, element } = previewElement("
Original
", "card"); + const selection = selectionFor(element); + const hook = renderTextCommitHook( + commitParams({ + previewIframeRef: { current: iframe }, + domEditSelection: selection, + persistDomEditOperations: vi.fn().mockRejectedValue(new Error("server said no")), + }), + ); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomTextCommit("Updated"); + }); + + expect(outcome).toEqual({ ok: false, reason: "persist-failed" }); + expect(element.innerHTML).toBe("Original"); + }); + + it("reports a text commit declined for an unselected target", async () => { + const persistDomEditOperations = vi.fn().mockResolvedValue(undefined); + const hook = renderTextCommitHook(commitParams({ persistDomEditOperations })); + + let outcome: unknown; + await act(async () => { + outcome = await hook.handleDomTextCommit("Updated"); + }); + + expect(outcome).toEqual({ ok: false, reason: "no-selection" }); + expect(persistDomEditOperations).not.toHaveBeenCalled(); + }); }); diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index 07f048274c..a4a9e03b1e 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -30,7 +30,9 @@ import { reportDomEditPersistFailure } from "./domEditPersistFailure"; import { bumpDomEditCommitMapVersion, bumpDomEditCommitVersion, + domEditCommitDeclined, runDomEditCommit, + runReportedDomEditCommit, } from "./domEditCommitRunner"; import { useDomEditAttributeCommits } from "./useDomEditAttributeCommits"; import type { InlineTextEditCommit } from "./useInlineTextEdit"; @@ -187,9 +189,12 @@ export function useDomEditTextCommits({ const handleDomStyleCommit = useCallback( async (property: string, value: string) => { - if (!domEditSelection) return; - if (isManualGeometryStyleProperty(property)) return; - if (!domEditSelection.capabilities.canEditStyles) return; + if (!domEditSelection) return domEditCommitDeclined("no-selection"); + if (isManualGeometryStyleProperty(property)) + return domEditCommitDeclined("geometry-property"); + if (!domEditSelection.capabilities.canEditStyles) { + return domEditCommitDeclined("styles-not-editable"); + } const styleCommitKey = `${getDomEditTargetKey(domEditSelection)}:${property}`; const isLatestStyleCommit = bumpDomEditCommitMapVersion( domStyleCommitVersionRef.current, @@ -210,7 +215,7 @@ export function useDomEditTextCommits({ // element in-browser immediately, so a reload would only cost a black blink. const skipRefresh = true; - await runDomEditCommit({ + return runReportedDomEditCommit({ capture: () => { if (!doc) return; const el = findElementForSelection(doc, domEditSelection, activeCompPath); @@ -268,8 +273,10 @@ export function useDomEditTextCommits({ const handleDomTextCommit = useCallback( async (value: string, fieldKey?: string) => { - if (!domEditSelection) return; - if (!isTextEditableSelection(domEditSelection)) return; + if (!domEditSelection) return domEditCommitDeclined("no-selection"); + if (!isTextEditableSelection(domEditSelection)) { + return domEditCommitDeclined("not-text-editable"); + } const isLatestTextCommit = bumpDomEditCommitVersion(domTextCommitVersionRef); const nextTextFields = buildNextDomTextFields(domEditSelection.textFields, value, fieldKey); const textCommit = planDomTextCommit(domEditSelection.textFields, nextTextFields, value); @@ -278,7 +285,7 @@ export function useDomEditTextCommits({ let editedElement: HTMLElement | null = null; let previousInnerHtml: string | null = null; - await runDomEditCommit({ + return runReportedDomEditCommit({ capture: () => { if (!doc) return; const el = findElementForSelection(doc, domEditSelection, activeCompPath); From 6cbbac02da0ac110ea240be13e171ff9d859fbf2 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 26 Aug 2026 18:26:24 -0400 Subject: [PATCH 2/5] 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. --- .../studio/src/hooks/domEditCommitRunner.ts | 1 + .../useDomEditPositionPatchCommit.test.tsx | 116 ++++++++++++++++++ .../hooks/useDomEditPositionPatchCommit.ts | 7 +- .../src/hooks/useElementLifecycleOps.ts | 9 +- 4 files changed, 130 insertions(+), 3 deletions(-) create mode 100644 packages/studio/src/hooks/useDomEditPositionPatchCommit.test.tsx diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts index 754037b8a1..c80cec249e 100644 --- a/packages/studio/src/hooks/domEditCommitRunner.ts +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -66,6 +66,7 @@ export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promi * programmatic caller has no screen, so it has to be told. */ export type DomEditCommitDeclineReason = + | "no-project" | "no-selection" | "geometry-property" | "styles-not-editable" diff --git a/packages/studio/src/hooks/useDomEditPositionPatchCommit.test.tsx b/packages/studio/src/hooks/useDomEditPositionPatchCommit.test.tsx new file mode 100644 index 0000000000..c9d2a3bdc2 --- /dev/null +++ b/packages/studio/src/hooks/useDomEditPositionPatchCommit.test.tsx @@ -0,0 +1,116 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { DomEditSelection } from "../components/editor/domEditing"; +import { DomEditSaveQueueOpenError } from "../utils/domEditSaveQueue"; +import { mountReactHarness } from "./domSelectionTestHarness"; +import { useDomEditPositionPatchCommit } from "./useDomEditPositionPatchCommit"; + +Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true); + +let cleanup: (() => void) | null = null; + +function selectionStub(): DomEditSelection { + const element = document.createElement("div"); + element.id = "card"; + return { + id: "card", + element, + label: "Card", + tagName: "div", + sourceFile: "index.html", + compositionPath: "index.html", + isCompositionHost: false, + isInsideLockedComposition: false, + boundingBox: { x: 0, y: 0, width: 100, height: 100 }, + textContent: null, + dataAttributes: {}, + inlineStyles: {}, + computedStyles: {}, + textFields: [], + capabilities: { + canSelect: true, + canEditStyles: true, + canCrop: true, + canMove: true, + canResize: true, + canApplyManualOffset: true, + canApplyManualSize: true, + canApplyManualRotation: true, + }, + }; +} + +function renderCommit(params: Parameters[0]) { + const captured: { commit: ReturnType | null } = { + commit: null, + }; + function Probe() { + captured.commit = useDomEditPositionPatchCommit(params); + return null; + } + const root = mountReactHarness(); + cleanup = () => act(() => root.unmount()); + if (!captured.commit) throw new Error("hook did not initialize"); + return captured.commit; +} + +function paramsWith(queueDomEditSave: (save: () => Promise) => Promise) { + const showToast = vi.fn(); + return { + showToast, + params: { + activeCompPath: "index.html", + persistDomEditOperations: vi.fn().mockResolvedValue(undefined), + queueDomEditSave, + showToast, + }, + }; +} + +const options = { label: "Move layer", coalesceKey: "path-offset:card" }; + +afterEach(() => { + cleanup?.(); + cleanup = null; + vi.restoreAllMocks(); +}); + +describe("useDomEditPositionPatchCommit", () => { + it("rejects when the save queue is paused, so the caller can revert its optimistic change", async () => { + const { showToast, params } = paramsWith(() => Promise.reject(new DomEditSaveQueueOpenError())); + const commit = renderCommit(params); + + await act(async () => { + await expect(commit(selectionStub(), [], options)).rejects.toBeInstanceOf( + DomEditSaveQueueOpenError, + ); + }); + + // No toast: the paused-save banner already tells the human, and one toast per + // blocked edit is what the original swallow existed to prevent. + expect(showToast).not.toHaveBeenCalled(); + }); + + it("toasts and rejects on an ordinary save failure", async () => { + const { showToast, params } = paramsWith(() => Promise.reject(new Error("server said no"))); + const commit = renderCommit(params); + + await act(async () => { + await expect(commit(selectionStub(), [], options)).rejects.toThrow("server said no"); + }); + + expect(showToast).toHaveBeenCalledWith("server said no"); + }); + + it("resolves when the write lands", async () => { + const { showToast, params } = paramsWith((save) => save()); + const commit = renderCommit(params); + + await act(async () => { + await expect(commit(selectionStub(), [], options)).resolves.toBeUndefined(); + }); + + expect(showToast).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/studio/src/hooks/useDomEditPositionPatchCommit.ts b/packages/studio/src/hooks/useDomEditPositionPatchCommit.ts index 1141a91d7c..1f6489abe4 100644 --- a/packages/studio/src/hooks/useDomEditPositionPatchCommit.ts +++ b/packages/studio/src/hooks/useDomEditPositionPatchCommit.ts @@ -35,7 +35,12 @@ export function useDomEditPositionPatchCommit({ skipRefresh: options.skipRefresh ?? true, }); }).catch((error) => { - if (error instanceof DomEditSaveQueueOpenError) return; + // A paused save queue is not worth a toast: the paused-save banner is + // already on screen, and one toast per blocked edit is what this branch + // exists to prevent. It still has to REJECT, though. Swallowing it + // resolved the commit, which skipped the caller's revert, so the element + // stayed where the drag put it while nothing reached the file. + if (error instanceof DomEditSaveQueueOpenError) throw error; showToast(error instanceof Error ? error.message : "Failed to save position"); trackStudioSaveFailure({ source: "dom_edit", diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index 94076151a0..57cdfb8fb4 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -20,6 +20,7 @@ import { type LayerRevealCommitOwnership, } from "../components/editor/useLayerRevealOverride"; import type { CommitDomEditPatchBatches, DomEditPatchBatch } from "./domEditCommitTypes"; +import { domEditCommitDeclined } from "./domEditCommitRunner"; import { cutoverCommittedOrThrow, type CutoverResult } from "../utils/sdkCutover"; import { studioWriteHeaders } from "../utils/studioFileVersion"; @@ -89,9 +90,9 @@ export function useElementLifecycleOps({ // fallow-ignore-next-line complexity async (selections: DomEditSelection[]) => { const pid = projectIdRef.current; - if (!pid) return; + if (!pid) return domEditCommitDeclined("no-project"); const [selection] = selections; - if (!selection) return; + if (!selection) return domEditCommitDeclined("no-selection"); const label = selections.length === 1 ? selection.label || selection.id || selection.selector || selection.tagName @@ -208,9 +209,13 @@ export function useElementLifecycleOps({ `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`, "info", ); + return { ok: true } as const; } catch (error) { const message = error instanceof Error ? error.message : "Failed to delete element"; showToast(message); + // The toast is what tells the human. The returned outcome is what tells + // a caller that has no screen to read. + return domEditCommitDeclined("persist-failed"); } }, [ From 2278c11d445fec4261240f91325d630f3ca2dba0 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 26 Aug 2026 22:16:25 -0400 Subject: [PATCH 3/5] fix(studio): honor DOM edit failure outcomes --- .../anchoredResizeCommitFeedsOffset.test.ts | 42 ++++++- .../editor/useDomEditOverlayGestures.ts | 10 +- .../studio/src/hooks/domEditCommitRunner.ts | 7 +- packages/studio/src/hooks/useDomEditWiring.ts | 2 +- .../src/hooks/useDomGeometryCommits.test.tsx | 1 + .../studio/src/hooks/useDomGeometryCommits.ts | 12 +- ...seElementLifecycleOps.multiDelete.test.tsx | 112 +++++++++++++----- .../src/hooks/useElementLifecycleOps.ts | 2 +- .../src/hooks/useGsapSelectionHandlers.ts | 6 +- 9 files changed, 148 insertions(+), 46 deletions(-) diff --git a/packages/studio/src/components/editor/anchoredResizeCommitFeedsOffset.test.ts b/packages/studio/src/components/editor/anchoredResizeCommitFeedsOffset.test.ts index 49065c709c..ab11dbd47d 100644 --- a/packages/studio/src/components/editor/anchoredResizeCommitFeedsOffset.test.ts +++ b/packages/studio/src/components/editor/anchoredResizeCommitFeedsOffset.test.ts @@ -1,6 +1,7 @@ // @vitest-environment happy-dom import { afterEach, describe, expect, it, vi } from "vitest"; +import { DomEditSaveQueueOpenError } from "../../utils/domEditSaveQueue"; import type { DomEditSelection } from "./domEditing"; import type { GestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures"; @@ -75,7 +76,9 @@ interface CommitCall { offset: { x: number; y: number } | undefined; } -function buildHarness() { +function buildHarness( + onBoxSizeCommit?: UseDomEditOverlayGesturesOptions["onBoxSizeCommitRef"]["current"], +) { const element = document.createElement("div"); document.body.append(element); @@ -136,9 +139,12 @@ function buildHarness() { onManualDragStartRef: ref(() => {}), onPathOffsetCommitRef: ref(() => {}), onGroupPathOffsetCommitRef: ref(() => {}), - onBoxSizeCommitRef: ref((_s, size, offset) => { - commits.push({ size, offset }); - }), + onBoxSizeCommitRef: ref( + onBoxSizeCommit ?? + ((_s, size, offset) => { + commits.push({ size, offset }); + }), + ), onRotationCommitRef: ref(() => {}), onCanvasPointerMoveRef: ref(() => Promise.resolve(null)), onCanvasMouseDown: () => {}, @@ -172,6 +178,15 @@ function evt(clientX: number, clientY: number) { } as unknown as React.PointerEvent; } +async function finishResize(handlers: ReturnType) { + handlers.startGesture("resize", evt(ORIGIN_CENTER.x + 100, ORIGIN_CENTER.y), { + resizeHandle: "se", + }); + handlers.onPointerMove(evt(ORIGIN_CENTER.x + 150, ORIGIN_CENTER.y)); + handlers.onPointerUp(evt(ORIGIN_CENTER.x + 150, ORIGIN_CENTER.y)); + await Promise.resolve(); +} + afterEach(() => { document.body.innerHTML = ""; }); @@ -211,4 +226,23 @@ describe("anchored corner resize — the release commit feeds the center-pin off expect(offset.x).toBeCloseTo(-(size.width - ORIGIN.width) / 2, 0); expect(offset.y).toBeCloseTo(-(size.height - ORIGIN.height) / 2, 0); }); + + it("does not log a paused save queue as an ordinary resize failure", async () => { + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { handlers } = buildHarness(() => Promise.reject(new DomEditSaveQueueOpenError())); + + await finishResize(handlers); + + expect(consoleError).not.toHaveBeenCalled(); + }); + + it("still logs an ordinary resize failure", async () => { + const failure = new Error("save failed"); + const consoleError = vi.spyOn(console, "error").mockImplementation(() => {}); + const { handlers } = buildHarness(() => Promise.reject(failure)); + + await finishResize(handlers); + + expect(consoleError).toHaveBeenCalledWith("resize commit failed", failure); + }); }); diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index 65e8bb4222..9f5dffbc56 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -58,6 +58,12 @@ import { import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug"; import { createGroupDragMover } from "./groupDragMove"; +import { DomEditSaveQueueOpenError } from "../../utils/domEditSaveQueue"; + +function logGestureCommitFailure(message: string, error: unknown): void { + if (error instanceof DomEditSaveQueueOpenError) return; + console.error(message, error); +} export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -409,7 +415,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu } void Promise.resolve(opts.onRotationCommitRef.current(sel, finalRotation)) .catch((error) => { - console.error("rotate commit failed", error); + logGestureCommitFailure("rotate commit failed", error); if ( g.manualEditDragToken && isStudioManualEditGestureCurrent(sel.element, g.manualEditDragToken) @@ -493,7 +499,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu opts.onBoxSizeCommitRef.current(sel, finalSize, finalOffset ?? undefined, restore), ) .catch((error) => { - console.error("resize commit failed", error); + logGestureCommitFailure("resize commit failed", error); }) .finally(() => { if (member) endManualOffsetDragMembers([member]); diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts index c80cec249e..12de67573a 100644 --- a/packages/studio/src/hooks/domEditCommitRunner.ts +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -61,9 +61,10 @@ export async function runDomEditCommit(config: DomEditCommitRunnerConfig): Promi * * `runDomEditCommit` resolves on persist failure by design (see its contract * above), so a caller cannot learn whether the write landed by awaiting it — a - * failed commit and a successful one are indistinguishable. The human path does - * not need to ask, because `onError` already put a toast on screen. A - * programmatic caller has no screen, so it has to be told. + * failed persist and a successful one are indistinguishable. Capture and apply + * bugs still reject. The human path does not need to ask about handled persist + * failures, because `onError` already put a toast on screen. A programmatic + * caller has no screen, so it has to be told. */ export type DomEditCommitDeclineReason = | "no-project" diff --git a/packages/studio/src/hooks/useDomEditWiring.ts b/packages/studio/src/hooks/useDomEditWiring.ts index 9b806957db..a6f42f8def 100644 --- a/packages/studio/src/hooks/useDomEditWiring.ts +++ b/packages/studio/src/hooks/useDomEditWiring.ts @@ -107,7 +107,7 @@ export interface UseDomEditWiringParams { resolvedFromValues?: Record, ) => Promise; removeAllKeyframes: (sel: DomEditSelection, animId: string) => Promise; - handleDomManualEditsReset: (sel: DomEditSelection) => void; + handleDomManualEditsReset: (sel: DomEditSelection) => Promise; } // fallow-ignore-next-line complexity diff --git a/packages/studio/src/hooks/useDomGeometryCommits.test.tsx b/packages/studio/src/hooks/useDomGeometryCommits.test.tsx index ac394c0c56..5f364f5cdf 100644 --- a/packages/studio/src/hooks/useDomGeometryCommits.test.tsx +++ b/packages/studio/src/hooks/useDomGeometryCommits.test.tsx @@ -52,6 +52,7 @@ describe("useDomGeometryCommits rollback", () => { commits!.handleDomBoxSizeCommit(selection, { width: 200, height: 160 }, { x: 30, y: 40 }), ).rejects.toBe(failure); await expect(commits!.handleDomRotationCommit(selection, { angle: 45 })).rejects.toBe(failure); + await expect(commits!.handleDomManualEditsReset(selection)).rejects.toBe(failure); expect(readStudioPathOffset(element)).toEqual({ x: 10, y: 20 }); expect(readStudioBoxSize(element)).toEqual({ width: 100, height: 80 }); diff --git a/packages/studio/src/hooks/useDomGeometryCommits.ts b/packages/studio/src/hooks/useDomGeometryCommits.ts index 8cabc37ac9..b72c6e1e5e 100644 --- a/packages/studio/src/hooks/useDomGeometryCommits.ts +++ b/packages/studio/src/hooks/useDomGeometryCommits.ts @@ -130,6 +130,9 @@ export function useDomGeometryCommits({ const handleDomManualEditsReset = useCallback( (selection: DomEditSelection) => { const element = selection.element; + const beforeOffset = captureStudioPathOffset(element); + const beforeSize = captureStudioBoxSize(element); + const beforeRotation = captureStudioRotation(element); const clearPatches = [ ...buildClearPathOffsetPatches(element), ...buildClearBoxSizePatches(element), @@ -139,11 +142,16 @@ export function useDomGeometryCommits({ clearStudioBoxSize(element); clearStudioRotation(element); // skipRefresh:false triggers reloadPreview() which re-syncs selection on load - void commitPositionPatchToHtml(selection, clearPatches, { + return commitPositionPatchToHtml(selection, clearPatches, { label: "Reset layer edits", coalesceKey: `manual-reset:${getDomEditTargetKey(selection)}`, skipRefresh: false, - }).catch(() => undefined); + }).catch((error) => { + restoreStudioPathOffset(element, beforeOffset); + restoreStudioBoxSize(element, beforeSize); + restoreStudioRotation(element, beforeRotation); + throw error; + }); }, [commitPositionPatchToHtml], ); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx index 245bc5d84e..cbedafe8c3 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx @@ -6,6 +6,8 @@ import { useElementLifecycleOps } from "./useElementLifecycleOps"; import { makeLifecycleOpsParams } from "./elementLifecycleOpsTestUtils"; import { mountReactHarness, makeSelection } from "./domSelectionTestHarness"; +Reflect.set(globalThis, "IS_REACT_ACT_ENVIRONMENT", true); + function selectionFor(id: string) { const el = document.createElement("div"); el.id = id; @@ -13,28 +15,51 @@ function selectionFor(id: string) { return { ...makeSelection(id, el), sourceFile: "index.html" }; } +function mountDeleteOps(overrides: Partial[0]> = {}) { + const captured: { ops: ReturnType | null } = { ops: null }; + function Probe() { + captured.ops = useElementLifecycleOps( + makeLifecycleOpsParams({ + commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never), + ...overrides, + }), + ); + return null; + } + mountReactHarness(); + if (!captured.ops) throw new Error("hook did not initialize"); + return captured.ops; +} + describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { const removed: string[] = []; const requests: string[] = []; let changes = true; + let removeOk = true; beforeEach(() => { removed.length = 0; requests.length = 0; changes = true; + removeOk = true; vi.stubGlobal( "fetch", vi.fn(async (url: string, init?: RequestInit) => { - requests.push(String(url)); + const requestUrl = String(url); + requests.push(requestUrl); const body = JSON.parse(String(init?.body ?? "{}")) as { targets?: { id?: string; selector?: string }[]; }; - for (const target of body.targets ?? []) { - const key = target.id ?? target.selector; - if (key) removed.push(key); - } + const keys = (body.targets ?? []) + .map((target) => target.id ?? target.selector) + .filter((key): key is string => key !== undefined); + removed.push(...keys); + const isRemove = requestUrl.includes("/file-mutations/remove-elements/"); + const status = isRemove && !removeOk ? 500 : 200; return { - ok: true, + ok: status === 200, + status, + text: async () => (status === 200 ? "" : "server said no"), json: async () => ({ changed: changes, content: "" }), } as unknown as Response; }), @@ -48,21 +73,12 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { it("removes every selected element, not just the first", async () => { // The reported bug: select several elements on the canvas, press Delete, and // one disappears while the rest stay — still drawn as selected. - let ops: ReturnType | null = null; - function Probe() { - ops = useElementLifecycleOps( - makeLifecycleOpsParams({ - commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never), - projectIdRef: { current: "p1" }, - }), - ); - return null; - } - mountReactHarness(); + const ops = mountDeleteOps({ projectIdRef: { current: "p1" } }); const selections = ["a", "b", "c"].map(selectionFor); + let outcome: unknown; await act(async () => { - await ops!.handleDomEditElementsDelete(selections); + outcome = await ops.handleDomEditElementsDelete(selections); }); // The defect: only the first was ever removed. @@ -70,6 +86,49 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { // And one request for the selection, not one per member: a canvas selection // runs to hundreds, and a round trip each made Delete look like a no-op. expect(requests.filter((url) => url.includes("remove-elements"))).toHaveLength(1); + expect(outcome).toEqual({ ok: true }); + }); + + it("reports a successful SDK delete as landed", async () => { + const ops = mountDeleteOps({ + projectIdRef: { current: "p1" }, + onTrySdkDelete: vi.fn(async () => ({ status: "committed", version: "v1" }) as const), + }); + + const target = { ...selectionFor("a"), hfId: "hf-a" }; + let outcome: unknown; + await act(async () => { + outcome = await ops.handleDomEditElementsDelete([target]); + }); + + expect(outcome).toEqual({ ok: true }); + expect(requests.some((url) => url.includes("remove-elements"))).toBe(false); + }); + + it("reports missing project and selection without starting a request", async () => { + const projectIdRef = { current: null as string | null }; + const ops = mountDeleteOps({ projectIdRef }); + + await expect(ops.handleDomEditElementsDelete([selectionFor("a")])).resolves.toEqual({ + ok: false, + reason: "no-project", + }); + projectIdRef.current = "p1"; + await expect(ops.handleDomEditElementsDelete([])).resolves.toEqual({ + ok: false, + reason: "no-selection", + }); + expect(requests).toEqual([]); + }); + + it("reports an HTTP write failure instead of only toasting", async () => { + removeOk = false; + const ops = mountDeleteOps({ projectIdRef: { current: "p1" } }); + + await expect(ops.handleDomEditElementsDelete([selectionFor("a")])).resolves.toEqual({ + ok: false, + reason: "persist-failed", + }); }); it("says so when the preview is stale instead of claiming a delete", async () => { @@ -78,23 +137,14 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { // nothing at all, with nothing on screen to explain it. changes = false; const showToast = vi.fn(); - let ops: ReturnType | null = null; - function Probe() { - ops = useElementLifecycleOps( - makeLifecycleOpsParams({ - commitDomEditPatchBatches: vi.fn(async () => ({ ok: true }) as never), - projectIdRef: { current: "p1" }, - showToast, - }), - ); - return null; - } - mountReactHarness(); + const ops = mountDeleteOps({ projectIdRef: { current: "p1" }, showToast }); + let outcome: unknown; await act(async () => { - await ops!.handleDomEditElementsDelete([selectionFor("a")]); + outcome = await ops.handleDomEditElementsDelete([selectionFor("a")]); }); expect(showToast.mock.calls.flat().join(" ")).toContain("out of date"); + expect(outcome).toEqual({ ok: false, reason: "persist-failed" }); }); }); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index 57cdfb8fb4..aa34b37229 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -142,7 +142,7 @@ export function useElementLifecycleOps({ `Deleted ${label}. Use Undo to restore ${sameFile.length === 1 ? "it" : "them"}.`, "info", ); - return; + return { ok: true } as const; } } diff --git a/packages/studio/src/hooks/useGsapSelectionHandlers.ts b/packages/studio/src/hooks/useGsapSelectionHandlers.ts index c7579219eb..9b39cd1ecb 100644 --- a/packages/studio/src/hooks/useGsapSelectionHandlers.ts +++ b/packages/studio/src/hooks/useGsapSelectionHandlers.ts @@ -111,7 +111,7 @@ export function useGsapSelectionHandlers({ ) => Promise; removeAllKeyframes: (sel: DomEditSelection, animId: string) => Promise; - handleDomManualEditsReset: (sel: DomEditSelection) => void; + handleDomManualEditsReset: (sel: DomEditSelection) => Promise; selectedGsapAnimations: GsapAnimation[]; showToast: (message: string, tone?: "error" | "info") => void; }) { @@ -230,7 +230,9 @@ export function useGsapSelectionHandlers({ }, ); if (domEditSelection.element.hasAttribute("data-hf-studio-path-offset")) { - handleDomManualEditsReset(domEditSelection); + // The reset owns rollback and the position commit already owns user and + // telemetry reporting. This is only the fire-and-forget UI boundary. + void handleDomManualEditsReset(domEditSelection).catch(() => undefined); } }, [domEditSelection, addGsapAnimation, handleDomManualEditsReset, trackGsapHandlerFailure], From 2a0a034dd5914a7e2aca732acd3f83d0825cbdfb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 26 Aug 2026 22:53:33 -0400 Subject: [PATCH 4/5] fix(studio): classify stale delete previews --- packages/studio/src/hooks/domEditCommitRunner.ts | 1 + .../src/hooks/useElementLifecycleOps.multiDelete.test.tsx | 2 +- packages/studio/src/hooks/useElementLifecycleOps.ts | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/hooks/domEditCommitRunner.ts b/packages/studio/src/hooks/domEditCommitRunner.ts index 12de67573a..b4a4a6cd2f 100644 --- a/packages/studio/src/hooks/domEditCommitRunner.ts +++ b/packages/studio/src/hooks/domEditCommitRunner.ts @@ -72,6 +72,7 @@ export type DomEditCommitDeclineReason = | "geometry-property" | "styles-not-editable" | "not-text-editable" + | "preview-stale" | "persist-failed"; export type DomEditCommitOutcome = { ok: true } | { ok: false; reason: DomEditCommitDeclineReason }; diff --git a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx index cbedafe8c3..93d1a90403 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx +++ b/packages/studio/src/hooks/useElementLifecycleOps.multiDelete.test.tsx @@ -145,6 +145,6 @@ describe("useElementLifecycleOps — deleting a canvas multi-selection", () => { }); expect(showToast.mock.calls.flat().join(" ")).toContain("out of date"); - expect(outcome).toEqual({ ok: false, reason: "persist-failed" }); + expect(outcome).toEqual({ ok: false, reason: "preview-stale" }); }); }); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index aa34b37229..ac803141a4 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -175,7 +175,8 @@ export function useElementLifecycleOps({ // matching at all means the preview is describing a document the file // does not have — say so rather than reporting a delete that happened. reloadPreview(); - throw new Error("Nothing to delete — the preview was out of date. Try again."); + showToast("Nothing to delete, the preview was out of date. Try again."); + return domEditCommitDeclined("preview-stale"); } const patchedContent = typeof removeData.content === "string" ? removeData.content : originalContent; From 69c4402f74b38954a71e36e7180ed5a92615e531 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Wed, 26 Aug 2026 23:32:58 -0400 Subject: [PATCH 5/5] fix(studio): enforce DOM edit outcome types --- packages/studio/src/hooks/useDomEditTextCommits.ts | 5 +++-- packages/studio/src/hooks/useElementLifecycleOps.ts | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/hooks/useDomEditTextCommits.ts b/packages/studio/src/hooks/useDomEditTextCommits.ts index a4a9e03b1e..557ee7abe9 100644 --- a/packages/studio/src/hooks/useDomEditTextCommits.ts +++ b/packages/studio/src/hooks/useDomEditTextCommits.ts @@ -33,6 +33,7 @@ import { domEditCommitDeclined, runDomEditCommit, runReportedDomEditCommit, + type DomEditCommitOutcome, } from "./domEditCommitRunner"; import { useDomEditAttributeCommits } from "./useDomEditAttributeCommits"; import type { InlineTextEditCommit } from "./useInlineTextEdit"; @@ -188,7 +189,7 @@ export function useDomEditTextCommits({ }); const handleDomStyleCommit = useCallback( - async (property: string, value: string) => { + async (property: string, value: string): Promise => { if (!domEditSelection) return domEditCommitDeclined("no-selection"); if (isManualGeometryStyleProperty(property)) return domEditCommitDeclined("geometry-property"); @@ -272,7 +273,7 @@ export function useDomEditTextCommits({ ); const handleDomTextCommit = useCallback( - async (value: string, fieldKey?: string) => { + async (value: string, fieldKey?: string): Promise => { if (!domEditSelection) return domEditCommitDeclined("no-selection"); if (!isTextEditableSelection(domEditSelection)) { return domEditCommitDeclined("not-text-editable"); diff --git a/packages/studio/src/hooks/useElementLifecycleOps.ts b/packages/studio/src/hooks/useElementLifecycleOps.ts index ac803141a4..793ebc237c 100644 --- a/packages/studio/src/hooks/useElementLifecycleOps.ts +++ b/packages/studio/src/hooks/useElementLifecycleOps.ts @@ -20,7 +20,7 @@ import { type LayerRevealCommitOwnership, } from "../components/editor/useLayerRevealOverride"; import type { CommitDomEditPatchBatches, DomEditPatchBatch } from "./domEditCommitTypes"; -import { domEditCommitDeclined } from "./domEditCommitRunner"; +import { domEditCommitDeclined, type DomEditCommitOutcome } from "./domEditCommitRunner"; import { cutoverCommittedOrThrow, type CutoverResult } from "../utils/sdkCutover"; import { studioWriteHeaders } from "../utils/studioFileVersion"; @@ -88,7 +88,7 @@ export function useElementLifecycleOps({ // fallow-ignore-next-line complexity const handleDomEditElementsDelete = useCallback( // fallow-ignore-next-line complexity - async (selections: DomEditSelection[]) => { + async (selections: DomEditSelection[]): Promise => { const pid = projectIdRef.current; if (!pid) return domEditCommitDeclined("no-project"); const [selection] = selections;