feat(studio): on-canvas motion-path overlay#1610
Conversation
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Review @ 0e07ce23 — on-canvas motion-path overlay sitting on #1609's helpers. Reviewed the overlay rendering, gesture handling, coord-system transforms, lifecycle, and integration with StudioPreviewArea.
Looks good
- Coord-system handling is careful.
clientToCompuses the LIVEiframe.getBoundingClientRect()rather than the cachedrect(line 361, with the explanatory comment). The reasoning is right —rectis stored pan-surface-relative for absolute positioning, so subtracting it from a viewportclientX/Ywould offset the projection by the surface gutter. Classic class of bug, well-handled. - Shadow-DOM-aware pan-surface lookup (line 156):
el.ownerDocument?.querySelector('[data-preview-pan-surface]')instead ofel.closest(...). The comment explains why (composition iframe is in the player's shadow DOM,closest()stops at the shadow root). Future-trap defused with a comment. - Cross-realm
HTMLElementcheck (isPreviewHtmlElement, line 85): the iframe-realm constructor check is necessary and correct —node instanceof HTMLElementwould silently false on preview-document nodes. vectorEffect=\"non-scaling-stroke\"on the path + ghost + node strokes keeps stroke widths constant under zoom. Matches sibling overlay conventions.- z-index layering:
z-40overDomEditOverlay'sz-10— drag handles correctly on top of the selection box. - Visibility parity: uses the SAME
isElementVisibleInPreviewasuseDomEditOverlayRects(the selection box hook). Path + selection share one truth about what's painted. Bigger win than it looks — drift here is the kind of bug that ships unnoticed for months. elementHomefolds in--hf-studio-offset-x/y(lines 74-77) so a gesture on a manually-dragged element draws its path at the actual rendered position, not the gsap-coord home. The bug it prevents is exactly the kind of thing canvas-overlay reviewers miss.- Read-only guards (
interactive = Boolean(animId) && !isPlaying) gate every drag + add + create cleanly.
Concerns
- Global
dblclicklistener registered onwindow(line 281). Effect deps:[createMode, selection, compositionSize, iframeRef, commitMutation].commitMutationcomes fromuseDomEditContext()— if it isn't memoized (useCallback), the listener re-attaches on every render of any consumer. Worth confirming the context value is stable. If not, bothaddEventListener/removeEventListenerthrash per frame while in create mode. Also: a globalwindowdblclickwhile in create mode means double-clicking ANYWHERE in the page (e.g. on a sidebar input to select a word) enters this handler; you bounds-check against the iframe rect, but the early-out only protects from the commit, not from the perf hit of listener evaluation on every dblclick. Consider attaching to the pan-surface element instead ofwindow, or at least confirm the context callback is stable. - Drag-vs-poll race during a soft-reload. The 250ms
setInterval(line 209) keeps rebuilding geometry. If a sibling edit commits during a drag andgeometry.nodesreorders,draft.indexnow points at a stale slot — the rendered draft + the eventualcommitNode(d.ref, ...)write to the rightref(good, becauserefis captured at drag-start) but the visual draft node on screen may have moved to the wrong slot during the drag. Solo-canvas → unlikely; collab-canvas (if that ever happens) → real. Worth either pausing the geometry refresh whiledragRef.currentis set, or guarding thegeometry.nodes.map((n, i) => i === draft.index ? ...)with aref-identity match instead ofindex. Not a blocker today; tag it for future collab work. buildMotionPathGeometryequality guard compares onlypoints(line 206:prev?.points === next?.points). Ifkindflips (linear→arc) butpointshappens to identically match (degenerate edge case), the stored geometry stays at the wrongkind→ wrongMotionNodeRefdiscriminator → wrong commit mutation type. Almost impossible in practice (kind flip implies waypoint vs keyframe ref shape change which usually changes coords too), but akind-comparison on the guard costs nothing.createModeflickers during selector transitions. Line 256:!geometry && Boolean(selection?.element) && !isPlaying. Between a new selection landing and the next 250ms poll computing geometry,geometryis null → the dashed "double-click to set a destination" ring flashes on top of an element that actually HAS a path. Worth either an initialrecompute()outside the interval (already there at line 208 — good, runs once per effect mount) plus a transient "loading" state, or just suppressing create-mode rendering for the first ~50ms after selector change.
Nits
- Sub-pixel drag threshold (line 409):
if (x === Math.round(d.initX) && y === Math.round(d.initY))treats a 0.49-comp-px gesture as a click. At high zoom (scale > 1), that's a sub-screen-pixel threshold — fine. At low zoom (scale < 1, e.g. 50% canvas zoom), 0.49 comp-px ≈ 1 screen-px, so a real 1-screen-pixel slip suppresses the click-handler intent (select + park playhead). Real but very minor friction. (nit) fallow-ignore-next-line complexityannotations on the component (line 226), the create-mode effect (line 257), andonDbl(line 262), andonUp(line 398). The component is doing real coordination work — these are justified — but theonDbleffect could split out the bounds-check + commit into a helper to drop the bypass. (nit)hasMotionPathPluginsynchronous check (line 103) returns false during the brief window before the plugin async-imports in the iframe. If a user selects + double-clicks an element FAST after iframe mount, create-mode silently no-ops on the first try (user re-tries → works). Documented behavior would help, but the path is recoverable. (nit)MotionPathNode's onPointerMove/onPointerUp are wired on every node (lines 564-565). Pointer capture is on the node element that received the down, so only the right node receives subsequent moves — but every other node still has those handlers attached. Cosmetic; React makes this cheap. (nit)
Questions
activeKeyframePctselection model: when the user clicks (no-drag) a keyframe node, you callsetActiveKeyframePct(d.ref.pct)andparkPlayheadOnKeyframe. If two animations on the same element both have a keyframe at the same pct, do they both light up?MotionPathNode'sselected = p.ref.type === \"keyframe\" && p.ref.pct === activeKeyframePct(line 560) is animation-blind. Confirm this is intended (the overlay only renders ONE animation's path at a time pereditableAnimationId, so probably fine) or that another guard prevents cross-anim highlight bleed.isPlayingre-entry:useEffectcleanup for the dblclick listener handles exit; what about a user hitting Play mid-create-mode? The effect deps includecreateModewhich depends on!isPlaying, so toggling Play unregisters cleanly. Good — but worth a single test that mounts → plays → unmounts doesn't leak the listener. (no test in this PR — acknowledged in body.)
What I didn't verify
- I didn't run the overlay manually in Studio (no local studio env). Trusted the PR body's manual-test description.
- I didn't trace
commitMutation's identity throughuseDomEditContextprovider to confirm stability — see concern 1 above. - I didn't check that the
motionPathOverlay'spoints-equality guard correctly handles geometry goingnull → non-null → null(selector changes); read says yes (prev?.points === next?.pointsmatchesundefined === undefinedonly if both null, otherwise re-renders). Confirmed by re-reading.
Solid PR. The geometry math + commit routing inherits the rigor from #1609; the overlay itself does the careful coordinate-system + shadow-DOM + cross-realm work this class of feature usually gets wrong. Main asks: (1) confirm commitMutation identity stability for the global dblclick listener, (2) consider a kind field on the geometry equality guard, (3) flicker handling on selector transition.
Not a stamp-blocker from my side; concerns are layered cleanly. Defer to Vai (the HF-runtime peer) for the GSAP plugin / soft-reload interaction lens.
— Rames D Jusso
vanceingalls
left a comment
There was a problem hiding this comment.
Reviewed at 0e07ce23. Concur with @james-russo-rames-d-jusso — the careful coordinate-system + shadow-DOM + cross-realm work is exactly what this class of feature usually gets wrong, and the overlay does it right.
Verified at HEAD:
- Cross-realm
isPreviewHtmlElementcheck uses iframecontentWindow.HTMLElement, NOTglobalThis.HTMLElement. Correct cross-realm pattern. Used at two call sites (rect-tick + dblclick handler). clientToCompuses LIVEiframe.getBoundingClientRect()rather than the cachedrect— the comment explains why (rect stored pan-surface-relative; mixing coordinate spaces would offset by the gutter). Right call.- Shadow-DOM-aware pan-surface lookup (
el.ownerDocument?.querySelector(...)instead ofel.closest) — future-trap defused with a comment. vectorEffect="non-scaling-stroke"keeps stroke widths constant under zoom.- RAF (
let raf = 0→cancelAnimationFrame(raf)) and 250mssetInterval(clearInterval(id)) both have cleanup. Points-equality guardprev?.points === next?.pointssuppresses redundant React state churn. No leak. STUDIO_KEYFRAMES_ENABLEDgate atStudioPreviewArea.tsxis an EXISTING flag from main (declared atmanualEditingAvailability.ts, defaultfalse, 7 prior consumers + changelog entry). Not introduced by this stack — decorative-gate-pattern check N/A.
Drag-vs-click ambiguity handled in onUp via Math.round(initX + screenDx/scale) === Math.round(initX) integer comparison. Uses pointer capture on grab target. Concur with @james-russo-rames-d-jusso's NIT on the sub-pixel threshold under low zoom (real but very minor friction).
Concur with @james-russo-rames-d-jusso:
- Global
dblclicklistener onwindowrather than the iframe / pan-surface — Rames's instinct to attach to the pan-surface is right. The bounds check protects the commit, not the perf-cost of listener evaluation on unrelated dblclicks. ConfirmcommitMutationidentity stability throughDomEditContextbefore optimizing. - Drag-vs-poll race: 250ms
setIntervalkeeps rebuilding geometry. If a sibling edit lands during a drag andgeometry.nodesreorders,draft.indexpoints at a stale slot. Solo-canvas safe (capturingrefat drag-start), but the fix Rames proposes (ref-identity match instead ofindex) is the right shape. prev?.points === next?.pointsequality guard: ifkindflips with identical points (degenerate edge), wrongMotionNodeRefdiscriminator → wrong commit mutation type. Almost impossible practically, but akindcomparison costs nothing.createModeflicker on selector transitions — the initialrecompute()outside the interval helps but there's still a sub-frame window of "double-click to set destination" dashed ring over an element that has a path.
Net-new — soft NIT:
useDomEditOverlayRects widens its visibility predicate from isElementVisibleForOverlay to the stronger isElementVisibleInPreview. PR body discloses this as intentional ("share one 'is the element actually painted?' rule") and it IS the right cross-overlay parity move. But it changes existing DOM-edit selection-overlay behavior: the selection box now disappears when the selected element is occluded by a later scene. Disclosed in body, plausibly desired, but worth a changelog entry — any user currently relying on the selection box being visible through scene fades sees a regression. Not a band-aid.
Net-new — soft NIT on pointer-cancel symmetry:
onMove / onUp are wired but no onPointerCancel cleanup. If a system-level pointer-cancel fires mid-drag (browser tab switch, modal hijack), dragRef.current stays set until the next onUp. Pointer capture release on cancel should cause subsequent events to skip these handlers anyway, so more theoretical than practical — but a one-line onPointerCancel={onUp} for robustness.
Net-new — inherited cross-PR drift from #1609:
editableAnimationId is playhead-blind while readRuntimeKeyframes (which feeds the geometry) is playhead-aware. For multi-position-tween elements (rare), the writer may commit to the wrong tween. Surfaces at commit time here. See my #1609 review.
Verdict: NIT. All asks are non-blocking; the selection-overlay widening is the strongest one to disclose more explicitly than the body does today.
Review by Via
…nd-aware geometry guard, gate createMode, screen-space drag threshold
ec4218d to
395c6f7
Compare
0e07ce2 to
0392a98
Compare
|
Subsumed by #1605 (retargeted to main with the full stack) |
* chore(producer): shim __filename/__dirname in the CJS banner Bundled CJS deps like wawoff2 call __dirname; without the shim they throw "__dirname is not defined in ES module" at render time. Also ignore .zed/. * chore(producer): use a template literal for the CJS banner (review nit) * feat(core): add GSAP keyframe + motion-path source mutations Array-form keyframe removal in both the recast and acorn writers, plus update/add/remove-motion-path-point and add-motion-path. Exclude _auto and data from tween property-group classification. * fix(core): address #1554 review — data-exclusion test, split-fix doc, motion-path sentinel, parity blocks - Regression test for the `data` GSAP-key exclusion (parallel to _auto). - splitAnimationsInScript: documented that .fromTo()/.to() correctly stay out of the from-branch (only .from() reverts) and the <= boundary; added mid-flight straddle tests. - addMotionPathToScript failure path returns id: null (was empty-string sentinel); caller updated. - Parity blocks for addKeyframeToScript array-form + updateKeyframeInScript (mirroring removeKeyframeFromScript). Surfaced a latent acorn array-form partial-props merge bug — documented as it.skip with a ready assertion (acorn cutover follow-up). * feat(core): route motion-path mutations through studio-api + fix clip stamping Wire the new mutations into the file save route. Only authored clips suppress descendant stamping, so auto-stamped animated scenes can inline-expand. Hide in-flow timed clips with `display:none` only when they are LEAF clips (no nested timed clips). `display:none` on a container removes its whole subtree, hiding descendants that are still inside their own visibility window — e.g. an in-flow composition root whose effective window clamps to the timeline end would black out a child video that should still show (the hdr-hlg regression). Containers keep `visibility:hidden`, which a visible descendant can override; only leaves leave the flow, which is all the split-overlap case needs. * feat(core): strip legacy path-offset/rotation + drop obsolete studio lint rule A position or rotation add/set mutation makes the GSAP timeline the single source of truth for that channel, so any lingering --hf-studio-offset / --hf-studio-rotation CSS var must be cleared to avoid double-applying. stripStudioEditsFromTarget now clears both channels, and the add-strip fires for the position AND rotation property groups. Also removes the obsolete `gsap_studio_edit_blocked` lint rule: it warned that Studio cannot save drag/resize edits to elements in a registered timeline — the exact premise the single-source work inverts (the timeline is now the edit target). Removed the rule, its now-unused TIMELINE_REGISTRY_ASSIGN_PATTERN import, and its 5 tests. * fix(core): address #1555 review — complete hold-sync, invalidate clip cache, strip rotation channel - HOLD_SYNC_MUTATION_TYPES: add add-motion-path (load-bearing — addMotionPathToScript authors past t=0 → first-frame snap-to-(0,0) without the hold), update-meta, shift-positions, scale-positions, split-animations. (add stays out: flat tweens only, syncPositionHoldsBeforeKeyframes is a no-op for non-keyframed tweens.) - init.ts: timedClip in-flow/leaf WeakMaps now invalidate on clipTreeSignature change; visible/hidden branches both go through isTimedClipInFlow (was .get() by accident). - keyframesWriteRotation mirrors keyframesWritePosition so a rotation-only keyframe set strips the stale --hf-studio-rotation channel. * feat(studio): GSAP runtime read layer + shared helpers * fix(studio): address #1607 review — cold-parse vs fetch-error budgets, isZeroDurationSet, array-ease tests - useGsapAnimationFetchFallback: discriminate resolved/fetch-error/cold; only the cold (warm-but-zero) race gets the full ~600ms retry budget — a hard fetch error retries once. - Extract isZeroDurationSet (was !(duration>0) duplicated); rejects NaN, documents intent. - parsePercentageKeyframes: cite GSAP even-index spread; tests that a per-entry/interior ease is stripped without shifting the other keyframes' percentages. * feat(studio): GSAP drag/commit/bridge editing infra * fix(studio): address #1608 review — facade awaits commit, strict stale-parse guard, clearProps restore BLOCKER: useSafeGsapCommitMutation now RETURNS the (.catch-chained) commit promise and the commitMutation facade awaits it — so await session.commitMutation(...) resolves AFTER the server save, fixing both consumers (useEnableKeyframes + useGestureCommit's showToast/requestSeek/idle, which were firing before the save landed). SafeGsapCommitMutation return type widened void→Promise<void> (fire-and-forget consumers ignore it). - stale-parse guard uses hasNonHoldTweenForElement (a leftover hold set no longer counts as live). - commitFlatViaKeyframes snapshots dragged gsap values before clearProps + restores after seek, so a failed commit leaves the dropped pose, not a cleared element. * feat(studio): motion-path geometry + commit helpers * docs(studio): address #1609 review — document occlusion fade-in invariant, donut limit, nearestPointOnPath t-semantics * feat(studio): on-canvas motion-path overlay * fix(studio): address #1610 review — scope dblclick to pan-surface, kind-aware geometry guard, gate createMode, screen-space drag threshold * feat(studio): keyframes flag, gesture recording + timeline/selection refinements * fix(studio): address #1611 review — fetch-first keyframe path, gated hydration, dev-gated debug + gesture warn, per-group gesture tweens - useEnableKeyframes: parse current source first (null-vs-[] distinction) so a delete-all's empty parse isn't overridden by a stale selectedGsapAnimations cache. - useStudioUrlState: freeze the hydration effect's time dep once hydrated (was re-running every tick). - useGestureRecording: dev-gated console.warn when the live-preview runtime throws (was silent). - playerStore: gate window.__playerStore behind dev (guarded import.meta.env.DEV). - useGestureCommit: partition recorded keyframes by property group → one add-with-keyframes per group, so a mixed gesture no longer yields an untagged legacy tween. * feat(studio): single-source manual offset + rotation via the GSAP timeline Dragging or rotating an element writes into the GSAP timeline (the single source of truth) instead of a parallel --hf-studio-offset / --hf-studio-rotation CSS var: static elements commit a tl.set (idempotent on re-edit), tweened elements edit keyframes, and the live preview moves via gsap.set so what you see equals what is written and renders. Removes the dual-channel CSS-var/transform reconciliation behind the fling / disappear / runaway / double-stack / wrong-start bug class — for BOTH position and rotation (gesture base read from the gsap transform, gsap.set live preview, tl.set/ keyframe commit, dropped the handleDom*Commit CSS fallbacks). Subcompositions edit the same single-source way, which surfaced and fixes: - resolve a subcomp element's source file via the composition-id map (the runtime drops the source linkage when inlining the subcomposition); - a selected element's selection box AND motion path use basic visibility, not the occlusion heuristic (a backgroundless opacity-1 scene above it is not an opaque cover); - soft reload rebuilds ONLY the committed composition's timeline, leaving other compositions' timelines intact (no cross-composition revert); - read keyframes from the element's OWN composition timeline (scan all timelines, not the first unstable key); - delete-all uses a soft reload too, so editing no longer hard-reloads the iframe. * fix(studio): address #1567 review — drop drag-intercept flag, harden softReload onerror, tighten runtime ladder, per-group gestures - DROP STUDIO_GSAP_DRAG_INTERCEPT_ENABLED: single-source GSAP intercept is the only position/rotation channel; the false branch silently killed drag+rotate (and let GSAP elements into the keyframe-corrupting CSS path). Removed flag + dead branch + env def + tests. - gsapSoftReload: plugin onerror no longer fakes success — signals onAsyncFailure so the caller full-reloads; honors __hfMotionPathPluginLoading so a concurrent reload can't queue a dup script. - gsapDragCommit: resolveDragRuntime narrows the as-any ladder; a mid-seek throw logs + drops partial reads (no phantom identity) and re-applies the drag override in finally. - MotionPathOverlay: park-timer cleanup keyed on animId change. - useGestureCommit: partitionKeyframesByGroup wraps the add-with-keyframes sites (per #1611 review). * feat(studio): patchRuntimeTweenInPlace — update a tween's values in place Defensive runtime helper: locate the element's tween in window.__timelines via the shared resolveRuntimeTween scan, update its set/keyframe vars, invalidate, and re-seek the playhead — without re-running the whole composition. Returns false (caller falls back to soft reload) for any shape it can't safely patch (no tween, dynamic/computed keyframes, motionPath arc, channel mismatch, or any error). Foundation for instant, flicker-free manual edits. * fix(studio): address #1612 review — channel-aware set resolution + decline dynamic-expression patches - resolveRuntimeTween gains an optional channels[] hint; for kind:set it prefers the set whose vars carry one of the patched channels and never returns a disjoint-only set (e.g. won't write {x,y} into a co-located {rotation} set). patchRuntimeTweenInPlace derives channels from the props. - patchSet declines (returns false → soft reload) when overwriting a string/dynamic vars[ch], instead of silently dropping the computed expression. * feat(studio): instantPatch fast path in runCommit A commit carrying an instantPatch option tries patchRuntimeTweenInPlace first; on success the preview updates in place with NO reload (instant), on false it falls back to the existing soft reload. Extracts the preview-sync tail into a testable applyPreviewSync helper. No behavior change when instantPatch is absent. * feat(studio): route static position/rotation set drags through instantPatch Static-element position and rotation set commits now attach instantPatch{selector, change:{kind:set}} so the drag updates in place with no reload. Structural ops (new tween add, delete-all, convert/split/materialize) and keyframe edits deliberately omit it and keep the soft reload — keyframe instant-patch needs object-form keyframe support in patchRuntimeTweenInPlace (deferred). * fix(studio): address #1613 review — derive instantPatch from the mutation, patch both coalesced commits, wire onAsyncFailure - commitStaticGsapPosition/Rotation derive instantPatch.change.props from the actual update-property mutation(s) sent (one source of truth → findUnsafeMutationValues-validated values flow into the patch; can't drift). - Coalesced x/y: the intermediate x commit also carries instantPatch{x}, the y commit {x,y}, so a second-POST failure still leaves the preview patched for what persisted. - applyPreviewSync passes reloadPreview as onAsyncFailure (plugin-CDN load error → full reload); per U4 the synchronous false still does NOT escalate. - (channel disambiguation from #1612 verified end-to-end: {x,y}→position set, {rotation}→rotation set.) * feat(studio): no full iframe remount for soft-reloadable edits A softReload edit (and the SDK single-script refresh) no longer escalates to a full reloadPreview() iframe remount when applySoftReload returns false — the live gsap.set already shows the value, and a remount is the worst flash + re-inlines subcomps (reverting their keyframes). verifyTimelinesPopulated now checks the expected target keys the re-run registers, so a correct scoped re-run doesn't spuriously report empty. Full reload stays only for the structural (no-softReload) and ambiguous-script paths. * feat(studio): pre-load MotionPathPlugin so motion-path edits don't async-flash ensureMotionPathPluginLoaded() runs once at the preview iframe-load seam (NLELayout onIframeLoad), eagerly loading + registering MotionPathPlugin without killing the timeline. So when a user adds a motion path to a composition that didn't originally use one, the soft reload runs synchronously instead of taking the kill-then-await-CDN async path (the flash). Idempotent + defensive; the existing async fallback stays for genuine cold-start/CDN-failure. * fix(studio): don't re-save + reload when source editor syncs externally The SourceEditor's CodeMirror update listener fired onChange on ANY docChanged — including the programmatic dispatch that syncs external content (e.g. a manual-edit commit writing the source back into the open editor). That made the editor re-save the file and bump refreshKey, fully reloading the preview iframe on every drag/keyframe edit — defeating the in-place instant patch and causing the flash. Annotate the programmatic sync (ExternalSync) and skip onChange for it, so only real keystrokes save. * fix(core): inject MotionPathPlugin into preview when a composition uses motionPath A studio-created motion path writes a gsap motionPath tween into the single-source timeline, but the preview HTML only loaded gsap core — so the first render threw "Invalid property motionPath ... Missing plugin?". Detect motionPath usage and inject MotionPathPlugin right after the composition's gsap script, version-matched to it. * fix(studio): dedup __hfMotionPathPluginLoading type decl (restack artifact) * fix(studio): address #1605 review — distinguish soft-reload failure modes + observability, SourceEditor focus guard BLOCKER: applySoftReload now returns SoftReloadResult ('applied' | 'verify-failed' | 'cannot-soft-reload') instead of a bare bool. applyPreviewSync + sdkRefresh escalate to a full reloadPreview() on the PERMANENT 'cannot-soft-reload' (no gsap/rebind hook/scopable key/script, or sync re-run threw) — fixing the silent-stale-preview U4 dropped — but still suppress the TRANSIENT 'verify-failed' (live gsap.set is correct). Telemetry: gsap_soft_reload_outcome (origin/result/escalated) + gsap_instant_patch_fallback, so the U4 invariant is enforced, not asserted. - SourceEditor: skip the programmatic external-sync replace while the editor is focused, so an in-flight commit doesn't clobber the user's uncommitted keystrokes (ExternalSync kept for unfocused). - Verified ensureMotionPathPluginLoaded already guards __hfMotionPathPluginLoading (no double-append). * fix(core): align __clipTree and __clipManifest ids via stableClipId Timeline inline expansion was dead for nested children inside index.html: the tree keyed id-less elements by a synthetic __clip-N while the manifest keyed them null, so parent<->child never joined. Both now resolve identity through stableClipId (id || data-hf-id), which every generated element has. * fix(core): strip baked runtime + tag comp root in preview assembly Comps that ship a baked inline runtime were double-loaded (preview injects its own) and the baked copy failed to parse inline (Unexpected token '<'). Strip it in buildSubCompositionHtml + the disk-fallback preview path. Also tag the comp root with data-composition-file so the studio resolves a comp's top-level elements to the right source file instead of defaulting to index.html (which made the GSAP panel parse the wrong, multi-timeline file). * feat(studio): set motion-path destination from a toolbar toggle Replaces the double-click-on-canvas UX (which painted text over the preview) with a 'Set motion destination' toggle next to Snap/Grid, shown only when the selected element can take a path. While armed, one canvas press places the destination. Also removes the dead TimelinePropertyRows component. * fix(studio): center timeline keyframe diamonds on their percentage Dropped clampDiamondLeft, which forced boundary keyframes fully inside the clip so a 0% diamond sat half a diamond right of the 0% point. Each diamond's midpoint now sits exactly on its % (the clip is overflow-visible). * fix(studio): resize static elements via tl.set, not a single-stop keyframes tween Resizing an element with no size animation wrote keyframes:{ <playhead%>: {width,height} } — one mid-point stop GSAP can't interpolate, so it rendered NaN/0 dimensions at every other frame and the element vanished (worst off 0%). Added commitStaticGsapSize (mirrors commitStaticGsapPosition): a static resize now writes tl.set({width,height}), held at all frames; re-resizing updates it in place. * fix(studio): negative-cache failed media probes Only successful probes were cached, so CORS/404 cross-origin media was re-probed every rAF-driven timeline re-derive, flooding the console. Remember failed URLs and skip them. * fix(studio): type window.setTimeout handle as number ReturnType<typeof window.setTimeout> infers NodeJS.Timeout when @types/node is present and clashes with the DOM number the call returns. Type it number. * fix(studio): drag/resize disappearance, stale-ID duplicates, soft-reload clearProps - Fix soft-reload clearProps destroying element inline styles — save cssText, clear, restore, strip only transform - Fix resize no-op on re-resize: delete+add instead of two update-property - Route set tweens through static resize path (convertToKeyframes skips sets) - Re-fetch animation ID before drag commit to prevent stale-ID duplicates - Guard editDebugLog for Node test environments - Fix NLELayout setState-during-render (move reset to useEffect) - Stop SnapToolbar pointer events propagating to canvas deselect handler - Enable click-to-add waypoints on cubic motion paths - Add whole-path drag offset (Alt+drag shifts all keyframes together) - Add Canvas shortcuts section to ShortcutsPanel - Extract useMotionPathData + commitGsapPositionFromDrag (filesize compliance) - Delete dead code (getElementDepth, isElementVisibleInPreview, unused exports)

Summary
This PR adds an on-canvas motion-path overlay to Studio. When you select an element that has positional motion (GSAP
x/ykeyframes or amotionPath), the overlay draws that motion as a dashed path directly over the preview canvas, with a draggable node at each keyframe / waypoint. You can drag nodes to reshape the motion, click on the path to insert a new stop, right-click a keyframe node to delete it, and — for an element with no motion yet — double-click the canvas to author a brand-newmotionPath. Every interaction commits to source and is undoable.This is the visible, interactive layer on top of the geometry/commit/selection helpers landed in #1609.
What's changed
MotionPathOverlay.tsx(new, ~580 LOC) — the overlay itselfAn
absolute,z-40SVG over the preview, drawn in declared composition coordinates (viewBox = 0 0 width height) so the path stays pinned to the element as GSAP applies transforms.useMotionPathData) — tracks the iframe rect everyrequestAnimationFrame, positioned relative to the[data-preview-pan-surface]wrapper (queried via the light DOM, since the composition iframe lives in the player's shadow DOM) so the SVG is clipped to the canvas under zoom/pan. Polls the GSAP runtime every 250ms (readRuntimeKeyframes→buildMotionPathGeometry) to rebuild geometry, with a points-equality guard. Resolves the target element live from the current iframe document each tick (soft reloads detach captured nodes). TracksvisibleInPreviewviaisElementVisibleInPreview.elementHome) — computes the element's layout-home center by walkingoffsetLeft/offsetTopto (not including) the composition root; GSAP x/y and motionPath coords are offsets from this. Folds in the manual CSS path offset (--hf-studio-offset-x/y) so a gesture on a manually-dragged element isn't drawn shifted.isPreviewHtmlElement) — preview nodes are instances of the iframe window'sHTMLElement, so this checks against the iframe realm's constructor.MotionPathNode.tsx, new) — each node is a diamond matching the timeline keyframe shape; selection enlarges it. Wider transparent grab target + hover×delete badge;vectorEffect="non-scaling-stroke"; themeableACCENTapplied inline (SVG attrs rejectvar()).screenDelta / scale); up either commits the move (commitNode, then parks the playhead on the edited keyframe so it previews at that keyframe) or, on a no-move, treats it as a click (selects the keyframe + parks the playhead so the next drag modifies it).motionPathwaypoint (commitAddWaypoint) for arcs or a keyframe at the interpolated tween-% (commitAddKeyframe) for linear, landing on the line so nothing jumps.×badge quick-removes a waypoint (offered only for non-cubic arcs with >2 nodes); keyframe nodes remove via right-click → the timeline'sKeyframeDiamondContextMenu.commitCreatePath).editableAnimationId); renders nothing without a rect, composition size, live anchor, or positional motion.StudioPreviewArea.tsxImports
MotionPathOverlay+useCompositionDimensionsand renders it alongside the DOM-edit overlay + snap toolbar, gated behindSTUDIO_KEYFRAMES_ENABLED, fed the iframe ref, current selection, composition dimensions, andisPlaying.useDomEditOverlayRects.ts/DomEditOverlay.tsxuseDomEditOverlayRectsnow usesisElementVisibleInPreviewso the selection box and motion-path overlay share one "is the element actually painted?" rule.DomEditOverlay.tsxis a one-line whitespace touch.Why
Editing motion by typing coordinates or dragging timeline diamonds is indirect — you can't see the trajectory. Drawing the path on the canvas with draggable nodes makes spatial motion directly manipulable, with insert/delete/create available without leaving the preview. Anchoring in composition coordinates and resolving the element live keeps the path glued to its target through seeks, soft-reload commits, zoom, and pan — the cases where a naive overlay drifts.
Testing
Overlay UI, exercised manually in Studio: select an element with x/y keyframes or a motionPath and confirm the path draws on it; drag nodes (motion updates + commits, undoable); click the path to insert a stop; right-click a keyframe node to delete; double-click with a motion-less element to author a new path; verify the path stays anchored through seek, play/pause, zoom, pan, and hides when covered by a later scene. The underlying helpers it builds on are covered in #1609; this PR adds the rendering/interaction layer (limited automated coverage).
bun run buildandbun run testpass.Stack
Part of the GSAP keyframe/motion-path stack:
#1553 → #1554 → #1555 → #1607 → #1608 → #1609 → #1610 → #1611 → #1567 → #1612 → #1613 → #1605. This PR (#1610) sits on #1609. Builds independently; the combined diff across the stack is byte-identical to the originally-reviewed work.