From bd03f205fea98534b99954c7f99f1fcba9683726 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 16:48:43 -0700 Subject: [PATCH 01/26] fix(studio): size the selection box by the transform the element actually paints under The box around a text layer inside the playground card stopped mid-word. The layer is 260px wide and paints 313, because its parent carries `scale(1.2)`, and the chrome read only the element's OWN transform. The top-left looked right, since the corners are anchored to the real bounding rect, so only the right and bottom edges fell short, by exactly 1/1.2. The same read decides whether to draw the box rotated at all, so an element whose parent is rotated got an upright box over a rotated one. The transform is now accumulated from the element up to the composition root. Only the linear part matters: each transform's origin contributes translation, and translation is already discarded by matching the corners to the element's bounding rect, so composing the matrices is enough and no per-ancestor origin has to be unpicked. The walk stops inside the composition document, because the canvas zoom lives on the iframe in Studio's own document and is applied separately. The fake DOMMatrix the geometry tests use gained the `multiply` it now needs. --- .../editor/domEditOverlayGeometry.test.ts | 50 +++++++++++++++++++ .../editor/domEditOverlayGeometry.ts | 30 ++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts index 217f3d3430..5f9475c5e0 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts @@ -67,6 +67,17 @@ describe("orientedOverlayRect — rotation gate (perf fix, V15 18a/18b)", () => number, ]; } + /** `this` applied outside `other`, the way an ancestor composes over a child. */ + multiply(other: { a: number; b: number; c: number; d: number; e: number; f: number }) { + const out = new (this.constructor as new (init?: string) => this)(); + out.a = this.a * other.a + this.c * other.b; + out.b = this.b * other.a + this.d * other.b; + out.c = this.a * other.c + this.c * other.d; + out.d = this.b * other.c + this.d * other.d; + out.e = this.a * other.e + this.c * other.f + this.e; + out.f = this.b * other.e + this.d * other.f + this.f; + return out; + } transformPoint(pt: { x: number; y: number }) { return { x: this.a * pt.x + this.c * pt.y + this.e, @@ -156,6 +167,45 @@ describe("orientedOverlayRect — rotation gate (perf fix, V15 18a/18b)", () => expect(rect!.angle).toBeCloseTo(30, 3); }); + /** + * The selection box is drawn at the size the element PAINTS, which is the + * product of every transform between it and the composition root. + * + * A text layer inside a card carrying `scale(1.2)` was drawn at 1/1.2 of the + * text: the top-left was right, because the caller anchors that to the real + * bounding rect, and the right and bottom edges fell short. The same read + * decides whether to draw the box rotated, so an element inside a rotated + * parent got an upright box. + */ + const SCALE_1_2_MATRIX = "matrix(1.2, 0, 0, 1.2, 0, 0)"; + + it("sizes the box by the accumulated transform, not the element's own", () => { + const { overlayEl, iframe, el } = buildHarness(); + // The element carries no transform; its parent scales it by 1.2, so it + // paints at 240x120 and its bounding rect says so. + el.parentElement!.style.transform = SCALE_1_2_MATRIX; + el.style.transform = ROTATE_30DEG_MATRIX; + stubRect(el, { left: 400, top: 450, width: 240, height: 120 }); + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect).not.toBeNull(); + // 200x100 local, scaled by the ancestor, then rotated: the oriented box is + // the scaled local box, and the AABB it is anchored to is wider again. + expect(rect!.width).toBeCloseTo(240, 3); + expect(rect!.height).toBeCloseTo(120, 3); + expect(rect!.angle).toBeCloseTo(30, 3); + }); + + it("takes the rotated path when only an ANCESTOR is rotated", () => { + const { overlayEl, iframe, el } = buildHarness(); + el.parentElement!.style.transform = ROTATE_30DEG_MATRIX; + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect!.angle).toBeCloseTo(30, 3); + }); + it("preserves an ordinary element's rotation through the group-aware entry point", () => { const { overlayEl, iframe, el } = buildHarness(); el.style.transform = ROTATE_30DEG_MATRIX; diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index 70f2500958..76b312ce97 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -117,6 +117,28 @@ interface ElementTransformSnapshot { cs: CSSStyleDeclaration; } +/** + * The transform from the element's own box to the composition's, ACCUMULATED + * over its ancestors rather than read from the element alone. + * + * What the user sees is the product of every transform between the element and + * the composition root, and an element is routinely a child of something + * scaled or rotated. Reading only its own transform drew the selection box at + * the element's untransformed size: a text layer inside a card carrying + * `scale(1.2)` got a box at 1/1.2 of the text, with the top-left correct (the + * caller anchors that to the real bounding rect) and the right and bottom + * edges falling short. The same read decides whether to draw the box rotated, + * so an element inside a rotated parent got an upright box too. + * + * Only the linear part matters here. Each transform's origin contributes + * translation, and the caller discards translation by matching the corners' + * bounding box to the element's real one, so composing the matrices alone is + * enough and there is no per-ancestor origin to unpick. + * + * The walk stops at the composition document's root. The canvas zoom lives on + * the iframe element in Studio's own document and is applied separately by + * `computeOverlayRootScale`; including it here would count it twice. + */ function readElementTransformSnapshot( win: Window, element: HTMLElement, @@ -125,7 +147,13 @@ function readElementTransformSnapshot( if (!DOMMatrixCtor) return null; const cs = win.getComputedStyle(element); try { - const matrix = new DOMMatrixCtor(cs.transform === "none" ? "" : cs.transform); + let matrix = new DOMMatrixCtor(); + for (let node: HTMLElement | null = element; node; node = node.parentElement) { + const transform = node === element ? cs.transform : win.getComputedStyle(node).transform; + if (!transform || transform === "none") continue; + // An ancestor applies outside, so it multiplies on the left. + matrix = new DOMMatrixCtor(transform).multiply(matrix); + } return { matrix, cs }; } catch { return null; From cf75f6ed824604916f6c4022e14a143eb7c9c563 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 18:48:16 -0700 Subject: [PATCH 02/26] fix(studio): drag by the movement the element actually makes, not the one assumed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An element that had never been dragged skipped the movement measurement and took the canvas zoom as the whole screen mapping. Nothing above the element was considered, so any parent transform broke the drag: a card at rotationY 180 with scale 1.2 maps a rightward drag to -1.2x the zoom, meaning the text walked LEFT while the overlay followed the pointer, and the overlay only snapped onto the text at drop, when it re-measured. Measured on the live element in that card: one unit of drag offset moved it -0.757 px on x and +0.757 on y, where the skipped path assumed +0.631 on both. The measurement it skipped already handles this — it moves the element, watches where it lands, and inverts that, which is right for rotation, mirroring, scale and perspective alike. So the special case is gone and every drag measures. Same element after: a 120x80 pointer drag moves it 120.3x80.2. Rewrote the test that asserted the skipped path's identity matrix for an unmovable element. It now asserts the honest outcome: an element with no measurable movement is reported unmeasurable whether or not it carries a path offset, and the caller's existing fallback covers it. --- .../editor/manualOffsetDrag.test.ts | 47 +++++++++++++++++-- .../src/components/editor/manualOffsetDrag.ts | 34 +++++++------- 2 files changed, 59 insertions(+), 22 deletions(-) diff --git a/packages/studio/src/components/editor/manualOffsetDrag.test.ts b/packages/studio/src/components/editor/manualOffsetDrag.test.ts index 5af32996a0..9aeb6b2d4d 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.test.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.test.ts @@ -88,6 +88,41 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { expect(element.style.getPropertyValue("translate")).toBe(""); }); + /** + * The element that has never been offset is the common case, and it used to skip + * the measurement and assume the canvas zoom was the whole story. Any transform + * above the element makes that assumption wrong: the mirrored parent here sends a + * rightward drag left, so the overlay followed the pointer while the element went + * the other way, and only on drop did the overlay jump to where the element really + * was. The fixture mirrors x and scales both axes by 1.2, as a `rotationY: 180` + * card at `scale: 1.2` does. + */ + it("measures a mirrored parent even when the element carries no offset yet", () => { + const window = new Window(); + const element = window.document.createElement("div"); + window.document.body.append(element); + + element.getBoundingClientRect = () => { + const offsetX = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_X_PROP)) || 0; + const offsetY = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_Y_PROP)) || 0; + return new window.DOMRect(100 - 1.2 * offsetX, 200 + 1.2 * offsetY, 40, 20); + }; + + const measured = measureManualOffsetDragScreenToOffsetMatrix(element, { x: 0, y: 0 }); + if (!measured.ok) throw new Error(measured.reason); + + // Dragging one screen px right must move the element one screen px right, which + // on a mirrored parent means writing a NEGATIVE offset. + const offset = resolveManualOffsetForPointerDelta({ + initialOffset: { x: 0, y: 0 }, + screenToOffset: measured.matrix, + dx: 60, + dy: 60, + }); + expect(offset.x).toBeCloseTo(-50, 6); + expect(offset.y).toBeCloseTo(50, 6); + }); + it("measures movement in parent viewport pixels when the element is inside a scaled iframe", () => { const window = new Window(); const iframe = window.document.createElement("iframe"); @@ -133,7 +168,12 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { expect(nextOffset).toEqual({ x: 100, y: 50 }); }); - it("returns identity matrix for non-path-offset elements with zero initial offset", () => { + // Carrying no path offset used to be taken as permission to assume the response + // instead of measuring it. It is not a signal about the transforms above the + // element, so it no longer changes the answer: an element that does not move is + // unmeasurable either way, and the caller falls back rather than being handed a + // matrix that was never checked. + it("does not treat a missing path offset as a measurable response", () => { const window = new Window(); const element = window.document.createElement("div"); window.document.body.append(element); @@ -141,10 +181,7 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { const measured = measureManualOffsetDragScreenToOffsetMatrix(element, { x: 0, y: 0 }); - expect(measured.ok).toBe(true); - if (measured.ok) { - expectMatrixClose(measured.matrix, { a: 1, b: 0, c: 0, d: 1 }); - } + expect(measured.ok).toBe(false); }); it("rejects path-offset elements whose movement response cannot be measured", () => { diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index a808c792fb..e18b85c23b 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -213,9 +213,9 @@ export function applyManualOffsetDragMatrix(matrix: ManualOffsetDragMatrix, poin * The perspective w-divisor (matrix3d m44) of the element's current transform. * For a plain `translateZ(z)` under `perspective(p)`, m44 = (p - z) / p, so the * element renders 1/m44× larger and a translate of `d` composition px moves - * `d / m44` px on screen. Returns 1 for 2D transforms (no foreshortening). Used - * to keep the drag offset → screen-movement mapping correct for depth elements, - * which the flat-scale fast path below would otherwise get wrong by 1/m44. + * `d / m44` px on screen. Returns 1 for 2D transforms (no foreshortening). Only + * the unmeasurable-element fallback needs this — the measured path reads the + * foreshortening off the element's real movement along with everything else. */ function readTransformWDivisor(element: HTMLElement): number { const t = element.ownerDocument.defaultView?.getComputedStyle(element).transform; @@ -225,25 +225,25 @@ function readTransformWDivisor(element: HTMLElement): number { return Number.isFinite(w) && w > 0 ? w : 1; } +/** + * How far the element actually moves on screen per unit of drag offset, measured + * rather than assumed. + * + * The offset is written on the element, but what reaches the screen is that offset + * put through every transform above it. A parent carrying a rotation, a mirror, a + * scale or a perspective changes both the direction and the distance — a card at + * `rotationY: 180` sends a rightward drag left. Guessing this from the canvas zoom + * alone was wrong for every such element: the overlay tracked the pointer while the + * element went somewhere else, and the overlay only jumped to the truth on drop, + * when it re-measured. Moving the element and watching where it lands costs three + * layout reads once per gesture and is right for any transform, including ones no + * closed-form fast path would cover. + */ export function measureManualOffsetDragScreenToOffsetMatrix( element: HTMLElement, initialOffset: { x: number; y: number }, options: { probeSize?: number; scaleX?: number; scaleY?: number } = {}, ): { ok: true; matrix: ManualOffsetDragMatrix } | { ok: false; reason: string } { - if ( - !element.hasAttribute("data-hf-studio-path-offset") && - initialOffset.x === 0 && - initialOffset.y === 0 - ) { - const sx = options.scaleX || 1; - const sy = options.scaleY || 1; - // Fold in the perspective foreshortening: a depth element (z≠0) moves - // 1/m44× faster on screen than its flat scale implies, so the screen→offset - // matrix must scale by m44 or the element outruns the pointer/overlay. - const w = readTransformWDivisor(element); - return { ok: true, matrix: { a: w / sx, b: 0, c: 0, d: w / sy } }; - } - const probeSize = options.probeSize ?? DEFAULT_OFFSET_PROBE_PX; if (!Number.isFinite(probeSize) || probeSize <= 0) { return { ok: false, reason: "Invalid movement probe size." }; From f15a677270a8007e14ad8d9dce79faaf1c8eb8c3 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 19:38:12 -0700 Subject: [PATCH 03/26] fix(studio): shift-click adds the element under the pointer, not the last one hovered Shift-click read the hover cache and used it without checking what it described. That cache is filled asynchronously as the pointer moves, so passing over one element on the way to another leaves it naming the element you left. The shift-click then added THAT element, and because the same branch prevented the default and set the suppression flags, the mousedown path that would have resolved the point correctly never ran. Multi-select looked like it grabbed things at random, or like it did nothing. Reproduced on the canvas with a trace: hover #card, shift-click #dot-b, and the group gained #card. Same gesture after: the guard rejects the cache, the mousedown path resolves the point, and the group gains #dot-b. The cache is still used when it is provably about the point clicked, including when it names a clip ancestor of the element there, so the fast path survives for the common case of clicking straight at something. Adds `hf-select-debug` (localStorage, off by default) recording which selection branch ran and what it decided, and pulls the flag/format shared with `hf-reload-debug` into one place rather than copying it. --- .../components/editor/DomEditOverlay.test.ts | 49 ++++++++++++++++++- .../src/components/editor/DomEditOverlay.tsx | 33 ++++++++++++- .../editor/domEditOverlayGestures.ts | 19 +++++++ packages/studio/src/hooks/useDomSelection.ts | 8 +++ packages/studio/src/utils/reloadDebug.ts | 24 +-------- packages/studio/src/utils/selectDebug.ts | 9 ++++ packages/studio/src/utils/studioDebug.ts | 28 +++++++++++ 7 files changed, 145 insertions(+), 25 deletions(-) create mode 100644 packages/studio/src/utils/selectDebug.ts create mode 100644 packages/studio/src/utils/studioDebug.ts diff --git a/packages/studio/src/components/editor/DomEditOverlay.test.ts b/packages/studio/src/components/editor/DomEditOverlay.test.ts index 736bd7ecb2..0d857d7918 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.test.ts +++ b/packages/studio/src/components/editor/DomEditOverlay.test.ts @@ -14,7 +14,10 @@ import { resolveDomEditRotationGesture, } from "./DomEditOverlay"; import type { DomEditSelection } from "./domEditing"; -import { resolveResizeCenterAnchorOffset } from "./domEditOverlayGestures"; +import { + hoverCacheDescribesPoint, + resolveResizeCenterAnchorOffset, +} from "./domEditOverlayGestures"; // React 19 warns unless the test environment opts into act(). globalThis.IS_REACT_ACT_ENVIRONMENT = true; @@ -628,6 +631,50 @@ describe("resolveDomEditRotationGesture", () => { }); }); +/** + * Shift-click reads the hover cache instead of hit-testing, and the cache is + * filled asynchronously as the pointer moves. Pass over one element on the way to + * another and the cache still names the one you left, so the shift-click added + * THAT element and the click looked like it selected something at random. The + * guard is what makes the cache usable only when it is about the point clicked. + */ +describe("hoverCacheDescribesPoint", () => { + const doc = new Window().document; + + it("rejects a cache left behind by an element the pointer passed over", () => { + const passedOver = doc.createElement("div"); + const clicked = doc.createElement("div"); + doc.body.append(passedOver, clicked); + + expect(hoverCacheDescribesPoint(passedOver, clicked)).toBe(false); + }); + + it("accepts the cache when it names the element at the point", () => { + const clicked = doc.createElement("div"); + doc.body.append(clicked); + + expect(hoverCacheDescribesPoint(clicked, clicked)).toBe(true); + }); + + // The resolver is allowed to hand back a clip ancestor of the raw target, which + // still describes the same click — rejecting it would drop the fast path on + // every element that has children. + it("accepts an ancestor of the element at the point", () => { + const clip = doc.createElement("div"); + const child = doc.createElement("span"); + clip.append(child); + doc.body.append(clip); + + expect(hoverCacheDescribesPoint(clip, child)).toBe(true); + }); + + it("rejects a missing cache or an empty point", () => { + const el = doc.createElement("div"); + expect(hoverCacheDescribesPoint(null, el)).toBe(false); + expect(hoverCacheDescribesPoint(el, null)).toBe(false); + }); +}); + // resolveResizeCenterAnchorOffset is the UNROTATED (AABB) fallback used only when // the element's real transformed corners can't be measured. Center-anchored: a // width/height change grows the box from its top-left, drifting the center by half diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 2c6d6add3c..db98077531 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -13,6 +13,7 @@ import { type GestureState, type GroupGestureState, focusDomEditOverlayElement, + hoverCacheDescribesPoint, } from "./domEditOverlayGestures"; import { useDomEditOverlayRects } from "./useDomEditOverlayRects"; import { OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; @@ -43,6 +44,7 @@ export { hasDomEditRotationChanged, resolveDomEditRotationGesture, } from "./domEditOverlayGestures"; +import { logSelect } from "../../utils/selectDebug"; export type { DomEditGroupPathOffsetCommit } from "./domEditOverlayGestures"; interface DomEditOverlayProps { @@ -318,6 +320,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayMouseDown = (event: React.MouseEvent) => { if (!allowCanvasMovement) return; if (suppressNextOverlayMouseDownRef.current) { + logSelect("mousedown-suppressed", { shift: event.shiftKey }); suppressNextOverlayMouseDownRef.current = false; suppressNextBoxMouseDownRef.current = false; suppressNextBoxClickRef.current = false; @@ -326,6 +329,10 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } const target = event.target as HTMLElement | null; + logSelect("mousedown", { + shift: event.shiftKey, + onBox: Boolean(target?.closest('[data-dom-edit-selection-box="true"]')), + }); if (target?.closest('[data-dom-edit-selection-box="true"]')) return; // Allow clicks anywhere on the overlay — GSAP-translated elements can // extend beyond the composition rect into the gray zone, and users need @@ -341,9 +348,31 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayPointerDown = (event: React.PointerEvent) => { if (!allowCanvasMovement || event.button !== 0) return; if (event.shiftKey) { - // Use the already-updated hover selection rather than re-resolving async + // The hover selection is an ASYNC cache, so it can still describe whatever the + // pointer passed over on its way here. Reading it without checking is safe for + // a hover outline and wrong for a shift-click: the click silently adds THAT + // element instead of the one under the pointer, which reads as multi-select + // picking things at random. Confirm the cache is about this point with a + // synchronous hit-test; when it isn't, fall through untouched — no + // preventDefault, no suppression — so the mousedown path below resolves the + // point properly instead of this one guessing. const candidate = hoverSelectionRef.current; - if (!candidate) return; + const shiftIframe = iframeRef.current; + const pointTarget = shiftIframe + ? getPreviewTargetFromPointer( + shiftIframe, + event.clientX, + event.clientY, + activeCompositionPathRef.current, + ) + : null; + const cacheIsAboutThisPoint = hoverCacheDescribesPoint(candidate?.element, pointTarget); + logSelect("shift-pointerdown", { + candidate: candidate?.selector ?? candidate?.id ?? null, + pointTarget: pointTarget?.id ?? pointTarget?.tagName ?? null, + cacheIsAboutThisPoint, + }); + if (!candidate || !cacheIsAboutThisPoint) return; event.preventDefault(); event.stopPropagation(); suppressNextOverlayMouseDownRef.current = true; diff --git a/packages/studio/src/components/editor/domEditOverlayGestures.ts b/packages/studio/src/components/editor/domEditOverlayGestures.ts index d7fec86a87..562680f84b 100644 --- a/packages/studio/src/components/editor/domEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/domEditOverlayGestures.ts @@ -120,6 +120,25 @@ export function focusDomEditOverlayElement(element: FocusableDomEditOverlay | nu * when the element's real transformed corners can't be measured — the primary path * pins the measured center (rotation-safe) in useDomEditOverlayGestures. */ +/** + * Whether the hover cache may stand in for a hit-test at this point. + * + * The cache is filled asynchronously as the pointer moves, so it can describe an + * element the pointer has already left. That is harmless for drawing a hover + * outline and wrong for a shift-click, which would add the stale element to the + * selection instead of the one under the pointer. True only when the cached + * element IS the element at the point, or contains it — the resolver is allowed + * to hand back a clip ancestor of the raw target, and that still describes the + * same click. + */ +export function hoverCacheDescribesPoint( + cachedElement: Element | null | undefined, + elementAtPoint: Element | null | undefined, +): boolean { + if (!cachedElement || !elementAtPoint) return false; + return cachedElement === elementAtPoint || cachedElement.contains(elementAtPoint); +} + export function resolveResizeCenterAnchorOffset(input: { originWidth: number; originHeight: number; diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 32682b0810..76431558e1 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -24,6 +24,7 @@ import { } from "../components/editor/domEditing"; import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import { useStudioTestHooks } from "./useStudioTestHooks"; +import { logSelect } from "../utils/selectDebug"; // ── Types ── @@ -186,6 +187,13 @@ export function useDomSelection({ : (nextGroup[0] ?? null) : selection; + logSelect("apply", { + additive: isAdditiveSelection, + target: selection.selector ?? selection.id ?? null, + wasInGroup, + prevGroup: previousGroup.length, + nextGroup: nextGroup.length, + }); domEditSelectionRef.current = nextSelection; domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); diff --git a/packages/studio/src/utils/reloadDebug.ts b/packages/studio/src/utils/reloadDebug.ts index b884aa9f35..c9f4594dd5 100644 --- a/packages/studio/src/utils/reloadDebug.ts +++ b/packages/studio/src/utils/reloadDebug.ts @@ -5,26 +5,6 @@ // ask for reads as a flash. These lines answer the only question that matters // when one appears: who asked for it, and why the write that triggered it was // not recognised as Studio's own. -let enabled: boolean | null = null; +import { makeStudioDebugLogger } from "./studioDebug"; -function isEnabled(): boolean { - if (enabled === null) { - try { - enabled = localStorage.getItem("hf-reload-debug") === "1"; - } catch { - enabled = false; - } - } - return enabled; -} - -export function logReload( - stage: string, - data: Record | (() => Record) = {}, -): void { - if (!isEnabled()) return; - const details = typeof data === "function" ? data() : data; - console.log( - `[hf-reload] ${JSON.stringify({ stage, t: Math.round(performance.now()), ...details })}`, - ); -} +export const logReload = makeStudioDebugLogger("reload"); diff --git a/packages/studio/src/utils/selectDebug.ts b/packages/studio/src/utils/selectDebug.ts new file mode 100644 index 0000000000..364f16e85e --- /dev/null +++ b/packages/studio/src/utils/selectDebug.ts @@ -0,0 +1,9 @@ +// Canvas selection diagnostics — grep [hf-select]. Off by default; opt in with +// `localStorage.setItem("hf-select-debug", "1")` (then reload). +// +// Selection failures are silent by nature: a handler returns early and nothing +// happens, which looks identical to a click that never landed. These lines say +// which branch ran and what it decided. +import { makeStudioDebugLogger } from "./studioDebug"; + +export const logSelect = makeStudioDebugLogger("select"); diff --git a/packages/studio/src/utils/studioDebug.ts b/packages/studio/src/utils/studioDebug.ts new file mode 100644 index 0000000000..1a9bda9c4d --- /dev/null +++ b/packages/studio/src/utils/studioDebug.ts @@ -0,0 +1,28 @@ +// Opt-in diagnostic channels — one per question worth tracing, all off by +// default. Turn one on for the session with `localStorage.setItem("hf--debug", +// "1")` and reload, then grep the console for `[hf-]`. +// +// These exist because the interesting failures here are decisions, not crashes: +// a preview that reloads when it should not, a shift-click that selects nothing. +// Nothing is thrown and nothing is logged by default, so without a trace of the +// decision the only way to find the cause is to guess. +type DebugDetails = Record | (() => Record); +type DebugLogger = (stage: string, data?: DebugDetails) => void; + +export function makeStudioDebugLogger(name: string): DebugLogger { + let enabled: boolean | null = null; + return (stage, data = {}) => { + if (enabled === null) { + try { + enabled = localStorage.getItem(`hf-${name}-debug`) === "1"; + } catch { + enabled = false; + } + } + if (!enabled) return; + const details = typeof data === "function" ? data() : data; + console.log( + `[hf-${name}] ${JSON.stringify({ stage, t: Math.round(performance.now()), ...details })}`, + ); + }; +} From 27275bacb40c6f2ac66dda7d66836b107428dc7f Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 20:16:49 -0700 Subject: [PATCH 04/26] fix(studio): keep every element a marquee caught, not just the first The marquee built the group correctly and then threw it away. It announced only the primary to the timeline, and the timeline is the source of truth for what is selected: the sync back to the canvas saw one selected id against a group of several, decided the canvas was stale, and replaced the group with that single element a moment after the drop. Drag a box around four things, get one. The whole set is announced now, and the primary goes in as its anchor rather than as a new single selection, so the set it just joined survives. This is the same reason the single-select path already anchors with preserveSet. A test drives applyMarqueeSelection with two elements and asserts both reach the timeline; it fails against the old single-id announce. --- packages/studio/src/App.tsx | 2 + .../src/hooks/useDomEditSession.test.tsx | 2 + .../studio/src/hooks/useDomEditSession.ts | 3 + .../studio/src/hooks/useDomSelection.test.ts | 74 ++++++++++++++++++- packages/studio/src/hooks/useDomSelection.ts | 25 +++++-- 5 files changed, 97 insertions(+), 9 deletions(-) diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 520b4e8554..3da8119b9a 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -91,6 +91,7 @@ export function StudioApp() { const captionSync = useCaptionSync(projectId); const timelineElements = usePlayerStore((s) => s.elements); const setSelectedTimelineElementId = usePlayerStore((s) => s.setSelectedElementId); + const setTimelineSelectionSet = usePlayerStore((s) => s.setSelectedElementIds); const timelineDuration = usePlayerStore((s) => s.duration); const isPlaying = usePlayerStore((s) => s.isPlaying); const isMasterView = !activeCompPath || activeCompPath === "index.html"; @@ -277,6 +278,7 @@ export function StudioApp() { previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed: panelLayout.setRightCollapsed, setRightPanelTab: panelLayout.setRightPanelTab, showToast, diff --git a/packages/studio/src/hooks/useDomEditSession.test.tsx b/packages/studio/src/hooks/useDomEditSession.test.tsx index 3c5a3e0ffd..299ea28899 100644 --- a/packages/studio/src/hooks/useDomEditSession.test.tsx +++ b/packages/studio/src/hooks/useDomEditSession.test.tsx @@ -230,6 +230,7 @@ describe("onReorderShadow source filter", () => { previewIframeRef: { current: null }, timelineElements: [], setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), showToast: vi.fn(), @@ -328,6 +329,7 @@ describe("bulk segment ease commits", () => { previewIframeRef: { current: null }, timelineElements: [], setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), showToast: vi.fn(), diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index efec011f95..4703a01cf3 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -38,6 +38,7 @@ export interface UseDomEditSessionParams { previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet: (ids: Set) => void; setRightCollapsed: (collapsed: boolean) => void; setRightPanelTab: (tab: RightPanelTab) => void; showToast: (message: string, tone?: "error" | "info") => void; @@ -80,6 +81,7 @@ export function useDomEditSession({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, showToast, @@ -137,6 +139,7 @@ export function useDomEditSession({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 53f4524644..48ca2799a8 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -5,6 +5,7 @@ import { createRoot } from "react-dom/client"; import { describe, expect, it, vi } from "vitest"; import { installReactActEnvironment, makeSelection } from "./domSelectionTestHarness"; import { useDomSelection } from "./useDomSelection"; +import type { TimelineElement } from "../player"; installReactActEnvironment(); @@ -14,11 +15,24 @@ interface HarnessProps { refreshKey: number; } -function renderHarness(initialProps: HarnessProps): { +interface TimelineSpies { + setSelectedTimelineElementId: ReturnType; + setTimelineSelectionSet: ReturnType; +} + +function renderHarness( + initialProps: HarnessProps, + options: { timelineElements?: TimelineElement[] } = {}, +): { current: () => ReturnType; rerender: (props: HarnessProps) => void; cleanup: () => void; + timeline: TimelineSpies; } { + const timeline: TimelineSpies = { + setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), + }; const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); @@ -32,8 +46,9 @@ function renderHarness(initialProps: HarnessProps): { compIdToSrc: new Map(), captionEditMode: false, previewIframeRef: { current: null }, - timelineElements: [], - setSelectedTimelineElementId: vi.fn(), + timelineElements: options.timelineElements ?? [], + setSelectedTimelineElementId: timeline.setSelectedTimelineElementId, + setTimelineSelectionSet: timeline.setTimelineSelectionSet, setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), previewIframe: null, @@ -61,6 +76,7 @@ function renderHarness(initialProps: HarnessProps): { act(() => root.unmount()); host.remove(); }, + timeline, }; } @@ -77,6 +93,58 @@ function setupSelectedHarness() { return { selection, harness }; } +function timelineElement(domId: string): TimelineElement { + return { + id: domId, + key: domId, + domId, + tag: "div", + start: 0, + duration: 1, + track: 0, + sourceFile: "index.html", + } as TimelineElement; +} + +/** + * A marquee builds the group correctly and then used to lose it: it announced only + * the primary to the timeline, the timeline is the source of truth for what is + * selected, and the sync back to the canvas replaced the group with that one + * element a moment after the drop. The whole set has to be announced, with the + * primary as its anchor rather than as a new single selection. + */ +describe("useDomSelection marquee", () => { + it("announces every marquee'd element to the timeline, anchored on the primary", () => { + const first = document.createElement("div"); + first.id = "card"; + const second = document.createElement("div"); + second.id = "chip"; + document.body.append(first, second); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card"), timelineElement("chip")] }, + ); + + act(() => + harness + .current() + .applyMarqueeSelection( + [makeSelection("Card", first), makeSelection("Chip", second)], + false, + ), + ); + + expect(harness.current().domEditGroupSelections).toHaveLength(2); + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenCalledWith( + new Set(["card", "chip"]), + ); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenCalledWith("card", { + preserveSet: true, + }); + harness.cleanup(); + }); +}); + describe("useDomSelection", () => { it("clears a committed selection when the active composition path changes", () => { const { selection, harness } = setupSelectedHarness(); diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 76431558e1..0766cc187e 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -49,6 +49,8 @@ export interface UseDomSelectionParams { previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + /** Publishes a whole multi-selection to the timeline; the anchor is set separately. */ + setTimelineSelectionSet: (ids: Set) => void; setRightCollapsed: (collapsed: boolean) => void; setRightPanelTab: (tab: RightPanelTab) => void; previewIframe: HTMLIFrameElement | null; @@ -111,6 +113,7 @@ export function useDomSelection({ previewIframeRef, timelineElements, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, @@ -535,16 +538,26 @@ export function useDomSelection({ domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - const nextTimelineId = - findMatchingTimelineElementId(nextSelection, timelineElements) ?? + // Publish the WHOLE set, not just the primary. The timeline is the source of + // truth for what is selected, and the sync back to the canvas replaces the + // canvas selection with whatever the timeline holds. Announcing one member + // therefore un-selects every other one the marquee just caught, a moment + // after the drop — the group is built correctly and then collapses to one. + // The anchor goes second, and preserves the set it is now a member of. + const timelineIdFor = (selection: DomEditSelection) => + findMatchingTimelineElementId(selection, timelineElements) ?? findTimelineIdByAncestor( - nextSelection.element, + selection.element, timelineElements, - nextSelection.sourceFile || "index.html", + selection.sourceFile || "index.html", ); - setSelectedTimelineElementId(nextTimelineId); + const nextTimelineId = timelineIdFor(nextSelection); + setTimelineSelectionSet( + new Set(nextGroup.map(timelineIdFor).filter((id): id is string => Boolean(id))), + ); + setSelectedTimelineElementId(nextTimelineId, { preserveSet: true }); }, - [applyDomSelection, timelineElements, setSelectedTimelineElementId], + [applyDomSelection, timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet], ); return { From bccc22245d13c3c6711ac8c40872aee86457b420 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 20:42:28 -0700 Subject: [PATCH 05/26] fix(studio): stop a group selection from erasing itself on the timeline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every canvas selection is mirrored onto the timeline, and the timeline syncs back — whatever it holds replaces the canvas selection a moment later. The mirror announced only the primary and anchored it with preserveSet, but preserving a set that does not contain the id empties the set, and an empty set syncs back as "nothing is selected". Adding a second element, or re-resolving a group after moving it, could therefore drop the whole selection rather than keep it. One helper now owns the mirror: publish the members, then anchor. A single selection keeps the previous contract deliberately, so a late async primary still cannot collapse a live group and a fresh click still collapses a stale one. The group re-resolve path also gains the ancestor id fallback the other callers already had — without it a member with no direct timeline row resolved to null and deselected everything. Two tests: a second element joining a selection, and a marquee, both assert the full set reaches the timeline. Both fail against the announce-the-primary-only version. --- .../studio/src/hooks/useDomSelection.test.ts | 34 +++++++ packages/studio/src/hooks/useDomSelection.ts | 95 ++++++++++--------- .../useDomSelectionSelectionGuards.test.ts | 3 + .../hooks/useTimelineSelectionPreviewSync.ts | 6 ++ 4 files changed, 93 insertions(+), 45 deletions(-) diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 48ca2799a8..02297938ec 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -145,6 +145,40 @@ describe("useDomSelection marquee", () => { }); }); +/** + * Adding a second element announced only that element, with preserveSet — and + * preserving a set that does not contain the id empties it. An empty timeline + * selection syncs back as "nothing is selected", so growing a group could wipe + * it instead, and so could re-resolving one after a move. + */ +describe("useDomSelection additive", () => { + it("announces both members when a second element joins the selection", () => { + const first = document.createElement("div"); + first.id = "card"; + const second = document.createElement("div"); + second.id = "chip"; + document.body.append(first, second); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card"), timelineElement("chip")] }, + ); + + act(() => harness.current().applyDomSelection(makeSelection("Card", first))); + act(() => + harness.current().applyDomSelection(makeSelection("Chip", second), { additive: true }), + ); + + expect(harness.current().domEditGroupSelections).toHaveLength(2); + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenLastCalledWith( + new Set(["card", "chip"]), + ); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenLastCalledWith("chip", { + preserveSet: true, + }); + harness.cleanup(); + }); +}); + describe("useDomSelection", () => { it("clears a committed selection when the active composition path changes", () => { const { selection, harness } = setupSelectedHarness(); diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 0766cc187e..24316012e8 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -149,6 +149,47 @@ export function useDomSelection({ // ── Callbacks ── + /** + * Mirror a canvas selection onto the timeline: the whole set first, then the + * primary as its anchor. + * + * The timeline is the source of truth for what is selected and it syncs back — + * whatever it holds replaces the canvas selection a moment later. Announcing only + * the primary therefore drops every other member. Worse, anchoring with + * `preserveSet` on an id the set does not yet contain empties the set outright, + * and an empty set syncs back as "nothing is selected" — which is how adding a + * second element, or moving a group, could wipe the selection instead of keeping + * it. Publishing the members first is what makes the anchor a member, so + * preserving the set is meaningful rather than destructive. + */ + const announceTimelineSelection = useCallback( + (group: DomEditSelection[], primary: DomEditSelection | null) => { + if (!primary) { + setTimelineSelectionSet(new Set()); + setSelectedTimelineElementId(null); + return; + } + const timelineIdFor = (selection: DomEditSelection) => + findMatchingTimelineElementId(selection, timelineElements) ?? + findTimelineIdByAncestor( + selection.element, + timelineElements, + selection.sourceFile || "index.html", + ); + // Only a real multi-selection publishes members. A single selection keeps the + // older contract on purpose: anchoring with preserveSet holds a live set the + // element already belongs to (a late async primary must not collapse a group) + // and collapses otherwise, which is what a fresh click means. + if (group.length > 1) { + setTimelineSelectionSet( + new Set(group.map(timelineIdFor).filter((id): id is string => Boolean(id))), + ); + } + setSelectedTimelineElementId(timelineIdFor(primary), { preserveSet: true }); + }, + [setSelectedTimelineElementId, setTimelineSelectionSet, timelineElements], + ); + const applyDomSelection = useCallback( // fallow-ignore-next-line complexity ( @@ -160,11 +201,12 @@ export function useDomSelection({ }, ) => { if (!selection) { + logSelect("clear", { hadGroup: domEditGroupSelectionsRef.current.length }); domEditSelectionRef.current = null; domEditGroupSelectionsRef.current = []; setDomEditSelection(null); setDomEditGroupSelections([]); - setSelectedTimelineElementId(null); + announceTimelineSelection([], null); return; } @@ -219,21 +261,13 @@ export function useDomSelection({ setRightPanelTab("design"); } } - const nextSelectedTimelineId = - findMatchingTimelineElementId(nextSelection, timelineElements) ?? - findTimelineIdByAncestor( - nextSelection.element, - timelineElements, - nextSelection.sourceFile || "index.html", - ); - // Late marquee notify: a primary already in the live set must not collapse it. - setSelectedTimelineElementId(nextSelectedTimelineId, { preserveSet: true }); + announceTimelineSelection(nextGroup, nextSelection); return; } - setSelectedTimelineElementId(null); + announceTimelineSelection([], null); }, - [setSelectedTimelineElementId, timelineElements, setRightCollapsed, setRightPanelTab], + [announceTimelineSelection, setRightCollapsed, setRightPanelTab], ); const clearDomSelection = useCallback(() => { @@ -447,21 +481,9 @@ export function useDomSelection({ setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - if (nextSelection) { - setSelectedTimelineElementId( - findMatchingTimelineElementId(nextSelection, timelineElements), - ); - } else { - setSelectedTimelineElementId(null); - } + announceTimelineSelection(nextGroup, nextSelection); }, - [ - activeCompPath, - buildDomSelectionFromTarget, - setSelectedTimelineElementId, - timelineElements, - previewIframeRef, - ], + [activeCompPath, announceTimelineSelection, buildDomSelectionFromTarget, previewIframeRef], ); // ── Effects ── @@ -538,26 +560,9 @@ export function useDomSelection({ domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - // Publish the WHOLE set, not just the primary. The timeline is the source of - // truth for what is selected, and the sync back to the canvas replaces the - // canvas selection with whatever the timeline holds. Announcing one member - // therefore un-selects every other one the marquee just caught, a moment - // after the drop — the group is built correctly and then collapses to one. - // The anchor goes second, and preserves the set it is now a member of. - const timelineIdFor = (selection: DomEditSelection) => - findMatchingTimelineElementId(selection, timelineElements) ?? - findTimelineIdByAncestor( - selection.element, - timelineElements, - selection.sourceFile || "index.html", - ); - const nextTimelineId = timelineIdFor(nextSelection); - setTimelineSelectionSet( - new Set(nextGroup.map(timelineIdFor).filter((id): id is string => Boolean(id))), - ); - setSelectedTimelineElementId(nextTimelineId, { preserveSet: true }); + announceTimelineSelection(nextGroup, nextSelection); }, - [applyDomSelection, timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet], + [applyDomSelection, announceTimelineSelection], ); return { diff --git a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts index dce0655946..dfed3d834c 100644 --- a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts +++ b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts @@ -49,6 +49,7 @@ interface HarnessProps { iframe: HTMLIFrameElement | null; timelineElements: TimelineElement[]; setSelectedTimelineElementId?: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet?: (ids: Set) => void; } function renderHarness(props: HarnessProps) { @@ -67,6 +68,8 @@ function renderHarness(props: HarnessProps) { previewIframeRef: { current: props.iframe }, timelineElements: props.timelineElements, setSelectedTimelineElementId: props.setSelectedTimelineElementId ?? vi.fn(), + setTimelineSelectionSet: + props.setTimelineSelectionSet ?? usePlayerStore.getState().setSelectedElementIds, setRightCollapsed: vi.fn(), setRightPanelTab: props.setRightPanelTab, previewIframe: props.iframe, diff --git a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts index 656568cb93..91fbfe377f 100644 --- a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts +++ b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts @@ -2,6 +2,7 @@ import { useEffect, useMemo, useRef } from "react"; import type { TimelineElement } from "../player"; import type { DomEditSelection } from "../components/editor/domEditing"; import { resolveTimelineIdForSelection } from "../utils/studioHelpers"; +import { logSelect } from "../utils/selectDebug"; interface UseTimelineSelectionPreviewSyncParams { selectedElementId: string | null; @@ -127,6 +128,11 @@ export function useTimelineSelectionPreviewSync({ return; } missingSelectionKeyRef.current = ""; + logSelect("timeline-sync", { + wanted: selectedIds.length, + had: currentIds.length, + resolved: selections.length, + }); if (selections.length === 0) { applyDomSelection(null, { revealPanel: false }); } else if (selections.length === 1) { From fbc892c071836f1a0f5d89221884d80a7d181ce6 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 21:03:25 -0700 Subject: [PATCH 06/26] chore(studio): trace what moves a dragged group and when MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A drag that jumps is a position that changed without the pointer asking for it, and nothing on that path says anything today, so the frame it diverges can only be guessed at. `hf-drag-debug` (localStorage, off by default) records the whole gesture: the mapping and start position each member got, the pointer delta against the delta actually applied on every eighth move, what each member was told to commit, and where they all sit at the drop, once the commit resolves, and 120/400/900ms later. That last group is the point of it. The source write, the preview reload and the timeline resume all land within a few frames of the drop, and any of them can put the elements back where they started before the new position arrives — a snap-back shows up as a settle sample reverting to the gesture-start reading. A gap between `pointer` and `applied` instead means snapping pulled the group off the cursor, which is a different fault with a different fix. --- .../editor/domEditOverlayStartGesture.ts | 17 ++++++ .../editor/manualOffsetDrag.test.ts | 50 ++++++++++++++++++ .../editor/useDomEditOverlayGestures.ts | 35 ++++++++++++- packages/studio/src/utils/dragDebug.ts | 52 +++++++++++++++++++ 4 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 packages/studio/src/utils/dragDebug.ts diff --git a/packages/studio/src/components/editor/domEditOverlayStartGesture.ts b/packages/studio/src/components/editor/domEditOverlayStartGesture.ts index 6576a9b5ba..390e05e66e 100644 --- a/packages/studio/src/components/editor/domEditOverlayStartGesture.ts +++ b/packages/studio/src/components/editor/domEditOverlayStartGesture.ts @@ -33,6 +33,7 @@ import { } from "./domEditOverlayGestures"; import { collectSnapContext, buildExcludeElements } from "./snapTargetCollection"; import { logResize, resetResizeMoveLog } from "../../utils/resizeDebug"; +import { logDrag, readDragPositions, resetDragMoveLog } from "../../utils/dragDebug"; export function startGroupDrag( e: React.PointerEvent, @@ -70,6 +71,22 @@ export function startGroupDrag( } members.push(result.member); } + resetDragMoveLog(); + logDrag("group-start", { + // A member whose mapping differs from its neighbours travels a different + // distance for the same pointer delta, which is the group coming apart. + members: Object.fromEntries( + members.map((member) => [ + member.key, + { + map: `${member.screenToOffset.a.toFixed(3)},${member.screenToOffset.d.toFixed(3)}`, + base: `${Math.round(member.baseGsap.x)},${Math.round(member.baseGsap.y)}`, + offset: `${Math.round(member.initialOffset.x)},${Math.round(member.initialOffset.y)}`, + }, + ]), + ), + at: readDragPositions(members), + }); const overlayEl = opts.overlayRef.current; const iframe = opts.iframeRef.current; diff --git a/packages/studio/src/components/editor/manualOffsetDrag.test.ts b/packages/studio/src/components/editor/manualOffsetDrag.test.ts index 9aeb6b2d4d..0db15b1149 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.test.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.test.ts @@ -197,6 +197,56 @@ describe("measureManualOffsetDragScreenToOffsetMatrix", () => { }); }); +/** + * A group drag is rigid: every member is handed the SAME pointer delta and must + * travel the same distance on screen, or the group visibly comes apart mid-drag. + * Members do not share a mapping though — each measures its own, because each can + * sit under different ancestor transforms. A member whose movement cannot be + * measured falls back to a guess, and this pins what that guess costs the group. + */ +describe("group drag stays rigid", () => { + function member(key: string, response: number, measurable: boolean) { + const window = new Window(); + const element = window.document.createElement("div"); + window.document.body.append(element); + element.getBoundingClientRect = () => { + const ox = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_X_PROP)) || 0; + const oy = Number.parseFloat(element.style.getPropertyValue(STUDIO_OFFSET_Y_PROP)) || 0; + const move = measurable ? response : 0; + return new window.DOMRect(100 + move * ox, 200 + move * oy, 40, 20); + }; + const result = createManualOffsetDragMember({ + key, + selection: { element } as never, + element, + rect: { left: 100, top: 200, width: 40, height: 20, editScaleX: 1, editScaleY: 1 }, + }); + if (!result.ok) throw new Error(result.reason); + return { member: result.member, response }; + } + + /** Screen distance this member travels for a pointer delta of `d`. */ + function screenTravel(entry: ReturnType, d: number): number { + const offset = resolveManualOffsetForPointerDelta({ + initialOffset: entry.member.initialOffset, + screenToOffset: entry.member.screenToOffset, + dx: d, + dy: 0, + }); + return offset.x * entry.response; + } + + it("moves every measurable member the same distance for one pointer delta", () => { + // Two members under different ancestor scales: one 1:1, one inside a half-scale + // parent. Different offsets, identical screen travel — that is what rigid means. + const a = member("a", 1, true); + const b = member("b", 0.5, true); + + expect(screenTravel(a, 60)).toBeCloseTo(60, 6); + expect(screenTravel(b, 60)).toBeCloseTo(60, 6); + }); +}); + describe("createManualOffsetDragMember uses raw CSS var offset", () => { it("ignores GSAP transform — initialOffset comes from CSS vars only", () => { const window = new Window(); diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index 7af8df2e7b..a6e55b10ae 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -52,6 +52,7 @@ import { import { hugRectForElement } from "./domEditOverlayCrop"; import { resolveSnapAdjustment, resolveEquidistanceGuides, SNAP_THRESHOLD_PX } from "./snapEngine"; import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; +import { logDrag, logDragMove, logDragSettle, readDragPositions } from "../../utils/dragDebug"; export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -162,7 +163,18 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu rect: { ...item.rect, left: item.rect.left + dx, top: item.rect.top + dy }, })), ); - for (const member of groupG.members) applyManualOffsetDragDraft(member, dx, dy); + const offsets: Record = {}; + for (const member of groupG.members) { + const next = applyManualOffsetDragDraft(member, dx, dy); + offsets[member.key] = `${Math.round(next.x)},${Math.round(next.y)}`; + } + logDragMove({ + pointer: `${Math.round(e.clientX - groupG.startX)},${Math.round(e.clientY - groupG.startY)}`, + // Any gap between these two is snapping pulling the group off the pointer. + applied: `${Math.round(dx)},${Math.round(dy)}`, + offsets, + at: readDragPositions(groupG.members), + }); return; } @@ -336,6 +348,17 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu selection: member.selection, next: applyManualOffsetDragCommit(member, dx, dy), })); + logDrag("drop", { + pointer: `${Math.round(rawDx)},${Math.round(rawDy)}`, + applied: `${Math.round(dx)},${Math.round(dy)}`, + committed: Object.fromEntries( + updates.map((update, index) => [ + groupG.members[index]?.key ?? String(index), + `${Math.round(update.next.x)},${Math.round(update.next.y)}`, + ]), + ), + at: readDragPositions(groupG.members), + }); void Promise.resolve(opts.onGroupPathOffsetCommitRef.current(updates)) .catch(() => { for (const member of groupG.members) { @@ -346,7 +369,15 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu restoreStudioPathOffset(member.element, member.initialPathOffset); } }) - .finally(() => endManualOffsetDragMembers(groupG.members)); + .finally(() => { + logDrag("committed", { at: readDragPositions(groupG.members) }); + endManualOffsetDragMembers(groupG.members); + // The gesture teardown resumes the paused timelines and re-seeks the + // player, which re-renders from whatever the preview currently holds. + // If the reloaded source has not landed yet that is the OLD position, + // so this is where a snap-back would show. + logDragSettle("settle", groupG.members); + }); return; } diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts new file mode 100644 index 0000000000..a0073fb50a --- /dev/null +++ b/packages/studio/src/utils/dragDebug.ts @@ -0,0 +1,52 @@ +// Canvas drag diagnostics — grep [hf-drag]. Off by default; opt in per session +// with `localStorage.setItem("hf-drag-debug", "1")` (then reload). +// +// A drag that "jumps" is a position that changed without the pointer asking. The +// pointer delta, what snapping did to it, what each member was told to move, and +// where each member actually ended up are logged at every stage, so the frame the +// position diverges from the pointer is visible rather than inferred. +import { makeStudioDebugLogger } from "./studioDebug"; + +export const logDrag = makeStudioDebugLogger("drag"); + +let moveN = 0; + +/** Per-pointermove logging, throttled: the first move then every 8th. */ +export function logDragMove(data: Record): void { + moveN += 1; + if (moveN % 8 === 1) logDrag("move", { n: moveN, ...data }); +} + +export function resetDragMoveLog(): void { + moveN = 0; +} + +/** Where these elements are rendered right now, in preview-document pixels. */ +export function readDragPositions( + elements: Array<{ key: string; element: HTMLElement }>, +): Record { + const positions: Record = {}; + for (const { key, element } of elements) { + const rect = element.getBoundingClientRect(); + positions[key] = `${Math.round(rect.left)},${Math.round(rect.top)}`; + } + return positions; +} + +/** + * Sample the group now and again after the commit has had time to land. The drop + * is the one moment a jump can hide: the source write, the preview reload and the + * timeline resume all happen within a few frames of each other, and any of them + * can put the elements back where they started before the new position arrives. + */ +export function logDragSettle( + stage: string, + elements: Array<{ key: string; element: HTMLElement }>, +): void { + logDrag(stage, { at: readDragPositions(elements) }); + const win = elements[0]?.element.ownerDocument.defaultView; + if (!win) return; + win.setTimeout(() => logDrag(`${stage}+120ms`, { at: readDragPositions(elements) }), 120); + win.setTimeout(() => logDrag(`${stage}+400ms`, { at: readDragPositions(elements) }), 400); + win.setTimeout(() => logDrag(`${stage}+900ms`, { at: readDragPositions(elements) }), 900); +} From e6cbb229e95782286bc987e581f5ceadb9c590d3 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 21:19:54 -0700 Subject: [PATCH 07/26] chore(studio): name the path that clears a selection after a group move MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drag trace showed the group landing exactly where it was dropped and staying there — no snap-back at any settle sample, and the pointer and the applied delta never more than 2px apart — but two milliseconds after the drop the selection was cleared with seven members still in it. The clear comes from the timeline sync deciding the timeline holds nothing, and that branch said nothing. It says so now, along with whether it is about to act on it. The mirror alongside it reports how many members it managed to publish and whether the anchor was among them, because a member with no timeline row of its own resolves to null and is dropped silently — publish none and the sync reads it back as an empty selection. --- packages/studio/src/hooks/useDomSelection.ts | 20 +++++++++++++------ .../hooks/useTimelineSelectionPreviewSync.ts | 7 +++++++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 24316012e8..ba5d01c0c1 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -180,12 +180,19 @@ export function useDomSelection({ // older contract on purpose: anchoring with preserveSet holds a live set the // element already belongs to (a late async primary must not collapse a group) // and collapses otherwise, which is what a fresh click means. - if (group.length > 1) { - setTimelineSelectionSet( - new Set(group.map(timelineIdFor).filter((id): id is string => Boolean(id))), - ); - } - setSelectedTimelineElementId(timelineIdFor(primary), { preserveSet: true }); + const members = group.map(timelineIdFor).filter((id): id is string => Boolean(id)); + const anchor = timelineIdFor(primary); + // A member with no timeline row of its own resolves to null and is dropped + // here, so a group can announce fewer ids than it has — or none, which reads + // back as an empty selection and takes the canvas selection with it. + logSelect("announce", { + group: group.length, + published: members.length, + anchor, + anchorPublished: anchor != null && members.includes(anchor), + }); + if (group.length > 1) setTimelineSelectionSet(new Set(members)); + setSelectedTimelineElementId(anchor, { preserveSet: true }); }, [setSelectedTimelineElementId, setTimelineSelectionSet, timelineElements], ); @@ -536,6 +543,7 @@ export function useDomSelection({ const applyMarqueeSelection = useCallback( // fallow-ignore-next-line complexity (selections: DomEditSelection[], additive: boolean) => { + logSelect("marquee", { hits: selections.length, additive }); if (selections.length === 0) { if (!additive) applyDomSelection(null, { revealPanel: false }); return; diff --git a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts index 91fbfe377f..f57ebcd2e6 100644 --- a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts +++ b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts @@ -94,6 +94,13 @@ export function useTimelineSelectionPreviewSync({ if (selectedIds.length === 0) { missingSelectionKeyRef.current = ""; + // The timeline holds nothing, so the canvas is about to hold nothing either. + // This is the path that silently drops a selection the user can still see. + logSelect("timeline-empty", { + had: currentIds.length, + previousKey: previousSelectedKey.length > 0, + clearing: previousSelectedKey.length > 0 && currentIds.length > 0, + }); if (previousSelectedKey.length > 0 && currentIds.length > 0) { applyDomSelection(null, { revealPanel: false }); } From 23dcdd88dc0a3803abca5b4c40db78bba554e3c0 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 21:29:24 -0700 Subject: [PATCH 08/26] fix(studio): losing one member of a group no longer deselects all of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a move the preview re-syncs and the selection is re-resolved against the new document. When the primary could not be found there, both re-resolve paths cleared the entire selection — so a group of five, all still on screen, was deselected because one of them failed to resolve. The trace showed the clear landing 600ms after the drop with five members still held, and the timeline sync running afterwards on an already-empty canvas, which ruled it out as the cause. A live group now re-resolves as a group and keeps whoever survived, picking a new primary from them; it only clears when nobody did. That is what refreshDomEditGroupSelectionsFromPreview was written for — it existed and was never called. Both clears also say which one they are and how many members were held, so if this is not the last of it the next trace names the path immediately. --- .../studio/src/hooks/useDomEditPreviewSync.ts | 23 +++++++++++++++++++ .../studio/src/hooks/useDomEditSession.ts | 3 +++ packages/studio/src/hooks/useDomEditWiring.ts | 6 +++++ packages/studio/src/hooks/useDomSelection.ts | 20 ++++++++++++++++ 4 files changed, 52 insertions(+) diff --git a/packages/studio/src/hooks/useDomEditPreviewSync.ts b/packages/studio/src/hooks/useDomEditPreviewSync.ts index 6a288b6ee6..cb0c334fe1 100644 --- a/packages/studio/src/hooks/useDomEditPreviewSync.ts +++ b/packages/studio/src/hooks/useDomEditPreviewSync.ts @@ -8,13 +8,17 @@ import { findElementForSelection, type DomEditSelection } from "../components/ed import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import type { SidebarTab } from "../components/sidebar/LeftSidebar"; import type { PatchTarget } from "../utils/sourcePatcher"; +import { logSelect } from "../utils/selectDebug"; interface UseDomEditPreviewSyncParams { previewIframe: HTMLIFrameElement | null; activeCompPath: string | null; captionEditMode: boolean; domEditSelectionRef: React.MutableRefObject; + domEditGroupSelectionsRef: React.MutableRefObject; domEditSelection: DomEditSelection | null; + /** Re-resolves a whole multi-selection against the current preview document. */ + refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise; applyDomSelection: ( selection: DomEditSelection | null, options?: { revealPanel?: boolean; preserveGroup?: boolean }, @@ -35,8 +39,10 @@ export function useDomEditPreviewSync({ activeCompPath, captionEditMode, domEditSelectionRef, + domEditGroupSelectionsRef, domEditSelection, applyDomSelection, + refreshDomEditGroupSelectionsFromPreview, buildDomSelectionFromTarget, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, @@ -72,6 +78,21 @@ export function useDomEditPreviewSync({ // Clear so overlay geometry isn't computed on a stale, detached node. // (Drag-release-in-gray-zone is handled separately by // suppressNextBoxClickRef; the dragged element still resolves here.) + // + // One lost member is not the whole selection though. A multi-select that + // loses its primary here used to be wiped entirely, so moving a group and + // having any one of its elements fail to re-resolve deselected all of + // them. Re-resolve the group instead and keep whoever survived; it only + // clears when nobody did. + const group = domEditGroupSelectionsRef.current; + logSelect("preview-sync-lost", { + target: currentSelection.selector ?? currentSelection.id ?? null, + group: group.length, + }); + if (group.length > 1) { + await refreshDomEditGroupSelectionsFromPreview(group); + return; + } applyDomSelection(null, { revealPanel: false }); return; } @@ -103,8 +124,10 @@ export function useDomEditPreviewSync({ applyDomSelection, buildDomSelectionFromTarget, captionEditMode, + domEditGroupSelectionsRef, domEditSelectionRef, previewIframe, + refreshDomEditGroupSelectionsFromPreview, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, applyStudioManualEditsToPreviewRef, diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index 4703a01cf3..5db396307d 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -129,6 +129,7 @@ export function useDomEditSession({ buildDomSelectionForTimelineElement, handleTimelineElementSelect, refreshDomEditSelectionFromPreview, + refreshDomEditGroupSelectionsFromPreview, applyMarqueeSelection, } = useDomSelection({ projectId, @@ -385,6 +386,8 @@ export function useDomEditSession({ activeCompPath, domEditSelection, domEditSelectionRef, + domEditGroupSelectionsRef, + refreshDomEditGroupSelectionsFromPreview, previewIframeRef, previewIframe, captionEditMode, diff --git a/packages/studio/src/hooks/useDomEditWiring.ts b/packages/studio/src/hooks/useDomEditWiring.ts index fbd049482e..9b49f16fa7 100644 --- a/packages/studio/src/hooks/useDomEditWiring.ts +++ b/packages/studio/src/hooks/useDomEditWiring.ts @@ -23,6 +23,8 @@ export interface UseDomEditWiringParams { activeCompPath: string | null; domEditSelection: DomEditSelection | null; domEditSelectionRef: React.MutableRefObject; + domEditGroupSelectionsRef: React.MutableRefObject; + refreshDomEditGroupSelectionsFromPreview: (selections: DomEditSelection[]) => Promise; previewIframeRef: React.RefObject; previewIframe: HTMLIFrameElement | null; captionEditMode: boolean; @@ -115,6 +117,8 @@ export function useDomEditWiring({ activeCompPath, domEditSelection, domEditSelectionRef, + domEditGroupSelectionsRef, + refreshDomEditGroupSelectionsFromPreview, previewIframeRef, previewIframe, captionEditMode, @@ -254,8 +258,10 @@ export function useDomEditWiring({ activeCompPath, captionEditMode, domEditSelectionRef, + domEditGroupSelectionsRef, domEditSelection, applyDomSelection, + refreshDomEditGroupSelectionsFromPreview, buildDomSelectionFromTarget, refreshPreviewDocumentVersion, syncPreviewHistoryHotkey, diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index ba5d01c0c1..dbe7d4b833 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -427,6 +427,13 @@ export function useDomSelection({ [applyDomSelection, buildDomSelectionForTimelineElement], ); + // Forward handle to the group refresher defined below: the single-selection + // refresher falls back to it when the primary is gone, and a ref keeps that from + // forcing either callback to be declared in the other's dependency list. + const refreshDomEditGroupSelectionsFromPreviewRef = useRef< + (selections: DomEditSelection[]) => Promise + >(async () => {}); + const refreshDomEditSelectionFromPreview = useCallback( // fallow-ignore-next-line complexity async (selection: DomEditSelection) => { @@ -441,6 +448,17 @@ export function useDomSelection({ const element = findElementForSelection(doc, selection, activeCompPath); if (!element) { + // Losing the primary is not losing the selection. When a group is live, + // re-resolve it and keep whoever still exists rather than wiping the lot. + const group = domEditGroupSelectionsRef.current; + logSelect("refresh-lost", { + target: selection.selector ?? selection.id ?? null, + group: group.length, + }); + if (group.length > 1) { + await refreshDomEditGroupSelectionsFromPreviewRef.current(group); + return; + } applyDomSelection(null, { revealPanel: false }); return; } @@ -493,6 +511,8 @@ export function useDomSelection({ [activeCompPath, announceTimelineSelection, buildDomSelectionFromTarget, previewIframeRef], ); + refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; + // ── Effects ── // Clear hover unconditionally on composition/project/preview change From d939b7dd2cf76254fa3ebb50957f1a82d73971f2 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 13:34:45 -0700 Subject: [PATCH 09/26] feat(studio): carry a multi-selection in the URL, and name the member that breaks away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A link to a bug hit with several elements selected only reproduced one of them, so the report read as "works for me". The hash now carries the rest as selGroup and reopens the whole selection; members whose element is gone are dropped rather than failing the others. Verified end to end in a real browser: select three, copy the hash, open it fresh, the same three come back. The drag trace also gains a rigidity check. A group moves as one object, so every member travels the same distance; one that does not IS the fault. Drift was being computed but only printed on every eighth frame, which is exactly how a single-frame divergence hides — it now prints on the frame it happens. The frame handler moves to its own module on the way past. It had grown a snap block and a trace block inside a function already juggling four gesture kinds, and it was over both the complexity and file-size gates. Not fixed: the jump itself. Two headful runs driving a real group drag showed the members staying rigid to the pixel, at the drop and 900ms after, so I have not reproduced it yet and will not guess at a fix. --- packages/studio/src/App.tsx | 2 + .../src/components/editor/groupDragMove.ts | 96 +++++++++++++++++++ .../src/components/editor/manualEditsDom.ts | 2 + .../src/components/editor/manualOffsetDrag.ts | 2 + .../editor/useDomEditOverlayGestures.ts | 67 +------------ .../studio/src/hooks/useStudioUrlState.ts | 49 ++++++++-- packages/studio/src/utils/dragDebug.ts | 31 ++++++ .../studio/src/utils/studioUrlState.test.ts | 31 ++++++ packages/studio/src/utils/studioUrlState.ts | 23 ++++- 9 files changed, 232 insertions(+), 71 deletions(-) create mode 100644 packages/studio/src/components/editor/groupDragMove.ts diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 3da8119b9a..538aebf9ec 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -418,6 +418,8 @@ export function StudioApp() { rightCollapsed: panelLayout.rightCollapsed, activeCompPathHydrated, domEditSelection: domEditSession.domEditSelection, + domEditGroupSelections: domEditSession.domEditGroupSelections, + applyMarqueeSelection: domEditSession.applyMarqueeSelection, buildDomSelectionFromTarget: domEditSession.buildDomSelectionFromTarget, applyDomSelection: domEditSession.applyDomSelection, setRightPanelTab: panelLayout.setRightPanelTab, diff --git a/packages/studio/src/components/editor/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts new file mode 100644 index 0000000000..2202d7c4e0 --- /dev/null +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -0,0 +1,96 @@ +import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry"; +import { resolveEquidistanceGuides, resolveSnapAdjustment, SNAP_THRESHOLD_PX } from "./snapEngine"; +import { applyManualOffsetDragDraft } from "./manualOffsetDrag"; +import type { GroupGestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures"; +import type { GroupOverlayItem } from "./domEditOverlayGeometry"; +import { + findNonRigidMembers, + logDrag, + logDragMove, + readDragPositions, +} from "../../utils/dragDebug"; + +/** + * One frame of a group drag, kept out of onPointerMove — which already handles + * four gesture kinds and reads better without this one's snapping arithmetic. + * The previous frame's positions live in the closure so the rigidity check below + * compares against the frame before, not against whatever was last sampled. + */ +export function createGroupDragMover( + opts: UseDomEditOverlayGesturesOptions, + setDraftGroupOverlayItems: (items: GroupOverlayItem[]) => void, +) { + let lastGroupPositions: Record = {}; + + /** Snap the group's delta to nearby edges, publishing the guides drawn for it. */ + // fallow-ignore-next-line complexity + const snapGroupDelta = ( + groupG: GroupGestureState, + e: React.PointerEvent, + proposed: { dx: number; dy: number }, + ) => { + const sc = groupG.snapContext; + if (!sc?.snapEnabled || sc.targets.length === 0) return proposed; + const groupBounds = resolveDomEditGroupOverlayRect(groupG.originItems.map((i) => i.rect)); + if (!groupBounds) return proposed; + const allTargets = sc.compositionTarget ? [...sc.targets, sc.compositionTarget] : sc.targets; + const snap = resolveSnapAdjustment({ + movingRect: groupBounds, + proposedDx: proposed.dx, + proposedDy: proposed.dy, + targets: allTargets, + gridEdges: sc.gridEdges ?? undefined, + threshold: SNAP_THRESHOLD_PX, + disabled: e.altKey, + }); + const movingRect = { + ...groupBounds, + left: groupBounds.left + snap.dx, + top: groupBounds.top + snap.dy, + }; + const spacingGuides = e.altKey + ? [] + : resolveEquidistanceGuides({ + movingRect, + targets: allTargets, + threshold: SNAP_THRESHOLD_PX, + }); + opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; + return { dx: snap.dx, dy: snap.dy }; + }; + + /** One frame of a group drag: snap the delta, redraw the boxes, move every member. */ + const moveGroupDrag = (groupG: GroupGestureState, e: React.PointerEvent) => { + const { dx, dy } = snapGroupDelta(groupG, e, { + dx: e.clientX - groupG.startX, + dy: e.clientY - groupG.startY, + }); + groupG.lastSnappedDx = dx; + groupG.lastSnappedDy = dy; + + setDraftGroupOverlayItems( + groupG.originItems.map((i) => ({ + ...i, + rect: { ...i.rect, left: i.rect.left + dx, top: i.rect.top + dy }, + })), + ); + const offsets: Record = {}; + for (const m of groupG.members) { + const n = applyManualOffsetDragDraft(m, dx, dy); + offsets[m.key] = `${Math.round(n.x)},${Math.round(n.y)}`; + } + const at = readDragPositions(groupG.members); + const px = Math.round(e.clientX - groupG.startX); + const py = Math.round(e.clientY - groupG.startY); + // A member breaking away IS the fault, so it reports on the frame it happens; + // the throttled line below would step over it. A gap between pointer and + // applied there is snapping pulling the group off the cursor. + const trace = { pointer: `${px},${py}`, applied: `${Math.round(dx)},${Math.round(dy)}`, at }; + const drift = findNonRigidMembers(lastGroupPositions, at); + if (drift.length > 0) logDrag("drift", { ...trace, drift }); + lastGroupPositions = at; + logDragMove({ ...trace, offsets }); + }; + + return moveGroupDrag; +} diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 9ac4588a26..80697e4e3d 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -221,6 +221,7 @@ function isIdentityAfterTranslateStrip(m: DOMMatrix): boolean { return m.is2D && m.a === 1 && m.b === 0 && m.c === 0 && m.d === 1; } +// fallow-ignore-next-line complexity function stripGsapTranslateFromTransform(element: HTMLElement): void { if (element.hasAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR)) return; const transform = element.style.getPropertyValue("transform"); @@ -256,6 +257,7 @@ function stripGsapTranslateFromTransform(element: HTMLElement): void { // and push the offset straight into GSAP's x/y via gsap.set; the var() offset is // still persisted (buildPathOffsetPatches), and GSAP re-reads it at init on // reload. Returns true when handled as GSAP (caller must skip the CSS path). +// fallow-ignore-next-line complexity function applyStudioPathOffsetViaGsap( element: HTMLElement, offset: { x: number; y: number }, diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index e18b85c23b..731c41813f 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -325,6 +325,8 @@ export function resolveManualOffsetForPointerDelta(input: { }; } +// Pre-existing complexity — surfaced by this branch touching the file, not by new logic. +// fallow-ignore-next-line complexity export function createManualOffsetDragMember(input: { key: string; selection: DomEditSelection; diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index a6e55b10ae..59967d8cc0 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -30,7 +30,6 @@ import { type GroupOverlayItem, type OverlayRect, orientedOverlayRect, - resolveDomEditGroupOverlayRect, } from "./domEditOverlayGeometry"; import { BLOCKED_MOVE_THRESHOLD_PX, @@ -52,7 +51,8 @@ import { import { hugRectForElement } from "./domEditOverlayCrop"; import { resolveSnapAdjustment, resolveEquidistanceGuides, SNAP_THRESHOLD_PX } from "./snapEngine"; import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; -import { logDrag, logDragMove, logDragSettle, readDragPositions } from "../../utils/dragDebug"; +import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug"; +import { createGroupDragMover } from "./groupDragMove"; export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -92,6 +92,8 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu }, ) => _startGesture(kind, e, opts, options); + const moveGroupDrag = createGroupDragMover(opts, setDraftGroupOverlayItems); + // fallow-ignore-next-line complexity const onPointerMove = (e: React.PointerEvent) => { const g = opts.gestureRef.current; @@ -115,66 +117,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu } if (groupG) { - let dx = e.clientX - groupG.startX; - let dy = e.clientY - groupG.startY; - - const sc = groupG.snapContext; - if (sc?.snapEnabled && sc.targets.length > 0) { - const groupBounds = resolveDomEditGroupOverlayRect( - groupG.originItems.map((item) => item.rect), - ); - if (groupBounds) { - const allTargets = sc.compositionTarget - ? [...sc.targets, sc.compositionTarget] - : sc.targets; - const snap = resolveSnapAdjustment({ - movingRect: groupBounds, - proposedDx: dx, - proposedDy: dy, - targets: allTargets, - gridEdges: sc.gridEdges ?? undefined, - threshold: SNAP_THRESHOLD_PX, - disabled: e.altKey, - }); - dx = snap.dx; - dy = snap.dy; - const movedRect = { - left: groupBounds.left + dx, - top: groupBounds.top + dy, - width: groupBounds.width, - height: groupBounds.height, - }; - const spacingGuides = e.altKey - ? [] - : resolveEquidistanceGuides({ - movingRect: movedRect, - targets: allTargets, - threshold: SNAP_THRESHOLD_PX, - }); - opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; - } - } - groupG.lastSnappedDx = dx; - groupG.lastSnappedDy = dy; - - setDraftGroupOverlayItems( - groupG.originItems.map((item) => ({ - ...item, - rect: { ...item.rect, left: item.rect.left + dx, top: item.rect.top + dy }, - })), - ); - const offsets: Record = {}; - for (const member of groupG.members) { - const next = applyManualOffsetDragDraft(member, dx, dy); - offsets[member.key] = `${Math.round(next.x)},${Math.round(next.y)}`; - } - logDragMove({ - pointer: `${Math.round(e.clientX - groupG.startX)},${Math.round(e.clientY - groupG.startY)}`, - // Any gap between these two is snapping pulling the group off the pointer. - applied: `${Math.round(dx)},${Math.round(dy)}`, - offsets, - at: readDragPositions(groupG.members), - }); + moveGroupDrag(groupG, e); return; } diff --git a/packages/studio/src/hooks/useStudioUrlState.ts b/packages/studio/src/hooks/useStudioUrlState.ts index e67e6d346a..263182676c 100644 --- a/packages/studio/src/hooks/useStudioUrlState.ts +++ b/packages/studio/src/hooks/useStudioUrlState.ts @@ -22,6 +22,8 @@ interface UseStudioUrlStateParams { rightCollapsed: boolean; activeCompPathHydrated: boolean; domEditSelection: DomEditSelection | null; + domEditGroupSelections: DomEditSelection[]; + applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => void; buildDomSelectionFromTarget: ( target: HTMLElement, options?: { preferClipAncestor?: boolean }, @@ -38,14 +40,25 @@ interface UseStudioUrlStateParams { initialState: StudioUrlState; } -function toPersistedSelection(selection: DomEditSelection | null): StudioUrlSelectionState | null { +function toPersistedSelection( + selection: DomEditSelection | null, + // Optional: a caller that only ever has one selection has nothing to add, and + // the URL must still carry that one rather than throwing on the way out. + group: DomEditSelection[] = [], +): StudioUrlSelectionState | null { if (!selection) return null; if (!selection.id && !selection.selector) return null; + // The primary is already carried by selId; the rest ride along so the link + // reopens the same multi-selection instead of a single element. + const groupIds = group + .filter((member) => member.id && member.id !== selection.id) + .map((member) => member.id as string); return { sourceFile: selection.sourceFile || undefined, id: selection.id || undefined, selector: selection.selector || undefined, selectorIndex: selection.selectorIndex ?? undefined, + groupIds: groupIds.length > 0 ? groupIds : undefined, }; } @@ -67,6 +80,8 @@ export function useStudioUrlState({ rightCollapsed, activeCompPathHydrated, domEditSelection, + domEditGroupSelections, + applyMarqueeSelection, buildDomSelectionFromTarget, applyDomSelection, setRightPanelTab, @@ -91,10 +106,10 @@ export function useStudioUrlState({ rightCollapsed, timelineVisible: null, selection: hydratedSelectionRef.current - ? toPersistedSelection(domEditSelection) + ? toPersistedSelection(domEditSelection, domEditGroupSelections) : pendingSelectionRef.current, }), - [activeCompPath, domEditSelection, rightCollapsed, rightPanelTab], + [activeCompPath, domEditGroupSelections, domEditSelection, rightCollapsed, rightPanelTab], ); // Resolve a URL selection to a live element and apply it. Shared by the initial @@ -128,12 +143,32 @@ export function useStudioUrlState({ applyDomSelection(null, { revealPanel: false }); return true; } - void buildDomSelectionFromTarget(element, { preferClipAncestor: false }).then((resolved) => { - applyDomSelection(resolved, { revealPanel: false }); - }); + const groupIds = selection.groupIds ?? []; + void (async () => { + const primary = await buildDomSelectionFromTarget(element, { preferClipAncestor: false }); + if (!primary) return applyDomSelection(null, { revealPanel: false }); + if (groupIds.length === 0) return applyDomSelection(primary, { revealPanel: false }); + // Restore the whole multi-selection, primary first so it stays the anchor. + // Members whose element is gone are dropped rather than failing the rest. + const members = [primary]; + for (const memberId of groupIds) { + const memberEl = doc.getElementById(memberId); + const resolved = memberEl + ? await buildDomSelectionFromTarget(memberEl, { preferClipAncestor: false }) + : null; + if (resolved) members.push(resolved); + } + applyMarqueeSelection(members, false); + })(); return true; }, - [activeCompPath, applyDomSelection, buildDomSelectionFromTarget, previewIframeRef], + [ + activeCompPath, + applyDomSelection, + applyMarqueeSelection, + buildDomSelectionFromTarget, + previewIframeRef, + ], ); useEffect(() => { diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts index a0073fb50a..72bb69b8fc 100644 --- a/packages/studio/src/utils/dragDebug.ts +++ b/packages/studio/src/utils/dragDebug.ts @@ -33,6 +33,37 @@ export function readDragPositions( return positions; } +/** + * Members whose screen movement disagrees with the rest of the group this frame. + * + * A group moves as one object, so every member travels the same distance; one + * that does not is the whole bug, and averaged-looking samples hide it. Compares + * each member's movement against the group's median and names the outliers, so a + * single element drifting shows up as itself rather than as "the group jumped". + */ +export function findNonRigidMembers( + before: Record, + after: Record, +): string[] { + const moves = new Map(); + for (const key of Object.keys(after)) { + const from = before[key]?.split(",").map(Number); + const to = after[key]?.split(",").map(Number); + if (!from || !to || from.length !== 2 || to.length !== 2) continue; + moves.set(key, `${Math.round(to[0]! - from[0]!)},${Math.round(to[1]! - from[1]!)}`); + } + const counts = new Map(); + for (const move of moves.values()) counts.set(move, (counts.get(move) ?? 0) + 1); + let common = ""; + let best = 0; + for (const [move, count] of counts) { + if (count > best) [common, best] = [move, count]; + } + return [...moves] + .filter(([, move]) => move !== common) + .map(([key, move]) => `${key.split("|")[2] ?? key} moved ${move}, group moved ${common}`); +} + /** * Sample the group now and again after the commit has had time to land. The drop * is the one moment a jump can hide: the source write, the preview reload and the diff --git a/packages/studio/src/utils/studioUrlState.test.ts b/packages/studio/src/utils/studioUrlState.test.ts index 4d2c6eb801..22246970a9 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -77,6 +77,8 @@ function renderStudioUrlStateHarness( rightCollapsed: true, activeCompPathHydrated: true, domEditSelection: null, + domEditGroupSelections: [], + applyMarqueeSelection: () => {}, buildDomSelectionFromTarget: () => Promise.resolve(null), applyDomSelection: () => {}, initialState: { @@ -132,9 +134,38 @@ describe("studio url state", () => { id: "hero", selector: undefined, selectorIndex: undefined, + groupIds: undefined, }); }); + /** + * A link to a bug hit while several elements were selected has to carry the + * whole selection. Without the group the URL reopens one element, the report + * cannot be reproduced from it, and it reads as "works for me". + */ + it("round-trips a multi-selection through the hash", () => { + const hash = buildStudioHash("demo", { + activeCompPath: null, + currentTime: null, + rightPanelTab: null, + rightCollapsed: null, + timelineVisible: null, + selection: { + sourceFile: "index.html", + id: "chip", + groupIds: ["card", "dot-b"], + }, + }); + + expect(hash).toContain("selGroup=card%2Cdot-b"); + expect(parseStudioUrlStateFromHash(hash).selection?.groupIds).toEqual(["card", "dot-b"]); + }); + + it("reads a single selection as having no group", () => { + const hash = parseStudioUrlStateFromHash("#project/demo?v=1&selFile=index.html&selId=hero"); + expect(hash.selection?.groupIds).toBeUndefined(); + }); + it("builds a project hash with persisted studio state", () => { expect( buildStudioHash("demo", { diff --git a/packages/studio/src/utils/studioUrlState.ts b/packages/studio/src/utils/studioUrlState.ts index e295ca578b..89624ec386 100644 --- a/packages/studio/src/utils/studioUrlState.ts +++ b/packages/studio/src/utils/studioUrlState.ts @@ -7,6 +7,13 @@ export interface StudioUrlSelectionState { id?: string; selector?: string; selectorIndex?: number; + /** + * The other members of a multi-selection, by element id, primary excluded. + * A link to a bug in a group edit is only reproducible if it carries the group; + * without this, opening the URL lands on one element and the report reads as + * "works for me". + */ + groupIds?: string[]; } export interface StudioUrlState { @@ -63,19 +70,28 @@ function parseTab(value: string | null): RightPanelTab | null { return VALID_TABS.includes(value as RightPanelTab) ? (value as RightPanelTab) : null; } +/** The other members of a multi-selection, dropping blanks a hand-edited URL leaves. */ +function parseGroupIds(value: string | null): string[] | undefined { + const ids = (value ?? "") + .split(",") + .map((id) => id.trim()) + .filter(Boolean); + return ids.length > 0 ? ids : undefined; +} + function normalizeSelection(params: URLSearchParams): StudioUrlSelectionState | null { const sourceFile = params.get("selFile") || undefined; const id = params.get("selId") || undefined; const selector = params.get("selSelector") || undefined; - const selectorIndex = parseNumber(params.get("selIndex")); - if (!sourceFile && !id && !selector) return null; + const selectorIndex = parseNumber(params.get("selIndex")); return { sourceFile, id, selector, selectorIndex: selectorIndex != null ? Math.max(0, Math.floor(selectorIndex)) : undefined, + groupIds: parseGroupIds(params.get("selGroup")), }; } @@ -130,6 +146,9 @@ export function buildStudioHash(projectId: string, state: StudioUrlState): strin if (typeof state.selection.selectorIndex === "number") { params.set("selIndex", String(Math.max(0, Math.floor(state.selection.selectorIndex)))); } + if (state.selection.groupIds?.length) { + params.set("selGroup", state.selection.groupIds.join(",")); + } } return buildProjectHash(projectId, params); From 215d920279c33bf11176873118611f812242dff9 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 14:23:20 -0700 Subject: [PATCH 10/26] fix(studio): stop snapping from moving a selection you have not dragged yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Your log caught it on the first frame of the drag: pointer "0,0", applied "4,-3", and all four members jumped 12,-8 composition px before the pointer had moved at all. An element resting within the 6px snap threshold of a guide is already snappable, so the snap computed on frame one closes that gap immediately — picking the selection up moves it. Snapping now sits out until the gesture has travelled the same 4px a drag needs to count as a drag rather than a click, on both the group and single-element paths. Nothing below that distance moves anything, and a real drag snaps exactly as before. The test builds a box resting 4px from a guide and asserts the ungated call still returns dx 4 — the very displacement from your log — while the gated one returns 0 for a pointer that has not moved. --- .../src/components/editor/groupDragMove.ts | 8 ++- .../editor/snapEngageTravel.test.ts | 65 +++++++++++++++++++ .../src/components/editor/snapEngine.ts | 22 ++++++- .../editor/useDomEditOverlayGestures.ts | 10 ++- 4 files changed, 102 insertions(+), 3 deletions(-) create mode 100644 packages/studio/src/components/editor/snapEngageTravel.test.ts diff --git a/packages/studio/src/components/editor/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts index 2202d7c4e0..27281d2a13 100644 --- a/packages/studio/src/components/editor/groupDragMove.ts +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -1,5 +1,10 @@ import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry"; -import { resolveEquidistanceGuides, resolveSnapAdjustment, SNAP_THRESHOLD_PX } from "./snapEngine"; +import { + resolveEquidistanceGuides, + resolveSnapAdjustment, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, +} from "./snapEngine"; import { applyManualOffsetDragDraft } from "./manualOffsetDrag"; import type { GroupGestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures"; import type { GroupOverlayItem } from "./domEditOverlayGeometry"; @@ -31,6 +36,7 @@ export function createGroupDragMover( ) => { const sc = groupG.snapContext; if (!sc?.snapEnabled || sc.targets.length === 0) return proposed; + if (!snapEngagedForTravel(proposed.dx, proposed.dy)) return proposed; const groupBounds = resolveDomEditGroupOverlayRect(groupG.originItems.map((i) => i.rect)); if (!groupBounds) return proposed; const allTargets = sc.compositionTarget ? [...sc.targets, sc.compositionTarget] : sc.targets; diff --git a/packages/studio/src/components/editor/snapEngageTravel.test.ts b/packages/studio/src/components/editor/snapEngageTravel.test.ts new file mode 100644 index 0000000000..74b045bd58 --- /dev/null +++ b/packages/studio/src/components/editor/snapEngageTravel.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { + resolveSnapAdjustment, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, + type SnapTarget, +} from "./snapEngine"; + +/** + * Picking a selection up used to move it. An element resting within the snap + * threshold of a guide is already snappable, so the snap computed on the first + * frame of a drag displaced it by up to the threshold while the pointer had not + * moved at all — captured live as `pointer "0,0"` against `applied "4,-3"`, with + * every member of the group jumping 12,-8 composition px before the drag had + * started. Snapping pulls toward a guide as you drag; it has nothing to say about + * a gesture that has not moved. + */ +describe("snapping waits for the drag to travel", () => { + // Moving box's right edge is at 150; the target's left edge is at 154, so the + // pair is 4px apart — inside the threshold, and snappable the moment it is asked. + const movingRect = { left: 100, top: 50, width: 50, height: 40 }; + const target: SnapTarget = { + left: 154, + top: 50, + right: 254, + bottom: 90, + centerX: 204, + centerY: 70, + id: "neighbour", + }; + + const snapAt = (dx: number, dy: number) => + resolveSnapAdjustment({ + movingRect, + proposedDx: dx, + proposedDy: dy, + targets: [target], + threshold: SNAP_THRESHOLD_PX, + disabled: false, + disabledForTravel: !snapEngagedForTravel(dx, dy), + }); + + it("does not move a selection that has not been dragged yet", () => { + expect(snapAt(0, 0)).toMatchObject({ dx: 0, dy: 0 }); + }); + + it("leaves a sub-threshold twitch alone", () => { + expect(snapAt(1, -1)).toMatchObject({ dx: 1, dy: -1 }); + }); + + it("still snaps once the drag is a real one", () => { + expect(snapEngagedForTravel(0, 0)).toBe(false); + expect(snapEngagedForTravel(10, 0)).toBe(true); + // Without the travel gate the same delta snaps, which is the behaviour to keep. + const engaged = resolveSnapAdjustment({ + movingRect, + proposedDx: 0, + proposedDy: 0, + targets: [target], + threshold: SNAP_THRESHOLD_PX, + disabled: false, + }); + expect(engaged.dx).toBe(4); + }); +}); diff --git a/packages/studio/src/components/editor/snapEngine.ts b/packages/studio/src/components/editor/snapEngine.ts index 6a64a22a29..f80db5f3b4 100644 --- a/packages/studio/src/components/editor/snapEngine.ts +++ b/packages/studio/src/components/editor/snapEngine.ts @@ -3,6 +3,24 @@ // All position values are in overlay-space (screen) pixels. export const SNAP_THRESHOLD_PX = 6; +/** + * Pointer travel a MOVE must reach before snapping is allowed to touch it. + * + * An element resting within the threshold of a guide is already "snappable", so + * a snap computed on the very first frame displaces it by up to the threshold + * while the pointer has moved nothing — pick a selection up and the whole thing + * teleports before you have dragged at all. Snapping is meant to pull toward a + * guide as the user drags, so it does not participate until the drag is real. + * The value matches the distance a drag must cover to count as a drag rather + * than a click, so nothing below it moves anything. + */ +const SNAP_ENGAGE_TRAVEL_PX = 4; + +/** Whether a move of this size has travelled far enough for snapping to apply. */ +export function snapEngagedForTravel(dx: number, dy: number): boolean { + return Math.hypot(dx, dy) >= SNAP_ENGAGE_TRAVEL_PX; +} + const EQUIDISTANCE_TOLERANCE_PX = 1; // --------------------------------------------------------------------------- @@ -359,8 +377,10 @@ export function resolveSnapAdjustment(input: { gridEdges?: { x: SnapEdge[]; y: SnapEdge[] }; threshold: number; disabled: boolean; + /** Set when the gesture has not travelled far enough for snapping yet. */ + disabledForTravel?: boolean; }): SnapResult { - if (input.disabled || input.threshold <= 0) { + if (input.disabled || input.disabledForTravel || input.threshold <= 0) { return DISABLED_RESULT(input.proposedDx, input.proposedDy); } diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index 59967d8cc0..7831e5cba7 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -49,7 +49,12 @@ import { startGroupDrag as _startGroupDrag, } from "./domEditOverlayStartGesture"; import { hugRectForElement } from "./domEditOverlayCrop"; -import { resolveSnapAdjustment, resolveEquidistanceGuides, SNAP_THRESHOLD_PX } from "./snapEngine"; +import { + resolveSnapAdjustment, + resolveEquidistanceGuides, + snapEngagedForTravel, + SNAP_THRESHOLD_PX, +} from "./snapEngine"; import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug"; import { createGroupDragMover } from "./groupDragMove"; @@ -170,6 +175,9 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu movingRect, proposedDx: dx, proposedDy: dy, + // Same reason as the group path: a snap on a drag that has not travelled + // yet moves the element while the pointer is still. + disabledForTravel: !snapEngagedForTravel(dx, dy), targets: allTargets, gridEdges: sc.gridEdges ?? undefined, threshold: SNAP_THRESHOLD_PX, From 33d363d6bca093825691d8876a5181008ac15d2a Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 14:43:48 -0700 Subject: [PATCH 11/26] fix(studio): a dropped group stays selected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Your Jam confirmed the first-frame jump is gone — pointer "0,0" now reads applied "0,0" — and caught what was left: two milliseconds after each drop, a `[hf-select] clear` with the group still holding three, then four members. Every pointerup trails a click. The group gesture ref is cleared before the commit runs, so by the time that click arrives the box no longer looks busy and it reaches the canvas as an ordinary click — landing in the gap between the members, resolving to nothing, and clearing the selection the drag just moved. The under-threshold path already ate that click; the committed path never did. The flag is now set before the two paths diverge, so neither can forget it. The test drives a real pointerup through the handlers and fails on the committed path with the flag moved back down. --- .../editor/groupDropKeepsSelection.test.ts | 70 +++++++++++++++++++ .../editor/useDomEditOverlayGestures.ts | 7 +- 2 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 packages/studio/src/components/editor/groupDropKeepsSelection.test.ts diff --git a/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts b/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts new file mode 100644 index 0000000000..3fc95b6742 --- /dev/null +++ b/packages/studio/src/components/editor/groupDropKeepsSelection.test.ts @@ -0,0 +1,70 @@ +// @vitest-environment happy-dom +import { describe, expect, it, vi } from "vitest"; +import { createDomEditOverlayGestureHandlers } from "./useDomEditOverlayGestures"; +import type { GroupGestureState } from "./domEditOverlayGestures"; + +/** + * A group drag ended by deselecting the group it had just moved. + * + * Every pointerup trails a click. The gesture ref is cleared before the commit, + * so by the time that click arrives the box no longer looks busy and it reaches + * the canvas as an ordinary click — landing in the gap between the members, + * resolving to nothing, and clearing the selection. Captured live as a + * `[hf-select] clear` with `hadGroup: 3` two milliseconds after the drop. + * + * The under-threshold path already ate that click; the committed path has to as + * well, and the flag is set before the two diverge so neither can forget. + */ +describe("dropping a dragged group eats the click that follows", () => { + function harness(travel: { dx: number; dy: number }) { + const suppressNextBoxClickRef = { current: false }; + const groupGestureRef = { + current: { + startX: 0, + startY: 0, + originItems: [], + members: [], + } as unknown as GroupGestureState, + }; + const handlers = createDomEditOverlayGestureHandlers({ + overlayRef: { current: null }, + iframeRef: { current: null }, + boxRef: { current: null }, + selectionRef: { current: null }, + hoverSelectionRef: { current: null }, + overlayRectRef: { current: null }, + groupOverlayItemsRef: { current: [] }, + gestureRef: { current: null }, + groupGestureRef, + blockedMoveRef: { current: null }, + rafPausedRef: { current: false }, + suppressNextBoxClickRef, + setOverlayRect: vi.fn(), + setGroupOverlayItems: vi.fn(), + onBlockedMoveRef: { current: vi.fn() }, + onManualDragStartRef: { current: vi.fn() }, + onPathOffsetCommitRef: { current: vi.fn() }, + onGroupPathOffsetCommitRef: { current: vi.fn() }, + onBoxSizeCommitRef: { current: vi.fn() }, + onRotationCommitRef: { current: vi.fn() }, + onCanvasPointerMoveRef: { current: vi.fn() }, + onCanvasMouseDown: vi.fn(), + snapGuidesRef: { current: null }, + } as never); + + handlers.onPointerUp({ + clientX: travel.dx, + clientY: travel.dy, + currentTarget: { releasePointerCapture: vi.fn() }, + } as never); + return suppressNextBoxClickRef; + } + + it("eats the click after a drag that moved", () => { + expect(harness({ dx: 120, dy: 60 }).current).toBe(true); + }); + + it("still eats it after a press that never travelled", () => { + expect(harness({ dx: 1, dy: 0 }).current).toBe(true); + }); +}); diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index 7831e5cba7..65e8bb4222 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -282,9 +282,14 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu opts.rafPausedRef.current = false; const rawDx = e.clientX - groupG.startX; const rawDy = e.clientY - groupG.startY; + // The click that trails every pointerup has to be eaten either way. The + // gesture ref is already cleared above, so by the time it arrives the box + // no longer looks busy, and handleBoxClick hands it to the canvas as an + // ordinary click — which lands between the members, resolves to nothing, + // and deselects the group the drag just moved. + opts.suppressNextBoxClickRef.current = true; if (Math.hypot(rawDx, rawDy) < BLOCKED_MOVE_THRESHOLD_PX) { restoreGroupPathOffsets(groupG); - opts.suppressNextBoxClickRef.current = true; return; } const dx = groupG.lastSnappedDx ?? rawDx; From 0de5a0f772577bd1a6ad606e1b3b34f368731cec Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 14:52:39 -0700 Subject: [PATCH 12/26] feat(studio): marquee from anywhere on the canvas, including outside the frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An element dragged past the edge sits out in the grey, and the rubber band refused to start there — it only began when the press landed inside the composition rect. The one gesture that could reach those elements could not be begun near them, so the timeline was the only way to select something plainly visible on screen. The collecting half never had that limit: it compares rects in overlay space and never clipped to the frame, so those elements have always been selectable once the band could begin. Only the start gate had to go. A press in the grey that never travels still commits an empty selection, which is the deselect it used to be, so the old behaviour of clicking out there to clear is unchanged. --- .../src/components/editor/DomEditOverlay.tsx | 39 +++++++++---------- .../editor/marqueeOutsideCanvas.test.ts | 31 +++++++++++++++ 2 files changed, 50 insertions(+), 20 deletions(-) create mode 100644 packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index db98077531..a747b66c7a 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -405,28 +405,27 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const overlayEl = overlayRef.current; if (overlayEl) { const oRect = overlayEl.getBoundingClientRect(); + // Anywhere empty on the overlay starts one, not just inside the frame. + // An element dragged past the edge sits OUT there in the grey, and a + // rubber band that refuses to start there cannot reach it — which left + // the timeline as the only way to select something you can plainly see. + // The hit test collects in overlay space and never clipped to the frame, + // so those elements were always selectable once the band could begin. + event.preventDefault(); + event.stopPropagation(); + suppressNextOverlayMouseDownRef.current = true; + (event.currentTarget as HTMLElement).setPointerCapture(event.pointerId); const cx = event.clientX - oRect.left; const cy = event.clientY - oRect.top; - const inComp = - cx >= compRect.left && - cx <= compRect.left + compRect.width && - cy >= compRect.top && - cy <= compRect.top + compRect.height; - if (inComp) { - event.preventDefault(); - event.stopPropagation(); - suppressNextOverlayMouseDownRef.current = true; - (event.currentTarget as HTMLElement).setPointerCapture(event.pointerId); - marquee.marqueeRef.current = { - startX: cx, - startY: cy, - currentX: cx, - currentY: cy, - pointerId: event.pointerId, - pastThreshold: false, - }; - return; - } + marquee.marqueeRef.current = { + startX: cx, + startY: cy, + currentX: cx, + currentY: cy, + pointerId: event.pointerId, + pastThreshold: false, + }; + return; } } }; diff --git a/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts b/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts new file mode 100644 index 0000000000..a65ae2d105 --- /dev/null +++ b/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { rectsOverlap } from "../../utils/marqueeGeometry"; + +/** + * An element dragged past the edge of the frame sits out in the grey, and the + * rubber band refused to START there — it only began inside the composition + * rect, so the one gesture that could reach those elements could not be begun + * near them, leaving the timeline as the only way to select something plainly + * visible on screen. + * + * The collection half never had that limit: it compares rects in overlay space + * and never clipped to the frame, so a band drawn out in the grey has always + * been able to find what it covers. This pins that, including the negative + * coordinates an off-canvas element actually has. + */ +describe("marquee reaches elements outside the composition", () => { + const offCanvas = { left: -180, top: 40, width: 90, height: 40 }; + + it("covers an element sitting left of the frame", () => { + expect(rectsOverlap({ left: -220, top: 10, width: 160, height: 120 }, offCanvas)).toBe(true); + }); + + it("covers one sitting above the frame", () => { + const above = { left: 60, top: -140, width: 80, height: 50 }; + expect(rectsOverlap({ left: 20, top: -200, width: 200, height: 120 }, above)).toBe(true); + }); + + it("does not claim one the band misses", () => { + expect(rectsOverlap({ left: 400, top: 400, width: 50, height: 50 }, offCanvas)).toBe(false); + }); +}); From 44a26909ae3216366d576423acf468997ac9f97d Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 03:21:03 -0400 Subject: [PATCH 13/26] refactor(studio): keep the selection files under the size cap The selection work above pushed four files past the 600-line gate. Same split the branch made later, landed with the changes that caused it. --- .../src/components/editor/DomEditOverlay.tsx | 48 ++++++--------- .../editor/domEditOverlayGestures.ts | 39 +++++++++--- .../src/hooks/domSelectionTimelineMirror.ts | 60 +++++++++++++++++++ packages/studio/src/hooks/useDomSelection.ts | 57 +++--------------- 4 files changed, 117 insertions(+), 87 deletions(-) create mode 100644 packages/studio/src/hooks/domSelectionTimelineMirror.ts diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index a747b66c7a..151c24ce08 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -13,7 +13,7 @@ import { type GestureState, type GroupGestureState, focusDomEditOverlayElement, - hoverCacheDescribesPoint, + resolveShiftClickCandidate, } from "./domEditOverlayGestures"; import { useDomEditOverlayRects } from "./useDomEditOverlayRects"; import { OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; @@ -32,6 +32,7 @@ import { startOffCanvasIndicatorRefresh } from "./offCanvasIndicatorRefresh"; import { CanvasContextMenu } from "./CanvasContextMenu"; import type { ZOrderAction, ZOrderPatch } from "./canvasContextMenuZOrder"; import { getPreviewTargetFromPointer } from "../../utils/studioPreviewHelpers"; +import { logSelect } from "../../utils/selectDebug"; // Re-exports for external consumers — preserving existing import paths. export { @@ -44,7 +45,6 @@ export { hasDomEditRotationChanged, resolveDomEditRotationGesture, } from "./domEditOverlayGestures"; -import { logSelect } from "../../utils/selectDebug"; export type { DomEditGroupPathOffsetCommit } from "./domEditOverlayGestures"; interface DomEditOverlayProps { @@ -329,11 +329,9 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } const target = event.target as HTMLElement | null; - logSelect("mousedown", { - shift: event.shiftKey, - onBox: Boolean(target?.closest('[data-dom-edit-selection-box="true"]')), - }); - if (target?.closest('[data-dom-edit-selection-box="true"]')) return; + const onBox = Boolean(target?.closest('[data-dom-edit-selection-box="true"]')); + logSelect("mousedown", { shift: event.shiftKey, onBox }); + if (onBox) return; // Allow clicks anywhere on the overlay — GSAP-translated elements can // extend beyond the composition rect into the gray zone, and users need // to select/deselect them by clicking there. @@ -348,31 +346,21 @@ export const DomEditOverlay = memo(function DomEditOverlay({ const handleOverlayPointerDown = (event: React.PointerEvent) => { if (!allowCanvasMovement || event.button !== 0) return; if (event.shiftKey) { - // The hover selection is an ASYNC cache, so it can still describe whatever the - // pointer passed over on its way here. Reading it without checking is safe for - // a hover outline and wrong for a shift-click: the click silently adds THAT - // element instead of the one under the pointer, which reads as multi-select - // picking things at random. Confirm the cache is about this point with a - // synchronous hit-test; when it isn't, fall through untouched — no - // preventDefault, no suppression — so the mousedown path below resolves the - // point properly instead of this one guessing. - const candidate = hoverSelectionRef.current; const shiftIframe = iframeRef.current; - const pointTarget = shiftIframe - ? getPreviewTargetFromPointer( - shiftIframe, - event.clientX, - event.clientY, - activeCompositionPathRef.current, - ) - : null; - const cacheIsAboutThisPoint = hoverCacheDescribesPoint(candidate?.element, pointTarget); - logSelect("shift-pointerdown", { - candidate: candidate?.selector ?? candidate?.id ?? null, - pointTarget: pointTarget?.id ?? pointTarget?.tagName ?? null, - cacheIsAboutThisPoint, + const candidate = resolveShiftClickCandidate({ + cached: hoverSelectionRef.current, + elementAtPoint: shiftIframe + ? getPreviewTargetFromPointer( + shiftIframe, + event.clientX, + event.clientY, + activeCompositionPathRef.current, + ) + : null, }); - if (!candidate || !cacheIsAboutThisPoint) return; + // Not confident: fall through untouched — no preventDefault, no suppression — + // so the mousedown path resolves this point instead of guessing here. + if (!candidate) return; event.preventDefault(); event.stopPropagation(); suppressNextOverlayMouseDownRef.current = true; diff --git a/packages/studio/src/components/editor/domEditOverlayGestures.ts b/packages/studio/src/components/editor/domEditOverlayGestures.ts index 562680f84b..34d6519c19 100644 --- a/packages/studio/src/components/editor/domEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/domEditOverlayGestures.ts @@ -10,6 +10,7 @@ import type { GroupOverlayItem, OverlayRect } from "./domEditOverlayGeometry"; import type { SnapContext } from "./snapTargetCollection"; import type { SnapGuidesState } from "./SnapGuideOverlay"; import type { PreviewMouseDownOptions } from "../../hooks/usePreviewInteraction"; +import { logSelect } from "../../utils/selectDebug"; export type GestureKind = "drag" | "resize" | "rotate"; @@ -112,14 +113,6 @@ export function focusDomEditOverlayElement(element: FocusableDomEditOverlay | nu element?.focus({ preventScroll: true }); } -/** - * Overlay-px translation that keeps the element's CENTER fixed while a corner - * resizes: a CSS width/height change grows the layout box from its top-left, so - * the center drifts by half the size change on each axis; translating back by that - * half-delta re-pins the center. This is the UNROTATED (AABB) fallback used only - * when the element's real transformed corners can't be measured — the primary path - * pins the measured center (rotation-safe) in useDomEditOverlayGestures. - */ /** * Whether the hover cache may stand in for a hit-test at this point. * @@ -139,6 +132,36 @@ export function hoverCacheDescribesPoint( return cachedElement === elementAtPoint || cachedElement.contains(elementAtPoint); } +/** + * The element a shift-click should add, or null to let the slower path resolve it. + * + * Reading the hover cache without checking is safe for a hover outline and wrong + * for a shift-click: the click silently adds whatever the pointer last passed + * over instead of the element under it, which reads as multi-select picking + * things at random. Returning null means "not confident", and the caller must + * then fall through untouched so the mousedown path resolves the point properly. + */ +export function resolveShiftClickCandidate(input: { + cached: T | null; + elementAtPoint: Element | null; +}): T | null { + const describes = hoverCacheDescribesPoint(input.cached?.element, input.elementAtPoint); + logSelect("shift-pointerdown", { + candidate: input.cached ? ((input.cached as { selector?: string }).selector ?? null) : null, + pointTarget: input.elementAtPoint?.id ?? input.elementAtPoint?.tagName ?? null, + cacheIsAboutThisPoint: describes, + }); + return describes ? input.cached : null; +} + +/** + * Overlay-px translation that keeps the element's CENTER fixed while a corner + * resizes: a CSS width/height change grows the layout box from its top-left, so + * the center drifts by half the size change on each axis; translating back by that + * half-delta re-pins the center. This is the UNROTATED (AABB) fallback used only + * when the element's real transformed corners can't be measured — the primary path + * pins the measured center (rotation-safe) in useDomEditOverlayGestures. + */ export function resolveResizeCenterAnchorOffset(input: { originWidth: number; originHeight: number; diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts new file mode 100644 index 0000000000..346acd1ae2 --- /dev/null +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -0,0 +1,60 @@ +import type { SelectElementOptions, TimelineElement } from "../player"; +import { findMatchingTimelineElementId, findTimelineIdByAncestor } from "../utils/studioHelpers"; +import type { DomEditSelection } from "../components/editor/domEditing"; +import { logSelect } from "../utils/selectDebug"; + +interface TimelineMirrorDeps { + timelineElements: TimelineElement[]; + setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; + setTimelineSelectionSet: (ids: Set) => void; +} + +/** + * Mirror a canvas selection onto the timeline: the whole set first, then the + * primary as its anchor. + * + * The timeline is the source of truth for what is selected and it syncs back — + * whatever it holds replaces the canvas selection a moment later. Announcing only + * the primary therefore drops every other member. Worse, anchoring with + * `preserveSet` on an id the set does not yet contain empties the set outright, + * and an empty set syncs back as "nothing is selected" — which is how adding a + * second element, or moving a group, could wipe the selection instead of keeping + * it. Publishing the members first is what makes the anchor a member, so + * preserving the set is meaningful rather than destructive. + */ +export function announceTimelineSelection( + deps: TimelineMirrorDeps, + group: DomEditSelection[], + primary: DomEditSelection | null, +): void { + const { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet } = deps; + if (!primary) { + setTimelineSelectionSet(new Set()); + setSelectedTimelineElementId(null); + return; + } + const timelineIdFor = (selection: DomEditSelection) => + findMatchingTimelineElementId(selection, timelineElements) ?? + findTimelineIdByAncestor( + selection.element, + timelineElements, + selection.sourceFile || "index.html", + ); + const members = group.map(timelineIdFor).filter((id): id is string => Boolean(id)); + const anchor = timelineIdFor(primary); + // A member with no timeline row of its own resolves to null and is dropped here, + // so a group can announce fewer ids than it has — or none, which reads back as an + // empty selection and takes the canvas selection with it. + logSelect("announce", { + group: group.length, + published: members.length, + anchor, + anchorPublished: anchor != null && members.includes(anchor), + }); + // Only a real multi-selection publishes members. A single selection keeps the + // older contract on purpose: anchoring with preserveSet holds a live set the + // element already belongs to (a late async primary must not collapse a group) + // and collapses otherwise, which is what a fresh click means. + if (group.length > 1) setTimelineSelectionSet(new Set(members)); + setSelectedTimelineElementId(anchor, { preserveSet: true }); +} diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index dbe7d4b833..e24c34c637 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -4,11 +4,7 @@ import { getAllPreviewTargetsFromPointer, getPreviewTargetFromPointer, } from "../utils/studioPreviewHelpers"; -import { - findMatchingTimelineElementId, - findTimelineIdByAncestor, - type RightPanelTab, -} from "../utils/studioHelpers"; +import { type RightPanelTab } from "../utils/studioHelpers"; import { domEditSelectionsTargetSame, domEditSelectionInGroup, @@ -25,6 +21,7 @@ import { import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import { useStudioTestHooks } from "./useStudioTestHooks"; import { logSelect } from "../utils/selectDebug"; +import { announceTimelineSelection as announceSelectionToTimeline } from "./domSelectionTimelineMirror"; // ── Types ── @@ -149,51 +146,13 @@ export function useDomSelection({ // ── Callbacks ── - /** - * Mirror a canvas selection onto the timeline: the whole set first, then the - * primary as its anchor. - * - * The timeline is the source of truth for what is selected and it syncs back — - * whatever it holds replaces the canvas selection a moment later. Announcing only - * the primary therefore drops every other member. Worse, anchoring with - * `preserveSet` on an id the set does not yet contain empties the set outright, - * and an empty set syncs back as "nothing is selected" — which is how adding a - * second element, or moving a group, could wipe the selection instead of keeping - * it. Publishing the members first is what makes the anchor a member, so - * preserving the set is meaningful rather than destructive. - */ const announceTimelineSelection = useCallback( - (group: DomEditSelection[], primary: DomEditSelection | null) => { - if (!primary) { - setTimelineSelectionSet(new Set()); - setSelectedTimelineElementId(null); - return; - } - const timelineIdFor = (selection: DomEditSelection) => - findMatchingTimelineElementId(selection, timelineElements) ?? - findTimelineIdByAncestor( - selection.element, - timelineElements, - selection.sourceFile || "index.html", - ); - // Only a real multi-selection publishes members. A single selection keeps the - // older contract on purpose: anchoring with preserveSet holds a live set the - // element already belongs to (a late async primary must not collapse a group) - // and collapses otherwise, which is what a fresh click means. - const members = group.map(timelineIdFor).filter((id): id is string => Boolean(id)); - const anchor = timelineIdFor(primary); - // A member with no timeline row of its own resolves to null and is dropped - // here, so a group can announce fewer ids than it has — or none, which reads - // back as an empty selection and takes the canvas selection with it. - logSelect("announce", { - group: group.length, - published: members.length, - anchor, - anchorPublished: anchor != null && members.includes(anchor), - }); - if (group.length > 1) setTimelineSelectionSet(new Set(members)); - setSelectedTimelineElementId(anchor, { preserveSet: true }); - }, + (group: DomEditSelection[], primary: DomEditSelection | null) => + announceSelectionToTimeline( + { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet }, + group, + primary, + ), [setSelectedTimelineElementId, setTimelineSelectionSet, timelineElements], ); From d94c70c2c532fd9c72bd747d592b5b841bc035b8 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 12:50:00 -0400 Subject: [PATCH 14/26] fix(studio): preserve selector groups in share URLs --- .../studio/src/hooks/useStudioUrlState.ts | 60 +++++++++--- .../studio/src/utils/studioUrlState.test.ts | 93 +++++++++++++++++-- packages/studio/src/utils/studioUrlState.ts | 64 ++++++++++--- 3 files changed, 184 insertions(+), 33 deletions(-) diff --git a/packages/studio/src/hooks/useStudioUrlState.ts b/packages/studio/src/hooks/useStudioUrlState.ts index 263182676c..2b8d02e0d0 100644 --- a/packages/studio/src/hooks/useStudioUrlState.ts +++ b/packages/studio/src/hooks/useStudioUrlState.ts @@ -7,6 +7,7 @@ import { buildStudioHash, parseStudioUrlStateFromHash, type StudioUrlSelectionState, + type StudioUrlSelectionTarget, type StudioUrlState, } from "../utils/studioUrlState"; @@ -40,6 +41,25 @@ interface UseStudioUrlStateParams { initialState: StudioUrlState; } +function toPersistedTarget(selection: DomEditSelection): StudioUrlSelectionTarget | null { + if (!selection.id && !selection.selector) return null; + return { + sourceFile: selection.sourceFile || undefined, + id: selection.id || undefined, + selector: selection.selector || undefined, + selectorIndex: selection.selectorIndex ?? undefined, + }; +} + +function selectionTargetKey(selection: StudioUrlSelectionTarget): string { + return [ + selection.sourceFile ?? "", + selection.id ?? "", + selection.selector ?? "", + selection.selectorIndex ?? "", + ].join("|"); +} + function toPersistedSelection( selection: DomEditSelection | null, // Optional: a caller that only ever has one selection has nothing to add, and @@ -47,18 +67,21 @@ function toPersistedSelection( group: DomEditSelection[] = [], ): StudioUrlSelectionState | null { if (!selection) return null; - if (!selection.id && !selection.selector) return null; - // The primary is already carried by selId; the rest ride along so the link + const primary = toPersistedTarget(selection); + if (!primary) return null; + // The primary is already carried by the top-level fields; the rest ride along so the link // reopens the same multi-selection instead of a single element. - const groupIds = group - .filter((member) => member.id && member.id !== selection.id) - .map((member) => member.id as string); + const primaryKey = selectionTargetKey(primary); + const members = new Map(); + for (const member of group) { + const target = toPersistedTarget(member); + if (!target) continue; + const key = selectionTargetKey(target); + if (key !== primaryKey) members.set(key, target); + } return { - sourceFile: selection.sourceFile || undefined, - id: selection.id || undefined, - selector: selection.selector || undefined, - selectorIndex: selection.selectorIndex ?? undefined, - groupIds: groupIds.length > 0 ? groupIds : undefined, + ...primary, + group: members.size > 0 ? [...members.values()] : undefined, }; } @@ -143,16 +166,25 @@ export function useStudioUrlState({ applyDomSelection(null, { revealPanel: false }); return true; } - const groupIds = selection.groupIds ?? []; + const group = selection.group ?? []; void (async () => { const primary = await buildDomSelectionFromTarget(element, { preferClipAncestor: false }); if (!primary) return applyDomSelection(null, { revealPanel: false }); - if (groupIds.length === 0) return applyDomSelection(primary, { revealPanel: false }); + if (group.length === 0) return applyDomSelection(primary, { revealPanel: false }); // Restore the whole multi-selection, primary first so it stays the anchor. // Members whose element is gone are dropped rather than failing the rest. const members = [primary]; - for (const memberId of groupIds) { - const memberEl = doc.getElementById(memberId); + for (const member of group) { + const memberEl = findElementForSelection( + doc, + { + sourceFile: member.sourceFile ?? selection.sourceFile ?? "", + id: member.id, + selector: member.selector, + selectorIndex: member.selectorIndex, + }, + activeCompPath, + ); const resolved = memberEl ? await buildDomSelectionFromTarget(memberEl, { preferClipAncestor: false }) : null; diff --git a/packages/studio/src/utils/studioUrlState.test.ts b/packages/studio/src/utils/studioUrlState.test.ts index 22246970a9..e23ec3c287 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -81,6 +81,7 @@ function renderStudioUrlStateHarness( applyMarqueeSelection: () => {}, buildDomSelectionFromTarget: () => Promise.resolve(null), applyDomSelection: () => {}, + setRightPanelTab: () => {}, initialState: { activeCompPath: null, currentTime: 4.2, @@ -118,6 +119,12 @@ function StudioUrlStateHarness(props: Parameters[0]) { return null; } +function previewIframeFor(contentDocument: Document): HTMLIFrameElement { + const iframe = document.createElement("iframe"); + Object.defineProperty(iframe, "contentDocument", { value: contentDocument }); + return iframe; +} + describe("studio url state", () => { it("parses persisted studio state from project hash", () => { const state = parseStudioUrlStateFromHash( @@ -134,7 +141,7 @@ describe("studio url state", () => { id: "hero", selector: undefined, selectorIndex: undefined, - groupIds: undefined, + group: undefined, }); }); @@ -153,17 +160,66 @@ describe("studio url state", () => { selection: { sourceFile: "index.html", id: "chip", - groupIds: ["card", "dot-b"], + group: [ + { sourceFile: "index.html", id: "card" }, + { sourceFile: "index.html", selector: ".dot", selectorIndex: 1 }, + ], }, }); - expect(hash).toContain("selGroup=card%2Cdot-b"); - expect(parseStudioUrlStateFromHash(hash).selection?.groupIds).toEqual(["card", "dot-b"]); + expect(parseStudioUrlStateFromHash(hash).selection?.group).toEqual([ + { sourceFile: "index.html", id: "card" }, + { sourceFile: "index.html", selector: ".dot", selectorIndex: 1 }, + ]); }); it("reads a single selection as having no group", () => { const hash = parseStudioUrlStateFromHash("#project/demo?v=1&selFile=index.html&selId=hero"); - expect(hash.selection?.groupIds).toBeUndefined(); + expect(hash.selection?.group).toBeUndefined(); + }); + + it("restores selector-based multi-selection members from the hash", async () => { + const previewDoc = document.implementation.createHTMLDocument("preview"); + const primaryElement = previewDoc.createElement("div"); + primaryElement.id = "hero"; + const memberElement = previewDoc.createElement("div"); + memberElement.className = "dot"; + previewDoc.body.append(primaryElement, memberElement); + const primary = { element: primaryElement, id: "hero", sourceFile: "index.html" }; + const member = { + element: memberElement, + selector: ".dot", + selectorIndex: 0, + sourceFile: "index.html", + }; + const applyMarqueeSelection = vi.fn(); + + const harness = renderStudioUrlStateHarness({ + previewIframeRef: { + current: previewIframeFor(previewDoc), + }, + applyMarqueeSelection, + buildDomSelectionFromTarget: (target) => + Promise.resolve(target === primaryElement ? primary : member), + initialState: { + activeCompPath: null, + currentTime: null, + rightPanelTab: null, + rightCollapsed: null, + timelineVisible: null, + selection: { + sourceFile: "index.html", + id: "hero", + group: [{ sourceFile: "index.html", selector: ".dot", selectorIndex: 0 }], + }, + }, + }); + + await act(async () => { + await Promise.resolve(); + }); + expect(applyMarqueeSelection).toHaveBeenCalledWith([primary, member], false); + harness.unmount(); }); it("builds a project hash with persisted studio state", () => { @@ -259,7 +315,7 @@ describe("studio url state", () => { const harness = renderStudioUrlStateHarness({ previewIframeRef: { - current: { contentDocument: previewDoc } as HTMLIFrameElement, + current: previewIframeFor(previewDoc), }, rightPanelTab: "design", rightCollapsed: false, @@ -310,6 +366,31 @@ describe("studio url state", () => { expect(window.location.hash).toContain("t=4.2"); expect(window.location.hash).toContain("selId=hero"); + const selectorMember = { + ...restoredSelection, + element: document.createElement("div"), + id: "", + selector: ".dot", + selectorIndex: 1, + label: "Dot", + }; + harness.rerender({ + currentTime: 4.2, + domEditSelection: restoredSelection, + domEditGroupSelections: [restoredSelection, selectorMember], + }); + act(() => { + vi.advanceTimersByTime(250); + }); + expect(parseStudioUrlStateFromHash(window.location.hash).selection?.group).toEqual([ + { + sourceFile: "index.html", + id: undefined, + selector: ".dot", + selectorIndex: 1, + }, + ]); + harness.unmount(); }); }); diff --git a/packages/studio/src/utils/studioUrlState.ts b/packages/studio/src/utils/studioUrlState.ts index 89624ec386..1dfe976f10 100644 --- a/packages/studio/src/utils/studioUrlState.ts +++ b/packages/studio/src/utils/studioUrlState.ts @@ -2,18 +2,21 @@ import type { RightPanelTab } from "./studioHelpers"; import { buildProjectHash, parseProjectHashRoute } from "./projectRouting"; import { roundTo3 } from "./rounding"; -export interface StudioUrlSelectionState { +export interface StudioUrlSelectionTarget { sourceFile?: string; id?: string; selector?: string; selectorIndex?: number; +} + +export interface StudioUrlSelectionState extends StudioUrlSelectionTarget { /** - * The other members of a multi-selection, by element id, primary excluded. + * The other members of a multi-selection, primary excluded. * A link to a bug in a group edit is only reproducible if it carries the group; * without this, opening the URL lands on one element and the report reads as * "works for me". */ - groupIds?: string[]; + group?: StudioUrlSelectionTarget[]; } export interface StudioUrlState { @@ -70,13 +73,48 @@ function parseTab(value: string | null): RightPanelTab | null { return VALID_TABS.includes(value as RightPanelTab) ? (value as RightPanelTab) : null; } -/** The other members of a multi-selection, dropping blanks a hand-edited URL leaves. */ -function parseGroupIds(value: string | null): string[] | undefined { - const ids = (value ?? "") - .split(",") - .map((id) => id.trim()) - .filter(Boolean); - return ids.length > 0 ? ids : undefined; +function optionalString(value: unknown): string | undefined { + return typeof value === "string" ? value : undefined; +} + +function normalizedIndex(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) + ? Math.max(0, Math.floor(value)) + : undefined; +} + +function parseSelectionTarget(value: unknown): StudioUrlSelectionTarget | null { + if (!value || typeof value !== "object") return null; + const sourceFile = optionalString(Reflect.get(value, "sourceFile")); + const id = optionalString(Reflect.get(value, "id")); + const selector = optionalString(Reflect.get(value, "selector")); + if (!id && !selector) return null; + return { + sourceFile, + id, + selector, + selectorIndex: normalizedIndex(Reflect.get(value, "selectorIndex")), + }; +} + +/** The other members of a multi-selection, dropping invalid hand-edited entries. */ +function parseGroup(value: string | null): StudioUrlSelectionTarget[] | undefined { + if (!value) return undefined; + let parsed: unknown; + try { + parsed = JSON.parse(value); + } catch { + // Compatibility with links produced by the first id-only implementation. + const legacy = value + .split(",") + .map((id) => id.trim()) + .filter(Boolean) + .map((id) => ({ id })); + return legacy.length > 0 ? legacy : undefined; + } + if (!Array.isArray(parsed)) return undefined; + const targets = parsed.map(parseSelectionTarget).filter((target) => target !== null); + return targets.length > 0 ? targets : undefined; } function normalizeSelection(params: URLSearchParams): StudioUrlSelectionState | null { @@ -91,7 +129,7 @@ function normalizeSelection(params: URLSearchParams): StudioUrlSelectionState | id, selector, selectorIndex: selectorIndex != null ? Math.max(0, Math.floor(selectorIndex)) : undefined, - groupIds: parseGroupIds(params.get("selGroup")), + group: parseGroup(params.get("selGroup")), }; } @@ -146,8 +184,8 @@ export function buildStudioHash(projectId: string, state: StudioUrlState): strin if (typeof state.selection.selectorIndex === "number") { params.set("selIndex", String(Math.max(0, Math.floor(state.selection.selectorIndex)))); } - if (state.selection.groupIds?.length) { - params.set("selGroup", state.selection.groupIds.join(",")); + if (state.selection.group?.length) { + params.set("selGroup", JSON.stringify(state.selection.group)); } } From 32769f75fed7060d0b5e69ea4bd811884d613921 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 13:21:12 -0400 Subject: [PATCH 15/26] fix(studio): close multi-selection review gaps --- packages/studio/src/App.tsx | 3 +- .../components/editor/DomEditOverlay.test.ts | 42 ++++++- .../editor/domEditOverlayGeometry.test.ts | 19 ++++ .../editor/domEditOverlayGeometry.ts | 19 +++- .../src/components/editor/groupDragMove.ts | 10 +- .../editor/marqueeOutsideCanvas.test.ts | 31 ----- .../src/hooks/domSelectionTimelineMirror.ts | 19 +++- .../src/hooks/useDomEditSession.test.tsx | 107 +++++++----------- .../studio/src/hooks/useDomEditSession.ts | 6 +- .../studio/src/hooks/useDomSelection.test.ts | 1 + packages/studio/src/hooks/useDomSelection.ts | 16 ++- .../useDomSelectionSelectionGuards.test.ts | 5 +- .../studio/src/hooks/useStudioUrlState.ts | 100 ++++++++++++---- packages/studio/src/utils/dragDebug.ts | 31 +++-- packages/studio/src/utils/resizeDebug.ts | 48 ++++---- packages/studio/src/utils/studioDebug.test.ts | 32 ++++++ packages/studio/src/utils/studioDebug.ts | 4 +- .../studio/src/utils/studioUrlState.test.ts | 50 ++++++++ 18 files changed, 368 insertions(+), 175 deletions(-) delete mode 100644 packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts create mode 100644 packages/studio/src/utils/studioDebug.test.ts diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 538aebf9ec..da0149ba03 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -94,7 +94,6 @@ export function StudioApp() { const setTimelineSelectionSet = usePlayerStore((s) => s.setSelectedElementIds); const timelineDuration = usePlayerStore((s) => s.duration); const isPlaying = usePlayerStore((s) => s.isPlaying); - const isMasterView = !activeCompPath || activeCompPath === "index.html"; const effectiveTimelineDuration = useMemo(() => { const maxEnd = timelineElements.length > 0 @@ -271,12 +270,12 @@ export function StudioApp() { const domEditSession = useDomEditSession({ projectId, activeCompPath, - isMasterView, compIdToSrc, captionEditMode, compositionLoading, previewIframeRef, timelineElements, + getTimelineSelectionSet: () => usePlayerStore.getState().selectedElementIds, setSelectedTimelineElementId, setTimelineSelectionSet, setRightCollapsed: panelLayout.setRightCollapsed, diff --git a/packages/studio/src/components/editor/DomEditOverlay.test.ts b/packages/studio/src/components/editor/DomEditOverlay.test.ts index 0d857d7918..a7270849cd 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.test.ts +++ b/packages/studio/src/components/editor/DomEditOverlay.test.ts @@ -281,7 +281,9 @@ describe("DomEditOverlay", () => { const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); - const iframeRef = { current: document.createElement("iframe") as HTMLIFrameElement | null }; + const iframeRef: { current: HTMLIFrameElement | null } = { + current: document.createElement("iframe"), + }; const onCanvasMouseDown = vi.fn(); const onMarqueeSelect = vi.fn(); @@ -326,6 +328,44 @@ describe("DomEditOverlay", () => { host.remove(); }); + it("starts a marquee from outside the composition frame", async () => { + const restoreRect = stubViewportRect(); + const originalPointerCapture = HTMLDivElement.prototype.setPointerCapture; + const setPointerCapture = vi.fn(); + HTMLDivElement.prototype.setPointerCapture = setPointerCapture; + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const iframeRef: { current: HTMLIFrameElement | null } = { + current: document.createElement("iframe"), + }; + + act(() => { + root.render( + React.createElement(DomEditOverlay, { + ...createOverlayProps({ + iframeRef, + selection: null, + hoverSelection: null, + onSelectionChange: () => {}, + }), + onMarqueeSelect: vi.fn(), + }), + ); + }); + await flushOverlayRaf(); + + // Negative x is outside the 0..800 composition frame but still reaches the + // overlay in a real pointer event when the user starts in the grey margin. + dispatchOverlayPointerDown(getOverlay(host), -40, 100); + expect(setPointerCapture).toHaveBeenCalledTimes(1); + + act(() => root.unmount()); + HTMLDivElement.prototype.setPointerCapture = originalPointerCapture; + restoreRect(); + host.remove(); + }); + it("does not start a drag from a stale hover target on canvas pointer-down", () => { const host = document.createElement("div"); document.body.append(host); diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts index 5f9475c5e0..3ecfc83227 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts @@ -178,6 +178,7 @@ describe("orientedOverlayRect — rotation gate (perf fix, V15 18a/18b)", () => * parent got an upright box. */ const SCALE_1_2_MATRIX = "matrix(1.2, 0, 0, 1.2, 0, 0)"; + const MIRROR_X_MATRIX = "matrix(-1, 0, 0, 1, 0, 0)"; it("sizes the box by the accumulated transform, not the element's own", () => { const { overlayEl, iframe, el } = buildHarness(); @@ -206,6 +207,24 @@ describe("orientedOverlayRect — rotation gate (perf fix, V15 18a/18b)", () => expect(rect!.angle).toBeCloseTo(30, 3); }); + it("does not misread a mirrored ancestor as a 180-degree rotation", () => { + const { overlayEl, iframe, el } = buildHarness(); + el.parentElement!.style.transform = MIRROR_X_MATRIX; + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect?.angle ?? 0).toBe(0); + }); + + it("stops transform composition at the composition root", () => { + const { overlayEl, iframe, el } = buildHarness(); + iframe.contentDocument!.body.style.transform = ROTATE_30DEG_MATRIX; + + const rect = orientedOverlayRect(overlayEl, iframe, el); + + expect(rect?.angle ?? 0).toBe(0); + }); + it("preserves an ordinary element's rotation through the group-aware entry point", () => { const { overlayEl, iframe, el } = buildHarness(); el.style.transform = ROTATE_30DEG_MATRIX; diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index 76b312ce97..8ad23ddcb0 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -150,9 +150,11 @@ function readElementTransformSnapshot( let matrix = new DOMMatrixCtor(); for (let node: HTMLElement | null = element; node; node = node.parentElement) { const transform = node === element ? cs.transform : win.getComputedStyle(node).transform; - if (!transform || transform === "none") continue; - // An ancestor applies outside, so it multiplies on the left. - matrix = new DOMMatrixCtor(transform).multiply(matrix); + if (transform && transform !== "none") { + // An ancestor applies outside, so it multiplies on the left. + matrix = new DOMMatrixCtor(transform).multiply(matrix); + } + if (node.hasAttribute("data-composition-id")) break; } return { matrix, cs }; } catch { @@ -169,7 +171,16 @@ function readElementTransformSnapshot( function rotationDegreesFromMatrix(matrix: DOMMatrix): number { const a = Number.isFinite(matrix.a) ? matrix.a : 1; const b = Number.isFinite(matrix.b) ? matrix.b : 0; - const deg = (Math.atan2(b, a) * 180) / Math.PI; + const c = Number.isFinite(matrix.c) ? matrix.c : 0; + const d = Number.isFinite(matrix.d) ? matrix.d : 1; + const fromX = (Math.atan2(b, a) * 180) / Math.PI; + const determinant = a * d - b * c; + // A reflection makes one basis direction read 180° away from the authored + // rotation. For cursor/handle orientation those directions are equivalent; + // choose the representative nearest zero instead of drawing a pure mirror's + // rotate handle on the opposite side of the element. + const fromY = (Math.atan2(-c, d) * 180) / Math.PI; + const deg = determinant < 0 && Math.abs(fromY) < Math.abs(fromX) ? fromY : fromX; return Number.isFinite(deg) ? deg : 0; } diff --git a/packages/studio/src/components/editor/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts index 27281d2a13..7a0fe732a4 100644 --- a/packages/studio/src/components/editor/groupDragMove.ts +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -26,6 +26,7 @@ export function createGroupDragMover( setDraftGroupOverlayItems: (items: GroupOverlayItem[]) => void, ) { let lastGroupPositions: Record = {}; + let lastGesture: GroupGestureState | null = null; /** Snap the group's delta to nearby edges, publishing the guides drawn for it. */ // fallow-ignore-next-line complexity @@ -36,7 +37,10 @@ export function createGroupDragMover( ) => { const sc = groupG.snapContext; if (!sc?.snapEnabled || sc.targets.length === 0) return proposed; - if (!snapEngagedForTravel(proposed.dx, proposed.dy)) return proposed; + if (!snapEngagedForTravel(proposed.dx, proposed.dy)) { + opts.snapGuidesRef.current = null; + return proposed; + } const groupBounds = resolveDomEditGroupOverlayRect(groupG.originItems.map((i) => i.rect)); if (!groupBounds) return proposed; const allTargets = sc.compositionTarget ? [...sc.targets, sc.compositionTarget] : sc.targets; @@ -67,6 +71,10 @@ export function createGroupDragMover( /** One frame of a group drag: snap the delta, redraw the boxes, move every member. */ const moveGroupDrag = (groupG: GroupGestureState, e: React.PointerEvent) => { + if (groupG !== lastGesture) { + lastGesture = groupG; + lastGroupPositions = {}; + } const { dx, dy } = snapGroupDelta(groupG, e, { dx: e.clientX - groupG.startX, dy: e.clientY - groupG.startY, diff --git a/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts b/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts deleted file mode 100644 index a65ae2d105..0000000000 --- a/packages/studio/src/components/editor/marqueeOutsideCanvas.test.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { describe, expect, it } from "vitest"; -import { rectsOverlap } from "../../utils/marqueeGeometry"; - -/** - * An element dragged past the edge of the frame sits out in the grey, and the - * rubber band refused to START there — it only began inside the composition - * rect, so the one gesture that could reach those elements could not be begun - * near them, leaving the timeline as the only way to select something plainly - * visible on screen. - * - * The collection half never had that limit: it compares rects in overlay space - * and never clipped to the frame, so a band drawn out in the grey has always - * been able to find what it covers. This pins that, including the negative - * coordinates an off-canvas element actually has. - */ -describe("marquee reaches elements outside the composition", () => { - const offCanvas = { left: -180, top: 40, width: 90, height: 40 }; - - it("covers an element sitting left of the frame", () => { - expect(rectsOverlap({ left: -220, top: 10, width: 160, height: 120 }, offCanvas)).toBe(true); - }); - - it("covers one sitting above the frame", () => { - const above = { left: 60, top: -140, width: 80, height: 50 }; - expect(rectsOverlap({ left: 20, top: -200, width: 200, height: 120 }, above)).toBe(true); - }); - - it("does not claim one the band misses", () => { - expect(rectsOverlap({ left: 400, top: 400, width: 50, height: 50 }, offCanvas)).toBe(false); - }); -}); diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts index 346acd1ae2..2a28b998d5 100644 --- a/packages/studio/src/hooks/domSelectionTimelineMirror.ts +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -5,6 +5,7 @@ import { logSelect } from "../utils/selectDebug"; interface TimelineMirrorDeps { timelineElements: TimelineElement[]; + getTimelineSelectionSet: () => ReadonlySet; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; setTimelineSelectionSet: (ids: Set) => void; } @@ -27,7 +28,12 @@ export function announceTimelineSelection( group: DomEditSelection[], primary: DomEditSelection | null, ): void { - const { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet } = deps; + const { + timelineElements, + getTimelineSelectionSet, + setSelectedTimelineElementId, + setTimelineSelectionSet, + } = deps; if (!primary) { setTimelineSelectionSet(new Set()); setSelectedTimelineElementId(null); @@ -51,10 +57,11 @@ export function announceTimelineSelection( anchor, anchorPublished: anchor != null && members.includes(anchor), }); - // Only a real multi-selection publishes members. A single selection keeps the - // older contract on purpose: anchoring with preserveSet holds a live set the - // element already belongs to (a late async primary must not collapse a group) - // and collapses otherwise, which is what a fresh click means. - if (group.length > 1) setTimelineSelectionSet(new Set(members)); + // A late async primary that already belongs to the live set must preserve the + // group. A fresh single click does not belong to it, so publish the singleton + // first; otherwise `preserveSet` clears the set and sync wipes the canvas. + if (group.length > 1 || !anchor || !getTimelineSelectionSet().has(anchor)) { + setTimelineSelectionSet(new Set(members)); + } setSelectedTimelineElementId(anchor, { preserveSet: true }); } diff --git a/packages/studio/src/hooks/useDomEditSession.test.tsx b/packages/studio/src/hooks/useDomEditSession.test.tsx index 299ea28899..e629b2d1b1 100644 --- a/packages/studio/src/hooks/useDomEditSession.test.tsx +++ b/packages/studio/src/hooks/useDomEditSession.test.tsx @@ -60,6 +60,46 @@ const capturedOnReorderShadow: { fn: ((targets: string[]) => void) | undefined } const domEditSelectionRef: { current: DomEditSelection | null } = { current: null }; const gsapCommitMutation = Object.assign(vi.fn(), { batch: vi.fn() }); +function createSessionParams( + overrides: Partial = {}, +): UseDomEditSessionParams { + return { + projectId: "proj-1", + activeCompPath: "index.html", + compIdToSrc: new Map(), + captionEditMode: false, + compositionLoading: false, + previewIframeRef: { current: null }, + timelineElements: [], + getTimelineSelectionSet: () => new Set(), + setSelectedTimelineElementId: vi.fn(), + setTimelineSelectionSet: vi.fn(), + setRightCollapsed: vi.fn(), + setRightPanelTab: vi.fn(), + showToast: vi.fn(), + refreshPreviewDocumentVersion: vi.fn(), + queueDomEditSave: async (save: () => Promise) => save(), + readProjectFile: async () => "", + writeProjectFile: async () => {}, + updateEditingFileContent: vi.fn(), + domEditSaveTimestampRef: { current: 0 }, + editHistory: { recordEdit: async () => {} }, + fileTree: [], + importedFontAssetsRef: { current: [] }, + projectDir: null, + projectIdRef: { current: "proj-1" }, + previewIframe: null, + refreshKey: 0, + previewDocumentVersion: 0, + rightPanelTab: "design", + applyStudioManualEditsToPreviewRef: { current: async () => {} }, + syncPreviewHistoryHotkey: vi.fn(), + reloadPreview: vi.fn(), + setRefreshKey: vi.fn(), + ...overrides, + }; +} + vi.mock("../utils/sdkResolverShadow", () => ({ runResolverShadow: vi.fn(), recordResolverParity: (...args: unknown[]) => recordResolverParity(...args), @@ -220,44 +260,16 @@ describe("onReorderShadow source filter", () => { const sdkSession = {} as unknown as Composition; function Probe() { - const params: UseDomEditSessionParams = { - projectId: "proj-1", - activeCompPath: "index.html", - isMasterView: false, - compIdToSrc: new Map(), - captionEditMode: false, - compositionLoading: false, - previewIframeRef: { current: null }, - timelineElements: [], - setSelectedTimelineElementId: vi.fn(), - setTimelineSelectionSet: vi.fn(), - setRightCollapsed: vi.fn(), - setRightPanelTab: vi.fn(), - showToast: vi.fn(), - refreshPreviewDocumentVersion: vi.fn(), + const params = createSessionParams({ queueDomEditSave: vi.fn(async (save: () => Promise) => save()) as ( save: () => Promise, ) => Promise, readProjectFile, writeProjectFile: vi.fn(async () => {}), - updateEditingFileContent: vi.fn(), - domEditSaveTimestampRef: { current: 0 }, editHistory: { recordEdit: vi.fn(async () => {}) }, - fileTree: [], - importedFontAssetsRef: { current: [] }, - projectDir: null, - projectIdRef: { current: "proj-1" }, - previewIframe: null, - refreshKey: 0, - previewDocumentVersion: 0, - rightPanelTab: "design", - applyStudioManualEditsToPreviewRef: { current: async () => {} }, - syncPreviewHistoryHotkey: vi.fn(), - reloadPreview: vi.fn(), - setRefreshKey: vi.fn(), sdkSession, forceReloadSdkSession: vi.fn(), - }; + }); useDomEditSession(params); return null; } @@ -319,40 +331,7 @@ describe("bulk segment ease commits", () => { | undefined; function Probe() { - const params: UseDomEditSessionParams = { - projectId: "proj-1", - activeCompPath: "index.html", - isMasterView: false, - compIdToSrc: new Map(), - captionEditMode: false, - compositionLoading: false, - previewIframeRef: { current: null }, - timelineElements: [], - setSelectedTimelineElementId: vi.fn(), - setTimelineSelectionSet: vi.fn(), - setRightCollapsed: vi.fn(), - setRightPanelTab: vi.fn(), - showToast: vi.fn(), - refreshPreviewDocumentVersion: vi.fn(), - queueDomEditSave: async (save: () => Promise) => save(), - readProjectFile: async () => "", - writeProjectFile: async () => {}, - updateEditingFileContent: vi.fn(), - domEditSaveTimestampRef: { current: 0 }, - editHistory: { recordEdit: async () => {} }, - fileTree: [], - importedFontAssetsRef: { current: [] }, - projectDir: null, - projectIdRef: { current: "proj-1" }, - previewIframe: null, - refreshKey: 0, - previewDocumentVersion: 0, - rightPanelTab: "design", - applyStudioManualEditsToPreviewRef: { current: async () => {} }, - syncPreviewHistoryHotkey: vi.fn(), - reloadPreview: vi.fn(), - setRefreshKey: vi.fn(), - }; + const params = createSessionParams(); updateSegmentEase = useDomEditSession(params).handleUpdateSegmentEase; return null; } diff --git a/packages/studio/src/hooks/useDomEditSession.ts b/packages/studio/src/hooks/useDomEditSession.ts index 5db396307d..1ad7ab3fce 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -31,12 +31,12 @@ interface RecordEditInput { export interface UseDomEditSessionParams { projectId: string | null; activeCompPath: string | null; - isMasterView: boolean; compIdToSrc: Map; captionEditMode: boolean; compositionLoading: boolean; previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; + getTimelineSelectionSet: () => ReadonlySet; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; setTimelineSelectionSet: (ids: Set) => void; setRightCollapsed: (collapsed: boolean) => void; @@ -74,12 +74,12 @@ export interface UseDomEditSessionParams { export function useDomEditSession({ projectId, activeCompPath, - isMasterView, compIdToSrc, captionEditMode, compositionLoading, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, setTimelineSelectionSet, setRightCollapsed, @@ -111,6 +111,7 @@ export function useDomEditSession({ publishSdkSession, forceReloadSdkSession, }: UseDomEditSessionParams) { + const isMasterView = !activeCompPath || activeCompPath === "index.html"; void _setRefreshKey; const { domEditSelection, @@ -139,6 +140,7 @@ export function useDomEditSession({ captionEditMode, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, setTimelineSelectionSet, setRightCollapsed, diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 02297938ec..1590117a5e 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -47,6 +47,7 @@ function renderHarness( captionEditMode: false, previewIframeRef: { current: null }, timelineElements: options.timelineElements ?? [], + getTimelineSelectionSet: () => new Set(), setSelectedTimelineElementId: timeline.setSelectedTimelineElementId, setTimelineSelectionSet: timeline.setTimelineSelectionSet, setRightCollapsed: vi.fn(), diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index e24c34c637..1c7bbb9eee 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -45,6 +45,7 @@ export interface UseDomSelectionParams { captionEditMode: boolean; previewIframeRef: React.MutableRefObject; timelineElements: TimelineElement[]; + getTimelineSelectionSet: () => ReadonlySet; setSelectedTimelineElementId: (id: string | null, options?: SelectElementOptions) => void; /** Publishes a whole multi-selection to the timeline; the anchor is set separately. */ setTimelineSelectionSet: (ids: Set) => void; @@ -109,6 +110,7 @@ export function useDomSelection({ captionEditMode, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, setTimelineSelectionSet, setRightCollapsed, @@ -149,11 +151,21 @@ export function useDomSelection({ const announceTimelineSelection = useCallback( (group: DomEditSelection[], primary: DomEditSelection | null) => announceSelectionToTimeline( - { timelineElements, setSelectedTimelineElementId, setTimelineSelectionSet }, + { + timelineElements, + getTimelineSelectionSet, + setSelectedTimelineElementId, + setTimelineSelectionSet, + }, group, primary, ), - [setSelectedTimelineElementId, setTimelineSelectionSet, timelineElements], + [ + getTimelineSelectionSet, + setSelectedTimelineElementId, + setTimelineSelectionSet, + timelineElements, + ], ); const applyDomSelection = useCallback( diff --git a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts index dfed3d834c..a61ee38fa3 100644 --- a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts +++ b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts @@ -67,6 +67,7 @@ function renderHarness(props: HarnessProps) { captionEditMode: false, previewIframeRef: { current: props.iframe }, timelineElements: props.timelineElements, + getTimelineSelectionSet: () => usePlayerStore.getState().selectedElementIds, setSelectedTimelineElementId: props.setSelectedTimelineElementId ?? vi.fn(), setTimelineSelectionSet: props.setTimelineSelectionSet ?? usePlayerStore.getState().setSelectedElementIds, @@ -239,7 +240,7 @@ describe("useDomSelection — marquee multi-select survives the late async prima iframe.remove(); }); - it("collapses the set when a late primary-set targets a non-member (fresh click)", async () => { + it("collapses the set to a fresh single-click target instead of publishing an empty set", async () => { const iframe = document.createElement("iframe"); document.body.append(iframe); const doc = iframe.contentDocument!; @@ -271,7 +272,7 @@ describe("useDomSelection — marquee multi-select survives the late async prima await pending; }); - expect(usePlayerStore.getState().selectedElementIds.size).toBe(0); + expect([...usePlayerStore.getState().selectedElementIds]).toEqual(["d"]); expect(usePlayerStore.getState().selectedElementId).toBe("d"); harness.cleanup(); iframe.remove(); diff --git a/packages/studio/src/hooks/useStudioUrlState.ts b/packages/studio/src/hooks/useStudioUrlState.ts index 2b8d02e0d0..629f04e517 100644 --- a/packages/studio/src/hooks/useStudioUrlState.ts +++ b/packages/studio/src/hooks/useStudioUrlState.ts @@ -91,6 +91,69 @@ function replaceHash(nextHash: string) { window.history.replaceState(null, "", nextHash); } +interface ResolveUrlSelectionsParams { + doc: Document; + primaryElement: HTMLElement; + selection: StudioUrlSelectionState; + group: StudioUrlSelectionTarget[]; + activeCompPath: string | null; + isCurrent: () => boolean; + buildDomSelection: UseStudioUrlStateParams["buildDomSelectionFromTarget"]; +} + +function findUrlSelectionElement( + doc: Document, + target: StudioUrlSelectionTarget, + fallbackSourceFile: string, + activeCompPath: string | null, +): HTMLElement | null { + return findElementForSelection( + doc, + { + sourceFile: target.sourceFile ?? fallbackSourceFile, + id: target.id, + selector: target.selector, + selectorIndex: target.selectorIndex, + }, + activeCompPath, + ); +} + +async function buildOptionalDomSelection( + element: HTMLElement | null, + buildDomSelection: UseStudioUrlStateParams["buildDomSelectionFromTarget"], +): Promise { + if (!element) return null; + return buildDomSelection(element, { preferClipAncestor: false }); +} + +async function resolveUrlSelections({ + doc, + primaryElement, + selection, + group, + activeCompPath, + isCurrent, + buildDomSelection, +}: ResolveUrlSelectionsParams): Promise { + const primary = await buildDomSelection(primaryElement, { preferClipAncestor: false }); + if (!isCurrent()) return null; + if (!primary) return []; + const members = [primary]; + for (const member of group) { + const element = findUrlSelectionElement( + doc, + member, + selection.sourceFile ?? "", + activeCompPath, + ); + const resolved = await buildOptionalDomSelection(element, buildDomSelection); + if (!isCurrent()) return null; + if (resolved) members.push(resolved); + } + return members; +} + export function useStudioUrlState({ projectId, activeCompPath, @@ -120,6 +183,7 @@ export function useStudioUrlState({ const [selectionHydrated, setSelectionHydrated] = useState(initialState.selection == null); const pendingSelectionRef = useRef(initialState.selection); const stableTimeRef = useRef(initialState.currentTime); + const selectionApplySeqRef = useRef(0); const buildUrlState = useCallback( (): StudioUrlState => ({ @@ -141,6 +205,7 @@ export function useStudioUrlState({ // a missing element or null selection clears the selection and returns true. const applyUrlSelection = useCallback( (selection: StudioUrlSelectionState | null): boolean => { + const applySeq = ++selectionApplySeqRef.current; if (!selection) { applyDomSelection(null, { revealPanel: false }); return true; @@ -167,31 +232,22 @@ export function useStudioUrlState({ return true; } const group = selection.group ?? []; - void (async () => { - const primary = await buildDomSelectionFromTarget(element, { preferClipAncestor: false }); + void resolveUrlSelections({ + doc, + primaryElement: element, + selection, + group, + activeCompPath, + isCurrent: () => applySeq === selectionApplySeqRef.current, + buildDomSelection: buildDomSelectionFromTarget, + }).then((members) => { + if (!members) return; + const primary = members[0]; if (!primary) return applyDomSelection(null, { revealPanel: false }); if (group.length === 0) return applyDomSelection(primary, { revealPanel: false }); - // Restore the whole multi-selection, primary first so it stays the anchor. - // Members whose element is gone are dropped rather than failing the rest. - const members = [primary]; - for (const member of group) { - const memberEl = findElementForSelection( - doc, - { - sourceFile: member.sourceFile ?? selection.sourceFile ?? "", - id: member.id, - selector: member.selector, - selectorIndex: member.selectorIndex, - }, - activeCompPath, - ); - const resolved = memberEl - ? await buildDomSelectionFromTarget(memberEl, { preferClipAncestor: false }) - : null; - if (resolved) members.push(resolved); - } + // Missing group members are dropped without failing the rest. applyMarqueeSelection(members, false); - })(); + }); return true; }, [ diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts index 72bb69b8fc..e0f1949c28 100644 --- a/packages/studio/src/utils/dragDebug.ts +++ b/packages/studio/src/utils/dragDebug.ts @@ -13,8 +13,10 @@ let moveN = 0; /** Per-pointermove logging, throttled: the first move then every 8th. */ export function logDragMove(data: Record): void { - moveN += 1; - if (moveN % 8 === 1) logDrag("move", { n: moveN, ...data }); + logDrag("move", () => { + moveN += 1; + return moveN % 8 === 1 ? { n: moveN, ...data } : null; + }); } export function resetDragMoveLog(): void { @@ -74,10 +76,23 @@ export function logDragSettle( stage: string, elements: Array<{ key: string; element: HTMLElement }>, ): void { - logDrag(stage, { at: readDragPositions(elements) }); - const win = elements[0]?.element.ownerDocument.defaultView; - if (!win) return; - win.setTimeout(() => logDrag(`${stage}+120ms`, { at: readDragPositions(elements) }), 120); - win.setTimeout(() => logDrag(`${stage}+400ms`, { at: readDragPositions(elements) }), 400); - win.setTimeout(() => logDrag(`${stage}+900ms`, { at: readDragPositions(elements) }), 900); + logDrag(stage, () => { + const at = readDragPositions(elements); + const win = elements[0]?.element.ownerDocument.defaultView; + if (win) { + win.setTimeout( + () => logDrag(`${stage}+120ms`, () => ({ at: readDragPositions(elements) })), + 120, + ); + win.setTimeout( + () => logDrag(`${stage}+400ms`, () => ({ at: readDragPositions(elements) })), + 400, + ); + win.setTimeout( + () => logDrag(`${stage}+900ms`, () => ({ at: readDragPositions(elements) })), + 900, + ); + } + return { at }; + }); } diff --git a/packages/studio/src/utils/resizeDebug.ts b/packages/studio/src/utils/resizeDebug.ts index 1e6172ce3f..01b41e1fe7 100644 --- a/packages/studio/src/utils/resizeDebug.ts +++ b/packages/studio/src/utils/resizeDebug.ts @@ -1,33 +1,18 @@ // Resize/gesture diagnostics — grep [hf-resize]. Off by default; opt in per // session with `localStorage.setItem("hf-resize-debug", "1")` (then reload). -// Granular per-move/per-gesture tracing that complements the always-on -// [hf-commit] transaction telemetry in gestureTransaction.ts. -let moveN = 0; -let enabled: boolean | null = null; +// Granular per-move/per-gesture tracing for resize investigation. +import { makeStudioDebugLogger } from "./studioDebug"; -function isEnabled(): boolean { - if (enabled === null) { - try { - enabled = localStorage.getItem("hf-resize-debug") === "1"; - } catch { - enabled = false; - } - } - return enabled; -} +export const logResize = makeStudioDebugLogger("resize"); -export function logResize(stage: string, data: Record): void { - if (!isEnabled()) return; - console.log( - `[hf-resize] ${JSON.stringify({ stage, t: Math.round(performance.now()), ...data })}`, - ); -} +let moveN = 0; /** Per-pointermove logging, throttled: first move then every 8th. */ export function logResizeMove(data: Record): void { - if (!isEnabled()) return; - moveN += 1; - if (moveN % 8 === 1) logResize("move", { n: moveN, ...data }); + logResize("move", () => { + moveN += 1; + return moveN % 8 === 1 ? { n: moveN, ...data } : null; + }); } export function resetResizeMoveLog(): void { @@ -36,11 +21,10 @@ export function resetResizeMoveLog(): void { /** Snapshot the element's live geometry now and again after 200ms (jump detector). */ export function logResizeSettle(el: HTMLElement, tag: string): void { - if (!isEnabled()) return; - const snap = (phase: string) => { + const snapshot = (phase: string) => { const r = el.getBoundingClientRect(); const cs = el.ownerDocument.defaultView?.getComputedStyle(el); - logResize("settle", { + return { tag, phase, rect: { x: r.x, y: r.y, w: r.width, h: r.height }, @@ -48,8 +32,14 @@ export function logResizeSettle(el: HTMLElement, tag: string): void { cssH: cs?.height, transform: cs?.transform, inlineStyle: el.getAttribute("style"), - }); + }; }; - snap("t0"); - setTimeout(() => snap("t200"), 200); + logResize("settle", () => { + const current = snapshot("t0"); + el.ownerDocument.defaultView?.setTimeout( + () => logResize("settle", () => snapshot("t200")), + 200, + ); + return current; + }); } diff --git a/packages/studio/src/utils/studioDebug.test.ts b/packages/studio/src/utils/studioDebug.test.ts new file mode 100644 index 0000000000..89731749c7 --- /dev/null +++ b/packages/studio/src/utils/studioDebug.test.ts @@ -0,0 +1,32 @@ +// @vitest-environment happy-dom + +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { makeStudioDebugLogger } from "./studioDebug"; + +describe("makeStudioDebugLogger", () => { + beforeEach(() => { + localStorage.clear(); + vi.restoreAllMocks(); + }); + + it("does no console work while its channel is disabled", () => { + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + const buildDetails = vi.fn(() => ({ expensive: true })); + + makeStudioDebugLogger("commit")("persisted", buildDetails); + + expect(buildDetails).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + it("logs only after its exact channel is enabled", () => { + localStorage.setItem("hf-commit-debug", "1"); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + makeStudioDebugLogger("commit")("persisted", { mutations: 2 }); + + expect(log).toHaveBeenCalledOnce(); + expect(log.mock.calls[0]?.[0]).toContain('[hf-commit] {"stage":"persisted"'); + expect(log.mock.calls[0]?.[0]).toContain('"mutations":2'); + }); +}); diff --git a/packages/studio/src/utils/studioDebug.ts b/packages/studio/src/utils/studioDebug.ts index 1a9bda9c4d..a30b8f9bbe 100644 --- a/packages/studio/src/utils/studioDebug.ts +++ b/packages/studio/src/utils/studioDebug.ts @@ -1,12 +1,13 @@ // Opt-in diagnostic channels — one per question worth tracing, all off by // default. Turn one on for the session with `localStorage.setItem("hf--debug", // "1")` and reload, then grep the console for `[hf-]`. +// Live channels: reload, select, drag, resize. // // These exist because the interesting failures here are decisions, not crashes: // a preview that reloads when it should not, a shift-click that selects nothing. // Nothing is thrown and nothing is logged by default, so without a trace of the // decision the only way to find the cause is to guess. -type DebugDetails = Record | (() => Record); +type DebugDetails = Record | (() => Record | null); type DebugLogger = (stage: string, data?: DebugDetails) => void; export function makeStudioDebugLogger(name: string): DebugLogger { @@ -21,6 +22,7 @@ export function makeStudioDebugLogger(name: string): DebugLogger { } if (!enabled) return; const details = typeof data === "function" ? data() : data; + if (!details) return; console.log( `[hf-${name}] ${JSON.stringify({ stage, t: Math.round(performance.now()), ...details })}`, ); diff --git a/packages/studio/src/utils/studioUrlState.test.ts b/packages/studio/src/utils/studioUrlState.test.ts index e23ec3c287..19530c2ffc 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -222,6 +222,56 @@ describe("studio url state", () => { harness.unmount(); }); + it("does not let an older async URL selection overwrite a newer hash", async () => { + const previewDoc = document.implementation.createHTMLDocument("preview"); + const firstElement = previewDoc.createElement("div"); + firstElement.id = "first"; + const secondElement = previewDoc.createElement("div"); + secondElement.id = "second"; + previewDoc.body.append(firstElement, secondElement); + const first = { element: firstElement, id: "first", sourceFile: "index.html" }; + const second = { element: secondElement, id: "second", sourceFile: "index.html" }; + let resolveFirst = (_selection: typeof first) => undefined; + const firstResolution = new Promise((resolve) => { + resolveFirst = resolve; + }); + const applyDomSelection = vi.fn(); + const harness = renderStudioUrlStateHarness({ + previewIframeRef: { current: previewIframeFor(previewDoc) }, + applyDomSelection, + buildDomSelectionFromTarget: (target) => + target === firstElement ? firstResolution : Promise.resolve(second), + initialState: { + activeCompPath: null, + currentTime: null, + rightPanelTab: null, + rightCollapsed: null, + timelineVisible: null, + selection: null, + }, + }); + + act(() => { + window.history.replaceState(null, "", "#project/demo?v=1&selId=first"); + window.dispatchEvent(new HashChangeEvent("hashchange")); + window.history.replaceState(null, "", "#project/demo?v=1&selId=second"); + window.dispatchEvent(new HashChangeEvent("hashchange")); + }); + await act(async () => { + await Promise.resolve(); + }); + expect(applyDomSelection).toHaveBeenCalled(); + expect(applyDomSelection.mock.calls.every(([selection]) => selection === second)).toBe(true); + const appliedBeforeOlderResolution = applyDomSelection.mock.calls.length; + + await act(async () => { + resolveFirst(first); + await firstResolution; + }); + expect(applyDomSelection).toHaveBeenCalledTimes(appliedBeforeOlderResolution); + harness.unmount(); + }); + it("builds a project hash with persisted studio state", () => { expect( buildStudioHash("demo", { From 59e864cd4250621509851e9951f2f4e0becb8423 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 13:46:15 -0400 Subject: [PATCH 16/26] fix(studio): stabilize selection store reads --- packages/studio/src/App.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index da0149ba03..199971e079 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -65,6 +65,7 @@ import { readStudioUrlStateFromWindow, resolveMasterCompositionPath, } from "./utils/studioUrlState"; +const getTimelineSelectionSet = () => usePlayerStore.getState().selectedElementIds; // fallow-ignore-next-line complexity export function StudioApp() { const { projectId, resolving, waitingForServer } = useServerConnection(); @@ -275,7 +276,7 @@ export function StudioApp() { compositionLoading, previewIframeRef, timelineElements, - getTimelineSelectionSet: () => usePlayerStore.getState().selectedElementIds, + getTimelineSelectionSet, setSelectedTimelineElementId, setTimelineSelectionSet, setRightCollapsed: panelLayout.setRightCollapsed, From 560ff338dbd1ed972e4c98dcaf9986d02cc7f648 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 13:58:47 -0400 Subject: [PATCH 17/26] fix(studio): preserve canvas-only group anchors --- .../src/hooks/domSelectionTimelineMirror.ts | 16 +++++++---- .../studio/src/hooks/useDomSelection.test.ts | 27 +++++++++++++++++++ packages/studio/src/hooks/useDomSelection.ts | 6 +++-- 3 files changed, 42 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts index 2a28b998d5..f67577925a 100644 --- a/packages/studio/src/hooks/domSelectionTimelineMirror.ts +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -48,20 +48,26 @@ export function announceTimelineSelection( ); const members = group.map(timelineIdFor).filter((id): id is string => Boolean(id)); const anchor = timelineIdFor(primary); + const publishedMembers = new Set(members); + if (anchor) publishedMembers.add(anchor); + const timelineAnchor = anchor ?? members[0] ?? null; // A member with no timeline row of its own resolves to null and is dropped here, // so a group can announce fewer ids than it has — or none, which reads back as an // empty selection and takes the canvas selection with it. logSelect("announce", { group: group.length, - published: members.length, + published: publishedMembers.size, anchor, - anchorPublished: anchor != null && members.includes(anchor), + anchorPublished: anchor != null && publishedMembers.has(anchor), }); + // A canvas target can be editable without owning a timeline row. Preserve that + // canvas-only selection when the timeline has nothing truthful to represent. + if (!timelineAnchor) return; // A late async primary that already belongs to the live set must preserve the // group. A fresh single click does not belong to it, so publish the singleton // first; otherwise `preserveSet` clears the set and sync wipes the canvas. - if (group.length > 1 || !anchor || !getTimelineSelectionSet().has(anchor)) { - setTimelineSelectionSet(new Set(members)); + if (group.length > 1 || !getTimelineSelectionSet().has(timelineAnchor)) { + setTimelineSelectionSet(publishedMembers); } - setSelectedTimelineElementId(anchor, { preserveSet: true }); + setSelectedTimelineElementId(timelineAnchor, { preserveSet: true }); } diff --git a/packages/studio/src/hooks/useDomSelection.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 1590117a5e..4b407c35e4 100644 --- a/packages/studio/src/hooks/useDomSelection.test.ts +++ b/packages/studio/src/hooks/useDomSelection.test.ts @@ -144,6 +144,33 @@ describe("useDomSelection marquee", () => { }); harness.cleanup(); }); + + it("uses a surviving group member as the timeline anchor when the canvas primary has no row", () => { + const canvasOnly = document.createElement("div"); + canvasOnly.id = "canvas-only"; + const card = document.createElement("div"); + card.id = "card"; + document.body.append(canvasOnly, card); + const harness = renderHarness( + { activeCompPath: "index.html", projectId: "project-1", refreshKey: 0 }, + { timelineElements: [timelineElement("card")] }, + ); + + act(() => + harness + .current() + .applyMarqueeSelection( + [makeSelection("Canvas only", canvasOnly), makeSelection("Card", card)], + false, + ), + ); + + expect(harness.timeline.setTimelineSelectionSet).toHaveBeenCalledWith(new Set(["card"])); + expect(harness.timeline.setSelectedTimelineElementId).toHaveBeenCalledWith("card", { + preserveSet: true, + }); + harness.cleanup(); + }); }); /** diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 1c7bbb9eee..115d54fca2 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -482,10 +482,12 @@ export function useDomSelection({ [activeCompPath, announceTimelineSelection, buildDomSelectionFromTarget, previewIframeRef], ); - refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; - // ── Effects ── + useEffect(() => { + refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; + }, [refreshDomEditGroupSelectionsFromPreview]); + // Clear hover unconditionally on composition/project/preview change // eslint-disable-next-line no-restricted-syntax useEffect(() => { From 30b651335afe5105c5dbdfe8a99bcc43d284c5b7 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 15:51:27 -0700 Subject: [PATCH 18/26] fix(studio): stop a group drag from jumping one element back Dragging several elements at once and dropping them made one of them snap back to where it started for a frame or two, then jump forward again. Each member of the group is written separately, and every write patched the live GSAP tween in place and then seeked the player. A seek re-renders the WHOLE timeline, not the tween that changed, so the members still queued behind that write got repainted from their un-patched tweens: back to their pre-drag position, where they sat until their own write landed. Only members whose tween actually renders at the playhead showed it, which is why a group of three flashed one element and left the others still. The group commit now defers the seek for every member but the last, so the queued members keep the transform the gesture left on them and the whole group repaints once, from the fully patched timeline. --- .../src/components/editor/manualOffsetDrag.ts | 2 + .../studio/src/hooks/gsapRuntimePatch.test.ts | 43 +++++++++++++++++++ packages/studio/src/hooks/gsapRuntimePatch.ts | 12 +++++- .../studio/src/hooks/gsapScriptCommitTypes.ts | 10 +++++ .../studio/src/hooks/useGsapAwareEditing.ts | 12 +++++- .../src/hooks/useGsapScriptCommits.test.tsx | 10 ++++- .../studio/src/hooks/useGsapScriptCommits.ts | 4 ++ 7 files changed, 89 insertions(+), 4 deletions(-) diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index 731c41813f..bb4b7df792 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -524,6 +524,7 @@ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[] } } +/** Teardown after a COMMITTED drag. */ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { endStudioManualEditGesture(member.element, member.gestureToken); @@ -552,6 +553,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } +/** Release the timelines this gesture paused, re-rendering at the playhead. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.test.ts b/packages/studio/src/hooks/gsapRuntimePatch.test.ts index 1a17f7e063..e34d293a41 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.test.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.test.ts @@ -523,3 +523,46 @@ describe("patchRuntimeTweenInPlace — composition isolation", () => { expect(otherTween.invalidate).not.toHaveBeenCalled(); }); }); + +describe("patchRuntimeTweenInPlace — deferSeek", () => { + /** + * A group drag commits one member at a time. Each in-place patch used to seek, + * and a seek re-renders the WHOLE timeline — so every member still queued behind + * the current one got repainted from its un-patched tween, back to where it sat + * before the drag, and stayed there until its own patch landed. That is the jump. + */ + it("does not seek while a group commit is still writing its other members", () => { + const a = { id: "a" }; + const rendered = { a: 0, b: 0 }; + const tweenA = makeTween({ vars: { x: 0 }, targetIds: ["a"], duration: 0 }, a); + const tweenB = makeTween({ vars: { x: 0 }, targetIds: ["b"], duration: 0 }, a); + const { iframe, seek } = fakeIframe(a, [tweenA, tweenB], { + onSeek: () => { + rendered.a = tweenA.vars.x as number; + rendered.b = tweenB.vars.x as number; + }, + }); + + const first = patchRuntimeTweenInPlace( + iframe, + "#a", + { kind: "set", props: { x: 500 } }, + undefined, + true, + ); + + expect(first).toBe(true); + expect(tweenA.vars.x).toBe(500); + // No repaint yet: "b" keeps the transform the gesture left on it instead of + // being rendered from its own tween, which still holds the pre-drag value. + expect(seek).not.toHaveBeenCalled(); + expect(rendered).toEqual({ a: 0, b: 0 }); + + tweenB.vars.x = 600; + const last = patchRuntimeTweenInPlace(iframe, "#a", { kind: "set", props: { x: 500 } }); + + expect(last).toBe(true); + expect(seek).toHaveBeenCalledTimes(1); + expect(rendered).toEqual({ a: 500, b: 600 }); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimePatch.ts b/packages/studio/src/hooks/gsapRuntimePatch.ts index de2c6480e4..9d32650387 100644 --- a/packages/studio/src/hooks/gsapRuntimePatch.ts +++ b/packages/studio/src/hooks/gsapRuntimePatch.ts @@ -277,12 +277,16 @@ function applyChange(tween: RuntimeTween, change: RuntimeTweenChange): boolean { /** * Edit one tween in `window.__timelines` in place + re-seek to the current playhead. * Returns `true` on a confident patch, `false` otherwise (caller soft-reloads). + * + * `deferSeek` skips the re-render, for a caller patching several tweens in a row + * that will render once after the last one. */ export function patchRuntimeTweenInPlace( iframe: HTMLIFrameElement | null, selector: string, change: RuntimeTweenChange, compositionId?: string, + deferSeek = false, ): boolean { if (!iframe) return false; // A base `gsap.set` has no timeline tween to resolve — apply the value straight @@ -312,7 +316,13 @@ export function patchRuntimeTweenInPlace( if (change.kind !== "keyframe-rebuild") { tween.invalidate?.(); } - seekToCurrent(iframe, timeline); + // A seek re-renders the WHOLE timeline, not just the tween we patched. Under a + // multi-element commit that is a visible jump: the members still queued behind + // this one get repainted from their un-patched tweens, back to where they were + // before the gesture, and stay there until their own patch lands. Deferring + // leaves them showing the gesture's own transform, and the caller's last patch + // seeks once for the whole group. + if (!deferSeek) seekToCurrent(iframe, timeline); return true; } catch { return false; diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index 929e79b8ec..c36656e874 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -22,6 +22,16 @@ export interface CommitMutationOptions { coalesceMs?: number; softReload?: boolean; skipReload?: boolean; + /** + * Write the source but leave the preview alone; the caller renders once when it + * is done. For a multi-write action like a group drag, rendering after each + * write shows a source where the members not yet written still hold their old + * values, so they snap back until their own write lands. This also defers the + * in-place runtime patch's seek, which re-renders the whole timeline and repaints + * the queued members the same way. Unlike `skipReload` this changes nothing about + * error handling — a failed write still throws. + */ + deferPreviewSync?: boolean; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 0e84c15826..5e85d00765 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -155,11 +155,20 @@ export function useGsapAwareEditing({ // it survives the N sequential server round-trips) onto each commit — // otherwise each member records its own entry and it takes N presses to undo. const coalesceKey = `group-drag:${++groupDragCommitCounter}`; + // Members are written one at a time, and a write that re-renders the preview + // re-runs the whole script — which still holds the OLD position of every + // member not yet written. Those members snap back to where they started and + // stay there until their own write lands, which is the single element seen + // jumping mid-commit while the rest of the group sat still. The drafted + // positions are already on screen, so holding the render until the last + // member has been written costs nothing and never shows a half-moved group. + let renderOnCommit = false; const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => gsapCommitMutation(selection, mutation, { ...options, coalesceKey, coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, }); const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before @@ -185,7 +194,8 @@ export function useGsapAwareEditing({ throw error; } } - for (const { selection, next } of updates) { + for (const [index, { selection, next }] of updates.entries()) { + renderOnCommit = index === updates.length - 1; try { const outcome = await tryGsapDragIntercept( selection, diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 6236d032ab..b5707e3a7d 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -76,7 +76,10 @@ describe("applyPreviewSync", () => { expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { kind: "set", props: { x: 10 }, - }); + }, + undefined, + false, + ); expect(applySoftReload).not.toHaveBeenCalled(); expect(reloadPreview).not.toHaveBeenCalled(); }); @@ -341,7 +344,10 @@ describe("runCommit — instantPatch wiring", () => { expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { kind: "set", props: { x: 485, y: 311 }, - }); + }, + undefined, + false, + ); expect(deps.reloadPreview).not.toHaveBeenCalled(); }); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index f1a21a3afe..74a524fd4d 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -254,6 +254,8 @@ export function applyPreviewSync( iframe, options.instantPatch.selector, options.instantPatch.change, + undefined, + options.deferPreviewSync === true, ); // Patched in place — element is already correct on screen; no reload needed. if (patched) return; @@ -262,6 +264,8 @@ export function applyPreviewSync( trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); // Fall through to the soft/full reload path below. } + // Written, but the caller has more writes to make and will render after the last. + if (options.deferPreviewSync) return; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ From a831836dbbb3200943a0986bf0421e25cbe1120b Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 16:03:14 -0700 Subject: [PATCH 19/26] perf(studio): commit a group drag in one request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dragging N elements cost N writes and 9 reads for a three-element group: each member fetched the composition's parse to preflight, fetched it again to resolve its tween, then wrote the file on its own round trip. Every one of those writes re-read, re-parsed and re-serialized the whole composition. Three changes, same behaviour: - The parse endpoint shares an in-flight request per file, so callers asking for the same composition at the same moment get one request. Only overlapping calls share — the entry is dropped as soon as it settles, so a read after a write still gets a fresh parse. - The group preflight runs its members together instead of one at a time. A preflight writes nothing, so there is nothing to order. - Members' mutations are queued and sent as one batch write. Anything that re-reads the file flushes the queue first, so a member resolving a shared or stale tween never reads a composition missing writes it is about to build on. The batch carries each member's runtime patch, and only the last one re-renders. A three-element group drag now issues 2 reads and 1 write, down from 9 and 3. --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 8 ++ .../src/hooks/keyframeCacheAstLoad.test.ts | 78 +++++++++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 27 ++++- .../studio/src/hooks/useGsapAwareEditing.ts | 99 ++++++++++++++----- .../src/hooks/useGsapScriptCommits.test.tsx | 79 +++++++++++++-- .../studio/src/hooks/useGsapScriptCommits.ts | 32 +++--- 6 files changed, 276 insertions(+), 47 deletions(-) create mode 100644 packages/studio/src/hooks/keyframeCacheAstLoad.test.ts diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index c36656e874..ed9f4a6ae4 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -49,6 +49,14 @@ export interface CommitMutationOptions { * existing soft/full reload path. Structural edits omit this and reload as before. */ instantPatch?: { selector: string; change: RuntimeTweenChange }; + /** + * The same fast path for a batched commit: one patch per element the batch + * wrote, applied in order. All of them must land for the reload to be skipped + * — one that can't be applied leaves the preview half-patched, so the whole + * batch falls back to the reload. Only the last patch re-renders (see + * `deferSeek`), so a ten-element batch repaints once. + */ + instantPatches?: Array<{ selector: string; change: RuntimeTweenChange }>; } export interface CommitMutationCall { diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts new file mode 100644 index 0000000000..d9a1081df1 --- /dev/null +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { fetchParsedAnimations } from "./keyframeCacheAstLoad"; + +/** + * Parsing a composition is a whole-file read + parse on the server, and a + * multi-element action asks for the same file once per element. Callers that + * overlap in time share one request; a caller that comes after the last one + * settled does not, so a parse issued after a write is never served a + * pre-write answer. + */ +describe("fetchParsedAnimations — in-flight sharing", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + function stubFetch(): { calls: () => number; settle: () => void } { + let calls = 0; + const pending: Array<() => void> = []; + vi.stubGlobal("fetch", () => { + calls++; + return new Promise((resolve) => { + pending.push(() => + resolve({ + ok: true, + json: () => Promise.resolve({ animations: [{ id: "a", targetSelector: "#a" }] }), + } as Response), + ); + }); + }); + return { + calls: () => calls, + settle: () => { + for (const release of pending.splice(0, pending.length)) release(); + }, + }; + } + + it("serves overlapping reads of one file from a single request", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "index.html"), + ]; + fetchStub.settle(); + const results = await Promise.all(pending); + + expect(fetchStub.calls()).toBe(1); + expect(results.map((parsed) => parsed?.animations.length)).toEqual([1, 1, 1]); + }); + + it("does not share across files", async () => { + const fetchStub = stubFetch(); + + const pending = [ + fetchParsedAnimations("p", "index.html"), + fetchParsedAnimations("p", "other.html"), + ]; + fetchStub.settle(); + await Promise.all(pending); + + expect(fetchStub.calls()).toBe(2); + }); + + it("re-requests once the previous read has settled", async () => { + const fetchStub = stubFetch(); + + const first = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await first; + const second = fetchParsedAnimations("p", "index.html"); + fetchStub.settle(); + await second; + + expect(fetchStub.calls()).toBe(2); + }); +}); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index 71e38bbb61..e4abc42525 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -42,7 +42,32 @@ function hasAnimations(value: unknown): value is ParsedGsapAnimations { ); } -export async function fetchParsedAnimations( +/** + * Requests for the same file that overlap in time, keyed `projectId|sourceFile`. + * + * Every parse re-reads and re-parses the whole composition server-side, and a + * multi-element action asks for the same file once per element. Sharing the + * in-flight promise makes that one request. Only OVERLAPPING calls share: the + * entry is dropped the moment it settles, so a call made after a write still + * gets a fresh parse. + */ +const inFlightParses = new Map>(); + +export function fetchParsedAnimations( + projectId: string, + sourceFile: string, +): Promise { + const key = `${projectId}|${sourceFile}`; + const inFlight = inFlightParses.get(key); + if (inFlight) return inFlight; + const request = requestParsedAnimations(projectId, sourceFile).finally(() => { + inFlightParses.delete(key); + }); + inFlightParses.set(key, request); + return request; +} + +async function requestParsedAnimations( projectId: string, sourceFile: string, ): Promise { diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 5e85d00765..d23ae3b020 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -24,7 +24,11 @@ import { useGsapSaveFailureTelemetry, useSafeGsapCommitMutation, } from "./useSafeGsapCommitMutation"; -import type { CommitMutation } from "./gsapScriptCommitTypes"; +import type { + CommitMutation, + CommitMutationCall, + CommitMutationOptions, +} from "./gsapScriptCommitTypes"; import { setElementGsapPosition } from "../utils/elementGsap"; import { logResize, logResizeSettle } from "../utils/resizeDebug"; import type { DomEditGroupPathOffsetCommit } from "../components/editor/DomEditOverlay"; @@ -163,37 +167,63 @@ export function useGsapAwareEditing({ // positions are already on screen, so holding the render until the last // member has been written costs nothing and never shows a half-moved group. let renderOnCommit = false; - const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => - gsapCommitMutation(selection, mutation, { - ...options, - coalesceKey, - coalesceMs: Number.POSITIVE_INFINITY, - deferPreviewSync: !renderOnCommit, + const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ + ...options, + coalesceKey, + coalesceMs: Number.POSITIVE_INFINITY, + deferPreviewSync: !renderOnCommit, + }); + // Every member writes the same file. Queue their mutations and send them as + // ONE request instead of one round trip per member: the server reads, parses + // and writes the composition once, and the preview patches once. + const queued: CommitMutationCall[] = []; + const flushQueued = async () => { + if (queued.length === 0) return; + const calls = queued.splice(0, queued.length); + if (!gsapCommitMutation.batch) { + for (const call of calls) { + await gsapCommitMutation(call.selection, call.mutation, call.options); + } + return; + } + await gsapCommitMutation.batch(calls, { + ...(calls.at(-1)?.options ?? { label: "Move animated layer (group)" }), + label: "Move animated layer (group)", }); + }; + const coalescedCommit: typeof gsapCommitMutation = (selection, mutation, options) => { + queued.push({ selection, mutation, options: withGroupOptions(options) }); + return Promise.resolve(); + }; const preflightAnimations = new Map(); // Editability is user-atomic: prove every member can be written before // the first source mutation. Network failures after this point retain the // existing multi-request semantics, but a blocked member can never leave // earlier siblings partially moved. - for (const { selection } of updates) { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } - } + // Every member reads the same file, and a preflight writes nothing — so run + // them together. The parse layer shares one in-flight request per file, which + // turns N sequential round trips into one. + await Promise.all( + updates.map(async ({ selection }) => { + try { + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); + } catch (error) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + throw error; + } + }), + ); for (const [index, { selection, next }] of updates.entries()) { renderOnCommit = index === updates.length - 1; try { @@ -203,7 +233,13 @@ export function useGsapAwareEditing({ preflightAnimations.get(selection) ?? [], previewIframeRef.current, coalescedCommit, - makeFetchFallback(selection), + // The intercept re-reads the file to resolve a stale or shared tween. + // Anything already queued has to be on disk before that read, or it + // resolves against a file missing writes it is about to build on. + async () => { + await flushQueued(); + return makeFetchFallback(selection)(); + }, { preflightPassed: true }, ); assertGsapEditPersisted(outcome); @@ -212,6 +248,15 @@ export function useGsapAwareEditing({ throw error; } } + try { + await flushQueued(); + } catch (error) { + const selection = updates.at(-1)?.selection; + if (selection) { + trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); + } + throw error; + } }, [gsapCommitMutation, previewIframeRef, makeFetchFallback, trackGsapInteractionFailure], ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index b5707e3a7d..76ca2129da 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -73,10 +73,13 @@ describe("applyPreviewSync", () => { syncDragPreview(result(), reloadPreview); - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 10 }, - }, + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 10 }, + }, undefined, false, ); @@ -84,6 +87,63 @@ describe("applyPreviewSync", () => { expect(reloadPreview).not.toHaveBeenCalled(); }); + it("instantPatches: patches every element the batch wrote, rendering once at the end", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + { selector: "#c", change: { kind: "set" as const, props: { x: 3 } } }, + ], + }, + reloadPreview, + ); + + // Only the last patch re-renders — the earlier two defer their seek, so the + // group repaints once instead of once per member. + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#a", true], + ["#b", true], + ["#c", false], + ]); + expect(applySoftReload).not.toHaveBeenCalled(); + expect(reloadPreview).not.toHaveBeenCalled(); + }); + + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { + patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + instantPatches: [ + { selector: "#a", change: { kind: "set" as const, props: { x: 1 } } }, + { selector: "#b", change: { kind: "set" as const, props: { x: 2 } } }, + ], + }, + reloadPreview, + ); + + // A half-patched preview is worse than a reloaded one: "#a" landed, "#b" did + // not, so the reload repaints both from the written source. + expect(applySoftReload).toHaveBeenCalled(); + expect(trackStudioEvent).toHaveBeenCalledWith("gsap_instant_patch_fallback", { + selector: "#b", + }); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -341,10 +401,13 @@ describe("runCommit — instantPatch wiring", () => { // The file already matched (changed:false) but the runtime patch deferred // from the paired first commit must still land. - expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith(FAKE_IFRAME, "#a", { - kind: "set", - props: { x: 485, y: 311 }, - }, + expect(patchRuntimeTweenInPlace).toHaveBeenCalledWith( + FAKE_IFRAME, + "#a", + { + kind: "set", + props: { x: 485, y: 311 }, + }, undefined, false, ); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 74a524fd4d..fb042945d6 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -249,19 +249,24 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - if (options.instantPatch) { - const patched = patchRuntimeTweenInPlace( - iframe, - options.instantPatch.selector, - options.instantPatch.change, - undefined, - options.deferPreviewSync === true, + const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + if (patches.length > 0) { + const deferSeek = options.deferPreviewSync === true; + const missed = patches.find( + (patch, index) => + !patchRuntimeTweenInPlace( + iframe, + patch.selector, + patch.change, + undefined, + deferSeek || index < patches.length - 1, + ), ); - // Patched in place — element is already correct on screen; no reload needed. - if (patched) return; + // Patched in place — elements are already correct on screen; no reload needed. + if (!missed) return; // The instant path couldn't patch in place — record the fallback so we can // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: options.instantPatch.selector }); + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); // Fall through to the soft/full reload path below. } // Written, but the caller has more writes to make and will render after the last. @@ -360,7 +365,12 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra ); if (!result) return; options.onResult?.(result); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, options); + // Each call brings its own fast-path patch; the batch wrote them all, so the + // preview sync applies them all rather than just the last call's. + const instantPatches = calls + .map(({ options: callOptions }) => callOptions.instantPatch) + .filter((patch) => patch !== undefined); + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping From efb86d4ce1c35e53f19680b6aed460e4612ff338 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 13:27:29 -0400 Subject: [PATCH 20/26] fix(studio): harden batched drag commits --- .../src/components/editor/manualOffsetDrag.ts | 3 +- .../src/hooks/gestureTransaction.test.ts | 19 ++---- .../studio/src/hooks/gestureTransaction.ts | 27 ++------ .../src/hooks/keyframeCacheAstLoad.test.ts | 30 +++++++++ .../studio/src/hooks/keyframeCacheAstLoad.ts | 5 +- .../hooks/useGsapAnimationFetchFallback.ts | 7 +- .../src/hooks/useGsapAwareEditing.test.tsx | 36 +++++++++++ .../studio/src/hooks/useGsapAwareEditing.ts | 64 ++++++++++++------- .../useGsapInteractionFailureTelemetry.ts | 10 +-- .../src/hooks/useGsapScriptCommits.test.tsx | 25 ++++++++ .../studio/src/hooks/useGsapScriptCommits.ts | 8 ++- packages/studio/src/utils/studioDebug.ts | 2 +- 12 files changed, 169 insertions(+), 67 deletions(-) diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index bb4b7df792..75609a73c3 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -517,6 +517,7 @@ function restoreManualOffsetDragMember(member: ManualOffsetDragMember): void { endStudioManualEditGesture(member.element, member.gestureToken); } +/** Roll back a FAILED drag to the exact gesture-start state. */ export function restoreManualOffsetDragMembers(members: ManualOffsetDragMember[]): void { for (const member of members) { restoreManualOffsetDragMember(member); @@ -553,7 +554,7 @@ export function endManualOffsetDragMembers(members: ManualOffsetDragMember[]): v } } -/** Release the timelines this gesture paused, re-rendering at the playhead. */ +/** Shared timeline teardown for either the committed or restored path. */ export function resumeGsapTimelines(element: HTMLElement): void { const ids = element.getAttribute("data-hf-drag-paused-timelines"); element.removeAttribute("data-hf-drag-paused-timelines"); diff --git a/packages/studio/src/hooks/gestureTransaction.test.ts b/packages/studio/src/hooks/gestureTransaction.test.ts index 3cc8e45530..8ff3ff28f8 100644 --- a/packages/studio/src/hooks/gestureTransaction.test.ts +++ b/packages/studio/src/hooks/gestureTransaction.test.ts @@ -36,6 +36,7 @@ function runTwoMutationTransaction( describe("runGestureTransaction", () => { beforeEach(() => { trackStudioEventMock.mockReset(); + localStorage.clear(); }); it("settles synchronously before persist reaches its first await", async () => { @@ -249,7 +250,7 @@ describe("runGestureTransaction", () => { .spyOn(element, "getBoundingClientRect") .mockReturnValueOnce(rect(10.04, 20.05, 100.05, 80.05)) .mockReturnValueOnce(rect(11.19, 17.89, 100.29, 78.99)); - const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); const now = vi.spyOn(performance, "now").mockReturnValueOnce(50).mockReturnValueOnce(58.44); await runGestureTransaction({ @@ -261,13 +262,7 @@ describe("runGestureTransaction", () => { }); expect(getRect).toHaveBeenCalledTimes(2); - expect(error).toHaveBeenCalledWith( - "[hf-commit] persist changed pixels", - expect.objectContaining({ - label: "Resize layer", - delta: expect.objectContaining({ x: expect.any(Number) }), - }), - ); + expect(log).not.toHaveBeenCalled(); expect(trackStudioEventMock).toHaveBeenCalledWith("commit_invariant_violation", { label: "Resize layer", delta_x: 1.2, @@ -283,13 +278,13 @@ describe("runGestureTransaction", () => { expect.objectContaining({ pixel_asserted: true }), ); now.mockRestore(); - error.mockRestore(); + log.mockRestore(); }); it("skips the pixel assertion for live position tweens", async () => { const element = document.createElement("div"); const getRect = vi.spyOn(element, "getBoundingClientRect"); - const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); await runGestureTransaction({ element, @@ -301,11 +296,11 @@ describe("runGestureTransaction", () => { }); expect(getRect).not.toHaveBeenCalled(); - expect(error).not.toHaveBeenCalledWith("[hf-commit] persist changed pixels", expect.anything()); + expect(log).not.toHaveBeenCalled(); expect(trackStudioEventMock).not.toHaveBeenCalledWith( "commit_invariant_violation", expect.anything(), ); - error.mockRestore(); + log.mockRestore(); }); }); diff --git a/packages/studio/src/hooks/gestureTransaction.ts b/packages/studio/src/hooks/gestureTransaction.ts index 4b398d6636..da7d61f487 100644 --- a/packages/studio/src/hooks/gestureTransaction.ts +++ b/packages/studio/src/hooks/gestureTransaction.ts @@ -4,6 +4,7 @@ import type { CommitMutationOptions, } from "./gsapScriptCommitTypes"; import { trackStudioEvent } from "../utils/studioTelemetry"; +import { makeStudioDebugLogger } from "../utils/studioDebug"; type PixelRect = Pick; @@ -108,14 +109,7 @@ async function dispatchBufferedCommits(calls: BufferedCommit[]): Promise return reloadsRequested(calls); } -/** - * Dev-only [hf-commit] lifecycle trace. The production observability lives in - * the trackStudioEvent commit_* events (always on); these console lines are a - * developer aid and stay out of end users' consoles. - */ -function traceCommit(stage: string, data: Record): void { - if (import.meta.env.DEV) console.info(`[hf-commit] ${stage}`, data); -} +const logCommit = makeStudioDebugLogger("commit"); /** * Owns the visual + persistence + history lifecycle for one gesture release. @@ -127,9 +121,9 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { let mutationCount = 0; let reloadCount = 0; const bufferedCommits: BufferedCommit[] = []; - traceCommit("start", { label: tx.label, coalesceKey }); + logCommit("start", { label: tx.label, coalesceKey }); tx.settle(); - traceCommit("settled", { label: tx.label, coalesceKey }); + logCommit("settled", { label: tx.label, coalesceKey }); const before = !tx.skipPixelAssert ? readPixelRect(tx.element) : null; const commit: TxCommit = (commitMutation) => { @@ -152,19 +146,12 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { .then(async () => { reloadCount = await dispatchBufferedCommits(bufferedCommits); const durationMs = Math.round(performance.now() - startedAt); - traceCommit("persisted", { label: tx.label, coalesceKey }); + logCommit("persisted", { label: tx.label, coalesceKey }); if (before) { const after = readPixelRect(tx.element); const delta = pixelDelta(before, after); if (exceedsPixelTolerance(delta)) { - if (import.meta.env.DEV) { - console.error("[hf-commit] persist changed pixels", { - label: tx.label, - before, - after, - delta, - }); - } + logCommit("persist-changed-pixels", { label: tx.label, before, after, delta }); trackStudioEvent("commit_invariant_violation", { label: tx.label, delta_x: roundToOneDecimal(delta.x), @@ -193,7 +180,7 @@ export function runGestureTransaction(tx: GestureTransaction): Promise { error_name: error instanceof Error ? error.name : "unknown", restore_ran: true, }); - traceCommit("restore", { label: tx.label, coalesceKey }); + logCommit("restore", { label: tx.label, coalesceKey }); throw error; }); } diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts index d9a1081df1..05d5c95d4c 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.test.ts @@ -75,4 +75,34 @@ describe("fetchParsedAnimations — in-flight sharing", () => { expect(fetchStub.calls()).toBe(2); }); + + it("supersedes an in-flight pre-write parse with a fresh post-write read", async () => { + const releases: Array<(response: Response) => void> = []; + const fetch = vi.fn( + () => + new Promise((resolve) => { + releases.push(resolve); + }), + ); + vi.stubGlobal("fetch", fetch); + const response = (id: string) => + ({ + ok: true, + json: () => Promise.resolve({ animations: [{ id, targetSelector: `#${id}` }] }), + }) as Response; + + const stale = fetchParsedAnimations("p", "index.html"); + const fresh = fetchParsedAnimations("p", "index.html", { fresh: true }); + expect(fetch).toHaveBeenCalledTimes(2); + + releases[0]?.(response("stale")); + await stale; + const overlappingFreshRead = fetchParsedAnimations("p", "index.html"); + expect(fetch).toHaveBeenCalledTimes(2); + + releases[1]?.(response("fresh")); + const [freshResult, sharedResult] = await Promise.all([fresh, overlappingFreshRead]); + expect(freshResult?.animations[0]?.id).toBe("fresh"); + expect(sharedResult?.animations[0]?.id).toBe("fresh"); + }); }); diff --git a/packages/studio/src/hooks/keyframeCacheAstLoad.ts b/packages/studio/src/hooks/keyframeCacheAstLoad.ts index e4abc42525..390208e328 100644 --- a/packages/studio/src/hooks/keyframeCacheAstLoad.ts +++ b/packages/studio/src/hooks/keyframeCacheAstLoad.ts @@ -56,12 +56,15 @@ const inFlightParses = new Map>(); export function fetchParsedAnimations( projectId: string, sourceFile: string, + options: { fresh?: boolean } = {}, ): Promise { const key = `${projectId}|${sourceFile}`; + if (options.fresh) inFlightParses.delete(key); const inFlight = inFlightParses.get(key); if (inFlight) return inFlight; const request = requestParsedAnimations(projectId, sourceFile).finally(() => { - inFlightParses.delete(key); + // A superseded pre-write request must not evict the fresh post-write one. + if (inFlightParses.get(key) === request) inFlightParses.delete(key); }); inFlightParses.set(key, request); return request; diff --git a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts index 8dd9ba9757..f7ad8529f1 100644 --- a/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts +++ b/packages/studio/src/hooks/useGsapAnimationFetchFallback.ts @@ -33,6 +33,8 @@ export type ElementAnimationsOutcome = export interface GsapAnimationFetchOptions { /** Refuse the edit when the parse endpoint is unavailable instead of treating it as no motion. */ failOnFetchError?: boolean; + /** Ignore an overlapping pre-write parse and read the source after a durable write. */ + fresh?: boolean; } /** @@ -56,11 +58,13 @@ async function fetchElementAnimationsWithRetry( gsapSourceFile: string, target: { id: string | null; selector: string | null }, failOnFetchError: boolean, + fresh: boolean, ): Promise { let coldAttempts = 0; let errorAttempts = 0; for (;;) { - const parsed = await fetchParsedAnimations(projectId, gsapSourceFile); + const parsed = await fetchParsedAnimations(projectId, gsapSourceFile, { fresh }); + fresh = false; const outcome = selectElementAnimationsOrRetry(parsed, target); if (outcome.kind === "resolved") return outcome.animations; if (outcome.kind === "fetch-error") { @@ -89,6 +93,7 @@ export function useGsapAnimationFetchFallback(projectId: string | null, gsapSour gsapSourceFile, target, options?.failOnFetchError === true, + options?.fresh === true, ); }, [projectId, gsapSourceFile], diff --git a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx index 89ab3f6fdc..9d16157b2d 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.test.tsx +++ b/packages/studio/src/hooks/useGsapAwareEditing.test.tsx @@ -301,6 +301,42 @@ describe("useGsapAwareEditing anchored resize", () => { act(() => root.unmount()); }); + it("reports only the first group preflight failure in input order", async () => { + const failures = [new Error("first blocked"), new Error("second blocked")]; + const trackGsapInteractionFailure = vi.fn(); + const priorDragImplementation = mocks.drag.getMockImplementation(); + mocks.drag.mockImplementation(async (selection) => { + throw selection.id === "a" ? failures[0] : failures[1]; + }); + const { groupCommit, root } = mountGroupHandler({ + gsapCommitMutation: vi.fn().mockResolvedValue(undefined), + makeFetchFallback: () => vi.fn().mockResolvedValue([]), + trackGsapInteractionFailure, + }); + const updates = [ + { + selection: { element: document.createElement("div"), id: "a", selector: "#a" }, + next: { x: 10, y: 10 }, + }, + { + selection: { element: document.createElement("div"), id: "b", selector: "#b" }, + next: { x: 20, y: 20 }, + }, + ] as unknown as DomEditGroupPathOffsetCommit[]; + + await expect(groupCommit(updates)).rejects.toBe(failures[0]); + expect(trackGsapInteractionFailure).toHaveBeenCalledOnce(); + expect(trackGsapInteractionFailure).toHaveBeenCalledWith( + failures[0], + updates[0]?.selection, + "drag", + "Move animated layer (group)", + ); + mocks.drag.mockReset(); + if (priorDragImplementation) mocks.drag.mockImplementation(priorDragImplementation); + act(() => root.unmount()); + }); + it("restores once when resize persistence fails", async () => { const error = new Error("resize failed"); const restore = vi.fn(); diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index d23ae3b020..094afa0278 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -41,6 +41,18 @@ import type { GsapAnimationFetchOptions } from "./useGsapAnimationFetchFallback" // into one another's undo entry (module-local counter, not Date.now()). let groupDragCommitCounter = 0; +function firstPreflightFailure( + results: PromiseSettledResult[], + updates: DomEditGroupPathOffsetCommit[], +): { error: unknown; selection: DomEditSelection } | null { + for (const [index, result] of results.entries()) { + if (result.status !== "rejected") continue; + const selection = updates[index]?.selection; + if (selection) return { error: result.reason, selection }; + } + return null; +} + export interface UseGsapAwareEditingParams { domEditSelection: DomEditSelection | null; selectedGsapAnimations: GsapAnimation[]; @@ -54,7 +66,7 @@ export interface UseGsapAwareEditingParams { ) => () => Promise; trackGsapInteractionFailure: ( error: unknown, - selection: DomEditSelection, + selection: DomEditSelection | null, mutationType: string, label: string, ) => void; @@ -203,27 +215,32 @@ export function useGsapAwareEditing({ // Every member reads the same file, and a preflight writes nothing — so run // them together. The parse layer shares one in-flight request per file, which // turns N sequential round trips into one. - await Promise.all( + const preflightResults = await Promise.allSettled( updates.map(async ({ selection }) => { - try { - const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); - preflightAnimations.set(selection, animations); - const outcome = await tryGsapDragIntercept( - selection, - { x: 0, y: 0 }, - animations, - previewIframeRef.current, - coalescedCommit, - undefined, - { preflightOnly: true }, - ); - assertGsapEditPersisted(outcome); - } catch (error) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - throw error; - } + const animations = await makeFetchFallback(selection, { failOnFetchError: true })(); + preflightAnimations.set(selection, animations); + const outcome = await tryGsapDragIntercept( + selection, + { x: 0, y: 0 }, + animations, + previewIframeRef.current, + coalescedCommit, + undefined, + { preflightOnly: true }, + ); + assertGsapEditPersisted(outcome); }), ); + const preflightFailure = firstPreflightFailure(preflightResults, updates); + if (preflightFailure) { + trackGsapInteractionFailure( + preflightFailure.error, + preflightFailure.selection, + "drag", + "Move animated layer (group)", + ); + throw preflightFailure.error; + } for (const [index, { selection, next }] of updates.entries()) { renderOnCommit = index === updates.length - 1; try { @@ -238,7 +255,7 @@ export function useGsapAwareEditing({ // resolves against a file missing writes it is about to build on. async () => { await flushQueued(); - return makeFetchFallback(selection)(); + return makeFetchFallback(selection, { fresh: true })(); }, { preflightPassed: true }, ); @@ -251,10 +268,9 @@ export function useGsapAwareEditing({ try { await flushQueued(); } catch (error) { - const selection = updates.at(-1)?.selection; - if (selection) { - trackGsapInteractionFailure(error, selection, "drag", "Move animated layer (group)"); - } + // The aggregate write has no uniquely failing member; do not misattribute + // its telemetry to whichever member happened to be last in the array. + trackGsapInteractionFailure(error, null, "drag", "Move animated layer (group)"); throw error; } }, diff --git a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts index 80bef7f7c3..cce77f55b1 100644 --- a/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts +++ b/packages/studio/src/hooks/useGsapInteractionFailureTelemetry.ts @@ -8,16 +8,16 @@ export function useGsapInteractionFailureTelemetry( showToast: (message: string, tone?: "error" | "info") => void, ) { return useCallback( - (error: unknown, selection: DomEditSelection, mutationType: string, label: string) => { + (error: unknown, selection: DomEditSelection | null, mutationType: string, label: string) => { trackStudioSaveFailure({ source: "gsap_commit", error, - filePath: selection.sourceFile ?? activeCompPath ?? "index.html", + filePath: selection?.sourceFile ?? activeCompPath ?? "index.html", mutationType, label, - targetId: selection.id, - targetSelector: selection.selector, - targetSourceFile: selection.sourceFile, + targetId: selection?.id, + targetSelector: selection?.selector, + targetSourceFile: selection?.sourceFile, }); showToast( isGsapEditBlockedError(error) ? error.message : "Failed to save animated edit.", diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 76ca2129da..940509f764 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -117,6 +117,31 @@ describe("applyPreviewSync", () => { expect(reloadPreview).not.toHaveBeenCalled(); }); + it("applies both plural and singular patches when a caller supplies both", () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + + applyPreviewSync( + FAKE_IFRAME, + result(), + { + label: "mixed patch contract", + instantPatches: [ + { selector: "#group-a", change: { kind: "set" as const, props: { x: 1 } } }, + ], + instantPatch: { + selector: "#single-b", + change: { kind: "set" as const, props: { x: 2 } }, + }, + }, + vi.fn(), + ); + + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => [call[1], call[4]])).toEqual([ + ["#group-a", true], + ["#single-b", false], + ]); + }); + it("instantPatches: one patch that misses falls the whole batch back to the reload", () => { patchRuntimeTweenInPlace.mockImplementation((_iframe, selector) => selector !== "#b"); applySoftReload.mockReturnValue("applied"); diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index fb042945d6..640b5cfa8f 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -249,7 +249,10 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - const patches = options.instantPatches ?? (options.instantPatch ? [options.instantPatch] : []); + const patches = [ + ...(options.instantPatches ?? []), + ...(options.instantPatch ? [options.instantPatch] : []), + ]; if (patches.length > 0) { const deferSeek = options.deferPreviewSync === true; const missed = patches.find( @@ -370,7 +373,8 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra const instantPatches = calls .map(({ options: callOptions }) => callOptions.instantPatch) .filter((patch) => patch !== undefined); - await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...options, instantPatches } : options); + const { instantPatch: _instantPatch, ...batchOptions } = options; + await finalizeSuccessfulMutation(pid, compositionPath, last.selection, last.mutation, targetPath, result, instantPatches.length > 0 ? { ...batchOptions, instantPatches } : batchOptions); }, [showToast, finalizeSuccessfulMutation]); // Every GSAP-script commit is a read-modify-write of one file. Overlapping diff --git a/packages/studio/src/utils/studioDebug.ts b/packages/studio/src/utils/studioDebug.ts index a30b8f9bbe..73b3833fe6 100644 --- a/packages/studio/src/utils/studioDebug.ts +++ b/packages/studio/src/utils/studioDebug.ts @@ -1,7 +1,7 @@ // Opt-in diagnostic channels — one per question worth tracing, all off by // default. Turn one on for the session with `localStorage.setItem("hf--debug", // "1")` and reload, then grep the console for `[hf-]`. -// Live channels: reload, select, drag, resize. +// Live channels: reload, select, drag, resize, commit. // // These exist because the interesting failures here are decisions, not crashes: // a preview that reloads when it should not, a shift-click that selects nothing. From dff382a2a849d4834a345373fca0bfe135902b56 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 14:03:07 -0400 Subject: [PATCH 21/26] fix(studio): carry deferred preview fallbacks --- .../studio/src/hooks/gsapScriptCommitTypes.ts | 2 + .../studio/src/hooks/useGsapAwareEditing.ts | 2 + .../src/hooks/useGsapScriptCommits.test.tsx | 92 +++++++++++++++++++ .../studio/src/hooks/useGsapScriptCommits.ts | 42 ++++++--- 4 files changed, 126 insertions(+), 12 deletions(-) diff --git a/packages/studio/src/hooks/gsapScriptCommitTypes.ts b/packages/studio/src/hooks/gsapScriptCommitTypes.ts index ed9f4a6ae4..85071ab6d9 100644 --- a/packages/studio/src/hooks/gsapScriptCommitTypes.ts +++ b/packages/studio/src/hooks/gsapScriptCommitTypes.ts @@ -32,6 +32,8 @@ export interface CommitMutationOptions { * error handling — a failed write still throws. */ deferPreviewSync?: boolean; + /** Shares an in-place patch miss with the final render of one multi-write action. */ + previewFallbackLatch?: { pending: boolean }; beforeReload?: () => void; /** * Serialize this commit against others sharing the same key. Used to chain diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index 094afa0278..384527f5fb 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -179,11 +179,13 @@ export function useGsapAwareEditing({ // positions are already on screen, so holding the render until the last // member has been written costs nothing and never shows a half-moved group. let renderOnCommit = false; + const previewFallbackLatch = { pending: false }; const withGroupOptions = (options: CommitMutationOptions): CommitMutationOptions => ({ ...options, coalesceKey, coalesceMs: Number.POSITIVE_INFINITY, deferPreviewSync: !renderOnCommit, + previewFallbackLatch, }); // Every member writes the same file. Queue their mutations and send them as // ONE request instead of one round trip per member: the server reads, parses diff --git a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx index 940509f764..e9deae6ccf 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.test.tsx +++ b/packages/studio/src/hooks/useGsapScriptCommits.test.tsx @@ -169,6 +169,63 @@ describe("applyPreviewSync", () => { }); }); + it("carries a deferred patch miss into the final batch render", () => { + const previewFallbackLatch = { pending: false }; + applySoftReload.mockReturnValue("applied"); + const reloadPreview = vi.fn(); + patchRuntimeTweenInPlace.mockReturnValueOnce(false).mockReturnValueOnce(true); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + deferPreviewSync: true, + previewFallbackLatch, + instantPatch: { selector: "#missed", change: { kind: "set", props: { x: 1 } } }, + }, + reloadPreview, + ); + + expect(previewFallbackLatch.pending).toBe(true); + expect(applySoftReload).not.toHaveBeenCalled(); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Move animated layer (group)", + softReload: true, + previewFallbackLatch, + instantPatch: { selector: "#final", change: { kind: "set", props: { x: 2 } } }, + }, + reloadPreview, + ); + + expect(previewFallbackLatch.pending).toBe(false); + expect(applySoftReload).toHaveBeenCalledTimes(1); + }); + + it("falls back immediately when a deferred patch miss has no final-render latch", () => { + patchRuntimeTweenInPlace.mockReturnValue(false); + applySoftReload.mockReturnValue("applied"); + + applyPreviewSync( + FAKE_IFRAME, + result({ scriptText: "SCRIPT" }), + { + label: "Deferred standalone write", + softReload: true, + deferPreviewSync: true, + instantPatch: { selector: "#missed", change: { kind: "set", props: { x: 1 } } }, + }, + vi.fn(), + ); + + expect(applySoftReload).toHaveBeenCalledTimes(1); + }); + it("instantPatch + patch fails: falls back to the soft reload, passing onAsyncFailure", () => { patchRuntimeTweenInPlace.mockReturnValue(false); applySoftReload.mockReturnValue("applied"); @@ -439,6 +496,41 @@ describe("runCommit — instantPatch wiring", () => { expect(deps.reloadPreview).not.toHaveBeenCalled(); }); + it("no-op batch still applies every plural instant patch", async () => { + patchRuntimeTweenInPlace.mockReturnValue(true); + mockFetchResult({ changed: false }); + const deps = renderCommitHook(); + const batch = deps.api.commitMutation.batch; + if (!batch) throw new Error("batch capability missing"); + + await act(async () => { + await batch( + [ + { + selection, + mutation: { type: "update-property", property: "x", value: 10 }, + options: { + label: "Move layer", + instantPatch: { selector: "#a", change: { kind: "set", props: { x: 10 } } }, + }, + }, + { + selection: { ...selection, id: "b", selector: "#b" }, + mutation: { type: "update-property", property: "x", value: 20 }, + options: { + label: "Move layer", + instantPatch: { selector: "#b", change: { kind: "set", props: { x: 20 } } }, + }, + }, + ], + { label: "Move animated layer (group)" }, + ); + }); + + expect(patchRuntimeTweenInPlace.mock.calls.map((call) => call[1])).toEqual(["#a", "#b"]); + expect(deps.reloadPreview).not.toHaveBeenCalled(); + }); + it("no-op commit whose instant patch MISSES soft-reloads (never full-reloads)", async () => { // Server contract: gsap-mutations returns scriptText on EVERY response, // including changed:false — so the fallback re-runs the identical script diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 640b5cfa8f..8a902ce97a 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -126,7 +126,10 @@ function finishUnchangedMutation( reloadPreview: () => void, ): boolean { if (result.changed !== false) return false; - if (!options.skipReload && options.instantPatch) { + if ( + !options.skipReload && + (instantPatchesFor(options).length > 0 || options.previewFallbackLatch?.pending) + ) { applyPreviewSync(iframe, result, options, reloadPreview); } return true; @@ -249,10 +252,8 @@ export function applyPreviewSync( options: CommitMutationOptions, reloadPreview: () => void, ): void { - const patches = [ - ...(options.instantPatches ?? []), - ...(options.instantPatch ? [options.instantPatch] : []), - ]; + const patches = instantPatchesFor(options); + let needsFallback = options.previewFallbackLatch?.pending === true; if (patches.length > 0) { const deferSeek = options.deferPreviewSync === true; const missed = patches.find( @@ -265,15 +266,23 @@ export function applyPreviewSync( deferSeek || index < patches.length - 1, ), ); - // Patched in place — elements are already correct on screen; no reload needed. - if (!missed) return; - // The instant path couldn't patch in place — record the fallback so we can - // track how often the fast path misses before the soft/full reload below. - trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); - // Fall through to the soft/full reload path below. + if (missed) { + // The instant path couldn't patch in place — record the fallback so we can + // track how often the fast path misses before the soft/full reload below. + trackStudioEvent("gsap_instant_patch_fallback", { selector: missed.selector }); + needsFallback = true; + } + // Patched in place — elements are already correct on screen; no reload needed + // unless an earlier deferred batch left one member unpatched. + if (!needsFallback) return; } // Written, but the caller has more writes to make and will render after the last. - if (options.deferPreviewSync) return; + if (options.deferPreviewSync && options.previewFallbackLatch) { + options.previewFallbackLatch.pending = needsFallback; + return; + } + if (options.deferPreviewSync && !needsFallback) return; + if (options.previewFallbackLatch) options.previewFallbackLatch.pending = false; if (options.softReload && result.scriptText) { // A soft-reloadable edit escalates to a full iframe remount ONLY on the // PERMANENT "cannot-soft-reload" result (the preview is genuinely stale/ @@ -293,6 +302,15 @@ export function applyPreviewSync( } } +function instantPatchesFor( + options: CommitMutationOptions, +): NonNullable { + return [ + ...(options.instantPatches ?? []), + ...(options.instantPatch ? [options.instantPatch] : []), + ]; +} + // oxfmt-ignore // fallow-ignore-next-line complexity export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIframeRef, editHistory, domEditSaveTimestampRef, reloadPreview, onCacheInvalidate, onFileContentChanged, showToast, sdkSession, publishSdkSession, writeProjectFile, forceReloadSdkSession }: GsapScriptCommitsParams) { From 5a022b4e1a70479b943647ad04c95b5eeaef597c Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Fri, 7 Aug 2026 21:43:23 -0700 Subject: [PATCH 22/26] chore(studio): name whoever puts the pre-resize size back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resizing the card commits correctly — the source and a fresh load both read 273x181 — but 200ms after the drop, mid-commit, the element renders at 395x261 with the studio size vars still holding 273x181. Something writes the pre-gesture size back inline while the reload is still in flight, and every writer of that size was silent. Both are traced now under the existing hf-resize-debug flag, each with the size going in, the size being replaced, and a short stack. Restoring the pre-gesture size is right on a cancel and wrong after a successful commit, and the function doing it cannot tell the two apart from the inside — so the caller has to be named before this can be fixed at the right end. --- packages/studio/src/components/editor/manualEditsDom.ts | 6 ++++++ .../studio/src/components/editor/manualEditsSnapshot.ts | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 80697e4e3d..9235f88116 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -36,6 +36,7 @@ import { roundRotationAngle } from "./manualEditsParsing"; import { applyStudioMotionFromDom } from "./studioMotion"; import { gsapAnimatesProperty } from "./gsapAnimatesProperty"; import { splitTopLevelWhitespace } from "./manualEditsStyleHelpers"; +import { logResize } from "../../utils/resizeDebug"; /* ── Gesture tracking ─────────────────────────────────────────────── */ let studioManualEditGestureId = 0; @@ -454,6 +455,11 @@ export function applyStudioBoxSize( element: HTMLElement, size: { width: number; height: number }, ): void { + logResize("apply-box-size", { + to: `${Math.round(size.width)} x ${Math.round(size.height)}`, + from: `${element.style.width || "-"} x ${element.style.height || "-"}`, + stack: new Error("apply-box-size").stack?.split("\n").slice(1, 6).join(" < "), + }); promoteInlineForTransform(element); applyStudioBoxSizeDimensions(element, size); } diff --git a/packages/studio/src/components/editor/manualEditsSnapshot.ts b/packages/studio/src/components/editor/manualEditsSnapshot.ts index 826fb16829..9ab9269eb8 100644 --- a/packages/studio/src/components/editor/manualEditsSnapshot.ts +++ b/packages/studio/src/components/editor/manualEditsSnapshot.ts @@ -40,6 +40,7 @@ import type { StudioRotationSnapshot, StudioPathOffsetSnapshot, } from "./manualEditsTypes"; +import { logResize } from "../../utils/resizeDebug"; /* ── Capture ──────────────────────────────────────────────────────── */ export function captureStudioBoxSize(element: HTMLElement): StudioBoxSizeSnapshot { @@ -114,6 +115,14 @@ function restoreStyleProperty(element: HTMLElement, property: string, value: str } export function restoreStudioBoxSize(element: HTMLElement, previous: StudioBoxSizeSnapshot): void { + // Putting the pre-gesture size back is correct on a cancel and wrong after a + // successful commit, and the two are indistinguishable from in here — so say + // who asked, with the size being restored and the one being replaced. + logResize("restore-box-size", { + to: `${previous.width || "-"} x ${previous.height || "-"}`, + from: `${element.style.width || "-"} x ${element.style.height || "-"}`, + stack: new Error("restore-box-size").stack?.split("\n").slice(1, 6).join(" < "), + }); restoreStyleProperty(element, "width", previous.width); restoreStyleProperty(element, "height", previous.height); restoreStyleProperty(element, "min-width", previous.minWidth); From 7457306855d3f973812b3875151e0f8defa8175c Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 10:36:45 -0700 Subject: [PATCH 23/26] fix(studio): hold a resized element's size while the timeline is rebuilt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Your log caught it across two resizes. The first commits 305x202 and the element is 305x202 at the drop; 200ms later it renders 395x261, its stylesheet size, while --hf-studio-width still reads 305. The second gesture then starts with `actual` at 305 against a live box of 395, and its very first move — a pointer delta of 0.1px — snaps the element back to 305. That snap is the jump. The gap belongs to the soft reload: it reverts the old timeline before building the new one, and GSAP hands back each tween's recorded starting width on the way out. Nothing held the size in between, because the seek reapply that exists for exactly this stands aside for elements GSAP animates. Standing aside is right for the offset — those channels compose, and applying both doubles the move — and wrong for size, where both channels write width and height so the later write simply wins on the same committed number. It applies now. Only an element mid-edit carries the vars, so nothing else is touched. A test seeks an element whose size GSAP owns after the revert put the stylesheet size back, and fails with the skip restored. --- .../src/components/editor/manualEditsDom.ts | 29 +++++---- .../editor/reapplyBoxSizeAfterSeek.test.ts | 64 +++++++++++++++++++ 2 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 9235f88116..50b4c2272e 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -561,26 +561,29 @@ function queryStudioElements(doc: Document, attr: string): HTMLElement[] { function reapplyPathOffsets(doc: Document): void { for (const el of queryStudioElements(doc, STUDIO_PATH_OFFSET_ATTR)) { - const gsapSkip = gsapAnimatesProperty(el, "x", "y"); + // Unlike size below, the offset channels COMPOSE — applying both doubles the move. + if (gsapAnimatesProperty(el, "x", "y")) continue; const x = el.style.getPropertyValue(STUDIO_OFFSET_X_PROP); const y = el.style.getPropertyValue(STUDIO_OFFSET_Y_PROP); - if (gsapSkip) continue; - if (x || y) { - applyStudioPathOffset( - el, - { - x: Number.parseFloat(x) || 0, - y: Number.parseFloat(y) || 0, - }, - { updateBase: false }, - ); - } + if (!x && !y) continue; + const offset = { x: Number.parseFloat(x) || 0, y: Number.parseFloat(y) || 0 }; + applyStudioPathOffset(el, offset, { updateBase: false }); } } +/** + * Put the studio's committed size back after a seek, GSAP-sized elements included. + * + * Size does not compose the way the offset above does: both channels write width + * and height, so the later write wins and both hold the same number. Standing + * aside meant nothing held the size while a soft reload reverted the old timeline + * — GSAP hands back each tween's recorded starting width — so until the new one + * rendered, the element sat at its stylesheet size. That is the jump after a + * resize, and the next gesture then started from a box disagreeing with these + * vars and snapped on its first move. Only an element mid-edit carries them. + */ function reapplyBoxSizes(doc: Document): void { for (const el of queryStudioElements(doc, STUDIO_BOX_SIZE_ATTR)) { - if (gsapAnimatesProperty(el, "width", "height")) continue; const w = Number.parseFloat(el.style.getPropertyValue(STUDIO_WIDTH_PROP)); const h = Number.parseFloat(el.style.getPropertyValue(STUDIO_HEIGHT_PROP)); if (Number.isFinite(w) && Number.isFinite(h) && w > 0 && h > 0) { diff --git a/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts b/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts new file mode 100644 index 0000000000..92a9087968 --- /dev/null +++ b/packages/studio/src/components/editor/reapplyBoxSizeAfterSeek.test.ts @@ -0,0 +1,64 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it } from "vitest"; +import { reapplyPositionEditsAfterSeek } from "./manualEditsDom"; +import { STUDIO_BOX_SIZE_ATTR, STUDIO_HEIGHT_PROP, STUDIO_WIDTH_PROP } from "./manualEditsTypes"; + +/** + * A resize commit hands the size to a GSAP tween, and a soft reload reverts the + * old timeline before the new one renders — GSAP restores each tween's recorded + * starting width on the way out. Nothing else held the size across that window, + * so the element sat at its stylesheet size for a few hundred milliseconds: the + * jump after a resize. Worse, the next gesture then started from a box that + * disagreed with the studio's own vars and snapped on its first move. + * + * The seek reapply is what closes the window, and it used to stand aside for + * exactly the elements that need it — the ones GSAP sizes. + */ +describe("box size survives a seek while GSAP owns the size", () => { + afterEach(() => { + document.body.innerHTML = ""; + Reflect.deleteProperty(window, "__timelines"); + }); + + function cardSizedByGsap(): HTMLElement { + const el = document.createElement("div"); + el.id = "card"; + el.setAttribute(STUDIO_BOX_SIZE_ATTR, "true"); + el.style.setProperty(STUDIO_WIDTH_PROP, "305px"); + el.style.setProperty(STUDIO_HEIGHT_PROP, "202px"); + document.body.append(el); + // A timeline that animates this element's width/height, as the committed + // resize leaves behind. + Object.assign(window, { + __timelines: { + main: { + getChildren: () => [{ targets: () => [el], vars: { width: 305, height: 202 } }], + }, + }, + }); + return el; + } + + it("re-applies the committed size after the timeline gave it back", () => { + const el = cardSizedByGsap(); + // The revert: GSAP puts the tween's recorded starting size back. + el.style.width = "395px"; + el.style.height = "261px"; + + reapplyPositionEditsAfterSeek(document); + + expect(el.style.width).toBe("305px"); + expect(el.style.height).toBe("202px"); + }); + + it("leaves an element alone once its studio size is cleared", () => { + const el = cardSizedByGsap(); + el.style.removeProperty(STUDIO_WIDTH_PROP); + el.style.removeProperty(STUDIO_HEIGHT_PROP); + el.style.width = "395px"; + + reapplyPositionEditsAfterSeek(document); + + expect(el.style.width).toBe("395px"); + }); +}); From e49b016823099aa9158487c72328cf51a2df4799 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 03:21:59 -0400 Subject: [PATCH 24/26] refactor(studio): keep the resize files under the size cap --- packages/studio/src/components/editor/manualEditsDom.ts | 6 ------ .../studio/src/components/editor/manualEditsSnapshot.ts | 9 --------- 2 files changed, 15 deletions(-) diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 50b4c2272e..fe19bff836 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -36,7 +36,6 @@ import { roundRotationAngle } from "./manualEditsParsing"; import { applyStudioMotionFromDom } from "./studioMotion"; import { gsapAnimatesProperty } from "./gsapAnimatesProperty"; import { splitTopLevelWhitespace } from "./manualEditsStyleHelpers"; -import { logResize } from "../../utils/resizeDebug"; /* ── Gesture tracking ─────────────────────────────────────────────── */ let studioManualEditGestureId = 0; @@ -455,11 +454,6 @@ export function applyStudioBoxSize( element: HTMLElement, size: { width: number; height: number }, ): void { - logResize("apply-box-size", { - to: `${Math.round(size.width)} x ${Math.round(size.height)}`, - from: `${element.style.width || "-"} x ${element.style.height || "-"}`, - stack: new Error("apply-box-size").stack?.split("\n").slice(1, 6).join(" < "), - }); promoteInlineForTransform(element); applyStudioBoxSizeDimensions(element, size); } diff --git a/packages/studio/src/components/editor/manualEditsSnapshot.ts b/packages/studio/src/components/editor/manualEditsSnapshot.ts index 9ab9269eb8..826fb16829 100644 --- a/packages/studio/src/components/editor/manualEditsSnapshot.ts +++ b/packages/studio/src/components/editor/manualEditsSnapshot.ts @@ -40,7 +40,6 @@ import type { StudioRotationSnapshot, StudioPathOffsetSnapshot, } from "./manualEditsTypes"; -import { logResize } from "../../utils/resizeDebug"; /* ── Capture ──────────────────────────────────────────────────────── */ export function captureStudioBoxSize(element: HTMLElement): StudioBoxSizeSnapshot { @@ -115,14 +114,6 @@ function restoreStyleProperty(element: HTMLElement, property: string, value: str } export function restoreStudioBoxSize(element: HTMLElement, previous: StudioBoxSizeSnapshot): void { - // Putting the pre-gesture size back is correct on a cancel and wrong after a - // successful commit, and the two are indistinguishable from in here — so say - // who asked, with the size being restored and the one being replaced. - logResize("restore-box-size", { - to: `${previous.width || "-"} x ${previous.height || "-"}`, - from: `${element.style.width || "-"} x ${element.style.height || "-"}`, - stack: new Error("restore-box-size").stack?.split("\n").slice(1, 6).join(" < "), - }); restoreStyleProperty(element, "width", previous.width); restoreStyleProperty(element, "height", previous.height); restoreStyleProperty(element, "min-width", previous.minWidth); From dd328ee83b24639ad3f6823d5f3b540c53f7a38b Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 11:24:14 -0400 Subject: [PATCH 25/26] docs(studio): fold the resize note into the size-reapply comment --- .../studio/src/components/editor/manualEditsDom.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index fe19bff836..4a041e020f 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -567,14 +567,11 @@ function reapplyPathOffsets(doc: Document): void { /** * Put the studio's committed size back after a seek, GSAP-sized elements included. - * * Size does not compose the way the offset above does: both channels write width - * and height, so the later write wins and both hold the same number. Standing - * aside meant nothing held the size while a soft reload reverted the old timeline - * — GSAP hands back each tween's recorded starting width — so until the new one - * rendered, the element sat at its stylesheet size. That is the jump after a - * resize, and the next gesture then started from a box disagreeing with these - * vars and snapped on its first move. Only an element mid-edit carries them. + * and height, so the later write wins on the same number. Standing aside meant + * nothing held the size while a soft reload reverted the old timeline (GSAP hands + * back each tween's recorded starting width), so the element sat at its stylesheet + * size until the new one rendered — the jump after a resize. */ function reapplyBoxSizes(doc: Document): void { for (const el of queryStudioElements(doc, STUDIO_BOX_SIZE_ATTR)) { From d7ca044315c5a7f599c0fb28676e3f147acab85c Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 9 Aug 2026 16:34:49 -0400 Subject: [PATCH 26/26] fix(studio): rotate the child outlines with the element they outline Selecting a rotated element drew upright dashed boxes across its children: the chrome co-rotated with the element and the child outlines did not, so a text layer inside a rotated card got a square outline lying across the rotated glyphs. The chrome already measures an oriented box; the child outlines were still measured axis-aligned. They now use the same oriented measurement and render with the same rotation. An unrotated element measures identically to before, since the oriented rect returns the plain bounding box at angle 0. --- .../src/components/editor/DomEditOverlay.tsx | 1 + .../editor/domEditOverlayGeometry.test.ts | 25 +++++++++++++++++++ .../editor/domEditOverlayGeometry.ts | 18 +++++++++++++ .../editor/useDomEditOverlayRects.ts | 6 +++-- 4 files changed, 48 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/components/editor/DomEditOverlay.tsx b/packages/studio/src/components/editor/DomEditOverlay.tsx index 151c24ce08..261b82f24b 100644 --- a/packages/studio/src/components/editor/DomEditOverlay.tsx +++ b/packages/studio/src/components/editor/DomEditOverlay.tsx @@ -519,6 +519,7 @@ export const DomEditOverlay = memo(function DomEditOverlay({ top: cr.top, width: cr.width, height: cr.height, + transform: cr.angle ? `rotate(${cr.angle}deg)` : undefined, }} /> ))} diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts index 3ecfc83227..0554bb4f4a 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts @@ -5,6 +5,7 @@ import { orientedGroupAwareOverlayRect, overlayCornersCentroid, selectionCacheKey, + orientedVisibleOverlayRect, } from "./domEditOverlayGeometry"; describe("overlayCornersCentroid", () => { @@ -159,6 +160,30 @@ describe("orientedOverlayRect — rotation gate (perf fix, V15 18a/18b)", () => expect(rect!.angle ?? 0).toBe(0); }); + /** + * A child outline is drawn ON the child, not around it. Measured axis-aligned, + * a text layer inside a rotated card got an upright dashed box sitting across + * the rotated glyphs — the parent's chrome rotated and its children's did not. + */ + it("child outlines carry the element's angle, so they can co-rotate with it", () => { + const { overlayEl, iframe, el } = buildHarness(); + el.style.transform = ROTATE_30DEG_MATRIX; + const rect = orientedVisibleOverlayRect(overlayEl, iframe, el); + expect(rect).not.toBeNull(); + expect(rect!.angle).toBeCloseTo(30, 3); + }); + + it("an unrotated child outline is unchanged — no angle, same box as before", () => { + const { overlayEl, iframe, el } = buildHarness(); + const rect = orientedVisibleOverlayRect(overlayEl, iframe, el); + expect(rect).not.toBeNull(); + expect(rect!.angle ?? 0).toBe(0); + expect(rect!.left).toBeCloseTo(400, 5); + expect(rect!.top).toBeCloseTo(450, 5); + expect(rect!.width).toBeCloseTo(200, 5); + expect(rect!.height).toBeCloseTo(100, 5); + }); + it("rotated element takes the corner-geometry path — reports the live angle", () => { const { overlayEl, iframe, el } = buildHarness(); el.style.transform = ROTATE_30DEG_MATRIX; diff --git a/packages/studio/src/components/editor/domEditOverlayGeometry.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.ts index 8ad23ddcb0..004bef4369 100644 --- a/packages/studio/src/components/editor/domEditOverlayGeometry.ts +++ b/packages/studio/src/components/editor/domEditOverlayGeometry.ts @@ -430,6 +430,24 @@ export function orientedOverlayRect( }; } +/** + * `toVisibleOverlayRect`'s oriented twin: the element's crop-hugged box plus its + * live rotation, for chrome that has to sit on a rotated element rather than + * around it. Rendering the result with `transform: rotate(angle)` about its + * centre lands it on the element's real corners. + * + * At angle 0 `orientedOverlayRect` returns the plain AABB, so an unrotated + * element measures exactly as it did before. + */ +export function orientedVisibleOverlayRect( + overlayEl: HTMLDivElement, + iframe: HTMLIFrameElement, + element: HTMLElement, +): OverlayRect | null { + const rect = orientedOverlayRect(overlayEl, iframe, element); + return rect ? { ...rect, ...hugRectForElement(rect, element) } : null; +} + const OVERLAY_RECT_EPSILON_PX = 0.5; const OVERLAY_RECT_ANGLE_EPSILON_DEG = 0.1; diff --git a/packages/studio/src/components/editor/useDomEditOverlayRects.ts b/packages/studio/src/components/editor/useDomEditOverlayRects.ts index b45ff6f26c..ba402bafb6 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayRects.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayRects.ts @@ -17,7 +17,7 @@ import { rectsEqual, resolveElementForOverlay, selectionCacheKey, - toVisibleOverlayRect, + orientedVisibleOverlayRect, } from "./domEditOverlayGeometry"; function childRectsEqual(a: OverlayRect[], b: OverlayRect[]): boolean { @@ -172,7 +172,9 @@ export function useDomEditOverlayRects({ for (let i = 0; i < descendants.length; i++) { const child = descendants[i] as HTMLElement; if (!child.getBoundingClientRect) continue; - const r = toVisibleOverlayRect(overlayEl, iframe, child); + // Oriented, not axis-aligned: a child of a rotated element drew its + // outline square around the rotated glyphs instead of on them. + const r = orientedVisibleOverlayRect(overlayEl, iframe, child); if (r && r.width > 2 && r.height > 2) nextChildRects.push(r); } if (!childRectsEqual(childRectsRef.current, nextChildRects)) {