fix(studio): resume drag-paused timelines instead of only re-seeking - #1876
Conversation
Drag start pauses every window.__timelines entry and records the list in data-hf-drag-paused-timelines; resumeGsapTimelines then removed the attribute and only re-seeked the player, never unpausing anything. The main timeline survives (seek-driven every frame) but play-state-driven sub-composition timelines froze permanently after any element drag, and deselecting could not recover them. Now unpauses exactly the recorded ids (never touching timelines the drag did not pause) before the player re-seek. Verified live: after a real drag on an animated element all scene timelines stay unpaused. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This stack of pull requests is managed by Graphite. Learn more about stacking. |
vanceingalls
left a comment
There was a problem hiding this comment.
R1 adversarial review — verdict: LGTM_WITH_NITS (COMMENT, not APPROVE per convention).
Mechanism check — symmetric with the pause side
Pause side (manualOffsetDrag.ts:352-368) does two guarded things: tl.pause() (only if !tl.paused?.()) and records the id list. Resume side (manualOffsetDrag.ts:527-532) now inverts exactly that: tl.paused(false) for each recorded id, then the pre-existing player.seek re-anchors position. Only ids the drag actually paused get resumed — a deliberately-paused-before-drag timeline stays paused. That matches the invariant.
Lifecycle / cross-cut checks
manualOffsetDrag.ts:512clears the attribute before the resume loop, so the state machine is idempotent even if the loop throws (it can't — innertry/catch), and a subsequent drag can't observe stale ids.- Multi-member drag:
createManualOffsetDragMembersets the attribute per-input-element, but the!tl.paused?.()guard at L356 means only the first member records ids; subsequent members' elements get no attribute.restoreManualOffsetDragMembers/endManualOffsetDragMemberscallresumeGsapTimelinesper member — the members without the attribute early-return at L513. No duplicate resume, no orphan. ✓ - Order (unpause → seek) is correct:
paused(false)doesn't reset time, andseek(t)then repositions from the current player time. Reversed order would produce the same visual result but risks a one-frame flash of the frozen position. - Element identity:
member.elementis the ref captured at drag-start. Even if the DOM node is detached before resume,ownerDocument.defaultViewstill resolves and__timelineslookup is by id — the window global — not the element. No mid-drag element-identity trap on this axis.
Tests — semantics, not presence
manualOffsetDrag.test.ts:365-402 asserts the resume outcome (paused state of recorded ids flips to false, non-recorded id's state is untouched, attribute is cleared, seek was called with the player's current time). That's the right shape — mutation-escapable-presence assertions would have missed the original bug where the attribute was cleared but nothing else fired. Good.
Nits (non-blocking)
- Type-signature asymmetry.
manualOffsetDrag.ts:344types the pause-side aspause?: () => void; paused?: () => boolean(nullary getter); L516 types the resume-side aspaused?: (value?: boolean) => boolean(getter/setter overload). Both match GSAP's real API but two files diverging on the same primitive invites future drift. Would unify into a shared `GsapTimelineHandle` type near the top of the file. - No wire-through test. The two added tests exercise
resumeGsapTimelinesin isolation via a manually-set attribute — they don't drivecreateManualOffsetDragMember→ attribute → resume in one pass. A single integration-shape test that pauses via the real drag-start path and asserts the resume unpauses exactly those ids would pin the pause/resume contract at both ends and catch a future refactor that renames the attribute on only one side. Not blocking; the isolation test is what would have caught the original bug. - Comma-delimited attribute value (line 365 join / 527 split) assumes no commas in timeline ids. GSAP composition ids in HF don't contain commas today, but if that ever changes the split silently drops resumes. Pre-existing, not this PR's job — flag for the followup.
- `cross-origin guard` comment on the resume-side try/catch is inherited from the pause path — the runtime iframe is same-origin so it never fires in practice. Fine to leave.
Body claim vs shipped diff
Body says "2 regression tests" — 2 tests added ✓. "16/16 in manualOffsetDrag.test.ts" and "live end-to-end with a rebuilt studio" are un-artifactable in the diff but the mechanism-level assertions are convincing.
CI: 39 required checks green at `136b5e4`. No prior reviews.
— Via
terencecho
left a comment
There was a problem hiding this comment.
Reviewed at 136b5e4.
Fix is symmetric with the pause side (manualOffsetDrag.ts:352-368 records ids after guarded tl.pause(); the resume now loops those same ids with tl.paused(false) before the pre-existing player-seek). Only drag-paused ids get resumed — a deliberately-paused-before-drag timeline is untouched. Attribute is cleared before the loop, so state is idempotent. Two added tests (manualOffsetDrag.test.ts:365-402) pin the outcome: recorded ids unpause, non-recorded main stays paused, attribute cleared, seek fires with the player time.
Type refinement on __timelines[id] from pause?: () => void to paused?: (value?: boolean) => boolean matches GSAP's real getter/setter overload.
All required checks green. LGTM.
miga-heygen
left a comment
There was a problem hiding this comment.
Approve. Independent read. Symmetric with the pause side — unpauses exactly the timelines the drag start paused (reads from data-hf-drag-paused-timelines, iterates + paused(false)), then re-seeks the player. Cross-origin try/catch guard is correct. Test pins the selective-resume invariant (main stays paused, drag-paused timelines resume) and the no-attribute no-op. Clean.
— Miga

What
One-cause fix for "selecting/dragging an element permanently kills its animations" in Studio.
createManualOffsetDragMember(drag start) pauses every timeline inwindow.__timelinesand records the list indata-hf-drag-paused-timelines.resumeGsapTimelinesthen removed the attribute and only re-seeked the player — it never unpaused anything. Themaintimeline survives because the player seeks it every frame (seek works on paused timelines), but play-state-driven sub-composition timelines froze permanently after any drag gesture, and deselecting couldn't recover them — nothing else ever resumes them. Symptom set reported: scrubbing still works, the container still animates in, the scene timelines are dead until a full reload.Fix
Resume exactly the recorded ids (
tl.paused(false)) before the player re-seek — timelines the drag didn't pause are never touched, so a deliberately-paused timeline stays paused.Verified
manualOffsetDrag.test.ts.paused:falseafterward (previously all readpaused:trueand never recovered).🤖 Generated with Claude Code