diff --git a/packages/studio/src/player/components/Timeline.tsx b/packages/studio/src/player/components/Timeline.tsx index 037b0bac1..38bef2479 100644 --- a/packages/studio/src/player/components/Timeline.tsx +++ b/packages/studio/src/player/components/Timeline.tsx @@ -28,6 +28,7 @@ import { useResolvedTimelineEditCallbacks } from "./useResolvedTimelineEditCallb import type { TimelineProps } from "./TimelineTypes"; import { useTrackGapMenu } from "./useTrackGapMenu"; import { useTimelineGapHighlights } from "./useTimelineGapHighlights"; +import { useTimelineKeyframeHandlers } from "./useTimelineKeyframeHandlers"; import { useStudioPlaybackContextOptional } from "../../contexts/StudioContext"; // Re-export pure utilities so existing imports from "./Timeline" still resolve. @@ -262,6 +263,17 @@ export const Timeline = memo(function Timeline({ const selectedKeyframes = usePlayerStore((s) => s.selectedKeyframes); const toggleSelectedKeyframe = usePlayerStore((s) => s.toggleSelectedKeyframe); + const { onClickKeyframe, onShiftClickKeyframe, onContextMenuKeyframe } = + useTimelineKeyframeHandlers({ + expandedElements, + keyframeCache, + onSelectElement, + onSeek, + setSelectedElementId, + setKfContextMenu, + toggleSelectedKeyframe, + }); + const selectedElement = useMemo( () => expandedElements.find((element) => (element.key ?? element.id) === selectedElementId) ?? null, @@ -496,40 +508,10 @@ export const Timeline = memo(function Timeline({ selectedKeyframes={selectedKeyframes} currentTime={currentTime} beatAnalysis={adjustedBeatAnalysis} - onClickKeyframe={(el, pct) => { - usePlayerStore.getState().clearSelectedKeyframes(); - const elKey = el.key ?? el.id; - setSelectedElementId(elKey); - onSelectElement?.(el); - // Select the clicked diamond (matches shift-click); cleared above so this single-selects. - toggleSelectedKeyframe(`${elKey}:${pct}`); - const absTime = el.start + (pct / 100) * el.duration; - onSeek?.(absTime); - const kfData = keyframeCache?.get(elKey); - const kf = kfData?.keyframes.find((k) => Math.abs(k.percentage - pct) < 0.5); - usePlayerStore.getState().setActiveKeyframePct(kf?.tweenPercentage ?? null); - }} - onShiftClickKeyframe={(elId, pct) => { - toggleSelectedKeyframe(`${elId}:${pct}`); - }} + onClickKeyframe={onClickKeyframe} + onShiftClickKeyframe={onShiftClickKeyframe} onMoveKeyframe={onMoveKeyframe} - onContextMenuKeyframe={(e, elId, pct) => { - const el = expandedElements.find((x) => (x.key ?? x.id) === elId); - if (!el) return; - setSelectedElementId(elId); - onSelectElement?.(el); - const kfData = keyframeCache.get(elId); - const kf = kfData?.keyframes.find((k) => Math.abs(k.percentage - pct) < 0.2); - setKfContextMenu({ - x: e.clientX + 4, - y: e.clientY + 2, - element: el, - elementId: elId, - percentage: pct, - tweenPercentage: kf?.tweenPercentage, - currentEase: kf?.ease ?? kfData?.ease, - }); - }} + onContextMenuKeyframe={onContextMenuKeyframe} onContextMenuClip={(e, el) => { e.preventDefault(); setSelectedElementId(el.key ?? el.id); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx index 2a41e7841..85804deb1 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.test.tsx @@ -4,6 +4,7 @@ import React, { act } from "react"; import { createRoot } from "react-dom/client"; import { afterEach, describe, expect, it, vi } from "vitest"; import { TimelineClipDiamonds, TimelineDiamondLane } from "./TimelineClipDiamonds"; +import { timelineKeyframeSelectionKey } from "./timelineKeyframeIdentity"; (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -90,7 +91,57 @@ describe("TimelineClipDiamonds", () => { diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0 })); }); - expect(onClickKeyframe).toHaveBeenCalledWith(50); + expect(onClickKeyframe).toHaveBeenCalledWith( + "clip-1", + expect.objectContaining({ percentage: 50 }), + ); + act(() => root.unmount()); + }); + + // The collapsed clip row and the expanded property lanes read the same cache, + // so a keyframe that carries a property group has to hash to the same key in + // both — otherwise collapsing a track silently drops the selection. + it("keys a grouped keyframe the same way collapsed as expanded", () => { + const groupedKeyframe = { + percentage: 50, + tweenPercentage: 25, + propertyGroup: "position", + animationId: "anim-1", + properties: { x: 100 }, + }; + const sharedKey = timelineKeyframeSelectionKey("clip-1", groupedKeyframe); + const host = document.createElement("div"); + document.body.append(host); + const root = createRoot(host); + const onClickKeyframe = vi.fn(); + act(() => { + root.render( + , + ); + }); + const diamond = host.querySelector('button[title="50%"]'); + // Highlighted from the shared key alone (the playhead is off-clip here). + expect(diamond?.querySelector("path:last-child")?.getAttribute("fill")).toBe("#4ba3d2"); + + act(() => { + diamond?.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0 })); + }); + expect(onClickKeyframe).toHaveBeenCalledWith("clip-1", { + percentage: 50, + tweenPercentage: 25, + propertyGroup: "position", + animationId: "anim-1", + }); act(() => root.unmount()); }); diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 10c58dfcf..cf3c516a1 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -44,9 +44,13 @@ interface TimelineClipDiamondsProps { currentPercentage: number; elementId: string; selectedKeyframes: ReadonlySet; - onClickKeyframe?: (percentage: number) => void; - onShiftClickKeyframe?: (elementId: string, percentage: number) => void; - onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void; + onClickKeyframe?: (elementId: string, keyframe: TimelineKeyframeTarget) => void; + onShiftClickKeyframe?: (elementId: string, keyframe: TimelineKeyframeTarget) => void; + onContextMenuKeyframe?: ( + e: React.MouseEvent, + elementId: string, + keyframe: TimelineKeyframeTarget, + ) => void; /** Drag-to-retime: move a keyframe to a new time, preserving its value + ease. * `keyframe` identifies the dragged keyframe (clip-relative percentage plus * whatever animation identity the row carries); `toClipPercentage` is the @@ -105,18 +109,21 @@ type DragState = { cancelled?: boolean; }; -function keyframeTarget( - keyframe: TimelineDiamondKeyframe, - groupAware: boolean, -): TimelineKeyframeTarget { - return groupAware - ? { - percentage: keyframe.percentage, - tweenPercentage: keyframe.tweenPercentage, - propertyGroup: keyframe.propertyGroup, - animationId: keyframe.animationId, - } - : { percentage: keyframe.percentage }; +/** + * The full identity of a diamond, used by every callback and by the selection + * key. Collapsed clip rows and expanded property lanes read the same cache, so + * they must hash a shared keyframe to the same key: dropping the group here for + * the collapsed row would leave a diamond selected in one view and unselected in + * the other, and would strip the animation id the retime/delete mutations use to + * pick between two animations that collide at one percentage. + */ +function keyframeTarget(keyframe: TimelineDiamondKeyframe): TimelineKeyframeTarget { + return { + percentage: keyframe.percentage, + tweenPercentage: keyframe.tweenPercentage, + propertyGroup: keyframe.propertyGroup, + animationId: keyframe.animationId, + }; } export const TimelineDiamondLane = memo(function TimelineDiamondLane({ @@ -216,16 +223,21 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ // Clip-%s of the sorted keyframes — the neighbour clamp (preview + drop) needs // the whole row to bound the dragged diamond between its immediate siblings. const sortedClipPcts = sorted.map((k) => k.percentage); - const sortedCenterXs = sorted.map((keyframe) => - Math.max(0, Math.min(clipWidthPx, (keyframe.percentage / 100) * clipWidthPx)), - ); - const markerMetrics = sortedCenterXs.map((centerX, index) => { - const previousGap = index > 0 ? centerX - sortedCenterXs[index - 1]! : Infinity; - const nextGap = - index < sortedCenterXs.length - 1 ? sortedCenterXs[index + 1]! - centerX : Infinity; + const centerXOf = (percentage: number) => + Math.max(0, Math.min(clipWidthPx, (percentage / 100) * clipWidthPx)); + // One record per diamond, carrying its own geometry, so the connector and + // button passes below read neighbours as values instead of index lookups. + const markers = sorted.map((keyframe, index) => { + const centerX = centerXOf(keyframe.percentage); + const previous = sorted[index - 1]; + const next = sorted[index + 1]; + const previousGap = previous ? centerX - centerXOf(previous.percentage) : Infinity; + const nextGap = next ? centerXOf(next.percentage) - centerX : Infinity; const nearestGap = Math.max(1, Math.min(previousGap, nextGap)); const hitWidth = Math.min(diamondSize, nearestGap); return { + keyframe, + centerX, hitWidth, visualSize: hitWidth === diamondSize ? diamondSize : Math.max(2, hitWidth - 2), }; @@ -248,25 +260,24 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ pointerEvents: "none", }} > - {sorted.map((kf, i) => { - if (i === 0) return null; - const prev = sorted[i - 1]!; - const x1 = sortedCenterXs[i - 1]!; - const x2 = sortedCenterXs[i]!; + {markers.map((marker, i) => { + const previous = markers[i - 1]; + if (!previous) return null; + const kf = marker.keyframe; + const x1 = previous.centerX; + const x2 = marker.centerX; if (x2 - x1 < 1) return null; - const connectorLeft = x1 + markerMetrics[i - 1]!.visualSize / 2; - const connectorWidth = - x2 - x1 - markerMetrics[i - 1]!.visualSize / 2 - markerMetrics[i]!.visualSize / 2; - // Group-aware target for the ease button: the segment ease is - // per-keyframe (each keyframe carries its own animationId/tweenPercentage). - // On a merged inline row the button is hidden where the segment is - // ambiguous (two source animations collide at this % with different - // eases; see easeAmbiguous) or the keyframe has no source animation id - // (runtime-scanned) so there is no tween to target. - const target = keyframeTarget(kf, true); + const connectorLeft = x1 + previous.visualSize / 2; + const connectorWidth = x2 - x1 - previous.visualSize / 2 - marker.visualSize / 2; + // The ease button targets one segment, so it needs the keyframe's own + // animationId/tweenPercentage. On a merged inline row the button is + // hidden where the segment is ambiguous (two source animations collide + // at this % with different eases; see easeAmbiguous) or the keyframe has + // no source animation id (runtime-scanned) so there is no tween to target. + const target = keyframeTarget(kf); const ease = kf.ease ?? globalEase; return ( - +
{ - const target = keyframeTarget(kf, groupAware); + {markers.map((marker, i) => { + const kf = marker.keyframe; + const target = keyframeTarget(kf); const kfKey = timelineKeyframeSelectionKey(elementId, target); // While dragging this diamond, render it at the live preview clip-%. const renderPct = preview?.kfKey === kfKey ? preview.clipPct : kf.percentage; @@ -344,7 +356,6 @@ export const TimelineDiamondLane = memo(function TimelineDiamondLane({ // The 0% diamond's left half lands in the reserved left gutter (the // content origin is inset past the label column, Figma-style) so it stays // fully visible instead of being clipped by the sticky label column. - const marker = markerMetrics[i]!; const leftPx = (renderPct / 100) * clipWidthPx - marker.hitWidth / 2; const isKfSelected = selectedKeyframes.has(kfKey); const atPlayhead = isSelected && Math.abs(kf.percentage - currentPercentage) < 0.5; @@ -565,12 +576,10 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds( props.onClickKeyframe?.(target.percentage)} - onShiftClickKeyframe={(target) => - props.onShiftClickKeyframe?.(props.elementId, target.percentage) - } + onClickKeyframe={(target) => props.onClickKeyframe?.(props.elementId, target)} + onShiftClickKeyframe={(target) => props.onShiftClickKeyframe?.(props.elementId, target)} onContextMenuKeyframe={(e, target) => - props.onContextMenuKeyframe?.(e, props.elementId, target.percentage) + props.onContextMenuKeyframe?.(e, props.elementId, target) } onMoveKeyframe={ props.onMoveKeyframe diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 8437bb64e..4ee3afc53 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -72,9 +72,13 @@ export interface TimelineLaneBaseProps { keyframeCache?: Map; selectedKeyframes: Set; currentTime: number; - onClickKeyframe?: (element: TimelineElement, percentage: number) => void; - onShiftClickKeyframe?: (elementId: string, percentage: number) => void; - onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void; + onClickKeyframe?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void; + onShiftClickKeyframe?: (elementId: string, keyframe: TimelineKeyframeTarget) => void; + onContextMenuKeyframe?: ( + e: React.MouseEvent, + elementId: string, + keyframe: TimelineKeyframeTarget, + ) => void; onMoveKeyframe?: ( elementId: string, keyframe: TimelineKeyframeTarget, @@ -474,7 +478,9 @@ export function TimelineLanes({ } elementId={elementKey} selectedKeyframes={selectedKeyframes} - onClickKeyframe={(pct) => onClickKeyframe?.(previewElement, pct)} + onClickKeyframe={(_elId, keyframe) => + onClickKeyframe?.(previewElement, keyframe) + } onShiftClickKeyframe={onShiftClickKeyframe} onContextMenuKeyframe={onContextMenuKeyframe} onMoveKeyframe={onMoveKeyframe} diff --git a/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx index 200f25d8c..387d84567 100644 --- a/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx +++ b/packages/studio/src/player/components/TimelinePropertyLanes.test.tsx @@ -424,7 +424,10 @@ describe("TimelinePropertyLanes", () => { act(() => { diamonds[1]?.dispatchEvent(new MouseEvent("pointerup", { bubbles: true, button: 0 })); }); - expect(onClickKeyframe).toHaveBeenCalledWith(50); + expect(onClickKeyframe).toHaveBeenCalledWith( + "clip-1", + expect.objectContaining({ percentage: 50 }), + ); act(() => root.unmount()); }); });