From e4d7bde64f3b3e500834231134bc9fdb3858ad41 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 16:40:52 +0200 Subject: [PATCH 1/7] feat(studio): add keyframe timeline state --- .../components/editor/AnimationCard.test.tsx | 93 +++------------ .../src/components/editor/AnimationCard.tsx | 2 +- packages/studio/src/hooks/gsapDragCommit.ts | 4 +- .../hooks/gsapKeyframeCacheHelpers.test.ts | 96 ++++++++++++++- .../src/hooks/gsapKeyframeCacheHelpers.ts | 60 ++++++++-- .../src/hooks/gsapScriptCommitHelpers.ts | 5 +- packages/studio/src/hooks/gsapShared.test.ts | 31 ++++- packages/studio/src/hooks/gsapShared.ts | 55 ++++++--- .../studio/src/hooks/gsapTweenSynth.test.ts | 31 ++++- packages/studio/src/hooks/gsapTweenSynth.ts | 49 +++++--- packages/studio/src/hooks/useDomSelection.ts | 4 + packages/studio/src/hooks/useGestureCommit.ts | 4 +- .../src/hooks/useGsapKeyframeOps.test.tsx | 37 ++++-- .../src/hooks/useGsapSelectionHandlers.ts | 22 ++-- .../studio/src/hooks/useGsapTweenCache.ts | 27 ++++- .../studio/src/hooks/useStudioTestHooks.ts | 51 ++++++++ .../studio/src/player/store/keyframeSlice.ts | 109 ++++++++++++++++++ .../src/player/store/playerStore.test.ts | 25 ++++ .../studio/src/player/store/playerStore.ts | 49 +------- packages/studio/src/telemetry/events.test.ts | 8 +- packages/studio/src/telemetry/events.ts | 14 ++- 21 files changed, 583 insertions(+), 193 deletions(-) create mode 100644 packages/studio/src/hooks/useStudioTestHooks.ts create mode 100644 packages/studio/src/player/store/keyframeSlice.ts diff --git a/packages/studio/src/components/editor/AnimationCard.test.tsx b/packages/studio/src/components/editor/AnimationCard.test.tsx index 802e07470d..ea51978c9a 100644 --- a/packages/studio/src/components/editor/AnimationCard.test.tsx +++ b/packages/studio/src/components/editor/AnimationCard.test.tsx @@ -45,17 +45,22 @@ function selectPreset(host: HTMLElement, presetId: string): string { return presetConfig.ease; } -function renderExpandedCard({ - animation, +/** Every test mounts the same card; only expansion, flat mode, and the spies differ. */ +function renderCard({ + animation = baseAnimation(), + defaultExpanded = true, flat, onUpdateMeta = vi.fn(), onUpdateKeyframeEase = vi.fn(), + onDeleteAnimation = noop, }: { - animation: GsapAnimation; + animation?: GsapAnimation; + defaultExpanded?: boolean; flat?: boolean; onUpdateMeta?: ReturnType; onUpdateKeyframeEase?: ReturnType; -}) { + onDeleteAnimation?: (id: string) => void; +} = {}) { const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); @@ -63,11 +68,11 @@ function renderExpandedCard({ root.render( { ], }, }); - const view = renderExpandedCard({ animation, onUpdateKeyframeEase }); + const view = renderCard({ animation, onUpdateKeyframeEase }); const segment = Array.from(view.host.querySelectorAll("button")).find((button) => button.textContent?.includes("0% → 50%"), @@ -101,6 +106,7 @@ describe("AnimationCard ease editing", () => { expect(onUpdateKeyframeEase).toHaveBeenCalledExactlyOnceWith(animation.id, 50, ease); expect(trackStudioSegmentEaseEdit).toHaveBeenCalledExactlyOnceWith({ + action: "commit", ease, }); act(() => view.root.unmount()); @@ -110,7 +116,7 @@ describe("AnimationCard ease editing", () => { const onUpdateMeta = vi.fn(); const onUpdateKeyframeEase = vi.fn(); const animation = baseAnimation({ id: "flat-tween" }); - const view = renderExpandedCard({ + const view = renderCard({ animation, flat: true, onUpdateMeta, @@ -128,23 +134,7 @@ describe("AnimationCard ease editing", () => { describe("AnimationCard flat branch", () => { it("renders a mint border-left and panel-token colors when flat", () => { - const host = document.createElement("div"); - document.body.append(host); - const root = createRoot(host); - act(() => { - root.render( - , - ); - }); + const { host, root } = renderCard({ defaultExpanded: false, flat: true }); const card = host.querySelector('[data-flat-effect-card="true"]'); expect(card).not.toBeNull(); expect(card?.className).toContain("border-panel-accent"); @@ -152,22 +142,7 @@ describe("AnimationCard flat branch", () => { }); it("still renders the legacy (non-flat) appearance when flat is omitted", () => { - const host = document.createElement("div"); - document.body.append(host); - const root = createRoot(host); - act(() => { - root.render( - , - ); - }); + const { host, root } = renderCard({ defaultExpanded: false }); expect(host.querySelector('[data-flat-effect-card="true"]')).toBeNull(); expect(host.textContent).toContain("power2.out"); act(() => root.unmount()); @@ -175,23 +150,7 @@ describe("AnimationCard flat branch", () => { it("toggles expanded state when the collapsed header button is clicked, in both modes", () => { for (const flat of [false, true]) { - const host = document.createElement("div"); - document.body.append(host); - const root = createRoot(host); - act(() => { - root.render( - , - ); - }); + const { host, root } = renderCard({ defaultExpanded: false, flat: flat || undefined }); expect(host.textContent).not.toContain("Remove"); const button = host.querySelector("button"); expect(button).not.toBeNull(); @@ -205,23 +164,7 @@ describe("AnimationCard flat branch", () => { it("invokes onDeleteAnimation with the animation id when Remove is clicked, in flat mode", () => { const onDeleteAnimation = vi.fn(); - const host = document.createElement("div"); - document.body.append(host); - const root = createRoot(host); - act(() => { - root.render( - , - ); - }); + const { host, root } = renderCard({ flat: true, onDeleteAnimation }); const buttons = Array.from(host.querySelectorAll("button")); const removeButton = buttons.find((b) => b.textContent === "Remove"); expect(removeButton).not.toBeUndefined(); diff --git a/packages/studio/src/components/editor/AnimationCard.tsx b/packages/studio/src/components/editor/AnimationCard.tsx index d178032d68..214d5b013d 100644 --- a/packages/studio/src/components/editor/AnimationCard.tsx +++ b/packages/studio/src/components/editor/AnimationCard.tsx @@ -267,7 +267,7 @@ export const AnimationCard = memo(function AnimationCard({ onToggle={setExpandedKfPct} onEaseCommit={(pct, ease) => { onUpdateKeyframeEase(animation.id, pct, ease); - trackStudioSegmentEaseEdit({ ease }); + trackStudioSegmentEaseEdit({ action: "commit", ease }); }} onApplyAll={ onSetAllKeyframeEases diff --git a/packages/studio/src/hooks/gsapDragCommit.ts b/packages/studio/src/hooks/gsapDragCommit.ts index 167ce438d5..4774c17e1b 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 } from "./gsapShared"; +import { computeElementPercentage, idSelector } from "./gsapShared"; import { computeDraggedGsapPosition } from "./draggedGsapPosition"; import type { RuntimeTweenChange } from "./gsapRuntimePatch"; import { isGestureTransactionCommit, runGestureTransaction } from "./gestureTransaction"; @@ -117,7 +117,7 @@ export async function materializeIfDynamic( const allScanned = scanAllRuntimeKeyframes(iframe); if (allScanned.size === 0) return; const allElements = Array.from(allScanned.entries()).map(([id, data]) => ({ - selector: `#${id}`, + selector: idSelector(id), keyframes: data.keyframes, easeEach: data.easeEach, })); diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index 16fc62b206..ee72035f5e 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -28,7 +28,7 @@ const animWithKeyframes = (id: string): GsapAnimation => ({ }); beforeEach(() => { - usePlayerStore.setState({ keyframeCache: new Map(), elements: [] }); + usePlayerStore.setState({ keyframeCache: new Map(), gsapAnimations: new Map(), elements: [] }); }); describe("clearKeyframeCacheForElement", () => { @@ -98,6 +98,41 @@ describe("clearKeyframeCacheForFile", () => { }); describe("updateKeyframeCacheFromParsed", () => { + it("serializes a multi-keyframe tween with a stable shape and animation identity", () => { + const animation: GsapAnimation = { + ...animWithKeyframes("hero"), + duration: 2, + resolvedStart: 3, + keyframes: { + format: "percentage", + keyframes: [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 50, properties: { x: 100 }, ease: "power1.inOut" }, + { percentage: 100, properties: { x: 200 } }, + ], + easeEach: "power1.inOut", + }, + }; + usePlayerStore.setState({ + elements: [ + { + id: "hero-clip", + domId: "hero", + tag: "div", + start: 2, + duration: 4, + track: 0, + }, + ], + }); + + updateKeyframeCacheFromParsed([animation], "scene.html", "hero", {}); + + expect(JSON.stringify(cache().get("scene.html#hero"))).toBe( + '{"format":"percentage","keyframes":[{"percentage":25,"properties":{"x":0},"tweenPercentage":0,"propertyGroup":"position","animationId":"hero"},{"percentage":50,"properties":{"x":100},"ease":"power1.inOut","tweenPercentage":50,"propertyGroup":"position","animationId":"hero"},{"percentage":75,"properties":{"x":200},"tweenPercentage":100,"propertyGroup":"position","animationId":"hero"}],"easeEach":"power1.inOut"}', + ); + }); + it("clears the bare key when the selected element no longer has keyframes", () => { // Element previously had keyframes, so a bare entry exists (writes set both). seed("index.html#box"); @@ -118,4 +153,63 @@ describe("updateKeyframeCacheFromParsed", () => { expect(cache().has("index.html#hero")).toBe(true); expect(cache().has("hero")).toBe(true); }); + + it("caches flat tweens as clip-relative start and end keyframes", () => { + const animation: GsapAnimation = { + id: "flat-box", + targetSelector: "#box", + method: "to", + position: 1, + properties: { x: 420 }, + duration: 2, + resolvedStart: 1, + ease: "power2.out", + propertyGroup: "position", + }; + usePlayerStore.setState({ + elements: [{ id: "box-clip", domId: "box", tag: "div", start: 1, duration: 2, track: 0 }], + }); + + updateKeyframeCacheFromParsed([animation], "scene.html", "box", {}); + + expect(cache().get("scene.html#box")).toEqual({ + format: "percentage", + keyframes: [ + { + percentage: 0, + properties: { x: 0 }, + tweenPercentage: 0, + propertyGroup: "position", + animationId: "flat-box", + }, + { + percentage: 100, + properties: { x: 420 }, + ease: "power2.out", + tweenPercentage: 100, + propertyGroup: "position", + animationId: "flat-box", + }, + ], + }); + expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]); + }); + + it("does not cache a flat tween without animatable numeric properties", () => { + const animation: GsapAnimation = { + id: "flat-box", + targetSelector: "#box", + method: "to", + position: 0, + properties: { backgroundColor: "#fff" }, + duration: 1, + propertyGroup: "visual", + }; + + updateKeyframeCacheFromParsed([animation], "scene.html", "box", {}); + + expect(cache().has("scene.html#box")).toBe(false); + expect(cache().has("box")).toBe(false); + expect(usePlayerStore.getState().gsapAnimations.has("scene.html#box")).toBe(false); + }); }); diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index 1b5bef62b0..58da5c7aae 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -5,6 +5,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; import { toAbsoluteTime } from "./gsapShared"; +import { synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( animations: GsapAnimation[], @@ -15,10 +16,16 @@ export function updateKeyframeCacheFromParsed( const { setKeyframeCache, elements } = usePlayerStore.getState(); const idsWithKeyframes = new Set(); const merged = new Map(); + const sourceAnimations = new Map(); for (const anim of animations) { const id = anim.targetSelector.match(/^#([\w-]+)/)?.[1]; - if (!id || !anim.keyframes) continue; + const kfSource = + anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? []; + if (!id || kfSource.length === 0) continue; idsWithKeyframes.add(id); + if (anim.propertyGroup) { + sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); + } // Convert tween-relative percentages to clip-relative so diamonds // render at the correct position within the timeline clip. @@ -29,7 +36,7 @@ export function updateKeyframeCacheFromParsed( ); const elStart = timelineEl?.start ?? 0; const elDuration = timelineEl?.duration ?? 1; - const clipKeyframes = anim.keyframes.keyframes.map((kf) => { + const clipKeyframes = kfSource.map((kf) => { const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); const clipPct = elDuration > 0 ? Math.round(((absTime - elStart) / elDuration) * 1000) / 10 : kf.percentage; @@ -38,6 +45,7 @@ export function updateKeyframeCacheFromParsed( percentage: clipPct, tweenPercentage: kf.percentage, propertyGroup: anim.propertyGroup, + animationId: anim.id, }; }); @@ -48,6 +56,17 @@ export function updateKeyframeCacheFromParsed( const prev = byPct.get(kf.percentage); if (prev) { prev.properties = { ...prev.properties, ...kf.properties }; + // Mirror deduplicateKeyframes: a same-% collision across different + // source animations is an ambiguous merged segment (the button can + // only target one arbitrary animation). Flag it so the collapsed row + // suppresses the inline ease button there. + if ( + prev.animationId !== undefined && + kf.animationId !== undefined && + prev.animationId !== kf.animationId + ) { + prev.easeAmbiguous = true; + } if (kf.ease) prev.ease = kf.ease; } else { byPct.set(kf.percentage, { ...kf, properties: { ...kf.properties } }); @@ -55,13 +74,18 @@ export function updateKeyframeCacheFromParsed( } existing.keyframes = Array.from(byPct.values()).sort((a, b) => a.percentage - b.percentage); } else { - merged.set(id, { ...anim.keyframes, keyframes: clipKeyframes }); + merged.set(id, { + ...anim.keyframes, + format: anim.keyframes?.format ?? "percentage", + keyframes: clipKeyframes, + }); } } for (const [id, entry] of merged) { setKeyframeCache(`${targetPath}#${id}`, entry); setKeyframeCache(id, entry); if (targetPath !== "index.html") setKeyframeCache(`index.html#${id}`, entry); + writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id)); } const targetId = (mutation as { targetSelector?: string }).targetSelector?.match(/^#([\w-]+)/)?.[1] ?? @@ -84,13 +108,12 @@ export function updateKeyframeCacheFromParsed( * a new cache map and re-render every subscriber. */ export function clearKeyframeCacheForElement(sourceFile: string, elementId: string): void { - const { keyframeCache, setKeyframeCache } = usePlayerStore.getState(); - const keys = - sourceFile === "index.html" - ? [`index.html#${elementId}`, elementId] - : [`${sourceFile}#${elementId}`, `index.html#${elementId}`, elementId]; + const { keyframeCache, setKeyframeCache, gsapAnimations, setGsapAnimations } = + usePlayerStore.getState(); + const keys = elementCacheKeys(sourceFile, elementId); for (const key of keys) { if (keyframeCache.has(key)) setKeyframeCache(key, undefined); + if (gsapAnimations.has(key)) setGsapAnimations(key, undefined); } } @@ -102,11 +125,11 @@ export function clearKeyframeCacheForElement(sourceFile: string, elementId: stri * absent from the re-scan) leaves no stale bare entry behind. */ export function clearKeyframeCacheForFile(sourceFile: string): void { - const { keyframeCache } = usePlayerStore.getState(); + const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); const sfPrefix = `${sourceFile}#`; const fallbackPrefix = "index.html#"; const ids = new Set(); - for (const key of keyframeCache.keys()) { + for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { const matchesFile = key.startsWith(sfPrefix) || (sourceFile !== "index.html" && key.startsWith(fallbackPrefix)); if (!matchesFile) continue; @@ -118,6 +141,23 @@ export function clearKeyframeCacheForFile(sourceFile: string): void { } } +function elementCacheKeys(sourceFile: string, elementId: string): string[] { + return sourceFile === "index.html" + ? [`index.html#${elementId}`, elementId] + : [`${sourceFile}#${elementId}`, `index.html#${elementId}`, elementId]; +} + +export function writeGsapAnimationsForElement( + sourceFile: string, + elementId: string, + animations: GsapAnimation[] | undefined, +): void { + const { setGsapAnimations } = usePlayerStore.getState(); + for (const key of elementCacheKeys(sourceFile, elementId)) { + setGsapAnimations(key, animations); + } +} + function buildCacheKey(sourceFile: string, elementId: string): string { return `${sourceFile}#${elementId}`; } diff --git a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts index d84c698608..e6d17338f3 100644 --- a/packages/studio/src/hooks/gsapScriptCommitHelpers.ts +++ b/packages/studio/src/hooks/gsapScriptCommitHelpers.ts @@ -2,12 +2,13 @@ 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"; export function ensureElementAddressable(selection: DomEditSelection): { selector: string; autoId?: string; } { - if (selection.id) return { selector: `#${selection.id}` }; + if (selection.id) return { selector: idSelector(selection.id) }; if (selection.selector) return { selector: selection.selector }; const el = selection.element; @@ -20,7 +21,7 @@ export function ensureElementAddressable(selection: DomEditSelection): { id = `${tag}-${n}`; } el.setAttribute("id", id); - return { selector: `#${id}`, autoId: id }; + return { selector: idSelector(id), autoId: id }; } export class GsapMutationHttpError extends Error { diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index ba45743488..30ed300a07 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; -import { isInstantHold, parsePercentageKeyframes } from "./gsapShared"; +import { idSelector, isInstantHold, parsePercentageKeyframes } from "./gsapShared"; describe("isInstantHold", () => { const animation = (method: GsapAnimation["method"], duration?: number) => @@ -74,3 +74,32 @@ describe("parsePercentageKeyframes", () => { expect(parsePercentageKeyframes({})).toBeNull(); }); }); + +describe("idSelector", () => { + it("uses #id for valid CSS identifiers", () => { + expect(idSelector("hero-word")).toBe("#hero-word"); + expect(idSelector("el_1")).toBe("#el_1"); + }); + + it("uses an attribute selector for ids that #id can't address (digit-leading, dots, spaces)", () => { + // #01-... / #a.b / #a b throw a SyntaxError in querySelector / GSAP, crashing + // the preview when such a target is committed (e.g. dragging the element). + expect(idSelector("01-hook-hero-word")).toBe('[id="01-hook-hero-word"]'); + expect(idSelector("my.class")).toBe('[id="my.class"]'); + expect(idSelector("1box")).toBe('[id="1box"]'); + }); + + it("escapes quotes and backslashes in the attribute selector value", () => { + expect(idSelector('1"x')).toBe('[id="1\\"x"]'); + }); + + it("only ever emits #id for ids that can't break querySelector", () => { + // Every id resolves to either a plain #id (only when safe) or an attribute + // selector — never a #id that would throw a SyntaxError. + for (const id of ["hero-word", "01-hook", "a.b", "a b", "1", "--x", '1"q']) { + const sel = idSelector(id); + if (sel.startsWith("#")) expect(sel).toBe(`#${id}`); + else expect(sel.startsWith('[id="')).toBe(true); + } + }); +}); diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 459f303b46..629e5976d1 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -53,8 +53,35 @@ export function isInstantHold(animation: GsapAnimation): boolean { * Returns `#id` if the selection has an id, otherwise the raw selector, * or null if neither exists. */ +/** + * A CSS-valid selector for an element id. `#id` for a valid CSS identifier, + * otherwise an `[id="..."]` attribute selector. IDs that start with a digit + * (e.g. "01-hook-hero-word") make `#id` an invalid selector, so + * `document.querySelector("#01-...")` / GSAP's `querySelectorAll` throw a + * SyntaxError — which surfaces as a masked cross-origin "Script error." and + * crashes the preview the moment such a target is committed (e.g. dragging). + */ +// Conservative: matches only ids that are unquestionably safe as a `#id` +// selector — ASCII identifier, starts with a letter/underscore (or a single +// leading hyphen), no dots/colons/spaces/digits-first. Anything it rejects +// (digit-leading like "01-hook-...", dots, spaces, non-ASCII, …) falls through +// to the attribute selector below, which is always valid. It can only ever err +// toward the safe form, never toward a `#id` that throws — and, unlike +// `CSS.escape`, it needs no browser global (this runs in node tests too). +const SAFE_HASH_ID = /^-?[A-Za-z_][\w-]*$/; + +export function idSelector(id: string): string { + // A `#id` selector is only valid for a CSS identifier. IDs that start with a + // digit (e.g. "01-hook-hero-word") make `document.querySelector("#01-...")` and + // GSAP's `querySelectorAll` throw a SyntaxError — surfacing as a masked + // cross-origin "Script error." that crashes the preview the moment such a + // target is committed (e.g. dragging the element). Address those via an + // attribute selector instead (quotes/backslashes escaped for the string). + return SAFE_HASH_ID.test(id) ? `#${id}` : `[id="${id.replace(/(["\\])/g, "\\$1")}"]`; +} + export function selectorFromSelection(selection: DomEditSelection): string | null { - if (selection.id) return `#${selection.id}`; + if (selection.id) return idSelector(selection.id); if (selection.selector) return selection.selector; return null; } @@ -118,6 +145,18 @@ export interface ParsedPercentageKeyframes { easeEach?: string; } +function collectAnimatableKeyframeProperties( + entry: Record, +): Record { + const properties: Record = {}; + for (const [property, value] of Object.entries(entry)) { + if (property === "ease") continue; + if (typeof value === "number") properties[property] = Math.round(value * 1000) / 1000; + else if (typeof value === "string") properties[property] = value; + } + return properties; +} + /** * Parse a GSAP percentage-keyframe object (`{ "0%": { x: 10 }, "100%": { x: 200 } }`) * into a sorted array of `{ percentage, properties }` entries. @@ -146,12 +185,7 @@ export function parsePercentageKeyframes( steps.forEach((entry, i) => { if (!entry || typeof entry !== "object") return; const percentage = steps.length > 1 ? Math.round((i / (steps.length - 1)) * 1000) / 10 : 0; - const properties: Record = {}; - for (const [pk, pv] of Object.entries(entry as Record)) { - if (pk === "ease") continue; - if (typeof pv === "number") properties[pk] = Math.round(pv * 1000) / 1000; - else if (typeof pv === "string") properties[pk] = pv; - } + const properties = collectAnimatableKeyframeProperties(entry as Record); if (Object.keys(properties).length > 0) keyframes.push({ percentage, properties }); }); return keyframes.length > 0 ? { keyframes } : null; @@ -165,12 +199,7 @@ export function parsePercentageKeyframes( const pctMatch = key.match(/^(\d+(?:\.\d+)?)%$/); if (!pctMatch || !val || typeof val !== "object") continue; const percentage = parseFloat(pctMatch[1]); - const properties: Record = {}; - for (const [pk, pv] of Object.entries(val as Record)) { - if (pk === "ease") continue; - if (typeof pv === "number") properties[pk] = Math.round(pv * 1000) / 1000; - else if (typeof pv === "string") properties[pk] = pv; - } + const properties = collectAnimatableKeyframeProperties(val as Record); if (Object.keys(properties).length > 0) { keyframes.push({ percentage, properties }); } diff --git a/packages/studio/src/hooks/gsapTweenSynth.test.ts b/packages/studio/src/hooks/gsapTweenSynth.test.ts index 3d8d771222..b5474cf1de 100644 --- a/packages/studio/src/hooks/gsapTweenSynth.test.ts +++ b/packages/studio/src/hooks/gsapTweenSynth.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; -import { synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; +import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; function anim(overrides: Partial): GsapAnimation { return { @@ -53,3 +53,32 @@ describe("synthesizeFlatTweenKeyframes", () => { expect(out).not.toBeNull(); }); }); + +describe("deduplicateKeyframes ease ambiguity", () => { + it("flags a same-% collision from different animations (different eases)", () => { + const merged = deduplicateKeyframes([ + { percentage: 45, properties: { x: 10 }, ease: "power2.in", animationId: "#a-position" }, + { percentage: 45, properties: { opacity: 1 }, ease: "power2.out", animationId: "#a-visual" }, + ]); + const kf = merged.find((k) => k.percentage === 45); + expect(kf?.easeAmbiguous).toBe(true); + }); + + it("flags a cross-animation collision even when the raw eases match", () => { + // The button can still only target one arbitrary animation, and each may + // inherit a different easeEach/animation ease that raw comparison misses. + const merged = deduplicateKeyframes([ + { percentage: 45, properties: { x: 10 }, ease: "power2.in", animationId: "#a-position" }, + { percentage: 45, properties: { opacity: 1 }, ease: "power2.in", animationId: "#a-visual" }, + ]); + expect(merged.find((k) => k.percentage === 45)?.easeAmbiguous).toBe(true); + }); + + it("does not flag a same-% collision within a single animation", () => { + const merged = deduplicateKeyframes([ + { percentage: 45, properties: { x: 10 }, ease: "power2.in", animationId: "#a-position" }, + { percentage: 45, properties: { y: 20 }, ease: "power2.out", animationId: "#a-position" }, + ]); + expect(merged.find((k) => k.percentage === 45)?.easeAmbiguous).toBeFalsy(); + }); +}); diff --git a/packages/studio/src/hooks/gsapTweenSynth.ts b/packages/studio/src/hooks/gsapTweenSynth.ts index edb849f28c..20daf0812f 100644 --- a/packages/studio/src/hooks/gsapTweenSynth.ts +++ b/packages/studio/src/hooks/gsapTweenSynth.ts @@ -5,14 +5,26 @@ import type { } from "@hyperframes/core/gsap-parser"; import { PROPERTY_DEFAULTS } from "./gsapShared"; -export function deduplicateKeyframes( - keyframes: GsapPercentageKeyframe[], -): GsapPercentageKeyframe[] { - const byPct = new Map(); +export function deduplicateKeyframes< + T extends GsapPercentageKeyframe & { animationId?: string; easeAmbiguous?: boolean }, +>(keyframes: T[]): T[] { + const byPct = new Map(); for (const kf of keyframes) { const existing = byPct.get(kf.percentage); if (existing) { existing.properties = { ...existing.properties, ...kf.properties }; + // Two DIFFERENT source animations with a keyframe at the same clip %: a + // single inline ease button can only target one of them, and which one is + // arbitrary (each may also inherit a different easeEach/animation ease, so + // comparing raw keyframe eases isn't enough). Flag it so the collapsed row + // hides the button there and the user edits per-lane instead. + if ( + existing.animationId !== undefined && + kf.animationId !== undefined && + existing.animationId !== kf.animationId + ) { + existing.easeAmbiguous = true; + } if (kf.ease) existing.ease = kf.ease; } else { byPct.set(kf.percentage, { ...kf, properties: { ...kf.properties } }); @@ -41,29 +53,40 @@ export function synthesizeFlatTweenKeyframes(anim: GsapAnimation): GsapKeyframes const fromProps = anim.fromProperties; if (!toProps || Object.keys(toProps).length === 0) return null; - const startProps: Record = {}; - const endProps: Record = {}; + const rawStart: Record = {}; + const rawEnd: Record = {}; if (anim.method === "from") { for (const [k, v] of Object.entries(toProps)) { - startProps[k] = v; - endProps[k] = PROPERTY_DEFAULTS[k] ?? 0; + rawStart[k] = v; + rawEnd[k] = PROPERTY_DEFAULTS[k] ?? 0; } } else if (anim.method === "fromTo" && fromProps) { - Object.assign(startProps, fromProps); - Object.assign(endProps, toProps); + Object.assign(rawStart, fromProps); + Object.assign(rawEnd, toProps); } else { for (const [k, v] of Object.entries(toProps)) { - startProps[k] = PROPERTY_DEFAULTS[k] ?? 0; - endProps[k] = v; + rawStart[k] = PROPERTY_DEFAULTS[k] ?? 0; + rawEnd[k] = v; } } + // Only numeric props are keyframe-interpolatable — a flat tween of a + // non-numeric prop (e.g. backgroundColor: "#fff") can't be a 2-keyframe lane. + const numericKeys = Object.keys(rawEnd).filter( + (k) => typeof rawStart[k] === "number" && typeof rawEnd[k] === "number", + ); + if (numericKeys.length === 0) return null; + const startProps = Object.fromEntries(numericKeys.map((k) => [k, rawStart[k]])); + const endProps = Object.fromEntries(numericKeys.map((k) => [k, rawEnd[k]])); + return { format: "percentage", keyframes: [ { percentage: 0, properties: startProps }, - { percentage: 100, properties: endProps }, + // Segment ease lives on the destination keyframe (Figma/AE model) so the + // lane + cache surface it; also kept data-level for useGsapTweenCache. + { percentage: 100, properties: endProps, ...(anim.ease ? { ease: anim.ease } : {}) }, ], ...(anim.ease ? { ease: anim.ease } : {}), }; diff --git a/packages/studio/src/hooks/useDomSelection.ts b/packages/studio/src/hooks/useDomSelection.ts index 903894c7e9..568c964228 100644 --- a/packages/studio/src/hooks/useDomSelection.ts +++ b/packages/studio/src/hooks/useDomSelection.ts @@ -24,6 +24,7 @@ import { type DomEditSelection, } from "../components/editor/domEditing"; import { reapplyPositionEditsAfterSeek } from "../components/editor/manualEdits"; +import { useStudioTestHooks } from "./useStudioTestHooks"; // ── Types ── @@ -506,6 +507,9 @@ export function useDomSelection({ applyDomSelection(null, { revealPanel: false }); }, [applyDomSelection, captionEditMode]); + // Dev-only headless-QA shortcut (window.__studioTest.selectByDomId). No-op in prod. + useStudioTestHooks({ previewIframeRef, buildDomSelectionFromTarget, applyDomSelection }); + const applyMarqueeSelection = useCallback( // fallow-ignore-next-line complexity (selections: DomEditSelection[], additive: boolean) => { diff --git a/packages/studio/src/hooks/useGestureCommit.ts b/packages/studio/src/hooks/useGestureCommit.ts index a0aba6747f..cdc51dcf97 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 } from "./gsapShared"; +import { isInstantHold, idSelector } from "./gsapShared"; type RecordedKeyframe = { percentage: number; @@ -168,7 +168,7 @@ export function useGestureCommit({ if (!sortedPcts.includes(0)) sortedPcts.unshift(0); } - const selector = sel.id ? `#${sel.id}` : sel.selector; + const selector = sel.id ? idSelector(sel.id) : sel.selector; if (!selector) { showToast("Cannot save — element has no selector", "error"); return; diff --git a/packages/studio/src/hooks/useGsapKeyframeOps.test.tsx b/packages/studio/src/hooks/useGsapKeyframeOps.test.tsx index e697ab6127..5e01b9e6da 100644 --- a/packages/studio/src/hooks/useGsapKeyframeOps.test.tsx +++ b/packages/studio/src/hooks/useGsapKeyframeOps.test.tsx @@ -19,11 +19,16 @@ afterEach(() => { const selection: DomEditSelection = { id: "box", selector: "#box" } as DomEditSelection; +function successfulCommitMutation() { + return vi.fn<(...args: unknown[]) => Promise>(async () => ({ ok: true })); +} + function renderKeyframeOps(over: { commitMutation: (...args: unknown[]) => Promise; trackGsapSaveFailure: (...args: unknown[]) => void; }) { const captured: { api: HookApi | null } = { api: null }; + // This hook harness intentionally mirrors the separate script-commit harness. function Probe() { // fallow-ignore-next-line code-duplication captured.api = useGsapKeyframeOps({ @@ -51,9 +56,7 @@ function renderKeyframeOps(over: { describe("useGsapKeyframeOps — resizeKeyframedTween", () => { it("issues a resize-keyframed-tween mutation with the remap + window", async () => { - const commitMutation = vi.fn<(...args: unknown[]) => Promise>(async () => ({ - ok: true, - })); + const commitMutation = successfulCommitMutation(); const trackGsapSaveFailure = vi.fn<(...args: unknown[]) => void>(); const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure }); @@ -102,10 +105,28 @@ describe("useGsapKeyframeOps — resizeKeyframedTween", () => { }); describe("useGsapKeyframeOps — keyframe transaction options", () => { + it("routes a flat-lane add through the add-keyframe writer mutation", async () => { + const commitMutation = successfulCommitMutation(); + const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure: vi.fn() }); + + await act(async () => { + await api.addKeyframeBatch(selection, "box-to-0-position", 50, { x: 210 }); + }); + + expect(commitMutation).toHaveBeenCalledWith( + selection, + { + type: "add-keyframe", + animationId: "box-to-0-position", + percentage: 50, + properties: { x: 210 }, + }, + { label: "Add keyframe at 50%", softReload: true }, + ); + }); + it("soft-reloads a standalone convert when the SDK path is unavailable", async () => { - const commitMutation = vi.fn<(...args: unknown[]) => Promise>(async () => ({ - ok: true, - })); + const commitMutation = successfulCommitMutation(); const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure: vi.fn() }); await act(async () => { @@ -123,9 +144,7 @@ describe("useGsapKeyframeOps — keyframe transaction options", () => { }); it("threads one coalesce key through skipped convert reload and terminal batch edit", async () => { - const commitMutation = vi.fn<(...args: unknown[]) => Promise>(async () => ({ - ok: true, - })); + const commitMutation = successfulCommitMutation(); const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure: vi.fn() }); const coalesceKey = "enable-keyframes:box-to-0-opacity:1"; diff --git a/packages/studio/src/hooks/useGsapSelectionHandlers.ts b/packages/studio/src/hooks/useGsapSelectionHandlers.ts index 0b11d760de..efde12caa5 100644 --- a/packages/studio/src/hooks/useGsapSelectionHandlers.ts +++ b/packages/studio/src/hooks/useGsapSelectionHandlers.ts @@ -173,8 +173,8 @@ export function useGsapSelectionHandlers({ ); const handleGsapDeleteAnimation = useCallback( - (animId: string) => { - const sel = domEditSelection ?? lastSelectionRef.current; + (animId: string, selectionOverride?: DomEditSelection | null) => { + const sel = selectionOverride ?? domEditSelection ?? lastSelectionRef.current; if (!sel) return; observeGsapMutation(deleteGsapAnimation(sel, animId), sel, "delete", "Delete GSAP animation"); }, @@ -298,17 +298,15 @@ export function useGsapSelectionHandlers({ percentage: number, properties: Record, commitOverrides?: Partial, + selectionOverride?: DomEditSelection | null, ) => { - if (!domEditSelection) return Promise.resolve(); - return addKeyframeBatch( - domEditSelection, - animId, - percentage, - properties, - commitOverrides, - ).catch((error) => { - trackGsapHandlerFailure(error, domEditSelection, "add-keyframe", "Add keyframe"); - }); + const sel = selectionOverride ?? domEditSelection ?? lastSelectionRef.current; + if (!sel) return Promise.resolve(); + return addKeyframeBatch(sel, animId, percentage, properties, commitOverrides).catch( + (error) => { + trackGsapHandlerFailure(error, sel, "add-keyframe", "Add keyframe"); + }, + ); }, [domEditSelection, addKeyframeBatch, trackGsapHandlerFailure], ); diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 226ec4c194..6a8e9d5481 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -6,6 +6,7 @@ import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeBrid import { clearKeyframeCacheForElement, clearKeyframeCacheForFile, + writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; import { toAbsoluteTime } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; @@ -328,6 +329,12 @@ export function useGsapAnimationsForElement( // fallow-ignore-next-line complexity useEffect(() => { if (!elementId) return; + const sourceAnimations = animations.filter( + (animation) => + animation.propertyGroup && (animation.keyframes || synthesizeFlatTweenKeyframes(animation)), + ); + if (sourceAnimations.length > 0) + writeGsapAnimationsForElement(sourceFile, elementId, sourceAnimations); // Resolve the element's time range from the player store so we can // convert tween-relative keyframe percentages to clip-relative ones. @@ -340,7 +347,11 @@ export function useGsapAnimationsForElement( ); const allKeyframes: Array< - GsapKeyframesData["keyframes"][0] & { tweenPercentage?: number; propertyGroup?: string } + GsapKeyframesData["keyframes"][0] & { + tweenPercentage?: number; + propertyGroup?: string; + animationId?: string; + } > = []; let format: GsapKeyframesData["format"] = "percentage"; let ease: string | undefined; @@ -378,6 +389,7 @@ export function useGsapAnimationsForElement( percentage: clipPct, tweenPercentage: k.percentage, propertyGroup: anim.propertyGroup, + animationId: anim.id, }); } format = kf.format; @@ -459,6 +471,7 @@ export function usePopulateKeyframeCacheForFile( const { elements, domClipChildren } = usePlayerStore.getState(); const doc = iframeRef?.current?.contentDocument; const mergedByElement = new Map(); + const sourceByElement = new Map(); for (const anim of parsed.animations) { if (anim.hasUnresolvedKeyframes) continue; // Position-only static holds are not keyframed animations — skip them so @@ -479,12 +492,16 @@ export function usePopulateKeyframeCacheForFile( // Attribute the tween to every element it animates (handles class / // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { + // kfData is already resolved (real keyframes OR a synthesized flat + // tween), so a grouped flat tween joins the store like a keyframed one. + if (anim.propertyGroup) { + sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); + } const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren); const clipKeyframes = kfData.keyframes.map((kf) => { const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - // 0.001% precision (matching useGsapAnimationsForElement above) so a - // beat-snapped keyframe centers exactly on the beat dot and the two - // caches agree on a keyframe's percentage. + // 0.001% precision (see useGsapAnimationsForElement) so a beat-snapped + // keyframe centers on the beat dot and both caches agree. const clipPct = elDuration > 0 ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 @@ -494,6 +511,7 @@ export function usePopulateKeyframeCacheForFile( percentage: clipPct, tweenPercentage: kf.percentage, propertyGroup: anim.propertyGroup, + animationId: anim.id, // parity with other cache writers; inline ease needs it }; }); const existing = mergedByElement.get(id); @@ -508,6 +526,7 @@ export function usePopulateKeyframeCacheForFile( setKeyframeCache(`${sf}#${id}`, kfData); setKeyframeCache(id, kfData); if (sf !== "index.html") setKeyframeCache(`index.html#${id}`, kfData); + writeGsapAnimationsForElement(sf, id, sourceByElement.get(id)); } astFetchDoneRef.current = fetchKey; }); diff --git a/packages/studio/src/hooks/useStudioTestHooks.ts b/packages/studio/src/hooks/useStudioTestHooks.ts new file mode 100644 index 0000000000..a00e01a455 --- /dev/null +++ b/packages/studio/src/hooks/useStudioTestHooks.ts @@ -0,0 +1,51 @@ +import { useEffect } from "react"; +import type { DomEditSelection } from "../components/editor/domEditing"; + +interface StudioTestHookDeps { + previewIframeRef: React.MutableRefObject; + buildDomSelectionFromTarget: (target: HTMLElement) => Promise; + applyDomSelection: ( + selection: DomEditSelection | null, + options?: { revealPanel?: boolean }, + ) => void; +} + +/** + * Dev-only headless-QA shortcut. Selecting an element normally requires a + * pixel-precise click inside the preview iframe, which automated verification + * can't reliably land. `window.__studioTest.selectByDomId(id)` resolves the + * DomEditSelection for a preview element by id and reveals the inspector — + * exactly what a click does — so a driver can open the property/ease panels and + * then focus a segment via `__playerStore.getState().setFocusedEaseSegment`. + * No-op in production builds. + */ +export function useStudioTestHooks({ + previewIframeRef, + buildDomSelectionFromTarget, + applyDomSelection, +}: StudioTestHookDeps): void { + // eslint-disable-next-line no-restricted-syntax + useEffect(() => { + let isDev = false; + try { + isDev = import.meta.env.DEV === true; + } catch { + isDev = false; + } + if (!isDev || typeof window === "undefined") return; + const api = { + selectByDomId: async (id: string): Promise => { + const element = previewIframeRef.current?.contentDocument?.getElementById(id) ?? null; + if (!element) return false; + const selection = await buildDomSelectionFromTarget(element); + if (!selection) return false; + applyDomSelection(selection, { revealPanel: true }); + return true; + }, + }; + (window as unknown as { __studioTest?: typeof api }).__studioTest = api; + return () => { + (window as unknown as { __studioTest?: typeof api }).__studioTest = undefined; + }; + }, [applyDomSelection, buildDomSelectionFromTarget, previewIframeRef]); +} diff --git a/packages/studio/src/player/store/keyframeSlice.ts b/packages/studio/src/player/store/keyframeSlice.ts new file mode 100644 index 0000000000..f219983bce --- /dev/null +++ b/packages/studio/src/player/store/keyframeSlice.ts @@ -0,0 +1,109 @@ +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import type { StoreApi } from "zustand"; + +/** Minimal keyframe cache types — mirrors GsapKeyframesData without pulling in Node-only gsap-parser. */ +export interface KeyframeCacheEntry { + format: string; + keyframes: Array<{ + percentage: number; + /** Original tween-relative percentage (server mutations need this, not the clip-relative `percentage`). */ + tweenPercentage?: number; + /** Which property group the source tween belongs to (position, scale, rotation, visual, etc.). */ + propertyGroup?: string; + /** Source tween id — lets the inline clip-row ease button target a specific segment. */ + animationId?: string; + properties: Record; + ease?: string; + /** Set when 2+ source animations collide at this percentage (a single inline + * ease button can't target one): the collapsed row hides the button here. */ + easeAmbiguous?: boolean; + }>; + ease?: string; + easeEach?: string; +} + +export interface KeyframeSlice { + /** Selected collapsed (`element:pct`) or expanded (`element:group:animation:clipPct`) diamonds. */ + selectedKeyframes: Set; + toggleSelectedKeyframe: (key: string) => void; + clearSelectedKeyframes: () => void; + + /** Clips whose keyframe property lanes are expanded in the timeline. */ + expandedClipIds: Set; + toggleClipExpanded: (id: string) => void; + setClipExpanded: (id: string, expanded: boolean) => void; + /** Union-expand clips (keyframed clips are expanded by default on load). */ + expandClips: (ids: readonly string[]) => void; + + /** elementId scopes the request to one element so a shared (class-selector) + * animation id can't open the ease editor on the wrong element. */ + focusedEaseSegment: { animationId: string; tweenPercentage: number; elementId: string } | null; + setFocusedEaseSegment: ( + target: { animationId: string; tweenPercentage: number; elementId: string } | null, + ) => void; + + /** Keyframe data per element id, populated from parsed GSAP animations. */ + keyframeCache: Map; + /** Unmerged source tweens per element; expanded property lanes read this, never keyframeCache. */ + gsapAnimations: Map; + setGsapAnimations: (elementId: string, animations: GsapAnimation[] | undefined) => void; + setKeyframeCache: (elementId: string, data: KeyframeCacheEntry | undefined) => void; +} + +export function createKeyframeSlice(set: StoreApi["setState"]): KeyframeSlice { + return { + selectedKeyframes: new Set(), + toggleSelectedKeyframe: (key) => + set((state) => { + const next = new Set(state.selectedKeyframes); + if (next.has(key)) next.delete(key); + else next.add(key); + return { selectedKeyframes: next }; + }), + clearSelectedKeyframes: () => set({ selectedKeyframes: new Set() }), + + expandedClipIds: new Set(), + toggleClipExpanded: (id) => + set((state) => { + const next = new Set(state.expandedClipIds); + if (next.has(id)) next.delete(id); + else next.add(id); + return { expandedClipIds: next }; + }), + setClipExpanded: (id, expanded) => + set((state) => { + if (state.expandedClipIds.has(id) === expanded) return state; + const next = new Set(state.expandedClipIds); + if (expanded) next.add(id); + else next.delete(id); + return { expandedClipIds: next }; + }), + expandClips: (ids) => + set((state) => { + if (ids.every((id) => state.expandedClipIds.has(id))) return state; + const next = new Set(state.expandedClipIds); + for (const id of ids) next.add(id); + return { expandedClipIds: next }; + }), + + focusedEaseSegment: null, + setFocusedEaseSegment: (target) => set({ focusedEaseSegment: target }), + + keyframeCache: new Map(), + setKeyframeCache: (elementId, data) => + set((state) => { + const next = new Map(state.keyframeCache); + if (data) next.set(elementId, data); + else next.delete(elementId); + return { keyframeCache: next }; + }), + gsapAnimations: new Map(), + setGsapAnimations: (elementId, animations) => + set((state) => { + const next = new Map(state.gsapAnimations); + if (animations) next.set(elementId, animations); + else next.delete(elementId); + return { gsapAnimations: next }; + }), + }; +} diff --git a/packages/studio/src/player/store/playerStore.test.ts b/packages/studio/src/player/store/playerStore.test.ts index ed86efaa36..1eb3f72c56 100644 --- a/packages/studio/src/player/store/playerStore.test.ts +++ b/packages/studio/src/player/store/playerStore.test.ts @@ -25,6 +25,31 @@ describe("usePlayerStore", () => { expect(state.loopEnabled).toBe(false); expect(state.zoomMode).toBe("fit"); expect(state.manualZoomPercent).toBe(100); + expect(state.expandedClipIds).toEqual(new Set()); + }); + }); + + describe("expandedClipIds", () => { + it("toggles clip membership", () => { + const store = usePlayerStore.getState(); + + store.toggleClipExpanded("clip-1"); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set(["clip-1"])); + + store.toggleClipExpanded("clip-1"); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set()); + }); + + it("sets clip membership idempotently", () => { + const store = usePlayerStore.getState(); + + store.setClipExpanded("clip-1", true); + store.setClipExpanded("clip-1", true); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set(["clip-1"])); + + store.setClipExpanded("clip-1", false); + store.setClipExpanded("clip-1", false); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set()); }); }); diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index 54b75a5c30..faf3776b5a 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -4,22 +4,9 @@ import type { BeatEditState } from "../../utils/beatEditing"; import type { ClipManifestClip } from "../lib/playbackTypes"; import { readStudioUiPreferences, writeStudioUiPreferences } from "../../utils/studioUiPreferences"; import { computePinnedZoomPercent } from "../components/timelineZoom"; +import { createKeyframeSlice, type KeyframeSlice } from "./keyframeSlice"; -/** Minimal keyframe cache types — mirrors GsapKeyframesData without pulling in Node-only gsap-parser. */ -export interface KeyframeCacheEntry { - format: string; - keyframes: Array<{ - percentage: number; - /** Original tween-relative percentage (server mutations need this, not the clip-relative `percentage`). */ - tweenPercentage?: number; - /** Which property group the source tween belongs to (position, scale, rotation, visual, etc.). */ - propertyGroup?: string; - properties: Record; - ease?: string; - }>; - ease?: string; - easeEach?: string; -} +export type { KeyframeCacheEntry } from "./keyframeSlice"; export interface TimelineElement { id: string; @@ -109,7 +96,7 @@ function resolveElementSelection( }; } -interface PlayerState { +interface PlayerState extends KeyframeSlice { isPlaying: boolean; currentTime: number; duration: number; @@ -140,11 +127,6 @@ interface PlayerState { activeTool: TimelineTool; setActiveTool: (tool: TimelineTool) => void; - /** Set of selected keyframe keys in format `${elementId}:${percentage}`. */ - selectedKeyframes: Set; - toggleSelectedKeyframe: (key: string) => void; - clearSelectedKeyframes: () => void; - /** Tween-relative percentage of the last-clicked keyframe diamond. Operations * (drag, resize, rotate) target this instead of recomputing from playhead. */ activeKeyframePct: number | null; @@ -193,10 +175,6 @@ interface PlayerState { toggleSelectedElementId: (id: string) => void; clearSelection: () => void; - /** Keyframe data per element id, populated from parsed GSAP animations. */ - keyframeCache: Map; - setKeyframeCache: (elementId: string, data: KeyframeCacheEntry | undefined) => void; - setIsPlaying: (playing: boolean) => void; setCurrentTime: (time: number) => void; setDuration: (duration: number) => void; @@ -327,15 +305,7 @@ export const usePlayerStore = create((set, get) => ({ activeTool: "select", setActiveTool: (tool) => set({ activeTool: tool }), - selectedKeyframes: new Set(), - toggleSelectedKeyframe: (key) => - set((s) => { - const next = new Set(s.selectedKeyframes); - if (next.has(key)) next.delete(key); - else next.add(key); - return { selectedKeyframes: next }; - }), - clearSelectedKeyframes: () => set({ selectedKeyframes: new Set() }), + ...createKeyframeSlice(set), activeKeyframePct: null, setActiveKeyframePct: (pct) => set({ activeKeyframePct: pct }), @@ -363,15 +333,6 @@ export const usePlayerStore = create((set, get) => ({ }), clearSelection: () => set({ selectedElementId: null, selectedElementIds: new Set() }), - keyframeCache: new Map(), - setKeyframeCache: (elementId, data) => - set((s) => { - const next = new Map(s.keyframeCache); - if (data) next.set(elementId, data); - else next.delete(elementId); - return { keyframeCache: next }; - }), - requestedSeekTime: null, requestSeek: (time) => set({ requestedSeekTime: time }), clearSeekRequest: () => set({ requestedSeekTime: null }), @@ -569,9 +530,11 @@ export const usePlayerStore = create((set, get) => ({ outPoint: null, activeTool: "select", selectedKeyframes: new Set(), + expandedClipIds: new Set(), selectedElementIds: new Set(), clipRevealRequest: null, keyframeCache: new Map(), + gsapAnimations: new Map(), beatAnalysis: null, beatEdits: null, beatUndo: [], diff --git a/packages/studio/src/telemetry/events.test.ts b/packages/studio/src/telemetry/events.test.ts index ad4e7f1401..d930adaba1 100644 --- a/packages/studio/src/telemetry/events.test.ts +++ b/packages/studio/src/telemetry/events.test.ts @@ -12,6 +12,7 @@ const { trackStudioRenderStart, trackStudioRazorSplit, trackStudioExpandedClipEdit, + trackStudioKeyframeLaneExpand, trackStudioSegmentEaseEdit, trackStudioFeedback, } = await import("./events"); @@ -72,8 +73,13 @@ describe("studio telemetry events", () => { expect(trackEvent).toHaveBeenCalledWith("studio_expanded_clip_edit", { action: "resize" }); }); + it("trackStudioKeyframeLaneExpand emits 'studio_keyframe_lane_expand' with expanded", () => { + trackStudioKeyframeLaneExpand({ expanded: true }); + expect(trackEvent).toHaveBeenCalledWith("studio_keyframe_lane_expand", { expanded: true }); + }); + it("trackStudioSegmentEaseEdit emits 'studio_segment_ease_edit' with action and ease", () => { - trackStudioSegmentEaseEdit({ ease: "power2.out" }); + trackStudioSegmentEaseEdit({ action: "commit", ease: "power2.out" }); expect(trackEvent).toHaveBeenCalledWith("studio_segment_ease_edit", { action: "commit", ease: "power2.out", diff --git a/packages/studio/src/telemetry/events.ts b/packages/studio/src/telemetry/events.ts index b73d008029..ecc80e9bfd 100644 --- a/packages/studio/src/telemetry/events.ts +++ b/packages/studio/src/telemetry/events.ts @@ -63,9 +63,17 @@ export function trackStudioExpandedClipEdit(props: { trackEvent("studio_expanded_clip_edit", { action: props.action }); } -// Adoption signal for committing an edit to a segment's ease. -export function trackStudioSegmentEaseEdit(props: { ease: string }): void { - trackEvent("studio_segment_ease_edit", { action: "commit", ease: props.ease }); +// Adoption signal for the per-clip keyframe-lane caret toggle. +export function trackStudioKeyframeLaneExpand(props: { expanded: boolean }): void { + trackEvent("studio_keyframe_lane_expand", { expanded: props.expanded }); +} + +// Adoption signal for opening and committing the per-segment ease editor. +export function trackStudioSegmentEaseEdit(props: { + action: "open" | "commit"; + ease?: string; +}): void { + trackEvent("studio_segment_ease_edit", { action: props.action, ease: props.ease }); } export function trackStudioFeedback(props: { rating: number; comment?: string }): void { From e34e529720376685f640559fc71b53c042afb7ea Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 25 Jul 2026 22:48:00 +0200 Subject: [PATCH 2/7] fix(studio): keep gsapAnimations in sync with the keyframe cache An ungrouped tween (mixed property groups classify to propertyGroup undefined) fed keyframeCache but was skipped by every gsapAnimations writer, so the collapsed row drew diamonds the expanded lanes had no source animation to render. Drop the property-group gate at all three writers; lane consumers already filter by group. Also route the same-percentage merge in updateKeyframeCacheFromParsed through deduplicateKeyframes so the easeAmbiguous rule has one owner. --- .../hooks/gsapKeyframeCacheHelpers.test.ts | 24 ++++++++++++ .../src/hooks/gsapKeyframeCacheHelpers.ts | 37 ++++++------------- .../studio/src/hooks/useGsapTweenCache.ts | 12 +++--- 3 files changed, 41 insertions(+), 32 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index ee72035f5e..7ac1ac2ce4 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -195,6 +195,30 @@ describe("updateKeyframeCacheFromParsed", () => { expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]); }); + it("records an ungrouped tween in gsapAnimations too, so the two stores agree", () => { + // `{ x, opacity }` spans two property groups, so the parser leaves + // propertyGroup undefined. Skipping it here used to cache diamonds with no + // source animation behind them: the collapsed row drew keyframes the + // expanded lanes could not render. + const animation: GsapAnimation = { + id: "mixed-box", + targetSelector: "#box", + method: "to", + position: 0, + properties: { x: 100, opacity: 0 }, + duration: 1, + resolvedStart: 0, + }; + usePlayerStore.setState({ + elements: [{ id: "box-clip", domId: "box", tag: "div", start: 0, duration: 1, track: 0 }], + }); + + updateKeyframeCacheFromParsed([animation], "scene.html", "box", {}); + + expect(cache().has("scene.html#box")).toBe(true); + expect(usePlayerStore.getState().gsapAnimations.get("scene.html#box")).toEqual([animation]); + }); + it("does not cache a flat tween without animatable numeric properties", () => { const animation: GsapAnimation = { id: "flat-box", diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index 58da5c7aae..d65ef1b988 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -5,7 +5,7 @@ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; import { toAbsoluteTime } from "./gsapShared"; -import { synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; +import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( animations: GsapAnimation[], @@ -23,9 +23,12 @@ export function updateKeyframeCacheFromParsed( anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? []; if (!id || kfSource.length === 0) continue; idsWithKeyframes.add(id); - if (anim.propertyGroup) { - sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); - } + // Every tween that fed keyframeCache also lands in gsapAnimations, group or + // not: a mixed-group tween (`{ x, opacity }` classifies to undefined) used to + // cache diamonds with no source animation behind them, so the collapsed row + // drew keyframes the expanded lanes couldn't render. Lane consumers do the + // group filtering themselves (animationContributesLane). + sourceAnimations.set(id, [...(sourceAnimations.get(id) ?? []), anim]); // Convert tween-relative percentages to clip-relative so diamonds // render at the correct position within the timeline clip. @@ -51,28 +54,10 @@ export function updateKeyframeCacheFromParsed( const existing = merged.get(id); if (existing) { - const byPct = new Map(); - for (const kf of [...existing.keyframes, ...clipKeyframes]) { - const prev = byPct.get(kf.percentage); - if (prev) { - prev.properties = { ...prev.properties, ...kf.properties }; - // Mirror deduplicateKeyframes: a same-% collision across different - // source animations is an ambiguous merged segment (the button can - // only target one arbitrary animation). Flag it so the collapsed row - // suppresses the inline ease button there. - if ( - prev.animationId !== undefined && - kf.animationId !== undefined && - prev.animationId !== kf.animationId - ) { - prev.easeAmbiguous = true; - } - if (kf.ease) prev.ease = kf.ease; - } else { - byPct.set(kf.percentage, { ...kf, properties: { ...kf.properties } }); - } - } - existing.keyframes = Array.from(byPct.values()).sort((a, b) => a.percentage - b.percentage); + // deduplicateKeyframes owns the same-% merge (including the easeAmbiguous + // flag downstream lanes read); a second copy of that rule here is how the + // two writers drift. + existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]); } else { merged.set(id, { ...anim.keyframes, diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 6a8e9d5481..feac2efacc 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -329,9 +329,9 @@ export function useGsapAnimationsForElement( // fallow-ignore-next-line complexity useEffect(() => { if (!elementId) return; + // No property-group filter: ungrouped tweens are recorded here as well. const sourceAnimations = animations.filter( - (animation) => - animation.propertyGroup && (animation.keyframes || synthesizeFlatTweenKeyframes(animation)), + (animation) => animation.keyframes || synthesizeFlatTweenKeyframes(animation), ); if (sourceAnimations.length > 0) writeGsapAnimationsForElement(sourceFile, elementId, sourceAnimations); @@ -493,10 +493,10 @@ export function usePopulateKeyframeCacheForFile( // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { // kfData is already resolved (real keyframes OR a synthesized flat - // tween), so a grouped flat tween joins the store like a keyframed one. - if (anim.propertyGroup) { - sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); - } + // tween), so a flat tween joins the store like a keyframed one. No + // property-group filter: this map must cover every tween the cache + // below records, or expanded lanes have nothing to render. + sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren); const clipKeyframes = kfData.keyframes.map((kf) => { const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); From acf6766ed8bc80e5a309b4b034c528999eb011bb Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 00:51:19 +0200 Subject: [PATCH 3/7] refactor(studio): one owner for clip-relative keyframe rows Each keyframe-cache writer re-derived a clip-relative percentage inline, and the post-commit writer rounded to 0.1% while the others used 0.001%. Selection keys embed that number, so a commit-time rewrite could orphan a live key. toClipPercentage owns the rounding, toClipKeyframes owns the whole row (percentage plus the tween percentage and animation identity the lanes read), and the parsed write reuses elementCacheKeys instead of open-coding the three key variants. --- .../src/hooks/gsapKeyframeCacheHelpers.ts | 28 +++------- packages/studio/src/hooks/gsapShared.test.ts | 21 +++++++- packages/studio/src/hooks/gsapShared.ts | 51 +++++++++++++++++++ .../studio/src/hooks/useGsapTweenCache.ts | 29 ++--------- 4 files changed, 82 insertions(+), 47 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index d65ef1b988..d6a1e3a82b 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -4,7 +4,7 @@ */ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; -import { toAbsoluteTime } from "./gsapShared"; +import { toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( @@ -32,25 +32,15 @@ export function updateKeyframeCacheFromParsed( // Convert tween-relative percentages to clip-relative so diamonds // render at the correct position within the timeline clip. - const tweenPos = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDur = anim.duration ?? 1; const timelineEl = elements.find( (el) => el.domId === id || (el.key ?? el.id) === `${targetPath}#${id}`, ); - const elStart = timelineEl?.start ?? 0; - const elDuration = timelineEl?.duration ?? 1; - const clipKeyframes = kfSource.map((kf) => { - const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - const clipPct = - elDuration > 0 ? Math.round(((absTime - elStart) / elDuration) * 1000) / 10 : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - animationId: anim.id, - }; - }); + const clipKeyframes = toClipKeyframes( + kfSource, + anim, + timelineEl?.start ?? 0, + timelineEl?.duration ?? 1, + ); const existing = merged.get(id); if (existing) { @@ -67,9 +57,7 @@ export function updateKeyframeCacheFromParsed( } } for (const [id, entry] of merged) { - setKeyframeCache(`${targetPath}#${id}`, entry); - setKeyframeCache(id, entry); - if (targetPath !== "index.html") setKeyframeCache(`index.html#${id}`, entry); + for (const key of elementCacheKeys(targetPath, id)) setKeyframeCache(key, entry); writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id)); } const targetId = diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 30ed300a07..1d3fc8f360 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -1,6 +1,11 @@ import { describe, it, expect } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; -import { idSelector, isInstantHold, parsePercentageKeyframes } from "./gsapShared"; +import { + idSelector, + isInstantHold, + parsePercentageKeyframes, + toClipPercentage, +} from "./gsapShared"; describe("isInstantHold", () => { const animation = (method: GsapAnimation["method"], duration?: number) => @@ -103,3 +108,17 @@ describe("idSelector", () => { } }); }); + +describe("toClipPercentage", () => { + // Selection keys embed this number, so every keyframe-cache writer has to round + // it identically: a coarser writer rewrites the cache with a different value and + // orphans the live selection key built from the finer one. + it("keeps three decimals so a beat-snapped keyframe lands on its beat", () => { + expect(toClipPercentage(1 / 3, 0, 1, 0)).toBe(33.333); + expect(toClipPercentage(2.5, 2, 4, 0)).toBe(12.5); + }); + + it("passes the tween percentage through for a zero-length clip", () => { + expect(toClipPercentage(5, 0, 0, 42)).toBe(42); + }); +}); diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 629e5976d1..fdfcc0aae9 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -216,3 +216,54 @@ export function parsePercentageKeyframes( export function toAbsoluteTime(tweenPos: number, tweenDur: number, percentage: number): number { return tweenPos + (percentage / 100) * tweenDur; } + +/** + * An absolute time as a percentage of a timeline clip, at the one precision every + * keyframe-cache writer must share. 0.001% keeps a beat-snapped keyframe centered + * on the beat dot, and because selection keys embed this number, a writer that + * rounds coarser would orphan a live selection the moment it rewrites the cache. + * A zero-length clip has no percentage to give, so the tween-% passes through. + */ +export function toClipPercentage( + absoluteTime: number, + clipStart: number, + clipDuration: number, + fallbackPercentage: number, +): number { + if (clipDuration <= 0) return fallbackPercentage; + return Math.round(((absoluteTime - clipStart) / clipDuration) * 100000) / 1000; +} + +/** + * One keyframe-cache row per tween keyframe: the percentage re-based onto the + * clip, the original tween percentage kept alongside it, and the animation + * identity every lane and selection key needs. Shared by the cache writers so + * they cannot drift in precision or in which identity fields they record. + */ +export function toClipKeyframes( + source: readonly T[], + anim: GsapAnimation, + clipStart: number, + clipDuration: number, +): Array< + T & { + tweenPercentage: number; + propertyGroup: GsapAnimation["propertyGroup"]; + animationId: string; + } +> { + const tweenStart = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); + const tweenDuration = anim.duration ?? 1; + return source.map((keyframe) => ({ + ...keyframe, + percentage: toClipPercentage( + toAbsoluteTime(tweenStart, tweenDuration, keyframe.percentage), + clipStart, + clipDuration, + keyframe.percentage, + ), + tweenPercentage: keyframe.percentage, + propertyGroup: anim.propertyGroup, + animationId: anim.id, + })); +} diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index feac2efacc..766b0f39cc 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -8,7 +8,7 @@ import { clearKeyframeCacheForFile, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime } from "./gsapShared"; +import { toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; function extractIdFromSelector(selector: string): string | null { @@ -378,12 +378,7 @@ export function useGsapAnimationsForElement( const tweenDur = anim.duration ?? elDuration; for (const k of kf.keyframes) { const absTime = toAbsoluteTime(tweenPos, tweenDur, k.percentage); - // 0.001% precision (was 0.1%) so a beat-snapped keyframe centers exactly - // on the beat dot, which is rendered at the true beat time. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : k.percentage; + const clipPct = toClipPercentage(absTime, elStart, elDuration, k.percentage); allKeyframes.push({ ...k, percentage: clipPct, @@ -486,9 +481,6 @@ export function usePopulateKeyframeCacheForFile( } const kfData = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim); if (!kfData) continue; - const tweenPos = - anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDur = anim.duration ?? 1; // Attribute the tween to every element it animates (handles class / // group / descendant selectors, not just `#id`). for (const id of resolveSelectorElementIds(anim.targetSelector, doc)) { @@ -498,22 +490,7 @@ export function usePopulateKeyframeCacheForFile( // below records, or expanded lanes have nothing to render. sourceByElement.set(id, [...(sourceByElement.get(id) ?? []), anim]); const { elStart, elDuration } = resolveClipTimingBasis(id, sf, elements, domClipChildren); - const clipKeyframes = kfData.keyframes.map((kf) => { - const absTime = toAbsoluteTime(tweenPos, tweenDur, kf.percentage); - // 0.001% precision (see useGsapAnimationsForElement) so a beat-snapped - // keyframe centers on the beat dot and both caches agree. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - animationId: anim.id, // parity with other cache writers; inline ease needs it - }; - }); + const clipKeyframes = toClipKeyframes(kfData.keyframes, anim, elStart, elDuration); const existing = mergedByElement.get(id); if (existing) { existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]); From d05ecb10910ec463370a3561d3b3868b5e94b287 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 02:13:38 +0200 Subject: [PATCH 4/7] fix(studio): scope the per-file keyframe-cache clear to its own keys R3 review follow-ups on the keyframe cache: - clearKeyframeCacheForFile collected ids from the index.html alias prefix too, so a re-scan of one composition file wiped rows a sibling file had just written (several files re-scan concurrently). Only the file's own prefixed keys name the ids now; clearKeyframeCacheForElement still takes the alias and bare key with them. - toClipKeyframes fell back to a fixed 1s tween duration, which put a duration-less tween's keyframes at a percentage no edit path agreed with. It now spans the clip, matching resolveEditableTweenDuration. - collectAnimatableKeyframeProperties takes `object` so call sites drop their `as Record` casts. Regression tests cover both fixes. --- .../hooks/gsapKeyframeCacheHelpers.test.ts | 14 +++++++++++ .../src/hooks/gsapKeyframeCacheHelpers.ts | 20 ++++++++-------- packages/studio/src/hooks/gsapShared.test.ts | 24 +++++++++++++++++++ packages/studio/src/hooks/gsapShared.ts | 13 +++++----- 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts index 7ac1ac2ce4..9bc45eec22 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.test.ts @@ -84,6 +84,20 @@ describe("clearKeyframeCacheForFile", () => { } }); + // Several composition files re-scan concurrently, so a clear that walked the + // index.html alias would delete rows a sibling file had just written. + it("leaves an index.html-owned element alone when another file re-scans", () => { + seed("index.html#title"); + seed("title"); + seed("comp.html#a"); + + clearKeyframeCacheForFile("comp.html"); + + expect(cache().has("index.html#title")).toBe(true); + expect(cache().has("title")).toBe(true); + expect(cache().has("comp.html#a")).toBe(false); + }); + it("leaves entries that belong to a different source file", () => { seed("comp.html#a"); seed("a"); diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index d6a1e3a82b..f9953f9fcd 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -92,22 +92,22 @@ export function clearKeyframeCacheForElement(sourceFile: string, elementId: stri /** * Clear every cached element of `sourceFile` before a full re-scan repopulates - * it. Collects the element ids that currently have a prefixed or index.html - * fallback key for the file and drops each through clearKeyframeCacheForElement - * so the bare key goes too — an element whose keyframes were removed (and so is - * absent from the re-scan) leaves no stale bare entry behind. + * it. Only the file's OWN prefixed keys name the ids to clear: every write sets + * the prefixed key (see elementCacheKeys), so the file's elements are all + * reachable that way, and clearKeyframeCacheForElement then takes the + * index.html alias and the bare key with them — an element whose keyframes were + * removed (and so is absent from the re-scan) leaves no stale bare entry + * behind. Reading the alias prefix here instead would collect ids owned by + * OTHER files, and several files re-scan concurrently, so this file's clear + * would wipe the entries a sibling file had just written. */ export function clearKeyframeCacheForFile(sourceFile: string): void { const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); const sfPrefix = `${sourceFile}#`; - const fallbackPrefix = "index.html#"; const ids = new Set(); for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { - const matchesFile = - key.startsWith(sfPrefix) || (sourceFile !== "index.html" && key.startsWith(fallbackPrefix)); - if (!matchesFile) continue; - const hashIdx = key.indexOf("#"); - if (hashIdx !== -1) ids.add(key.slice(hashIdx + 1)); + if (!key.startsWith(sfPrefix)) continue; + ids.add(key.slice(sfPrefix.length)); } for (const id of ids) { clearKeyframeCacheForElement(sourceFile, id); diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 1d3fc8f360..144dcfb7c7 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -4,6 +4,7 @@ import { idSelector, isInstantHold, parsePercentageKeyframes, + toClipKeyframes, toClipPercentage, } from "./gsapShared"; @@ -122,3 +123,26 @@ describe("toClipPercentage", () => { expect(toClipPercentage(5, 0, 0, 42)).toBe(42); }); }); + +describe("toClipKeyframes", () => { + const durationless: GsapAnimation = { + id: "a1", + method: "to", + targetSelector: "#box", + vars: {}, + resolvedStart: 0, + } as GsapAnimation; + + // A tween with no duration spans its clip everywhere else in Studio + // (resolveEditableTweenDuration), so the cache rows have to agree: a fixed 1s + // basis put the end keyframe at 25% of a 4s clip instead of 100%. + it("spans the clip when the tween has no duration", () => { + const rows = toClipKeyframes([{ percentage: 0 }, { percentage: 100 }], durationless, 0, 4); + expect(rows.map((row) => row.percentage)).toEqual([0, 100]); + }); + + it("keeps the tween percentage and the animation identity on every row", () => { + const rows = toClipKeyframes([{ percentage: 50 }], durationless, 0, 4); + expect(rows[0]).toMatchObject({ tweenPercentage: 50, animationId: "a1" }); + }); +}); diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index fdfcc0aae9..4780d74074 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -145,9 +145,7 @@ export interface ParsedPercentageKeyframes { easeEach?: string; } -function collectAnimatableKeyframeProperties( - entry: Record, -): Record { +function collectAnimatableKeyframeProperties(entry: object): Record { const properties: Record = {}; for (const [property, value] of Object.entries(entry)) { if (property === "ease") continue; @@ -185,7 +183,7 @@ export function parsePercentageKeyframes( steps.forEach((entry, i) => { if (!entry || typeof entry !== "object") return; const percentage = steps.length > 1 ? Math.round((i / (steps.length - 1)) * 1000) / 10 : 0; - const properties = collectAnimatableKeyframeProperties(entry as Record); + const properties = collectAnimatableKeyframeProperties(entry); if (Object.keys(properties).length > 0) keyframes.push({ percentage, properties }); }); return keyframes.length > 0 ? { keyframes } : null; @@ -199,7 +197,7 @@ export function parsePercentageKeyframes( const pctMatch = key.match(/^(\d+(?:\.\d+)?)%$/); if (!pctMatch || !val || typeof val !== "object") continue; const percentage = parseFloat(pctMatch[1]); - const properties = collectAnimatableKeyframeProperties(val as Record); + const properties = collectAnimatableKeyframeProperties(val); if (Object.keys(properties).length > 0) { keyframes.push({ percentage, properties }); } @@ -253,7 +251,10 @@ export function toClipKeyframes( } > { const tweenStart = anim.resolvedStart ?? (typeof anim.position === "number" ? anim.position : 0); - const tweenDuration = anim.duration ?? 1; + // A duration-less tween spans the clip, the same rule the edit paths use + // (resolveEditableTweenDuration). A fixed 1s here put its keyframes at a + // percentage no editor agreed with. + const tweenDuration = anim.duration ?? clipDuration; return source.map((keyframe) => ({ ...keyframe, percentage: toClipPercentage( From 3c7400af890aae0140f7bf577bd002eb05050319 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 27 Jul 2026 19:02:21 +0200 Subject: [PATCH 5/7] fix(studio): close the review findings in this PR instead of at the stack tip The R1/R3 residuals on this PR were fixed at the top of the stack, so they only cleared once every branch above landed. They belong here, next to the code they correct: - `idFromSelector` inverts `idSelector` for both regex readers, so the post-commit cache refresh stops skipping the CSS-unsafe ids `idSelector` exists to support. - `deduplicateKeyframes` drops `ease` when it is ambiguous; the flag was the only honest answer and the last-writer-wins curve belonged to an arbitrary colliding tween. - `isStaticPositionHold` is now the single owner of the hold skip. The `sourceAnimations` filter and the `allKeyframes` filter had diverged on whether `immediateRender` counts as a property. - The keyframe-cache setters no-op when the write changes nothing, instead of handing every subscriber a fresh Map. - `reset()` clears `focusedEaseSegment`. - The test hook `delete`s its window key rather than setting it to undefined, so feature detection still works. - The `toClipKeyframes` fixture uses `as unknown as T` with the justification CONTRIBUTING.md asks for. --- .../src/hooks/gsapKeyframeCacheHelpers.ts | 7 ++--- packages/studio/src/hooks/gsapShared.test.ts | 22 ++++++++++++-- packages/studio/src/hooks/gsapShared.ts | 21 +++++++++++++ packages/studio/src/hooks/gsapTweenSynth.ts | 26 +++++++++++++++- .../studio/src/hooks/useGsapTweenCache.ts | 30 +++++-------------- .../studio/src/hooks/useStudioTestHooks.ts | 4 ++- .../studio/src/player/store/keyframeSlice.ts | 13 ++++++++ .../studio/src/player/store/playerStore.ts | 1 + 8 files changed, 93 insertions(+), 31 deletions(-) diff --git a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts index f9953f9fcd..1e806a0298 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -4,7 +4,7 @@ */ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; -import { toClipKeyframes } from "./gsapShared"; +import { idFromSelector, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( @@ -18,7 +18,7 @@ export function updateKeyframeCacheFromParsed( const merged = new Map(); const sourceAnimations = new Map(); for (const anim of animations) { - const id = anim.targetSelector.match(/^#([\w-]+)/)?.[1]; + const id = idFromSelector(anim.targetSelector); const kfSource = anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? []; if (!id || kfSource.length === 0) continue; @@ -61,8 +61,7 @@ export function updateKeyframeCacheFromParsed( writeGsapAnimationsForElement(targetPath, id, sourceAnimations.get(id)); } const targetId = - (mutation as { targetSelector?: string }).targetSelector?.match(/^#([\w-]+)/)?.[1] ?? - selectionId; + idFromSelector((mutation as { targetSelector?: string }).targetSelector) ?? selectionId; if (targetId && !idsWithKeyframes.has(targetId)) { clearKeyframeCacheForElement(targetPath, targetId); } diff --git a/packages/studio/src/hooks/gsapShared.test.ts b/packages/studio/src/hooks/gsapShared.test.ts index 144dcfb7c7..0fb17bfb74 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { + idFromSelector, idSelector, isInstantHold, parsePercentageKeyframes, @@ -125,13 +126,16 @@ describe("toClipPercentage", () => { }); describe("toClipKeyframes", () => { - const durationless: GsapAnimation = { + // Fixture carries only the fields the function under test reads; the + // double-cast is the documented way to stand in for the full runtime shape + // (CONTRIBUTING.md). + const durationless = { id: "a1", method: "to", targetSelector: "#box", vars: {}, resolvedStart: 0, - } as GsapAnimation; + } as unknown as GsapAnimation; // A tween with no duration spans its clip everywhere else in Studio // (resolveEditableTweenDuration), so the cache rows have to agree: a fixed 1s @@ -146,3 +150,17 @@ describe("toClipKeyframes", () => { expect(rows[0]).toMatchObject({ tweenPercentage: 50, animationId: "a1" }); }); }); + +describe("idFromSelector", () => { + it("round-trips every shape idSelector emits", () => { + for (const id of ["hero-word", "el_1", "01-hook-hero-word", "my.class", "1box", '1"x']) { + expect(idFromSelector(idSelector(id))).toBe(id); + } + }); + + it("returns null for a selector that does not address an id", () => { + expect(idFromSelector(".dot")).toBeNull(); + expect(idFromSelector("[data-hf-id='x']")).toBeNull(); + expect(idFromSelector(undefined)).toBeNull(); + }); +}); diff --git a/packages/studio/src/hooks/gsapShared.ts b/packages/studio/src/hooks/gsapShared.ts index 4780d74074..906b984196 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -80,6 +80,27 @@ export function idSelector(id: string): string { return SAFE_HASH_ID.test(id) ? `#${id}` : `[id="${id.replace(/(["\\])/g, "\\$1")}"]`; } +/** + * Inverse of {@link idSelector}: the element id a target selector addresses, or + * null for a selector that is not id-based (a class, a tag, a descendant path). + * + * Both shapes have to be read back, not just `#id`. Every writer emits through + * `idSelector`, so a digit-leading, dotted or otherwise CSS-unsafe id lands in + * the source as `[id="01-hook-hero"]`. A reader that only matched `#id` saw no + * id at all for those elements and skipped them — which is how the post-commit + * keyframe-cache refresh silently stopped running for exactly the ids + * `idSelector` was added to support. + */ +export function idFromSelector(selector: string | undefined | null): string | null { + if (!selector) return null; + const hash = selector.match(/^#([\w-]+)/); + if (hash) return hash[1] ?? null; + const attribute = selector.match(/^\[id="((?:\\.|[^"\\])*)"\]/); + if (!attribute) return null; + // Undo the quote/backslash escaping idSelector applies. + return (attribute[1] ?? "").replace(/\\(["\\])/g, "$1"); +} + export function selectorFromSelection(selection: DomEditSelection): string | null { if (selection.id) return idSelector(selection.id); if (selection.selector) return selection.selector; diff --git a/packages/studio/src/hooks/gsapTweenSynth.ts b/packages/studio/src/hooks/gsapTweenSynth.ts index 20daf0812f..a3b100be4d 100644 --- a/packages/studio/src/hooks/gsapTweenSynth.ts +++ b/packages/studio/src/hooks/gsapTweenSynth.ts @@ -5,6 +5,23 @@ import type { } from "@hyperframes/core/gsap-parser"; import { PROPERTY_DEFAULTS } from "./gsapShared"; +/** + * A static position hold (only x/y, no real motion) is a `set`, not a keyframe — + * it must not synthesize a diamond. Covers both `tl.set(...)` and the + * `tl.to({ duration: 0, immediateRender: true })` hold that remove-all-keyframes + * collapses to (otherwise shown as a stray 0% keyframe). + * + * Single owner: the collapsed keyframe cache and the expanded property lanes' + * `gsapAnimations` map MUST agree on it, or a hold draws a phantom expanded lane + * with no matching collapsed diamond. + */ +export function isStaticPositionHold(anim: GsapAnimation): boolean { + if (anim.keyframes) return false; + if (anim.method !== "set" && (anim.duration ?? 0) !== 0) return false; + const propKeys = Object.keys(anim.properties).filter((k) => k !== "immediateRender"); + return propKeys.length > 0 && propKeys.every((k) => k === "x" || k === "y"); +} + export function deduplicateKeyframes< T extends GsapPercentageKeyframe & { animationId?: string; easeAmbiguous?: boolean }, >(keyframes: T[]): T[] { @@ -25,7 +42,14 @@ export function deduplicateKeyframes< ) { existing.easeAmbiguous = true; } - if (kf.ease) existing.ease = kf.ease; + // Whichever tween iterated last used to win `ease`, so the merged + // keyframe carried an arbitrary one of the colliding curves. Readers that + // do not check easeAmbiguous (drag readouts, lane hints) then showed a + // curve belonging to a different animation than the one an edit targets. + // Drop it instead: ambiguous means "no single ease", and the flag is the + // only honest answer. + if (existing.easeAmbiguous) delete existing.ease; + else if (kf.ease) existing.ease = kf.ease; } else { byPct.set(kf.percentage, { ...kf, properties: { ...kf.properties } }); } diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 766b0f39cc..c1ae295b45 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -9,7 +9,11 @@ import { writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; import { toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; -import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; +import { + deduplicateKeyframes, + isStaticPositionHold, + synthesizeFlatTweenKeyframes, +} from "./gsapTweenSynth"; function extractIdFromSelector(selector: string): string | null { const match = selector.match(/^#([\w-]+)/); @@ -357,18 +361,7 @@ export function useGsapAnimationsForElement( let ease: string | undefined; let easeEach: string | undefined; for (const anim of animations) { - // A static position hold (only x/y, no real motion) is a `set`, not a - // keyframe — don't synthesize a diamond for it. Covers both `tl.set(...)` - // and the `tl.to({ duration: 0, immediateRender: true })` hold that - // remove-all-keyframes collapses to (which is otherwise shown as a stray - // 0% keyframe). - if ( - !anim.keyframes && - Object.keys(anim.properties).length > 0 && - Object.keys(anim.properties).every((k) => k === "x" || k === "y") && - (anim.method === "set" || (anim.duration ?? 0) === 0) - ) - continue; + if (isStaticPositionHold(anim)) continue; const kf = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim); if (!kf) continue; // Convert tween-relative percentages to clip-relative so diamonds @@ -469,16 +462,7 @@ export function usePopulateKeyframeCacheForFile( const sourceByElement = new Map(); for (const anim of parsed.animations) { if (anim.hasUnresolvedKeyframes) continue; - // Position-only static holds are not keyframed animations — skip them so - // they don't draw a timeline diamond. Covers both a `tl.set(...)` and the - // `tl.to({ duration: 0, immediateRender: true })` that remove-all-keyframes - // collapses a keyframed tween to. - if (!anim.keyframes && (anim.method === "set" || (anim.duration ?? 0) === 0)) { - const propKeys = Object.keys(anim.properties).filter((k) => k !== "immediateRender"); - if (propKeys.length > 0 && propKeys.every((k) => k === "x" || k === "y")) { - continue; - } - } + if (isStaticPositionHold(anim)) continue; const kfData = anim.keyframes ?? synthesizeFlatTweenKeyframes(anim); if (!kfData) continue; // Attribute the tween to every element it animates (handles class / diff --git a/packages/studio/src/hooks/useStudioTestHooks.ts b/packages/studio/src/hooks/useStudioTestHooks.ts index a00e01a455..3453d0be8f 100644 --- a/packages/studio/src/hooks/useStudioTestHooks.ts +++ b/packages/studio/src/hooks/useStudioTestHooks.ts @@ -45,7 +45,9 @@ export function useStudioTestHooks({ }; (window as unknown as { __studioTest?: typeof api }).__studioTest = api; return () => { - (window as unknown as { __studioTest?: typeof api }).__studioTest = undefined; + // delete, not `= undefined`: an own key holding undefined keeps + // `"__studioTest" in window` true, which defeats feature detection. + delete (window as unknown as { __studioTest?: typeof api }).__studioTest; }; }, [applyDomSelection, buildDomSelectionFromTarget, previewIframeRef]); } diff --git a/packages/studio/src/player/store/keyframeSlice.ts b/packages/studio/src/player/store/keyframeSlice.ts index f219983bce..7a97d986ce 100644 --- a/packages/studio/src/player/store/keyframeSlice.ts +++ b/packages/studio/src/player/store/keyframeSlice.ts @@ -92,6 +92,13 @@ export function createKeyframeSlice(set: StoreApi["setState"]): K keyframeCache: new Map(), setKeyframeCache: (elementId, data) => set((state) => { + // A write that changes nothing must not emit a new Map: the cache has + // several hot writers (per-element effect, file populate, post-commit + // updater, delete) and every no-op re-rendered every subscriber. + if ( + data ? state.keyframeCache.get(elementId) === data : !state.keyframeCache.has(elementId) + ) + return state; const next = new Map(state.keyframeCache); if (data) next.set(elementId, data); else next.delete(elementId); @@ -100,6 +107,12 @@ export function createKeyframeSlice(set: StoreApi["setState"]): K gsapAnimations: new Map(), setGsapAnimations: (elementId, animations) => set((state) => { + if ( + animations + ? state.gsapAnimations.get(elementId) === animations + : !state.gsapAnimations.has(elementId) + ) + return state; const next = new Map(state.gsapAnimations); if (animations) next.set(elementId, animations); else next.delete(elementId); diff --git a/packages/studio/src/player/store/playerStore.ts b/packages/studio/src/player/store/playerStore.ts index faf3776b5a..e8c101c8fd 100644 --- a/packages/studio/src/player/store/playerStore.ts +++ b/packages/studio/src/player/store/playerStore.ts @@ -531,6 +531,7 @@ export const usePlayerStore = create((set, get) => ({ activeTool: "select", selectedKeyframes: new Set(), expandedClipIds: new Set(), + focusedEaseSegment: null, selectedElementIds: new Set(), clipRevealRequest: null, keyframeCache: new Map(), From b386b55f735a0217751e26245fbf677c8ff5d303 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 27 Jul 2026 19:09:16 +0200 Subject: [PATCH 6/7] fix(studio): clamp the timeline scrub to 0 instead of dropping it Dragging the playhead to the start of the composition needed a very slow drag. The scrub surface begins GUTTER + TRACKS_LEFT_PAD px right of the viewport edge, and both scrub paths bailed out when the pointer sat left of that origin rather than clamping. So the last 80px of the drag toward zero silently did nothing: the playhead stuck at whatever the last in-range sample reported, and only a drag slow enough to sample inside the thin sliver before the origin ever reached 0. Both paths now share getTimelineScrubTime, which clamps to [0, duration]. One owner, so the live-feedback path and the committed-seek path cannot disagree about the edge again. --- .../player/components/timelineLayout.test.ts | 44 +++++++++++++++++++ .../src/player/components/timelineLayout.ts | 23 ++++++++++ .../player/components/useTimelinePlayhead.ts | 11 +++-- .../components/useTimelineRangeSelection.ts | 16 ++++--- 4 files changed, 85 insertions(+), 9 deletions(-) diff --git a/packages/studio/src/player/components/timelineLayout.test.ts b/packages/studio/src/player/components/timelineLayout.test.ts index 3642dea953..dde3a5cfc8 100644 --- a/packages/studio/src/player/components/timelineLayout.test.ts +++ b/packages/studio/src/player/components/timelineLayout.test.ts @@ -7,6 +7,7 @@ import { GUTTER, TRACKS_LEFT_PAD, getTimelineRowTop, + getTimelineScrubTime, getTimelineRowFromY, getTimelineCanvasHeight, resolveTimelineAssetDrop, @@ -105,3 +106,46 @@ describe("track-area breathing pad y-math", () => { }); }); }); + +describe("getTimelineScrubTime", () => { + const at = (clientX: number, duration = 10) => + getTimelineScrubTime({ + clientX, + viewportLeft: 0, + scrollLeft: 0, + pixelsPerSecond: 100, + duration, + }); + const origin = GUTTER + TRACKS_LEFT_PAD; + + it("maps the content origin to t=0", () => { + expect(at(origin)).toBe(0); + expect(at(origin + 250)).toBe(2.5); + }); + + // The bug: a pointer left of the origin used to abort the scrub instead of + // clamping, so dragging the playhead to the start only worked if a sample + // happened to land in the few px before t=0. + it("clamps a pointer left of the origin to 0 instead of dropping the scrub", () => { + expect(at(origin - 1)).toBe(0); + expect(at(origin - 500)).toBe(0); + expect(at(0)).toBe(0); + }); + + it("clamps past the end to the duration", () => { + expect(at(origin + 5000)).toBe(10); + }); + + it("returns 0 for a degenerate zoom or duration", () => { + expect( + getTimelineScrubTime({ + clientX: 500, + viewportLeft: 0, + scrollLeft: 0, + pixelsPerSecond: 0, + duration: 10, + }), + ).toBe(0); + expect(at(origin + 250, Number.NaN)).toBe(0); + }); +}); diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index c2326734f3..9a1b51108e 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -321,6 +321,29 @@ export function getTimelinePlayheadLeft(time: number, pixelsPerSecond: number): ); } +/** + * Inverse of {@link getTimelinePlayheadLeft}: the scrub time under a viewport + * clientX. Clamped to [0, duration], NOT rejected — the scrub surface starts + * `GUTTER + TRACKS_LEFT_PAD` px right of the viewport edge, so any pointer left + * of t=0 maps to a negative offset. Callers used to bail on that instead of + * clamping, which made the last 80px of the drag to zero silently do nothing: + * the playhead stuck wherever the last in-range sample landed, and only a very + * slow drag that happened to sample inside the sliver before the origin reached + * 0. Every scrub path shares this so they cannot diverge on the edge again. + */ +export function getTimelineScrubTime(input: { + clientX: number; + viewportLeft: number; + scrollLeft: number; + pixelsPerSecond: number; + duration: number; +}): number { + const { clientX, viewportLeft, scrollLeft, pixelsPerSecond, duration } = input; + if (!(pixelsPerSecond > 0) || !Number.isFinite(duration)) return 0; + const x = clientX - viewportLeft + scrollLeft - GUTTER - TRACKS_LEFT_PAD; + return Math.max(0, Math.min(duration, x / pixelsPerSecond)); +} + export function getTimelineCanvasHeight(trackCount: number): number { // RULER_H + top pad + lanes + bottom pad. The old TIMELINE_SCROLL_BUFFER is // subsumed by TRACKS_BOTTOM_PAD (which is larger), so the drag-into-void space diff --git a/packages/studio/src/player/components/useTimelinePlayhead.ts b/packages/studio/src/player/components/useTimelinePlayhead.ts index 6706c0f7b3..f7c3ab6b30 100644 --- a/packages/studio/src/player/components/useTimelinePlayhead.ts +++ b/packages/studio/src/player/components/useTimelinePlayhead.ts @@ -6,6 +6,7 @@ import { GUTTER, TRACKS_LEFT_PAD, getTimelinePlayheadLeft, + getTimelineScrubTime, getTimelineScrollLeftForZoomTransition, getTimelineScrollLeftForZoomAnchor, shouldAutoScrollTimeline, @@ -130,9 +131,13 @@ export function useTimelinePlayhead({ const el = scrollRef.current; if (!el || effectiveDuration <= 0) return; const rect = el.getBoundingClientRect(); - const x = clientX - rect.left + el.scrollLeft - GUTTER - TRACKS_LEFT_PAD; - if (x < 0) return; - const time = Math.max(0, Math.min(effectiveDuration, x / pps)); + const time = getTimelineScrubTime({ + clientX, + viewportLeft: rect.left, + scrollLeft: el.scrollLeft, + pixelsPerSecond: pps, + duration: effectiveDuration, + }); liveTime.notify(time); onSeek?.(time); }, diff --git a/packages/studio/src/player/components/useTimelineRangeSelection.ts b/packages/studio/src/player/components/useTimelineRangeSelection.ts index 1df2a74fee..6edc0ff443 100644 --- a/packages/studio/src/player/components/useTimelineRangeSelection.ts +++ b/packages/studio/src/player/components/useTimelineRangeSelection.ts @@ -7,7 +7,7 @@ import { } from "./timelineEditing"; import type { TimelineElement } from "../store/playerStore"; import { liveTime, usePlayerStore } from "../store/playerStore"; -import { GUTTER, TRACKS_LEFT_PAD } from "./timelineLayout"; +import { GUTTER, TRACKS_LEFT_PAD, getTimelineScrubTime } from "./timelineLayout"; import { computeMarqueeSelection, getMarqueeRect, @@ -286,11 +286,15 @@ export function useTimelineRangeSelection({ const el = scrollRef.current; if (el) { const rect = el.getBoundingClientRect(); - const x = clientX - rect.left + el.scrollLeft - GUTTER - TRACKS_LEFT_PAD; - if (x >= 0) { - const dur = el.scrollWidth / pps; - liveTime.notify(Math.max(0, Math.min(dur, x / pps))); - } + liveTime.notify( + getTimelineScrubTime({ + clientX, + viewportLeft: rect.left, + scrollLeft: el.scrollLeft, + pixelsPerSecond: pps, + duration: el.scrollWidth / pps, + }), + ); } if (!seekRafRef.current) { seekRafRef.current = requestAnimationFrame(() => { From 521bba64370c54d3f7eb5bc435d694b51666e9b2 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 27 Jul 2026 19:51:51 +0200 Subject: [PATCH 7/7] refactor(studio): resolve tween selector ids through the shared reader The local extractIdFromSelector duplicated the `#id`-only regex that idFromSelector replaced, so both DOM-less paths in resolveSelectorElementIds (no-iframe fallback and querySelectorAll-throw recovery) read no id at all for the bracketed `[id="..."]` form writers emit for CSS-unsafe ids. Deleted the duplicate and imported the shared reader; both forms now resolve. --- .../studio/src/hooks/useGsapTweenCache.test.ts | 16 ++++++++++++++++ packages/studio/src/hooks/useGsapTweenCache.ts | 17 +++++++---------- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/studio/src/hooks/useGsapTweenCache.test.ts b/packages/studio/src/hooks/useGsapTweenCache.test.ts index d9ff5d4a02..0492a1f501 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.test.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.test.ts @@ -90,4 +90,20 @@ describe("resolveSelectorElementIds", () => { expect(resolveSelectorElementIds("#card .label", null)).toEqual(["card"]); expect(resolveSelectorElementIds(".dot", null)).toEqual([]); }); + + // The `[id="…"]` form is what writers emit for a CSS-unsafe id (digit-leading, + // dotted). The old local `#id`-only regex read no id at all for those, so they + // silently dropped out of both DOM-less paths. + it("falls back to a bracketed id when there is no DOM", () => { + expect(resolveSelectorElementIds('[id="01-hook"] .label', null)).toEqual(["01-hook"]); + }); + + it("falls back to a bracketed id when querySelectorAll rejects the selector", () => { + const doc = { + querySelectorAll: () => { + throw new SyntaxError("bad selector"); + }, + } as unknown as Document; + expect(resolveSelectorElementIds('[id="01-hook"]:has(>*)', doc)).toEqual(["01-hook"]); + }); }); diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index c1ae295b45..8ec8481738 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -8,25 +8,22 @@ import { clearKeyframeCacheForFile, writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; +import { idFromSelector, toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; import { deduplicateKeyframes, isStaticPositionHold, synthesizeFlatTweenKeyframes, } from "./gsapTweenSynth"; -function extractIdFromSelector(selector: string): string | null { - const match = selector.match(/^#([\w-]+)/); - return match ? match[1] : null; -} - /** * Resolve a tween's target selector to the ids of the element(s) it animates. * A bare `#id` resolves directly; anything else (a class like `.dot`, a group * `.a, .b`, or a descendant selector) is matched against the live preview DOM so * class/selector tweens (e.g. `gsap.from(".dot", {stagger})`) attribute to every - * element they animate — not just one parsed from the string. Falls back to a - * leading `#id` when there's no DOM (so the cache still populates pre-iframe). + * element they animate — not just one parsed from the string. Falls back to the + * leading id when there's no DOM (so the cache still populates pre-iframe); + * `idFromSelector` reads both `#id` and the `[id="…"]` form writers emit for + * CSS-unsafe ids, so those elements resolve pre-iframe too. */ // fallow-ignore-next-line complexity export function resolveSelectorElementIds( @@ -36,7 +33,7 @@ export function resolveSelectorElementIds( const bareId = selector.match(/^#([\w-]+)$/); if (bareId) return [bareId[1]]; if (!doc) { - const lead = extractIdFromSelector(selector); + const lead = idFromSelector(selector); return lead ? [lead] : []; } const ids = new Set(); @@ -48,7 +45,7 @@ export function resolveSelectorElementIds( if (el.id) ids.add(el.id); } } catch { - const lead = extractIdFromSelector(sel); + const lead = idFromSelector(sel); if (lead) ids.add(lead); } }