diff --git a/packages/studio/src/hooks/gsapKeyframeCommit.ts b/packages/studio/src/hooks/gsapKeyframeCommit.ts index c2715984e..eb674b7d6 100644 --- a/packages/studio/src/hooks/gsapKeyframeCommit.ts +++ b/packages/studio/src/hooks/gsapKeyframeCommit.ts @@ -1,7 +1,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import type { DomEditSelection } from "../components/editor/domEditingTypes"; import { absoluteToPercentageForAnimation, findTweenAtTime } from "../utils/globalTimeCompiler"; -import { PROPERTY_DEFAULTS, selectorFromSelection } from "./gsapShared"; +import { PROPERTY_DEFAULTS, selectorFromSelection, writeTargetSelector } from "./gsapShared"; import { roundToCenti } from "../utils/rounding"; type CommitFn = ( @@ -22,6 +22,9 @@ export async function commitKeyframeAtTimeImpl( properties: Record, commitMutation: CommitFn, ): Promise { + // Matching an authored tween is a string compare against what the author + // wrote, so it keeps using the selection's own selector; the NEW tween below + // is authored with the one-element form instead. const selector = selectorFromSelection(selection); if (!selector) return; @@ -65,7 +68,7 @@ export async function commitKeyframeAtTimeImpl( selection, { type: "add-with-keyframes" as const, - targetSelector: selector, + targetSelector: writeTargetSelector(selection) ?? selector, position: absoluteTime, duration: defaultDuration, keyframes: [ diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index caa1c2625..bba2713b4 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -181,6 +181,86 @@ export function selectorFromSelection(selection: DomEditSelection): string | nul return null; } +/** `[name="value"]`, with the quote/backslash escaping a CSS string needs. */ +function attributeSelector(name: string, value: string): string { + return `[${name}="${value.replace(/(["\\])/g, "\\$1")}"]`; +} + +function matchesExactlyOne(doc: Document, selector: string, element: Element): boolean { + try { + const matches = doc.querySelectorAll(selector); + return matches.length === 1 && matches[0] === element; + } catch { + return false; + } +} + +/** + * A structural address for an element that carries no identity of its own: + * `:nth-child` steps up to the nearest ancestor that IS uniquely addressable + * (an id or a data-hf-id). This is the `selector` + `selectorIndex` pair the + * selection already carries, resolved through the live DOM the index was + * counted in — an index can't be spelled in CSS, but the element's position can. + */ +function structuralSelector(element: Element): string | null { + const doc = element.ownerDocument; + if (!doc) return null; + const parts: string[] = []; + for (let node: Element | null = element; node; node = node.parentElement) { + if (node !== element) { + const id = node instanceof HTMLElement ? node.id : ""; + const hfId = node.getAttribute("data-hf-id"); + if (id) { + parts.unshift(idSelector(id)); + break; + } + if (hfId) { + parts.unshift(attributeSelector("data-hf-id", hfId)); + break; + } + } + const parent = node.parentElement; + if (!parent) break; + const index = Array.prototype.indexOf.call(parent.children, node) + 1; + if (index < 1) return null; + parts.unshift(`${node.tagName.toLowerCase()}:nth-child(${index})`); + } + if (parts.length === 0) return null; + const selector = parts.join(" > "); + return matchesExactlyOne(doc, selector, element) ? selector : null; +} + +/** + * The selector to author a NEW tween with. Distinct from + * {@link selectorFromSelection}, which must keep returning the exact string an + * already-authored tween is string-matched against (findTweenAtTime): this one + * has to ADDRESS ONE ELEMENT. + * + * `buildStableSelector` hands back a bare class for any element without an id, + * so "add keyframe at playhead" on one of five `.group` siblings wrote + * `tl.set(".group", …)` — a tween that animates all five and that + * {@link resolveSelectorElementIds} reads back as all five, collapsing their + * timeline rows into one. Every rung below resolves to exactly one element. + * + * ponytail: the last rung returns the bare selector unchanged rather than null. + * Refusing to write would turn a mis-targeted add into a silently dead button; + * it is only reachable with no live DOM to disambiguate against. + */ +export function writeTargetSelector(selection: DomEditSelection): string | null { + if (selection.id) return idSelector(selection.id); + if (selection.hfId) return attributeSelector("data-hf-id", selection.hfId); + const element = selection.element; + const doc = element?.ownerDocument; + if (element && doc) { + if (selection.selector && matchesExactlyOne(doc, selection.selector, element)) { + return selection.selector; + } + const structural = structuralSelector(element); + if (structural) return structural; + } + return selection.selector ?? null; +} + // ── Percentage computation ──────────────────────────────────────────────────── /** diff --git a/packages/studio/src/hooks/gsapShared.writeTarget.test.ts b/packages/studio/src/hooks/gsapShared.writeTarget.test.ts new file mode 100644 index 000000000..627007cb0 --- /dev/null +++ b/packages/studio/src/hooks/gsapShared.writeTarget.test.ts @@ -0,0 +1,147 @@ +// @vitest-environment jsdom +import { afterEach, describe, expect, it, vi } from "vitest"; +import { parseGsapScript } from "@hyperframes/core/gsap-parser"; +import { addAnimationWithKeyframesToScript } from "@hyperframes/parsers/gsap-writer-acorn"; +import type { DomEditSelection } from "../components/editor/domEditingTypes"; +import { buildStableSelector, getSelectorIndex } from "../components/editor/domEditingDom"; +import { resolveSelectorElementIds, writeTargetSelector } from "./gsapShared"; +import { commitKeyframeAtTimeImpl } from "./gsapKeyframeCommit"; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +/** + * A selection built the way production builds it (getDomLayerPatchTarget), so + * the class-only case under test is the real one: buildStableSelector hands back + * a BARE class for an element with no id / hf-id / composition id. + */ +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 — the shape that made one add collapse the timeline. */ +function mountGroupSiblings(): HTMLElement[] { + document.body.innerHTML = ` +
+
+
+
+
+
+
+ `; + return Array.from(document.querySelectorAll(".group")); +} + +const BASE_SCRIPT = ` +const tl = gsap.timeline({ paused: true }); +tl.to("#scene", { opacity: 1, duration: 0.5 }, 0); +`.trim(); + +const KEYFRAMES = [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: 40 } }, +]; + +/** Author the selector into a real script and read the tween's targets back. */ +function roundTripTargets(selector: string): { script: string; targets: string[] } { + const { script } = addAnimationWithKeyframesToScript(BASE_SCRIPT, selector, 0, 1, KEYFRAMES); + const added = parseGsapScript(script).animations.find((a) => a.targetSelector === selector); + if (!added) throw new Error(`written selector did not re-parse: ${selector}`); + return { script, targets: resolveSelectorElementIds(added.targetSelector, document) }; +} + +describe("writeTargetSelector", () => { + it("addresses exactly one element when the only identity is a shared class", () => { + const groups = mountGroupSiblings(); + const selection = selectionFor(groups[2]); + + // Precondition: this is the defect's input — a bare class hitting all five. + expect(selection.selector).toBe(".group"); + expect(document.querySelectorAll(".group")).toHaveLength(5); + + const written = writeTargetSelector(selection); + + expect(written).toBeTruthy(); + expect(document.querySelectorAll(written!)).toHaveLength(1); + expect(document.querySelector(written!)).toBe(groups[2]); + }); + + it("keeps a unique #id target", () => { + document.body.innerHTML = `
`; + const el = document.querySelector("#box")!; + + expect(writeTargetSelector(selectionFor(el))).toBe("#box"); + }); + + it("prefers data-hf-id over a generated structural selector", () => { + document.body.innerHTML = ` +
+ `; + const el = document.querySelectorAll(".group")[1]!; + + const written = writeTargetSelector(selectionFor(el)); + + expect(written).toBe('[data-hf-id="hf-42"]'); + expect(document.querySelector(written!)).toBe(el); + }); + + it("keeps an already-unique class selector as authored", () => { + document.body.innerHTML = `
`; + const el = document.querySelector(".header")!; + + expect(writeTargetSelector(selectionFor(el))).toBe(".header"); + }); +}); + +describe("writeTargetSelector — write/read round trip", () => { + it("re-parses and attributes the new tween to the one element it targeted", () => { + const groups = mountGroupSiblings(); + // Ids let resolveSelectorElementIds name the attributed elements; the + // SELECTION still has none, so the write path still faces the bare class. + groups.forEach((el, i) => el.setAttribute("id", `group-${i}`)); + const selection = { ...selectionFor(groups[2]), id: undefined } as DomEditSelection; + + const written = writeTargetSelector(selection); + const { targets } = roundTripTargets(written!); + + expect(targets).toEqual(["group-2"]); + }); + + it("does not widen the add to the element's four class siblings", () => { + const groups = mountGroupSiblings(); + groups.forEach((el, i) => el.setAttribute("id", `group-${i}`)); + const selection = { ...selectionFor(groups[0]), id: undefined } as DomEditSelection; + + // The old bare-class write attributed the one add to every sibling — the + // attribution blow-up behind "one add collapsed the timeline to a single row". + expect(roundTripTargets(".group").targets).toHaveLength(5); + + expect(roundTripTargets(writeTargetSelector(selection)!).targets).toHaveLength(1); + }); +}); + +describe("commitKeyframeAtTimeImpl — new-tween target", () => { + it("authors a one-element selector when no tween exists at the playhead", async () => { + const groups = mountGroupSiblings(); + const selection = selectionFor(groups[3]); + const commitMutation = vi.fn(async () => undefined); + + await commitKeyframeAtTimeImpl(selection, 1, [], { x: 12 }, commitMutation); + + const mutation = commitMutation.mock.calls[0]?.[1] as { targetSelector: string }; + expect(mutation.targetSelector).not.toBe(".group"); + expect(document.querySelectorAll(mutation.targetSelector)).toHaveLength(1); + expect(document.querySelector(mutation.targetSelector)).toBe(groups[3]); + }); +}); diff --git a/packages/studio/src/hooks/useEnableKeyframes.test.ts b/packages/studio/src/hooks/useEnableKeyframes.test.ts index f9cac5e9e..19cb61861 100644 --- a/packages/studio/src/hooks/useEnableKeyframes.test.ts +++ b/packages/studio/src/hooks/useEnableKeyframes.test.ts @@ -466,3 +466,47 @@ describe("useEnableKeyframes — flat tween transaction", () => { }); }); }); + +describe("useEnableKeyframes — new tween on a class-only element", () => { + it("targets the selected sibling alone, not every element sharing its class", async () => { + window.location.hash = "#/project/test-project"; + usePlayerStore.setState({ currentTime: 1 }); + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ ok: true, json: async () => ({ animations: [] }) })), + ); + const scene = document.body.appendChild(document.createElement("div")); + scene.id = "scene"; + scene.innerHTML = ''.repeat(5); + const groups = Array.from(scene.querySelectorAll(".group")); + const commitMutation = vi.fn(async () => undefined); + const enable = renderEnableKeyframes({ + // A bare `.group` here writes `tl.to(".group", …)`, which animates all five + // siblings and reads back as all five — one add wiped the timeline. + domEditSelection: { + selector: ".group", + selectorIndex: 3, + sourceFile: "index.html", + element: groups[3], + dataAttributes: { start: "0", duration: "2" }, + } as unknown as DomEditSelection, + selectedGsapAnimations: [], + previewIframeRef: { + current: { + contentWindow: { gsap: { getProperty: () => 7 } }, + } as unknown as HTMLIFrameElement, + }, + handleGsapAddAnimation: vi.fn(), + handleGsapConvertToKeyframes: vi.fn(), + handleGsapRemoveKeyframe: vi.fn(), + commitMutation, + }); + + await act(async () => enable()); + + const mutation = commitMutation.mock.calls[0]?.[0] as { targetSelector: string }; + expect(mutation?.targetSelector).toBeTruthy(); + expect(document.querySelectorAll(mutation.targetSelector)).toHaveLength(1); + expect(document.querySelector(mutation.targetSelector)).toBe(groups[3]); + }); +}); diff --git a/packages/studio/src/hooks/useEnableKeyframes.ts b/packages/studio/src/hooks/useEnableKeyframes.ts index 38c8f5008..c74869efa 100644 --- a/packages/studio/src/hooks/useEnableKeyframes.ts +++ b/packages/studio/src/hooks/useEnableKeyframes.ts @@ -18,6 +18,7 @@ import { KEYFRAME_PCT_MATCH, isInstantHold, resolveEditableTweenDuration, + writeTargetSelector, } from "./gsapShared"; import { absoluteToPercentage, @@ -523,7 +524,10 @@ export function useEnableKeyframes( sel.dataAttributes?.duration, t, ); - const selector = selectorFromSelection(sel); + // A brand-new tween: author it against the one element the user selected. + // The bare class selectorFromSelection hands back for an id-less element + // animates every sibling sharing the class (see writeTargetSelector). + const selector = writeTargetSelector(sel); if (!selector) { session.handleGsapAddAnimation("to");