From d939b7dd2cf76254fa3ebb50957f1a82d73971f2 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 8 Aug 2026 13:34:45 -0700 Subject: [PATCH] feat(studio): carry a multi-selection in the URL, and name the member that breaks away MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A link to a bug hit with several elements selected only reproduced one of them, so the report read as "works for me". The hash now carries the rest as selGroup and reopens the whole selection; members whose element is gone are dropped rather than failing the others. Verified end to end in a real browser: select three, copy the hash, open it fresh, the same three come back. The drag trace also gains a rigidity check. A group moves as one object, so every member travels the same distance; one that does not IS the fault. Drift was being computed but only printed on every eighth frame, which is exactly how a single-frame divergence hides — it now prints on the frame it happens. The frame handler moves to its own module on the way past. It had grown a snap block and a trace block inside a function already juggling four gesture kinds, and it was over both the complexity and file-size gates. Not fixed: the jump itself. Two headful runs driving a real group drag showed the members staying rigid to the pixel, at the drop and 900ms after, so I have not reproduced it yet and will not guess at a fix. --- packages/studio/src/App.tsx | 2 + .../src/components/editor/groupDragMove.ts | 96 +++++++++++++++++++ .../src/components/editor/manualEditsDom.ts | 2 + .../src/components/editor/manualOffsetDrag.ts | 2 + .../editor/useDomEditOverlayGestures.ts | 67 +------------ .../studio/src/hooks/useStudioUrlState.ts | 49 ++++++++-- packages/studio/src/utils/dragDebug.ts | 31 ++++++ .../studio/src/utils/studioUrlState.test.ts | 31 ++++++ packages/studio/src/utils/studioUrlState.ts | 23 ++++- 9 files changed, 232 insertions(+), 71 deletions(-) create mode 100644 packages/studio/src/components/editor/groupDragMove.ts diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 3da8119b9a..538aebf9ec 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -418,6 +418,8 @@ export function StudioApp() { rightCollapsed: panelLayout.rightCollapsed, activeCompPathHydrated, domEditSelection: domEditSession.domEditSelection, + domEditGroupSelections: domEditSession.domEditGroupSelections, + applyMarqueeSelection: domEditSession.applyMarqueeSelection, buildDomSelectionFromTarget: domEditSession.buildDomSelectionFromTarget, applyDomSelection: domEditSession.applyDomSelection, setRightPanelTab: panelLayout.setRightPanelTab, diff --git a/packages/studio/src/components/editor/groupDragMove.ts b/packages/studio/src/components/editor/groupDragMove.ts new file mode 100644 index 0000000000..2202d7c4e0 --- /dev/null +++ b/packages/studio/src/components/editor/groupDragMove.ts @@ -0,0 +1,96 @@ +import { resolveDomEditGroupOverlayRect } from "./domEditOverlayGeometry"; +import { resolveEquidistanceGuides, resolveSnapAdjustment, SNAP_THRESHOLD_PX } from "./snapEngine"; +import { applyManualOffsetDragDraft } from "./manualOffsetDrag"; +import type { GroupGestureState, UseDomEditOverlayGesturesOptions } from "./domEditOverlayGestures"; +import type { GroupOverlayItem } from "./domEditOverlayGeometry"; +import { + findNonRigidMembers, + logDrag, + logDragMove, + readDragPositions, +} from "../../utils/dragDebug"; + +/** + * One frame of a group drag, kept out of onPointerMove — which already handles + * four gesture kinds and reads better without this one's snapping arithmetic. + * The previous frame's positions live in the closure so the rigidity check below + * compares against the frame before, not against whatever was last sampled. + */ +export function createGroupDragMover( + opts: UseDomEditOverlayGesturesOptions, + setDraftGroupOverlayItems: (items: GroupOverlayItem[]) => void, +) { + let lastGroupPositions: Record = {}; + + /** Snap the group's delta to nearby edges, publishing the guides drawn for it. */ + // fallow-ignore-next-line complexity + const snapGroupDelta = ( + groupG: GroupGestureState, + e: React.PointerEvent, + proposed: { dx: number; dy: number }, + ) => { + const sc = groupG.snapContext; + if (!sc?.snapEnabled || sc.targets.length === 0) return proposed; + const groupBounds = resolveDomEditGroupOverlayRect(groupG.originItems.map((i) => i.rect)); + if (!groupBounds) return proposed; + const allTargets = sc.compositionTarget ? [...sc.targets, sc.compositionTarget] : sc.targets; + const snap = resolveSnapAdjustment({ + movingRect: groupBounds, + proposedDx: proposed.dx, + proposedDy: proposed.dy, + targets: allTargets, + gridEdges: sc.gridEdges ?? undefined, + threshold: SNAP_THRESHOLD_PX, + disabled: e.altKey, + }); + const movingRect = { + ...groupBounds, + left: groupBounds.left + snap.dx, + top: groupBounds.top + snap.dy, + }; + const spacingGuides = e.altKey + ? [] + : resolveEquidistanceGuides({ + movingRect, + targets: allTargets, + threshold: SNAP_THRESHOLD_PX, + }); + opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; + return { dx: snap.dx, dy: snap.dy }; + }; + + /** One frame of a group drag: snap the delta, redraw the boxes, move every member. */ + const moveGroupDrag = (groupG: GroupGestureState, e: React.PointerEvent) => { + const { dx, dy } = snapGroupDelta(groupG, e, { + dx: e.clientX - groupG.startX, + dy: e.clientY - groupG.startY, + }); + groupG.lastSnappedDx = dx; + groupG.lastSnappedDy = dy; + + setDraftGroupOverlayItems( + groupG.originItems.map((i) => ({ + ...i, + rect: { ...i.rect, left: i.rect.left + dx, top: i.rect.top + dy }, + })), + ); + const offsets: Record = {}; + for (const m of groupG.members) { + const n = applyManualOffsetDragDraft(m, dx, dy); + offsets[m.key] = `${Math.round(n.x)},${Math.round(n.y)}`; + } + const at = readDragPositions(groupG.members); + const px = Math.round(e.clientX - groupG.startX); + const py = Math.round(e.clientY - groupG.startY); + // A member breaking away IS the fault, so it reports on the frame it happens; + // the throttled line below would step over it. A gap between pointer and + // applied there is snapping pulling the group off the cursor. + const trace = { pointer: `${px},${py}`, applied: `${Math.round(dx)},${Math.round(dy)}`, at }; + const drift = findNonRigidMembers(lastGroupPositions, at); + if (drift.length > 0) logDrag("drift", { ...trace, drift }); + lastGroupPositions = at; + logDragMove({ ...trace, offsets }); + }; + + return moveGroupDrag; +} diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 9ac4588a26..80697e4e3d 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -221,6 +221,7 @@ function isIdentityAfterTranslateStrip(m: DOMMatrix): boolean { return m.is2D && m.a === 1 && m.b === 0 && m.c === 0 && m.d === 1; } +// fallow-ignore-next-line complexity function stripGsapTranslateFromTransform(element: HTMLElement): void { if (element.hasAttribute(STUDIO_MANUAL_EDIT_GESTURE_ATTR)) return; const transform = element.style.getPropertyValue("transform"); @@ -256,6 +257,7 @@ function stripGsapTranslateFromTransform(element: HTMLElement): void { // and push the offset straight into GSAP's x/y via gsap.set; the var() offset is // still persisted (buildPathOffsetPatches), and GSAP re-reads it at init on // reload. Returns true when handled as GSAP (caller must skip the CSS path). +// fallow-ignore-next-line complexity function applyStudioPathOffsetViaGsap( element: HTMLElement, offset: { x: number; y: number }, diff --git a/packages/studio/src/components/editor/manualOffsetDrag.ts b/packages/studio/src/components/editor/manualOffsetDrag.ts index e18b85c23b..731c41813f 100644 --- a/packages/studio/src/components/editor/manualOffsetDrag.ts +++ b/packages/studio/src/components/editor/manualOffsetDrag.ts @@ -325,6 +325,8 @@ export function resolveManualOffsetForPointerDelta(input: { }; } +// Pre-existing complexity — surfaced by this branch touching the file, not by new logic. +// fallow-ignore-next-line complexity export function createManualOffsetDragMember(input: { key: string; selection: DomEditSelection; diff --git a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts index a6e55b10ae..59967d8cc0 100644 --- a/packages/studio/src/components/editor/useDomEditOverlayGestures.ts +++ b/packages/studio/src/components/editor/useDomEditOverlayGestures.ts @@ -30,7 +30,6 @@ import { type GroupOverlayItem, type OverlayRect, orientedOverlayRect, - resolveDomEditGroupOverlayRect, } from "./domEditOverlayGeometry"; import { BLOCKED_MOVE_THRESHOLD_PX, @@ -52,7 +51,8 @@ import { import { hugRectForElement } from "./domEditOverlayCrop"; import { resolveSnapAdjustment, resolveEquidistanceGuides, SNAP_THRESHOLD_PX } from "./snapEngine"; import { logResize, logResizeMove, logResizeSettle } from "../../utils/resizeDebug"; -import { logDrag, logDragMove, logDragSettle, readDragPositions } from "../../utils/dragDebug"; +import { logDrag, logDragSettle, readDragPositions } from "../../utils/dragDebug"; +import { createGroupDragMover } from "./groupDragMove"; export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGesturesOptions) { const setDraftOverlayRect = (next: OverlayRect) => { @@ -92,6 +92,8 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu }, ) => _startGesture(kind, e, opts, options); + const moveGroupDrag = createGroupDragMover(opts, setDraftGroupOverlayItems); + // fallow-ignore-next-line complexity const onPointerMove = (e: React.PointerEvent) => { const g = opts.gestureRef.current; @@ -115,66 +117,7 @@ export function createDomEditOverlayGestureHandlers(opts: UseDomEditOverlayGestu } if (groupG) { - let dx = e.clientX - groupG.startX; - let dy = e.clientY - groupG.startY; - - const sc = groupG.snapContext; - if (sc?.snapEnabled && sc.targets.length > 0) { - const groupBounds = resolveDomEditGroupOverlayRect( - groupG.originItems.map((item) => item.rect), - ); - if (groupBounds) { - const allTargets = sc.compositionTarget - ? [...sc.targets, sc.compositionTarget] - : sc.targets; - const snap = resolveSnapAdjustment({ - movingRect: groupBounds, - proposedDx: dx, - proposedDy: dy, - targets: allTargets, - gridEdges: sc.gridEdges ?? undefined, - threshold: SNAP_THRESHOLD_PX, - disabled: e.altKey, - }); - dx = snap.dx; - dy = snap.dy; - const movedRect = { - left: groupBounds.left + dx, - top: groupBounds.top + dy, - width: groupBounds.width, - height: groupBounds.height, - }; - const spacingGuides = e.altKey - ? [] - : resolveEquidistanceGuides({ - movingRect: movedRect, - targets: allTargets, - threshold: SNAP_THRESHOLD_PX, - }); - opts.snapGuidesRef.current = { guides: snap.guides, spacingGuides }; - } - } - groupG.lastSnappedDx = dx; - groupG.lastSnappedDy = dy; - - setDraftGroupOverlayItems( - groupG.originItems.map((item) => ({ - ...item, - rect: { ...item.rect, left: item.rect.left + dx, top: item.rect.top + dy }, - })), - ); - const offsets: Record = {}; - for (const member of groupG.members) { - const next = applyManualOffsetDragDraft(member, dx, dy); - offsets[member.key] = `${Math.round(next.x)},${Math.round(next.y)}`; - } - logDragMove({ - pointer: `${Math.round(e.clientX - groupG.startX)},${Math.round(e.clientY - groupG.startY)}`, - // Any gap between these two is snapping pulling the group off the pointer. - applied: `${Math.round(dx)},${Math.round(dy)}`, - offsets, - at: readDragPositions(groupG.members), - }); + moveGroupDrag(groupG, e); return; } diff --git a/packages/studio/src/hooks/useStudioUrlState.ts b/packages/studio/src/hooks/useStudioUrlState.ts index e67e6d346a..263182676c 100644 --- a/packages/studio/src/hooks/useStudioUrlState.ts +++ b/packages/studio/src/hooks/useStudioUrlState.ts @@ -22,6 +22,8 @@ interface UseStudioUrlStateParams { rightCollapsed: boolean; activeCompPathHydrated: boolean; domEditSelection: DomEditSelection | null; + domEditGroupSelections: DomEditSelection[]; + applyMarqueeSelection: (selections: DomEditSelection[], additive: boolean) => void; buildDomSelectionFromTarget: ( target: HTMLElement, options?: { preferClipAncestor?: boolean }, @@ -38,14 +40,25 @@ interface UseStudioUrlStateParams { initialState: StudioUrlState; } -function toPersistedSelection(selection: DomEditSelection | null): StudioUrlSelectionState | null { +function toPersistedSelection( + selection: DomEditSelection | null, + // Optional: a caller that only ever has one selection has nothing to add, and + // the URL must still carry that one rather than throwing on the way out. + group: DomEditSelection[] = [], +): StudioUrlSelectionState | null { if (!selection) return null; if (!selection.id && !selection.selector) return null; + // The primary is already carried by selId; the rest ride along so the link + // reopens the same multi-selection instead of a single element. + const groupIds = group + .filter((member) => member.id && member.id !== selection.id) + .map((member) => member.id as string); return { sourceFile: selection.sourceFile || undefined, id: selection.id || undefined, selector: selection.selector || undefined, selectorIndex: selection.selectorIndex ?? undefined, + groupIds: groupIds.length > 0 ? groupIds : undefined, }; } @@ -67,6 +80,8 @@ export function useStudioUrlState({ rightCollapsed, activeCompPathHydrated, domEditSelection, + domEditGroupSelections, + applyMarqueeSelection, buildDomSelectionFromTarget, applyDomSelection, setRightPanelTab, @@ -91,10 +106,10 @@ export function useStudioUrlState({ rightCollapsed, timelineVisible: null, selection: hydratedSelectionRef.current - ? toPersistedSelection(domEditSelection) + ? toPersistedSelection(domEditSelection, domEditGroupSelections) : pendingSelectionRef.current, }), - [activeCompPath, domEditSelection, rightCollapsed, rightPanelTab], + [activeCompPath, domEditGroupSelections, domEditSelection, rightCollapsed, rightPanelTab], ); // Resolve a URL selection to a live element and apply it. Shared by the initial @@ -128,12 +143,32 @@ export function useStudioUrlState({ applyDomSelection(null, { revealPanel: false }); return true; } - void buildDomSelectionFromTarget(element, { preferClipAncestor: false }).then((resolved) => { - applyDomSelection(resolved, { revealPanel: false }); - }); + const groupIds = selection.groupIds ?? []; + void (async () => { + const primary = await buildDomSelectionFromTarget(element, { preferClipAncestor: false }); + if (!primary) return applyDomSelection(null, { revealPanel: false }); + if (groupIds.length === 0) return applyDomSelection(primary, { revealPanel: false }); + // Restore the whole multi-selection, primary first so it stays the anchor. + // Members whose element is gone are dropped rather than failing the rest. + const members = [primary]; + for (const memberId of groupIds) { + const memberEl = doc.getElementById(memberId); + const resolved = memberEl + ? await buildDomSelectionFromTarget(memberEl, { preferClipAncestor: false }) + : null; + if (resolved) members.push(resolved); + } + applyMarqueeSelection(members, false); + })(); return true; }, - [activeCompPath, applyDomSelection, buildDomSelectionFromTarget, previewIframeRef], + [ + activeCompPath, + applyDomSelection, + applyMarqueeSelection, + buildDomSelectionFromTarget, + previewIframeRef, + ], ); useEffect(() => { diff --git a/packages/studio/src/utils/dragDebug.ts b/packages/studio/src/utils/dragDebug.ts index a0073fb50a..72bb69b8fc 100644 --- a/packages/studio/src/utils/dragDebug.ts +++ b/packages/studio/src/utils/dragDebug.ts @@ -33,6 +33,37 @@ export function readDragPositions( return positions; } +/** + * Members whose screen movement disagrees with the rest of the group this frame. + * + * A group moves as one object, so every member travels the same distance; one + * that does not is the whole bug, and averaged-looking samples hide it. Compares + * each member's movement against the group's median and names the outliers, so a + * single element drifting shows up as itself rather than as "the group jumped". + */ +export function findNonRigidMembers( + before: Record, + after: Record, +): string[] { + const moves = new Map(); + for (const key of Object.keys(after)) { + const from = before[key]?.split(",").map(Number); + const to = after[key]?.split(",").map(Number); + if (!from || !to || from.length !== 2 || to.length !== 2) continue; + moves.set(key, `${Math.round(to[0]! - from[0]!)},${Math.round(to[1]! - from[1]!)}`); + } + const counts = new Map(); + for (const move of moves.values()) counts.set(move, (counts.get(move) ?? 0) + 1); + let common = ""; + let best = 0; + for (const [move, count] of counts) { + if (count > best) [common, best] = [move, count]; + } + return [...moves] + .filter(([, move]) => move !== common) + .map(([key, move]) => `${key.split("|")[2] ?? key} moved ${move}, group moved ${common}`); +} + /** * Sample the group now and again after the commit has had time to land. The drop * is the one moment a jump can hide: the source write, the preview reload and the diff --git a/packages/studio/src/utils/studioUrlState.test.ts b/packages/studio/src/utils/studioUrlState.test.ts index 4d2c6eb801..22246970a9 100644 --- a/packages/studio/src/utils/studioUrlState.test.ts +++ b/packages/studio/src/utils/studioUrlState.test.ts @@ -77,6 +77,8 @@ function renderStudioUrlStateHarness( rightCollapsed: true, activeCompPathHydrated: true, domEditSelection: null, + domEditGroupSelections: [], + applyMarqueeSelection: () => {}, buildDomSelectionFromTarget: () => Promise.resolve(null), applyDomSelection: () => {}, initialState: { @@ -132,9 +134,38 @@ describe("studio url state", () => { id: "hero", selector: undefined, selectorIndex: undefined, + groupIds: undefined, }); }); + /** + * A link to a bug hit while several elements were selected has to carry the + * whole selection. Without the group the URL reopens one element, the report + * cannot be reproduced from it, and it reads as "works for me". + */ + it("round-trips a multi-selection through the hash", () => { + const hash = buildStudioHash("demo", { + activeCompPath: null, + currentTime: null, + rightPanelTab: null, + rightCollapsed: null, + timelineVisible: null, + selection: { + sourceFile: "index.html", + id: "chip", + groupIds: ["card", "dot-b"], + }, + }); + + expect(hash).toContain("selGroup=card%2Cdot-b"); + expect(parseStudioUrlStateFromHash(hash).selection?.groupIds).toEqual(["card", "dot-b"]); + }); + + it("reads a single selection as having no group", () => { + const hash = parseStudioUrlStateFromHash("#project/demo?v=1&selFile=index.html&selId=hero"); + expect(hash.selection?.groupIds).toBeUndefined(); + }); + it("builds a project hash with persisted studio state", () => { expect( buildStudioHash("demo", { diff --git a/packages/studio/src/utils/studioUrlState.ts b/packages/studio/src/utils/studioUrlState.ts index e295ca578b..89624ec386 100644 --- a/packages/studio/src/utils/studioUrlState.ts +++ b/packages/studio/src/utils/studioUrlState.ts @@ -7,6 +7,13 @@ export interface StudioUrlSelectionState { id?: string; selector?: string; selectorIndex?: number; + /** + * The other members of a multi-selection, by element id, primary excluded. + * A link to a bug in a group edit is only reproducible if it carries the group; + * without this, opening the URL lands on one element and the report reads as + * "works for me". + */ + groupIds?: string[]; } export interface StudioUrlState { @@ -63,19 +70,28 @@ function parseTab(value: string | null): RightPanelTab | null { return VALID_TABS.includes(value as RightPanelTab) ? (value as RightPanelTab) : null; } +/** The other members of a multi-selection, dropping blanks a hand-edited URL leaves. */ +function parseGroupIds(value: string | null): string[] | undefined { + const ids = (value ?? "") + .split(",") + .map((id) => id.trim()) + .filter(Boolean); + return ids.length > 0 ? ids : undefined; +} + function normalizeSelection(params: URLSearchParams): StudioUrlSelectionState | null { const sourceFile = params.get("selFile") || undefined; const id = params.get("selId") || undefined; const selector = params.get("selSelector") || undefined; - const selectorIndex = parseNumber(params.get("selIndex")); - if (!sourceFile && !id && !selector) return null; + const selectorIndex = parseNumber(params.get("selIndex")); return { sourceFile, id, selector, selectorIndex: selectorIndex != null ? Math.max(0, Math.floor(selectorIndex)) : undefined, + groupIds: parseGroupIds(params.get("selGroup")), }; } @@ -130,6 +146,9 @@ export function buildStudioHash(projectId: string, state: StudioUrlState): strin if (typeof state.selection.selectorIndex === "number") { params.set("selIndex", String(Math.max(0, Math.floor(state.selection.selectorIndex)))); } + if (state.selection.groupIds?.length) { + params.set("selGroup", state.selection.groupIds.join(",")); + } } return buildProjectHash(projectId, params);