From 3282e5e67aa09a4e5f46586a2333277b625c8e66 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 14 Jun 2026 12:41:25 -0700 Subject: [PATCH 1/4] feat(studio): drag keyframes with beat snapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keyframe diamonds are draggable with live preview and snap to the music beat grid (requires VITE_STUDIO_ENABLE_KEYFRAMES=1). Drag model: a tween start point trims the front (end fixed), an end point resizes (start fixed), an intermediate keyframe moves within the tween (adjacent segments resize, others untouched; start/end moves remap the intermediates to preserve their absolute times). The keyframe snaps to the nearest beat within ~8px, centered exactly on the dot. Reliability: the commit resolves the dragged element's selection + parsed animations on demand (awaited) instead of relying on the async DOM-edit session, picks the tween whose window contains the keyframe's original time among same-group tweens, and holds the dropped position optimistically until the cache round-trip lands. Cache clip% precision raised to 0.001% so the marker lands exactly where dropped. Pure match/plan logic + unit tests in editor/keyframeMove.ts. Co-Authored-By: Claude Opus 4.8 (1M context) Co-authored-by: Miguel Ángel --- .../src/components/StudioPreviewArea.tsx | 66 +++++--- .../components/editor/keyframeMove.test.ts | 88 +++++++++++ .../src/components/editor/keyframeMove.ts | 144 ++++++++++++++++++ .../src/hooks/useGsapSelectionHandlers.ts | 31 ++-- .../studio/src/hooks/useGsapTweenCache.ts | 4 +- .../studio/src/player/components/Timeline.tsx | 11 ++ .../src/player/components/TimelineCanvas.tsx | 8 + .../components/TimelineClipDiamonds.tsx | 84 ++++++++-- .../src/player/components/timelineEditing.ts | 28 ++++ 9 files changed, 414 insertions(+), 50 deletions(-) create mode 100644 packages/studio/src/components/editor/keyframeMove.test.ts create mode 100644 packages/studio/src/components/editor/keyframeMove.ts diff --git a/packages/studio/src/components/StudioPreviewArea.tsx b/packages/studio/src/components/StudioPreviewArea.tsx index dd6fa11aa..cfa981a3e 100644 --- a/packages/studio/src/components/StudioPreviewArea.tsx +++ b/packages/studio/src/components/StudioPreviewArea.tsx @@ -18,6 +18,8 @@ import { useDomEditContext } from "../contexts/DomEditContext"; import { TimelineEditProvider } from "../contexts/TimelineEditContext"; import type { BlockPreviewInfo } from "./sidebar/BlocksTab"; import { readStudioUiPreferences } from "../utils/studioUiPreferences"; +import { fetchParsedAnimations } from "../hooks/useGsapTweenCache"; +import { pickKeyframeTween, computeKeyframeMovePlan } from "./editor/keyframeMove"; import type { GestureRecordingState } from "./editor/GestureRecordControl"; export interface StudioPreviewAreaProps { @@ -128,6 +130,7 @@ export function StudioPreviewArea({ handleGsapAddKeyframe, handleGsapConvertToKeyframes, handleGsapDeleteAllForElement, + buildDomSelectionForTimelineElement, } = useDomEditContext(); const [snapPrefs, setSnapPrefs] = useState(() => { @@ -169,31 +172,43 @@ export function StudioPreviewArea({ if (anim.keyframes) handleGsapUpdateMeta(anim.id, { ease }); } }, - onMoveKeyframe: (_el: TimelineElement, oldPct: number, newPct: number) => { - const cacheKey = domEditSelection?.id ?? ""; - const cached = usePlayerStore.getState().keyframeCache.get(cacheKey); + // fallow-ignore-next-line complexity + onMoveKeyframe: async (_el: TimelineElement, oldPct: number, newPct: number) => { + // Resolve the dragged element's selection + parsed animations on demand + // (both awaited and cached) rather than relying on the async DOM-edit + // session being loaded for this element — that coupling made the commit + // intermittently no-op (revert) when dragging before the session caught up. + if (!projectId) return; + const sourceFile = _el.sourceFile || activeCompPath || "index.html"; + const [selection, parsed] = await Promise.all([ + buildDomSelectionForTimelineElement(_el), + fetchParsedAnimations(projectId, sourceFile), + ]); + if (!selection || !parsed) return; + + const cached = usePlayerStore.getState().keyframeCache.get(_el.key ?? _el.id); const cachedKf = cached?.keyframes.find((k) => Math.abs(k.percentage - oldPct) < 0.2); - const group = cachedKf?.propertyGroup; - const anim = - (group ? selectedGsapAnimations.find((a) => a.propertyGroup === group) : undefined) ?? - selectedGsapAnimations.find((a) => a.keyframes); - if (!anim?.keyframes) return; - const tweenOldPct = cachedKf?.tweenPercentage ?? oldPct; - const kf = anim.keyframes.keyframes.find((k) => Math.abs(k.percentage - tweenOldPct) < 0.2); - if (!kf) return; - const tweenStart = anim.resolvedStart ?? 0; - const tweenDur = anim.duration ?? 1; - const newAbsTime = _el.start + (newPct / 100) * _el.duration; - const tweenNewPct = - tweenDur > 0 - ? Math.max( - 0, - Math.min(100, Math.round(((newAbsTime - tweenStart) / tweenDur) * 1000) / 10), - ) - : 0; - handleGsapRemoveKeyframe(anim.id, tweenOldPct); - for (const [prop, val] of Object.entries(kf.properties)) { - handleGsapAddKeyframe(anim.id, tweenNewPct, prop, val); + const origAbsTime = _el.start + (oldPct / 100) * _el.duration; + const anim = pickKeyframeTween( + parsed.animations, + _el, + origAbsTime, + cachedKf?.propertyGroup, + ); + if (!anim) return; + + const plan = computeKeyframeMovePlan( + anim, + cachedKf?.tweenPercentage ?? oldPct, + _el, + newPct, + ); + if (plan.meta) handleGsapUpdateMeta(anim.id, plan.meta, selection); + for (const pct of plan.removes) handleGsapRemoveKeyframe(anim.id, pct, selection); + for (const add of plan.adds) { + for (const [prop, val] of Object.entries(add.properties)) { + handleGsapAddKeyframe(anim.id, add.pct, prop, val, selection); + } } }, onToggleKeyframeAtPlayhead: (el: TimelineElement) => { @@ -231,6 +246,9 @@ export function StudioPreviewArea({ handleGsapUpdateMeta, handleGsapAddKeyframe, handleGsapConvertToKeyframes, + buildDomSelectionForTimelineElement, + projectId, + activeCompPath, ], ); diff --git a/packages/studio/src/components/editor/keyframeMove.test.ts b/packages/studio/src/components/editor/keyframeMove.test.ts new file mode 100644 index 000000000..808519f62 --- /dev/null +++ b/packages/studio/src/components/editor/keyframeMove.test.ts @@ -0,0 +1,88 @@ +import { describe, it, expect } from "vitest"; +import { pickKeyframeTween, computeKeyframeMovePlan } from "./keyframeMove"; + +const flat = (id: string, target: string, position: number, duration: number, group?: string) => ({ + id, + targetSelector: target, + position, + duration, + resolvedStart: position, + propertyGroup: group, +}); + +const el = { start: 0, duration: 10, domId: "box", selector: "#box" }; + +describe("pickKeyframeTween", () => { + it("matches by the element's selector", () => { + const anims = [flat("a", "#other", 0, 5), flat("b", "#box", 2, 3)]; + expect(pickKeyframeTween(anims, el, 3, undefined)?.id).toBe("b"); + }); + + it("prefers the dragged keyframe's property group", () => { + const anims = [flat("pos", "#box", 0, 8, "position"), flat("vis", "#box", 0, 8, "visual")]; + expect(pickKeyframeTween(anims, el, 1, "visual")?.id).toBe("vis"); + }); + + it("among same-group tweens picks the one whose window contains the original time", () => { + const fadeIn = flat("in", "#box", 1, 1, "visual"); + const fadeOut = flat("out", "#box", 8, 1, "visual"); + expect(pickKeyframeTween([fadeIn, fadeOut], el, 8.5, "visual")?.id).toBe("out"); + expect(pickKeyframeTween([fadeIn, fadeOut], el, 1.2, "visual")?.id).toBe("in"); + }); + + it("returns undefined when there are no tweens", () => { + expect(pickKeyframeTween([], el, 1, undefined)).toBeUndefined(); + }); +}); + +describe("computeKeyframeMovePlan — flat tween", () => { + const anim = flat("t", "#box", 2, 4); // window [2, 6] + + it("start point trims the front, keeping the end fixed", () => { + // newPct 30% → abs 3 → start moves to 3, duration shrinks to 3. + const plan = computeKeyframeMovePlan(anim, 0, el, 30); + expect(plan.meta).toEqual({ position: 3, duration: 3 }); + expect(plan.removes).toEqual([]); + }); + + it("end point resizes, keeping the start", () => { + // tweenOldPct 100 (end) → newPct 80% → abs 8 → duration 6, start unchanged. + const plan = computeKeyframeMovePlan(anim, 100, el, 80); + expect(plan.meta).toEqual({ position: 2, duration: 6 }); + }); +}); + +describe("computeKeyframeMovePlan — keyframe-array tween", () => { + const anim = { + id: "k", + targetSelector: "#box", + position: 0, + duration: 10, + resolvedStart: 0, + keyframes: { + keyframes: [ + { percentage: 0, properties: { x: 0 } }, + { percentage: 50, properties: { x: 50 } }, + { percentage: 100, properties: { x: 100 } }, + ], + }, + }; + + it("moves an intermediate keyframe without touching the tween or others", () => { + // mid keyframe (tweenPct 50) → newPct 70% → abs 7 → 70% of the tween. + const plan = computeKeyframeMovePlan(anim, 50, el, 70); + expect(plan.meta).toBeUndefined(); + expect(plan.removes).toEqual([50]); + expect(plan.adds).toEqual([{ pct: 70, properties: { x: 50 } }]); + }); + + it("start move remaps intermediates to preserve their absolute times", () => { + // start (tweenPct 0) → newPct 20% → abs 2 → window [2,10]. The 50% keyframe + // was at abs 5 → now (5-2)/8 = 37.5%. + const plan = computeKeyframeMovePlan(anim, 0, el, 20); + expect(plan.meta).toEqual({ position: 2, duration: 8 }); + expect(plan.removes).toContain(50); + const mid = plan.adds.find((a) => a.properties.x === 50); + expect(mid?.pct).toBeCloseTo(37.5, 1); + }); +}); diff --git a/packages/studio/src/components/editor/keyframeMove.ts b/packages/studio/src/components/editor/keyframeMove.ts new file mode 100644 index 000000000..861dfc28b --- /dev/null +++ b/packages/studio/src/components/editor/keyframeMove.ts @@ -0,0 +1,144 @@ +/** + * Pure helpers for committing a keyframe-diamond drag: pick the tween the + * dragged keyframe belongs to, and compute the GSAP mutations (tween + * position/duration and/or keyframe add/remove) for the move. Kept free of + * React/store so the timeline drag handler stays a thin orchestrator. + */ + +interface TweenLike { + id: string; + targetSelector: string; + position: number | string; + duration?: number; + resolvedStart?: number; + propertyGroup?: string; + keyframes?: { keyframes: { percentage: number; properties: Record }[] }; +} + +interface ElementWindow { + start: number; + duration: number; + domId?: string; + selector?: string; +} + +export interface KeyframeMovePlan { + /** Tween timing change (start/end point drags). */ + meta?: { position: number; duration: number }; + /** Keyframe percentages to remove, then re-add (intermediate move / remap). */ + removes: number[]; + adds: { pct: number; properties: Record }[]; +} + +const round3 = (n: number) => Math.round(n * 1000) / 1000; +const clampPct = (n: number) => Math.max(0, Math.min(100, Math.round(n * 100) / 100)); +const MIN_DUR = 0.05; + +function tweenWindow(a: TweenLike): { start: number; dur: number } { + return { + start: a.resolvedStart ?? (typeof a.position === "number" ? a.position : 0), + dur: a.duration ?? 0, + }; +} + +type Kf = { percentage: number; properties: Record }; + +/** + * Remap every keyframe except `keepIdx` from the old tween window to the new one + * so their absolute times stay fixed after a start/end resize. Returns the + * remove/add ops (empty for flat tweens, which have no intermediates). + */ +function remapKeyframes( + kfs: Kf[], + keepIdx: number, + oldStart: number, + oldDur: number, + newStart: number, + newDur: number, +): Pick { + const removes: number[] = []; + const adds: KeyframeMovePlan["adds"] = []; + if (newDur <= 0) return { removes, adds }; + for (let i = 0; i < kfs.length; i++) { + if (i === keepIdx) continue; + const k = kfs[i]!; + const absT = oldStart + (k.percentage / 100) * oldDur; + const remapped = clampPct(((absT - newStart) / newDur) * 100); + if (Math.abs(remapped - k.percentage) < 0.05) continue; + removes.push(k.percentage); + adds.push({ pct: remapped, properties: k.properties }); + } + return { removes, adds }; +} + +/** + * Pick the tween the dragged keyframe belongs to: restrict to the element's + * selector and (if known) the keyframe's property group, then choose the one + * whose time window contains — or is nearest — the keyframe's original time. + * An element can have several tweens in one group (e.g. fade-in + fade-out). + */ +export function pickKeyframeTween( + anims: T[], + el: ElementWindow, + origAbsTime: number, + group: string | undefined, +): T | undefined { + const selectors = [el.domId ? `#${el.domId}` : null, el.selector].filter(Boolean); + const forEl = anims.filter((a) => selectors.includes(a.targetSelector)); + const pool = forEl.length > 0 ? forEl : anims; + const groupPool = group ? pool.filter((a) => a.propertyGroup === group) : []; + const candidates = groupPool.length > 0 ? groupPool : pool; + if (candidates.length === 0) return undefined; + const dist = (a: T): number => { + const { start, dur } = tweenWindow(a); + if (origAbsTime >= start && origAbsTime <= start + dur) return 0; + return Math.min(Math.abs(origAbsTime - start), Math.abs(origAbsTime - (start + dur))); + }; + return candidates.reduce((best, a) => (dist(a) < dist(best) ? a : best), candidates[0]!); +} + +/** + * Compute the mutations for moving a keyframe to `newPct` (clip-relative): + * - start point → trim front (position moves, end fixed), + * - end point → resize (duration changes, start fixed), + * - intermediate → move only that keyframe; start/end moves remap the other + * keyframes so their absolute times stay put. + */ +// fallow-ignore-next-line complexity +export function computeKeyframeMovePlan( + anim: TweenLike, + tweenOldPct: number, + el: ElementWindow, + newPct: number, +): KeyframeMovePlan { + const newAbsTime = el.start + (newPct / 100) * el.duration; + const tweenStart = tweenWindow(anim).start; + const tweenDur = anim.duration ?? el.duration; + const kfs = anim.keyframes + ? anim.keyframes.keyframes.slice().sort((a, b) => a.percentage - b.percentage) + : null; + const idx = kfs ? kfs.findIndex((k) => Math.abs(k.percentage - tweenOldPct) < 0.5) : -1; + + if (kfs && idx > 0 && idx < kfs.length - 1) { + const movedPct = tweenDur > 0 ? clampPct(((newAbsTime - tweenStart) / tweenDur) * 100) : 0; + return { removes: [tweenOldPct], adds: [{ pct: movedPct, properties: kfs[idx]!.properties }] }; + } + + const isStartPoint = kfs ? idx === 0 : tweenOldPct <= 50; + let newStart = tweenStart; + let newDur = tweenDur; + if (isStartPoint) { + const end = tweenStart + tweenDur; + newStart = Math.max(0, Math.min(newAbsTime, end - MIN_DUR)); + newDur = end - newStart; + } else { + newDur = Math.max(MIN_DUR, newAbsTime - tweenStart); + } + + const windowChanged = newStart !== tweenStart || newDur !== tweenDur; + const remap = + kfs && windowChanged + ? remapKeyframes(kfs, idx, tweenStart, tweenDur, newStart, newDur) + : { removes: [], adds: [] }; + return { meta: { position: round3(newStart), duration: round3(newDur) }, ...remap }; +} diff --git a/packages/studio/src/hooks/useGsapSelectionHandlers.ts b/packages/studio/src/hooks/useGsapSelectionHandlers.ts index a25abb3b9..7a9ee09db 100644 --- a/packages/studio/src/hooks/useGsapSelectionHandlers.ts +++ b/packages/studio/src/hooks/useGsapSelectionHandlers.ts @@ -110,9 +110,14 @@ export function useGsapSelectionHandlers({ ); const handleGsapUpdateMeta = useCallback( - (animId: string, updates: { duration?: number; ease?: string; position?: number }) => { - if (!domEditSelection) return; - updateGsapMeta(domEditSelection, animId, updates); + ( + animId: string, + updates: { duration?: number; ease?: string; position?: number }, + selectionOverride?: DomEditSelection | null, + ) => { + const sel = selectionOverride ?? domEditSelection ?? lastSelectionRef.current; + if (!sel) return; + updateGsapMeta(sel, animId, updates); }, [domEditSelection, updateGsapMeta], ); @@ -191,9 +196,16 @@ export function useGsapSelectionHandlers({ ); const handleGsapAddKeyframe = useCallback( - (animId: string, percentage: number, property: string, value: number | string) => { - if (!domEditSelection) return; - addKeyframe(domEditSelection, animId, percentage, property, value); + ( + animId: string, + percentage: number, + property: string, + value: number | string, + selectionOverride?: DomEditSelection | null, + ) => { + const sel = selectionOverride ?? domEditSelection ?? lastSelectionRef.current; + if (!sel) return; + addKeyframe(sel, animId, percentage, property, value); }, [domEditSelection, addKeyframe], ); @@ -208,9 +220,10 @@ export function useGsapSelectionHandlers({ [domEditSelection, addKeyframeBatch, trackGsapHandlerFailure], ); const handleGsapRemoveKeyframe = useCallback( - (animId: string, percentage: number) => { - if (!domEditSelection) return; - removeKeyframe(domEditSelection, animId, percentage); + (animId: string, percentage: number, selectionOverride?: DomEditSelection | null) => { + const sel = selectionOverride ?? domEditSelection ?? lastSelectionRef.current; + if (!sel) return; + removeKeyframe(sel, animId, percentage); }, [domEditSelection, removeKeyframe], ); diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index 7f1d1fb82..aa5f6b2a5 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -283,9 +283,11 @@ 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) * 1000) / 10 + ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 : k.percentage; allKeyframes.push({ ...k, diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index c7b184f59..347a6907b 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -19,6 +19,7 @@ import { type KeyframeDiamondContextMenuState, } from "./KeyframeDiamondContextMenu"; import { useTimelineClipDrag } from "./useTimelineClipDrag"; +import { snapKeyframePctToBeat } from "./timelineEditing"; import { ClipContextMenu } from "./ClipContextMenu"; import { GUTTER, @@ -470,6 +471,16 @@ export const Timeline = memo(function Timeline({ onDragKeyframe={(el, oldPct, newPct) => { onMoveKeyframe?.(el, oldPct, newPct); }} + onSnapKeyframePct={(el, pct) => + snapKeyframePctToBeat(el, pct, adjustedBeatAnalysis?.beatTimes, pps) + } + onPickKeyframeElement={(el) => { + const elKey = el.key ?? el.id; + if (selectedElementId !== elKey) { + setSelectedElementId(elKey); + onSelectElement?.(el); + } + }} onContextMenuKeyframe={(e, elId, pct) => { const el = elements.find((x) => (x.key ?? x.id) === elId); if (el) { diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index e3321d61f..84fd2ad63 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -92,6 +92,10 @@ interface TimelineCanvasProps { onClickKeyframe?: (element: TimelineElement, percentage: number) => void; onShiftClickKeyframe?: (elementId: string, percentage: number) => void; onDragKeyframe?: (element: TimelineElement, oldPct: number, newPct: number) => void; + /** Snap a keyframe's clip-relative % to the nearest beat (returns unchanged when none in range). */ + onSnapKeyframePct?: (element: TimelineElement, pct: number) => number; + /** Select the element when a keyframe drag starts (loads its GSAP session). */ + onPickKeyframeElement?: (element: TimelineElement) => void; onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void; onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void; beatAnalysis?: MusicBeatAnalysis | null; @@ -140,6 +144,8 @@ export const TimelineCanvas = memo(function TimelineCanvas({ onClickKeyframe, onShiftClickKeyframe, onDragKeyframe, + onSnapKeyframePct, + onPickKeyframeElement, onContextMenuKeyframe, onContextMenuClip, beatAnalysis, @@ -436,6 +442,8 @@ export const TimelineCanvas = memo(function TimelineCanvas({ onDragKeyframe={(oldPct, newPct) => onDragKeyframe?.(previewElement, oldPct, newPct) } + snapPct={(pct) => onSnapKeyframePct?.(previewElement, pct) ?? pct} + onPickForDrag={() => onPickKeyframeElement?.(previewElement)} onContextMenuKeyframe={onContextMenuKeyframe} /> )} diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index bbe8b0ac9..39faa3685 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -1,4 +1,4 @@ -import { memo, useRef } from "react"; +import { memo, useEffect, useRef, useState } from "react"; interface KeyframeEntry { percentage: number; @@ -26,6 +26,13 @@ interface TimelineClipDiamondsProps { onShiftClickKeyframe?: (elementId: string, percentage: number) => void; onDragKeyframe?: (percentage: number, newPercentage: number) => void; onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void; + /** Snap a clip-relative percentage to the nearest beat (returns it unchanged + * when no beat is within range). Drives live beat-snapping while dragging. */ + snapPct?: (percentage: number) => number; + /** Select this element when a keyframe drag begins, so its GSAP session is + * loaded by the time the move commits (diamonds render on unselected clips + * too, and a drag suppresses the selecting click). */ + onPickForDrag?: () => void; } const DIAMOND_RATIO = 0.8; @@ -43,8 +50,32 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ onShiftClickKeyframe, onDragKeyframe, onContextMenuKeyframe, + snapPct, + onPickForDrag, }: TimelineClipDiamondsProps) { - const dragRef = useRef<{ startX: number; startPct: number } | null>(null); + // Live drag: which keyframe (by original %) is being dragged and its current + // (beat-snapped) %, so the diamond + its connecting lines follow the cursor. + const dragRef = useRef<{ origPct: number; pct: number; moved: boolean } | null>(null); + const [drag, setDrag] = useState<{ origPct: number; pct: number } | null>(null); + // Commit through the latest callback, not the one captured at pointer-down: + // selecting the element on drag-start loads its GSAP session asynchronously, + // and the commit must use the closure that sees the loaded session. + const onDragKeyframeRef = useRef(onDragKeyframe); + onDragKeyframeRef.current = onDragKeyframe; + // Optimistic hold: after a commit, keep the diamond at the dropped position + // until the cache reflects the change (the file round-trip rewrites + // keyframesData), so it doesn't flash back to the old spot in between. + const pendingRef = useRef(false); + const pendingTimerRef = useRef | null>(null); + + useEffect(() => { + if (!pendingRef.current) return; + pendingRef.current = false; + if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current); + setDrag(null); + }, [keyframesData]); + + useEffect(() => () => clearTimeout(pendingTimerRef.current ?? undefined), []); if (clipWidthPx < 20) return null; @@ -66,26 +97,44 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ const handlePointerDown = (e: React.PointerEvent, pct: number) => { if (e.button !== 0) return; e.stopPropagation(); + // Select the element up front so its GSAP session loads during the drag and + // the commit (which resolves the animation from the selection) isn't a no-op. + onPickForDrag?.(); const startX = e.clientX; + dragRef.current = { origPct: pct, pct, moved: false }; const handleMove = (me: PointerEvent) => { + const d = dragRef.current; + if (!d) return; const dx = me.clientX - startX; - if (Math.abs(dx) > 4) { - dragRef.current = { startX, startPct: pct }; - } + // 4px dead zone so a click doesn't register as a drag. + if (!d.moved && Math.abs(dx) <= 4) return; + d.moved = true; + const rawPct = Math.max(0, Math.min(100, pct + (dx / clipWidthPx) * 100)); + const snapped = snapPct ? snapPct(rawPct) : rawPct; + d.pct = snapped; + setDrag({ origPct: pct, pct: snapped }); }; - const handleUp = (ue: PointerEvent) => { + const handleUp = () => { document.removeEventListener("pointermove", handleMove); document.removeEventListener("pointerup", handleUp); - const start = dragRef.current; + const d = dragRef.current; dragRef.current = null; - if (!start) return; - const dx = ue.clientX - start.startX; - const dPct = (dx / clipWidthPx) * 100; - const newPct = Math.max(0, Math.min(100, Math.round(start.startPct + dPct))); - if (Math.abs(newPct - start.startPct) > 0.5) { - onDragKeyframe?.(start.startPct, newPct); + const willCommit = !!(d && d.moved && Math.abs(d.pct - d.origPct) > 0.5); + if (willCommit && d) { + // Hold the dropped position optimistically; the effect clears it once the + // cache round-trip lands (fallback timeout in case it never does). + pendingRef.current = true; + setDrag({ origPct: d.origPct, pct: d.pct }); + if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current); + pendingTimerRef.current = setTimeout(() => { + pendingRef.current = false; + setDrag(null); + }, 2000); + onDragKeyframeRef.current?.(d.origPct, d.pct); + } else { + setDrag(null); } }; @@ -93,13 +142,16 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ document.addEventListener("pointerup", handleUp); }; + // Effective % for rendering: the dragged keyframe follows the (snapped) cursor. + const effPct = (p: number): number => (drag && drag.origPct === p ? drag.pct : p); + return (
{sorted.map((kf, i) => { if (i === 0) return null; const prev = sorted[i - 1]!; - const x1 = (prev.percentage / 100) * clipWidthPx; - const x2 = (kf.percentage / 100) * clipWidthPx; + const x1 = (effPct(prev.percentage) / 100) * clipWidthPx; + const x2 = (effPct(kf.percentage) / 100) * clipWidthPx; return (
{ - const leftPx = (kf.percentage / 100) * clipWidthPx - half; + const leftPx = (effPct(kf.percentage) / 100) * clipWidthPx - half; const kfKey = `${elementId}:${kf.percentage}`; const isKfSelected = selectedKeyframes.has(kfKey); const atPlayhead = isSelected && Math.abs(kf.percentage - currentPercentage) < 0.5; diff --git a/packages/studio/src/player/components/timelineEditing.ts b/packages/studio/src/player/components/timelineEditing.ts index 773f3ddaf..bcefba970 100644 --- a/packages/studio/src/player/components/timelineEditing.ts +++ b/packages/studio/src/player/components/timelineEditing.ts @@ -111,6 +111,34 @@ export function resolveTimelineMove( }; } +/** + * Snap a keyframe's clip-relative percentage to the nearest beat within ~8px, + * mapping through composition time (pct → time → nearest beat → pct). Returns + * the percentage unchanged when no beat is in range, so dragging stays free + * between beats. + */ +export function snapKeyframePctToBeat( + el: { start: number; duration: number }, + pct: number, + beatTimes: number[] | undefined, + pixelsPerSecond: number, +): number { + if (!beatTimes || beatTimes.length === 0 || el.duration <= 0) return pct; + const t = el.start + (pct / 100) * el.duration; + const snapSecs = 8 / Math.max(pixelsPerSecond, 1); + let best = t; + let bestDist = snapSecs; + for (const bt of beatTimes) { + const d = Math.abs(bt - t); + if (d < bestDist) { + bestDist = d; + best = bt; + } + } + if (best === t) return pct; + return Math.max(0, Math.min(100, ((best - el.start) / el.duration) * 100)); +} + export function resolveTimelineResize( input: TimelineResizeInput, edge: "start" | "end", From 2f655e49e19feb0b617a3c05c413be29bcfbfc21 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 14 Jun 2026 16:29:05 -0700 Subject: [PATCH 2/4] fix(studio): harden keyframe drag commit (review follow-ups) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - pickKeyframeTween no longer falls back to ALL animations on a selector mismatch — it only picks among the dragged element's own tweens, so a class/compound-selector mismatch can't edit a different element. No match → no-op. - computeKeyframeMovePlan bails to a no-op when a keyframe-array tween's dragged keyframe can't be located (stale cache / precision drift) instead of falling through to an end-point resize that silently rescaled the whole tween and re-timed every keyframe. - usePopulateKeyframeCacheForFile clipPct now uses 0.001% precision (matching useGsapAnimationsForElement) so beat-snapped keyframes from the file-wide cache also center on the dot and the two caches agree. - The optimistic drag hold only releases once the cache reflects the committed position (a keyframe near the held %), so an unrelated cache rebuild no longer flashes the diamond back to its old spot. - A drag's document listeners are cleaned up on unmount, so an unmount mid-drag (clip delete / comp switch / zoom-out) no longer leaks them. Co-Authored-By: Claude Opus 4.8 (1M context) Co-authored-by: Miguel Ángel --- .../components/editor/keyframeMove.test.ts | 13 +++++++++ .../src/components/editor/keyframeMove.ts | 15 +++++++--- .../studio/src/hooks/useGsapTweenCache.ts | 5 +++- .../components/TimelineClipDiamonds.tsx | 29 ++++++++++++++++++- 4 files changed, 56 insertions(+), 6 deletions(-) diff --git a/packages/studio/src/components/editor/keyframeMove.test.ts b/packages/studio/src/components/editor/keyframeMove.test.ts index 808519f62..d7ee679e3 100644 --- a/packages/studio/src/components/editor/keyframeMove.test.ts +++ b/packages/studio/src/components/editor/keyframeMove.test.ts @@ -33,6 +33,11 @@ describe("pickKeyframeTween", () => { it("returns undefined when there are no tweens", () => { expect(pickKeyframeTween([], el, 1, undefined)).toBeUndefined(); }); + + it("returns undefined rather than editing another element on a selector mismatch", () => { + const anims = [flat("a", "#other", 0, 5), flat("b", ".unrelated", 2, 3)]; + expect(pickKeyframeTween(anims, el, 3, undefined)).toBeUndefined(); + }); }); describe("computeKeyframeMovePlan — flat tween", () => { @@ -85,4 +90,12 @@ describe("computeKeyframeMovePlan — keyframe-array tween", () => { const mid = plan.adds.find((a) => a.properties.x === 50); expect(mid?.pct).toBeCloseTo(37.5, 1); }); + + it("is a no-op when the dragged keyframe can't be located (stale cache)", () => { + // tweenOldPct 33 matches no keyframe (0/50/100) → must NOT resize the tween. + const plan = computeKeyframeMovePlan(anim, 33, el, 70); + expect(plan.meta).toBeUndefined(); + expect(plan.removes).toEqual([]); + expect(plan.adds).toEqual([]); + }); }); diff --git a/packages/studio/src/components/editor/keyframeMove.ts b/packages/studio/src/components/editor/keyframeMove.ts index 861dfc28b..6b38ca794 100644 --- a/packages/studio/src/components/editor/keyframeMove.ts +++ b/packages/studio/src/components/editor/keyframeMove.ts @@ -85,10 +85,12 @@ export function pickKeyframeTween( ): T | undefined { const selectors = [el.domId ? `#${el.domId}` : null, el.selector].filter(Boolean); const forEl = anims.filter((a) => selectors.includes(a.targetSelector)); - const pool = forEl.length > 0 ? forEl : anims; - const groupPool = group ? pool.filter((a) => a.propertyGroup === group) : []; - const candidates = groupPool.length > 0 ? groupPool : pool; - if (candidates.length === 0) return undefined; + // Only ever pick among THIS element's tweens. Don't fall back to all + // animations — a selector mismatch (e.g. a class/compound-selector tween) + // would otherwise edit a different element's keyframes. No match → no-op. + if (forEl.length === 0) return undefined; + const groupPool = group ? forEl.filter((a) => a.propertyGroup === group) : []; + const candidates = groupPool.length > 0 ? groupPool : forEl; const dist = (a: T): number => { const { start, dur } = tweenWindow(a); if (origAbsTime >= start && origAbsTime <= start + dur) return 0; @@ -119,6 +121,11 @@ export function computeKeyframeMovePlan( : null; const idx = kfs ? kfs.findIndex((k) => Math.abs(k.percentage - tweenOldPct) < 0.5) : -1; + // Keyframe-array tween but the dragged keyframe couldn't be located (stale + // cache / precision drift): no-op rather than falling through to an end-point + // resize that would silently rescale the whole tween and re-time every key. + if (kfs && idx === -1) return { removes: [], adds: [] }; + if (kfs && idx > 0 && idx < kfs.length - 1) { const movedPct = tweenDur > 0 ? clampPct(((newAbsTime - tweenStart) / tweenDur) * 100) : 0; return { removes: [tweenOldPct], adds: [{ pct: movedPct, properties: kfs[idx]!.properties }] }; diff --git a/packages/studio/src/hooks/useGsapTweenCache.ts b/packages/studio/src/hooks/useGsapTweenCache.ts index aa5f6b2a5..de071fe8e 100644 --- a/packages/studio/src/hooks/useGsapTweenCache.ts +++ b/packages/studio/src/hooks/useGsapTweenCache.ts @@ -383,9 +383,12 @@ export function usePopulateKeyframeCacheForFile( const elDuration = timelineEl?.duration ?? 1; 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) * 1000) / 10 + ? Math.round(((absTime - elStart) / elDuration) * 100000) / 1000 : kf.percentage; return { ...kf, diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 39faa3685..65432403c 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -66,16 +66,35 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ // until the cache reflects the change (the file round-trip rewrites // keyframesData), so it doesn't flash back to the old spot in between. const pendingRef = useRef(false); + const pendingHeldPctRef = useRef(null); const pendingTimerRef = useRef | null>(null); + // Cleanup for an in-flight drag's document listeners, so an unmount mid-drag + // (clip deleted, comp switch, zoom-out → early return) doesn't leak them. + const dragCleanupRef = useRef<(() => void) | null>(null); useEffect(() => { if (!pendingRef.current) return; + // Only release the optimistic hold once the cache actually reflects the + // committed position (a keyframe near the held %). An unrelated cache + // rebuild (e.g. elementCount change) rebuilds keyframesData with the SAME + // percentages — releasing then would flash the diamond back to the old spot. + const held = pendingHeldPctRef.current; + if (held != null && !keyframesData.keyframes.some((k) => Math.abs(k.percentage - held) < 0.3)) { + return; + } pendingRef.current = false; + pendingHeldPctRef.current = null; if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current); setDrag(null); }, [keyframesData]); - useEffect(() => () => clearTimeout(pendingTimerRef.current ?? undefined), []); + useEffect( + () => () => { + clearTimeout(pendingTimerRef.current ?? undefined); + dragCleanupRef.current?.(); + }, + [], + ); if (clipWidthPx < 20) return null; @@ -119,6 +138,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ const handleUp = () => { document.removeEventListener("pointermove", handleMove); document.removeEventListener("pointerup", handleUp); + dragCleanupRef.current = null; const d = dragRef.current; dragRef.current = null; const willCommit = !!(d && d.moved && Math.abs(d.pct - d.origPct) > 0.5); @@ -126,10 +146,12 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ // Hold the dropped position optimistically; the effect clears it once the // cache round-trip lands (fallback timeout in case it never does). pendingRef.current = true; + pendingHeldPctRef.current = d.pct; setDrag({ origPct: d.origPct, pct: d.pct }); if (pendingTimerRef.current) clearTimeout(pendingTimerRef.current); pendingTimerRef.current = setTimeout(() => { pendingRef.current = false; + pendingHeldPctRef.current = null; setDrag(null); }, 2000); onDragKeyframeRef.current?.(d.origPct, d.pct); @@ -138,6 +160,11 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ } }; + dragCleanupRef.current = () => { + document.removeEventListener("pointermove", handleMove); + document.removeEventListener("pointerup", handleUp); + }; + document.addEventListener("pointermove", handleMove); document.addEventListener("pointerup", handleUp); }; From 2d20e425a8c2d3ba5e034bd109ad7db3d2d3b9b3 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 14 Jun 2026 16:46:26 -0700 Subject: [PATCH 3/4] feat(studio): shrink + lower keyframe diamonds under the beat strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a clip's track shows the beat-dot strip (the top band), its keyframe diamonds and connecting lines render at 45% size and centered in the region below the band, so they don't collide with the dots. Full size and vertically centered otherwise. Co-Authored-By: Claude Opus 4.8 (1M context) Co-authored-by: Miguel Ángel --- .../studio/src/player/components/BeatStrip.tsx | 2 +- .../src/player/components/TimelineCanvas.tsx | 13 ++++++++++--- .../src/player/components/TimelineClipDiamonds.tsx | 14 +++++++++++--- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/studio/src/player/components/BeatStrip.tsx b/packages/studio/src/player/components/BeatStrip.tsx index 5dbb4add7..dc78e58fa 100644 --- a/packages/studio/src/player/components/BeatStrip.tsx +++ b/packages/studio/src/player/components/BeatStrip.tsx @@ -3,7 +3,7 @@ import { moveBeatCompositionTime, deleteBeatAtCompositionTime } from "../../util import { usePlayerStore } from "../store/playerStore"; import { CLIP_Y } from "./timelineLayout"; -const BEAT_BAND_H = 14; // dark band height at top of track +export const BEAT_BAND_H = 14; // dark band height at top of track const BEAT_HIT_W = 12; // grab width per beat (px) /** Hide both layers when beats are packed tighter than this (px) — too dense to read. */ diff --git a/packages/studio/src/player/components/TimelineCanvas.tsx b/packages/studio/src/player/components/TimelineCanvas.tsx index 84fd2ad63..558aaa8e2 100644 --- a/packages/studio/src/player/components/TimelineCanvas.tsx +++ b/packages/studio/src/player/components/TimelineCanvas.tsx @@ -217,6 +217,14 @@ export const TimelineCanvas = memo(function TimelineCanvas({ const ts = trackStyles.get(trackNum) ?? getTrackStyle(""); const isPendingTrack = draggedClip?.started === true && !trackOrder.includes(trackNum) && els.length === 0; + // The beat-dot strip occupies the top of this track's lane (active track, + // or the music track when nothing is selected). When shown, keyframe + // diamonds shrink + drop to the bottom half so they don't collide with it. + const beatStripOnTrack = + (beatAnalysis?.beatTimes?.length ?? 0) >= 2 && + (selectedElementId + ? els.some((e) => (e.key ?? e.id) === selectedElementId) + : els.some(isMusicTrack)); return (
{/* Beat dots on the active track (the one holding the selection), falling back to the music track when nothing is selected. */} - {(selectedElementId - ? els.some((e) => (e.key ?? e.id) === selectedElementId) - : els.some(isMusicTrack)) && ( + {beatStripOnTrack && ( a.percentage - b.percentage); const baseColor = isSelected ? accentColor : "#a3a3a3"; const baseOpacity = isSelected ? 0.4 : 0.25; @@ -185,7 +193,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ className="absolute" style={{ left: x1, - top: "50%", + top: centerY, width: x2 - x1, height: 2, transform: "translateY(-1px)", @@ -211,7 +219,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ className="absolute" style={{ left: leftPx, - top: "50%", + top: centerY, transform: "translateY(-50%)", width: diamondSize, height: diamondSize, From 3ac1da4274a15c855cd458c0081844edac96ebc6 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Sun, 14 Jun 2026 17:09:55 -0700 Subject: [PATCH 4/4] fix(studio): ignore keyframe re-drag during the optimistic-hold window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a drop, the diamond is held at its dropped position (via effPct) until the file round-trip lands, but `pct` passed to handlePointerDown still comes from props (the pre-drop position). Re-grabbing the same keyframe in that window would track the drag from a stale origin and commit against the wrong tween (or no-op via the stale-cache guard). Skip starting a drag while a hold is pending; it clears on the cache match (≤2s fallback). Click selection is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) Co-authored-by: Miguel Ángel --- .../studio/src/player/components/TimelineClipDiamonds.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index b84e80511..6f946ed9d 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -124,6 +124,11 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({ const handlePointerDown = (e: React.PointerEvent, pct: number) => { if (e.button !== 0) return; e.stopPropagation(); + // Ignore a new drag while a prior drop is still settling: `pct` comes from + // props (the pre-drop position) but the diamond is held at its dropped spot + // via effPct(), so a re-grab would track from a stale origin and commit + // against the wrong tween. The hold clears on the cache round-trip (≤2s). + if (pendingRef.current) return; // Select the element up front so its GSAP session loads during the drag and // the commit (which resolves the animation from the selection) isn't a no-op. onPickForDrag?.();