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 01/25] 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 02/25] 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 03/25] 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 04/25] 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 05/25] 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 06/25] 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 07/25] 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); } } From d518972f8b2b5c163875cd5d99c4ef6f0fce2113 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 16:41:43 +0200 Subject: [PATCH 08/25] feat(studio): add variable timeline timing and layout --- packages/studio/src/App.tsx | 27 +-- .../studio/src/hooks/timelineMoveAdapter.ts | 11 +- .../src/hooks/useTimelineEditing.test.tsx | 209 ++++++++++++++++-- .../studio/src/hooks/useTimelineEditing.ts | 75 ++++--- .../src/hooks/useTimelineEditingTypes.ts | 2 + .../src/hooks/useTimelineGroupEditing.ts | 85 +++---- .../timelineClipDragPreview.test.ts | 53 ++++- .../components/timelineClipDragPreview.ts | 52 +++-- .../player/components/timelineCollision.ts | 17 +- .../player/components/timelineLayout.test.ts | 75 +++++++ .../src/player/components/timelineLayout.ts | 127 +++++++++-- .../player/components/timelineMarquee.test.ts | 97 ++++++-- .../src/player/components/timelineMarquee.ts | 24 +- .../player/components/useTimelineClipDrag.ts | 5 +- 14 files changed, 671 insertions(+), 188 deletions(-) diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index ad131e3a62..eff75a40ea 100644 --- a/packages/studio/src/App.tsx +++ b/packages/studio/src/App.tsx @@ -1,7 +1,7 @@ import { useState, useCallback, useRef, useMemo, useEffect, useLayoutEffect } from "react"; import type { LeftSidebarHandle, SidebarTab } from "./components/sidebar/LeftSidebar"; import { useRenderQueue } from "./components/renders/useRenderQueue"; -import { usePlayerStore, type TimelineElement } from "./player"; +import { usePlayerStore } from "./player"; import { StudioOverlays } from "./components/StudioOverlays"; import { SaveQueuePausedBanner } from "./components/SaveQueuePausedBanner"; import { useCaptionStore } from "./captions/store"; @@ -12,9 +12,12 @@ import { useFileManager } from "./hooks/useFileManager"; import { usePreviewPersistence } from "./hooks/usePreviewPersistence"; import { usePreviewDocumentVersion } from "./hooks/usePreviewDocumentVersion"; import { useTimelineEditing } from "./hooks/useTimelineEditing"; -import { persistTimelineMoveEditsAtomically } from "./hooks/timelineMoveAdapter"; +import { + persistTimelineMoveEditsAtomically, + type TimelineMoveEditsHandler, + type TimelineMoveOperation, +} from "./hooks/timelineMoveAdapter"; import type { TimelineZIndexReorderCommit } from "./hooks/useTimelineEditingTypes"; -import type { TimelineStackingReorderIntent } from "./player/components/timelineStacking"; import type { BlockPreviewInfo } from "./components/sidebar/BlocksTab"; import { useDomEditSession } from "./hooks/useDomEditSession"; import { useSdkSelectionSync } from "./hooks/useSdkSelectionSync"; @@ -62,7 +65,6 @@ import { } from "./utils/studioUrlState"; import { trackStudioSessionStart } from "./telemetry/events"; import { hasFiredSessionStart, markSessionStartFired } from "./telemetry/config"; -type TimelineMoveOperation = Parameters[2]; // fallow-ignore-next-line complexity export function StudioApp() { const { projectId, resolving, waitingForServer } = useServerConnection(); @@ -154,6 +156,7 @@ export function StudioApp() { reloadPreview: () => setRefreshKey((k) => k + 1), pendingTimelineEditPathRef, }); + const invalidateGsapCacheRef = useRef<() => void>(() => {}); const timelineEditing = useTimelineEditing({ projectId, activeCompPath, @@ -171,20 +174,11 @@ export function StudioApp() { sdkSession: editFlowSdkSession, publishSdkSession: sdkHandle.publish, forceReloadSdkSession: sdkHandle.forceReload, + invalidateGsapCache: () => invalidateGsapCacheRef.current(), handleDomZIndexReorderCommitRef, }); - const handleTimelineElementsMove = useCallback( - async ( - edits: Array<{ - element: TimelineElement; - updates: Pick & { - stackingReorder?: TimelineStackingReorderIntent | null; - }; - }>, - coalesceKey?: string, - operation: TimelineMoveOperation = "timing", - coalesceMs?: number, - ) => { + const handleTimelineElementsMove: TimelineMoveEditsHandler = useCallback( + async (edits, coalesceKey, operation: TimelineMoveOperation = "timing", coalesceMs) => { const deps = { handleTimelineGroupMove: timelineEditing.handleTimelineGroupMove }; await persistTimelineMoveEditsAtomically(edits, coalesceKey, operation, deps, coalesceMs); }, @@ -228,7 +222,6 @@ export function StudioApp() { const domEditDeleteBridge = (s: DomEditSelection) => handleDomEditElementDeleteRef.current(s); const resetKeyframesRef = useRef<() => boolean>(() => false); const deleteSelectedKeyframesRef = useRef<() => void>(() => {}); - const invalidateGsapCacheRef = useRef<() => void>(() => {}); const { handleCopy, handlePaste, handleCut } = useClipboard({ projectId, activeCompPath, diff --git a/packages/studio/src/hooks/timelineMoveAdapter.ts b/packages/studio/src/hooks/timelineMoveAdapter.ts index f5b9b2fb65..ee7544e8ea 100644 --- a/packages/studio/src/hooks/timelineMoveAdapter.ts +++ b/packages/studio/src/hooks/timelineMoveAdapter.ts @@ -4,7 +4,7 @@ import type { TimelineGroupMoveChange, } from "./useTimelineGroupEditing"; -interface MoveEdit { +export interface TimelineMoveEdit { element: TimelineElement; updates: Pick; } @@ -18,8 +18,15 @@ interface AtomicMoveDeps { export type TimelineMoveOperation = "timing" | "lane-reorder" | "track-insert"; +export type TimelineMoveEditsHandler = ( + edits: TimelineMoveEdit[], + coalesceKey?: string, + operation?: TimelineMoveOperation, + coalesceMs?: number, +) => Promise; + export function persistTimelineMoveEditsAtomically( - edits: MoveEdit[], + edits: TimelineMoveEdit[], coalesceKey: string | undefined, operation: TimelineMoveOperation, deps: AtomicMoveDeps, diff --git a/packages/studio/src/hooks/useTimelineEditing.test.tsx b/packages/studio/src/hooks/useTimelineEditing.test.tsx index bc88aca67d..18d81aa55d 100644 --- a/packages/studio/src/hooks/useTimelineEditing.test.tsx +++ b/packages/studio/src/hooks/useTimelineEditing.test.tsx @@ -10,6 +10,17 @@ import { jsonResponse, requestUrl } from "./fetchStubTestUtils"; import { useElementLifecycleOps } from "./useElementLifecycleOps"; import { useTimelineEditing } from "./useTimelineEditing"; +vi.mock("../components/editor/manualEditingAvailability", async (importOriginal) => { + const actual = + await importOriginal(); + return { + ...actual, + STUDIO_SDK_CUTOVER_ENABLED: true, + STUDIO_SDK_CUTOVER_FAMILIES: new Set(["timing"]), + STUDIO_SDK_RESOLVER_SHADOW_ENABLED: false, + }; +}); + (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; type ZIndexEntry = { @@ -108,7 +119,9 @@ function renderTimelineEditingHook(input: { }) => Promise; reloadPreview?: () => void; sdkSession?: Awaited> | null; + publishSdkSession?: NonNullable[0]["publishSdkSession"]>; forceReloadSdkSession?: () => void; + invalidateGsapCache?: () => void; showToast?: (message: string, kind?: string) => void; }): { move: ReturnType["handleTimelineElementMove"]; @@ -140,7 +153,9 @@ function renderTimelineEditingHook(input: { pendingTimelineEditPathRef: { current: new Set() }, uploadProjectFiles: vi.fn(), sdkSession: input.sdkSession, + publishSdkSession: input.publishSdkSession, forceReloadSdkSession: input.forceReloadSdkSession, + invalidateGsapCache: input.invalidateGsapCache, handleDomZIndexReorderCommitRef: commitRef, }); move = hook.handleTimelineElementMove; @@ -163,6 +178,9 @@ function renderTimelineEditingHook(input: { type TimelineRecordEdit = NonNullable< Parameters[0]["recordEdit"] >; +type TimelinePublishSdkSession = NonNullable< + Parameters[0]["publishSdkSession"] +>; function renderTimelineEditingHookWithLifecycle(input: { timelineElements: TimelineElement[]; @@ -227,28 +245,41 @@ async function flushAsyncWork(): Promise { * with `gsapBody`. Returns the mock for call inspection. */ function stubProjectFetch(files: string | Record, gsapBody?: unknown) { - // Keep this test server's capability, file-read, and mutation routes together; - // splitting the fixture would obscure the request sequence asserted by callers. - // fallow-ignore-next-line complexity - const fetchMock = vi.fn(async (input: Parameters[0]): Promise => { - const url = requestUrl(input); - if (url.includes("/api/projects/p1/gsap-mutation-capabilities")) { - return jsonResponse({ atomicOwnershipPairs: true }); - } - if (url.includes("/api/projects/p1/files/")) { - if (typeof files === "string") return jsonResponse({ content: files }); - const path = decodeURIComponent(url.split("/files/")[1] ?? "index.html"); - return jsonResponse({ content: files[path] }); - } - if (url.includes("/api/projects/p1/gsap-mutations/")) { - const path = decodeURIComponent(url.split("/gsap-mutations/")[1] ?? "index.html"); - const content = typeof files === "string" ? files : (files[path] ?? ""); - return jsonResponse( - gsapBody ?? { mutated: false, scriptText: null, before: content, after: content }, - ); - } - throw new Error(`Unexpected fetch: ${url}`); - }); + const pathAfter = (url: string, marker: string) => + decodeURIComponent(url.split(marker)[1] ?? "index.html"); + const fileContent = (path: string) => (typeof files === "string" ? files : files[path]); + // One handler per route, so the mock itself stays a lookup: the request + // sequence callers assert on is still readable top to bottom. + const routes: Array<[marker: string, respond: (url: string) => Response]> = [ + [ + "/api/projects/p1/gsap-mutation-capabilities", + () => jsonResponse({ atomicOwnershipPairs: true }), + ], + [ + "/api/projects/p1/files/", + (url) => jsonResponse({ content: fileContent(pathAfter(url, "/files/")) }), + ], + [ + "/api/projects/p1/gsap-mutations/", + (url) => { + const content = fileContent(pathAfter(url, "/gsap-mutations/")) ?? ""; + return jsonResponse( + gsapBody ?? { mutated: false, scriptText: null, before: content, after: content }, + ); + }, + ], + ]; + const fetchMock = vi.fn( + async ( + input: Parameters[0], + _init?: Parameters[1], + ): Promise => { + const url = requestUrl(input); + const route = routes.find(([marker]) => url.includes(marker)); + if (!route) throw new Error(`Unexpected fetch: ${url}`); + return route[1](url); + }, + ); vi.stubGlobal("fetch", fetchMock); return fetchMock; } @@ -285,6 +316,39 @@ function setupSingleClipHarness(options?: { return { iframe, clip, commit, writeProjectFile, reloadPreview, fetchMock, ...hook }; } +const SDK_KEYFRAMED_SOURCE = [ + `
`, + `
`, + `
`, + ``, +].join("\n"); + +async function setupSdkKeyframedClipHarness() { + const iframe = createPreviewIframe([{ id: "clip", track: 0 }]); + const clip = timelineElement({ id: "clip", track: 0, zIndex: 0, start: 1 }); + const sdkSession = await openComposition(SDK_KEYFRAMED_SOURCE); + const writeProjectFile = vi.fn<(...args: unknown[]) => Promise>(async () => {}); + const invalidateGsapCache = vi.fn(); + const fetchMock = stubProjectFetch(SDK_KEYFRAMED_SOURCE); + usePlayerStore.getState().setDuration(10); + const hook = renderTimelineEditingHook({ + timelineElements: [clip], + iframe, + onZIndexCommit: vi.fn().mockResolvedValue(undefined), + projectId: "p1", + writeProjectFile, + recordEdit: vi.fn(async () => {}), + sdkSession, + publishSdkSession: vi.fn(() => "published"), + invalidateGsapCache, + }); + return { clip, fetchMock, hook, invalidateGsapCache, writeProjectFile }; +} + /** Assert a lane write landed in both the live iframe DOM and the persisted file. */ function expectLanePersisted( iframe: HTMLIFrameElement, @@ -710,6 +774,58 @@ describe("useTimelineEditing timeline z-index reorder", () => { h.unmount(); }); + it("shifts authored GSAP positions after an SDK-backed clip move commits", async () => { + const { clip, fetchMock, hook, invalidateGsapCache, writeProjectFile } = + await setupSdkKeyframedClipHarness(); + + await act(async () => { + await hook.move(clip, { start: 2.25, track: clip.track }); + }); + + expect(writeProjectFile.mock.calls[0]?.[1]).toContain('data-start="2.25"'); + const mutationCall = fetchMock.mock.calls.find((call) => + requestUrl(call[0]).includes("/gsap-mutations/"), + ); + expect(mutationCall).toBeDefined(); + const init = mutationCall?.[1] as RequestInit | undefined; + expect(JSON.parse(String(init?.body))).toEqual({ + type: "shift-positions", + targetSelector: "#clip", + delta: 1.25, + }); + expect(invalidateGsapCache).toHaveBeenCalledTimes(1); + + hook.unmount(); + }); + + it("scales authored GSAP positions after an SDK-backed clip resize commits", async () => { + const { clip, fetchMock, hook, invalidateGsapCache, writeProjectFile } = + await setupSdkKeyframedClipHarness(); + + await act(async () => { + await hook.resize(clip, { start: 2, duration: 4, playbackStart: undefined }); + }); + + expect(writeProjectFile.mock.calls[0]?.[1]).toContain('data-start="2"'); + expect(writeProjectFile.mock.calls[0]?.[1]).toContain('data-duration="4"'); + const mutationCall = fetchMock.mock.calls.find((call) => + requestUrl(call[0]).includes("/gsap-mutations/"), + ); + expect(mutationCall).toBeDefined(); + const init = mutationCall?.[1] as RequestInit | undefined; + expect(JSON.parse(String(init?.body))).toEqual({ + type: "scale-positions", + targetSelector: "#clip", + oldStart: 1, + oldDuration: 2, + newStart: 2, + newDuration: 4, + }); + expect(invalidateGsapCache).toHaveBeenCalledTimes(1); + + hook.unmount(); + }); + it("persists a vertical-only lane move (start unchanged) through the single-element fallback", async () => { // Regression: `if (!startChanged) return` ran BEFORE the file persist, so a // pure lane change routed through onMoveElement (no onMoveElements wired) @@ -821,6 +937,55 @@ describe("useTimelineEditing timeline z-index reorder", () => { unmount(); }); + it("shifts every keyed clip and invalidates the cache after an SDK-backed group move", async () => { + const source = [ + `
`, + `
`, + `
`, + `
`, + ``, + ].join("\n"); + const { iframe, a, b } = makeTwoClipPair(); + const sdkSession = await openComposition(source); + const fetchMock = stubProjectFetch(source); + const invalidateGsapCache = vi.fn(); + usePlayerStore.getState().setDuration(10); + const hook = renderTimelineEditingHook({ + timelineElements: [a, b], + iframe, + onZIndexCommit: vi.fn().mockResolvedValue(undefined), + projectId: "p1", + writeProjectFile: vi.fn<(...args: unknown[]) => Promise>(async () => {}), + recordEdit: vi.fn(async () => {}), + sdkSession, + publishSdkSession: vi.fn(() => "published"), + invalidateGsapCache, + }); + + await act(async () => { + await hook.groupMove([ + { element: a, start: 1 }, + { element: b, start: 2 }, + ]); + }); + + const mutations = fetchMock.mock.calls + .filter((call) => requestUrl(call[0]).includes("/gsap-mutations/")) + .map((call) => JSON.parse(String((call[1] as RequestInit | undefined)?.body))); + expect(mutations).toEqual([ + { type: "shift-positions", targetSelector: "#a", delta: 1 }, + { type: "shift-positions", targetSelector: "#b", delta: 1 }, + ]); + expect(invalidateGsapCache).toHaveBeenCalledTimes(1); + + hook.unmount(); + }); + it("partitions a group move by source file while keeping one undo entry", async () => { const files: Record = { "index.html": '
', diff --git a/packages/studio/src/hooks/useTimelineEditing.ts b/packages/studio/src/hooks/useTimelineEditing.ts index 2c53299a20..bf846f06fe 100644 --- a/packages/studio/src/hooks/useTimelineEditing.ts +++ b/packages/studio/src/hooks/useTimelineEditing.ts @@ -58,6 +58,7 @@ export function useTimelineEditing({ sdkSession, publishSdkSession, forceReloadSdkSession, + invalidateGsapCache, handleDomZIndexReorderCommitRef, }: UseTimelineEditingOptions) { const projectIdRef = useRef(projectId); @@ -118,6 +119,7 @@ export function useTimelineEditing({ domEditSaveTimestampRef, editQueueRef, forceReloadSdkSession, + invalidateGsapCache, isRecordingRef, pendingTimelineEditPathRef, previewIframeRef, @@ -184,21 +186,24 @@ export function useTimelineEditing({ ); }; const coalesceKey = `timeline-move:${element.hfId ?? element.id}`; + const finishMoveGsapSync = () => + // Every timing writer converges the same GSAP positions after its + // durable clip-start commit. The SDK owns the attribute write; this + // sync owns only the dependent animation rewrite and preview refresh. + finishClipTimingFallback({ + iframe: previewIframeRef.current, + reloadPreview, + projectId: projectIdRef.current, + targetPath, + domId: element.domId, + label: "Move timeline clip", + coalesceKey, + recordEdit, + edit: { kind: "shift", delta: updates.start - element.start }, + }).finally(() => invalidateGsapCache?.()); const moveFallback = () => - enqueueEdit(element, "Move timeline clip", buildMovePatches, coalesceKey).then(() => - // Soft-reload with the server's rewritten GSAP script — the timing-only move already patched - // DOM + store, so swapping the script avoids the all-clips flash; falls back to reloadPreview(). - finishClipTimingFallback({ - iframe: previewIframeRef.current, - reloadPreview, - projectId: projectIdRef.current, - targetPath, - domId: element.domId, - label: "Move timeline clip", - coalesceKey, - recordEdit, - edit: { kind: "shift", delta: updates.start - element.start }, - }), + enqueueEdit(element, "Move timeline clip", buildMovePatches, coalesceKey).then( + finishMoveGsapSync, ); return reorderDone .then(() => { @@ -221,9 +226,10 @@ export function useTimelineEditing({ readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path), publishSession: publishSdkSession, }, - { label: "Move timeline clip", coalesceKey }, + { label: "Move timeline clip", coalesceKey, skipRefresh: true }, ).then((result) => { if (!cutoverCommittedOrThrow(result)) return moveFallback(); + return finishMoveGsapSync(); }); } return moveFallback(); @@ -250,6 +256,7 @@ export function useTimelineEditing({ timelineElements, handleDomZIndexReorderCommitRef, showToast, + invalidateGsapCache, ], ); @@ -287,23 +294,25 @@ export function useTimelineEditing({ // script (timing-only resize) — same no-flash path as move; full reload is // the fallback. const coalesceKey = `timeline-resize:${element.hfId ?? element.id}`; + const finishResizeGsapSync = () => + finishClipTimingFallback({ + iframe: previewIframeRef.current, + reloadPreview, + projectId: projectIdRef.current, + targetPath, + domId: element.domId, + label: "Resize timeline clip", + coalesceKey, + recordEdit, + edit: { + kind: "scale", + from: { start: element.start, duration: element.duration }, + to: { start: updates.start, duration: updates.duration }, + }, + }).finally(() => invalidateGsapCache?.()); const resizeFallback = () => - enqueueEdit(element, "Resize timeline clip", buildResizePatches, coalesceKey).then(() => - finishClipTimingFallback({ - iframe: previewIframeRef.current, - reloadPreview, - projectId: projectIdRef.current, - targetPath, - domId: element.domId, - label: "Resize timeline clip", - coalesceKey, - recordEdit, - edit: { - kind: "scale", - from: { start: element.start, duration: element.duration }, - to: { start: updates.start, duration: updates.duration }, - }, - }), + enqueueEdit(element, "Resize timeline clip", buildResizePatches, coalesceKey).then( + finishResizeGsapSync, ); const persistDone = sdkSession && element.hfId && !hasPbsAdjustment && !needsExtension @@ -323,9 +332,10 @@ export function useTimelineEditing({ readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path), publishSession: publishSdkSession, }, - { label: "Resize timeline clip", coalesceKey }, + { label: "Resize timeline clip", coalesceKey, skipRefresh: true }, ).then((result) => { if (!cutoverCommittedOrThrow(result)) return resizeFallback(); + return finishResizeGsapSync(); }) : resizeFallback(); return persistDone.catch((error) => { @@ -346,6 +356,7 @@ export function useTimelineEditing({ reloadPreview, domEditSaveTimestampRef, showToast, + invalidateGsapCache, ], ); diff --git a/packages/studio/src/hooks/useTimelineEditingTypes.ts b/packages/studio/src/hooks/useTimelineEditingTypes.ts index a391177064..bd3df209d8 100644 --- a/packages/studio/src/hooks/useTimelineEditingTypes.ts +++ b/packages/studio/src/hooks/useTimelineEditingTypes.ts @@ -46,6 +46,8 @@ export interface UseTimelineEditingOptions { publishSdkSession?: PublishSdkSession; /** Resync the SDK session after a server-authoritative timeline write. */ forceReloadSdkSession?: () => void; + /** Reparse authored animations after a timing rewrite changes their positions. */ + invalidateGsapCache?: () => void; handleDomZIndexReorderCommitRef?: MutableRefObject; } diff --git a/packages/studio/src/hooks/useTimelineGroupEditing.ts b/packages/studio/src/hooks/useTimelineGroupEditing.ts index 8a3fe83a81..2b6ac46e70 100644 --- a/packages/studio/src/hooks/useTimelineGroupEditing.ts +++ b/packages/studio/src/hooks/useTimelineGroupEditing.ts @@ -54,6 +54,7 @@ interface UseTimelineGroupEditingOptions { domEditSaveTimestampRef: MutableRefObject; editQueueRef: MutableRefObject>; forceReloadSdkSession?: () => void; + invalidateGsapCache?: () => void; isRecordingRef?: RefObject; pendingTimelineEditPathRef: MutableRefObject>; previewIframeRef: RefObject; @@ -110,6 +111,7 @@ export function useTimelineGroupEditing({ domEditSaveTimestampRef, editQueueRef, forceReloadSdkSession, + invalidateGsapCache, isRecordingRef, pendingTimelineEditPathRef, previewIframeRef, @@ -212,7 +214,12 @@ export function useTimelineGroupEditing({ readProjectFile: (path) => readFileContent(projectIdRef.current ?? "", path), publishSession: publishSdkSession, }, - { label: input.label, coalesceKey: input.coalesceKey, coalesceMs: input.coalesceMs }, + { + label: input.label, + coalesceKey: input.coalesceKey, + coalesceMs: input.coalesceMs, + skipRefresh: true, + }, ); return cutoverCommittedOrThrow(result); }, @@ -282,25 +289,25 @@ export function useTimelineGroupEditing({ coalesceKey, coalesceMs, }); - if (handledBySdk) return; - - await persistServerBatch( - projectId, - "Move timeline clips", - changes.map((change) => ({ - element: change.element, - buildPatches: (original, target) => - buildTimelineMoveTimingPatch( - original, - target, - change.start, - change.element.duration, - change.track, - ), - })), - coalesceKey, - coalesceMs, - ); + if (!handledBySdk) { + await persistServerBatch( + projectId, + "Move timeline clips", + changes.map((change) => ({ + element: change.element, + buildPatches: (original, target) => + buildTimelineMoveTimingPatch( + original, + target, + change.start, + change.element.duration, + change.track, + ), + })), + coalesceKey, + coalesceMs, + ); + } // Track-only: no timing delta → no GSAP positions to shift and no // reload (see the trackOnly doc above). Mixed batches (any start // change) keep the full fallback below. @@ -323,6 +330,7 @@ export function useTimelineGroupEditing({ return shiftGsapPositions(projectId, changePath, domId, delta); }, }); + invalidateGsapCache?.(); }).catch((error) => { // Failed persist: revert the optimistic duration readout + live root // alongside the gesture owner's store rollback. @@ -340,6 +348,7 @@ export function useTimelineGroupEditing({ reloadPreview, trySdkBatchPersist, showToast, + invalidateGsapCache, ], ); @@ -384,23 +393,23 @@ export function useTimelineGroupEditing({ coalesceKey, coalesceMs, }); - if (handledBySdk) return; - - await persistServerBatch( - projectId, - "Resize timeline clips", - changes.map((change) => ({ - element: change.element, - buildPatches: (original, target) => - buildTimelineResizeTimingPatch(original, target, change.element, { - start: change.start, - duration: change.duration, - playbackStart: change.playbackStart, - }), - })), - coalesceKey, - coalesceMs, - ); + if (!handledBySdk) { + await persistServerBatch( + projectId, + "Resize timeline clips", + changes.map((change) => ({ + element: change.element, + buildPatches: (original, target) => + buildTimelineResizeTimingPatch(original, target, change.element, { + start: change.start, + duration: change.duration, + playbackStart: change.playbackStart, + }), + })), + coalesceKey, + coalesceMs, + ); + } await finishGroupTimingGsapFallback({ projectId, iframe: previewIframeRef.current, @@ -428,6 +437,7 @@ export function useTimelineGroupEditing({ ); }, }); + invalidateGsapCache?.(); }).catch((error) => { // Failed persist: revert the optimistic duration readout + live root // alongside the gesture owner's store rollback. @@ -445,6 +455,7 @@ export function useTimelineGroupEditing({ reloadPreview, trySdkBatchPersist, showToast, + invalidateGsapCache, ], ); diff --git a/packages/studio/src/player/components/timelineClipDragPreview.test.ts b/packages/studio/src/player/components/timelineClipDragPreview.test.ts index 5f9bae4ef8..c71e9e73f6 100644 --- a/packages/studio/src/player/components/timelineClipDragPreview.test.ts +++ b/packages/studio/src/player/components/timelineClipDragPreview.test.ts @@ -6,7 +6,7 @@ import { type DragPreviewContext, } from "./timelineClipDragPreview"; import type { DraggedClipState } from "./timelineClipDragTypes"; -import { RULER_H, TRACKS_TOP_PAD, TRACK_H } from "./timelineLayout"; +import { LANE_H, RULER_H, TRACKS_TOP_PAD, TRACK_H } from "./timelineLayout"; // ───────────────────────────────────────────────────────────────────────────── // Regression bed for the live-reproduced BUG 1: a PLAIN HORIZONTAL drag of a clip @@ -55,13 +55,17 @@ function fakeScroll(): HTMLDivElement { } as unknown as HTMLDivElement; } -function ctx(): DragPreviewContext { +function ctx( + rowHeights?: readonly number[], + elements: TimelineElement[] = fixtureElements, +): DragPreviewContext { return { scroll: fakeScroll(), pps: PPS, duration: 44.5, trackOrder: [0, 1, 2], - elements: fixtureElements, + elements, + rowHeights, selectedKeys: new Set(), buildSnapTargets: () => [], audioTracks: new Set(), @@ -145,6 +149,49 @@ describe("computeDragPreview — plain horizontal drag never arms a phantom inse const next = computeDragPreview(drag, originClientX, yForRow(-0.6), ctx()); expect(next.insertRow).toBe(0); // a new TOP track will be created on drop }); + + it("keeps a horizontal drag in the body of an expanded row out of insert mode", () => { + const rowHeights = [TRACK_H + 2 * LANE_H, TRACK_H, TRACK_H]; + const clientY = RULER_H + TRACKS_TOP_PAD + rowHeights[0] - 8; + const { drag, clientX } = horizontalDrag(moodboard, 0.5, 2); + const next = computeDragPreview( + { ...drag, originClientY: clientY, pointerClientY: clientY }, + clientX, + clientY, + ctx(rowHeights), + ); + expect(next.insertRow).toBeNull(); + expect(next.previewTrack).toBe(0); + }); + + it("uses the expanded row midpoint when choosing the side for an automatic insert", () => { + const rowHeights = [TRACK_H + 2 * LANE_H, TRACK_H]; + const dragged = clip("dragged", 0, 0, 1, 3); + const occupied = [dragged, clip("block-0", 0, 0, 1, 2), clip("block-1", 1, 0, 1, 1)]; + const clientY = RULER_H + TRACKS_TOP_PAD + 30; + const drag: DraggedClipState = { + element: dragged, + originClientX: 0, + originClientY: clientY, + originScrollLeft: 0, + originScrollTop: 0, + pointerClientX: 0, + pointerClientY: clientY, + pointerOffsetX: 0, + pointerOffsetY: 0, + previewStart: 0, + previewTrack: 0, + insertRow: null, + snapTime: null, + snapType: null, + started: true, + }; + const next = computeDragPreview(drag, 0, clientY, { + ...ctx(rowHeights, occupied), + trackOrder: [0, 1], + }); + expect(next.insertRow).toBe(0); + }); }); describe("computeResizePreview — composition source continuity", () => { diff --git a/packages/studio/src/player/components/timelineClipDragPreview.ts b/packages/studio/src/player/components/timelineClipDragPreview.ts index 0e2d92b551..e5c054c035 100644 --- a/packages/studio/src/player/components/timelineClipDragPreview.ts +++ b/packages/studio/src/player/components/timelineClipDragPreview.ts @@ -1,6 +1,11 @@ import { resolveTimelineMove, resolveTimelineResize } from "./timelineEditing"; import type { TimelineElement } from "../store/playerStore"; -import { TRACK_H, getTimelineRowFromY, INSERT_BOUNDARY_BAND } from "./timelineLayout"; +import { + getTimelineInsertBoundaryBand, + getTimelineRowFromY, + getTimelineRowHeight, + getTimelineRowPositionFromY, +} from "./timelineLayout"; import { isMusicTrack, isAudioTimelineElement } from "../../utils/timelineInspector"; import { TIMELINE_SNAP_PX, @@ -27,6 +32,7 @@ export interface DragPreviewContext { pps: number; duration: number; trackOrder: number[]; + rowHeights?: readonly number[]; elements: TimelineElement[]; selectedKeys: ReadonlySet; buildSnapTargets: BuildSnapTargets; @@ -81,20 +87,26 @@ function resolveDropPlacement( desiredTrack: number, ctx: DragPreviewContext, ): { track: number; insertRow: number | null } { - const { scroll, trackOrder, elements } = ctx; + const { scroll, trackOrder, rowHeights, elements } = ctx; // rowFloat = the pointer's position in track-heights from the top lane; a // near-boundary hover requests a deliberate new-track insert. Uses the // shared row→y inverse so the top breathing pad is subtracted consistently. - const rowFloat = scroll - ? getTimelineRowFromY(clientY - scroll.getBoundingClientRect().top + scroll.scrollTop) - : 0; - // Geometry-exact band (the clip inset) so an insert only arms in the visible - // gutter BETWEEN clip bodies — dragging over a clip body is a lane move, never a - // phantom insert (the plain-horizontal-drag misfire). See INSERT_BOUNDARY_BAND. - const rawInsertRow = resolveInsertRow(rowFloat, trackOrder.length, INSERT_BOUNDARY_BAND); + const rowPosition = scroll + ? getTimelineRowPositionFromY( + clientY - scroll.getBoundingClientRect().top + scroll.scrollTop, + rowHeights, + ) + : { rowFloat: 0, row: 0, fraction: 0, rowHeight: getTimelineRowHeight(0, rowHeights) }; + // Geometry-exact band (the clip inset divided by this row's actual height) so + // an insert only arms in the visible gutter between clip bodies. + const rawInsertRow = resolveInsertRow( + rowPosition.rowFloat, + trackOrder.length, + getTimelineInsertBoundaryBand(rowPosition.rowHeight), + ); // Pointer sub-row half: when a drop must auto-create a track (aimed span // occupied, no free lane), open it on the side the pointer is nearer. - const preferInsertAbove = rowFloat - Math.floor(rowFloat) < 0.5; + const preferInsertAbove = rowPosition.fraction < 0.5; const audioTracks = ctx.audioTracks ?? new Set(elements.filter(isAudioTimelineElement).map((e) => e.track)); return resolveZoneDropPlacement({ @@ -120,24 +132,34 @@ export function computeDragPreview( ): DraggedClipState { const { scroll, pps, duration, trackOrder, elements, selectedKeys, buildSnapTargets } = ctx; const dragMaxStart = resolveDragMaxStart(scroll, pps, duration); + const scrollTop = scroll?.scrollTop ?? drag.originScrollTop; + const scrollRectTop = scroll?.getBoundingClientRect().top ?? 0; + const originRow = getTimelineRowFromY( + drag.originClientY - scrollRectTop + drag.originScrollTop, + ctx.rowHeights, + ); + const currentRow = getTimelineRowFromY(clientY - scrollRectTop + scrollTop, ctx.rowHeights); + // resolveTimelineMove's vertical axis is expressed in track-height units. + // Feeding cumulative row coordinates with a unit height preserves its existing + // threshold/create-track behavior while supporting variable pixel heights. const nextMove = resolveTimelineMove( { start: drag.element.start, track: drag.element.track, duration: drag.element.duration, originClientX: drag.originClientX, - originClientY: drag.originClientY, + originClientY: originRow, originScrollLeft: drag.originScrollLeft, - originScrollTop: drag.originScrollTop, + originScrollTop: 0, currentScrollLeft: scroll?.scrollLeft ?? drag.originScrollLeft, - currentScrollTop: scroll?.scrollTop ?? drag.originScrollTop, + currentScrollTop: 0, pixelsPerSecond: pps, - trackHeight: TRACK_H, + trackHeight: 1, maxStart: dragMaxStart, trackOrder, }, clientX, - clientY, + currentRow, ); // The music track defines the beats, so it must not snap to them — // but it still snaps to the playhead and other clip edges. diff --git a/packages/studio/src/player/components/timelineCollision.ts b/packages/studio/src/player/components/timelineCollision.ts index 1c9c6c5a59..c4f4c728e4 100644 --- a/packages/studio/src/player/components/timelineCollision.ts +++ b/packages/studio/src/player/components/timelineCollision.ts @@ -1,4 +1,5 @@ import type { TimelineElement } from "../store/playerStore"; +import { INSERT_BOUNDARY_BAND } from "./timelineLayout"; /** * Keep a landing track inside the dragged clip's kind-zone: visual clips stay in @@ -139,28 +140,18 @@ export function resolveZoneDropPlacement(input: { return { track: placement.track, insertRow: null }; } -/** - * Fallback half-width (fraction of a track height) of the insert band straddling - * a lane boundary — used only when the caller passes no explicit band. Production - * threads the geometry-exact `INSERT_BOUNDARY_BAND` (timelineLayout.ts, = the clip - * inset `CLIP_Y / TRACK_H`) so the band matches the rendered inter-clip gutter and - * NEVER reaches into a clip body. Kept in sync with that constant; do not widen it - * back toward the old 0.32 (which armed an insert across ~64% of every row — the - * misfire that turned a plain horizontal drag into a phantom track insert). - */ -const INSERT_BAND = 3 / 48; - /** * Decide whether a vertical drag is inserting a new track at a lane boundary. * `rowFloat` is the pointer's position in track-height units from the top of the * first lane (0 = top of lane 0). Returns the boundary row to insert at * (0 = above the top lane, `trackCount` = below the bottom), or null when the - * pointer is over a lane's middle band (a normal move/target). + * pointer is over a lane's middle band (a normal move/target). The default band + * preserves collapsed-row behavior; production passes the concrete row's band. */ export function resolveInsertRow( rowFloat: number, trackCount: number, - band: number = INSERT_BAND, + band: number = INSERT_BOUNDARY_BAND, ): number | null { if (trackCount === 0) return 0; if (rowFloat <= 0) return 0; diff --git a/packages/studio/src/player/components/timelineLayout.test.ts b/packages/studio/src/player/components/timelineLayout.test.ts index dde3a5cfc8..c21618faaf 100644 --- a/packages/studio/src/player/components/timelineLayout.test.ts +++ b/packages/studio/src/player/components/timelineLayout.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from "vitest"; import { RULER_H, TRACK_H, + LANE_H, TRACKS_TOP_PAD, TRACKS_BOTTOM_PAD, GUTTER, @@ -9,10 +10,84 @@ import { getTimelineRowTop, getTimelineScrubTime, getTimelineRowFromY, + getTimelineRowOffsets, getTimelineCanvasHeight, + trackHeights, resolveTimelineAssetDrop, } from "./timelineLayout"; +describe("variable timeline row geometry", () => { + const tracks = [ + [{ clipId: "a", laneCount: 0 }], + [{ clipId: "b", laneCount: 2 }], + [{ clipId: "c", laneCount: 1 }], + ]; + + it("resolves every row to the base height when no clip is expanded", () => { + expect(trackHeights(tracks)).toEqual([TRACK_H, TRACK_H, TRACK_H]); + expect(trackHeights(3)).toEqual([TRACK_H, TRACK_H, TRACK_H]); + }); + + it("adds one lane height per lane on an expanded clip", () => { + expect(trackHeights(tracks, new Set(["b"]))).toEqual([TRACK_H, TRACK_H + 2 * LANE_H, TRACK_H]); + }); + + it("derives row tops from cumulative offsets", () => { + const heights = trackHeights(tracks, new Set(["b"])); + expect(getTimelineRowOffsets(heights)).toEqual([ + 0, + TRACK_H, + 2 * TRACK_H + 2 * LANE_H, + 3 * TRACK_H + 2 * LANE_H, + ]); + expect(getTimelineRowTop(2, heights)).toBe(RULER_H + TRACKS_TOP_PAD + 2 * TRACK_H + 2 * LANE_H); + }); + + it("maps y inside an expanded lane region back to the expanded track", () => { + const heights = trackHeights(tracks, new Set(["b"])); + const yInSecondExpandedLane = getTimelineRowTop(1, heights) + TRACK_H + LANE_H * 1.5; + const row = getTimelineRowFromY(yInSecondExpandedLane, heights); + expect(Math.floor(row)).toBe(1); + expect(row).toBeGreaterThan(1.5); + expect(row).toBeLessThan(2); + }); + + it("sums resolved row heights into the canvas height", () => { + const heights = trackHeights(tracks, new Set(["b"])); + expect(getTimelineCanvasHeight(heights)).toBe( + RULER_H + TRACKS_TOP_PAD + 3 * TRACK_H + 2 * LANE_H + TRACKS_BOTTOM_PAD, + ); + }); +}); + +describe("collapsed timeline row geometry characterization", () => { + it.each([ + [0, 74], + [1, 122], + [4, 266], + ])("keeps row %i at content y=%i", (row, expectedTop) => { + expect(getTimelineRowTop(row)).toBe(expectedTop); + }); + + it.each([ + [74, 0], + [86, 0.25], + [146, 1.5], + [290, 4.5], + ])("maps content y=%i to fractional row %f", (contentY, expectedRow) => { + expect(getTimelineRowFromY(contentY)).toBe(expectedRow); + }); + + it.each([ + [0, 146], + [1, 194], + [3, 290], + [5, 386], + ])("keeps the %i-track canvas height at %i", (trackCount, expectedHeight) => { + expect(getTimelineCanvasHeight(trackCount)).toBe(expectedHeight); + }); +}); + describe("track-area breathing pad y-math", () => { describe("getTimelineRowTop", () => { it("offsets the first lane below the ruler by the top pad", () => { diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index 9a1b51108e..615df7d670 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -4,25 +4,20 @@ import type { ZoomMode } from "../store/playerStore"; /* ── Layout constants ──────────────────────────────────────────────── */ export const GUTTER = 32; export const TRACK_H = 48; +export const LANE_H = 28; export const RULER_H = 24; export const CLIP_Y = 3; export const CLIP_HANDLE_W = 18; + /** - * Half-width (as a fraction of TRACK_H) of the new-track INSERT band that - * straddles each lane boundary. Deliberately equals the clip's vertical inset - * (`CLIP_Y / TRACK_H`): a clip body fills [CLIP_Y, TRACK_H − CLIP_Y] of its row, - * so the ONLY region this band covers is the visible empty gutter between two - * clip bodies (plus the top/bottom breathing pads, handled separately by the - * rowFloat ≤ 0 / ≥ trackCount extremes). Aiming at a clip body is therefore a - * move-to-that-lane; only the inter-clip gap arms an insert — see resolveInsertRow. - * Threaded into resolveInsertRow by the drag preview so the hit band can never - * drift from the rendered clip geometry. + * Collapsed-row characterization value for the new-track INSERT band. Runtime + * hit-testing uses getTimelineInsertBoundaryBand with the concrete row height. */ export const INSERT_BOUNDARY_BAND = CLIP_Y / TRACK_H; /** * Breathing room INSIDE the scroll area (CapCut-style), threaded through every * track-row y computation via {@link getTimelineRowTop} — never inline a magic - * offset; a track row's top is always `RULER_H + TRACKS_TOP_PAD + row*TRACK_H`. + * offset; a track row's top is always ruler + top pad + cumulative row heights. * * - TRACKS_TOP_PAD: empty space between the (sticky) ruler and the first track * (~half a track height) so the first clip isn't jammed under the ruler. @@ -50,17 +45,108 @@ export const TRACKS_LEFT_PAD = 48; * placeholder/insertion top and every pointer-y→row inversion goes through this * (or its inverse in {@link getTimelineRowFromY}) so the pad can never drift. */ -export function getTimelineRowTop(row: number): number { - return RULER_H + TRACKS_TOP_PAD + row * TRACK_H; +interface TimelineTrackHeightClip { + clipId: string; + laneCount: number; +} + +type TimelineTrackHeightInput = readonly (readonly TimelineTrackHeightClip[])[]; + +/** + * Resolve each track's full height. Without expansion state every row is the + * legacy TRACK_H; if multiple clips in one track expand, the tallest one owns + * the shared row height. + */ +export function trackHeights( + tracks: number | TimelineTrackHeightInput, + expandedClipIds?: ReadonlySet, +): number[] { + if (typeof tracks === "number") { + return Array.from({ length: Math.max(0, Math.trunc(tracks)) }, () => TRACK_H); + } + return tracks.map((clips) => { + let laneCount = 0; + if (expandedClipIds) { + for (const clip of clips) { + if (expandedClipIds.has(clip.clipId)) laneCount = Math.max(laneCount, clip.laneCount); + } + } + return TRACK_H + Math.max(0, Math.trunc(laneCount)) * LANE_H; + }); +} + +function validRowHeight(height: number | undefined): number { + if (height === undefined || !Number.isFinite(height) || height <= 0) return TRACK_H; + return height; +} + +/** Cumulative top offsets, including the final bottom boundary. */ +export function getTimelineRowOffsets(rowHeights: readonly number[]): number[] { + const offsets = [0]; + for (const height of rowHeights) { + offsets.push((offsets[offsets.length - 1] ?? 0) + validRowHeight(height)); + } + return offsets; +} + +export function getTimelineRowHeight(row: number, rowHeights: readonly number[] = []): number { + return validRowHeight(rowHeights[row]); +} + +function getTimelineRowOffset(row: number, rowHeights: readonly number[]): number { + if (rowHeights.length === 0) return row * TRACK_H; + const offsets = getTimelineRowOffsets(rowHeights); + if (row <= 0) return row * getTimelineRowHeight(0, rowHeights); + if (row >= rowHeights.length) { + return (offsets[rowHeights.length] ?? 0) + (row - rowHeights.length) * TRACK_H; + } + const wholeRow = Math.floor(row); + const fraction = row - wholeRow; + return (offsets[wholeRow] ?? 0) + fraction * getTimelineRowHeight(wholeRow, rowHeights); +} + +export function getTimelineRowTop(row: number, rowHeights: readonly number[] = []): number { + return RULER_H + TRACKS_TOP_PAD + getTimelineRowOffset(row, rowHeights); } /** * Inverse of {@link getTimelineRowTop}: the fractional row index for a content- - * space y (used for insert-row / drop-lane decisions). Subtracts the ruler and - * top pad before dividing by the track height. + * space y (used for insert-row / drop-lane decisions). Locates the concrete row + * from cumulative offsets, then returns its local fractional position. */ -export function getTimelineRowFromY(contentY: number): number { - return (contentY - RULER_H - TRACKS_TOP_PAD) / TRACK_H; +export function getTimelineRowFromY(contentY: number, rowHeights: readonly number[] = []): number { + const y = contentY - RULER_H - TRACKS_TOP_PAD; + if (rowHeights.length === 0) return y / TRACK_H; + if (y < 0) return y / getTimelineRowHeight(0, rowHeights); + + const offsets = getTimelineRowOffsets(rowHeights); + for (let row = 0; row < rowHeights.length; row += 1) { + const bottom = offsets[row + 1] ?? 0; + if (y < bottom) { + const top = offsets[row] ?? 0; + return row + (y - top) / getTimelineRowHeight(row, rowHeights); + } + } + return rowHeights.length + (y - (offsets[rowHeights.length] ?? 0)) / TRACK_H; +} + +export function getTimelineRowPositionFromY( + contentY: number, + rowHeights: readonly number[] = [], +): { rowFloat: number; row: number; fraction: number; rowHeight: number } { + const rowFloat = getTimelineRowFromY(contentY, rowHeights); + const row = Math.floor(rowFloat); + return { + rowFloat, + row, + fraction: rowFloat - row, + rowHeight: getTimelineRowHeight(row, rowHeights), + }; +} + +/** Fractional insert band for the concrete row under a pointer. */ +export function getTimelineInsertBoundaryBand(rowHeight: number): number { + return CLIP_Y / validRowHeight(rowHeight); } /** * While a clip drag is live, the rendered timeline extends this far past the @@ -344,11 +430,16 @@ export function getTimelineScrubTime(input: { return Math.max(0, Math.min(duration, x / pixelsPerSecond)); } -export function getTimelineCanvasHeight(trackCount: number): number { +export function getTimelineCanvasHeight(trackCountOrHeights: number | readonly 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 // below the last lane is real scrollable surface, not a hidden buffer. - return RULER_H + TRACKS_TOP_PAD + Math.max(0, trackCount) * TRACK_H + TRACKS_BOTTOM_PAD; + const heights = + typeof trackCountOrHeights === "number" + ? trackHeights(trackCountOrHeights) + : trackCountOrHeights; + const rowsHeight = getTimelineRowOffsets(heights).at(-1) ?? 0; + return RULER_H + TRACKS_TOP_PAD + rowsHeight + TRACKS_BOTTOM_PAD; } /* ── UI helpers ───────────────────────────────────────────────────── */ diff --git a/packages/studio/src/player/components/timelineMarquee.test.ts b/packages/studio/src/player/components/timelineMarquee.test.ts index 732076edb9..7c6673b552 100644 --- a/packages/studio/src/player/components/timelineMarquee.test.ts +++ b/packages/studio/src/player/components/timelineMarquee.test.ts @@ -9,6 +9,7 @@ import { } from "./timelineMarquee"; import { GUTTER, + LANE_H, TRACK_H, RULER_H, CLIP_Y, @@ -16,7 +17,9 @@ import { getTimelineRowTop, } from "./timelineLayout"; -// Canvas-space time origin: right edge of the sticky gutter + the left pad. +// Canvas-space time origin used by the breathing-pad (default) test cases: right +// edge of the sticky gutter + the left pad. Other cases pass GUTTER or LABEL_COL_W +// directly as contentOrigin to test the plain/keyframe-label-column origins. const ORIGIN = GUTTER + TRACKS_LEFT_PAD; describe("isTimelineRulerPress", () => { @@ -94,9 +97,9 @@ describe("getTimelineClipRect", () => { const trackOrder = [0, 2, 5]; it("maps start/duration to x via pps and the track row to y via the shared row→y helper", () => { - const rect = getTimelineClipRect({ start: 2, duration: 3, track: 2 }, trackOrder, 100); + const rect = getTimelineClipRect({ start: 2, duration: 3, track: 2 }, trackOrder, 100, GUTTER); expect(rect).toEqual({ - left: ORIGIN + 200, + left: GUTTER + 200, top: getTimelineRowTop(1) + CLIP_Y, width: 300, height: TRACK_H - CLIP_Y * 2, @@ -104,25 +107,55 @@ describe("getTimelineClipRect", () => { }); it("places the first visible track below the ruler + top breathing pad", () => { - const rect = getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 50); + const rect = getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 50, GUTTER); expect(rect?.top).toBe(getTimelineRowTop(0) + CLIP_Y); - expect(rect?.left).toBe(ORIGIN); + expect(rect?.left).toBe(GUTTER); }); it("uses the row index in trackOrder, not the raw track number", () => { - const rect = getTimelineClipRect({ start: 0, duration: 1, track: 5 }, trackOrder, 50); + const rect = getTimelineClipRect({ start: 0, duration: 1, track: 5 }, trackOrder, 50, GUTTER); expect(rect?.top).toBe(getTimelineRowTop(2) + CLIP_Y); }); + it("uses cumulative tops and the resolved height for an expanded row", () => { + const rowHeights = [TRACK_H + 2 * LANE_H, TRACK_H, TRACK_H]; + const rect = getTimelineClipRect( + { start: 0, duration: 1, track: 0 }, + trackOrder, + 50, + GUTTER, + rowHeights, + ); + expect(rect).toMatchObject({ + top: getTimelineRowTop(0, rowHeights) + CLIP_Y, + height: rowHeights[0] - CLIP_Y * 2, + }); + expect( + getTimelineClipRect({ start: 0, duration: 1, track: 2 }, trackOrder, 50, GUTTER, rowHeights) + ?.top, + ).toBe(getTimelineRowTop(1, rowHeights) + CLIP_Y); + }); + it("enforces the 4px minimum rendered width", () => { - const rect = getTimelineClipRect({ start: 0, duration: 0.01, track: 0 }, trackOrder, 10); + const rect = getTimelineClipRect( + { start: 0, duration: 0.01, track: 0 }, + trackOrder, + 10, + GUTTER, + ); expect(rect?.width).toBe(4); }); it("returns null for a track that is not displayed or an invalid pps", () => { - expect(getTimelineClipRect({ start: 0, duration: 1, track: 9 }, trackOrder, 100)).toBeNull(); - expect(getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 0)).toBeNull(); - expect(getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, NaN)).toBeNull(); + expect( + getTimelineClipRect({ start: 0, duration: 1, track: 9 }, trackOrder, 100, GUTTER), + ).toBeNull(); + expect( + getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, 0, GUTTER), + ).toBeNull(); + expect( + getTimelineClipRect({ start: 0, duration: 1, track: 0 }, trackOrder, NaN, GUTTER), + ).toBeNull(); }); }); @@ -140,29 +173,48 @@ describe("computeMarqueeSelection", () => { it("selects only the clips the marquee rect intersects", () => { const marquee = { left: ORIGIN, top: row0Top, width: 50, height: 10 }; - const { ids, primaryId } = computeMarqueeSelection({ clips, trackOrder, pps, marquee }); + const { ids, primaryId } = computeMarqueeSelection({ + clips, + trackOrder, + pps, + contentOrigin: ORIGIN, + marquee, + }); expect(ids).toEqual(new Set(["a"])); expect(primaryId).toBe("a"); }); it("selects across tracks when the rect spans multiple rows", () => { const marquee = { left: ORIGIN, top: row0Top, width: 60, height: row1Top - row0Top + 5 }; - const { ids } = computeMarqueeSelection({ clips, trackOrder, pps, marquee }); + const { ids } = computeMarqueeSelection({ + clips, + trackOrder, + pps, + contentOrigin: ORIGIN, + marquee, + }); expect(ids).toEqual(new Set(["a", "c"])); }); it("excludes clips outside the rect horizontally", () => { const marquee = { left: ORIGIN + 140, top: row0Top, width: 50, height: 10 }; - const { ids } = computeMarqueeSelection({ clips, trackOrder, pps, marquee }); + const { ids } = computeMarqueeSelection({ + clips, + trackOrder, + pps, + contentOrigin: ORIGIN, + marquee, + }); expect(ids).toEqual(new Set()); }); it("returns null primaryId and keeps the base when nothing is hit (additive)", () => { - const marquee = { left: ORIGIN + 140, top: row0Top, width: 50, height: 10 }; + const marquee = { left: GUTTER + 140, top: row0Top, width: 50, height: 10 }; const { ids, primaryId } = computeMarqueeSelection({ clips, trackOrder, pps, + contentOrigin: GUTTER, marquee, baseSelection: ["b"], }); @@ -171,11 +223,12 @@ describe("computeMarqueeSelection", () => { }); it("unions additive base selection with new hits; primary comes from the marquee", () => { - const marquee = { left: ORIGIN, top: row1Top, width: 100, height: 10 }; + const marquee = { left: GUTTER, top: row1Top, width: 100, height: 10 }; const { ids, primaryId } = computeMarqueeSelection({ clips, trackOrder, pps, + contentOrigin: GUTTER, marquee, baseSelection: ["b"], }); @@ -186,12 +239,13 @@ describe("computeMarqueeSelection", () => { it("shrinking the rect live drops clips it no longer covers", () => { const wide = { left: ORIGIN, top: row0Top, width: 320, height: 10 }; const narrow = { left: ORIGIN, top: row0Top, width: 80, height: 10 }; - expect(computeMarqueeSelection({ clips, trackOrder, pps, marquee: wide }).ids).toEqual( - new Set(["a", "b"]), - ); - expect(computeMarqueeSelection({ clips, trackOrder, pps, marquee: narrow }).ids).toEqual( - new Set(["a"]), - ); + expect( + computeMarqueeSelection({ clips, trackOrder, pps, contentOrigin: ORIGIN, marquee: wide }).ids, + ).toEqual(new Set(["a", "b"])); + expect( + computeMarqueeSelection({ clips, trackOrder, pps, contentOrigin: ORIGIN, marquee: narrow }) + .ids, + ).toEqual(new Set(["a"])); }); it("ignores clips on hidden/undisplayed tracks", () => { @@ -200,6 +254,7 @@ describe("computeMarqueeSelection", () => { clips: [{ id: "x", start: 0, duration: 1, track: 7 }], trackOrder, pps, + contentOrigin: GUTTER, marquee, }); expect(ids).toEqual(new Set()); diff --git a/packages/studio/src/player/components/timelineMarquee.ts b/packages/studio/src/player/components/timelineMarquee.ts index 21c18913d1..9ddc02d9f3 100644 --- a/packages/studio/src/player/components/timelineMarquee.ts +++ b/packages/studio/src/player/components/timelineMarquee.ts @@ -1,9 +1,9 @@ import { GUTTER, - TRACK_H, RULER_H, CLIP_Y, TRACKS_LEFT_PAD, + getTimelineRowHeight, getTimelineRowTop, } from "./timelineLayout"; import { rectsOverlap, type Rect } from "../../utils/marqueeGeometry"; @@ -68,22 +68,24 @@ export function getMarqueeRect( /** * A clip's rendered rect in canvas/content coordinates (the same space the - * marquee rect lives in): x from GUTTER + start * pps, y from the clip's row - * index within the visible track order (RULER_H + row * TRACK_H + CLIP_Y). + * marquee rect lives in): x from the shared content origin + start * pps, y from the clip's row + * index within the visible track order (cumulative row top + CLIP_Y). * Returns null when the clip's track is not currently displayed. */ export function getTimelineClipRect( clip: Pick, trackOrder: number[], pps: number, + contentOrigin: number = GUTTER + TRACKS_LEFT_PAD, + rowHeights: readonly number[] = [], ): Rect | null { const row = trackOrder.indexOf(clip.track); if (row < 0 || !Number.isFinite(pps) || pps <= 0) return null; return { - left: GUTTER + TRACKS_LEFT_PAD + clip.start * pps, - top: getTimelineRowTop(row) + CLIP_Y, + left: contentOrigin + clip.start * pps, + top: getTimelineRowTop(row, rowHeights) + CLIP_Y, width: Math.max(clip.duration * pps, MIN_CLIP_W), - height: TRACK_H - CLIP_Y * 2, + height: getTimelineRowHeight(row, rowHeights) - CLIP_Y * 2, }; } @@ -103,13 +105,21 @@ export function computeMarqueeSelection(input: { clips: MarqueeClipInput[]; trackOrder: number[]; pps: number; + contentOrigin?: number; marquee: Rect; baseSelection?: Iterable; + rowHeights?: readonly number[]; }): MarqueeSelectionResult { const ids = new Set(input.baseSelection ?? []); let primaryId: string | null = null; for (const clip of input.clips) { - const rect = getTimelineClipRect(clip, input.trackOrder, input.pps); + const rect = getTimelineClipRect( + clip, + input.trackOrder, + input.pps, + input.contentOrigin, + input.rowHeights, + ); if (rect && rectsOverlap(rect, input.marquee)) { ids.add(clip.id); primaryId = clip.id; diff --git a/packages/studio/src/player/components/useTimelineClipDrag.ts b/packages/studio/src/player/components/useTimelineClipDrag.ts index 31050cccaa..90d4100813 100644 --- a/packages/studio/src/player/components/useTimelineClipDrag.ts +++ b/packages/studio/src/player/components/useTimelineClipDrag.ts @@ -48,6 +48,7 @@ interface UseTimelineClipDragInput { ppsRef: React.RefObject; durationRef: React.RefObject; trackOrderRef: React.RefObject; + rowHeightsRef?: React.RefObject; onMoveElement?: ( element: TimelineElement, updates: Pick, @@ -85,6 +86,7 @@ export function useTimelineClipDrag({ ppsRef, durationRef, trackOrderRef, + rowHeightsRef, onMoveElement, onMoveElements, onResizeElement, @@ -241,13 +243,14 @@ export function useTimelineClipDrag({ pps: ppsRef.current, duration: durationRef.current, trackOrder: trackOrderRef.current, + rowHeights: rowHeightsRef?.current, elements: elementsRef.current, selectedKeys: usePlayerStore.getState().selectedElementIds, buildSnapTargets, audioTracks: dragAudioTracksRef.current, }); }, - [scrollRef, ppsRef, durationRef, trackOrderRef, buildSnapTargets], + [scrollRef, ppsRef, durationRef, trackOrderRef, rowHeightsRef, buildSnapTargets], ); // Recompute the trim preview for a pointer x. Shared by the pointermove resize From 95213d73536dfa29eae931672a8c875a22f5ff3e Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 00:54:23 +0200 Subject: [PATCH 09/25] refactor(studio): give resolveTimelineMove a row-based vertical axis Rows stopped sharing one pixel height when lanes gained expansion, so the only production caller was passing cumulative row coordinates with trackHeight 1 and both scrollTops zeroed. The parameter names described units the values no longer carried. The vertical axis is now a row index and the caller keeps ownership of folding scroll and per-row heights into it. --- .../components/timelineClipDragPreview.ts | 10 ++-- .../player/components/timelineEditing.test.ts | 46 ++++++++----------- .../src/player/components/timelineEditing.ts | 15 +++--- 3 files changed, 29 insertions(+), 42 deletions(-) diff --git a/packages/studio/src/player/components/timelineClipDragPreview.ts b/packages/studio/src/player/components/timelineClipDragPreview.ts index e5c054c035..982f42fbff 100644 --- a/packages/studio/src/player/components/timelineClipDragPreview.ts +++ b/packages/studio/src/player/components/timelineClipDragPreview.ts @@ -139,22 +139,18 @@ export function computeDragPreview( ctx.rowHeights, ); const currentRow = getTimelineRowFromY(clientY - scrollRectTop + scrollTop, ctx.rowHeights); - // resolveTimelineMove's vertical axis is expressed in track-height units. - // Feeding cumulative row coordinates with a unit height preserves its existing - // threshold/create-track behavior while supporting variable pixel heights. + // resolveTimelineMove's vertical axis is row indices, which is why the pointer + // and scroll pixels are folded into originRow/currentRow above. const nextMove = resolveTimelineMove( { start: drag.element.start, track: drag.element.track, duration: drag.element.duration, originClientX: drag.originClientX, - originClientY: originRow, + originRow, originScrollLeft: drag.originScrollLeft, - originScrollTop: 0, currentScrollLeft: scroll?.scrollLeft ?? drag.originScrollLeft, - currentScrollTop: 0, pixelsPerSecond: pps, - trackHeight: 1, maxStart: dragMaxStart, trackOrder, }, diff --git a/packages/studio/src/player/components/timelineEditing.test.ts b/packages/studio/src/player/components/timelineEditing.test.ts index f2c2913ead..fc5aa95533 100644 --- a/packages/studio/src/player/components/timelineEditing.test.ts +++ b/packages/studio/src/player/components/timelineEditing.test.ts @@ -27,14 +27,13 @@ describe("resolveTimelineMove", () => { track: 2, duration: 2, originClientX: 100, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 8, trackOrder: [0, 1, 2, 3, 4], }, 245, - 200, + 0, ), ).toEqual({ start: 2.7, track: 2 }); }); @@ -47,14 +46,13 @@ describe("resolveTimelineMove", () => { track: 1, duration: 3, originClientX: 200, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 10, trackOrder: [0, 1, 5, 9], }, 150, - 390, + 190 / 72, ), ).toEqual({ start: 1.5, track: 9 }); }); @@ -67,14 +65,13 @@ describe("resolveTimelineMove", () => { track: 0, duration: 4, originClientX: 300, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 6, trackOrder: [0, 10, 20], }, -100, - -200, + -400 / 72, ), ).toEqual({ start: 0, track: -1 }); @@ -85,14 +82,13 @@ describe("resolveTimelineMove", () => { track: 10, duration: 4, originClientX: 300, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 6, trackOrder: [0, 10, 20], }, 500, - 200, + 0, ), ).toEqual({ start: 6, track: 10 }); }); @@ -105,14 +101,13 @@ describe("resolveTimelineMove", () => { track: 0, duration: 2, originClientX: 100, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 8, trackOrder: [0, 10, 20], }, 100, - 150, + -50 / 72, ), ).toEqual({ start: 1, track: -1 }); }); @@ -125,19 +120,18 @@ describe("resolveTimelineMove", () => { track: 20, duration: 2, originClientX: 100, - originClientY: 200, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 8, trackOrder: [0, 10, 20], }, 100, - 250, + 50 / 72, ), ).toEqual({ start: 1, track: 21 }); }); - it("accounts for scroll displacement while dragging", () => { + it("accounts for horizontal scroll displacement while dragging", () => { expect( resolveTimelineMove( { @@ -145,18 +139,17 @@ describe("resolveTimelineMove", () => { track: 0, duration: 2, originClientX: 100, - originClientY: 200, + originRow: 0, originScrollLeft: 0, - originScrollTop: 0, currentScrollLeft: 100, - currentScrollTop: 144, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 8, + // Vertical scroll never reaches here: the drag preview folds it into the + // row index it passes, because rows no longer share one pixel height. trackOrder: [0, 1, 2, 3], }, 100, - 200, + 2, ), ).toEqual({ start: 2, track: 2 }); }); @@ -195,9 +188,8 @@ describe("resolveTimelineMove", () => { track: 1, duration: 2, originClientX: 0, - originClientY: 0, + originRow: 0, pixelsPerSecond: 100, - trackHeight: 72, maxStart: 8, trackOrder: [0, 1], layerOrder: layers.map((layer) => layer.id), @@ -206,7 +198,7 @@ describe("resolveTimelineMove", () => { stackingElements, }, 0, - -72, + -1, ); expect(result).toEqual({ diff --git a/packages/studio/src/player/components/timelineEditing.ts b/packages/studio/src/player/components/timelineEditing.ts index 9b08a67ce6..f183807b08 100644 --- a/packages/studio/src/player/components/timelineEditing.ts +++ b/packages/studio/src/player/components/timelineEditing.ts @@ -46,13 +46,11 @@ export interface TimelineMoveInput { track: number; duration: number; originClientX: number; - originClientY: number; + /** Vertical position as a track-row index, not pixels: rows vary in height. */ + originRow: number; originScrollLeft?: number; - originScrollTop?: number; currentScrollLeft?: number; - currentScrollTop?: number; pixelsPerSecond: number; - trackHeight: number; maxStart: number; trackOrder: number[]; layerOrder?: TimelineLayerId[]; @@ -108,7 +106,7 @@ export function resolveTimelineAutoScroll( export function resolveTimelineMove( input: TimelineMoveInput, clientX: number, - clientY: number, + currentRow: number, ): { start: number; track: number; @@ -117,11 +115,12 @@ export function resolveTimelineMove( stackingReorder?: TimelineStackingReorderIntent | null; } { const scrollDeltaX = (input.currentScrollLeft ?? 0) - (input.originScrollLeft ?? 0); - const scrollDeltaY = (input.currentScrollTop ?? 0) - (input.originScrollTop ?? 0); const deltaTime = (clientX - input.originClientX + scrollDeltaX) / Math.max(input.pixelsPerSecond, 1); - const trackDeltaRaw = - (clientY - input.originClientY + scrollDeltaY) / Math.max(input.trackHeight, 1); + // Rows, so vertical scroll and per-row heights are the caller's problem: rows + // have varied in height since lanes expand, and a single trackHeight can't + // describe them. + const trackDeltaRaw = currentRow - input.originRow; const deltaTrack = Math.round(trackDeltaRaw); const nextStart = clamp( roundToCentiseconds(input.start + deltaTime), From 4c7703ff8f136b44db21e5e81061a79c959c2bd0 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 02:16:28 +0200 Subject: [PATCH 10/25] refactor(studio): drop the duplicated row-top docblock The getTimelineRowTop docblock had a second copy sitting on TimelineTrackHeightClip, where it describes nothing. Only the one on the function stays. --- .../studio/src/player/components/timelineLayout.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index 615df7d670..2677bf3a45 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -38,13 +38,6 @@ export const TRACKS_BOTTOM_PAD = Math.round(TRACK_H * 1.5); */ export const TRACKS_LEFT_PAD = 48; -/** - * The y (content-space) of the top edge of track ROW index `row` (0 = first - * displayed lane). The single source of truth for row→y — the ruler height plus - * the top breathing pad plus whole track lanes above it. Every clip/ghost/ - * placeholder/insertion top and every pointer-y→row inversion goes through this - * (or its inverse in {@link getTimelineRowFromY}) so the pad can never drift. - */ interface TimelineTrackHeightClip { clipId: string; laneCount: number; @@ -105,6 +98,13 @@ function getTimelineRowOffset(row: number, rowHeights: readonly number[]): numbe return (offsets[wholeRow] ?? 0) + fraction * getTimelineRowHeight(wholeRow, rowHeights); } +/** + * The y (content-space) of the top edge of track ROW index `row` (0 = first + * displayed lane). The single source of truth for row→y — the ruler height plus + * the top breathing pad plus whole track lanes above it. Every clip/ghost/ + * placeholder/insertion top and every pointer-y→row inversion goes through this + * (or its inverse in {@link getTimelineRowFromY}) so the pad can never drift. + */ export function getTimelineRowTop(row: number, rowHeights: readonly number[] = []): number { return RULER_H + TRACKS_TOP_PAD + getTimelineRowOffset(row, rowHeights); } From e36fb385bc71af52c16c7e6059305c4bc2da2b7a Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 16:46:58 +0200 Subject: [PATCH 11/25] feat(studio): add timeline keyframe retiming interactions --- .../components/editor/keyframeRetime.test.ts | 11 +- .../src/components/editor/keyframeRetime.ts | 3 +- .../nle/useTimelineEditCallbacks.ts | 9 +- .../components/KeyframeDiamondContextMenu.tsx | 34 +- .../components/TimelineClipDiamonds.test.tsx | 322 +++++++++++++++++- .../components/TimelineClipDiamonds.tsx | 284 +++++++++++++-- .../src/player/components/TimelineLanes.tsx | 2 +- .../player/components/timelineCallbacks.ts | 2 +- .../components/timelineKeyframeIdentity.ts | 17 + .../useTimelineKeyframeHandlers.test.tsx | 108 ++++++ .../components/useTimelineKeyframeHandlers.ts | 57 +++- 11 files changed, 772 insertions(+), 77 deletions(-) create mode 100644 packages/studio/src/player/components/timelineKeyframeIdentity.ts create mode 100644 packages/studio/src/player/components/useTimelineKeyframeHandlers.test.tsx diff --git a/packages/studio/src/components/editor/keyframeRetime.test.ts b/packages/studio/src/components/editor/keyframeRetime.test.ts index 876c5129a5..fc8844e54d 100644 --- a/packages/studio/src/components/editor/keyframeRetime.test.ts +++ b/packages/studio/src/components/editor/keyframeRetime.test.ts @@ -1,3 +1,6 @@ +// Boundary cases share an arrange/assert shape on purpose: each case states its +// own window, drag, and expected remap so a failure reads without cross-referencing. +// fallow-ignore-file code-duplication import { describe, expect, it } from "vitest"; import { resolveKeyframeRetime, type RetimeKeyframe } from "./keyframeRetime"; @@ -94,12 +97,12 @@ describe("resolveKeyframeRetime — resize (past the tween boundary)", () => { expect(r.kind).toBe("resize"); expect(r.position).toBeCloseTo(2, 5); // start unchanged expect(r.duration).toBeCloseTo(6, 5); // 8 - 2 - // abs 2/4/8 over the new [2,8] window → 0 / 33.3 / 100. pctRemap carries each + // abs 2/4/8 over the new [2,8] window → 0 / 33.333 / 100. pctRemap carries each // existing keyframe's old→new tween-%; the commit re-keys in place (value + // ease + _auto preserved by round-tripping the source node, not re-emitted here). expect(r.pctRemap).toEqual([ { from: 0, to: 0 }, - { from: 50, to: 33.3 }, + { from: 50, to: 33.333 }, { from: 100, to: 100 }, ]); }); @@ -113,10 +116,10 @@ describe("resolveKeyframeRetime — resize (past the tween boundary)", () => { expect(r.kind).toBe("resize"); expect(r.position).toBeCloseTo(0.5, 5); expect(r.duration).toBeCloseTo(5.5, 5); // 6 - 0.5 - // abs 0.5/4/6 over [0.5,6] → 0 / 63.6 / 100. + // abs 0.5/4/6 over [0.5,6] → 0 / 63.636 / 100. expect(r.pctRemap).toEqual([ { from: 0, to: 0 }, - { from: 50, to: 63.6 }, + { from: 50, to: 63.636 }, { from: 100, to: 100 }, ]); }); diff --git a/packages/studio/src/components/editor/keyframeRetime.ts b/packages/studio/src/components/editor/keyframeRetime.ts index f235b418ed..f7fa1d1344 100644 --- a/packages/studio/src/components/editor/keyframeRetime.ts +++ b/packages/studio/src/components/editor/keyframeRetime.ts @@ -55,7 +55,6 @@ const EPSILON_TIME = 1e-4; const MIN_TWEEN_DURATION = 0.01; const round3 = (n: number) => Math.round(n * 1000) / 1000; -const round1 = (n: number) => Math.round(n * 10) / 10; // 0.1% precision const clamp = (n: number, lo: number, hi: number) => Math.max(lo, Math.min(hi, n)); /** Resolve timing for a flat tween's synthesized start/end diamond. */ @@ -153,7 +152,7 @@ export function resolveKeyframeRetime(opts: { const pctRemap: KeyframePctRemap[] = keyframes.map((kf, i) => { const absTime = i === draggedIdx ? dropAbsTime : tweenStart + (kf.percentage / 100) * tweenDuration; - return { from: kf.percentage, to: round1(((absTime - newStart) / newDuration) * 100) }; + return { from: kf.percentage, to: round3(((absTime - newStart) / newDuration) * 100) }; }); return { diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index 76122eb47c..a1fa658521 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -152,13 +152,13 @@ export function useTimelineEditCallbacks({ // resizes the tween — position/duration grow so the dragged keyframe lands at // the drop while every other keyframe keeps its absolute time (value+ease too). // fallow-ignore-next-line complexity - onMoveKeyframe: (_elId: string, fromClipPct: number, toClipPct: number) => { + onMoveKeyframe: async (_elId: string, fromClipPct: number, toClipPct: number) => { const target = resolveKeyframeTarget(fromClipPct); const sel = domEditSelection; - if (!target || !sel) return; + if (!target || !sel) return false; const anim = selectedGsapAnimations.find((a) => a.id === target.animId); const tweenStart = anim ? resolveTweenStart(anim) : null; - if (!anim || tweenStart === null) return; + if (!anim || tweenStart === null) return false; const tweenDuration = anim.duration ?? resolveTweenDuration(anim); const sourceFile = sel.sourceFile || activeCompPath || "index.html"; const { elements, domClipChildren } = usePlayerStore.getState(); @@ -200,7 +200,10 @@ export function useTimelineEditCallbacks({ duration: decision.duration, }); } + } else { + return false; } + return true; }, onChangeKeyframeEase: (_elId: string, _pct: number, ease: string) => { for (const anim of selectedGsapAnimations) { diff --git a/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx b/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx index 34a1cc41ff..e40f0c295b 100644 --- a/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx +++ b/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx @@ -8,18 +8,32 @@ export interface KeyframeDiamondContextMenuState { elementId: string; percentage: number; tweenPercentage?: number; + propertyGroup?: string; + animationId?: string; currentEase?: string; } interface KeyframeDiamondContextMenuProps { state: KeyframeDiamondContextMenuState; onClose: () => void; - onDelete: (elementId: string, percentage: number) => void; + onDelete: ( + elementId: string, + percentage: number, + propertyGroup?: string, + tweenPercentage?: number, + animationId?: string, + ) => void; onDeleteAll: (elementId: string) => void; onChangeEase?: (elementId: string, percentage: number, ease: string) => void; onCopyProperties?: (elementId: string, percentage: number) => void; /** Retime the keyframe to the current playhead, preserving its value + ease. */ - onMoveToPlayhead?: (elementId: string, fromPercentage: number) => void; + onMoveToPlayhead?: ( + elementId: string, + fromPercentage: number, + propertyGroup?: string, + tweenPercentage?: number, + animationId?: string, + ) => void; } export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMenu({ @@ -51,7 +65,13 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe // Pass clip-% — resolveKeyframeTarget keys the cache lookup on clip-% // and returns the tween-% for the mutation. Passing tween-% here would // miss the lookup on any tween whose window is shorter than the clip. - onMoveToPlayhead(state.elementId, state.percentage); + onMoveToPlayhead( + state.elementId, + state.percentage, + state.propertyGroup, + state.tweenPercentage, + state.animationId, + ); onClose(); }} > @@ -64,7 +84,13 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe type="button" className="w-full flex items-center gap-2 px-3 py-1.5 text-xs text-red-400 hover:bg-neutral-800 cursor-pointer text-left" onClick={() => { - onDelete(state.elementId, state.percentage); + onDelete( + state.elementId, + state.percentage, + state.propertyGroup, + state.tweenPercentage, + state.animationId, + ); onClose(); }} > diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx index 254e5d6086..d22ac46a11 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx @@ -3,7 +3,7 @@ import React, { act } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { TimelineClipDiamonds } from "./TimelineClipDiamonds"; +import { TimelineClipDiamonds, TimelineDiamondLane } from "./TimelineClipDiamonds"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -84,12 +84,24 @@ describe("TimelineClipDiamonds", () => { const root = createRoot(host); act(() => { root.render( - { selectedKeyframes={new Set()} onClickKeyframe={onClickKeyframe} onMoveKeyframe={onMoveKeyframe} + groupAware />, ); }); @@ -117,7 +130,12 @@ describe("TimelineClipDiamonds", () => { diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 104 })); }); - expect(onClickKeyframe).toHaveBeenCalledWith(50); + expect(onClickKeyframe).toHaveBeenCalledWith({ + percentage: 50, + tweenPercentage: 100, + propertyGroup: "position", + animationId: "anim-1", + }); expect(onMoveKeyframe).not.toHaveBeenCalled(); act(() => root.unmount()); }); @@ -126,20 +144,39 @@ describe("TimelineClipDiamonds", () => { // keyframe) committed the move but never selected/parked on the result — // the diamond it was just dragged looked exactly like one nothing happened // to. Select it at its NEW position too. - it("selects the keyframe at its new position after a real drag-retime", () => { + it("reselects a retimed keyframe with its post-move tween percentage", () => { const onClickKeyframe = vi.fn(); - const onMoveKeyframe = vi.fn(); + const onMoveKeyframe = vi.fn().mockResolvedValue(true); const host = document.createElement("div"); document.body.append(host); const root = createRoot(host); act(() => { root.render( - { selectedKeyframes={new Set()} onClickKeyframe={onClickKeyframe} onMoveKeyframe={onMoveKeyframe} + groupAware + />, + ); + }); + const diamond = host.querySelector('button[title="40%"]'); + expect(diamond).not.toBeNull(); + + act(() => { + diamond!.dispatchEvent( + pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 80 }), + ); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 100 })); + }); + + expect(onMoveKeyframe).toHaveBeenCalledWith( + { + percentage: 40, + tweenPercentage: 50, + propertyGroup: "position", + animationId: "anim-1", + }, + 50, + ); + expect(onClickKeyframe).toHaveBeenCalledWith({ + percentage: 50, + tweenPercentage: 75, + propertyGroup: "position", + animationId: "anim-1", + }); + act(() => root.unmount()); + }); + + it("composes a rapid second retime from the pending position", () => { + const onMoveKeyframe = vi.fn().mockResolvedValue(true); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , ); }); @@ -161,13 +273,109 @@ describe("TimelineClipDiamonds", () => { diamond!.dispatchEvent( pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 100 }), ); - // 4px at a 200px clip width is 2 clip-% — well past the no-op epsilon, - // a real retime. - diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 104 })); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 150 })); + + // The cache still exposes 50%, but this second +10% drag starts at the + // pending 75% destination and must therefore land at 85%, not 60%. + diamond!.dispatchEvent( + pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 150 }), + ); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 170 })); + }); + + // The second move must identify the FROM keyframe by the pending (already- + // moved) position 75%, not the stale rendered 50%; otherwise the serialized + // mutation can't locate the keyframe the first move relocated. + expect(onMoveKeyframe).toHaveBeenNthCalledWith( + 2, + { + percentage: 75, + tweenPercentage: 75, + propertyGroup: "position", + animationId: "anim-1", + }, + 85, + ); + act(() => root.unmount()); + }); + + it.each([ + ["returns false", () => Promise.resolve(false)], + ["rejects", () => Promise.reject(new Error("retime failed"))], + ])("clears a failed pending retime when the callback %s", async (_label, settle) => { + const onMoveKeyframe = vi.fn().mockImplementationOnce(settle).mockResolvedValue(true); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); }); + const diamond = host.querySelector('button[title="50%"]'); + expect(diamond).not.toBeNull(); - expect(onMoveKeyframe).toHaveBeenCalledWith("clip-1", 50, 52); - expect(onClickKeyframe).toHaveBeenCalledWith(52); + await act(async () => { + diamond!.dispatchEvent( + pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 100 }), + ); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 150 })); + await Promise.resolve(); + }); + + act(() => { + diamond!.dispatchEvent( + pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 100 }), + ); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 120 })); + }); + + expect(onMoveKeyframe).toHaveBeenNthCalledWith( + 2, + { + percentage: 50, + tweenPercentage: 50, + propertyGroup: "position", + animationId: "anim-1", + }, + 60, + ); act(() => root.unmount()); }); @@ -212,4 +420,88 @@ describe("TimelineClipDiamonds", () => { expect(suppressClickRef.current).toBe(true); act(() => root.unmount()); }); + + const renderSegmentLane = (lastAmbiguous: boolean) => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const kf = (percentage: number, extra: Record = {}) => ({ + percentage, + tweenPercentage: percentage, + propertyGroup: "position", + animationId: "anim-1", + properties: { x: percentage }, + ...extra, + }); + act(() => { + root.render( + , + ); + }); + return { host, root }; + }; + + it("hides the inline ease button on an ambiguous merged segment", () => { + // Segments 0->50 and 50->100; the 50->100 segment ends on the ambiguous + // keyframe, so its hover/ease-button area is not rendered. + const { host, root } = renderSegmentLane(true); + expect(host.querySelectorAll("[data-keyframe-ease-segment]").length).toBe(1); + act(() => root.unmount()); + }); + + it("keeps the inline ease button on unambiguous merged segments", () => { + const { host, root } = renderSegmentLane(false); + expect(host.querySelectorAll("[data-keyframe-ease-segment]").length).toBe(2); + act(() => root.unmount()); + }); + + it("hides the inline ease button on a segment with no source animation id", () => { + // A runtime-scanned keyframe has no animationId, so there is no tween to + // target; the segment ending on it must not render a (dead) ease button. + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const kf = (percentage: number, animationId?: string) => ({ + percentage, + tweenPercentage: percentage, + propertyGroup: "position", + ...(animationId ? { animationId } : {}), + properties: { x: percentage }, + }); + act(() => { + root.render( + , + ); + }); + expect(host.querySelectorAll("[data-keyframe-ease-segment]").length).toBe(1); + act(() => root.unmount()); + }); }); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 129104dfb8..29fb6b9c77 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -1,22 +1,33 @@ -import { memo, useRef, useState } from "react"; +import { Fragment, memo, useEffect, useRef, useState } from "react"; import { BEAT_BAND_H } from "./BeatStrip"; import { KEYFRAME_DRAG_THRESHOLD_PX, previewClipPct, resolveKeyframeDrag, } from "../../components/editor/keyframeDrag"; - -interface KeyframeEntry { +import { MiniCurveSvg } from "../../components/editor/EaseCurveSection"; +import { clipToTweenPercentage } from "../../components/editor/KeyframeNavigation"; +import { LANE_H } from "./timelineLayout"; +import { + timelineKeyframeSelectionKey, + type TimelineKeyframeTarget, +} from "./timelineKeyframeIdentity"; +interface TimelineDiamondKeyframe { percentage: number; /** Tween-relative percentage (the retime mutation keys on this, not clip %). */ tweenPercentage?: number; + propertyGroup?: string; + 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; } interface KeyframeCacheEntry { format: string; - keyframes: KeyframeEntry[]; + keyframes: TimelineDiamondKeyframe[]; ease?: string; easeEach?: string; } @@ -32,7 +43,7 @@ interface TimelineClipDiamondsProps { isSelected: boolean; currentPercentage: number; elementId: string; - selectedKeyframes: Set; + selectedKeyframes: ReadonlySet; onClickKeyframe?: (percentage: number) => void; onShiftClickKeyframe?: (elementId: string, percentage: number) => void; onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void; @@ -44,13 +55,33 @@ interface TimelineClipDiamondsProps { elementId: string, fromClipPercentage: number, toClipPercentage: number, - ) => void; + ) => Promise; + /** Open the segment ease editor for the hovered mid-point button — available on + * the inline clip row too, not just the expanded lanes. */ + onSelectSegment?: (elementId: string, target: TimelineKeyframeTarget) => void; /** Set while resolving a diamond press so the ancestor clip's onClick (which * toggles selection off when already selected) ignores the native "click" * the browser auto-synthesizes after this button's pointerdown+pointerup. */ suppressClickRef?: React.RefObject; } +interface TimelineDiamondLaneProps extends Omit< + TimelineClipDiamondsProps, + | "onClickKeyframe" + | "onShiftClickKeyframe" + | "onContextMenuKeyframe" + | "onMoveKeyframe" + | "onSelectSegment" +> { + groupAware?: boolean; + globalEase?: string; + onSelectSegment?: (target: TimelineKeyframeTarget) => void; + onClickKeyframe?: (target: TimelineKeyframeTarget) => void; + onShiftClickKeyframe?: (target: TimelineKeyframeTarget) => void; + onContextMenuKeyframe?: (e: React.MouseEvent, target: TimelineKeyframeTarget) => void; + onMoveKeyframe?: (target: TimelineKeyframeTarget, toClipPercentage: number) => Promise; +} + const DIAMOND_RATIO = 0.8; // Percentage tolerance for rendering keyframes near clip boundaries. Keyframes // slightly outside [0, 100] (from rounding or stale cache during the async @@ -66,7 +97,21 @@ type DragState = { moved: boolean; }; -export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ +function keyframeTarget( + keyframe: TimelineDiamondKeyframe, + groupAware: boolean, +): TimelineKeyframeTarget { + return groupAware + ? { + percentage: keyframe.percentage, + tweenPercentage: keyframe.tweenPercentage, + propertyGroup: keyframe.propertyGroup, + animationId: keyframe.animationId, + } + : { percentage: keyframe.percentage }; +} + +export const TimelineDiamondLane = memo(function TimelineDiamondLane({ keyframesData, clipWidthPx, clipHeightPx, @@ -80,14 +125,35 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ onShiftClickKeyframe, onContextMenuKeyframe, onMoveKeyframe, + onSelectSegment, suppressClickRef, -}: TimelineClipDiamondsProps) { + groupAware = false, + globalEase = "none", +}: TimelineDiamondLaneProps) { // Hooks must run before the early return below. const dragRef = useRef(null); + // Pending retime destination (clip + tween %) per keyframe key, so a rapid + // second drag composes from where the first move left the keyframe (whose + // cache entry has not rebuilt yet) instead of the stale rendered value. + const pendingRetimeRef = useRef(new Map()); + useEffect(() => { + // Clear a pending entry once the authoritative cache reflects a keyframe at + // ~its destination. Match by tolerance, not equality: cache writers round + // clip %s, so an exact check would leak an entry after every successful retime. + for (const [key, pending] of pendingRetimeRef.current) { + if (keyframesData.keyframes.some((k) => Math.abs(k.percentage - pending.clipPct) < 0.2)) { + pendingRetimeRef.current.delete(key); + } + } + }, [keyframesData.keyframes]); // Visual-only preview of the dragged diamond's clip-% — no runtime/GSAP hold // (that optimistic hold was the #1763 flake). The atomic move-keyframe commit // on drop re-keys the diamond from source. const [preview, setPreview] = useState<{ kfKey: string; clipPct: number } | null>(null); + // Index of the segment whose mid-point ease button is revealed on hover, like + // Figma. Null = no segment hovered → no button shown (resting state is just + // the connector line + diamonds). + const [hoveredSegment, setHoveredSegment] = useState(null); // The button element can re-render (reposition/unmount) synchronously from // the state updates onClickKeyframe/onMoveKeyframe trigger, before the // browser gets to auto-synthesize the "click" event that normally follows @@ -108,7 +174,12 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ // When the beat strip occupies the top band, shrink the diamonds and center // them in the remaining bottom region so they don't collide with it. - const diamondSize = Math.round(clipHeightPx * (beatsActive ? 0.45 : DIAMOND_RATIO)); + // One consistent keyframe-diamond size everywhere (clip bars + property lanes), + // matching the property-lane size (LANE_H · ratio). Beat-strip tracks still + // shrink to fit under the strip. + const diamondSize = beatsActive + ? Math.round(clipHeightPx * 0.45) + : Math.round(LANE_H * DIAMOND_RATIO); const half = diamondSize / 2; const centerY = beatsActive ? BEAT_BAND_H + (clipHeightPx - BEAT_BAND_H) / 2 : clipHeightPx / 2; const sorted = keyframesData.keyframes @@ -141,32 +212,90 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ const x1 = Math.max(0, Math.min(clipWidthPx, (prev.percentage / 100) * clipWidthPx)); const x2 = Math.max(0, Math.min(clipWidthPx, (kf.percentage / 100) * clipWidthPx)); if (x2 - x1 < 1) return null; + // Group-aware target for the ease button: the segment ease is + // per-keyframe (each keyframe carries its own animationId/tweenPercentage). + // On a merged inline row the button is hidden where the segment is + // ambiguous (two source animations collide at this % with different + // eases; see easeAmbiguous) or the keyframe has no source animation id + // (runtime-scanned) so there is no tween to target. + const target = keyframeTarget(kf, true); + const ease = kf.ease ?? globalEase; return ( -
+ +
+ {onSelectSegment && !kf.easeAmbiguous && kf.animationId !== undefined && ( +
setHoveredSegment(i)} + onMouseLeave={() => setHoveredSegment((h) => (h === i ? null : h))} + > + {hoveredSegment === i && ( + + )} +
+ )} + ); })} {sorted.map((kf, i) => { - const kfKey = `${elementId}:${kf.percentage}`; + const target = keyframeTarget(kf, groupAware); + const kfKey = timelineKeyframeSelectionKey(elementId, target); // While dragging this diamond, render it at the live preview clip-%. const renderPct = preview?.kfKey === kfKey ? preview.clipPct : kf.percentage; - // Center the diamond ON its keyframe %: left = (% · width) − half so the - // diamond's midpoint sits exactly at the percentage. At 0% the midpoint - // is the clip's left edge (the left half overflows, which the - // overflow-visible clip shows) — NOT shifted fully inside. + // Center the diamond ON its keyframe %: left = (% · width) − half, so the + // diamond's midpoint sits exactly on the playhead/ruler x for that time. + // The 0% diamond's left half lands in the reserved left gutter (the + // content origin is inset past the label column, Figma-style) so it stays + // fully visible instead of being clipped by the sticky label column. const leftPx = (renderPct / 100) * clipWidthPx - half; const isKfSelected = selectedKeyframes.has(kfKey); const atPlayhead = isSelected && Math.abs(kf.percentage - currentPercentage) < 0.5; @@ -181,7 +310,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ dragRef.current = { kfKey, startX: e.clientX, - fromClipPct: kf.percentage, + fromClipPct: pendingRetimeRef.current.get(kfKey)?.clipPct ?? kf.percentage, moved: false, }; } @@ -212,8 +341,8 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ if (!d || d.kfKey !== kfKey) { if (e.button !== 0) return; suppressNextClick(); - if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage); - else onClickKeyframe?.(kf.percentage); + if (e.shiftKey) onShiftClickKeyframe?.(target); + else onClickKeyframe?.(target); return; } e.stopPropagation(); @@ -235,14 +364,54 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ // back onto ~the same position — no real retime, so treat it as the // click it was. Otherwise a normal click with a few px of mouse/ // trackpad drift silently does nothing: no selection, no move. - if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage); - else onClickKeyframe?.(kf.percentage); + if (e.shiftKey) onShiftClickKeyframe?.(target); + else onClickKeyframe?.(target); } else if (res.kind === "move" && res.toClipPct != null) { - onMoveKeyframe?.(elementId, d.fromClipPct, res.toClipPct); + const animKfs = + target.animationId === undefined + ? keyframesData.keyframes + : keyframesData.keyframes.filter((k) => k.animationId === target.animationId); + // Clamp to the mapped tween range: clipToTweenPercentage extrapolates + // linearly, so a boundary drag past the range would otherwise reselect + // an out-of-range tween % (e.g. 150%) even though the mutation clamps + // the moved endpoint back to the boundary. + const tweenPcts = animKfs + .map((k) => k.tweenPercentage) + .filter((v): v is number => typeof v === "number"); + const clampTween = (v: number) => + tweenPcts.length + ? Math.max(Math.min(...tweenPcts), Math.min(Math.max(...tweenPcts), v)) + : v; + const newTweenPct = clampTween(clipToTweenPercentage(animKfs, res.toClipPct)); + // For a rapid second retime the diamond still renders the stale cache + // position, so identify the FROM keyframe by the pending (already-moved) + // position; the mutation locates the source keyframe by this identity. + const pendingBefore = pendingRetimeRef.current.get(kfKey); + const fromTarget = pendingBefore + ? { + ...target, + percentage: pendingBefore.clipPct, + tweenPercentage: pendingBefore.tweenPct, + } + : target; + const pending = { clipPct: res.toClipPct, tweenPct: newTweenPct }; + pendingRetimeRef.current.set(kfKey, pending); + const clearPending = () => { + if (pendingRetimeRef.current.get(kfKey) === pending) { + pendingRetimeRef.current.delete(kfKey); + } + }; + void onMoveKeyframe?.(fromTarget, res.toClipPct).then((committed) => { + if (!committed) clearPending(); + }, clearPending); // A retime still targeted this exact diamond — park/select it at its // new position, same as a plain click, or a drag that actually moved // something looks identical to one that silently did nothing. - onClickKeyframe?.(res.toClipPct); + onClickKeyframe?.({ + ...target, + percentage: res.toClipPct, + tweenPercentage: newTweenPct, + }); } }; @@ -251,6 +420,10 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ key={`${i}-${kf.percentage}`} type="button" className="absolute" + data-keyframe-group={groupAware ? kf.propertyGroup : undefined} + data-keyframe-percentage={ + groupAware ? (kf.tweenPercentage ?? kf.percentage) : undefined + } style={{ left: leftPx, top: centerY, @@ -268,10 +441,19 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ onPointerDown={onPointerDown} onPointerMove={onPointerMove} onPointerUp={onPointerUp} + onPointerCancel={(e) => { + // Browser/OS cancellation (or lost capture) ends the drag without a + // pointerup, so clear the armed drag and preview or a ghost diamond + // stays stuck at the last previewed position. + if (dragRef.current?.kfKey !== kfKey) return; + dragRef.current = null; + setPreview(null); + e.currentTarget.releasePointerCapture?.(e.pointerId); + }} onContextMenu={(e) => { e.preventDefault(); e.stopPropagation(); - onContextMenuKeyframe?.(e, elementId, kf.percentage); + onContextMenuKeyframe?.(e, target); }} title={`${kf.percentage}%`} > @@ -297,3 +479,33 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
); }); + +export const TimelineClipDiamonds = memo(function TimelineClipDiamonds( + props: TimelineClipDiamondsProps, +) { + return ( + props.onClickKeyframe?.(target.percentage)} + onShiftClickKeyframe={(target) => + props.onShiftClickKeyframe?.(props.elementId, target.percentage) + } + onContextMenuKeyframe={(e, target) => + props.onContextMenuKeyframe?.(e, props.elementId, target.percentage) + } + onMoveKeyframe={ + props.onMoveKeyframe + ? (target, toClipPercentage) => + props.onMoveKeyframe?.(props.elementId, target.percentage, toClipPercentage) ?? + Promise.resolve(false) + : undefined + } + onSelectSegment={ + props.onSelectSegment + ? (target) => props.onSelectSegment?.(props.elementId, target) + : undefined + } + /> + ); +}); diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 75069c1a0b..13707c05c7 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -78,7 +78,7 @@ export interface TimelineLaneBaseProps { elementId: string, fromClipPercentage: number, toClipPercentage: number, - ) => void; + ) => Promise; onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void; /** * Right-click on EMPTY lane space (not on a clip — those preventDefault diff --git a/packages/studio/src/player/components/timelineCallbacks.ts b/packages/studio/src/player/components/timelineCallbacks.ts index 2ba62c5bd6..71db7d8637 100644 --- a/packages/studio/src/player/components/timelineCallbacks.ts +++ b/packages/studio/src/player/components/timelineCallbacks.ts @@ -70,6 +70,6 @@ export interface TimelineEditCallbacks { elementId: string, fromClipPercentage: number, toClipPercentage: number, - ) => void; + ) => Promise; onToggleKeyframeAtPlayhead?: (element: TimelineElement) => void; } diff --git a/packages/studio/src/player/components/timelineKeyframeIdentity.ts b/packages/studio/src/player/components/timelineKeyframeIdentity.ts new file mode 100644 index 0000000000..8cf58d7118 --- /dev/null +++ b/packages/studio/src/player/components/timelineKeyframeIdentity.ts @@ -0,0 +1,17 @@ +export interface TimelineKeyframeTarget { + percentage: number; + tweenPercentage?: number; + propertyGroup?: string; + animationId?: string; +} + +export function timelineKeyframeSelectionKey( + elementId: string, + target: TimelineKeyframeTarget, +): string { + if (!target.propertyGroup) return `${elementId}:${target.percentage}`; + const groupKey = target.animationId + ? `${target.propertyGroup}:${target.animationId}` + : target.propertyGroup; + return `${elementId}:${groupKey}:${target.percentage}`; +} diff --git a/packages/studio/src/player/components/useTimelineKeyframeHandlers.test.tsx b/packages/studio/src/player/components/useTimelineKeyframeHandlers.test.tsx new file mode 100644 index 0000000000..0c10202dbf --- /dev/null +++ b/packages/studio/src/player/components/useTimelineKeyframeHandlers.test.tsx @@ -0,0 +1,108 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { mountReactHarness } from "../../hooks/domSelectionTestHarness"; +import type { TimelineElement } from "../store/playerStore"; +import { usePlayerStore } from "../store/playerStore"; +import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; +import { useTimelineKeyframeHandlers } from "./useTimelineKeyframeHandlers"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const trackStudioSegmentEaseEdit = vi.hoisted(() => vi.fn()); +vi.mock("../../telemetry/events", () => ({ trackStudioSegmentEaseEdit })); + +const ELEMENT: TimelineElement = { + id: "clip-1", + label: "Hero card", + tag: "div", + start: 1, + duration: 2, + track: 0, +}; + +const TARGET: TimelineKeyframeTarget = { + percentage: 50, + tweenPercentage: 50, + propertyGroup: "position", + animationId: "position-tween", +}; + +const FLAT_TWEEN_TARGET: TimelineKeyframeTarget = { + percentage: 100, + tweenPercentage: 100, + propertyGroup: "position", + animationId: "position-tween", +}; + +afterEach(() => { + document.body.innerHTML = ""; + trackStudioSegmentEaseEdit.mockClear(); + usePlayerStore.setState({ focusedEaseSegment: null }); +}); + +describe("useTimelineKeyframeHandlers", () => { + it("tracks opening the segment ease editor when a timeline segment is selected", () => { + let onSelectSegment: ((elementId: string, target: TimelineKeyframeTarget) => void) | undefined; + + function Harness() { + ({ onSelectSegment } = useTimelineKeyframeHandlers({ + expandedElements: [ELEMENT], + keyframeCache: new Map(), + setSelectedElementId: vi.fn(), + setKfContextMenu: vi.fn(), + toggleSelectedKeyframe: vi.fn(), + })); + return null; + } + + const root = mountReactHarness(); + act(() => onSelectSegment?.(ELEMENT.id, TARGET)); + + expect(trackStudioSegmentEaseEdit).toHaveBeenCalledOnce(); + expect(trackStudioSegmentEaseEdit).toHaveBeenCalledWith({ action: "open" }); + act(() => root.unmount()); + }); + + it("focuses a flat tween segment without seeking, while keyframe clicks still seek", () => { + const onSeek = vi.fn(); + const onSelectElement = vi.fn(); + const setSelectedElementId = vi.fn(); + let onClickKeyframe: + | ((el: TimelineElement, target: TimelineKeyframeTarget) => void) + | undefined; + let onSelectSegment: ((elementId: string, target: TimelineKeyframeTarget) => void) | undefined; + + function Harness() { + ({ onClickKeyframe, onSelectSegment } = useTimelineKeyframeHandlers({ + expandedElements: [ELEMENT], + keyframeCache: new Map(), + onSelectElement, + onSeek, + setSelectedElementId, + setKfContextMenu: vi.fn(), + toggleSelectedKeyframe: vi.fn(), + })); + return null; + } + + const root = mountReactHarness(); + + // Selecting a segment must NOT move the playhead. + act(() => onSelectSegment?.(ELEMENT.id, FLAT_TWEEN_TARGET)); + expect(onSeek).not.toHaveBeenCalled(); + expect(usePlayerStore.getState().focusedEaseSegment).toEqual({ + animationId: "position-tween", + tweenPercentage: 100, + elementId: ELEMENT.id, + }); + expect(setSelectedElementId).toHaveBeenCalledWith(ELEMENT.id); + expect(onSelectElement).toHaveBeenCalledWith(ELEMENT); + + // Clicking the keyframe itself still seeks to it (start 1 + 50% of 2 = 2). + act(() => onClickKeyframe?.(ELEMENT, TARGET)); + expect(onSeek).toHaveBeenCalledExactlyOnceWith(2); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/useTimelineKeyframeHandlers.ts b/packages/studio/src/player/components/useTimelineKeyframeHandlers.ts index d9a91abd63..e697d49277 100644 --- a/packages/studio/src/player/components/useTimelineKeyframeHandlers.ts +++ b/packages/studio/src/player/components/useTimelineKeyframeHandlers.ts @@ -1,7 +1,12 @@ import { useCallback, type MouseEvent as ReactMouseEvent } from "react"; +import { trackStudioSegmentEaseEdit } from "../../telemetry/events"; import type { TimelineElement, KeyframeCacheEntry } from "../store/playerStore"; import { usePlayerStore } from "../store/playerStore"; import type { KeyframeDiamondContextMenuState } from "./KeyframeDiamondContextMenu"; +import { + timelineKeyframeSelectionKey, + type TimelineKeyframeTarget, +} from "./timelineKeyframeIdentity"; interface UseTimelineKeyframeHandlersInput { expandedElements: TimelineElement[]; @@ -23,42 +28,71 @@ export function useTimelineKeyframeHandlers({ toggleSelectedKeyframe, }: UseTimelineKeyframeHandlersInput) { const onClickKeyframe = useCallback( - (el: TimelineElement, pct: number) => { + (el: TimelineElement, target: TimelineKeyframeTarget, options?: { seek?: boolean }) => { usePlayerStore.getState().clearSelectedKeyframes(); const elKey = el.key ?? el.id; setSelectedElementId(elKey); onSelectElement?.(el); - toggleSelectedKeyframe(`${elKey}:${pct}`); - onSeek?.(el.start + (pct / 100) * el.duration); + toggleSelectedKeyframe(timelineKeyframeSelectionKey(elKey, target)); + // Clicking a diamond seeks the playhead to it; selecting a segment to edit + // its ease (options.seek === false) must NOT move the playhead. + if (options?.seek !== false) { + onSeek?.(el.start + (target.percentage / 100) * el.duration); + } const kfData = keyframeCache.get(elKey); - const kf = kfData?.keyframes.find((item) => Math.abs(item.percentage - pct) < 0.5); - usePlayerStore.getState().setActiveKeyframePct(kf?.tweenPercentage ?? null); + const kf = kfData?.keyframes.find( + (item) => Math.abs(item.percentage - target.percentage) < 0.5, + ); + usePlayerStore + .getState() + .setActiveKeyframePct(target.tweenPercentage ?? kf?.tweenPercentage ?? null); }, [keyframeCache, onSeek, onSelectElement, setSelectedElementId, toggleSelectedKeyframe], ); const onShiftClickKeyframe = useCallback( - (elId: string, pct: number) => { - toggleSelectedKeyframe(`${elId}:${pct}`); + (elId: string, target: TimelineKeyframeTarget) => { + toggleSelectedKeyframe(timelineKeyframeSelectionKey(elId, target)); }, [toggleSelectedKeyframe], ); + const onSelectSegment = useCallback( + (elId: string, target: TimelineKeyframeTarget) => { + const el = expandedElements.find((item) => (item.key ?? item.id) === elId); + if (!el) return; + onClickKeyframe(el, target, { seek: false }); + if (target.animationId !== undefined && target.tweenPercentage !== undefined) { + usePlayerStore.getState().setFocusedEaseSegment({ + animationId: target.animationId, + tweenPercentage: target.tweenPercentage, + elementId: elId, + }); + trackStudioSegmentEaseEdit({ action: "open" }); + } + }, + [expandedElements, onClickKeyframe], + ); + const onContextMenuKeyframe = useCallback( - (e: ReactMouseEvent, elId: string, pct: number) => { + (e: ReactMouseEvent, elId: string, target: TimelineKeyframeTarget) => { const el = expandedElements.find((item) => (item.key ?? item.id) === elId); if (el) { setSelectedElementId(elId); onSelectElement?.(el); } const kfData = keyframeCache.get(elId); - const kf = kfData?.keyframes.find((item) => Math.abs(item.percentage - pct) < 0.2); + const kf = kfData?.keyframes.find( + (item) => Math.abs(item.percentage - target.percentage) < 0.2, + ); setKfContextMenu({ x: e.clientX + 4, y: e.clientY + 2, elementId: elId, - percentage: pct, - tweenPercentage: kf?.tweenPercentage, + percentage: target.percentage, + tweenPercentage: target.tweenPercentage ?? kf?.tweenPercentage, + propertyGroup: target.propertyGroup, + animationId: target.animationId, currentEase: kf?.ease ?? kfData?.ease, }); }, @@ -67,6 +101,7 @@ export function useTimelineKeyframeHandlers({ return { onClickKeyframe, + onSelectSegment, onShiftClickKeyframe, onContextMenuKeyframe, }; From 706f537f33bcf182f6b294806384d377c1b1d747 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sat, 25 Jul 2026 22:53:14 +0200 Subject: [PATCH 12/25] fix(studio): let Escape cancel a keyframe retime and throttle its preview Escape now ends an in-flight diamond drag the way it already ends clip and element drags: the armed gesture is marked cancelled, the preview is dropped, and the pointerup that follows is swallowed instead of falling through to the click branch. The preview also flushes once per animation frame instead of once per pointermove, so a high-rate trackpad no longer re-renders every diamond in the row several times a frame. Single-diamond retime stays the documented scope; multi-select drag needs a batched mutation the script ops do not express yet. --- .../components/TimelineClipDiamonds.test.tsx | 47 +++++++++++++ .../components/TimelineClipDiamonds.tsx | 66 +++++++++++++++++-- 2 files changed, 106 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx index d22ac46a11..7a6022a846 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx @@ -379,6 +379,53 @@ describe("TimelineClipDiamonds", () => { act(() => root.unmount()); }); + it("cancels an in-flight retime on Escape without committing or selecting", () => { + const onClickKeyframe = vi.fn(); + const onMoveKeyframe = vi.fn().mockResolvedValue(true); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + const diamond = host.querySelector('button[title="40%"]'); + expect(diamond).not.toBeNull(); + + act(() => { + diamond!.dispatchEvent( + pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 80 }), + ); + document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true })); + diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 100 })); + }); + + // Escape ends the gesture: no retime is written, and the release is not + // reinterpreted as a click that would park the playhead on the keyframe. + expect(onMoveKeyframe).not.toHaveBeenCalled(); + expect(onClickKeyframe).not.toHaveBeenCalled(); + act(() => root.unmount()); + }); + // Regression: onClickKeyframe's state updates can re-render the diamond // button out from under the gesture before the browser auto-synthesizes the // "click" event that follows a button's pointerdown+pointerup. That orphaned diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 29fb6b9c77..37510955b0 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -95,6 +95,13 @@ type DragState = { startX: number; fromClipPct: number; moved: boolean; + /** Latest pointer x, flushed to the preview once per frame. */ + lastX: number; + /** Index in the sorted row, needed by the neighbour clamp off the render path. */ + index: number; + /** Escape was pressed: the drag is dead, and the pointerup that follows is + * swallowed rather than falling through to the click branch. */ + cancelled?: boolean; }; function keyframeTarget( @@ -150,6 +157,31 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ // (that optimistic hold was the #1763 flake). The atomic move-keyframe commit // on drop re-keys the diamond from source. const [preview, setPreview] = useState<{ kfKey: string; clipPct: number } | null>(null); + // One preview render per frame: a 120Hz trackpad fires pointermove far faster + // than the lane can repaint, and every diamond in the row re-evaluates its + // memo on each of those renders. + const previewFrameRef = useRef(null); + const cancelPreviewFrame = () => { + if (previewFrameRef.current === null) return; + cancelAnimationFrame(previewFrameRef.current); + previewFrameRef.current = null; + }; + // Escape backs out of an in-flight retime, the way clip and element drags + // already do. Nothing was written yet (the commit happens on pointerup), so + // dropping the preview is the whole undo. + useEffect(() => { + const onKeyDown = (event: KeyboardEvent) => { + if (event.key !== "Escape" || !dragRef.current || dragRef.current.cancelled) return; + dragRef.current.cancelled = true; + cancelPreviewFrame(); + setPreview(null); + }; + document.addEventListener("keydown", onKeyDown); + return () => { + document.removeEventListener("keydown", onKeyDown); + cancelPreviewFrame(); + }; + }, []); // Index of the segment whose mid-point ease button is revealed on hover, like // Figma. Null = no segment hovered → no button shown (resting state is just // the connector line + diamonds). @@ -310,6 +342,8 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ dragRef.current = { kfKey, startX: e.clientX, + lastX: e.clientX, + index: i, fromClipPct: pendingRetimeRef.current.get(kfKey)?.clipPct ?? kf.percentage, moved: false, }; @@ -317,26 +351,38 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ }; const onPointerMove = (e: React.PointerEvent) => { const d = dragRef.current; - if (!d || d.kfKey !== kfKey) return; + if (!d || d.kfKey !== kfKey || d.cancelled) return; + d.lastX = e.clientX; if (!d.moved && Math.abs(e.clientX - d.startX) >= KEYFRAME_DRAG_THRESHOLD_PX) { d.moved = true; } - if (d.moved) { + if (!d.moved || previewFrameRef.current !== null) return; + previewFrameRef.current = requestAnimationFrame(() => { + previewFrameRef.current = null; + const live = dragRef.current; + if (!live || live.kfKey !== kfKey || live.cancelled) return; setPreview({ kfKey, clipPct: previewClipPct({ - pointerDownX: d.startX, - pointerMoveX: e.clientX, + pointerDownX: live.startX, + pointerMoveX: live.lastX, clipWidthPx, - draggedClipPct: d.fromClipPct, - draggedIndex: i, + draggedClipPct: live.fromClipPct, + draggedIndex: live.index, sortedClipPcts, }), }); - } + }); }; const onPointerUp = (e: React.PointerEvent) => { const d = dragRef.current; + if (d?.kfKey === kfKey && d.cancelled) { + // Escape already ended this drag; the release is not a click. + dragRef.current = null; + e.currentTarget.releasePointerCapture?.(e.pointerId); + suppressNextClick(); + return; + } // No drag armed (canDrag false / non-primary press) → treat as a click. if (!d || d.kfKey !== kfKey) { if (e.button !== 0) return; @@ -347,9 +393,14 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ } e.stopPropagation(); dragRef.current = null; + cancelPreviewFrame(); setPreview(null); e.currentTarget.releasePointerCapture?.(e.pointerId); suppressNextClick(); + // Single-diamond retime by design: a multi-select drag would have to + // move every selected keyframe as one mutation, which the script ops + // do not express yet. Selecting several and dragging one moves only + // the dragged one. const res = resolveKeyframeDrag({ pointerDownX: d.startX, pointerUpX: e.clientX, @@ -447,6 +498,7 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ // stays stuck at the last previewed position. if (dragRef.current?.kfKey !== kfKey) return; dragRef.current = null; + cancelPreviewFrame(); setPreview(null); e.currentTarget.releasePointerCapture?.(e.pointerId); }} From 6e0118cb3f5adf43bec8ee08bac89a164b480328 Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Sun, 26 Jul 2026 00:57:53 +0200 Subject: [PATCH 13/25] refactor(studio): guard the diamond connector's previous keyframe CONTRIBUTING.md asks for a guard clause rather than a non-null assertion outside an already-checked path. The index check and the lookup are now the same guard. --- .../studio/src/player/components/TimelineClipDiamonds.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 37510955b0..a592a4db50 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -239,8 +239,8 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ }} > {sorted.map((kf, i) => { - if (i === 0) return null; - const prev = sorted[i - 1]!; + const prev = sorted[i - 1]; + if (!prev) return null; const x1 = Math.max(0, Math.min(clipWidthPx, (prev.percentage / 100) * clipWidthPx)); const x2 = Math.max(0, Math.min(clipWidthPx, (kf.percentage / 100) * clipWidthPx)); if (x2 - x1 < 1) return null; From fed5e5b71df505b2598fe31522eba1710ea62c7c Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 16:51:46 +0200 Subject: [PATCH 14/25] feat(studio): add timeline property lanes --- .../components/TimelineClipDiamonds.tsx | 2 +- .../components/TimelinePropertyLanes.test.tsx | 433 ++++++++++++++++++ .../components/TimelinePropertyLanes.tsx | 163 +++++++ .../src/player/components/timelineLayout.ts | 4 + .../useAutoExpandKeyframedClips.test.tsx | 75 +++ .../components/useAutoExpandKeyframedClips.ts | 36 ++ 6 files changed, 712 insertions(+), 1 deletion(-) create mode 100644 packages/studio/src/player/components/TimelinePropertyLanes.test.tsx create mode 100644 packages/studio/src/player/components/TimelinePropertyLanes.tsx create mode 100644 packages/studio/src/player/components/useAutoExpandKeyframedClips.test.tsx create mode 100644 packages/studio/src/player/components/useAutoExpandKeyframedClips.ts diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index a592a4db50..1a066d5d03 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -12,7 +12,7 @@ import { timelineKeyframeSelectionKey, type TimelineKeyframeTarget, } from "./timelineKeyframeIdentity"; -interface TimelineDiamondKeyframe { +export interface TimelineDiamondKeyframe { percentage: number; /** Tween-relative percentage (the retime mutation keys on this, not clip %). */ tweenPercentage?: number; diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx new file mode 100644 index 0000000000..475dbd1e96 --- /dev/null +++ b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx @@ -0,0 +1,433 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import type { GsapAnimation, PropertyGroupName } from "@hyperframes/core/gsap-parser"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TimelineClipDiamonds } from "./TimelineClipDiamonds"; +import { + getTimelinePropertyLanes, + TimelinePropertyLanes, + type TimelinePropertyLanesProps, +} from "./TimelinePropertyLanes"; +import { timelineKeyframeSelectionKey } from "./timelineKeyframeIdentity"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +function animation( + id: string, + propertyGroup: PropertyGroupName, + keyframes: Array<{ + percentage: number; + properties: Record; + ease?: string; + }>, +): GsapAnimation { + return { + id, + targetSelector: "#clip-1", + method: "to", + position: 0, + duration: 1, + properties: {}, + propertyGroup, + keyframes: { format: "percentage", keyframes }, + }; +} + +function flatAnimation( + id: string, + propertyGroup: PropertyGroupName, + properties: Record, +): GsapAnimation { + return { + id, + targetSelector: "#clip-1", + method: "to", + position: 0, + duration: 1, + properties, + propertyGroup, + }; +} + +function renderPropertyLanes(overrides: Partial = {}): { + host: HTMLDivElement; + root: Root; +} { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + return { host, root }; +} + +function laneDiamonds(host: HTMLElement, group: string): HTMLButtonElement[] { + return Array.from( + host.querySelectorAll( + `[data-property-group="${group}"] button[data-keyframe-percentage]`, + ), + ); +} + +function expectLanePercentages(host: HTMLElement, group: string, percentages: string[]) { + expect(laneDiamonds(host, group).map((diamond) => diamond.dataset.keyframePercentage)).toEqual( + percentages, + ); +} + +function laneEaseButtons(host: HTMLElement, group: string): HTMLButtonElement[] { + return Array.from( + host.querySelectorAll( + `[data-property-group="${group}"] button[data-keyframe-ease-button]`, + ), + ); +} + +function laneEaseSegments(host: HTMLElement, group: string): HTMLElement[] { + return Array.from( + host.querySelectorAll( + `[data-property-group="${group}"] [data-keyframe-ease-segment]`, + ), + ); +} + +// The mid-segment ease button is revealed on hover (Figma parity), so tests must +// hover the segment strip before its button exists. React derives onMouseEnter +// from a bubbling mouseover, so dispatching that is what arms the hover. +function revealEaseButton(segment: HTMLElement): HTMLButtonElement | null { + act(() => { + segment.dispatchEvent(new MouseEvent("mouseover", { bubbles: true })); + }); + return segment.querySelector("button[data-keyframe-ease-button]"); +} + +const POSITION_SEGMENT_ANIMATION = animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 50, properties: { x: 50 } }, +]); + +describe("TimelinePropertyLanes", () => { + it("returns a position lane with synthesized endpoints for a flat tween", () => { + const lanes = getTimelinePropertyLanes( + [flatAnimation("position-tween", "position", { x: 420 })], + 0, + 1, + ); + + expect(lanes).toHaveLength(1); + expect(lanes[0]?.group).toBe("position"); + expect(lanes[0]?.keyframes).toEqual([ + { + percentage: 0, + tweenPercentage: 0, + properties: { x: 0 }, + propertyGroup: "position", + animationId: "position-tween", + }, + { + percentage: 100, + tweenPercentage: 100, + properties: { x: 420 }, + propertyGroup: "position", + animationId: "position-tween", + }, + ]); + }); + + it("returns both flat and authored keyframe property groups", () => { + const lanes = getTimelinePropertyLanes( + [ + flatAnimation("position-tween", "position", { x: 420 }), + animation("visual-tween", "visual", [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 100, properties: { opacity: 1 } }, + ]), + ], + 0, + 1, + ); + + expect(lanes.map((lane) => lane.group)).toEqual(["position", "visual"]); + expect(lanes.map((lane) => lane.keyframes.map((keyframe) => keyframe.percentage))).toEqual([ + [0, 100], + [0, 100], + ]); + }); + + it("renders each source property group at its independent keyframe positions", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0, y: 0 } }, + { percentage: 50, properties: { x: 100, y: 20 } }, + { percentage: 100, properties: { x: 200, y: 40 } }, + ]), + animation("visual-tween", "visual", [{ percentage: 25, properties: { opacity: 0.5 } }]), + ]; + + const { host, root } = renderPropertyLanes({ animations }); + const position = laneDiamonds(host, "position"); + const visual = laneDiamonds(host, "visual"); + + expect(position).toHaveLength(3); + expect(visual).toHaveLength(1); + // Diamonds are centered on their true keyframe time (0% at -half); the + // reserved left gutter (content origin inset, tested at the Timeline level) + // keeps the overflowing left half visible rather than clamping it inward. + expect(position.map((diamond) => diamond.style.left)).toEqual(["-11px", "89px", "189px"]); + expect(visual[0]?.style.left).toBe("39px"); + expect( + host.querySelectorAll('[data-property-group="position"] [data-keyframe-connector]'), + ).toHaveLength(2); + act(() => root.unmount()); + }); + + it("keeps both groups' diamonds when their source keyframes share 0% and 100%", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 100, properties: { x: 100 } }, + ]), + animation("visual-tween", "visual", [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 100, properties: { opacity: 1 } }, + ]), + ]; + + const { host, root } = renderPropertyLanes({ animations }); + + expectLanePercentages(host, "position", ["0", "100"]); + expectLanePercentages(host, "visual", ["0", "100"]); + act(() => root.unmount()); + }); + + it("renders an authored hold keyframe whose value equals its predecessor", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 10 } }, + { percentage: 50, properties: { x: 10 } }, + { percentage: 100, properties: { x: 20 } }, + ]), + ]; + + const { host, root } = renderPropertyLanes({ animations }); + + expectLanePercentages(host, "position", ["0", "50", "100"]); + act(() => root.unmount()); + }); + + it("keeps Position@50% selection distinct from Opacity@50%", () => { + const onClickKeyframe = vi.fn(); + const animations = [ + animation("position-tween", "position", [{ percentage: 50, properties: { x: 50 } }]), + animation("visual-tween", "visual", [{ percentage: 50, properties: { opacity: 0.5 } }]), + ]; + const { host, root } = renderPropertyLanes({ animations, onClickKeyframe }); + const position = laneDiamonds(host, "position")[0]!; + + act(() => { + position.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, button: 0 })); + }); + + const target = onClickKeyframe.mock.calls[0]?.[0]; + expect(target).toEqual({ + animationId: "position-tween", + percentage: 50, + propertyGroup: "position", + tweenPercentage: 50, + }); + + act(() => { + root.render( + , + ); + }); + + const positionFill = laneDiamonds(host, "position")[0]?.querySelector("path:last-child"); + const visualFill = laneDiamonds(host, "visual")[0]?.querySelector("path:last-child"); + expect(positionFill?.getAttribute("fill")).toBe("#4ba3d2"); + expect(visualFill?.getAttribute("fill")).toBe("#a3a3a3"); + act(() => root.unmount()); + }); + + it("reveals one midpoint ease button per segment on hover, regardless of selection", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 50, properties: { x: 50 } }, + { percentage: 100, properties: { x: 100 } }, + ]), + ]; + const { host, root } = renderPropertyLanes({ animations, onSelectSegment: vi.fn() }); + + const segments = laneEaseSegments(host, "position"); + expect(segments).toHaveLength(2); + expect(segments.map((segment) => segment.style.left)).toEqual(["0px", "100px"]); + expect(laneDiamonds(host, "position")).toHaveLength(3); + // Resting state: no button until a segment is hovered. + expect(laneEaseButtons(host, "position")).toHaveLength(0); + + // Hovering reveals exactly one button — the hovered segment's. + expect(revealEaseButton(segments[0]!)).not.toBeNull(); + expect(laneEaseButtons(host, "position")).toHaveLength(1); + + // The ease button is available on hover even when the element is NOT selected + // (a lane shows for the track's active/primary clip, not only the selected one). + act(() => { + root.render( + , + ); + }); + const unselectedSegments = laneEaseSegments(host, "position"); + expect(unselectedSegments).toHaveLength(2); + expect(revealEaseButton(unselectedSegments[0]!)).not.toBeNull(); + act(() => root.unmount()); + }); + + it("reveals each segment's button with its destination keyframe ease curve", () => { + const animations = [ + animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 33, properties: { x: 33 }, ease: "none" }, + { percentage: 66, properties: { x: 66 }, ease: "power2.out" }, + { + percentage: 100, + properties: { x: 100 }, + ease: "custom(M0,0 C0.1,0.2 0.3,0.9 1,1)", + }, + ]), + ]; + const { host, root } = renderPropertyLanes({ animations, onSelectSegment: vi.fn() }); + + const segments = laneEaseSegments(host, "position"); + expect(segments).toHaveLength(3); + const paths = segments.map((segment) => + revealEaseButton(segment)?.querySelector("path")?.getAttribute("d"), + ); + expect(paths).toHaveLength(3); + expect(new Set(paths).size).toBe(3); + act(() => root.unmount()); + }); + + it("selects the destination keyframe when a hovered segment's ease button is clicked", () => { + const onSelectSegment = vi.fn(); + const { host, root } = renderPropertyLanes({ + animations: [POSITION_SEGMENT_ANIMATION], + onSelectSegment, + }); + + const button = revealEaseButton(laneEaseSegments(host, "position")[0]!); + act(() => button?.click()); + + expect(onSelectSegment).toHaveBeenCalledWith({ + animationId: "position-tween", + percentage: 50, + propertyGroup: "position", + tweenPercentage: 50, + }); + act(() => root.unmount()); + }); + + it("routes a colliding Position segment to the Position animation", () => { + const onSelectSegment = vi.fn(); + const animations = [ + POSITION_SEGMENT_ANIMATION, + animation("visual-tween", "visual", [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 50, properties: { opacity: 0.5 } }, + ]), + ]; + const { host, root } = renderPropertyLanes({ animations, onSelectSegment }); + + const button = revealEaseButton(laneEaseSegments(host, "position")[0]!); + act(() => button?.click()); + + expect(onSelectSegment.mock.calls[0]?.[0]).toMatchObject({ + animationId: "position-tween", + propertyGroup: "position", + }); + act(() => root.unmount()); + }); + + it("keeps the collapsed TimelineClipDiamonds positions and callback contract unchanged", () => { + const onClickKeyframe = vi.fn(); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + act(() => { + root.render( + , + ); + }); + const diamonds = Array.from(host.querySelectorAll("button")); + + // Unified keyframe-diamond size (LANE_H·ratio ≈ 22px, half 11) on collapsed + // clips too, so 0% sits at -11px regardless of clip-bar height. + expect(diamonds.map((diamond) => diamond.style.left)).toEqual(["-11px", "89px"]); + expect(diamonds[1]?.querySelector("path:last-child")?.getAttribute("fill")).toBe("#4ba3d2"); + act(() => { + diamonds[1]?.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, button: 0 })); + }); + expect(onClickKeyframe).toHaveBeenCalledWith(50); + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.tsx new file mode 100644 index 0000000000..bd518e7727 --- /dev/null +++ b/packages/studio/src/player/components/TimelinePropertyLanes.tsx @@ -0,0 +1,163 @@ +import type { MouseEvent as ReactMouseEvent, RefObject } from "react"; +import { + classifyPropertyGroup, + type GsapAnimation, + type PropertyGroupName, +} from "@hyperframes/core/gsap-parser"; +import { toAbsoluteTime } from "../../hooks/gsapShared"; +import { synthesizeFlatTweenKeyframes } from "../../hooks/gsapTweenSynth"; +import { TimelineDiamondLane, type TimelineDiamondKeyframe } from "./TimelineClipDiamonds"; +import { LANE_H, getTimelineLaneTop } from "./timelineLayout"; +import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; + +export interface TimelinePropertyLanesProps { + animations: readonly GsapAnimation[]; + clipStart: number; + clipDuration: number; + clipLeftPx: number; + clipWidthPx: number; + accentColor: string; + isSelected: boolean; + currentPercentage: number; + elementId: string; + selectedKeyframes: ReadonlySet; + onSelectSegment?: (target: TimelineKeyframeTarget) => void; + onClickKeyframe?: (target: TimelineKeyframeTarget) => void; + onShiftClickKeyframe?: (target: TimelineKeyframeTarget) => void; + onContextMenuKeyframe?: (e: ReactMouseEvent, target: TimelineKeyframeTarget) => void; + onMoveKeyframe?: (target: TimelineKeyframeTarget, toClipPercentage: number) => Promise; + suppressClickRef?: RefObject; +} + +function hasGroupProperty( + properties: Record, + group: PropertyGroupName, +): boolean { + return Object.keys(properties).some((property) => classifyPropertyGroup(property) === group); +} + +/** The tween's editable keyframes: its real keyframes, or the start→end pair + * synthesized for a flat tween. Empty for a tween that animates nothing. */ +function animationKeyframes(animation: GsapAnimation) { + return animation.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(animation)?.keyframes ?? []; +} + +/** A tween contributes a property lane when it has a group and at least one + * editable keyframe (real or synthesized). */ +export function animationContributesLane(animation: GsapAnimation): boolean { + return !!animation.propertyGroup && animationKeyframes(animation).length > 0; +} + +function sourceGroups(animations: readonly GsapAnimation[]) { + const groups = new Map(); + for (const animation of animations) { + if (!animation.propertyGroup || !animationContributesLane(animation)) continue; + const groupAnimations = groups.get(animation.propertyGroup) ?? []; + groupAnimations.push(animation); + groups.set(animation.propertyGroup, groupAnimations); + } + return groups; +} + +function groupKeyframes( + animations: readonly GsapAnimation[], + group: PropertyGroupName, + clipStart: number, + clipDuration: number, +): TimelineDiamondKeyframe[] { + const keyframes: TimelineDiamondKeyframe[] = []; + for (const animation of animations) { + const tweenStart = + animation.resolvedStart ?? (typeof animation.position === "number" ? animation.position : 0); + const tweenDuration = animation.duration ?? clipDuration; + for (const keyframe of animationKeyframes(animation)) { + if (!hasGroupProperty(keyframe.properties, group)) continue; + const absoluteTime = toAbsoluteTime(tweenStart, tweenDuration, keyframe.percentage); + keyframes.push({ + ...keyframe, + percentage: ((absoluteTime - clipStart) / clipDuration) * 100, + tweenPercentage: keyframe.percentage, + propertyGroup: group, + animationId: animation.id, + }); + } + } + return keyframes; +} + +export function getTimelinePropertyLanes( + animations: readonly GsapAnimation[], + clipStart: number, + clipDuration: number, +) { + if (clipDuration <= 0) return []; + return Array.from(sourceGroups(animations), ([group, groupAnimations]) => ({ + group, + animations: groupAnimations, + keyframes: groupKeyframes(groupAnimations, group, clipStart, clipDuration), + })).filter((lane) => lane.keyframes.length > 0); +} + +export function TimelinePropertyLanes({ + animations, + clipStart, + clipDuration, + clipLeftPx, + clipWidthPx, + accentColor, + isSelected, + currentPercentage, + elementId, + selectedKeyframes, + onSelectSegment, + onClickKeyframe, + onShiftClickKeyframe, + onContextMenuKeyframe, + onMoveKeyframe, + suppressClickRef, +}: TimelinePropertyLanesProps) { + if (clipWidthPx < 20 || clipDuration <= 0) return null; + const lanes = getTimelinePropertyLanes(animations, clipStart, clipDuration); + + if (lanes.length === 0) return null; + return ( + <> + {lanes.map(({ group, animations: groupAnimations, keyframes }, laneIndex) => ( +
+ +
+ ))} + + ); +} diff --git a/packages/studio/src/player/components/timelineLayout.ts b/packages/studio/src/player/components/timelineLayout.ts index 2677bf3a45..74eda4efe0 100644 --- a/packages/studio/src/player/components/timelineLayout.ts +++ b/packages/studio/src/player/components/timelineLayout.ts @@ -9,6 +9,10 @@ export const RULER_H = 24; export const CLIP_Y = 3; export const CLIP_HANDLE_W = 18; +export function getTimelineLaneTop(laneIndex: number): number { + return TRACK_H + Math.max(0, Math.trunc(laneIndex)) * LANE_H; +} + /** * Collapsed-row characterization value for the new-track INSERT band. Runtime * hit-testing uses getTimelineInsertBoundaryBand with the concrete row height. diff --git a/packages/studio/src/player/components/useAutoExpandKeyframedClips.test.tsx b/packages/studio/src/player/components/useAutoExpandKeyframedClips.test.tsx new file mode 100644 index 0000000000..fdf7ff5f08 --- /dev/null +++ b/packages/studio/src/player/components/useAutoExpandKeyframedClips.test.tsx @@ -0,0 +1,75 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot } from "react-dom/client"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { usePlayerStore } from "../store/playerStore"; +import { useAutoExpandKeyframedClips } from "./useAutoExpandKeyframedClips"; + +const studioShell = vi.hoisted(() => ({ projectId: "project-a" })); +vi.mock("../../contexts/StudioContext", () => ({ + useStudioShellContextOptional: () => studioShell, +})); + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; + studioShell.projectId = "project-a"; + usePlayerStore.getState().reset(); +}); + +const animations = new Map([ + [ + "clip-1", + [ + { + id: "position-tween", + targetSelector: "#clip-1", + method: "to", + position: 0, + duration: 1, + properties: { x: 100 }, + propertyGroup: "position", + }, + ], + ], +]); + +function AutoExpandHarness({ value }: { value: Map }) { + useAutoExpandKeyframedClips(value); + return null; +} + +describe("useAutoExpandKeyframedClips", () => { + it("preserves manual collapse within a project and expands again in a different project", () => { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const render = (projectId: string, value = new Map(animations)) => { + studioShell.projectId = projectId; + act(() => root.render()); + }; + + const projectAAnimations = new Map(animations); + render("project-a", projectAAnimations); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set(["clip-1"])); + + act(() => usePlayerStore.getState().toggleClipExpanded("clip-1")); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set()); + + const refreshedProjectAAnimations = new Map(animations); + render("project-a", refreshedProjectAAnimations); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set()); + + render("project-b", refreshedProjectAAnimations); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set()); + + render("project-b", new Map()); + render("project-b"); + expect(usePlayerStore.getState().expandedClipIds).toEqual(new Set(["clip-1"])); + + act(() => root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/useAutoExpandKeyframedClips.ts b/packages/studio/src/player/components/useAutoExpandKeyframedClips.ts new file mode 100644 index 0000000000..1ff3e1d91c --- /dev/null +++ b/packages/studio/src/player/components/useAutoExpandKeyframedClips.ts @@ -0,0 +1,36 @@ +import { useEffect, useRef } from "react"; +import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; +import { usePlayerStore } from "../store/playerStore"; +import { STUDIO_KEYFRAMES_ENABLED } from "../../components/editor/manualEditingAvailability"; +import { useStudioShellContextOptional } from "../../contexts/StudioContext"; +import { animationContributesLane } from "./TimelinePropertyLanes"; + +/** + * Keyframed clips start expanded (AE/Figma default). Auto-expands each clip the + * first time it contributes a lane — real keyframes OR a synthesizable flat tween + * — tracked per-clip so a later user collapse sticks and never bounces back open + * (and clips added later still auto-expand). + */ +export function useAutoExpandKeyframedClips(gsapAnimations: Map): void { + const expandClips = usePlayerStore((s) => s.expandClips); + const projectId = useStudioShellContextOptional()?.projectId ?? null; + const seen = useRef({ projectId, source: gsapAnimations, clips: new Set() }); + useEffect(() => { + if (!STUDIO_KEYFRAMES_ENABLED) return; + if (seen.current.projectId !== projectId) { + const sourceChanged = seen.current.source !== gsapAnimations; + seen.current = { projectId, source: gsapAnimations, clips: new Set() }; + if (!sourceChanged) return; + } else { + seen.current.source = gsapAnimations; + } + const fresh: string[] = []; + for (const [key, animations] of gsapAnimations) { + if (seen.current.clips.has(key)) continue; + if (animations.some(animationContributesLane)) fresh.push(key); + } + if (fresh.length === 0) return; + for (const key of fresh) seen.current.clips.add(key); + expandClips(fresh); + }, [gsapAnimations, expandClips, projectId]); +} From 8bb31b3949ca7320242fbeb26451512deedae9cc Mon Sep 17 00:00:00 2001 From: Miguel Angel Simon Sierra Date: Mon, 20 Jul 2026 16:52:35 +0200 Subject: [PATCH 15/25] feat(studio): add keyframe track headers --- .../components/editor/KeyframeNavigation.tsx | 48 +- .../player/components/LayerDisclosureRow.tsx | 56 ++ .../components/TimelineTrackHeader.test.tsx | 262 +++++++++ .../player/components/TimelineTrackHeader.tsx | 552 ++++++++++++++++++ .../player/components/timelineCallbacks.ts | 32 +- .../src/player/components/timelineLayout.ts | 1 + 6 files changed, 937 insertions(+), 14 deletions(-) create mode 100644 packages/studio/src/player/components/LayerDisclosureRow.tsx create mode 100644 packages/studio/src/player/components/TimelineTrackHeader.test.tsx create mode 100644 packages/studio/src/player/components/TimelineTrackHeader.tsx diff --git a/packages/studio/src/components/editor/KeyframeNavigation.tsx b/packages/studio/src/components/editor/KeyframeNavigation.tsx index c54047c305..b8016f1a6b 100644 --- a/packages/studio/src/components/editor/KeyframeNavigation.tsx +++ b/packages/studio/src/components/editor/KeyframeNavigation.tsx @@ -22,6 +22,36 @@ interface KeyframeNavigationProps { const TOLERANCE = 0.5; +interface NavigableKeyframe { + percentage: number; + tweenPercentage?: number; + properties: Record; +} + +export function getKeyframeNavigationState( + keyframes: readonly Keyframe[], + currentPercentage: number, + property?: string, +) { + const propertyKeyframes = property + ? keyframes.filter((keyframe) => property in keyframe.properties) + : keyframes; + return { + propertyKeyframes, + prevKeyframe: + propertyKeyframes + .filter((keyframe) => keyframe.percentage < currentPercentage - TOLERANCE) + .at(-1) ?? null, + nextKeyframe: + propertyKeyframes.find((keyframe) => keyframe.percentage > currentPercentage + TOLERANCE) ?? + null, + currentKeyframe: + propertyKeyframes.find( + (keyframe) => Math.abs(keyframe.percentage - currentPercentage) <= TOLERANCE, + ) ?? null, + }; +} + /** * Convert a clip-relative percentage (element lifetime, used for display/seek) to * the TWEEN-relative percentage the GSAP writer/runtime key on. The clip→tween @@ -92,18 +122,12 @@ export const KeyframeNavigation = memo(function KeyframeNavigation({ onRemoveKeyframe, onConvertToKeyframes, }: KeyframeNavigationProps) { - // Find keyframes that contain this property - const propertyKeyframes = keyframes?.filter((kf) => property in kf.properties) ?? []; - - const prevKf = - propertyKeyframes.filter((kf) => kf.percentage < currentPercentage - TOLERANCE).at(-1) ?? null; - - const nextKf = - propertyKeyframes.find((kf) => kf.percentage > currentPercentage + TOLERANCE) ?? null; - - const atCurrent = - propertyKeyframes.find((kf) => Math.abs(kf.percentage - currentPercentage) <= TOLERANCE) ?? - null; + const { + propertyKeyframes, + prevKeyframe: prevKf, + nextKeyframe: nextKf, + currentKeyframe: atCurrent, + } = getKeyframeNavigationState(keyframes ?? [], currentPercentage, property); // Diamond state let diamondState: DiamondState; diff --git a/packages/studio/src/player/components/LayerDisclosureRow.tsx b/packages/studio/src/player/components/LayerDisclosureRow.tsx new file mode 100644 index 0000000000..b630482d69 --- /dev/null +++ b/packages/studio/src/player/components/LayerDisclosureRow.tsx @@ -0,0 +1,56 @@ +import { CaretRight } from "@phosphor-icons/react"; +import type { TimelineElement } from "../store/playerStore"; +import { LABEL_COL_W, TRACK_H } from "./timelineLayout"; + +// Layer row (Figma order: disclosure ▸/▾, diamond, name) — the disclosure lives +// here, not on the clip bar, and re-expands a collapsed layer. +export function LayerDisclosureRow({ + keyframeClip, + isExpanded, + gutterBackground, + onToggleClipExpanded, +}: { + keyframeClip: TimelineElement; + isExpanded: boolean; + gutterBackground: string; + onToggleClipExpanded: () => void; +}) { + const name = keyframeClip.label ?? keyframeClip.domId ?? keyframeClip.id; + return ( +
+ + + ◇ + + {name} +
+ ); +} diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx new file mode 100644 index 0000000000..57d6fb127c --- /dev/null +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -0,0 +1,262 @@ +// @vitest-environment happy-dom + +import React, { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import type { GsapAnimation, PropertyGroupName } from "@hyperframes/core/gsap-parser"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { TimelinePropertyLanes } from "./TimelinePropertyLanes"; +import { TimelineTrackHeader } from "./TimelineTrackHeader"; +import { defaultTimelineTheme } from "./timelineTheme"; +import type { TimelineElement } from "../store/playerStore"; +import type { TimelineEditCallbacks } from "./timelineCallbacks"; +import { LABEL_COL_W } from "./timelineLayout"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +afterEach(() => { + document.body.innerHTML = ""; +}); + +const ELEMENT: TimelineElement = { + id: "clip-1", + label: "Hero card", + tag: "div", + start: 0, + duration: 2, + track: 0, +}; + +function animation( + id: string, + propertyGroup: PropertyGroupName, + keyframes: Array<{ + percentage: number; + properties: Record; + }>, +): GsapAnimation { + return { + id, + targetSelector: "#clip-1", + method: "to", + position: 0, + duration: 2, + properties: {}, + propertyGroup, + keyframes: { format: "percentage", keyframes }, + }; +} + +const POSITION = animation("position-tween", "position", [ + { percentage: 0, properties: { x: 0, y: 0 } }, + { percentage: 50, properties: { x: 100, y: 50 } }, + { percentage: 100, properties: { x: 200, y: 100 } }, +]); + +const OPACITY = animation("opacity-tween", "visual", [ + { percentage: 0, properties: { opacity: 0 } }, + { percentage: 50, properties: { opacity: 0.5 } }, + { percentage: 100, properties: { opacity: 1 } }, +]); + +interface RenderHeaderOptions { + animations?: GsapAnimation[]; + currentTime?: number; + expanded?: boolean; + onSeek?: (time: number) => void; + onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"]; +} + +function renderHeader(options: RenderHeaderOptions = {}): { + host: HTMLDivElement; + root: Root; + rerender: (next: RenderHeaderOptions) => void; +} { + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const render = (next: RenderHeaderOptions) => { + act(() => { + root.render( + , + ); + }); + }; + render(options); + return { host, root, rerender: render }; +} + +function click(host: HTMLElement, label: string) { + const button = host.querySelector(`button[aria-label="${label}"]`); + expect(button).not.toBeNull(); + act(() => button?.click()); +} + +describe("TimelineTrackHeader", () => { + it("adds and removes a keyframe on the explicitly targeted property-group tween", () => { + const onTogglePropertyGroupKeyframe = vi.fn(); + const view = renderHeader({ currentTime: 0.5, onTogglePropertyGroupKeyframe }); + + click(view.host, "Toggle Opacity keyframe"); + expect(onTogglePropertyGroupKeyframe).toHaveBeenLastCalledWith( + ELEMENT, + expect.objectContaining({ + animationId: "opacity-tween", + propertyGroup: "visual", + tweenPercentage: 25, + properties: { opacity: 0.25 }, + remove: false, + }), + ); + + view.rerender({ currentTime: 1, onTogglePropertyGroupKeyframe }); + click(view.host, "Toggle Opacity keyframe"); + expect(onTogglePropertyGroupKeyframe).toHaveBeenLastCalledWith( + ELEMENT, + expect.objectContaining({ + animationId: "opacity-tween", + propertyGroup: "visual", + tweenPercentage: 50, + properties: { opacity: 0.5 }, + remove: true, + }), + ); + expect(onTogglePropertyGroupKeyframe).not.toHaveBeenCalledWith( + ELEMENT, + expect.objectContaining({ animationId: "position-tween" }), + ); + act(() => view.root.unmount()); + }); + + it("seeks only to the selected group's adjacent keyframes", () => { + const onSeek = vi.fn(); + const view = renderHeader({ + currentTime: 1, + animations: [ + POSITION, + animation("opacity-tween", "visual", [ + { percentage: 25, properties: { opacity: 0.25 } }, + { percentage: 50, properties: { opacity: 0.5 } }, + { percentage: 75, properties: { opacity: 0.75 } }, + ]), + ], + onSeek, + }); + + click(view.host, "Next Position keyframe"); + expect(onSeek).toHaveBeenLastCalledWith(2); + click(view.host, "Previous Position keyframe"); + expect(onSeek).toHaveBeenLastCalledWith(0); + expect(onSeek).not.toHaveBeenCalledWith(1.5); + act(() => view.root.unmount()); + }); + + it("fills the toggle diamond exactly at that group's keyframe", () => { + const view = renderHeader({ currentTime: 0.5 }); + const positionToggle = view.host.querySelector( + 'button[aria-label="Toggle Position keyframe"]', + ); + expect(positionToggle?.textContent).toBe("◇"); + + view.rerender({ currentTime: 1 }); + expect( + view.host.querySelector('button[aria-label="Toggle Position keyframe"]') + ?.textContent, + ).toBe("◆"); + act(() => view.root.unmount()); + }); + + it("updates formatted group values when the playhead moves", () => { + const view = renderHeader({ currentTime: 0.5 }); + expect(view.host.querySelector('[data-property-group="position"]')?.textContent).toContain( + "50, 25", + ); + expect(view.host.querySelector('[data-property-group="visual"]')?.textContent).toContain("25%"); + + view.rerender({ currentTime: 1.5 }); + expect(view.host.querySelector('[data-property-group="position"]')?.textContent).toContain( + "150, 75", + ); + expect(view.host.querySelector('[data-property-group="visual"]')?.textContent).toContain("75%"); + act(() => view.root.unmount()); + }); + + it("disables the previous chevron at or before the group's first keyframe", () => { + const view = renderHeader({ currentTime: 0 }); + const prevAt0 = view.host.querySelector( + 'button[aria-label="Previous Position keyframe"]', + ); + expect(prevAt0).not.toBeNull(); + expect(prevAt0?.disabled).toBe(true); + + view.rerender({ currentTime: 1 }); + const prevAt1 = view.host.querySelector( + 'button[aria-label="Previous Position keyframe"]', + ); + expect(prevAt1?.disabled).toBe(false); + act(() => view.root.unmount()); + }); + + it("uses the same lane row offsets when collapsed, expanded once, and expanded multiple times", () => { + const view = renderHeader({ expanded: false }); + expect(view.host.querySelectorAll("[data-timeline-lane-top]")).toHaveLength(0); + + const assertAligned = (animations: GsapAnimation[]) => { + view.rerender({ animations }); + const lanesHost = document.createElement("div"); + document.body.append(lanesHost); + const lanesRoot = createRoot(lanesHost); + act(() => { + lanesRoot.render( + , + ); + }); + expect( + Array.from(view.host.querySelectorAll("[data-timeline-lane-top]")).map( + (row) => row.style.top, + ), + ).toEqual( + Array.from(lanesHost.querySelectorAll("[data-timeline-lane-top]")).map( + (row) => row.style.top, + ), + ); + expect( + Array.from(lanesHost.querySelectorAll("[data-timeline-property-lane]")).map( + (row) => row.style.left, + ), + ).toEqual(animations.map(() => "120px")); + act(() => lanesRoot.unmount()); + }; + + assertAligned([POSITION]); + assertAligned([POSITION, OPACITY]); + act(() => view.root.unmount()); + }); +}); diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx new file mode 100644 index 0000000000..e27812aab9 --- /dev/null +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -0,0 +1,552 @@ +import { useState } from "react"; +import { Eye, EyeSlash } from "@phosphor-icons/react"; +import { + classifyPropertyGroup, + type GsapAnimation, + type PropertyGroupName, +} from "@hyperframes/core/gsap-parser"; +import { + clipToTweenPercentage, + getKeyframeNavigationState, +} from "../../components/editor/KeyframeNavigation"; +import { Music } from "../../icons/SystemIcons"; +import { + absoluteToPercentageForAnimation, + isTimeWithinTween, + resolveTweenDuration, + resolveTweenStart, +} from "../../utils/globalTimeCompiler"; +import type { TimelineElement } from "../store/playerStore"; +import type { + TimelineEditCallbacks, + TimelinePropertyGroupKeyframeToggle, +} from "./timelineCallbacks"; +import { getTimelinePropertyLanes } from "./TimelinePropertyLanes"; +import { LayerDisclosureRow } from "./LayerDisclosureRow"; +import { LABEL_COL_W, LANE_H, getTimelineLaneTop } from "./timelineLayout"; +import type { TimelineTheme } from "./timelineTheme"; + +interface TimelineTrackHeaderProps { + trackNumber: number; + trackLabel: string; + contentOrigin: number; + /** The track's active keyframe clip (selected, else primary) — the one whose + * disclosure + property rows this header shows, whether expanded or not. */ + keyframeClip: TimelineElement | null; + isExpanded: boolean; + animations: readonly GsapAnimation[]; + currentTime: number; + isTrackHidden: boolean; + isAudioTrack: boolean; + isActive: boolean; + isHovered: boolean; + theme: TimelineTheme; + onToggleClipExpanded: () => void; + onToggleTrackHidden: TimelineEditCallbacks["onToggleTrackHidden"]; + onTogglePropertyGroupKeyframe?: TimelineEditCallbacks["onTogglePropertyGroupKeyframe"]; + onSeek?: (time: number) => void; +} + +function roundValue(value: number): string { + return String(Math.round(value * 100) / 100); +} + +function propertyValueAt( + animation: GsapAnimation, + property: string, + tweenPercentage: number, +): number | string | undefined { + const keyframes = animation.keyframes?.keyframes ?? []; + const values = keyframes + .filter((keyframe) => property in keyframe.properties) + .map((keyframe) => ({ + percentage: keyframe.percentage, + value: keyframe.properties[property], + })); + const before = values.filter((value) => value.percentage <= tweenPercentage).at(-1); + const after = values.find((value) => value.percentage >= tweenPercentage); + if (!before) return after?.value; + if (!after) return before.value; + if ( + typeof before.value !== "number" || + typeof after.value !== "number" || + before.percentage === after.percentage + ) { + return before.value; + } + const progress = (tweenPercentage - before.percentage) / (after.percentage - before.percentage); + return before.value + (after.value - before.value) * progress; +} + +function valuesAt( + animation: GsapAnimation, + group: PropertyGroupName, + tweenPercentage: number, +): Record { + const propertyNames = new Set(); + for (const keyframe of animation.keyframes?.keyframes ?? []) { + for (const property of Object.keys(keyframe.properties)) { + if (classifyPropertyGroup(property) === group) propertyNames.add(property); + } + } + const values: Record = {}; + for (const property of propertyNames) { + const value = propertyValueAt(animation, property, tweenPercentage); + if (value !== undefined) values[property] = value; + } + return values; +} + +function groupLabel(group: PropertyGroupName, properties: Record): string { + if (group === "visual" && ("opacity" in properties || "autoAlpha" in properties)) { + return "Opacity"; + } + if (group !== "other") return `${group[0]?.toUpperCase() ?? ""}${group.slice(1)}`; + const property = Object.keys(properties)[0]; + return property ? `${property[0]?.toUpperCase() ?? ""}${property.slice(1)}` : "Other"; +} + +type LaneValues = Record; + +function defaultValueReadout(values: LaneValues): string { + return Object.values(values) + .map((value) => (typeof value === "number" ? roundValue(value) : value)) + .join(", "); +} + +function positionValueReadout(values: LaneValues): string | null { + const x = values.x; + const y = values.y; + return typeof x === "number" && typeof y === "number" + ? `${roundValue(x)}, ${roundValue(y)}` + : null; +} + +function rotationValueReadout(values: LaneValues): string | null { + return typeof values.rotation === "number" ? `${roundValue(values.rotation)}°` : null; +} + +function visualValueReadout(values: LaneValues): string | null { + const opacity = values.opacity ?? values.autoAlpha; + return typeof opacity === "number" + ? `${roundValue(Math.abs(opacity) <= 1 ? opacity * 100 : opacity)}%` + : null; +} + +const GROUP_VALUE_READOUTS: Partial< + Record string | null> +> = { + position: positionValueReadout, + rotation: rotationValueReadout, + visual: visualValueReadout, +}; + +function valueReadout(group: PropertyGroupName, values: Record): string { + return GROUP_VALUE_READOUTS[group]?.(values) ?? defaultValueReadout(values); +} + +function VisibilityButton({ + hidden, + trackNumber, + visible, + onToggle, +}: { + hidden: boolean; + trackNumber: number; + visible: boolean; + onToggle: TimelineEditCallbacks["onToggleTrackHidden"]; +}) { + if (!visible) return - {name} + + {name} +
); } diff --git a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx index 57d6fb127c..094b54a5f6 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.test.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.test.tsx @@ -167,6 +167,37 @@ describe("TimelineTrackHeader", () => { act(() => view.root.unmount()); }); + // The lane header sits inside the track row, whose own click handler selects + // the track. Every control in the label column has to own its click, or + // seeking to a keyframe also reselects whatever is behind the header. + it("keeps lane-header control clicks off the ancestor track row", () => { + const onAncestorClick = vi.fn(); + const view = renderHeader({ + currentTime: 1, + onSeek: vi.fn(), + onTogglePropertyGroupKeyframe: vi.fn(), + }); + // React 18 delegates from the root container, so an ancestor of it is where + // a leaked click actually shows up. + document.body.addEventListener("click", onAncestorClick); + + // Every control in the lane's label column, found by row rather than by + // label, so a wording change to one button can't silently drop it here. + const controls = view.host.querySelectorAll( + '[data-property-group="position"] button', + ); + expect(controls.length).toBeGreaterThanOrEqual(3); + for (const button of controls) { + act(() => { + button.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + } + + document.body.removeEventListener("click", onAncestorClick); + expect(onAncestorClick).not.toHaveBeenCalled(); + act(() => view.root.unmount()); + }); + it("fills the toggle diamond exactly at that group's keyframe", () => { const view = renderHeader({ currentTime: 0.5 }); const positionToggle = view.host.querySelector( diff --git a/packages/studio/src/player/components/TimelineTrackHeader.tsx b/packages/studio/src/player/components/TimelineTrackHeader.tsx index e27812aab9..c0fac5b494 100644 --- a/packages/studio/src/player/components/TimelineTrackHeader.tsx +++ b/packages/studio/src/player/components/TimelineTrackHeader.tsx @@ -197,7 +197,11 @@ function LegacyTrackHeader({ {isAudioTrack && (