fix(sdk): moveElement survives GSAP animation per-axis via runtime delta translate#1875
fix(sdk): moveElement survives GSAP animation per-axis via runtime delta translate#1875vanceingalls wants to merge 3 commits into
Conversation
…lta translate A committed moveElement wrote data-x/data-y but nothing rendered them: hosts shimmed CSS translate, which GSAP folds into the cached transform at first parse and then discards on the animated axis at every seek — dragging an animated element kept only the un-animated axis. Spike-proven on GSAP 3.15: a translate set AFTER GSAP's first parse is never read, folded, or cleared across seeks and composes natively with the animated transform. So: - moveElement captures the pre-edit baseline once (data-hf-edit-base-x/y) - the runtime (new core runtime/positionEdits.ts, applied at timeline bind — after GSAP parse) renders translate = (data-x − base), a pure delta that composes with GSAP tweens, tl.set positions, and CSS alike - applyDraft now drives the drag preview through the same translate channel (the --hf-studio-dx/dy vars had no consumer outside authored Studio bridges), and commitPreview mirrors the committed move onto the live element so it holds without an srcdoc reload Acceptance: packages/engine/scripts/test-runtime-position-edits-browser.ts (real Chrome + GSAP + runtime IIFE, no Studio shell) — X-animated, Y-animated, and static elements hold both edited axes across the full seek range. New subpath export @hyperframes/core/runtime/position-edits. Known limitation (documented): a tween created lazily at runtime that first-parses a marked element after apply folds the edit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fixes six issues from adversarial review of the moveElement stack:
- Runtime: apply position edits at init as well as at timeline bind, so
committed moves render in compositions with no usable GSAP timeline
(CSS/WAAPI-animated or fully static) — previously the apply was
unreachable outside the boundDuration > 0 bind branch and the edit
silently vanished from reloads and renders.
- Runtime: guard bind-path re-apply against post-fold double-apply — if
the previously written translate was consumed externally (a lazily
created tween folding it into GSAP's cached transform), skip instead
of re-setting it on top ({force} escape hatch for editor commits).
- Adapter: stop writing the --hf-studio-dx/dy custom properties during
drags — compositions with the documented var-consuming drag-bridge
CSS moved by twice the pointer delta (var transform + new inline
translate). The inline translate is now the only draft channel;
deltas accumulate in adapter fields. Docs updated to match.
- Adapter: switching applyDraft to a new id reverts the abandoned
element's draft translate instead of leaving it displaced with no op.
- Adapter: cancelPreview restores the raw inline translate (removing it
when there was none), so a stylesheet-authored translate is never
promoted to a permanent inline style.
- Adapter: commitPreview reverts the draft and clears state when
dispatch throws, instead of leaving the element shifted by an
uncommitted draft.
Cleanups: reuse readCurrentTranslate from the core module (was a
verbatim copy), drop the dead __hfApplyPositionEdits window hook.
Browser acceptance test now also covers the GSAP-free composition path.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
miga-heygen
left a comment
There was a problem hiding this comment.
Review — SDK edit fix: moveElement survives GSAP animation per-axis
Verdict: LGTM ✅
This is a really well-engineered fix for a subtle and nasty bug. GSAP folds any CSS translate present at first parse into its cached transform and discards it on animated axes — so SDK moveElement edits were silently lost per-axis. The fix is a runtime delta-translate mechanism that composes additively after GSAP's initial parse, using the independent CSS translate longhand.
What's good
- Root cause is spot-on:
handleMoveElementcaptures a pre-edit baseline on first edit, andpositionEdits.tsrenders the delta at runtime viatranslatelonghand — the only mechanism that composes with GSAP without fighting its cache. - GSAP-fold detection via WeakMap (
lastAppliedTranslate) is the minimal guard against double-apply without GSAP introspection. Clean. - Iframe drag channel rewrite: direct
translateinstead of the old--hf-studio-dx/dyCSS custom properties, with proper revert/cancel/commit semantics._mirrorCommittedMovecorrectly uses_draftPrevTranslate(captured at drag start) rather than the current inline value (which would be the draft-composed value). Subtle correctness point, handled correctly. - Inverse/undo in
mutate.tscorrectly removes baseline attributes via null-valued inverse patch — verified by the test. - Idempotency:
applyPositionEditsfires at both init and timeline-bind, covering GSAP-free and post-parse windows. Capture-once viaEDIT_ORIGINAL_TRANSLATE_ATTR+ WeakMap fold guard ensures no double-apply. Comments explain the dual-call well for future maintainers. - Test coverage is exceptional: unit tests for compose/apply logic, iframe adapter draft/commit/cancel lifecycle, mutate baseline capture/inverse, AND a real-browser acceptance test with GSAP + runtime IIFE validating computed positions at 4 time samples across 3 element types with 0.5px tolerance.
Ponytail
splitTopLevelWhitespaceduplication is justified — runtime IIFE can't import studio code. Fallow-ignore comment documents the isolation.readCurrentTranslatetry/catch for cross-origin iframe edge cases — necessary.num()helper:parseFloat(value ?? "")would achieve the same as the null guard +Number.isFinitecheck, but behavior is identical either way. Pure nit.- Lean already. Ship.
12 files changed, +858 / −61
miguel-heygen
left a comment
There was a problem hiding this comment.
Audited the runtime position-edit path, SDK iframe draft/commit path, mutation baseline capture, export wiring, and the browser acceptance test. The key contracts line up: moveElement captures a first-edit baseline, runtime renders the data-x/y delta through CSS translate after init/timeline-bind, iframe drafts use the same composition path without leaving stale draft state, and inverse patches remove the baseline attributes. CI is green, including SDK/core tests, browser/runtime checks, and regression shards.
Non-blocking note: the lazy-GSAP-first-parse limitation is documented in positionEdits.ts; if we ever support late-created tweens on edited elements, that path needs a separate strategy. Not a blocker for the current bug.
— Magi
Verdict: APPROVE
Reasoning: The fix targets the GSAP per-axis loss at the right boundary, keeps editor live-preview semantics coherent, and has focused unit plus real-browser coverage for the failure mode.
james-russo-rames-d-jusso
left a comment
There was a problem hiding this comment.
Reviewed at fe9459149b17e0373104c0c44d9e653ba77f11ff (second-pass; layered under Miga's LGTM + Miguel's APPROVE from earlier today).
Note: PR body 🤖 footer + both commits (2506c94f15, fe9459149b) carry Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> trailers; HF convention flags these — worth git rebase -i before merge if not squash-merging.
Summary — Second-pass on top of the domain team's approval. I focused on cross-cutting angles they naturally deprioritized: observability on the documented degradation path, cross-repo bridge coordination, subpath-export completeness, browser-test coverage of the boundary case that ORIGINATED the bug, and third-party SDK consumer backwards-compat. Miga's + Miguel's coverage of the core mechanism (delta-translate math, iframe draft/commit lifecycle, inverse-symmetric baseline, WeakMap fold guard, idempotency at init + timeline-bind) is complete and I have nothing to add there.
Cross-repo status confirmed clean-additive: grepped Pacific + all heygen-com/* — the only remaining --hf-studio-dx/--hf-studio-dy references live inside heygen-com/hyperframes itself (the docs and iframe.ts refs are all updated to describe the new translate-based path). No paired Pacific PR required; the removed vars simply become inert without breaking any downstream reader.
Subpath export @hyperframes/core/runtime/position-edits verified complete in packages/core/package.json at HEAD — declared in both dev exports (bun/node/import/types → .ts) and publishConfig.exports (import/types → .js/.d.ts). No subpath-trap.
Second-pass concerns
-
🟡
packages/core/src/runtime/positionEdits.ts:97-103— silent degradation on the documented lazy-tween fold-loss path. Miguel flagged the limitation non-blocking; my concern is that when it hits in prod, a customer sees "mymoveElementdidn't stick" with zero diagnostic signal. The WeakMap-guard early-return here is exactly the "lazy tween first-parsed after apply and folded the translate" case — silent skip degrading to fold-loss.init.ts:3,39already importsemitAnalyticsEvent+swallow; a one-lineemitAnalyticsEvent("moveedit_fold_loss", { hfId: el.getAttribute("data-hf-id") ?? null })at the skip site turns an invisible partial failure into an observable one. Cheap; helpful for the eventual "we should support late-created tweens" workstream Miguel already noted. -
🟡
packages/engine/scripts/test-runtime-position-edits-browser.ts:107-113— both-axis-animated element uncovered. Fixture has#ax(X-only tween),#ay(Y-only),#st(static). Missing: an element animated on both axes in the same tween (gsap.fromTo("#axy", { x:0, y:0 }, { x:400, y:300, duration:4 })). This is the intersection case where per-axis fold-loss ORIGINATED per the PR body ("dragging an animated element kept only X or only Y" — implies both axes actively animated is the common shape). Adding#axywith the same 4-time-sample assertion pattern is ~10 lines and closes the last obvious geometry gap. Also worth considering: agsap.set("#el", { x: 200 }, 1.0)at a specific time (tl.set), since the PR body explicitly claims the fix "composes identically over GSAP tweens,tl.setpositions, and CSS positioning" but only the tween path is asserted in-browser. -
🟡 AI-authorship trailers. PR body ends with
🤖 Generated with Claude Code; both commits (2506c94f15,fe9459149b) carryCo-Authored-By: Claude Fable 5 <noreply@anthropic.com>. HF convention typically strips these — worth an interactive rebase before merge, or use squash-merge with a cleaned message.
Questions
- ↩️
packages/core/src/runtime/positionEdits.ts:110-118— when a customer's OWN code writes an inlinetranslate: 10px 20pxbefore runtime init AND the element carriesdata-hf-edit-base-x/y(e.g. a third-party SDK consumer chose to persist HF-style baseline attrs alongside their own translate authoring), the firstapplyPositionEditToElementcaptures10px 20pxintoEDIT_ORIGINAL_TRANSLATE_ATTRand additively composes the delta. That's semantically correct (customer's translate is preserved + edit stacks on top), but I want to check the intent — is composition-with-customer-authored-translate a supported contract, or should we detect + warn that HF is treating their inline translate as a baseline? I don't have evidence of a real consumer doing this today; asking because the SDK subpath is now public.
What I didn't verify
- Did NOT re-verify what Miga + Miguel covered: delta-translate math,
composeTranslatecorrectness across all 4 branches, iframe adapter draft/commit/cancel state machine,_mirrorCommittedMoveusing_draftPrevTranslate, inverse-symmetric baseline, unit test suites. - Did NOT check
packages/engine/scripts/test-runtime-position-edits-browser.tsfixture beyond scenario coverage — assuming Miga's "exceptional test coverage" call on the assertion mechanics. - Did NOT trace GSAP's internal 3.15 fold behavior — trusting the PR body's spike claim + Miguel's audit.
- Did NOT run the browser acceptance test locally; CI green covers it (all 40+ required checks pass at HEAD).
— Rames D Jusso
… fold-loss telemetry Addresses PR #1875 review feedback (Rames, Miga): - Prime the element's GSAP transform parse (gsap.getProperty) before the first translate apply — positioned tl.set()s and tweens that first RENDER after the apply now reuse the cache instead of folding the edit. This closes the lazy-first-parse fold-loss for any page where GSAP is loaded at apply time; the residual limitation is GSAP itself loading after the apply. Proven by the extended browser acceptance test. - Emit position_edit_fold_skipped analytics at the fold-guard skip site so the residual degradation is observable instead of silent. - Browser acceptance test: add a both-axis-animated element (the shape that originated the per-axis loss) and a positioned tl.set() element, asserted across the full seek range. - Simplify the num() null guard (review nit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks all — feedback addressed in the latest commit: @james-russo-rames-d-jusso (Rames):
@miga-heygen: @miguel-heygen: the lazy-tween limitation you flagged is now substantially closed by the parse priming above — late-created tweens on edited elements compose correctly as long as GSAP was loaded when the edit was applied. |
miguel-heygen
left a comment
There was a problem hiding this comment.
Reviewed the R2 delta at 38876885.
Verified the two prior follow-ups are addressed: the fold-skip guard now emits position_edit_fold_skipped through runtime analytics at the skip site, and the browser acceptance fixture now covers X-only, Y-only, both-axis GSAP, positioned tl.set, static, and GSAP-free paths. Also verified the GSAP cache priming is best-effort before the first translate write and stale --hf-studio-dx/dy consumers are grep-clean in HyperFrames/Pacific. CI is green.
Verdict: APPROVE
Reasoning: The incremental fix closes the prior coverage and observability gaps without changing the accepted runtime contract.
Problem
A committed
moveElement(which correctly writesdata-x/data-y) did not durably reposition an element that is GSAP-animated on that axis. After a seek or reload, the element rendered with the animation's value on its animated axis and the editor's offset only on the non-animated axis — dragging an animated element kept only X or only Y. Root cause chain:data-x/data-yhad zero render-time consumers — only the editor-model parser and generator touch them. Hosts (Pacific) papered over this with a CSS-translate shim.translatethat is present at its first parse of an element into its cached transform and setstranslate: none; an absolute tween then discards the folded value on the animated axis. Measured and reproduced (headless-Chrome spike, GSAP 3.15).translateset after GSAP's first parse is never read, folded, or cleared across seeks — it composes natively with the animated transform, with zero accumulation. That timing rule is the fix.This closes the 2026-06-29 HANDOFF (
moveElement-survives-animation) and locks the edit-vs-time semantics: canvas edits apply at every point in time; the animation plays relative to the edited position. Keyframe-at-playhead editing stays a future explicit opt-in op.Design
handleMoveElementcaptures the pre-edit baseline once per element intodata-hf-edit-base-x/y(inverse-symmetric; undo removes them).packages/core/src/runtime/positionEdits.ts) renderstranslate = (data-x − base, data-y − base)— a pure delta, so generator-built compositions that duplicatedata-xintotl.set()don't double-apply, and the edit composes identically over GSAP tweens,tl.setpositions, and CSS positioning.WeakMapguard skips re-apply when the written translate was consumed externally (lazy-tween fold), preventing double-apply; editor commits force.applyDraftcomposes the pre-drag translate with the accumulated delta (visible drag in any composition, no host CSS needed — the old--hf-studio-dx/dyvars are no longer written, which also removes a 2× drag on compositions with the documented var-consuming bridge CSS),cancelPreviewrestores the raw inline value, andcommitPreviewmirrors the committed attributes onto the live element so the position holds without an srcdoc reload.@hyperframes/core/runtime/position-edits.Hardening (from high-effort adversarial review, all fixed in the second commit)
boundDuration > 0bind branch).applyDraftto a new id reverts the abandoned element.cancelPreviewnever promotes a stylesheet-authored translate to a permanent inline style.commitPreviewreverts the draft when dispatch throws.Known limitation (documented in the module)
A tween created lazily at runtime that first-parses a marked element after the apply folds the edit (degrades to the pre-existing fold-loss, never double-applies).
Verification
packages/engine/scripts/test-runtime-position-edits-browser.ts(real Chrome + GSAP + runtime IIFE, plain embedded runtime, no Studio shell): X-animated, Y-animated, and static elements hold both edited axes at every sampled time across the full seek range (exact expected values), plus a GSAP-free composition renders the edit at init.test:hyperframe-runtime-cichain, engine + SDK typecheck — all green.fallow audit --gate new-only: 0 new findings.🤖 Generated with Claude Code