Skip to content

Commit 1ea0ed5

Browse files
miguel-heygenmiguel-heygen
authored
fix(studio): stabilize manual drag targets (#1393)
## Problem Studio manual drag had two bad target paths in `hf-keyframes-test`: - Drag could start from a cached hover selection instead of the selected overlay, so moving `#cta` could write an offset to the wrong element while the overlay appeared correct. - `#stage` is the visual 1920x1080 canvas, but the composition metadata lives on `<html data-composition-id ...>`. Studio treated `#stage` as a normal movable layer, so dragging it moved the coordinate basis the overlay depends on and left Undo with no reliable saved edit to reverse. ## Fix - Manual drag now starts only from the selected overlay bounds; canvas pointer-down no longer starts movement from `hoverSelectionRef`. - Added a structural root-layer heuristic: a direct body child matching the composition root's declared `data-width` / `data-height` is selectable and style-editable, but cannot receive manual offset/size/rotation edits. - Manual geometry commits now return/rethrow the save promise so failed writes roll back optimistic DOM changes instead of silently sticking. - GSAP-targeted CSS fallback edits are blocked before applying manual geometry when the GSAP drag intercept is unavailable. ## Tests - New regression coverage for stale-hover drag start and full-canvas root-stage capability resolution. - Focused DOM-edit suite: 119/119 pass. - Full Studio suite: 800 pass, 18 todo. - Studio typecheck clean; oxlint/oxfmt clean; lefthook pre-commit clean. - Studio build passes with the existing Vite chunk-size warning. - Browser verified with `agent-browser`: attempted `#stage` drag writes no offset and keeps Undo disabled; `#cta` drag writes exactly one offset to `#cta`; Undo returns the fixture to zero offsets. Local browser artifacts: `artifacts/studio-cta-drag/stage-cta-drag-fix.webm`, `artifacts/studio-cta-drag/stage-cta-drag-fix-final.png`.
1 parent d0a7f7d commit 1ea0ed5

8 files changed

Lines changed: 390 additions & 96 deletions

File tree

‎packages/studio/src/components/editor/DomEditOverlay.test.ts‎

Lines changed: 159 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import React, { act } from "react";
44
import { createRoot } from "react-dom/client";
5-
import { describe, expect, it, vi } from "vitest";
5+
import { beforeEach, describe, expect, it, vi } from "vitest";
66
import { Window } from "happy-dom";
77
import {
88
DomEditOverlay,
@@ -19,13 +19,21 @@ import type { DomEditSelection } from "./domEditing";
1919
// React 19 warns unless the test environment opts into act().
2020
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
2121

22+
const gestureSpies = vi.hoisted(() => ({
23+
startGesture: vi.fn(() => true),
24+
startGroupDrag: vi.fn(),
25+
onPointerMove: vi.fn(),
26+
onPointerUp: vi.fn(),
27+
clearPointerState: vi.fn(),
28+
}));
29+
2230
vi.mock("./useDomEditOverlayGestures", () => ({
2331
createDomEditOverlayGestureHandlers: () => ({
24-
startGesture: () => true,
25-
startGroupDrag: () => {},
26-
onPointerMove: () => {},
27-
onPointerUp: () => {},
28-
clearPointerState: () => {},
32+
startGesture: gestureSpies.startGesture,
33+
startGroupDrag: gestureSpies.startGroupDrag,
34+
onPointerMove: gestureSpies.onPointerMove,
35+
onPointerUp: gestureSpies.onPointerUp,
36+
clearPointerState: gestureSpies.clearPointerState,
2937
}),
3038
}));
3139

@@ -34,9 +42,18 @@ vi.mock("./useDomEditOverlayRects", async () => {
3442
const { rectsEqual } = await import("./domEditOverlayGeometry");
3543

3644
return {
37-
useDomEditOverlayRects: () => {
38-
const [overlayRect, setOverlayRectState] = React.useState(null);
39-
const overlayRectRef = React.useRef(null);
45+
useDomEditOverlayRects: (options: { selectionRef: { current: unknown } }) => {
46+
const defaultSelectionRect = {
47+
left: 24,
48+
top: 36,
49+
width: 180,
50+
height: 72,
51+
editScaleX: 1,
52+
editScaleY: 1,
53+
};
54+
const initialOverlayRect = options.selectionRef.current ? defaultSelectionRect : null;
55+
const [overlayRect, setOverlayRectState] = React.useState(initialOverlayRect);
56+
const overlayRectRef = React.useRef(initialOverlayRect);
4057
const [groupOverlayItems, setGroupOverlayItemsState] = React.useState([]);
4158
const groupOverlayItemsRef = React.useRef([]);
4259

@@ -85,6 +102,30 @@ vi.mock("./domEditOverlayGeometry", async () => {
85102
};
86103
});
87104

105+
function createOverlayProps(args: {
106+
iframeRef: { current: HTMLIFrameElement | null };
107+
selection: DomEditSelection | null;
108+
hoverSelection: DomEditSelection | null;
109+
onSelectionChange: (next: DomEditSelection) => void;
110+
}) {
111+
return {
112+
iframeRef: args.iframeRef,
113+
activeCompositionPath: null,
114+
selection: args.selection,
115+
hoverSelection: args.hoverSelection,
116+
groupSelections: [],
117+
onCanvasMouseDown: () => {},
118+
onCanvasPointerMove: () => Promise.resolve(args.hoverSelection ?? args.selection),
119+
onCanvasPointerLeave: () => {},
120+
onSelectionChange: args.onSelectionChange,
121+
onBlockedMove: () => {},
122+
onPathOffsetCommit: () => {},
123+
onGroupPathOffsetCommit: () => {},
124+
onBoxSizeCommit: () => {},
125+
onRotationCommit: () => {},
126+
};
127+
}
128+
88129
describe("focusDomEditOverlayElement", () => {
89130
it("focuses the canvas overlay without scrolling", () => {
90131
const calls: Array<FocusOptions | undefined> = [];
@@ -97,7 +138,94 @@ describe("focusDomEditOverlayElement", () => {
97138
});
98139

99140
describe("DomEditOverlay", () => {
100-
it("renders selected bounds right after clicking a movable selection", async () => {
141+
beforeEach(() => {
142+
gestureSpies.startGesture.mockClear();
143+
gestureSpies.startGroupDrag.mockClear();
144+
gestureSpies.onPointerMove.mockClear();
145+
gestureSpies.onPointerUp.mockClear();
146+
gestureSpies.clearPointerState.mockClear();
147+
});
148+
149+
it("does not start a drag from a stale hover target on canvas pointer-down", () => {
150+
const host = document.createElement("div");
151+
document.body.append(host);
152+
const root = createRoot(host);
153+
const selection: DomEditSelection = {
154+
element: document.createElement("div"),
155+
id: "cta-label",
156+
selector: ".cta-label",
157+
selectorIndex: 0,
158+
sourceFile: "index.html",
159+
tagName: "span",
160+
label: "CTA Label",
161+
textContent: "Add to basket",
162+
textFields: [],
163+
capabilities: {
164+
canEditText: true,
165+
canEditLayout: true,
166+
canMove: true,
167+
canApplyManualOffset: true,
168+
canApplyManualSize: false,
169+
canApplyManualRotation: false,
170+
canAdjustOpacity: true,
171+
canAdjustFill: true,
172+
canAdjustBorderRadius: true,
173+
canAdjustStroke: true,
174+
canAdjustShadow: true,
175+
canAdjustZIndex: true,
176+
},
177+
computedStyle: {
178+
display: "inline",
179+
position: "static",
180+
},
181+
};
182+
183+
let currentSelection: DomEditSelection | null = null;
184+
const iframeRef = { current: document.createElement("iframe") as HTMLIFrameElement | null };
185+
186+
function Harness() {
187+
const [selected, setSelected] = React.useState<DomEditSelection | null>(null);
188+
currentSelection = selected;
189+
190+
return React.createElement(
191+
DomEditOverlay,
192+
createOverlayProps({
193+
iframeRef,
194+
selection: selected,
195+
hoverSelection: selection,
196+
onSelectionChange: (next: DomEditSelection) => setSelected(next),
197+
}),
198+
);
199+
}
200+
201+
act(() => {
202+
root.render(React.createElement(Harness));
203+
});
204+
205+
const overlay = host.querySelector('[aria-label="Composition canvas"]') as HTMLDivElement;
206+
expect(overlay).toBeTruthy();
207+
208+
act(() => {
209+
overlay.dispatchEvent(
210+
new PointerEvent("pointerdown", {
211+
bubbles: true,
212+
button: 0,
213+
clientX: 120,
214+
clientY: 80,
215+
}),
216+
);
217+
});
218+
219+
expect(gestureSpies.startGesture).not.toHaveBeenCalled();
220+
expect(currentSelection).toBe(null);
221+
222+
act(() => {
223+
root.unmount();
224+
});
225+
host.remove();
226+
});
227+
228+
it("starts movement from the selected bounds", async () => {
101229
// The overlay's compRect updates via a RAF loop reading iframe + overlay
102230
// getBoundingClientRect. happy-dom returns all zeros for newly-created
103231
// elements with no layout, so without stubs the RAF early-returns
@@ -153,32 +281,24 @@ describe("DomEditOverlay", () => {
153281
},
154282
};
155283

156-
let currentSelection: DomEditSelection | null = null;
284+
let currentSelection: DomEditSelection | null = selection;
157285
const iframeRef = { current: document.createElement("iframe") as HTMLIFrameElement | null };
158286
const originalPointerCapture = HTMLDivElement.prototype.setPointerCapture;
159287
HTMLDivElement.prototype.setPointerCapture = () => {};
160288

161289
function Harness() {
162-
const [selected, setSelected] = React.useState<DomEditSelection | null>(null);
290+
const [selected, setSelected] = React.useState<DomEditSelection | null>(selection);
163291
currentSelection = selected;
164292

165-
return React.createElement(DomEditOverlay, {
166-
iframeRef,
167-
activeCompositionPath: null,
168-
selection: selected,
169-
// Simulate the element being hovered before pointer-down (real users always hover first)
170-
hoverSelection: selection,
171-
groupSelections: [],
172-
onCanvasMouseDown: () => {},
173-
onCanvasPointerMove: () => Promise.resolve(selection),
174-
onCanvasPointerLeave: () => {},
175-
onSelectionChange: (next: DomEditSelection) => setSelected(next),
176-
onBlockedMove: () => {},
177-
onPathOffsetCommit: () => {},
178-
onGroupPathOffsetCommit: () => {},
179-
onBoxSizeCommit: () => {},
180-
onRotationCommit: () => {},
181-
});
293+
return React.createElement(
294+
DomEditOverlay,
295+
createOverlayProps({
296+
iframeRef,
297+
selection: selected,
298+
hoverSelection: null,
299+
onSelectionChange: (next: DomEditSelection) => setSelected(next),
300+
}),
301+
);
182302
}
183303

184304
act(() => {
@@ -197,8 +317,13 @@ describe("DomEditOverlay", () => {
197317
const overlay = host.querySelector('[aria-label="Composition canvas"]') as HTMLDivElement;
198318
expect(overlay).toBeTruthy();
199319

320+
const selectionBox = host.querySelector(
321+
'[data-dom-edit-selection-box="true"]',
322+
) as HTMLDivElement;
323+
expect(selectionBox).toBeTruthy();
324+
200325
act(() => {
201-
overlay.dispatchEvent(
326+
selectionBox.dispatchEvent(
202327
new PointerEvent("pointerdown", {
203328
bubbles: true,
204329
button: 0,
@@ -209,7 +334,10 @@ describe("DomEditOverlay", () => {
209334
});
210335

211336
expect(currentSelection).toBe(selection);
212-
expect(host.querySelector('[data-dom-edit-selection-box="true"]')).toBeTruthy();
337+
expect(gestureSpies.startGesture).toHaveBeenCalledWith(
338+
"drag",
339+
expect.objectContaining({ button: 0 }),
340+
);
213341

214342
act(() => {
215343
root.unmount();

‎packages/studio/src/components/editor/DomEditOverlay.tsx‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { memo, useMemo, useRef, useState, type RefObject } from "react";
22
import { useMountEffect } from "../../hooks/useMountEffect";
33
import { type DomEditSelection } from "./domEditing";
4-
import { resolveDomEditGroupOverlayRect, toOverlayRect } from "./domEditOverlayGeometry";
4+
import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry";
55
import {
66
type BlockedMoveState,
77
type FocusableDomEditOverlay,
@@ -304,28 +304,6 @@ export const DomEditOverlay = memo(function DomEditOverlay({
304304

305305
const target = event.target as HTMLElement | null;
306306
if (target?.closest('[data-dom-edit-selection-box="true"]')) return;
307-
308-
const candidate = hoverSelectionRef.current;
309-
if (!candidate?.capabilities.canApplyManualOffset) return;
310-
311-
const overlayEl = overlayRef.current;
312-
const iframe = iframeRef.current;
313-
const candidateRect =
314-
overlayEl && iframe ? toOverlayRect(overlayEl, iframe, candidate.element) : null;
315-
if (!candidateRect) return;
316-
317-
suppressNextOverlayMouseDownRef.current = true;
318-
selectionRef.current = candidate;
319-
setOverlayRect(candidateRect);
320-
const didStartGesture = gestures.startGesture("drag", event, {
321-
selection: candidate,
322-
rect: candidateRect,
323-
});
324-
if (!didStartGesture) {
325-
suppressNextOverlayMouseDownRef.current = false;
326-
return;
327-
}
328-
onSelectionChangeRef.current(candidate);
329307
};
330308

331309
const handleBoxClick = (event: React.MouseEvent<HTMLDivElement>) => {

‎packages/studio/src/components/editor/PropertyPanel.tsx‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,6 @@ export {
3434
setCssFilterFunctionPx,
3535
} from "./propertyPanelHelpers";
3636

37-
/* ------------------------------------------------------------------ */
38-
/* PropertyPanel */
39-
/* ------------------------------------------------------------------ */
40-
4137
// fallow-ignore-next-line complexity
4238
export const PropertyPanel = memo(function PropertyPanel({
4339
projectId,
@@ -177,10 +173,12 @@ export const PropertyPanel = memo(function PropertyPanel({
177173
return;
178174
}
179175
const current = readStudioPathOffset(element.element);
180-
onSetManualOffset(element, {
181-
x: axis === "x" ? parsed : current.x,
182-
y: axis === "y" ? parsed : current.y,
183-
});
176+
void Promise.resolve(
177+
onSetManualOffset(element, {
178+
x: axis === "x" ? parsed : current.x,
179+
y: axis === "y" ? parsed : current.y,
180+
}),
181+
).catch(() => undefined);
184182
};
185183

186184
// fallow-ignore-next-line complexity
@@ -204,17 +202,19 @@ export const PropertyPanel = memo(function PropertyPanel({
204202
current.height > 0
205203
? current.height
206204
: (parsePxMetricValue(styles.height ?? "") ?? element.boundingBox.height);
207-
onSetManualSize(element, {
208-
width: axis === "width" ? parsed : width,
209-
height: axis === "height" ? parsed : height,
210-
});
205+
void Promise.resolve(
206+
onSetManualSize(element, {
207+
width: axis === "width" ? parsed : width,
208+
height: axis === "height" ? parsed : height,
209+
}),
210+
).catch(() => undefined);
211211
};
212212

213213
const manualRotation = readStudioRotation(element.element);
214214
const commitManualRotation = (nextValue: string) => {
215215
const parsed = Number.parseFloat(nextValue);
216216
if (!Number.isFinite(parsed)) return;
217-
onSetManualRotation(element, { angle: parsed });
217+
void Promise.resolve(onSetManualRotation(element, { angle: parsed })).catch(() => undefined);
218218
};
219219

220220
const elStart = Number.parseFloat(element?.dataAttributes?.start ?? "0") || 0;

0 commit comments

Comments
 (0)