From 594ed21957b8a77a6643a7e6868c9052c6f40c26 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 1 Oct 2026 09:24:39 -0700 Subject: [PATCH 1/3] fix(studio): a move keeps an authored xPercent centring set in the saved file --- packages/parsers/src/gsapConstants.ts | 18 ++++++ packages/parsers/src/gsapParser.ts | 12 ++-- .../parsers/src/gsapWriter.parity.test.ts | 11 ++++ packages/parsers/src/gsapWriterAcorn.ts | 6 +- .../src/hooks/gsapRuntimeBridge.test.ts | 55 +++++++++++++++++++ .../studio/src/hooks/gsapRuntimeBridge.ts | 8 ++- 6 files changed, 100 insertions(+), 10 deletions(-) diff --git a/packages/parsers/src/gsapConstants.ts b/packages/parsers/src/gsapConstants.ts index 563ca09c7ff..290c9b65d79 100644 --- a/packages/parsers/src/gsapConstants.ts +++ b/packages/parsers/src/gsapConstants.ts @@ -5,6 +5,8 @@ * without pulling in gsapParser (which depends on recast / @babel/parser). */ +import type { GsapAnimation } from "./gsapSerialize.js"; + export const SUPPORTED_PROPS = [ // 2D Transforms "x", @@ -69,6 +71,22 @@ for (const [group, props] of Object.entries(PROPERTY_GROUPS) as [ for (const p of props) PROP_TO_GROUP.set(p, group); } +type PositionWrite = Pick< + GsapAnimation, + "propertyGroup" | "properties" | "fromProperties" | "keyframes" +>; + +/** A position write that sets x or y. An xPercent/yPercent centring set never duplicates one. */ +export function isXYPositionWrite(a: PositionWrite): boolean { + const xy = (props?: Record) => !!props && ("x" in props || "y" in props); + return ( + a.propertyGroup === "position" && + (xy(a.properties) || + xy(a.fromProperties) || + !!a.keyframes?.keyframes.some((k) => xy(k.properties))) + ); +} + export function classifyPropertyGroup(prop: string): PropertyGroupName { return PROP_TO_GROUP.get(prop) ?? "other"; } diff --git a/packages/parsers/src/gsapParser.ts b/packages/parsers/src/gsapParser.ts index 0e83cdbb5d9..b2c0577bf83 100644 --- a/packages/parsers/src/gsapParser.ts +++ b/packages/parsers/src/gsapParser.ts @@ -49,7 +49,11 @@ export { classifyPropertyGroup, classifyTweenPropertyGroup, } from "./gsapConstants"; -import { classifyPropertyGroup, classifyTweenPropertyGroup } from "./gsapConstants"; +import { + classifyPropertyGroup, + classifyTweenPropertyGroup, + isXYPositionWrite, +} from "./gsapConstants"; import type { PropertyGroupName } from "./gsapConstants"; import { clipTweenMatcher, hasExplicitTime } from "./clipTweens"; import { @@ -1790,8 +1794,8 @@ function removeCallFromAst(call: TweenCallInfo): void { /** * Recast twin of {@link dedupePositionWritesInScript} (acorn). Enforce "exactly * one position write per element": keep `keepId` (or the LAST position write in - * source order if stale), remove every OTHER pure-position write - * (`propertyGroup === "position"` — tl.to/from/fromTo flat-or-keyframed, tl.set, + * source order if stale), remove every OTHER x/y position write + * (`isXYPositionWrite` — tl.to/from/fromTo flat-or-keyframed, tl.set, * standalone gsap.set, incl. degenerate duration:0 tweens). Non-position writes * for the selector are left untouched. */ @@ -1807,7 +1811,7 @@ export function dedupePositionWritesInScript( return script; } const posWrites = parsed.located.filter( - (l) => l.animation.targetSelector === selector && l.animation.propertyGroup === "position", + (l) => l.animation.targetSelector === selector && isXYPositionWrite(l.animation), ); if (posWrites.length <= 1) return script; const keeper = posWrites.find((l) => l.id === keepId) ?? posWrites[posWrites.length - 1]!; diff --git a/packages/parsers/src/gsapWriter.parity.test.ts b/packages/parsers/src/gsapWriter.parity.test.ts index a5be3e89c7a..13a7f9e204d 100644 --- a/packages/parsers/src/gsapWriter.parity.test.ts +++ b/packages/parsers/src/gsapWriter.parity.test.ts @@ -198,6 +198,17 @@ describe("parity: dedupePositionWritesInScript (recast vs acorn)", () => { expect(modelOf(acornOut)).toEqual(modelOf(recastOut)); }); + it("keeps an xPercent/yPercent centring set beside an x/y tween in both writers", () => { + const centred = ` + const tl = gsap.timeline({ paused: true }); + gsap.set("#box", { xPercent: -50, yPercent: -50 }); + tl.to("#box", { x: 120, y: 60, duration: 4, ease: "none" }, 0); + `; + const keepId = parseGsapScriptAcorn(centred).animations.find((a) => a.method === "to")!.id; + expect(dedupePosAcorn(centred, "#box", keepId)).toBe(centred); + expect(dedupePosRecast(centred, "#box", keepId)).toBe(centred); + }); + it("no-op when 0 or 1 position writes — both writers", () => { const single = ` const tl = gsap.timeline({ paused: true }); diff --git a/packages/parsers/src/gsapWriterAcorn.ts b/packages/parsers/src/gsapWriterAcorn.ts index ed5f294e0ae..8077725bb97 100644 --- a/packages/parsers/src/gsapWriterAcorn.ts +++ b/packages/parsers/src/gsapWriterAcorn.ts @@ -24,7 +24,7 @@ import { type ParsedGsapAcornForWrite, type TweenCallInfo, } from "./gsapParserAcorn.js"; -import { classifyPropertyGroup } from "./gsapConstants.js"; +import { classifyPropertyGroup, isXYPositionWrite } from "./gsapConstants.js"; import type { PropertyGroupName } from "./gsapConstants.js"; import { findObjectArrayKeyframeIndex, @@ -686,7 +686,7 @@ export function removeAnimationFromScript(script: string, animationId: string): * * Keeps `keepId` (the write the commit just edited); falls back to the LAST * position write in source order (the runtime-effective one) if `keepId` is stale. - * Removes every OTHER pure-position write (`propertyGroup === "position"`, which + * Removes every OTHER x/y position write (`isXYPositionWrite`, which * covers tl.to/from/fromTo flat-or-keyframed, tl.set, and standalone gsap.set, * including degenerate duration:0 tweens). Non-position writes for the same * selector (rotation / opacity / size / mixed) are left untouched. @@ -699,7 +699,7 @@ export function dedupePositionWritesInScript( const parsed = parseGsapScriptAcornForWrite(script); if (!parsed) return script; const posWrites = parsed.located.filter( - (l) => l.animation.targetSelector === selector && l.animation.propertyGroup === "position", + (l) => l.animation.targetSelector === selector && isXYPositionWrite(l.animation), ); if (posWrites.length <= 1) return script; const keeper = posWrites.find((l) => l.id === keepId) ?? posWrites[posWrites.length - 1]!; diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts index 39c53ef3524..d6f206b51cb 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts @@ -567,3 +567,58 @@ describe("tryGsapDragIntercept — motion paths", () => { ); }); }); + +describe("tryGsapDragIntercept — position self-heal", () => { + const xyTween = { + ...stalePositionAnim, + id: "#puck-b-to-0-position", + position: 0, + resolvedStart: 0, + }; + const centringSet = { + id: "#puck-b-set-0-position", + targetSelector: "#puck-b", + propertyGroup: "position", + method: "set", + properties: { xPercent: -50, yPercent: -50 }, + position: 0, + duration: 0, + } as unknown as GsapAnimation; + const xySet = { ...centringSet, properties: { x: 10, y: 20 } } as GsapAnimation; + const liveTween = { + targets: () => [{ id: "puck-b" }], + vars: { x: -180, y: -60, duration: 2 }, + duration: () => 2, + startTime: () => 0, + }; + + async function drag(second: GsapAnimation) { + usePlayerStore.setState({ currentTime: 1 }); + const animations = [second, xyTween]; + const commitMutation = vi.fn(); + await tryGsapDragIntercept( + selection, + { x: -50, y: 30 }, + animations, + fakeIframe("puck-b", [liveTween]), + commitMutation, + async () => animations, + ); + return commitMutation.mock.calls.map(([, mutation]) => mutation); + } + + it("keeps an xPercent/yPercent centring set and moves the x/y tween", async () => { + const mutations = await drag(centringSet); + expect(mutations.map((m) => m.type)).not.toContain("consolidate-position-writes"); + expect(mutations.length).toBeGreaterThan(0); + expect(mutations.every((m) => m.animationId === xyTween.id)).toBe(true); + }); + + it("consolidates a second x/y write", async () => { + const mutations = await drag(xySet); + expect(mutations[0]).toMatchObject({ + type: "consolidate-position-writes", + keepAnimationId: xyTween.id, + }); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index d5581cdf698..fb00fe2f717 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -9,6 +9,7 @@ * easing, or seek position. */ import type { GsapAnimation, PropertyGroupName } from "@hyperframes/core/gsap-parser"; +import { isXYPositionWrite } from "@hyperframes/parsers/gsap-constants"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { usePlayerStore } from "../player/store/playerStore"; @@ -245,8 +246,7 @@ export async function tryGsapDragIntercept( // the live keyframed/real tween if present (else any), strip the rest, so the // commit below updates ONE write instead of fighting duplicates. let workingAnimations = animations; - const isPosWrite = (a: GsapAnimation) => - a.targetSelector === selector && a.propertyGroup === "position"; + const isPosWrite = (a: GsapAnimation) => a.targetSelector === selector && isXYPositionWrite(a); if (animations.filter(isPosWrite).length > 1 && fetchFallbackAnimations) { const fresh = await fetchFallbackAnimations(); const dupes = fresh.filter(isPosWrite); @@ -331,7 +331,9 @@ export async function tryGsapDragIntercept( const fresh = await fetchFallbackAnimations(); const freshMatch = fresh.find( (a) => - a.targetSelector === posAnim!.targetSelector && a.propertyGroup === posAnim!.propertyGroup, + a.targetSelector === posAnim!.targetSelector && + a.propertyGroup === posAnim!.propertyGroup && + isXYPositionWrite(a) === isXYPositionWrite(posAnim!), ); if (freshMatch && freshMatch.id !== posAnim.id) { posAnim = freshMatch; From 2798d026f7163cefcf8af7ccccee8581e9a36c00 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 1 Oct 2026 10:43:01 -0700 Subject: [PATCH 2/3] fix(studio): a drag on a centred element no longer flashes off-centre on release --- packages/studio/src/hooks/gsapDragPositionCommit.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/studio/src/hooks/gsapDragPositionCommit.ts b/packages/studio/src/hooks/gsapDragPositionCommit.ts index 794e335cc25..6641af79bc6 100644 --- a/packages/studio/src/hooks/gsapDragPositionCommit.ts +++ b/packages/studio/src/hooks/gsapDragPositionCommit.ts @@ -1,4 +1,5 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import { PROPERTY_GROUPS } from "@hyperframes/parsers/gsap-constants"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { usePlayerStore } from "../player/store/playerStore"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; @@ -179,7 +180,9 @@ async function commitFlatViaKeyframes( if (runtime && ts !== null) { const { gsapLib, el, mainTl } = runtime; const draggedValues: Record = {}; - for (const key of Object.keys(properties)) { + // Clearing x/y clears GSAP's whole transform, and the seek does not bring back an xPercent + // set outside the timeline, so the restore below carries every position channel. + for (const key of new Set([...Object.keys(properties), ...PROPERTY_GROUPS.position])) { const v = Number(gsapLib.getProperty(el, key)); if (Number.isFinite(v)) draggedValues[key] = v; } From a65719d28af6c8266ccdb5a7e4b2066d8b8affc5 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Thu, 1 Oct 2026 11:10:12 -0700 Subject: [PATCH 3/3] refactor(studio): name the channels the drag probe restores instead of commenting --- packages/studio/src/hooks/gsapDragPositionCommit.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/studio/src/hooks/gsapDragPositionCommit.ts b/packages/studio/src/hooks/gsapDragPositionCommit.ts index 6641af79bc6..7610e7e1243 100644 --- a/packages/studio/src/hooks/gsapDragPositionCommit.ts +++ b/packages/studio/src/hooks/gsapDragPositionCommit.ts @@ -180,9 +180,11 @@ async function commitFlatViaKeyframes( if (runtime && ts !== null) { const { gsapLib, el, mainTl } = runtime; const draggedValues: Record = {}; - // Clearing x/y clears GSAP's whole transform, and the seek does not bring back an xPercent - // set outside the timeline, so the restore below carries every position channel. - for (const key of new Set([...Object.keys(properties), ...PROPERTY_GROUPS.position])) { + const channelsTheClearWipes = new Set([ + ...Object.keys(properties), + ...PROPERTY_GROUPS.position, + ]); + for (const key of channelsTheClearWipes) { const v = Number(gsapLib.getProperty(el, key)); if (Number.isFinite(v)) draggedValues[key] = v; }