fix(studio): scope timeline context targets (#2708)

This commit is contained in:
Miguel Ángel
2026-08-03 21:19:49 -07:00
committed by GitHub
parent 4e7fcf7f2a
commit 423c5ffadb
14 changed files with 455 additions and 57 deletions
@@ -1,7 +1,7 @@
// @vitest-environment happy-dom
import { act } from "react";
import { createRoot } from "react-dom/client";
import { describe, expect, it, vi } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { TimelineElement } from "../store/playerStore";
import {
KeyframeDiamondContextMenu,
@@ -10,6 +10,10 @@ import {
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
afterEach(() => {
document.body.innerHTML = "";
});
const element = { id: "box", start: 0, duration: 2, track: 0 } as unknown as TimelineElement;
const state: KeyframeDiamondContextMenuState = {
@@ -87,4 +91,15 @@ describe("KeyframeDiamondContextMenu", () => {
act(() => root.unmount());
host.remove();
});
// The layer-wide delete takes every keyframed tween. Opened from a diamond it
// has to stay on that diamond's own lane, or right-clicking the opacity lane
// silently clears position too.
it("deletes all keyframes from the animation that opened the menu", () => {
const onDeleteAll = vi.fn();
clickMenuItem("Delete All Keyframes", { onDeleteAll });
expect(onDeleteAll).toHaveBeenCalledExactlyOnceWith(element, "box-to-1-position");
});
});
@@ -7,6 +7,8 @@ import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
export interface KeyframeDiamondContextMenuState {
x: number;
y: number;
/** Timeline project session that created this portaled target. */
sessionEpoch?: number;
element: TimelineElement;
elementId: string;
percentage: number;
@@ -23,7 +25,7 @@ interface KeyframeDiamondContextMenuProps {
* floor in removeMotionPathPointInScript): an entry that silently no-ops is
* worse than no entry. */
onDelete?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
onDeleteAll: (element: TimelineElement) => void;
onDeleteAll: (element: TimelineElement, animationId?: string) => void;
/** Retime the keyframe to the current playhead, preserving its value + ease. */
onMoveToPlayhead?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void;
}
@@ -93,7 +95,7 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe
type="button"
className="w-full flex items-center gap-2 px-3 py-1.5 text-xs text-red-400 hover:bg-neutral-800 cursor-pointer text-left"
onClick={() => {
onDeleteAll(state.element);
onDeleteAll(state.element, state.animationId);
onClose();
}}
>
@@ -12,7 +12,7 @@ import { TimelineEmptyState } from "./TimelineEmptyState";
import { TimelineCanvas } from "./TimelineCanvas";
import { type KeyframeDiamondContextMenuState } from "./KeyframeDiamondContextMenu";
import { useTimelineClipDrag } from "./useTimelineClipDrag";
import { TimelineOverlays } from "./TimelineOverlays";
import { TimelineOverlays, type ClipContextMenuState } from "./TimelineOverlays";
import { useTimelineEditPinning } from "./useTimelineEditPinning";
import { useTimelineStackingSync } from "./useTimelineStackingSync";
import { useTimelineGeometry } from "./useTimelineGeometry";
@@ -141,11 +141,7 @@ export const Timeline = memo(function Timeline({
const shiftHeld = useTimelineShiftModifier();
const [showPopover, setShowPopover] = useState(false);
const [kfContextMenu, setKfContextMenu] = useState<KeyframeDiamondContextMenuState | null>(null);
const [clipContextMenu, setClipContextMenu] = useState<{
x: number;
y: number;
element: TimelineElement;
} | null>(null);
const [clipContextMenu, setClipContextMenu] = useState<ClipContextMenuState | null>(null);
const setContainerRef = useCallback((el: HTMLDivElement | null) => {
containerRef.current = el;
}, []);
@@ -557,7 +553,12 @@ export const Timeline = memo(function Timeline({
setSelectedElementId(el.key ?? el.id);
onSelectElement?.(el);
dismissGapMenu();
setClipContextMenu({ x: e.clientX, y: e.clientY, element: el });
setClipContextMenu({
x: e.clientX,
y: e.clientY,
element: el,
sessionEpoch: usePlayerStore.getState().timelineSessionEpoch,
});
}}
onContextMenuLane={(e, track, time) => {
if (draggedClip?.started || resizingClip) return;
@@ -568,6 +569,8 @@ export const Timeline = memo(function Timeline({
{activeTool === "razor" && razorGuideX !== null && <TimelineRazorGuide x={razorGuideX} />}
</div>
<TimelineOverlays
elements={expandedElements}
elementsRef={expandedElementsRef}
theme={theme}
showShortcutHint={showShortcutHint}
showPopover={showPopover}
@@ -0,0 +1,54 @@
import { describe, expect, it } from "vitest";
import type { TimelineElement } from "../store/playerStore";
import { resolveTimelineContextElement } from "./TimelineOverlays";
const captured: TimelineElement = {
id: "child",
key: "parent::child",
tag: "div",
start: 1,
duration: 2,
track: 3,
};
describe("resolveTimelineContextElement", () => {
it("returns the current expanded model instead of the captured snapshot", () => {
const current = { ...captured, start: 4, track: 7 };
expect(
resolveTimelineContextElement({
capturedElement: captured,
targetSessionEpoch: 2,
sessionEpoch: 2,
selectedElementId: "parent::child",
elements: [current],
}),
).toBe(current);
});
it("resolves synthetic expanded children that are absent from raw store elements", () => {
expect(
resolveTimelineContextElement({
capturedElement: captured,
targetSessionEpoch: 2,
sessionEpoch: 2,
selectedElementId: "parent::child",
elements: [captured],
}),
).toBe(captured);
});
it("rejects stale sessions, changed selection, and removed elements", () => {
const input = {
capturedElement: captured,
targetSessionEpoch: 2,
sessionEpoch: 2,
selectedElementId: "parent::child",
elements: [captured],
};
expect(resolveTimelineContextElement({ ...input, sessionEpoch: 3 })).toBeNull();
expect(resolveTimelineContextElement({ ...input, selectedElementId: "other" })).toBeNull();
expect(resolveTimelineContextElement({ ...input, elements: [] })).toBeNull();
});
});
@@ -1,4 +1,6 @@
import { useEffect, type MutableRefObject } from "react";
import type { TimelineElement } from "../store/playerStore";
import { usePlayerStore } from "../store/playerStore";
import type { TimelineTheme } from "./timelineTheme";
import type { TimelineRangeSelection } from "./timelineEditing";
import type { TimelineEditCallbacks } from "./timelineCallbacks";
@@ -11,10 +13,11 @@ import { ClipContextMenu } from "./ClipContextMenu";
import { TrackGapContextMenu } from "./TrackGapContextMenu";
import { TimelineShortcutHint } from "./TimelineShortcutHint";
interface ClipContextMenuState {
export interface ClipContextMenuState {
x: number;
y: number;
element: TimelineElement;
sessionEpoch: number;
}
/** Resolved model for the empty-lane-space (track gap) context menu. */
@@ -28,6 +31,8 @@ interface TrackGapContextMenuState {
}
interface TimelineOverlaysProps {
elements: readonly TimelineElement[];
elementsRef: MutableRefObject<readonly TimelineElement[]>;
theme: TimelineTheme;
showShortcutHint: boolean;
showPopover: boolean;
@@ -52,10 +57,49 @@ interface TimelineOverlaysProps {
onHoverGapAction: (action: "close-gap" | "close-all" | null) => void;
}
interface TimelineContextTargetInput {
capturedElement: TimelineElement;
targetSessionEpoch: number | undefined;
sessionEpoch: number;
selectedElementId: string | null;
elements: readonly TimelineElement[];
}
/** The captured project session and current selection jointly own a context target. */
export function resolveTimelineContextElement({
capturedElement,
targetSessionEpoch,
sessionEpoch,
selectedElementId,
elements,
}: TimelineContextTargetInput): TimelineElement | null {
const identity = capturedElement.key ?? capturedElement.id;
if (targetSessionEpoch !== sessionEpoch) return null;
if (selectedElementId !== identity) return null;
return elements.find((element) => (element.key ?? element.id) === identity) ?? null;
}
function readTimelineContextElement(
capturedElement: TimelineElement,
targetSessionEpoch: number | undefined,
elements: readonly TimelineElement[],
): TimelineElement | null {
const state = usePlayerStore.getState();
return resolveTimelineContextElement({
capturedElement,
targetSessionEpoch,
sessionEpoch: state.timelineSessionEpoch,
selectedElementId: state.selectedElementId,
elements,
});
}
// The timeline's floating overlays, rendered as siblings above the scroll area:
// the shortcut hint, the range-edit popover, the keyframe-diamond context menu,
// and the clip context menu.
export function TimelineOverlays({
elements,
elementsRef,
theme,
showShortcutHint,
showPopover,
@@ -79,6 +123,39 @@ export function TimelineOverlays({
onCloseAllTrackGaps,
onHoverGapAction,
}: TimelineOverlaysProps) {
const selectedElementId = usePlayerStore((state) => state.selectedElementId);
const sessionEpoch = usePlayerStore((state) => state.timelineSessionEpoch);
const kfTargetSessionEpoch = kfContextMenu?.sessionEpoch;
const clipTargetSessionEpoch = clipContextMenu?.sessionEpoch;
const keyframeElement = kfContextMenu
? resolveTimelineContextElement({
capturedElement: kfContextMenu.element,
targetSessionEpoch: kfTargetSessionEpoch,
sessionEpoch,
selectedElementId,
elements,
})
: null;
const clipElement = clipContextMenu
? resolveTimelineContextElement({
capturedElement: clipContextMenu.element,
targetSessionEpoch: clipTargetSessionEpoch,
sessionEpoch,
selectedElementId,
elements,
})
: null;
const readCurrentElement = (element: TimelineElement, targetSessionEpoch: number | undefined) =>
readTimelineContextElement(element, targetSessionEpoch, elementsRef.current);
useEffect(() => {
if (kfContextMenu && !keyframeElement) setKfContextMenu(null);
}, [keyframeElement, kfContextMenu, setKfContextMenu]);
useEffect(() => {
if (clipContextMenu && !clipElement) setClipContextMenu(null);
}, [clipContextMenu, clipElement, setClipContextMenu]);
return (
<>
{showShortcutHint && !showPopover && !rangeSelection && (
@@ -98,29 +175,45 @@ export function TimelineOverlays({
/>
)}
{kfContextMenu && (
{kfContextMenu && keyframeElement && (
<KeyframeDiamondContextMenu
state={kfContextMenu}
state={{ ...kfContextMenu, element: keyframeElement }}
onClose={() => setKfContextMenu(null)}
onDelete={(elId, keyframe) => onDeleteKeyframe?.(elId, keyframe)}
onDeleteAll={(element) => onDeleteAllKeyframes?.(element)}
onDelete={(...args) => {
if (!readCurrentElement(keyframeElement, kfTargetSessionEpoch)) return;
onDeleteKeyframe?.(...args);
}}
onDeleteAll={(_element, animationId) => {
const element = readCurrentElement(keyframeElement, kfTargetSessionEpoch);
if (element) onDeleteAllKeyframes?.(element, animationId);
}}
onMoveToPlayhead={
onMoveKeyframeToPlayhead ? (...args) => onMoveKeyframeToPlayhead(...args) : undefined
onMoveKeyframeToPlayhead
? (_element, ...args) => {
const element = readCurrentElement(keyframeElement, kfTargetSessionEpoch);
if (element) onMoveKeyframeToPlayhead(element, ...args);
}
: undefined
}
/>
)}
{clipContextMenu && (
{clipContextMenu && clipElement && (
<ClipContextMenu
x={clipContextMenu.x}
y={clipContextMenu.y}
element={clipContextMenu.element}
element={clipElement}
currentTime={currentTime}
onClose={() => setClipContextMenu(null)}
onSplit={(el, time) => onSplitElement?.(el, time)}
onDelete={(el) => {
onSplit={(_element, time) => {
const element = readCurrentElement(clipElement, clipTargetSessionEpoch);
if (element) onSplitElement?.(element, time);
}}
onDelete={() => {
const element = readCurrentElement(clipElement, clipTargetSessionEpoch);
if (!element) return;
pinZoomBeforeEdit();
onDeleteElement?.(el);
onDeleteElement?.(element);
}}
/>
)}
@@ -73,7 +73,8 @@ export interface TimelineEditCallbacks {
onRazorSplit?: (element: TimelineElement, splitTime: number) => Promise<void> | void;
onRazorSplitAll?: (splitTime: number) => Promise<void> | void;
onDeleteKeyframe?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
onDeleteAllKeyframes?: (element: TimelineElement) => void;
onDeleteAllKeyframes?: (element: TimelineElement, animationId?: string) => void;
onChangeKeyframeEase?: (elementId: string, percentage: number, ease: string) => void;
onMoveKeyframeToPlayhead?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void;
/** Drag-to-retime: `keyframe` identifies the dragged keyframe (its percentage
* is clip-relative), `toClipPercentage` is the neighbour-clamped drop. */
@@ -45,7 +45,7 @@ const COLLIDING_TARGET: TimelineKeyframeTarget = {
afterEach(() => {
document.body.innerHTML = "";
vi.restoreAllMocks();
usePlayerStore.setState({ focusedEaseSegment: null });
usePlayerStore.setState({ focusedEaseSegment: null, timelineSessionEpoch: 0 });
});
/**
@@ -126,4 +126,39 @@ describe("useTimelineKeyframeHandlers", () => {
expect(onSeek).toHaveBeenCalledExactlyOnceWith(2);
act(() => root.unmount());
});
it("scopes a keyframe context target to the opening timeline session", () => {
const setKfContextMenu = vi.fn();
usePlayerStore.setState({ timelineSessionEpoch: 4 });
function Harness() {
const { onContextMenuKeyframe } = useTimelineKeyframeHandlers({
expandedElements: [ELEMENT],
keyframeCache: new Map(),
setSelectedElementId: vi.fn(),
setKfContextMenu,
toggleSelectedKeyframe: vi.fn(),
});
return (
<button
type="button"
onContextMenu={(event) => onContextMenuKeyframe(event, ELEMENT.id, TARGET)}
/>
);
}
const root = mountReactHarness(<Harness />);
const button = document.querySelector("button");
expect(button).not.toBeNull();
act(() => {
button?.dispatchEvent(
new MouseEvent("contextmenu", { bubbles: true, clientX: 10, clientY: 20 }),
);
});
expect(setKfContextMenu).toHaveBeenCalledWith(
expect.objectContaining({ elementId: ELEMENT.id, sessionEpoch: 4, x: 14, y: 22 }),
);
act(() => root.unmount());
});
});
@@ -550,12 +550,13 @@ export function useTimelineKeyframeHandlers({
setKfContextMenu({
x: e.clientX + 4,
y: e.clientY + 2,
sessionEpoch: usePlayerStore.getState().timelineSessionEpoch,
element: el,
elementId: elId,
percentage: target.percentage,
tweenPercentage: target.tweenPercentage ?? kf?.tweenPercentage,
propertyGroup: target.propertyGroup,
animationId: target.animationId,
element: el,
currentEase: kf?.ease ?? kfData?.ease,
});
},
@@ -0,0 +1,64 @@
// @vitest-environment happy-dom
import React, { act } from "react";
import { createRoot } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import { useTrackGapMenu } from "./useTrackGapMenu";
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
const lane: TimelineElement[] = [
{ id: "first", tag: "div", start: 0, duration: 1, track: 0 },
{ id: "second", tag: "div", start: 3, duration: 1, track: 0 },
];
afterEach(() => {
usePlayerStore.getState().reset();
document.body.innerHTML = "";
});
describe("useTrackGapMenu", () => {
it("does not commit an anchor captured in a previous project session", () => {
usePlayerStore.setState({ elements: lane, timelineSessionEpoch: 1 });
const host = document.createElement("div");
document.body.append(host);
const root = createRoot(host);
const onMoveElement = vi.fn();
const onMoveElements = vi.fn();
let api: ReturnType<typeof useTrackGapMenu> | null = null;
function getApi(): ReturnType<typeof useTrackGapMenu> {
if (!api) throw new Error("gap menu harness did not render");
return api;
}
function Probe() {
api = useTrackGapMenu({
tracks: [[0, lane]],
expandedElementsRef: { current: lane },
trackOrderRef: { current: [0] },
onMoveElement,
onMoveElements,
});
return null;
}
act(() => root.render(<Probe />));
act(() => getApi().openGapMenu({ x: 10, y: 20, track: 0, time: 2 }));
expect(getApi().gapMenuModel).not.toBeNull();
const staleCloseTrackGap = getApi().closeTrackGap;
const staleCloseAllTrackGaps = getApi().closeAllTrackGaps;
act(() => {
usePlayerStore.setState({ timelineSessionEpoch: 2 });
staleCloseTrackGap();
staleCloseAllTrackGaps();
});
expect(getApi().gapMenuModel).toBeNull();
expect(onMoveElement).not.toHaveBeenCalled();
expect(onMoveElements).not.toHaveBeenCalled();
act(() => root.unmount());
});
});
@@ -1,4 +1,4 @@
import { useCallback, useMemo, useState, type MutableRefObject } from "react";
import { useCallback, useEffect, useMemo, useState, type MutableRefObject } from "react";
import { usePlayerStore, type TimelineElement } from "../store/playerStore";
import type { DragCommitDeps } from "./timelineClipDragCommit";
import {
@@ -21,6 +21,7 @@ interface TrackGapMenuAnchor {
y: number;
track: number;
time: number;
sessionEpoch: number;
}
/** Gap strips to paint on one lane while a menu row is hovered. */
@@ -32,9 +33,9 @@ export interface TrackGapHighlight {
/**
* Track-gap context menu (right-click on empty lane space) — state, the
* derived menu model, and the two commit actions. Extracted from Timeline.tsx
* as a cohesive unit (600-line studio cap); behavior identical.
* as a cohesive unit (600-line studio cap).
*
* Only the ANCHOR (and the hovered row) is state; the menu model (gap under
* Only the session-stamped ANCHOR (and the hovered row) is state; the menu model (gap under
* the pointer, compaction, movability) derives from live `tracks` so an open
* menu reflects concurrent edits. `gapHighlight` — the strips TimelineCanvas
* paints while "Close gap" / "Close all gaps" is hovered — derives the same
@@ -55,12 +56,16 @@ export function useTrackGapMenu({
onMoveElements: DragCommitDeps["onMoveElements"];
}) {
const updateElement = usePlayerStore((s) => s.updateElement);
const sessionEpoch = usePlayerStore((s) => s.timelineSessionEpoch);
const [gapContextMenu, setGapContextMenu] = useState<TrackGapMenuAnchor | null>(null);
const [hoveredGapAction, setHoveredGapAction] = useState<"close-gap" | "close-all" | null>(null);
const gapMenuLaneElements = useMemo(
() => (gapContextMenu ? (tracks.find(([t]) => t === gapContextMenu.track)?.[1] ?? []) : null),
[gapContextMenu, tracks],
() =>
gapContextMenu?.sessionEpoch === sessionEpoch
? (tracks.find(([t]) => t === gapContextMenu.track)?.[1] ?? [])
: null,
[gapContextMenu, sessionEpoch, tracks],
);
const gapMenuModel = useMemo(() => {
if (!gapContextMenu || !gapMenuLaneElements) return null;
@@ -99,7 +104,12 @@ export function useTrackGapMenu({
}, [gapContextMenu, gapMenuLaneElements, hoveredGapAction]);
const closeTrackGap = useCallback(() => {
if (!gapContextMenu || !gapMenuLaneElements) return;
if (
!gapContextMenu ||
!gapMenuLaneElements ||
gapContextMenu.sessionEpoch !== usePlayerStore.getState().timelineSessionEpoch
)
return;
commitCloseTrackGap(gapMenuLaneElements, gapContextMenu.time, {
elements: expandedElementsRef.current,
trackOrder: trackOrderRef.current,
@@ -117,7 +127,12 @@ export function useTrackGapMenu({
onMoveElements,
]);
const closeAllTrackGaps = useCallback(() => {
if (!gapMenuLaneElements) return;
if (
!gapContextMenu ||
!gapMenuLaneElements ||
gapContextMenu.sessionEpoch !== usePlayerStore.getState().timelineSessionEpoch
)
return;
commitCloseAllTrackGaps(gapMenuLaneElements, {
elements: expandedElementsRef.current,
trackOrder: trackOrderRef.current,
@@ -126,6 +141,7 @@ export function useTrackGapMenu({
onMoveElements,
});
}, [
gapContextMenu,
gapMenuLaneElements,
expandedElementsRef,
trackOrderRef,
@@ -134,15 +150,22 @@ export function useTrackGapMenu({
onMoveElements,
]);
const openGapMenu = useCallback((anchor: TrackGapMenuAnchor) => {
const openGapMenu = useCallback((anchor: Omit<TrackGapMenuAnchor, "sessionEpoch">) => {
setHoveredGapAction(null);
setGapContextMenu(anchor);
setGapContextMenu({
...anchor,
sessionEpoch: usePlayerStore.getState().timelineSessionEpoch,
});
}, []);
const dismissGapMenu = useCallback(() => {
setHoveredGapAction(null);
setGapContextMenu(null);
}, []);
useEffect(() => {
if (gapContextMenu && gapContextMenu.sessionEpoch !== sessionEpoch) dismissGapMenu();
}, [dismissGapMenu, gapContextMenu, sessionEpoch]);
return {
gapMenuModel,
gapHighlight,