From 26f0341c4ad2096d01c2a74915586426d9aa7d28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 18 Jun 2026 10:41:37 -0400 Subject: [PATCH] feat(studio): GSAP drag/commit/bridge editing infra Drag-to-keyframe commit + parkPlayheadOnKeyframe, a stale-parse guard, from()/ fromTo() out-of-range replace-not-add, and the gsap.set path-offset helper. --- .../src/components/editor/manualEditsDom.ts | 94 ++++--- .../editor/manualEditsDomGsap.test.ts | 82 ++++++ .../studio/src/hooks/gsapDragCommit.test.ts | 258 ++++++++++++++++++ packages/studio/src/hooks/gsapDragCommit.ts | 171 ++++++++---- .../src/hooks/gsapRuntimeBridge.test.ts | 94 +++++++ .../studio/src/hooks/gsapRuntimeBridge.ts | 35 +-- .../studio/src/hooks/useGsapAwareEditing.ts | 30 +- .../studio/src/hooks/useGsapScriptCommits.ts | 4 +- 8 files changed, 637 insertions(+), 131 deletions(-) create mode 100644 packages/studio/src/components/editor/manualEditsDomGsap.test.ts create mode 100644 packages/studio/src/hooks/gsapDragCommit.test.ts create mode 100644 packages/studio/src/hooks/gsapRuntimeBridge.test.ts diff --git a/packages/studio/src/components/editor/manualEditsDom.ts b/packages/studio/src/components/editor/manualEditsDom.ts index 98d05bde31..c7a9fee5e0 100644 --- a/packages/studio/src/components/editor/manualEditsDom.ts +++ b/packages/studio/src/components/editor/manualEditsDom.ts @@ -250,6 +250,50 @@ function stripGsapTranslateFromTransform(element: HTMLElement): void { } } +// GSAP owns the element's `transform` (it bakes x/y into a matrix and writes +// `translate: none` every tick). Folding the drag offset into a CSS `translate` +// — as the non-GSAP path does — composes ON TOP of GSAP's transform, and the +// subsequent strip/reapply math compounds into a runaway matrix that flings the +// element off-canvas. So for GSAP-animated elements we keep `translate: none` +// and push the offset straight into GSAP's x/y via gsap.set; the var() offset is +// still persisted (buildPathOffsetPatches), and GSAP re-reads it at init on +// reload. Returns true when handled as GSAP (caller must skip the CSS path). +function applyStudioPathOffsetViaGsap( + element: HTMLElement, + offset: { x: number; y: number }, +): boolean { + if (!gsapAnimatesProperty(element, "x", "y")) return false; + element.style.setProperty("translate", "none"); + const win = element.ownerDocument.defaultView as + | (Window & { + gsap?: { + set: (el: Element, vars: Record) => void; + getProperty: (el: Element, prop: string) => number; + }; + }) + | null; + if (win?.gsap) { + const baseX = Number.parseFloat(element.getAttribute("data-hf-drag-gsap-base-x") ?? ""); + const baseY = Number.parseFloat(element.getAttribute("data-hf-drag-gsap-base-y") ?? ""); + const origX = Number.parseFloat(element.getAttribute("data-hf-drag-initial-offset-x") ?? ""); + const origY = Number.parseFloat(element.getAttribute("data-hf-drag-initial-offset-y") ?? ""); + const gsapBaseX = Number.isFinite(baseX) + ? baseX + : (win.gsap.getProperty(element, "x") as number); + const gsapBaseY = Number.isFinite(baseY) + ? baseY + : (win.gsap.getProperty(element, "y") as number); + if (!Number.isFinite(baseX)) + element.setAttribute("data-hf-drag-gsap-base-x", String(gsapBaseX)); + if (!Number.isFinite(baseY)) + element.setAttribute("data-hf-drag-gsap-base-y", String(gsapBaseY)); + const deltaX = offset.x - (Number.isFinite(origX) ? origX : 0); + const deltaY = offset.y - (Number.isFinite(origY) ? origY : 0); + win.gsap.set(element, { x: gsapBaseX + deltaX, y: gsapBaseY + deltaY }); + } + return true; +} + export function applyStudioPathOffset( element: HTMLElement, offset: { x: number; y: number }, @@ -257,6 +301,10 @@ export function applyStudioPathOffset( ): void { promoteInlineForTransform(element); writeStudioPathOffsetVars(element, offset, { updateBase: options.updateBase ?? true }); + // GSAP elements: route through gsap.set, NOT a CSS translate (would corrupt the + // matrix). Symmetrical with applyStudioPathOffsetDraft — the commit path used to + // skip this branch, which is what flung dragged GSAP elements off-canvas. + if (applyStudioPathOffsetViaGsap(element, offset)) return; element.style.setProperty( "translate", composeTranslateValue( @@ -274,45 +322,13 @@ export function applyStudioPathOffsetDraft( ): void { promoteInlineForTransform(element); writeStudioPathOffsetVars(element, offset, { updateBase: false }); - - const isGsapAnimated = gsapAnimatesProperty(element, "x", "y"); - if (isGsapAnimated) { - element.style.setProperty("translate", "none"); - const win = element.ownerDocument.defaultView as - | (Window & { - gsap?: { - set: (el: Element, vars: Record) => void; - getProperty: (el: Element, prop: string) => number; - }; - }) - | null; - if (win?.gsap) { - const baseX = Number.parseFloat(element.getAttribute("data-hf-drag-gsap-base-x") ?? ""); - const baseY = Number.parseFloat(element.getAttribute("data-hf-drag-gsap-base-y") ?? ""); - const origX = Number.parseFloat(element.getAttribute("data-hf-drag-initial-offset-x") ?? ""); - const origY = Number.parseFloat(element.getAttribute("data-hf-drag-initial-offset-y") ?? ""); - const gsapBaseX = Number.isFinite(baseX) - ? baseX - : (win.gsap.getProperty(element, "x") as number); - const gsapBaseY = Number.isFinite(baseY) - ? baseY - : (win.gsap.getProperty(element, "y") as number); - if (!Number.isFinite(baseX)) - element.setAttribute("data-hf-drag-gsap-base-x", String(gsapBaseX)); - if (!Number.isFinite(baseY)) - element.setAttribute("data-hf-drag-gsap-base-y", String(gsapBaseY)); - const deltaX = offset.x - (Number.isFinite(origX) ? origX : 0); - const deltaY = offset.y - (Number.isFinite(origY) ? origY : 0); - win.gsap.set(element, { x: gsapBaseX + deltaX, y: gsapBaseY + deltaY }); - } - } else { - // Non-GSAP elements: use CSS translate as before. - element.style.setProperty( - "translate", - composeTranslateValue(element, `${Math.round(offset.x)}px`, `${Math.round(offset.y)}px`), - ); - stripGsapTranslateFromTransform(element); - } + if (applyStudioPathOffsetViaGsap(element, offset)) return; + // Non-GSAP elements: use CSS translate as before. + element.style.setProperty( + "translate", + composeTranslateValue(element, `${Math.round(offset.x)}px`, `${Math.round(offset.y)}px`), + ); + stripGsapTranslateFromTransform(element); } /* ── Box size apply ───────────────────────────────────────────────── */ diff --git a/packages/studio/src/components/editor/manualEditsDomGsap.test.ts b/packages/studio/src/components/editor/manualEditsDomGsap.test.ts new file mode 100644 index 0000000000..9800e556ab --- /dev/null +++ b/packages/studio/src/components/editor/manualEditsDomGsap.test.ts @@ -0,0 +1,82 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from "vitest"; +import { applyStudioPathOffset, applyStudioPathOffsetDraft } from "./manualEditsDom"; + +/** + * Regression: dragging a GSAP-animated element (e.g. a flat `to(#el, {x})` tween) + * must NOT fold the offset into a CSS `translate`. GSAP owns `style.transform`, so + * a CSS translate composes on top of it and the strip/reapply math compounds into + * a runaway matrix that flings the element off-canvas. Both the live draft and the + * commit must instead push the offset into GSAP's x/y via gsap.set and keep + * `translate: none`. Before the fix, the commit (applyStudioPathOffset) skipped the + * GSAP branch the draft already had — that asymmetry caused the off-canvas jump. + */ + +function makeGsapWindow( + el: HTMLElement, + gsapSet: (e: Element, v: Record) => void, +) { + const win = el.ownerDocument.defaultView as unknown as { + __timelines?: Record; + gsap?: unknown; + }; + win.__timelines = { + playground: { + getChildren: () => [{ targets: () => [el], vars: { x: -260 } }], + }, + }; + win.gsap = { + set: gsapSet, + getProperty: () => 0, + }; +} + +afterEach(() => { + const win = window as unknown as { __timelines?: unknown; gsap?: unknown }; + delete win.__timelines; + delete win.gsap; +}); + +describe("applyStudioPathOffset — GSAP-owned transform", () => { + it("non-GSAP element folds the offset into a CSS translate var()", () => { + const el = document.createElement("div"); + document.body.appendChild(el); + + applyStudioPathOffset(el, { x: -120, y: 40 }); + + expect(el.style.translate).toContain("var(--hf-studio-offset-x"); + expect(el.style.getPropertyValue("--hf-studio-offset-x")).toBe("-120px"); + expect(el.style.getPropertyValue("--hf-studio-offset-y")).toBe("40px"); + }); + + it("GSAP element keeps translate:none and routes the offset through gsap.set", () => { + const el = document.createElement("div"); + el.id = "puck-a"; + document.body.appendChild(el); + const gsapSet = vi.fn(); + makeGsapWindow(el, gsapSet); + + applyStudioPathOffset(el, { x: -409, y: 398 }); + + // No CSS translate to collide with GSAP's transform. + expect(el.style.translate).toBe("none"); + expect(el.style.translate).not.toContain("var("); + // Offset pushed into GSAP's x/y (gsapBase 0 + delta = the offset itself here). + expect(gsapSet).toHaveBeenCalledWith(el, { x: -409, y: 398 }); + }); + + it("draft and commit treat a GSAP element identically (translate:none)", () => { + const el = document.createElement("div"); + el.id = "puck-a"; + document.body.appendChild(el); + makeGsapWindow(el, vi.fn()); + + applyStudioPathOffsetDraft(el, { x: -50, y: 10 }); + const draftTranslate = el.style.translate; + applyStudioPathOffset(el, { x: -50, y: 10 }); + const commitTranslate = el.style.translate; + + expect(draftTranslate).toBe("none"); + expect(commitTranslate).toBe("none"); + }); +}); diff --git a/packages/studio/src/hooks/gsapDragCommit.test.ts b/packages/studio/src/hooks/gsapDragCommit.test.ts new file mode 100644 index 0000000000..af9b893b19 --- /dev/null +++ b/packages/studio/src/hooks/gsapDragCommit.test.ts @@ -0,0 +1,258 @@ +import { describe, expect, it, beforeEach } from "vitest"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { + commitGsapPositionFromDrag, + parkPlayheadOnKeyframe, + type GsapDragCommitCallbacks, +} from "./gsapDragCommit"; +import { usePlayerStore } from "../player/store/playerStore"; + +// Minimal selection whose element has no drag-baseline attributes (origX/Y = 0). +const selection = (): DomEditSelection => + ({ + id: "puck-a", + selector: "#puck-a", + element: { + style: { getPropertyValue: () => "", setProperty: () => {} }, + getAttribute: () => null, + removeAttribute: () => {}, + getBoundingClientRect: () => ({ top: 0, left: 0 }), + }, + }) as unknown as DomEditSelection; + +const flatTween = (): GsapAnimation => + ({ + id: "#puck-a-to", + targetSelector: "#puck-a", + method: "to", + resolvedStart: 1.2, + duration: 2.2, + properties: { x: -260 }, + }) as unknown as GsapAnimation; + +// What the flat tween becomes after convert-to-keyframes (returned by fetchAnimations). +const convertedTween = (): GsapAnimation => + ({ + id: "#puck-a-converted", + targetSelector: "#puck-a", + method: "to", + resolvedStart: 1.2, + duration: 2.2, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0, y: 0 } }, + { percentage: 100, properties: { x: -260, y: 0 } }, + ], + }, + }) as unknown as GsapAnimation; + +function recordingCallbacks(): { + types: string[]; + mutations: Array>; + callbacks: GsapDragCommitCallbacks; +} { + const types: string[] = []; + const mutations: Array> = []; + return { + types, + mutations, + callbacks: { + commitMutation: async (_sel, mutation) => { + types.push(mutation.type as string); + mutations.push(mutation); + }, + fetchAnimations: async () => [convertedTween()], + }, + }; +} + +describe("commitGsapPositionFromDrag — flat tween", () => { + beforeEach(() => { + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + }); + + it("extends the existing tween (never spawns a parallel one) when dragged OUTSIDE its range", async () => { + usePlayerStore.setState({ currentTime: 6 }); // outside [1.2, 3.4] + const { types, callbacks } = recordingCallbacks(); + + await commitGsapPositionFromDrag( + selection(), + flatTween(), + { x: -100, y: 0 }, + { x: 0, y: 0 }, + null, + "#puck-a", + callbacks, + ); + + expect(types).toContain("convert-to-keyframes"); + expect(types).toContain("replace-with-keyframes"); // the extend + expect(types).not.toContain("add-with-keyframes"); // regression: no parallel tween + }); + + it("adds a keyframe at the playhead when dragged INSIDE its range", async () => { + usePlayerStore.setState({ currentTime: 2 }); // inside [1.2, 3.4] + const { types, callbacks } = recordingCallbacks(); + + await commitGsapPositionFromDrag( + selection(), + flatTween(), + { x: -100, y: 0 }, + { x: 0, y: 0 }, + null, + "#puck-a", + callbacks, + ); + + expect(types).toContain("add-keyframe"); + expect(types).not.toContain("add-with-keyframes"); + }); + + it("MODIFIES the selected keyframe (no extend) when one is selected, even past the tween end", async () => { + // User clicked the 100% diamond (activeKeyframePct=100), playhead drifted past + // the end. Expect: convert + add-keyframe AT 100% — not replace-with-keyframes. + usePlayerStore.setState({ currentTime: 6, activeKeyframePct: 100 }); // outside [1.2, 3.4] + const { types, mutations, callbacks } = recordingCallbacks(); + + await commitGsapPositionFromDrag( + selection(), + flatTween(), + { x: -100, y: 0 }, + { x: 0, y: 0 }, + null, + "#puck-a", + callbacks, + ); + + expect(types).toContain("add-keyframe"); + expect(types).not.toContain("replace-with-keyframes"); // not extended + const addKf = mutations.find((m) => m.type === "add-keyframe"); + expect(addKf?.percentage).toBe(100); // modified the selected endpoint + // consumed: cleared so the next free drag doesn't keep modifying + expect(usePlayerStore.getState().activeKeyframePct).toBeNull(); + // parked the playhead on the edited keyframe (1.2 start + 100% * 2.2 dur), + // so the edit is visible instead of rendering the base pose + expect(usePlayerStore.getState().requestedSeekTime).toBe(3.4); + }); +}); + +describe("commitGsapPositionFromDrag — keyframed tween backfill", () => { + beforeEach(() => { + usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null }); + }); + + const keyframedTween = (): GsapAnimation => + ({ + id: "#puck-a-kf", + targetSelector: "#puck-a", + method: "to", + resolvedStart: 1.2, + duration: 2.2, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: -260 } }, + ], + }, + }) as unknown as GsapAnimation; + + it("passes backfillDefaults so a newly-introduced prop doesn't move the other keyframes", async () => { + // Drag the 0% keyframe DOWN (introduces y on an x-only tween). The add-keyframe + // must carry backfillDefaults at the element's base so 100% gets y:0, not y:780. + usePlayerStore.setState({ currentTime: 1.2, activeKeyframePct: 0 }); + const { mutations, callbacks } = recordingCallbacks(); + + await commitGsapPositionFromDrag( + selection(), + keyframedTween(), + { x: 0, y: 780 }, // studioOffset: dragged straight down + { x: 0, y: 0 }, // gsapPos → base falls back to {0,0} (selection has no base attrs) + null, + "#puck-a", + callbacks, + ); + + const addKf = mutations.find((m) => m.type === "add-keyframe"); + expect(addKf).toBeDefined(); + expect(addKf?.percentage).toBe(0); // edited the selected 0% keyframe + expect(addKf?.properties).toMatchObject({ y: 780 }); + expect(addKf?.backfillDefaults).toEqual({ x: 0, y: 0 }); // base → 100% gets y:0 + }); +}); + +describe("commitGsapPositionFromDrag — from() tween dragged outside its range", () => { + beforeEach(() => usePlayerStore.setState({ currentTime: 0, activeKeyframePct: null })); + + const fromTween = (): GsapAnimation => + ({ + id: "#title-from-400", + targetSelector: "#title", + method: "from", + resolvedStart: 0.4, + duration: 0.9, + properties: { y: 70 }, + }) as unknown as GsapAnimation; + + it("REPLACES the split position from() tween (no parallel tween → no drop jump)", async () => { + usePlayerStore.setState({ currentTime: 2.13 }); // outside [0.4, 1.3] + const types: string[] = []; + const mutations: Array> = []; + const callbacks: GsapDragCommitCallbacks = { + commitMutation: async (_s, m) => { + types.push(m.type as string); + mutations.push(m); + }, + // After split-into-property-groups, the position group is a from() tween (no keyframes). + fetchAnimations: async () => [ + { + id: "#title-from-400-position", + targetSelector: "#title", + method: "from", + propertyGroup: "position", + resolvedStart: 0.4, + duration: 0.9, + properties: { y: 70 }, + } as unknown as GsapAnimation, + ], + }; + + await commitGsapPositionFromDrag( + selection(), + fromTween(), + { x: 0, y: -333 }, + { x: 0, y: 0 }, + null, + "#title", + callbacks, + ); + + expect(types).toContain("split-into-property-groups"); + expect(types).toContain("replace-with-keyframes"); + expect(types).not.toContain("add-with-keyframes"); // regression: no parallel tween + const replace = mutations.find((m) => m.type === "replace-with-keyframes"); + expect(replace?.animationId).toBe("#title-from-400-position"); // replaces the split from() + }); +}); + +describe("parkPlayheadOnKeyframe", () => { + beforeEach(() => usePlayerStore.setState({ requestedSeekTime: null })); + + const tween = (): GsapAnimation => + ({ + id: "#x", + targetSelector: "#x", + method: "to", + resolvedStart: 1.2, + duration: 2.2, + }) as unknown as GsapAnimation; + + it("seeks to the keyframe's absolute time so the element previews AT it, not at base", () => { + parkPlayheadOnKeyframe(tween(), 0); // tween start + expect(usePlayerStore.getState().requestedSeekTime).toBe(1.2); + parkPlayheadOnKeyframe(tween(), 100); // tween end + expect(usePlayerStore.getState().requestedSeekTime).toBe(3.4); + parkPlayheadOnKeyframe(tween(), 50); // midpoint + expect(usePlayerStore.getState().requestedSeekTime).toBe(2.3); + }); +}); diff --git a/packages/studio/src/hooks/gsapDragCommit.ts b/packages/studio/src/hooks/gsapDragCommit.ts index dffc1b07a6..8ab3599ebd 100644 --- a/packages/studio/src/hooks/gsapDragCommit.ts +++ b/packages/studio/src/hooks/gsapDragCommit.ts @@ -32,6 +32,17 @@ export function computeCurrentPercentage( return computeElementPercentage(usePlayerStore.getState().currentTime, selection, animation); } +// When a drag edits a SELECTED keyframe, park the playhead on that keyframe's exact +// time. Otherwise the playhead can sit a frame outside the tween (e.g. 1.1666 vs a +// 1.2 start), so the post-commit reseek renders the element's base pose and the edit +// looks like it snapped away. Keeping the playhead on the edited keyframe avoids that. +export function parkPlayheadOnKeyframe(anim: GsapAnimation, pct: number): void { + const ts = resolveTweenStart(anim); + const td = resolveTweenDuration(anim); + if (ts == null || !td || td <= 0) return; + usePlayerStore.getState().requestSeek(roundTo3(ts + (pct / 100) * td)); +} + // ── Dynamic keyframe materialization ────────────────────────────────────── export async function materializeIfDynamic( @@ -93,6 +104,7 @@ async function extendTweenAndAddKeyframe( tweenDuration: number, callbacks: GsapDragCommitCallbacks, beforeReload?: () => void, + backfillDefaults?: Record, ): Promise { const tweenEnd = tweenStart + tweenDuration; const newStart = Math.min(targetTime, tweenStart); @@ -104,7 +116,13 @@ async function extendTweenAndAddKeyframe( for (const kf of existingKfs) { const absTime = tweenStart + (kf.percentage / 100) * tweenDuration; const newPct = Math.round(((absTime - newStart) / newDuration) * 1000) / 10; - remappedKfs.push({ percentage: newPct, properties: { ...kf.properties } }); + const props: Record = { ...kf.properties }; + // Backfill props the new keyframe introduces but this one lacks, so GSAP + // doesn't hold the new prop's value across keyframes that omit it. + for (const k of Object.keys(properties)) { + if (!(k in props) && backfillDefaults?.[k] != null) props[k] = backfillDefaults[k]; + } + remappedKfs.push({ percentage: newPct, properties: props }); } const targetPct = Math.round(((targetTime - newStart) / newDuration) * 1000) / 10; @@ -133,9 +151,11 @@ async function commitKeyframedPosition( properties: Record, callbacks: GsapDragCommitCallbacks, beforeReload?: () => void, + backfillDefaults?: Record, ): Promise { const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState(); - const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim); + const computedPct = computeCurrentPercentage(selection, anim); + const pct = activeKeyframePct ?? computedPct; await callbacks.commitMutation( selection, { @@ -143,10 +163,18 @@ async function commitKeyframedPosition( animationId: anim.id, percentage: pct, properties, + // Backfill any newly-introduced prop (e.g. `y` on an x-only tween) into the + // OTHER keyframes at the element's base value. Without it, GSAP holds the new + // prop's value across keyframes that omit it — so editing one keyframe drags + // the others to the same position. + ...(backfillDefaults ? { backfillDefaults } : {}), }, { label: `Move layer (keyframe ${pct}%)`, softReload: true, beforeReload }, ); - if (activeKeyframePct != null) setActiveKeyframePct(null); + if (activeKeyframePct != null) { + setActiveKeyframePct(null); + parkPlayheadOnKeyframe(anim, pct); + } } /** @@ -162,11 +190,16 @@ async function commitFlatViaKeyframes( beforeReload?: () => void, iframe?: HTMLIFrameElement | null, selector?: string, + backfillDefaults?: Record, ): Promise { const ct = usePlayerStore.getState().currentTime; const ts = resolveTweenStart(anim); const td = resolveTweenDuration(anim); - const outsideRange = ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); + // A flat tween shows two diamonds (0% / 100%). If the user selected one and then + // dragged, modify THAT endpoint — don't extend or place at the drifted playhead. + const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState(); + const outsideRange = + activeKeyframePct == null && ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); // Read the runtime position at the tween's start time so the 0% keyframe // captures the actual interpolated value (e.g. x=300 after a preceding slide), @@ -180,6 +213,13 @@ async function commitFlatViaKeyframes( const timelines = iframeWin?.__timelines; const mainTl = timelines ? (Object.values(timelines)[0] as any) : null; if (gsapLib && el && mainTl?.seek) { + // Clear the live drag's gsap overrides first. Otherwise a property the + // tween doesn't animate (e.g. `y` on a flat `to({x})`) keeps the dragged + // value through the seek and pollutes the 0% keyframe (it would start at + // the dropped position instead of animating there). After clearing, the + // seek reapplies the timeline's real interpolated values for animated + // props, and untweened props fall back to their base (0). + gsapLib.set(el, { clearProps: Object.keys(properties).join(",") }); mainTl.seek(ts); for (const key of Object.keys(properties)) { const v = Number(gsapLib.getProperty(el, key)); @@ -193,50 +233,42 @@ async function commitFlatViaKeyframes( } if (outsideRange && ts !== null) { - // Outside the tween's range: add a brand new keyframed tween at the drag - // time instead of extending/replacing the existing one. This keeps all - // existing tweens untouched and creates a clean hold at the dragged position. - const tweenEnd = ts + td; - const holdStart = ct > tweenEnd ? tweenEnd : ct; - const holdEnd = ct > tweenEnd ? ct : ts; - const holdDur = Math.max(0.01, holdEnd - holdStart); - const kfs = - ct > tweenEnd - ? [ - { percentage: 0, properties: resolvedFromValues }, - { percentage: 100, properties }, - ] - : [ - { percentage: 0, properties }, - { percentage: 100, properties: resolvedFromValues }, - ]; - console.log( - "[drag:5] outside range — adding new tween", - JSON.stringify({ - ct, - ts, - td, - holdStart: roundTo3(holdStart), - holdDur: roundTo3(holdDur), - from: resolvedFromValues, - to: properties, - }), - ); + // Outside the tween's range: EXTEND the existing tween to reach the playhead + // instead of spawning a parallel tween (which left the element with two + // competing tweens, so edits hit one while the selected keyframe lived on the + // other). Convert the flat tween to keyframes, then extend + add at the + // playhead — existing keyframes keep their absolute times. + const coalesceKey = `gsap:convert-drag:${anim.id}`; await callbacks.commitMutation( selection, { - type: "add-with-keyframes", - targetSelector: anim.targetSelector, - position: roundTo3(holdStart), - duration: roundTo3(holdDur), - keyframes: kfs, + type: "convert-to-keyframes", + animationId: anim.id, + ...(Object.keys(resolvedFromValues).length > 0 ? { resolvedFromValues } : {}), }, - { label: "Move layer (new keyframe)", softReload: true, beforeReload }, + { label: "Convert to keyframes for drag", skipReload: true, coalesceKey }, + ); + const fresh = callbacks.fetchAnimations ? await callbacks.fetchAnimations() : []; + const converted = + fresh.find((a) => a.targetSelector === anim.targetSelector && a.keyframes) ?? anim; + const convertedStart = resolveTweenStart(converted) ?? ts; + const convertedDur = resolveTweenDuration(converted) || td; + await extendTweenAndAddKeyframe( + selection, + converted, + properties, + ct, + convertedStart, + convertedDur, + callbacks, + beforeReload, ); return; } - // Inside range: convert the flat tween to keyframes, then add at current %. + // Inside range (or a selected endpoint): convert the flat tween to keyframes, + // then add/modify at the target %. A selected diamond pins the % to that endpoint + // (0 / 100) so the drag edits it exactly; otherwise use the playhead %. const coalesceKey = `gsap:convert-drag:${anim.id}`; await callbacks.commitMutation( selection, @@ -247,7 +279,9 @@ async function commitFlatViaKeyframes( }, { label: "Convert to keyframes for drag", skipReload: true, coalesceKey }, ); - const pct = computeCurrentPercentage(selection, anim); + const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim); + const editedSelected = activeKeyframePct != null; + if (editedSelected) setActiveKeyframePct(null); await callbacks.commitMutation( selection, @@ -256,9 +290,11 @@ async function commitFlatViaKeyframes( animationId: anim.id, percentage: pct, properties, + ...(backfillDefaults ? { backfillDefaults } : {}), }, { label: `Move layer (keyframe ${pct}%)`, softReload: true, beforeReload, coalesceKey }, ); + if (editedSelected) parkPlayheadOnKeyframe(anim, pct); } // ── Main drag commit ────────────────────────────────────────────────────── @@ -302,6 +338,9 @@ export async function commitGsapPositionFromDrag( el.removeAttribute("data-hf-drag-initial-offset-y"); }; + // The element's base (un-animated) pose — used to backfill any prop the drag + // newly introduces (e.g. `y` on an x-only tween) into the other keyframes. + const backfillDefaults: Record = { x: baseGsapX, y: baseGsapY }; const ct = usePlayerStore.getState().currentTime; if (anim.keyframes) { const newId = await materializeIfDynamic(anim, iframe, callbacks.commitMutation, selection); @@ -311,7 +350,10 @@ export async function commitGsapPositionFromDrag( const ts = resolveTweenStart(effectiveAnim); const td = resolveTweenDuration(effectiveAnim); const outsideRange = ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); - if (outsideRange) { + // A selected keyframe (clicked diamond) means "modify THIS keyframe" — never + // extend, even if the playhead drifted a frame past the tween's end. + const hasSelectedKeyframe = usePlayerStore.getState().activeKeyframePct != null; + if (outsideRange && !hasSelectedKeyframe) { await extendTweenAndAddKeyframe( selection, effectiveAnim, @@ -321,15 +363,26 @@ export async function commitGsapPositionFromDrag( td, callbacks, restoreOffset, + backfillDefaults, ); } else { - await commitKeyframedPosition(selection, effectiveAnim, dragProps, callbacks, restoreOffset); + await commitKeyframedPosition( + selection, + effectiveAnim, + dragProps, + callbacks, + restoreOffset, + backfillDefaults, + ); } } else if (anim.method === "from" || anim.method === "fromTo") { const ct = usePlayerStore.getState().currentTime; const ts = resolveTweenStart(anim); const td = resolveTweenDuration(anim); - const outsideRange = ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); + // A selected keyframe means "modify it" — skip the extend/split branch. + const hasSelectedKeyframe = usePlayerStore.getState().activeKeyframePct != null; + const outsideRange = + !hasSelectedKeyframe && ts !== null && td > 0 && (ct < ts - 0.01 || ct > ts + td + 0.01); const dragProps: Record = { x: newX, y: newY }; if (outsideRange && ts !== null) { @@ -359,6 +412,7 @@ export async function commitGsapPositionFromDrag( posTd, callbacks, restoreOffset, + backfillDefaults, ); return; } @@ -389,19 +443,26 @@ export async function commitGsapPositionFromDrag( } keyframes.sort((a, b) => a.percentage - b.percentage); + // REPLACE the split position `from()` tween with the keyframed one (same id) + // instead of adding a parallel tween. Two position tweens on the same element + // fight on the shared axis — the leftover `from()` snaps to its natural state + // on the soft-reload re-seek, which is the visible "jump" after dropping. + const baseKf = { + targetSelector: anim.targetSelector, + position: roundTo3(newStart), + duration: roundTo3(newDuration), + keyframes, + }; await callbacks.commitMutation( selection, - { - type: "add-with-keyframes", - targetSelector: anim.targetSelector, - position: roundTo3(newStart), - duration: roundTo3(newDuration), - keyframes, - }, + existingPosAnim + ? { type: "replace-with-keyframes", animationId: existingPosAnim.id, ...baseKf } + : { type: "add-with-keyframes", ...baseKf }, { label: "Move layer (from extended)", softReload: true, beforeReload: restoreOffset }, ); } else { - // Inside tween range: convert then add keyframe at current time + // Inside tween range (or a selected keyframe): convert then add/modify at + // the selected endpoint % if one is active, else the playhead %. const coalesceKey = `gsap:convert-drag:${anim.id}`; await callbacks.commitMutation( selection, @@ -411,7 +472,9 @@ export async function commitGsapPositionFromDrag( }, { label: "Convert from() for drag", skipReload: true, coalesceKey }, ); - const pct = computeCurrentPercentage(selection, anim); + const { activeKeyframePct, setActiveKeyframePct } = usePlayerStore.getState(); + const pct = activeKeyframePct ?? computeCurrentPercentage(selection, anim); + if (activeKeyframePct != null) setActiveKeyframePct(null); await callbacks.commitMutation( selection, { @@ -419,6 +482,7 @@ export async function commitGsapPositionFromDrag( animationId: anim.id, percentage: pct, properties: dragProps, + ...(backfillDefaults ? { backfillDefaults } : {}), }, { label: `Move layer (keyframe ${pct}%)`, @@ -437,6 +501,7 @@ export async function commitGsapPositionFromDrag( restoreOffset, iframe, selector, + backfillDefaults, ); } } diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.test.ts b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts new file mode 100644 index 0000000000..9ff5c79019 --- /dev/null +++ b/packages/studio/src/hooks/gsapRuntimeBridge.test.ts @@ -0,0 +1,94 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { tryGsapDragIntercept } from "./gsapRuntimeBridge"; + +/** + * Regression: `selectedGsapAnimations` (and the fetch fallback) is an async + * server-parse that LAGS a delete-all. A drag in that window would resolve a + * phantom position tween from the stale cache and re-commit it — resurrecting the + * just-deleted animation. tryGsapDragIntercept must trust the live runtime: if no + * non-hold tween exists for the element, it bails (returns false → CSS fallback) + * instead of committing. + */ + +// A preview iframe whose runtime timeline holds `children`, resolves the element, +// and exposes a gsap stub — so the drag can reach the commit path (the guard, not +// a missing gsap, must be what stops it). +function fakeIframe(elId: string, children: unknown[]): HTMLIFrameElement { + const timeline = { getChildren: () => children, duration: () => 14.6 }; + const el = { id: elId }; + return { + contentWindow: { + __timelines: { "index.html": timeline }, + gsap: { getProperty: () => 0 }, + }, + contentDocument: { querySelector: (sel: string) => (sel === `#${elId}` ? el : null) }, + } as unknown as HTMLIFrameElement; +} + +// A selection whose element answers the reads commitGsapPositionFromDrag makes — +// so without the guard the drag would reach commitMutation (resurrecting the tween). +const fakeElement = { + id: "puck-b", + style: { getPropertyValue: () => "" }, + getAttribute: () => null, + getBoundingClientRect: () => ({ top: 100, left: 100, width: 50, height: 50 }), +} as unknown as HTMLElement; + +const selection = { + id: "puck-b", + selector: "#puck-b", + element: fakeElement, +} as unknown as DomEditSelection; + +// A stale parse-cache entry: a position tween the server still reports post-delete. +const stalePositionAnim = { + id: "#puck-b-to-1000-position", + targetSelector: "#puck-b", + propertyGroup: "position", + method: "to", + properties: { x: -180, y: -60 }, + position: 1, + resolvedStart: 1, + duration: 2, +} as unknown as GsapAnimation; + +afterEach(() => vi.restoreAllMocks()); + +describe("tryGsapDragIntercept — stale-parse guard (no resurrection after delete-all)", () => { + it("bails without committing when the runtime has no tween (only the parse is stale)", async () => { + const commitMutation = vi.fn(); + // Runtime empty (tween deleted) — readRuntimeKeyframes returns null. + const iframe = fakeIframe("puck-b", []); + + const handled = await tryGsapDragIntercept( + selection, + { x: -50, y: 30 }, + [stalePositionAnim], + iframe, + commitMutation, + ); + + expect(handled).toBe(false); + expect(commitMutation).not.toHaveBeenCalled(); + }); + + it("does not trip the stale-parse guard when the runtime still has the tween", async () => { + const logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + const liveTween = { + targets: () => [{ id: "puck-b" }], + vars: { x: -120, y: 40, duration: 1 }, + duration: () => 1, + startTime: () => 1, + }; + // No fake gsap → it returns false later (at the gsapPos read), but the point + // is the stale-parse guard must NOT be the reason. + const iframe = fakeIframe("puck-b", [liveTween]); + + await tryGsapDragIntercept(selection, { x: -50, y: 30 }, [stalePositionAnim], iframe, vi.fn()); + + const staleLogged = logSpy.mock.calls.some((c) => String(c[1] ?? "").includes("stale parse")); + expect(staleLogged).toBe(false); + }); +}); diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index 576372ddae..d8cbee753b 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -21,6 +21,7 @@ import { import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import type { GsapDragCommitCallbacks } from "./gsapDragCommit"; import { getIframeGsap, queryIframeElement, selectorFromSelection } from "./gsapShared"; +import { readRuntimeKeyframes } from "./gsapRuntimeKeyframes"; import { roundTo3 } from "../utils/rounding"; // ── Runtime reads ────────────────────────────────────────────────────────── @@ -198,15 +199,6 @@ export async function tryGsapDragIntercept( fetchFallbackAnimations?: () => Promise, ): Promise { const selector = selectorFromSelection(selection); - console.log( - "[drag:4] tryGsapDragIntercept", - JSON.stringify({ - sel: selection.id, - selector, - animCount: animations.length, - groups: animations.map((a) => a.propertyGroup).filter(Boolean), - }), - ); if (!selector) { return false; } @@ -218,12 +210,6 @@ export async function tryGsapDragIntercept( commitMutation, fetchFallbackAnimations, ); - console.log( - "[drag:4] resolveGroupTween('position') →", - resolved - ? JSON.stringify({ id: resolved.anim.id, group: resolved.anim.propertyGroup }) - : "null", - ); let posAnim = resolved?.anim ?? null; if (!posAnim) { @@ -231,27 +217,26 @@ export async function tryGsapDragIntercept( if (!posAnim && fetchFallbackAnimations) { const fresh = await fetchFallbackAnimations(); posAnim = findGsapPositionAnimation(fresh, selector); - console.log( - "[drag:4] findGsapPositionAnimation (fetched) →", - posAnim ? posAnim.id : "null", - "freshCount:", - fresh.length, - ); } } if (!posAnim) { return false; } + // The live runtime is authoritative; `selectedGsapAnimations` (and the fetch + // fallback) is an async server-parse that LAGS a delete-all, so `posAnim` can + // be a phantom of a just-deleted tween. If the live timeline has no non-hold + // tween for this element, the parse is stale — bail so the drag falls back to + // the CSS path instead of resurrecting the deleted animation from stale cache. + if (!readRuntimeKeyframes(iframe, selector)) { + return false; + } + const gsapPos = readGsapPositionFromIframe(iframe, selector); if (!gsapPos) { return false; } - console.log( - "[drag:4] committing GSAP position drag", - JSON.stringify({ posAnimId: posAnim.id, gsapPos }), - ); await commitGsapPositionFromDrag(selection, posAnim, offset, gsapPos, iframe, selector, { commitMutation, fetchAnimations: fetchFallbackAnimations, diff --git a/packages/studio/src/hooks/useGsapAwareEditing.ts b/packages/studio/src/hooks/useGsapAwareEditing.ts index cd4b55acbf..6b135d4aae 100644 --- a/packages/studio/src/hooks/useGsapAwareEditing.ts +++ b/packages/studio/src/hooks/useGsapAwareEditing.ts @@ -18,6 +18,10 @@ import { tryGsapRotationIntercept, } from "./gsapRuntimeBridge"; import { useAnimatedPropertyCommit } from "./useAnimatedPropertyCommit"; +import { + useGsapSaveFailureTelemetry, + useSafeGsapCommitMutation, +} from "./useSafeGsapCommitMutation"; import type { CommitMutation } from "./gsapScriptCommitTypes"; export interface UseGsapAwareEditingParams { @@ -98,17 +102,6 @@ export function useGsapAwareEditing({ const handleGsapAwarePathOffsetCommit = useCallback( async (selection: DomEditSelection, next: { x: number; y: number }) => { const hasGsapAnims = selectedGsapAnimations.length > 0; - console.log( - "[drag:3] handleGsapAwarePathOffsetCommit", - JSON.stringify({ - sel: selection.id, - offset: next, - hasGsapAnims, - interceptEnabled: STUDIO_GSAP_DRAG_INTERCEPT_ENABLED, - animCount: selectedGsapAnimations.length, - animIds: selectedGsapAnimations.map((a) => a.id).slice(0, 5), - }), - ); if (hasGsapAnims && !STUDIO_GSAP_DRAG_INTERCEPT_ENABLED) { showToast(GSAP_CSS_FALLBACK_BLOCKED_MESSAGE, "error"); throw new Error(GSAP_CSS_FALLBACK_BLOCKED_MESSAGE); @@ -232,13 +225,24 @@ export function useGsapAwareEditing({ ); // ── Thin commitMutation facade ── + // Routes through the canonical safe wrapper so a server-save failure surfaces a + // toast + save telemetry instead of silently reverting — parity with the + // arc/keyframe/animation ops that all go through useSafeGsapCommitMutation. + + const noopCommit = useCallback(async () => {}, []); + const trackGsapSaveFailure = useGsapSaveFailureTelemetry(null); + const safeGsapCommit = useSafeGsapCommitMutation( + gsapCommitMutation ?? noopCommit, + trackGsapSaveFailure, + showToast, + ); const commitMutation = useCallback( async (mutation: Record, options: { label: string; softReload?: boolean }) => { if (!domEditSelection) return; - await gsapCommitMutation?.(domEditSelection, mutation, options); + safeGsapCommit(domEditSelection, mutation, options); }, - [domEditSelection, gsapCommitMutation], + [domEditSelection, safeGsapCommit], ); // Unroll all computed (helper/loop) tweens in the active timeline into literal diff --git a/packages/studio/src/hooks/useGsapScriptCommits.ts b/packages/studio/src/hooks/useGsapScriptCommits.ts index 1c15af70d5..422b8621b5 100644 --- a/packages/studio/src/hooks/useGsapScriptCommits.ts +++ b/packages/studio/src/hooks/useGsapScriptCommits.ts @@ -82,8 +82,10 @@ export function useGsapScriptCommits({ projectIdRef, activeCompPath, previewIfra if (options.skipReload) return; if (result.parsed?.animations) updateKeyframeCacheFromParsed(result.parsed.animations, targetPath, selection.id ?? undefined, mutation); options.beforeReload?.(); + let applied: "soft" | "full" = "full"; if (options.softReload && result.scriptText) { - if (!applySoftReload(previewIframeRef.current, result.scriptText)) reloadPreview(); + applied = applySoftReload(previewIframeRef.current, result.scriptText) ? "soft" : "full"; + if (applied === "full") reloadPreview(); } else { reloadPreview(); }