feat(studio): on-canvas motion-path overlay#1560
Merged
miguel-heygen merged 1 commit intoJun 18, 2026
Merged
Conversation
This was referenced Jun 18, 2026
Collaborator
Author
This was referenced Jun 18, 2026
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
d9d0a45 to
ba99ae0
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
8a34a71 to
c6c515d
Compare
jrusso1020
approved these changes
Jun 18, 2026
jrusso1020
left a comment
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Approved at ba99ae07 per Rames D Jusso "stamp-ready with 3 nits." On-canvas motion-path overlay — UI layer, no architectural concerns. Deferring his 3 nits to inline resolution.
Reminder: PR body is the unfilled template (stack-wide process flag); please fill in before merge.
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
c6c515d to
87a7fd0
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
ba99ae0 to
676ba06
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
87a7fd0 to
7bb643e
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
676ba06 to
f9e5f4f
Compare
vanceingalls
left a comment
vanceingalls
left a comment
Collaborator
There was a problem hiding this comment.
Concurring with the consolidated stack review at HEAD 676ba067. One add: where the cheap test-gap-closing units live.
The 559-line overlay is mostly SVG render + pointer-event delegation into already-tested helpers (buildMotionPathGeometry, nearestPointOnPath, commitNode, commitAddWaypoint, parkPlayheadOnKeyframe). Two testable units could be lifted out with low ceremony and would pin the parts most prone to silent regression:
elementHome(packages/studio/src/components/editor/MotionPathOverlay.tsx:53-78) — the DOM-walk plus--hf-studio-offset-x/yinclusion rule. A jsdom + manual offset-stubbing test pins the "translate var() detection" branch, which is exactly the clever-but-fragile heuristic prone to silent regression on iframe-realm changes.isPreviewHtmlElement(MotionPathOverlay.tsx:84-91) — the cross-realm constructor check. Four lines of test demonstrate the iframe-realm path.
The orchestrating render is genuinely event-delegation glue where unit value is low; an @testing-library/react smoke test that asserts "given a geometry, N nodes render at the expected coordinates" would be a cheap follow-up but is the lower-priority option. Not a blocker — these closures sharpen confidence post-merge rather than gate it.
Review by Via
jrusso1020
approved these changes
Jun 18, 2026
jrusso1020
left a comment
jrusso1020
left a comment
Collaborator
There was a problem hiding this comment.
Re-approved at f9e5f4fd post-restack. On-canvas motion-path overlay; clean UI layer.
Echoing Via's R2 finding: two testable units worth extracting for cheap coverage closure — elementHome (MotionPathOverlay.tsx:53-78, DOM-walk + offset-rule, jsdom-testable) and isPreviewHtmlElement (:84-91, cross-realm constructor check, 4-line test). Non-blocking, fine as a follow-up.
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
f9e5f4f to
789f860
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
7bb643e to
0230fe5
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
789f860 to
80f1a0f
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
0230fe5 to
ce7e60a
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
80f1a0f to
896e1ae
Compare
SVG overlay with draggable keyframe-diamond nodes, hover-add, right-click remove, and context menu; mounted in the preview area.
miguel-heygen
force-pushed
the
feat/studio-motion-path-helpers
branch
from
ce7e60a to
7f979c5
Compare
miguel-heygen
force-pushed
the
feat/studio-motion-path-overlay-ui
branch
from
896e1ae to
c66ee48
Compare
Base automatically changed from
feat/studio-motion-path-helpers
to
feat/cli-keyframes-command
June 18, 2026 21:47
miguel-heygen
added a commit
that referenced
this pull request
Jun 18, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Also fixes "Add keyframe at playhead" for array-form keyframe tweens
(keyframes: [{x,y},…]):
- readElementPosition derived the captured props from anim.properties, which
is empty for array-form keyframes, so the position came back empty and the
add silently no-oped. It now falls back to the union of the keyframe stops'
property keys (then x/y).
- The add site and the toolbar button state computed the playhead percentage
from the element clip range, not the tween range, so keyframes landed at the
wrong percentage. Both now use the tween-relative basis.
- When the playhead is outside the tween range, the button is disabled instead
of silently no-oping (or, post-basis-fix, deleting the boundary keyframe).
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
miguel-heygen
added a commit
that referenced
this pull request
Jun 18, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Also fixes "Add keyframe at playhead" for array-form keyframe tweens
(keyframes: [{x,y},…]):
- readElementPosition derived the captured props from anim.properties, which
is empty for array-form keyframes, so the position came back empty and the
add silently no-oped. It now falls back to the union of the keyframe stops'
property keys (then x/y).
- The add site and the toolbar button state computed the playhead percentage
from the element clip range, not the tween range, so keyframes landed at the
wrong percentage. Both now use the tween-relative basis.
- When the playhead is outside the tween range, "add keyframe at playhead" now
extends the tween to reach it and adds a keyframe there, keeping the existing
keyframes at their absolute times (percentages rescale into the new range),
instead of disabling or no-oping.
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
miguel-heygen
added a commit
that referenced
this pull request
Jun 18, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Also makes "Add keyframe at playhead" do the right thing for every GSAP
animation shape, never disabling or silently no-oping:
- Array-form keyframe tweens (keyframes: [{x,y},…]): readElementPosition now
derives the captured props from the keyframe stops (top-level properties is
empty for array form), and the percentage uses the tween range, not the clip
range — so the add lands at the right spot instead of no-oping.
- Out of the tween range, the action extends the tween to reach the playhead
and adds a hold there, rescaling existing keyframes to keep their absolute
timing (was: disabled / destructive).
- Flat tweens (to/from/fromTo) convert to their natural keyframes, then take
the same add/extend path — so an out-of-range playhead extends them too.
- set() is promoted to a two-stop tween from the set's time to the playhead.
- motionPath/arc tweens add a waypoint at the on-path position (matching
segment, so the curve is preserved) instead of being linearized; outside the
range they extend their duration. A small merge threshold avoids duplicate
waypoints at the path endpoints.
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
miguel-heygen
added a commit
that referenced
this pull request
Jun 18, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Makes "Add keyframe at playhead" do the right thing for every GSAP animation
shape, never disabling or silently no-oping:
- Array-form keyframe tweens (keyframes: [{x,y},…]): readElementPosition now
derives the captured props from the keyframe stops (top-level properties is
empty for array form), and the percentage uses the tween range, not the clip
range — so the add lands at the right spot instead of no-oping.
- Out of the tween range, the action extends the tween to reach the playhead
and adds a hold there, rescaling existing keyframes to keep their absolute
timing (was: disabled / destructive).
- Flat tweens (to/from/fromTo) convert to their natural keyframes, then take
the same add/extend path.
- set() is promoted to a two-stop tween from the set's time to the playhead.
- motionPath/arc tweens add a waypoint at the on-path position (matching
segment, so the curve is preserved) instead of being linearized; outside the
range they extend their duration, with a merge threshold against duplicates.
Also fixes deep-link hydration: on a fresh full-page load the player runtime
isn't ready to honor the first requestSeek, so the one-shot seek latched without
moving the playhead, and the selection hydration (gated on the seek settling)
never ran — a URL with ?t=…&selId=… restored neither. A bounded heartbeat now
retries the seek until the player honors it, then the selection restores.
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
miguel-heygen
added a commit
that referenced
this pull request
Jun 18, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Makes "Add keyframe at playhead" do the right thing for every GSAP animation
shape, never disabling or silently no-oping:
- Array-form keyframe tweens (keyframes: [{x,y},…]): readElementPosition now
derives the captured props from the keyframe stops (top-level properties is
empty for array form), and the percentage uses the tween range, not the clip
range — so the add lands at the right spot instead of no-oping.
- Out of the tween range, the action extends the tween to reach the playhead
and adds a hold there, rescaling existing keyframes to keep their absolute
timing (was: disabled / destructive).
- Flat tweens (to/from/fromTo) convert to their natural keyframes, then take
the same add/extend path.
- set() is promoted to a two-stop tween from the set's time to the playhead.
- motionPath/arc tweens add a waypoint at the on-path position (matching
segment, so the curve is preserved) instead of being linearized; outside the
range they extend their duration, with a merge threshold against duplicates.
Also fixes deep-link hydration. A URL with ?t=…&selId=… restored neither the
playhead nor the selection on a fresh load: useStudioUrlState requests the seek
before the player runtime mounts its requestedSeekTime subscription, so the
request never reached pendingSeekRef, and initializeAdapter (which drained only
pendingSeekRef when the adapter became ready) started at 0 — which also blocked
selection hydration (gated on the seek settling). Fixed at the source:
initializeAdapter now reconciles the store's requestedSeekTime as well, so a seek
requested any time before the adapter is ready lands deterministically.
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
miguel-heygen
added a commit
that referenced
this pull request
Jun 19, 2026
Consolidates the studio side of the GSAP keyframe/motion-path work into one
PR: runtime read layer + shared helpers, drag/commit/bridge editing infra,
motion-path geometry + commit helpers, on-canvas motion-path overlay, and the
keyframes flag with gesture recording + timeline/selection refinements.
Makes "Add keyframe at playhead" do the right thing for every GSAP animation
shape, never disabling or silently no-oping:
- Array-form keyframe tweens (keyframes: [{x,y},…]): readElementPosition now
derives the captured props from the keyframe stops (top-level properties is
empty for array form), and the percentage uses the tween range, not the clip
range — so the add lands at the right spot instead of no-oping.
- Out of the tween range, the action extends the tween to reach the playhead
and adds a hold there, rescaling existing keyframes to keep their absolute
timing (was: disabled / destructive).
- Flat tweens (to/from/fromTo) convert to their natural keyframes, then take
the same add/extend path.
- set() is promoted to a two-stop tween from the set's time to the playhead.
- motionPath/arc tweens add a waypoint at the on-path position (matching
segment, so the curve is preserved) instead of being linearized; outside the
range they extend their duration, with a merge threshold against duplicates.
Also fixes deep-link hydration. A URL with ?t=…&selId=… restored neither the
playhead nor the selection on a fresh load: useStudioUrlState requests the seek
before the player runtime mounts its requestedSeekTime subscription, so the
request never reached pendingSeekRef, and initializeAdapter (which drained only
pendingSeekRef when the adapter became ready) started at 0 — which also blocked
selection hydration (gated on the seek settling). Fixed at the source:
initializeAdapter now reconciles the store's requestedSeekTime as well, so a seek
requested any time before the adapter is ready lands deterministically.
Supersedes the separately-reviewed studio PRs #1557, #1558, #1559, #1560, #1561.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack: GSAP keyframe + motion-path editing — on-canvas overlay (#1553 → #1561).
What
Render the selected element's GSAP motion path over the canvas: a dashed polyline through its x/y keyframes (or motionPath waypoints) with a draggable diamond node at each. Dragging a node rewrites the keyframe and commits to source; clicking a node selects that keyframe and seeks to its time.
Why
Direct, visual editing of motion paths in the Studio, matching the keyframe diamonds in the timeline.
How
MotionPathOverlay.tsx+MotionPathNode.tsx: SVG overlay drawn in declared composition coordinates, positioned over the preview iframe (which lives in the player's shadow DOM), so the path doesn't drift under GSAP transforms or canvas zoom/pan.useDomEditOverlayRects.ts/StudioPreviewArea.tsx/DomEditOverlay.tsx: overlay positioning + wiring.Test plan