diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 520b4e8554..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(); @@ -91,9 +92,9 @@ 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"; const effectiveTimelineDuration = useMemo(() => { const maxEnd = timelineElements.length > 0 @@ -270,13 +271,14 @@ export function StudioApp() { const domEditSession = useDomEditSession({ projectId, activeCompPath, - isMasterView, compIdToSrc, captionEditMode, compositionLoading, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed: panelLayout.setRightCollapsed, setRightPanelTab: panelLayout.setRightPanelTab, showToast, @@ -416,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/DomEditOverlay.test.ts b/packages/studio/src/components/editor/DomEditOverlay.test.ts index 736bd7ecb2..a7270849cd 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; @@ -278,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(); @@ -323,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); @@ -628,6 +671,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..151c24ce08 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, + resolveShiftClickCandidate, } from "./domEditOverlayGestures"; import { useDomEditOverlayRects } from "./useDomEditOverlayRects"; import { OffCanvasIndicators, type OffCanvasRect } from "./OffCanvasIndicators"; @@ -31,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 { @@ -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,7 +329,9 @@ export const DomEditOverlay = memo(function DomEditOverlay({ return; } const target = event.target as HTMLElement | null; - 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. @@ -341,8 +346,20 @@ 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 - const candidate = hoverSelectionRef.current; + const shiftIframe = iframeRef.current; + const candidate = resolveShiftClickCandidate({ + cached: hoverSelectionRef.current, + elementAtPoint: shiftIframe + ? getPreviewTargetFromPointer( + shiftIframe, + event.clientX, + event.clientY, + activeCompositionPathRef.current, + ) + : null, + }); + // 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(); @@ -376,28 +393,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/domEditOverlayGeometry.test.ts b/packages/studio/src/components/editor/domEditOverlayGeometry.test.ts index 217f3d3430..3ecfc83227 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,64 @@ 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)"; + 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(); + // 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("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 70f2500958..8ad23ddcb0 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,15 @@ 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") { + // 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 { return null; @@ -141,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/domEditOverlayGestures.ts b/packages/studio/src/components/editor/domEditOverlayGestures.ts index d7fec86a87..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,6 +113,47 @@ export function focusDomEditOverlayElement(element: FocusableDomEditOverlay | nu element?.focus({ preventScroll: true }); } +/** + * 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); +} + +/** + * 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 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/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts new file mode 100644 index 0000000000..7a0fe732a4 --- /dev/null +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -0,0 +1,110 @@ +import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry"; +import { + resolveEquidistanceGuides, + resolveSnapAdjustment, + snapEngagedForTravel, + 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 = {}; + let lastGesture: GroupGestureState | null = null; + + /** 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; + 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; + 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) => { + if (groupG !== lastGesture) { + lastGesture = groupG; + lastGroupPositions = {}; + } + 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/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/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.test.ts b/packages/studio/src/components/editor/manualOffsetDrag.test.ts index 5af32996a0..0db15b1149 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", () => { @@ -160,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/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index a808c792fb..731c41813f 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." }; @@ -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/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 7af8df2e7b..65e8bb4222 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, @@ -50,8 +49,15 @@ 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"; export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -91,6 +97,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; @@ -114,55 +122,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 }, - })), - ); - for (const member of groupG.members) applyManualOffsetDragDraft(member, dx, dy); + moveGroupDrag(groupG, e); return; } @@ -215,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, @@ -319,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; @@ -336,6 +304,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 +325,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/hooks/domSelectionTimelineMirror.ts b/packages/studio/src/hooks/domSelectionTimelineMirror.ts new file mode 100644 index 0000000000..f67577925a --- /dev/null +++ b/packages/studio/src/hooks/domSelectionTimelineMirror.ts @@ -0,0 +1,73 @@ +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[]; + getTimelineSelectionSet: () => ReadonlySet; + 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, + getTimelineSelectionSet, + 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); + 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: publishedMembers.size, + 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 || !getTimelineSelectionSet().has(timelineAnchor)) { + setTimelineSelectionSet(publishedMembers); + } + setSelectedTimelineElementId(timelineAnchor, { preserveSet: true }); +} 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.test.tsx b/packages/studio/src/hooks/useDomEditSession.test.tsx index 3c5a3e0ffd..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,43 +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(), - 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; } @@ -318,39 +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(), - 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 efec011f95..1ad7ab3fce 100644 --- a/packages/studio/src/hooks/useDomEditSession.ts +++ b/packages/studio/src/hooks/useDomEditSession.ts @@ -31,13 +31,14 @@ 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; setRightPanelTab: (tab: RightPanelTab) => void; showToast: (message: string, tone?: "error" | "info") => void; @@ -73,13 +74,14 @@ export interface UseDomEditSessionParams { export function useDomEditSession({ projectId, activeCompPath, - isMasterView, compIdToSrc, captionEditMode, compositionLoading, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, showToast, @@ -109,6 +111,7 @@ export function useDomEditSession({ publishSdkSession, forceReloadSdkSession, }: UseDomEditSessionParams) { + const isMasterView = !activeCompPath || activeCompPath === "index.html"; void _setRefreshKey; const { domEditSelection, @@ -127,6 +130,7 @@ export function useDomEditSession({ buildDomSelectionForTimelineElement, handleTimelineElementSelect, refreshDomEditSelectionFromPreview, + refreshDomEditGroupSelectionsFromPreview, applyMarqueeSelection, } = useDomSelection({ projectId, @@ -136,7 +140,9 @@ export function useDomEditSession({ captionEditMode, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, @@ -382,6 +388,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.test.ts b/packages/studio/src/hooks/useDomSelection.test.ts index 53f4524644..4b407c35e4 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,10 @@ function renderHarness(initialProps: HarnessProps): { compIdToSrc: new Map(), captionEditMode: false, previewIframeRef: { current: null }, - timelineElements: [], - setSelectedTimelineElementId: vi.fn(), + timelineElements: options.timelineElements ?? [], + getTimelineSelectionSet: () => new Set(), + setSelectedTimelineElementId: timeline.setSelectedTimelineElementId, + setTimelineSelectionSet: timeline.setTimelineSelectionSet, setRightCollapsed: vi.fn(), setRightPanelTab: vi.fn(), previewIframe: null, @@ -61,6 +77,7 @@ function renderHarness(initialProps: HarnessProps): { act(() => root.unmount()); host.remove(); }, + timeline, }; } @@ -77,6 +94,119 @@ 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(); + }); + + 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(); + }); +}); + +/** + * 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 32682b0810..115d54fca2 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, @@ -24,6 +20,8 @@ import { } from "../components/editor/domEditing"; import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; import { useStudioTestHooks } from "./useStudioTestHooks"; +import { logSelect } from "../utils/selectDebug"; +import { announceTimelineSelection as announceSelectionToTimeline } from "./domSelectionTimelineMirror"; // ── Types ── @@ -47,7 +45,10 @@ 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; setRightCollapsed: (collapsed: boolean) => void; setRightPanelTab: (tab: RightPanelTab) => void; previewIframe: HTMLIFrameElement | null; @@ -109,7 +110,9 @@ export function useDomSelection({ captionEditMode, previewIframeRef, timelineElements, + getTimelineSelectionSet, setSelectedTimelineElementId, + setTimelineSelectionSet, setRightCollapsed, setRightPanelTab, previewIframe, @@ -145,6 +148,26 @@ export function useDomSelection({ // ── Callbacks ── + const announceTimelineSelection = useCallback( + (group: DomEditSelection[], primary: DomEditSelection | null) => + announceSelectionToTimeline( + { + timelineElements, + getTimelineSelectionSet, + setSelectedTimelineElementId, + setTimelineSelectionSet, + }, + group, + primary, + ), + [ + getTimelineSelectionSet, + setSelectedTimelineElementId, + setTimelineSelectionSet, + timelineElements, + ], + ); + const applyDomSelection = useCallback( // fallow-ignore-next-line complexity ( @@ -156,11 +179,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; } @@ -186,6 +210,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); @@ -208,21 +239,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(() => { @@ -375,6 +398,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) => { @@ -389,6 +419,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; } @@ -436,25 +477,17 @@ 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 ── + useEffect(() => { + refreshDomEditGroupSelectionsFromPreviewRef.current = refreshDomEditGroupSelectionsFromPreview; + }, [refreshDomEditGroupSelectionsFromPreview]); + // Clear hover unconditionally on composition/project/preview change // eslint-disable-next-line no-restricted-syntax useEffect(() => { @@ -503,6 +536,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; @@ -527,16 +561,9 @@ export function useDomSelection({ domEditGroupSelectionsRef.current = nextGroup; setDomEditSelection(nextSelection); setDomEditGroupSelections(nextGroup); - const nextTimelineId = - findMatchingTimelineElementId(nextSelection, timelineElements) ?? - findTimelineIdByAncestor( - nextSelection.element, - timelineElements, - nextSelection.sourceFile || "index.html", - ); - setSelectedTimelineElementId(nextTimelineId); + announceTimelineSelection(nextGroup, nextSelection); }, - [applyDomSelection, timelineElements, setSelectedTimelineElementId], + [applyDomSelection, announceTimelineSelection], ); return { diff --git a/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts b/packages/studio/src/hooks/useDomSelectionSelectionGuards.test.ts index dce0655946..a61ee38fa3 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) { @@ -66,7 +67,10 @@ 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, setRightCollapsed: vi.fn(), setRightPanelTab: props.setRightPanelTab, previewIframe: props.iframe, @@ -236,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!; @@ -268,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 e67e6d346a..629f04e517 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"; @@ -22,6 +23,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,8 +41,7 @@ interface UseStudioUrlStateParams { initialState: StudioUrlState; } -function toPersistedSelection(selection: DomEditSelection | null): StudioUrlSelectionState | null { - if (!selection) return null; +function toPersistedTarget(selection: DomEditSelection): StudioUrlSelectionTarget | null { if (!selection.id && !selection.selector) return null; return { sourceFile: selection.sourceFile || undefined, @@ -49,12 +51,109 @@ function toPersistedSelection(selection: DomEditSelection | null): StudioUrlSele }; } +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 + // the URL must still carry that one rather than throwing on the way out. + group: DomEditSelection[] = [], +): StudioUrlSelectionState | null { + if (!selection) return null; + 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 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 { + ...primary, + group: members.size > 0 ? [...members.values()] : undefined, + }; +} + function replaceHash(nextHash: string) { if (typeof window === "undefined") return; if (window.location.hash === nextHash) return; 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, @@ -67,6 +166,8 @@ export function useStudioUrlState({ rightCollapsed, activeCompPathHydrated, domEditSelection, + domEditGroupSelections, + applyMarqueeSelection, buildDomSelectionFromTarget, applyDomSelection, setRightPanelTab, @@ -82,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 => ({ @@ -91,10 +193,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 @@ -103,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; @@ -128,12 +231,32 @@ export function useStudioUrlState({ applyDomSelection(null, { revealPanel: false }); return true; } - void buildDomSelectionFromTarget(element, { preferClipAncestor: false }).then((resolved) => { - applyDomSelection(resolved, { revealPanel: false }); + const group = selection.group ?? []; + 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 }); + // Missing group members are dropped without failing the rest. + applyMarqueeSelection(members, false); }); return true; }, - [activeCompPath, applyDomSelection, buildDomSelectionFromTarget, previewIframeRef], + [ + activeCompPath, + applyDomSelection, + applyMarqueeSelection, + buildDomSelectionFromTarget, + previewIframeRef, + ], ); useEffect(() => { diff --git a/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts b/packages/studio/src/hooks/useTimelineSelectionPreviewSync.ts index 656568cb93..f57ebcd2e6 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; @@ -93,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 }); } @@ -127,6 +135,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) { diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts new file mode 100644 index 0000000000..e0f1949c28 --- /dev/null +++ b/packages/studio/src/utils/dragDebug.ts @@ -0,0 +1,98 @@ +// 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 { + logDrag("move", () => { + moveN += 1; + return moveN % 8 === 1 ? { n: moveN, ...data } : null; + }); +} + +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; +} + +/** + * 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 + * 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, () => { + 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/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/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/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.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 new file mode 100644 index 0000000000..a30b8f9bbe --- /dev/null +++ b/packages/studio/src/utils/studioDebug.ts @@ -0,0 +1,30 @@ +// 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 | null); +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; + 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 4d2c6eb801..19530c2ffc 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -77,8 +77,11 @@ function renderStudioUrlStateHarness( rightCollapsed: true, activeCompPathHydrated: true, domEditSelection: null, + domEditGroupSelections: [], + applyMarqueeSelection: () => {}, buildDomSelectionFromTarget: () => Promise.resolve(null), applyDomSelection: () => {}, + setRightPanelTab: () => {}, initialState: { activeCompPath: null, currentTime: 4.2, @@ -116,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( @@ -132,7 +141,135 @@ describe("studio url state", () => { id: "hero", selector: undefined, selectorIndex: undefined, + group: 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", + group: [ + { sourceFile: "index.html", id: "card" }, + { sourceFile: "index.html", selector: ".dot", selectorIndex: 1 }, + ], + }, + }); + + 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?.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("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", () => { @@ -228,7 +365,7 @@ describe("studio url state", () => { const harness = renderStudioUrlStateHarness({ previewIframeRef: { - current: { contentDocument: previewDoc } as HTMLIFrameElement, + current: previewIframeFor(previewDoc), }, rightPanelTab: "design", rightCollapsed: false, @@ -279,6 +416,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 e295ca578b..1dfe976f10 100644 --- a/packages/studio/src/utils/studioUrlState.ts +++ b/packages/studio/src/utils/studioUrlState.ts @@ -2,13 +2,23 @@ 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, 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". + */ + group?: StudioUrlSelectionTarget[]; +} + export interface StudioUrlState { activeCompPath: string | null; currentTime: number | null; @@ -63,19 +73,63 @@ function parseTab(value: string | null): RightPanelTab | null { return VALID_TABS.includes(value as RightPanelTab) ? (value as RightPanelTab) : null; } +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 { 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, + group: parseGroup(params.get("selGroup")), }; } @@ -130,6 +184,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.group?.length) { + params.set("selGroup", JSON.stringify(state.selection.group)); + } } return buildProjectHash(projectId, params);