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/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..9bc45eec22 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", () => { @@ -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"); @@ -98,6 +112,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 +167,87 @@ 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("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", + 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..1e806a0298 100644 --- a/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts +++ b/packages/studio/src/hooks/gsapKeyframeCacheHelpers.ts @@ -4,7 +4,8 @@ */ import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; import { usePlayerStore, type KeyframeCacheEntry } from "../player/store/playerStore"; -import { toAbsoluteTime } from "./gsapShared"; +import { idFromSelector, toClipKeyframes } from "./gsapShared"; +import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; export function updateKeyframeCacheFromParsed( animations: GsapAnimation[], @@ -15,57 +16,52 @@ 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 id = idFromSelector(anim.targetSelector); + const kfSource = + anim.keyframes?.keyframes ?? synthesizeFlatTweenKeyframes(anim)?.keyframes ?? []; + if (!id || kfSource.length === 0) continue; idsWithKeyframes.add(id); + // 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. - 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 = anim.keyframes.keyframes.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, - }; - }); + const clipKeyframes = toClipKeyframes( + kfSource, + anim, + timelineEl?.start ?? 0, + timelineEl?.duration ?? 1, + ); 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 }; - 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, 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); + for (const key of elementCacheKeys(targetPath, id)) setKeyframeCache(key, entry); + 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); } @@ -84,40 +80,56 @@ 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); } } /** * 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 } = usePlayerStore.getState(); + const { keyframeCache, gsapAnimations } = usePlayerStore.getState(); const sfPrefix = `${sourceFile}#`; - const fallbackPrefix = "index.html#"; const ids = new Set(); - for (const key of keyframeCache.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)); + for (const key of [...keyframeCache.keys(), ...gsapAnimations.keys()]) { + if (!key.startsWith(sfPrefix)) continue; + ids.add(key.slice(sfPrefix.length)); } for (const id of ids) { clearKeyframeCacheForElement(sourceFile, id); } } +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..0fb17bfb74 100644 --- a/packages/studio/src/hooks/gsapShared.test.ts +++ b/packages/studio/src/hooks/gsapShared.test.ts @@ -1,6 +1,13 @@ import { describe, it, expect } from "vitest"; import type { GsapAnimation } from "@hyperframes/core/gsap-parser"; -import { isInstantHold, parsePercentageKeyframes } from "./gsapShared"; +import { + idFromSelector, + idSelector, + isInstantHold, + parsePercentageKeyframes, + toClipKeyframes, + toClipPercentage, +} from "./gsapShared"; describe("isInstantHold", () => { const animation = (method: GsapAnimation["method"], duration?: number) => @@ -74,3 +81,86 @@ 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); + } + }); +}); + +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); + }); +}); + +describe("toClipKeyframes", () => { + // 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 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 + // 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" }); + }); +}); + +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 459f303b46..906b984196 100644 --- a/packages/studio/src/hooks/gsapShared.ts +++ b/packages/studio/src/hooks/gsapShared.ts @@ -53,8 +53,56 @@ 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")}"]`; +} + +/** + * 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 `#${selection.id}`; + if (selection.id) return idSelector(selection.id); if (selection.selector) return selection.selector; return null; } @@ -118,6 +166,16 @@ export interface ParsedPercentageKeyframes { easeEach?: string; } +function collectAnimatableKeyframeProperties(entry: object): 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 +204,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); if (Object.keys(properties).length > 0) keyframes.push({ percentage, properties }); }); return keyframes.length > 0 ? { keyframes } : null; @@ -165,12 +218,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); if (Object.keys(properties).length > 0) { keyframes.push({ percentage, properties }); } @@ -187,3 +235,57 @@ 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); + // 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( + 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/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..a3b100be4d 100644 --- a/packages/studio/src/hooks/gsapTweenSynth.ts +++ b/packages/studio/src/hooks/gsapTweenSynth.ts @@ -5,15 +5,51 @@ import type { } from "@hyperframes/core/gsap-parser"; import { PROPERTY_DEFAULTS } from "./gsapShared"; -export function deduplicateKeyframes( - keyframes: GsapPercentageKeyframe[], -): GsapPercentageKeyframe[] { - const byPct = new Map(); +/** + * 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[] { + const byPct = new Map(); for (const kf of keyframes) { const existing = byPct.get(kf.percentage); if (existing) { existing.properties = { ...existing.properties, ...kf.properties }; - if (kf.ease) existing.ease = kf.ease; + // 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; + } + // 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 } }); } @@ -41,29 +77,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/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/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.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 226ec4c194..8ec8481738 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -6,22 +6,24 @@ import { readRuntimeKeyframes, scanAllRuntimeKeyframes } from "./gsapRuntimeBrid import { clearKeyframeCacheForElement, clearKeyframeCacheForFile, + writeGsapAnimationsForElement, } from "./gsapKeyframeCacheHelpers"; -import { toAbsoluteTime } from "./gsapShared"; -import { deduplicateKeyframes, synthesizeFlatTweenKeyframes } from "./gsapTweenSynth"; - -function extractIdFromSelector(selector: string): string | null { - const match = selector.match(/^#([\w-]+)/); - return match ? match[1] : null; -} +import { idFromSelector, toAbsoluteTime, toClipPercentage, toClipKeyframes } from "./gsapShared"; +import { + deduplicateKeyframes, + isStaticPositionHold, + synthesizeFlatTweenKeyframes, +} from "./gsapTweenSynth"; /** * 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( @@ -31,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(); @@ -43,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); } } @@ -328,6 +330,12 @@ 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.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,24 +348,17 @@ 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; 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 @@ -367,17 +368,13 @@ 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, tweenPercentage: k.percentage, propertyGroup: anim.propertyGroup, + animationId: anim.id, }); } format = kf.format; @@ -459,43 +456,22 @@ 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 - // 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; - 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)) { + // kfData is already resolved (real keyframes OR a synthesized flat + // 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); - // 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. - const clipPct = - elDuration > 0 - ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 - : kf.percentage; - return { - ...kf, - percentage: clipPct, - tweenPercentage: kf.percentage, - propertyGroup: anim.propertyGroup, - }; - }); + const clipKeyframes = toClipKeyframes(kfData.keyframes, anim, elStart, elDuration); const existing = mergedByElement.get(id); if (existing) { existing.keyframes = deduplicateKeyframes([...existing.keyframes, ...clipKeyframes]); @@ -508,6 +484,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..3453d0be8f --- /dev/null +++ b/packages/studio/src/hooks/useStudioTestHooks.ts @@ -0,0 +1,53 @@ +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 () => { + // 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/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..982f42fbff 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,30 @@ 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 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: drag.originClientY, + originRow, originScrollLeft: drag.originScrollLeft, - originScrollTop: drag.originScrollTop, currentScrollLeft: scroll?.scrollLeft ?? drag.originScrollLeft, - currentScrollTop: scroll?.scrollTop ?? drag.originScrollTop, pixelsPerSecond: pps, - trackHeight: TRACK_H, 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/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), diff --git a/packages/studio/src/player/components/timelineLayout.test.ts b/packages/studio/src/player/components/timelineLayout.test.ts index 3642dea953..c21618faaf 100644 --- a/packages/studio/src/player/components/timelineLayout.test.ts +++ b/packages/studio/src/player/components/timelineLayout.test.ts @@ -2,16 +2,92 @@ import { describe, it, expect } from "vitest"; import { RULER_H, TRACK_H, + LANE_H, TRACKS_TOP_PAD, TRACKS_BOTTOM_PAD, GUTTER, TRACKS_LEFT_PAD, 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", () => { @@ -105,3 +181,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..2677bf3a45 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. @@ -43,6 +38,66 @@ export const TRACKS_BOTTOM_PAD = Math.round(TRACK_H * 1.5); */ export const TRACKS_LEFT_PAD = 48; +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); +} + /** * 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 @@ -50,17 +105,48 @@ 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; +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 @@ -321,11 +407,39 @@ export function getTimelinePlayheadLeft(time: number, pixelsPerSecond: number): ); } -export function getTimelineCanvasHeight(trackCount: number): 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(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 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(() => { diff --git a/packages/studio/src/player/store/keyframeSlice.ts b/packages/studio/src/player/store/keyframeSlice.ts new file mode 100644 index 0000000000..7a97d986ce --- /dev/null +++ b/packages/studio/src/player/store/keyframeSlice.ts @@ -0,0 +1,122 @@ +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) => { + // 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); + return { keyframeCache: next }; + }), + 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); + 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..e8c101c8fd 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,12 @@ export const usePlayerStore = create((set, get) => ({ outPoint: null, activeTool: "select", selectedKeyframes: new Set(), + expandedClipIds: new Set(), + focusedEaseSegment: null, 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 {