From 3d924360667a15745d85dc060689d913b5dcc5d7 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Tue, 28 Jul 2026 16:25:41 +0200 Subject: [PATCH] fix(studio): author every new tween against one element U3 fixed "add keyframe at playhead" widening a write to every sibling sharing a class, but wired writeTargetSelector into only two paths. The same bug was still reachable from the add-animation button, drag, resize, rotate, gesture recording, and the property panel: each derived its target from selectorFromSelection, which hands back a bare class for an id-less element, so one edit authored a tween over all five siblings and the timeline collapsed their rows into one. Route every path that authors a NEW tween through the existing ladder: - ensureElementAddressable now accepts selection.selector only when it addresses exactly one element, so the id-minting fallback right below it (previously unreachable whenever any selector was present) does the work. - gsapDragCommit's five new-tween branches go through one newTweenTarget helper; instant patches reuse the written target so the runtime moves the element the source write names. - useGestureCommit and useAnimatedPropertyCommit keep the existing selector for matching/retargeting and author new tweens with a separate write selector. Retargets of an EXISTING tween are deliberately untouched: they keep anim.targetSelector, so a tween aimed at a whole group stays aimed at it. Narrowing the write alone regressed idempotency, verified by test: the "is there already a write for this element" lookups matched targetSelector by string, so the next nudge missed the write it had just made and appended a second, conflicting one. The read half now falls back to the live DOM (tweenTargetsElement, same contract as getAnimationsForElement), which also still matches a deliberate group tween. Tests reproduce each site through a real writer, re-parse with the real parser, and resolve through resolveSelectorElementIds (what feeds the keyframe cache and the lanes), plus pins for the new-tween vs retarget-existing distinction so a future change cannot collapse the two. --- packages/studio/src/hooks/gsapDragCommit.ts | 38 +- .../src/hooks/gsapDragStaticSetHelpers.ts | 22 +- .../studio/src/hooks/gsapResizeIntercept.ts | 4 +- .../studio/src/hooks/gsapRuntimeBridge.ts | 5 +- .../src/hooks/gsapScriptCommitHelpers.ts | 19 +- packages/studio/src/hooks/gsapShared.ts | 32 +- .../studio/src/hooks/newTweenTarget.test.ts | 364 ++++++++++++++++++ .../src/hooks/newTweenTargetHooks.test.tsx | 255 ++++++++++++ .../src/hooks/useAnimatedPropertyCommit.ts | 21 +- packages/studio/src/hooks/useGestureCommit.ts | 18 +- 10 files changed, 748 insertions(+), 30 deletions(-) create mode 100644 packages/studio/src/hooks/newTweenTarget.test.ts create mode 100644 packages/studio/src/hooks/newTweenTargetHooks.test.tsx diff --git a/packages/studio/src/hooks/gsapDragCommit.ts b/packages/studio/src/hooks/gsapDragCommit.ts index 4774c17e1..aab4eebcb 100644 --- a/packages/studio/src/hooks/gsapDragCommit.ts +++ b/packages/studio/src/hooks/gsapDragCommit.ts @@ -12,7 +12,7 @@ import { usePlayerStore } from "../player/store/playerStore"; import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeKeyframes"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import { roundTo3 } from "../utils/rounding"; -import { computeElementPercentage, idSelector } from "./gsapShared"; +import { computeElementPercentage, idSelector, writeTargetSelector } from "./gsapShared"; import { computeDraggedGsapPosition } from "./draggedGsapPosition"; import type { RuntimeTweenChange } from "./gsapRuntimePatch"; import { isGestureTransactionCommit, runGestureTransaction } from "./gestureTransaction"; @@ -44,6 +44,18 @@ export interface GsapDragCommitCallbacks { fetchAnimations?: () => Promise; } +/** + * The target for a tween these helpers are about to CREATE. Callers derive + * `selector` with `selectorFromSelection`, which hands back a bare class for an + * id-less element: authoring that widens a one-element drag/resize/rotate into a + * write over every sibling sharing the class. Retargets of an EXISTING tween + * must NOT come through here (they keep `anim.targetSelector`, so a tween the + * author aimed at a group stays aimed at it). + */ +function newTweenTarget(selection: DomEditSelection, selector: string): string { + return writeTargetSelector(selection) ?? selector; +} + // Re-export for backward compatibility with existing imports. export function computeCurrentPercentage( selection: DomEditSelection, @@ -75,7 +87,7 @@ async function replaceKeyframedPositionHold( selection, { type: "add", - targetSelector: selector, + targetSelector: newTweenTarget(selection, selector), method: "set", position: 0, properties, @@ -199,11 +211,14 @@ export async function commitStaticGsapPosition( } // New static hold → a base `gsap.set` (off-timeline, no 0% keyframe marker), with // an instant patch so the first nudge shows immediately (no soft-reload flash). + // The patch reuses the WRITTEN target so the runtime moves exactly the element + // the source write names. + const target = newTweenTarget(selection, selector); await callbacks.commitMutation( selection, { type: "add", - targetSelector: selector, + targetSelector: target, method: "set", position: 0, properties: { x: newX, y: newY }, @@ -212,7 +227,10 @@ export async function commitStaticGsapPosition( { label: "Move layer", softReload: true, - instantPatch: { selector, change: { kind: "global-set", props: { x: newX, y: newY } } }, + instantPatch: { + selector: target, + change: { kind: "global-set", props: { x: newX, y: newY } }, + }, }, ); } @@ -252,11 +270,12 @@ export async function commitStaticGsapRotation( return; } // New static hold → off-timeline `gsap.set` (no 0% keyframe marker) + instant patch. + const target = newTweenTarget(selection, selector); await callbacks.commitMutation( selection, { type: "add", - targetSelector: selector, + targetSelector: target, method: "set", position: 0, properties: { rotation: newRotation }, @@ -265,7 +284,10 @@ export async function commitStaticGsapRotation( { label: "Rotate layer", softReload: true, - instantPatch: { selector, change: { kind: "global-set", props: { rotation: newRotation } } }, + instantPatch: { + selector: target, + change: { kind: "global-set", props: { rotation: newRotation } }, + }, }, ); } @@ -305,7 +327,7 @@ export async function commitStaticGsapSize( selection, { type: "add", - targetSelector: selector, + targetSelector: newTweenTarget(selection, selector), method: "set", position: 0, properties: { width, height }, @@ -393,7 +415,7 @@ export async function commitKeyframedSizeFromResize( selection, { type: "add-with-keyframes", - targetSelector: selector, + targetSelector: newTweenTarget(selection, selector), position: roundTo3(ts), duration: roundTo3(td), keyframes, diff --git a/packages/studio/src/hooks/gsapDragStaticSetHelpers.ts b/packages/studio/src/hooks/gsapDragStaticSetHelpers.ts index 243ddbd81..ecba30bd4 100644 --- a/packages/studio/src/hooks/gsapDragStaticSetHelpers.ts +++ b/packages/studio/src/hooks/gsapDragStaticSetHelpers.ts @@ -1,6 +1,6 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import type { RuntimeTweenChange, SetPatchProps } from "./gsapRuntimePatch"; -import { isInstantHold } from "./gsapShared"; +import { isInstantHold, tweenTargetsElement } from "./gsapShared"; /** The shape of an `update-property` mutation a static-set nudge POSTs. */ interface UpdatePropertyMutation { @@ -34,12 +34,13 @@ export function setPatchFromUpdateProperty( function findPositionSetAnimation( animations: GsapAnimation[], selector: string, + element?: Element | null, ): GsapAnimation | null { return ( animations.find( (a) => a.method === "set" && - a.targetSelector === selector && + tweenTargetsElement(a.targetSelector, selector, element) && ("x" in a.properties || "y" in a.properties), ) ?? null ); @@ -61,12 +62,16 @@ function findPositionSetAnimation( export function findExistingPositionWrite( animations: GsapAnimation[], selector: string, + element?: Element | null, ): GsapAnimation | null { - const set = findPositionSetAnimation(animations, selector); + const set = findPositionSetAnimation(animations, selector, element); if (set) return set; return ( animations.find( - (a) => a.targetSelector === selector && a.propertyGroup === "position" && isInstantHold(a), + (a) => + tweenTargetsElement(a.targetSelector, selector, element) && + a.propertyGroup === "position" && + isInstantHold(a), ) ?? null ); } @@ -74,10 +79,14 @@ export function findExistingPositionWrite( export function findRotationSetAnimation( animations: GsapAnimation[], selector: string, + element?: Element | null, ): GsapAnimation | null { return ( animations.find( - (a) => isInstantHold(a) && a.targetSelector === selector && "rotation" in a.properties, + (a) => + isInstantHold(a) && + tweenTargetsElement(a.targetSelector, selector, element) && + "rotation" in a.properties, ) ?? null ); } @@ -85,12 +94,13 @@ export function findRotationSetAnimation( export function findSizeSetAnimation( animations: GsapAnimation[], selector: string, + element?: Element | null, ): GsapAnimation | null { return ( animations.find( (a) => isInstantHold(a) && - a.targetSelector === selector && + tweenTargetsElement(a.targetSelector, selector, element) && ("width" in a.properties || "height" in a.properties), ) ?? null ); diff --git a/packages/studio/src/hooks/gsapResizeIntercept.ts b/packages/studio/src/hooks/gsapResizeIntercept.ts index a0042b602..e9eecef82 100644 --- a/packages/studio/src/hooks/gsapResizeIntercept.ts +++ b/packages/studio/src/hooks/gsapResizeIntercept.ts @@ -79,7 +79,7 @@ export async function tryGsapResizeIntercept( if (!anim || isInstantHold(anim)) { const sel = selectorFromSelection(selection); if (!sel) return false; - const sizeSet = anim ?? findSizeSetAnimation(animations, sel); + const sizeSet = anim ?? findSizeSetAnimation(animations, sel, selection.element); // If the element is animated (has a real tween, not just a static size // hold), keyframe the size at the playhead so other keyframes keep theirs — @@ -242,7 +242,7 @@ export async function tryGsapResizeIntercept( const currentAnimations = fetchFallbackAnimations ? await fetchFallbackAnimations() : (resolved?.animations ?? animations); - const existingSet = findExistingPositionWrite(currentAnimations, selector); + const existingSet = findExistingPositionWrite(currentAnimations, selector, selection.element); // Delta chosen so the drag-path math composes back to exactly `corrected` // (no drag scratch attrs exist during a resize, so base = gsapPos). await commitStaticGsapPosition( diff --git a/packages/studio/src/hooks/gsapRuntimeBridge.ts b/packages/studio/src/hooks/gsapRuntimeBridge.ts index d102e7a81..5f8514630 100644 --- a/packages/studio/src/hooks/gsapRuntimeBridge.ts +++ b/packages/studio/src/hooks/gsapRuntimeBridge.ts @@ -213,7 +213,7 @@ export async function tryGsapDragIntercept( const existingSet = posAnim && isInstantHold(posAnim) && posAnim.targetSelector === selector ? posAnim - : findExistingPositionWrite(resolvedAnimations, selector); + : findExistingPositionWrite(resolvedAnimations, selector, selection.element); await commitStaticGsapPosition(selection, offset, gsapPos, selector, existingSet, { commitMutation, fetchAnimations: fetchFallbackAnimations, @@ -301,7 +301,8 @@ export async function tryGsapRotationIntercept( // rotation set in place, else add a new one. This replaces the old // `--hf-studio-rotation` CSS-var fallback (the same dual-channel bug class). if (!anim || isInstantHold(anim)) { - const existingSet = anim ?? findRotationSetAnimation(resolvedAnimations, selector); + const existingSet = + anim ?? findRotationSetAnimation(resolvedAnimations, selector, selection.element); await commitStaticGsapRotation(selection, newRotation, selector, existingSet, { commitMutation, fetchAnimations: fetchFallbackAnimations, diff --git a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts index e6d17338f..8c6e45a02 100644 --- a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts +++ b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts @@ -2,17 +2,32 @@ import { findUnsafeDomPatchValues } from "@hyperframes/core/studio-api/finite-mu import type { DomEditSelection } from "../components/editor/domEditingTypes"; export { PROPERTY_DEFAULTS } from "./gsapShared"; -import { idSelector } from "./gsapShared"; +import { idSelector, matchesExactlyOne } from "./gsapShared"; +/** + * The selector to author a NEW tween against, minting an id on the element when + * it has no address of its own. + * + * `selection.selector` is only usable when it addresses ONE element: + * `buildStableSelector` hands back a bare class for an id-less element, so + * returning it unconditionally aimed "add animation" at every sibling sharing + * the class (the attribution blow-up that collapsed the timeline to one row, + * see writeTargetSelector). A non-unique selector falls through to the id mint + * below, which is the stronger fix here than a structural path: the id it writes + * back to the source also makes every later lookup for this element exact. + */ export function ensureElementAddressable(selection: DomEditSelection): { selector: string; autoId?: string; } { if (selection.id) return { selector: idSelector(selection.id) }; - if (selection.selector) return { selector: selection.selector }; const el = selection.element; const doc = el.ownerDocument; + if (selection.selector && matchesExactlyOne(doc, selection.selector, el)) { + return { selector: selection.selector }; + } + const tag = el.tagName.toLowerCase(); let id = tag; let n = 1; diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index a7ce0ee4b..fbf44f786 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -186,7 +186,12 @@ function attributeSelector(name: string, value: string): string { return `[${name}="${value.replace(/(["\\])/g, "\\$1")}"]`; } -function matchesExactlyOne(doc: Document, selector: string, element: Element): boolean { +/** + * Whether `selector` addresses `element` AND NOTHING ELSE. The single test for + * "this string is safe to author a new tween against": a selector that also + * hits siblings writes a tween that animates all of them. + */ +export function matchesExactlyOne(doc: Document, selector: string, element: Element): boolean { try { const matches = doc.querySelectorAll(selector); return matches.length === 1 && matches[0] === element; @@ -286,6 +291,31 @@ export function existingTweenTargetSelector( return selectorFromSelection(selection); } +/** + * The read half of {@link writeTargetSelector}: does an already-authored tween + * write THIS element? + * + * String equality against `selectorFromSelection` alone is not enough once new + * tweens are authored with a narrowed one-element selector: the next edit would + * miss the write it just made and append a second, conflicting one. Falling back + * to the live DOM keeps the pair consistent, and still matches a deliberate group + * tween (`.group` matches each of its siblings) so merges into it keep working. + */ +export function tweenTargetsElement( + targetSelector: string, + selector: string, + element: Element | null | undefined, +): boolean { + if (targetSelector === selector) return true; + if (!element) return false; + try { + return element.matches(targetSelector); + } catch { + // Not a selector `matches()` understands, so it never addressed this element. + return false; + } +} + // ── Percentage computation ──────────────────────────────────────────────────── /** diff --git a/packages/studio/src/hooks/newTweenTarget.test.ts b/packages/studio/src/hooks/newTweenTarget.test.ts new file mode 100644 index 000000000..33ef3e7dd --- /dev/null +++ b/packages/studio/src/hooks/newTweenTarget.test.ts @@ -0,0 +1,364 @@ +// @vitest-environment jsdom +/** + * Sibling writers of the "add keyframe at playhead" path fixed in + * gsapShared.writeTarget.test.ts. Every mutation that authors a NEW tween must + * address ONE element; a bare class attributes the write to every sibling that + * shares it, which is what collapsed the timeline to a single row. + * + * Same round trip as the U3 test: author through the real writer, re-parse with + * the real parser, resolve through the very function that feeds the keyframe + * cache and the lanes. + */ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { parseGsapScript } from "@hyperframes/core/gsap-parser"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { buildStableSelector, getSelectorIndex } from "../components/editor/domEditingDom"; +import { resolveSelectorElementIds } from "./gsapShared"; +import { ensureElementAddressable } from "./gsapScriptCommitHelpers"; +import { + commitStaticGsapPosition, + commitStaticGsapRotation, + commitStaticGsapSize, + commitKeyframedSizeFromResize, + commitWholePathOffset, + findExistingPositionWrite, +} from "./gsapDragCommit"; +import { promoteSetToKeyframes } from "./useEnableKeyframes"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function selectionFor(el: HTMLElement): DomEditSelection { + const selector = buildStableSelector(el); + return { + element: el, + id: el.id || undefined, + hfId: el.getAttribute("data-hf-id") || undefined, + selector, + selectorIndex: getSelectorIndex(document, el, selector, "index.html", null), + sourceFile: "index.html", + dataAttributes: { start: "0", duration: "2" }, + } as unknown as DomEditSelection; +} + +/** Five class-only siblings, each with an id so attribution is nameable. */ +function mountGroupSiblings(): HTMLElement[] { + document.body.innerHTML = ` +
+
+
+
+
+
+
+ `; + return Array.from(document.querySelectorAll(".group")); +} + +/** + * The selection production hands these writers: the element HAS an id in the + * DOM (so resolveSelectorElementIds can name it) but the SELECTION carries none, + * which is the id-less shape buildStableSelector answers with a bare class. + */ +function classOnlySelection(el: HTMLElement): DomEditSelection { + return { ...selectionFor(el), id: undefined, selector: ".group" } as DomEditSelection; +} + +/** The elements a written targetSelector actually attributes the tween to. */ +function attributedTo(targetSelector: string): string[] { + return resolveSelectorElementIds(targetSelector, document); +} + +function recorder() { + const mutations: Array> = []; + const commitMutation = vi.fn(async (_sel, mutation, _opts) => { + mutations.push(mutation as Record); + }); + return { mutations, callbacks: { commitMutation } as never }; +} + +/** Every `targetSelector` a run of mutations wrote. */ +function writtenTargets(mutations: Array>): string[] { + return mutations.map((m) => m.targetSelector).filter((s): s is string => typeof s === "string"); +} + +describe("ensureElementAddressable — add-animation button", () => { + it("addresses one element when the selection's only identity is a shared class", () => { + // Truly id-less siblings: the shape production reaches this path with (an + // element WITH an id never gets here, selection.id short-circuits above). + document.body.innerHTML = ` +
+
+
+ `; + const el = document.querySelectorAll(".group")[1]!; + const selection = selectionFor(el); + expect(selection.selector).toBe(".group"); + + const { selector, autoId } = ensureElementAddressable(selection); + + expect(autoId).toBeTruthy(); + expect(document.querySelectorAll(selector)).toHaveLength(1); + expect(document.querySelector(selector)).toBe(el); + expect(attributedTo(selector)).toEqual([autoId]); + }); + + it("keeps a unique #id target", () => { + document.body.innerHTML = `
`; + const el = document.querySelector("#box")!; + + expect(ensureElementAddressable(selectionFor(el)).selector).toBe("#box"); + }); + + it("keeps an already-unique class selector as authored", () => { + document.body.innerHTML = `
`; + const el = document.querySelector(".header")!; + + expect(ensureElementAddressable(selectionFor(el)).selector).toBe(".header"); + }); + + it("still mints an id when there is no live element to disambiguate against", () => { + document.body.innerHTML = `
`; + const el = document.querySelector("#scene > div")!; + const selection = { ...selectionFor(el), selector: undefined } as DomEditSelection; + + const { selector, autoId } = ensureElementAddressable(selection); + + expect(autoId).toBe("div"); + expect(selector).toBe("#div"); + }); +}); + +describe("gsapDragCommit — new-tween targets", () => { + it("commitStaticGsapPosition authors the new set against one element", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + + await commitStaticGsapPosition( + classOnlySelection(groups[2]!), + { x: 10, y: 10 }, + { x: 0, y: 0 }, + ".group", + null, + callbacks, + ); + + expect(attributedTo(writtenTargets(mutations)[0]!)).toEqual(["group-2"]); + }); + + it("commitStaticGsapRotation authors the new set against one element", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + + await commitStaticGsapRotation(classOnlySelection(groups[1]!), 42, ".group", null, callbacks); + + expect(attributedTo(writtenTargets(mutations)[0]!)).toEqual(["group-1"]); + }); + + it("commitStaticGsapSize authors the new set against one element", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + + await commitStaticGsapSize( + classOnlySelection(groups[4]!), + { width: 100, height: 50 }, + ".group", + null, + callbacks, + ); + + expect(attributedTo(writtenTargets(mutations)[0]!)).toEqual(["group-4"]); + }); + + it("commitKeyframedSizeFromResize authors the new keyframe tween against one element", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + const animatedTween = { + id: "t1", + targetSelector: ".group", + method: "to", + properties: {}, + resolvedStart: 0, + duration: 2, + keyframes: { keyframes: [{ percentage: 0, properties: { x: 0 } }] }, + } as unknown as GsapAnimation; + + const handled = await commitKeyframedSizeFromResize( + classOnlySelection(groups[3]!), + { width: 80, height: 40 }, + ".group", + null, + animatedTween, + callbacks, + ); + + expect(handled).toBe(true); + expect(attributedTo(writtenTargets(mutations)[0]!)).toEqual(["group-3"]); + }); + + it("commitStaticGsapPosition replaces a corrupt keyframed hold against one element", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + const corruptHold = { + id: "hold-1", + targetSelector: ".group", + method: "to", + properties: {}, + duration: 0, + keyframes: { keyframes: [{ percentage: 0, properties: { x: 0, y: 0 } }] }, + } as unknown as GsapAnimation; + + await commitStaticGsapPosition( + classOnlySelection(groups[0]!), + { x: 5, y: 5 }, + { x: 0, y: 0 }, + ".group", + corruptHold, + callbacks, + ); + + expect(attributedTo(writtenTargets(mutations)[0]!)).toEqual(["group-0"]); + }); +}); + +describe("gsapDragCommit — retargeting an existing tween is left alone", () => { + it("commitWholePathOffset keeps the tween's own group target", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + const groupTween = { + id: "t-group", + targetSelector: ".group", + method: "to", + properties: { x: 100 }, + resolvedStart: 0, + duration: 2, + } as unknown as GsapAnimation; + + await commitWholePathOffset( + classOnlySelection(groups[2]!), + groupTween, + { x: 10, y: 0 }, + { x: 0, y: 0 }, + null, + ".group", + callbacks, + ); + + // A tween the author aimed at all five siblings must STAY aimed at all five: + // narrowing it here would silently drop four elements out of the animation. + expect(writtenTargets(mutations)[0]).toBe(".group"); + expect(attributedTo(writtenTargets(mutations)[0]!)).toHaveLength(5); + }); +}); + +/** + * The write selector and the "is there already a write for this element?" + * lookup are two halves of one contract. Narrowing only the write half would + * make the next nudge miss its own previous write and append a second one — the + * duplicate-position-write bug findExistingPositionWrite exists to prevent. + */ +describe("a re-nudge updates its own previous write instead of stacking a second one", () => { + it("finds the write the first nudge authored", async () => { + const groups = mountGroupSiblings(); + const selection = classOnlySelection(groups[2]!); + const first = recorder(); + + await commitStaticGsapPosition( + selection, + { x: 10, y: 10 }, + { x: 0, y: 0 }, + ".group", + null, + first.callbacks, + ); + const written = writtenTargets(first.mutations)[0]!; + + // Read the first write back the way the next drag does: parse the source, + // then run the production lookup for this element's position write. + const script = ` +const tl = gsap.timeline({ paused: true }); +gsap.set(${JSON.stringify(written)}, { x: 10, y: 10 }); +`.trim(); + const animations = parseGsapScript(script).animations; + const existing = findExistingPositionWrite(animations, ".group", selection.element); + expect(existing).toBeTruthy(); + + const second = recorder(); + await commitStaticGsapPosition( + selection, + { x: 5, y: 0 }, + { x: 10, y: 10 }, + ".group", + existing, + second.callbacks, + ); + + expect(second.mutations[0]!.type).toBe("update-properties"); + expect(second.mutations[0]!.animationId).toBe(existing!.id); + }); +}); + +/** + * Candidate C. Every `replace-with-keyframes` in useEnableKeyframes names an + * `animationId` parsed out of the CURRENT SOURCE (the anims list comes from + * tryFetchAnimationsForElement), so each one rewrites a tween the author already + * has. Narrowing those to one element would silently drop the other four + * siblings out of an animation that was aimed at the group on purpose. + */ +describe("useEnableKeyframes — rewriting an existing tween keeps its group target", () => { + it("promoteSetToKeyframes leaves a group-authored set aimed at the group", async () => { + const groups = mountGroupSiblings(); + const mutations: Array> = []; + const setAnim = { + id: "set-group", + targetSelector: ".group", + method: "set", + properties: { x: 0, y: 0 }, + resolvedStart: 0, + duration: 0, + } as unknown as GsapAnimation; + const session = { + commitMutation: async (mutation: Record) => { + mutations.push(mutation); + }, + handleGsapRemoveKeyframe: vi.fn(), + }; + + // Playhead at the set: the branch that replaces it with a single keyframe, + // which can source its value from the set itself (no live iframe needed). + await promoteSetToKeyframes(session as never, classOnlySelection(groups[2]!), setAnim, 0, null); + + expect(mutations[0]!.type).toBe("replace-with-keyframes"); + expect(mutations[0]!.animationId).toBe("set-group"); + expect(mutations[0]!.targetSelector).toBe(".group"); + expect(attributedTo(mutations[0]!.targetSelector as string)).toHaveLength(5); + }); +}); + +describe("the written selector survives the real writer and parser", () => { + it("re-parses to a tween attributed to the one element it targeted", async () => { + const groups = mountGroupSiblings(); + const { mutations, callbacks } = recorder(); + + await commitStaticGsapPosition( + classOnlySelection(groups[2]!), + { x: 10, y: 10 }, + { x: 0, y: 0 }, + ".group", + null, + callbacks, + ); + const written = writtenTargets(mutations)[0]!; + + const script = ` +const tl = gsap.timeline({ paused: true }); +gsap.set(${JSON.stringify(written)}, { x: 10, y: 10 }); +`.trim(); + const parsed = parseGsapScript(script).animations.find((a) => a.targetSelector === written); + + expect(parsed).toBeTruthy(); + expect(resolveSelectorElementIds(parsed!.targetSelector, document)).toEqual(["group-2"]); + }); +}); diff --git a/packages/studio/src/hooks/newTweenTargetHooks.test.tsx b/packages/studio/src/hooks/newTweenTargetHooks.test.tsx new file mode 100644 index 000000000..8ed60bd79 --- /dev/null +++ b/packages/studio/src/hooks/newTweenTargetHooks.test.tsx @@ -0,0 +1,255 @@ +// @vitest-environment jsdom +/** + * Hook-level siblings of newTweenTarget.test.ts: the property panel and the + * gesture recorder also author NEW tweens, so they face the same bare-class + * blow-up. Driven through the real hooks so the classification under test + * (new tween vs retarget of an existing one) is the production one. + */ +import React, { act } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { usePlayerStore } from "../player/store/playerStore"; +import { resolveSelectorElementIds } from "./gsapShared"; +import { useAnimatedPropertyCommit } from "./useAnimatedPropertyCommit"; +import { useGestureCommit } from "./useGestureCommit"; +import { mountReactHarness, installReactActEnvironment } from "./domSelectionTestHarness"; + +installReactActEnvironment(); + +const frozenSamples = [ + { time: 0, properties: { x: 0, y: 0 } }, + { time: 0.5, properties: { x: 60, y: 10 } }, + { time: 1, properties: { x: 120, y: 40 } }, +]; + +vi.mock("./useGestureRecording", () => ({ + useGestureRecording: () => ({ + startRecording: vi.fn(), + stopRecording: () => frozenSamples, + clearSamples: vi.fn(), + samples: frozenSamples, + }), +})); + +afterEach(() => { + document.body.innerHTML = ""; + usePlayerStore.setState({ autoKeyframeEnabled: true, currentTime: 0 }); +}); + +function mountGroupSiblings(): HTMLElement[] { + document.body.innerHTML = ` +
+
+
+
+
+
+
+ `; + return Array.from(document.querySelectorAll(".group")); +} + +/** The id-less shape buildStableSelector answers with a bare class. */ +function classOnlySelection(el: HTMLElement): DomEditSelection { + return { + element: el, + id: undefined, + selector: ".group", + selectorIndex: Array.prototype.indexOf.call(el.parentElement!.children, el), + sourceFile: "index.html", + dataAttributes: { start: "0", duration: "2" }, + } as unknown as DomEditSelection; +} + +function attributedTo(targetSelector: unknown): string[] { + return resolveSelectorElementIds(String(targetSelector), document); +} + +type Commit = ( + selection: DomEditSelection, + props: Record, +) => Promise; + +function renderPropertyCommit( + animations: GsapAnimation[], + mutations: Array>, + onReady: (commit: Commit) => void, +) { + function Harness() { + const { commitAnimatedProperties } = useAnimatedPropertyCommit({ + selectedGsapAnimations: animations, + gsapCommitMutation: async (_sel: DomEditSelection, mutation: Record) => { + mutations.push(mutation); + }, + addGsapAnimation: vi.fn(), + convertToKeyframes: vi.fn(), + previewIframeRef: { current: null }, + bumpGsapCache: vi.fn(), + } as never); + onReady(commitAnimatedProperties); + return null; + } + return mountReactHarness(); +} + +describe("useAnimatedPropertyCommit — new-tween targets", () => { + it("authors a new static set against one element", async () => { + const groups = mountGroupSiblings(); + const mutations: Array> = []; + let commit: Commit | undefined; + const root = renderPropertyCommit([], mutations, (fn) => (commit = fn)); + + await act(async () => { + await commit!(classOnlySelection(groups[2]!), { rotationX: 30 }); + }); + + const added = mutations.find((m) => m.type === "add"); + expect(added).toBeTruthy(); + expect(attributedTo(added!.targetSelector)).toEqual(["group-2"]); + act(() => root.unmount()); + }); + + it("authors the first same-group keyframe tween against one element", async () => { + const groups = mountGroupSiblings(); + // A position tween exists, so the element IS animated; a rotation edit has + // no same-group tween to join and must author a fresh one. + const positionTween = { + id: "t-pos", + targetSelector: ".group", + propertyGroup: "position", + method: "to", + properties: { x: 0 }, + resolvedStart: 0, + duration: 2, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: 50 } }, + ], + }, + } as unknown as GsapAnimation; + const mutations: Array> = []; + let commit: Commit | undefined; + const root = renderPropertyCommit([positionTween], mutations, (fn) => (commit = fn)); + + await act(async () => { + await commit!(classOnlySelection(groups[1]!), { rotationY: 15 }); + }); + + const added = mutations.find((m) => m.type === "add-with-keyframes"); + expect(added).toBeTruthy(); + expect(attributedTo(added!.targetSelector)).toEqual(["group-1"]); + act(() => root.unmount()); + }); + + it("leaves an existing group tween aimed at its whole group", async () => { + const groups = mountGroupSiblings(); + usePlayerStore.setState({ autoKeyframeEnabled: false, currentTime: 0 }); + const groupTween = { + id: "t-pos", + targetSelector: ".group", + propertyGroup: "position", + method: "to", + properties: { x: 0 }, + resolvedStart: 0, + duration: 2, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0, y: 0 } }, + { percentage: 100, properties: { x: 50, y: 0 } }, + ], + }, + } as unknown as GsapAnimation; + const mutations: Array> = []; + let commit: Commit | undefined; + const root = renderPropertyCommit([groupTween], mutations, (fn) => (commit = fn)); + + await act(async () => { + await commit!(classOnlySelection(groups[2]!), { x: 90 }); + }); + + const replaced = mutations.find((m) => m.type === "replace-with-keyframes"); + expect(replaced).toBeTruthy(); + expect(replaced!.targetSelector).toBe(".group"); + act(() => root.unmount()); + }); +}); + +function renderGestureCommit( + animations: GsapAnimation[], + mutations: Array>, + selection: DomEditSelection, +) { + let toggle: (() => void) | undefined; + function Harness() { + const { handleToggleRecording } = useGestureCommit({ + domEditSessionRef: { + current: { + domEditSelection: selection, + selectedGsapAnimations: animations, + commitMutation: async (mutation: Record) => { + mutations.push(mutation); + }, + }, + } as never, + previewIframeRef: { current: document.createElement("iframe") }, + showToast: vi.fn(), + isGestureRecordingRef: { current: false }, + }); + toggle = handleToggleRecording; + return null; + } + const root = mountReactHarness(); + return { root, toggle: () => toggle!() }; +} + +describe("useGestureCommit — new-tween targets", () => { + it("authors the recorded tween against one element", async () => { + const groups = mountGroupSiblings(); + const mutations: Array> = []; + const { root, toggle } = renderGestureCommit([], mutations, classOnlySelection(groups[3]!)); + + act(() => toggle()); // start + await act(async () => { + toggle(); // stop + commit + await Promise.resolve(); + }); + + const added = mutations.find((m) => m.type === "add-with-keyframes"); + expect(added).toBeTruthy(); + expect(attributedTo(added!.targetSelector)).toEqual(["group-3"]); + act(() => root.unmount()); + }); + + it("leaves a merged existing group tween aimed at its whole group", async () => { + const groups = mountGroupSiblings(); + const existing = { + id: "t-pos", + targetSelector: ".group", + propertyGroup: "position", + method: "to", + properties: { x: 0 }, + resolvedStart: 0, + duration: 2, + keyframes: { keyframes: [{ percentage: 0, properties: { x: 0, y: 0 } }] }, + } as unknown as GsapAnimation; + const mutations: Array> = []; + const { root, toggle } = renderGestureCommit( + [existing], + mutations, + classOnlySelection(groups[3]!), + ); + + act(() => toggle()); + await act(async () => { + toggle(); + await Promise.resolve(); + }); + + const replaced = mutations.find((m) => m.type === "replace-with-keyframes"); + expect(replaced).toBeTruthy(); + expect(replaced!.targetSelector).toBe(".group"); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts index a454a4bf6..346f60a78 100644 --- a/packages/studio/src/hooks/useAnimatedPropertyCommit.ts +++ b/packages/studio/src/hooks/useAnimatedPropertyCommit.ts @@ -14,7 +14,13 @@ import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { usePlayerStore } from "../player/store/playerStore"; import { readAllAnimatedProperties, readGsapProperty } from "./gsapRuntimeBridge"; import type { SetPatchProps } from "./gsapRuntimePatch"; -import { selectorFromSelection, computeElementPercentage, isInstantHold } from "./gsapShared"; +import { + selectorFromSelection, + computeElementPercentage, + isInstantHold, + writeTargetSelector, + tweenTargetsElement, +} from "./gsapShared"; import { resolveTweenStart, resolveTweenDuration } from "../utils/globalTimeCompiler"; import { roundTo3 } from "../utils/rounding"; import { commitWholePropertyOffset } from "./gsapWholePropertyOffsetCommit"; @@ -185,7 +191,9 @@ async function commitStaticSet( batch.push(entry); byGroup.set(group, batch); } - const staticWrites = animations.filter((a) => isInstantHold(a) && a.targetSelector === selector); + const staticWrites = animations.filter( + (a) => isInstantHold(a) && tweenTargetsElement(a.targetSelector, selector, selection.element), + ); // Resolve every group's target BEFORE committing anything, and coalesce // groups that land on the SAME write into one commit: the snapshot is captured // once, so if two groups resolved to one legacy mixed write, a first @@ -243,11 +251,14 @@ async function addGlobalStaticSet( for (const [k, v] of batch) { if (typeof v === "number") numericProps[k as keyof SetPatchProps] = v; } + // A brand-new write, so it must address ONE element: `selector` is the bare + // class an id-less selection yields, which would hold every sibling. + const target = writeTargetSelector(selection) ?? selector; await commit( selection, { type: "add", - targetSelector: selector, + targetSelector: target, method: "set", position: 0, properties: Object.fromEntries(batch), @@ -259,7 +270,7 @@ async function addGlobalStaticSet( ...(Object.keys(numericProps).length > 0 ? { instantPatch: { - selector, + selector: target, change: { kind: "global-set" as const, props: numericProps }, }, } @@ -503,7 +514,7 @@ export function useAnimatedPropertyCommit(deps: CommitAnimatedPropertyDeps) { selection, { type: "add-with-keyframes", - targetSelector: selector, + targetSelector: writeTargetSelector(selection) ?? selector, position: roundTo3(tStart), duration: roundTo3(tDur), keyframes, diff --git a/packages/studio/src/hooks/useGestureCommit.ts b/packages/studio/src/hooks/useGestureCommit.ts index cdc51dcf9..6115f2ff6 100644 --- a/packages/studio/src/hooks/useGestureCommit.ts +++ b/packages/studio/src/hooks/useGestureCommit.ts @@ -13,7 +13,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import type { CommitMutationOptions } from "./gsapScriptCommitTypes"; import { roundTo3 } from "../utils/rounding"; import { classifyPropertyGroup } from "@hyperframes/core/gsap-parser"; -import { isInstantHold, idSelector } from "./gsapShared"; +import { isInstantHold, idSelector, writeTargetSelector, tweenTargetsElement } from "./gsapShared"; type RecordedKeyframe = { percentage: number; @@ -168,11 +168,17 @@ export function useGestureCommit({ if (!sortedPcts.includes(0)) sortedPcts.unshift(0); } + // Two different jobs, two different selectors. `selector` is the string an + // ALREADY-AUTHORED tween is matched against (and retargeted with, so a + // tween aimed at a whole group stays aimed at it). `writeSelector` is what + // a NEW tween is authored with: the bare class the id-less case yields here + // would record the gesture onto every sibling sharing it. const selector = sel.id ? idSelector(sel.id) : sel.selector; if (!selector) { showToast("Cannot save — element has no selector", "error"); return; } + const writeSelector = writeTargetSelector(sel) ?? selector; if (liveSession.commitMutation) { const recStart = recordingStartTimeRef.current; const rawKeyframes = sortedPcts.map((pct) => ({ @@ -186,7 +192,11 @@ export function useGestureCommit({ ); const allAnims = liveSession.selectedGsapAnimations ?? []; const existingPositionTween = hasPositionProps - ? allAnims.find((a) => a.propertyGroup === "position" && a.targetSelector === selector) + ? allAnims.find( + (a) => + a.propertyGroup === "position" && + tweenTargetsElement(a.targetSelector, selector, sel.element), + ) : undefined; if (existingPositionTween) { if (isInstantHold(existingPositionTween)) { @@ -261,7 +271,7 @@ export function useGestureCommit({ await liveSession.commitMutation( { type: "add-with-keyframes", - targetSelector: selector, + targetSelector: writeSelector, position: roundTo3(recStart), duration: roundTo3(duration), keyframes: groupKfs, @@ -287,7 +297,7 @@ export function useGestureCommit({ await liveSession.commitMutation( { type: "add-with-keyframes", - targetSelector: selector, + targetSelector: writeSelector, position: roundTo3(recStart), duration: roundTo3(duration), keyframes: groupKfs,