feat(studio): wire snap engine into preview drag and resize gestures#1223
Conversation
4fd5589 to
3fbaee8
Compare
1e4d380 to
1bbb5f0
Compare
1bbb5f0 to
31dab4f
Compare
3fbaee8 to
360441f
Compare
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Summary: wires #1227 + #1228 into the preview gesture system. Collect snap context once at gesture start (cached on GestureState), call resolveSnapAdjustment / resolveResizeSnapAdjustment per pointermove, write guides to snapGuidesRef (imperative path through #1228's RAF loop), commit on pointer-up using lastSnappedDx/Dy instead of raw pointer delta.
PR body is detailed and accurately describes the implementation. The collect-once + per-frame-compute pattern is the right shape; the lastSnappedDx/Dy field neatly closes the "snap visually during drag, jump on release" footgun. Subtree-exclude logic in buildExcludeElements + collectVisibleElements is correct on read (continue before recursion cuts the dragged element's subtree from snap targets). No blockers — handful of concerns / nits, mostly consistency and future-proofing.
Concerns
useDomEditOverlayGestures.ts(lines 155 vs 226) — inconsistent short-circuit. Group drag gates onsc?.snapEnabled && sc.targets.length > 0; single drag gates only onsc?.snapEnabled. Whentargetsis empty, single-drag still calls the engine, computes empty candidates, and writes{ guides: [], spacingGuides: [...] }to the ref (the overlay reads empty and shows nothing — harmless waste but a wasted per-pointermove pass on a small target list). Either match the group-drag guard or drop it from group drag for consistency.- Snap-pref toggling during an active drag doesn't take effect until the next gesture. Because
snapContextis collected once at gesture start (the design point — fine),sc.snapEnabledandsc.gridEdgesare snapshots; flipping the toolbar's magnet button while mid-drag is silently ignored until release+re-drag. Acceptable per the collect-once intent, but the PR body doesn't call this out. Either document as "mid-gesture toggle deferred" or, if you want it live, readreadStudioUiPreferences().snapEnabledper pointermove (cheap, JSON.parse is the main cost, but you can pass the ref directly). - No integration tests on the gesture wiring. The unit-test-friendly engine in #1227 is great, but the bug-prone half of any snap system is exactly here: gesture-handler call order,
lastSnappedDx/Dypropagation to commit, snap-context lifetime across the rotate/drag/resize branches, the group-vs-single-drag asymmetry above.testing-library+ a jsdom RTL setup can cover the high-value cases (mount overlay → firepointerdown+ 3pointermove+pointerup→ assertonPathOffsetCommitgot snapped deltas; alt-drag asserts no snap; gesture cancel clearssnapGuidesRef). The all-manual test plan is a lot to ask of every future reviewer of this code path.
Nits
- Three near-identical snap call blocks (group drag at 154–188, single drag at 224–261, resize at 283–306) — already acknowledged via the file-level
fallow-ignore-file code-duplication. Extract a small helper likeapplySnap({ kind: 'drag' | 'resize', sc, movingRect, dx, dy, altKey })that returns{ dx, dy, guides, spacingGuides }; each branch shrinks to a one-liner and the consistency issue above goes away by construction. (nit) DomEditOverlay.tsx:155–189— thecompRectRAF loop runs unconditionally for the component's whole lifetime, doing twogetBoundingClientRect()calls + aquerySelectorper frame even when the user isn't touching the canvas. Gate the loop ongridVisible || selection != null(the only consumers ofcompRect) for a free 60Hz idle drain reclaim on the same hardware #1225 is targeting. (nit)- Group drag:
resolveDomEditGroupOverlayRect(groupG.originItems.map((item) => item.rect))is recomputed every pointermove (line 156). SinceoriginItemsis fixed for the gesture, hoist this intogroupG.originGroupBoundsat gesture start (or memoize once). (nit) - Snap defaults now live in three places —
SnapToolbar.SNAP_DEFAULTS(#1228),snapTargetCollection.collectSnapContextinline??s (#1228), andStudioPreviewArea.useStateinitial state (here, lines 121–124). Same defaults today; trivial drift waiting to happen. Single source: aSTUDIO_SNAP_DEFAULTSexport fromstudioUiPreferences.ts. (nit) StudioPreviewArea.tsx:260—</>) : nullJSX cramming. Mostly cosmetic; Prettier should normalise this if the project uses it. (nit)
Questions
- Alt-to-disable-snap — Figma uses Alt for "show distance measurements" (the dashed guides you have for equidistance), Cmd/Ctrl for "disable snap". Was Alt chosen for snap-disable deliberately (to mirror the existing Shift-15°-rotation modifier shape) or is there a future plan for distance-display under another modifier?
- The group-vs-single short-circuit asymmetry (concern #1) — intentional or oversight?
- Mid-gesture pref toggling (concern #2) — confirm "deferred until next gesture" is the desired behaviour, or worth surfacing the prefs ref into the pointermove path?
- The two RAF loops in this layer (
DomEditOverlay.compRecthere +SnapGuideOverlay.updatefrom #1228) both run unconditionally. Acceptable as-is, or worth consolidating into one shared RAF that fans out?
What I didn't verify
useDomEditOverlayRects(imported, not read) — assumed it pauses correctly during gestures via therafPausedRefmechanism, which the new code respects.applyManualOffsetDragDraft/applyManualOffsetDragCommitsemantics — read the call sites only.resolveDomEditResizeGesturemath against theeditScaleX/Ypaths — types check; didn't trace the BCR-reread invariant under non-uniform scale.- Cross-iframe pointer-event behaviour (pointerup landing outside the iframe, pointercancel from OS gestures). The
clearPointerStatehandler looks defensive; not exercised live. - Manual test plan items all unchecked — assumed Miguel will run them locally pre-merge; not blocking on it.
- Stack-coupling: the
pickBestorder-dependent tie-break (#1227) surfaces here as drag jitter at the midpoint between near edges; thebuildGridSnapEdgesscaleYdrop (#1228) surfaces as snap-vs-visual-grid divergence on non-uniform scales; the 80-target cap (#1228) surfaces as silent no-snap on dense compositions. All flagged on their respective PRs, not duplicating here.
— Rames D Jusso
360441f to
30faaba
Compare
31dab4f to
69dc0a1
Compare
30faaba to
0139d6c
Compare
930761b to
adf28b7
Compare
0139d6c to
538ac55
Compare
adf28b7 to
f712f9b
Compare
b9a1399 to
f8d1fc0
Compare
Connect snap engine and UI components to the preview canvas gesture system. Dragging or resizing elements now shows Figma-style alignment guides with snap-to-edge, snap-to-center, and grid snap. - Collect snap targets once at gesture start, reuse per frame - resolveSnapAdjustment called per pointermove during drag - resolveResizeSnapAdjustment for resize gestures - lastSnappedDx/Dy stored on GestureState for consistent drop - Alt/Option key temporarily disables snap - SnapToolbar rendered in preview area with snap prefs state
f712f9b to
40569ef
Compare

Summary
Connects the snap engine and UI components from #1227/#1228 to the Studio preview canvas gesture system. After this PR, dragging or resizing any element in the preview shows Figma-style alignment guides, and the element snaps to nearby edges, centers, and grid lines.
How the wiring works
The integration follows a collect-once, compute-per-frame pattern — the same approach used by Figma and After Effects for their snap systems:
Gesture start (
domEditOverlayStartGesture.ts): callscollectSnapContext()to walk the iframe DOM and build snap targets for all visible elements. Targets are stored onGestureStateand reused for the entire gesture — no DOM reads during drag.Per-frame drag (
useDomEditOverlayGestures.ts): callsresolveSnapAdjustment()with the moving element's rect, proposed deltas, and the pre-collected targets. The engine returns adjusted deltas + guide positions. Guide positions are written tosnapGuidesRefwhich theSnapGuideOverlayRAF loop reads to update guide line DOM positions — zero React re-renders.Per-frame resize: calls
resolveResizeSnapAdjustment()which only snaps the active resize edges (the ones the user is dragging), not the anchored edges.Drop commit: stores
lastSnappedDx/lastSnappedDyon gesture state during drag. On pointer-up, uses the snapped deltas (not raw pointer position) for the final commit — prevents the "snap jump" where the element visually snaps during drag but jumps to unsnapped position on release.Alt/Option key: temporarily disables snap while held, same modifier pattern as existing Shift-for-15°-rotation-snap.
Files changed
domEditOverlayGestures.tssnapContext,lastSnappedDx/lastSnappedDytoGestureStateandGroupGestureStatedomEditOverlayStartGesture.tscollectSnapContext()at gesture start,buildExcludeElements()to exclude dragged elementuseDomEditOverlayGestures.tsDomEditOverlay.tsxGridOverlay+SnapGuideOverlay, tracks composition rect for grid positioningStudioPreviewArea.tsxSnapToolbar, manages snap preferences state, passes grid props to overlaySnap drop consistency
A subtle but critical detail: during drag,
resolveSnapAdjustmentreturns adjusted deltas that visually snap the element. On pointer-up, the commit must use these snapped deltas — not recompute from the raw pointer position. Without this, the element would visually snap during drag but jump to the unsnapped position on release. This is stored as two numbers on the gesture state (lastSnappedDx,lastSnappedDy) — 16 bytes, zero cost.Test plan
Sto toggle snap on/off,Gto toggle gridStack: 3/4 — depends on #1228, next: fit-to-children (#1224)