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

Author: miguel-heygen

packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx

@@ -317,17 +317,44 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
   });
 
   it("deletes all keyframes through the clicked non-selected element's identity", async () => {
-    const { circle, selection } = arrangeClickedCircle();
+    const circle: TimelineElement = {
+      ...element,
+      id: "circle",
+      key: "scenes/main.html#circle",
+      domId: "circle",
+      sourceFile: "scenes/main.html",
+    };
+    const circleSelection = { id: "circle", selector: "#circle", sourceFile: "scenes/main.html" };
+    const scaleAnimation: GsapAnimation = {
+      ...otherKeyframedAnimation,
+      id: "circle-to-0-scale",
+      properties: {},
+      propertyGroup: "scale",
+      keyframes: {
+        format: "percentage",
+        keyframes: [
+          { percentage: 0, properties: { scale: 1 } },
+          { percentage: 100, properties: { scale: 2 } },
+        ],
+      },
+    };
+    usePlayerStore.setState({
+      elements: [element, circle],
+      gsapAnimations: new Map([
+        ["scenes/main.html#circle", [otherKeyframedAnimation, scaleAnimation]],
+      ]),
+    });
+    mocks.actions.buildDomSelectionForTimelineElement.mockResolvedValue(circleSelection);
     const view = renderCallbacks();
 
     await act(async () => {
-      view.callbacks.onDeleteAllKeyframes?.(circle);
+      view.callbacks.onDeleteAllKeyframes?.(circle, scaleAnimation.id);
       await Promise.resolve();
     });
 
     expect(mocks.actions.handleGsapRemoveAllKeyframes).toHaveBeenCalledWith(
-      otherKeyframedAnimation.id,
-      selection,
+      scaleAnimation.id,
+      circleSelection,
     );
     view.unmount();
   });
@@ -360,6 +387,19 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
     view.unmount();
   });
 
+  it("does not delete a different lane when an explicit animation identity is stale", async () => {
+    const { circle } = arrangeClickedCircle();
+    const view = renderCallbacks();
+
+    await act(async () => {
+      view.callbacks.onDeleteAllKeyframes?.(circle, "missing-animation-id");
+      await Promise.resolve();
+    });
+
+    expect(mocks.actions.handleGsapRemoveAllKeyframes).not.toHaveBeenCalled();
+    view.unmount();
+  });
+
   it("aborts every mutation when the clicked element resolves no selection", async () => {
     const { circle } = arrangeClickedCircle();
     mocks.actions.buildDomSelectionForTimelineElement.mockResolvedValue(null);
@@ -391,6 +431,40 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
     view.unmount();
   });
 
+  it("does not delete a different keyframe when its explicit animation identity is stale", () => {
+    const view = renderCallbacks();
+
+    act(() => {
+      view.callbacks.onDeleteKeyframe?.("box", {
+        percentage: 100,
+        propertyGroup: "position",
+        tweenPercentage: 100,
+        animationId: "missing-animation-id",
+      });
+    });
+
+    expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled();
+    expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled();
+    view.unmount();
+  });
+
+  it("does not move a different keyframe when its explicit animation identity is stale", async () => {
+    const view = renderCallbacks();
+
+    await act(async () => {
+      view.callbacks.onMoveKeyframeToPlayhead?.(element, {
+        percentage: 100,
+        propertyGroup: "position",
+        tweenPercentage: 100,
+        animationId: "missing-animation-id",
+      });
+      await Promise.resolve();
+    });
+
+    expect(mocks.actions.handleGsapMoveKeyframeToPlayhead).not.toHaveBeenCalled();
+    view.unmount();
+  });
+
   it("moves a keyframe to the playhead through the clicked non-selected element's identity", async () => {
     const { circle, selection } = arrangeClickedCircle();
     const view = renderCallbacks();

packages/studio/src/components/nle/useTimelineEditCallbacks.ts

@@ -189,17 +189,22 @@ export function useTimelineEditCallbacks({
       onSplitElement: handleTimelineElementSplit,
       onRazorSplit: handleRazorSplit,
       onRazorSplitAll: handleRazorSplitAll,
-      onDeleteAllKeyframes: (element) => {
+      onDeleteAllKeyframes: (element, animationId) => {
         // Hold the element where it is (collapse keyframes to a static set) rather
         // than deleting the whole animation — deleting strands a stale GSAP base
         // that the next drag adds to, flinging the element off-screen.
         const elementKey = getTimelineElementIdentity(element);
-        // Every keyframed tween on the layer, not just the first: a layer with
-        // position AND opacity keyframes left the second one keyframed, so
-        // "Delete All Keyframes" visibly did half the job.
-        const anims = resolveElementAnimations(elementKey).filter(
-          (animation) => animation.keyframes,
-        );
+        // An explicit animation id scopes the delete to the lane whose menu was
+        // opened; without one this is the layer-wide action, and that means
+        // EVERY keyframed tween, not just the first. A layer with position AND
+        // opacity keyframes used to leave the second one keyframed, so "Delete
+        // All Keyframes" visibly did half the job. A stale id matches nothing
+        // and deletes nothing, which is the point: it never falls back to a
+        // lane the user did not click.
+        const animations = resolveElementAnimations(elementKey);
+        const anims = animationId
+          ? animations.filter((animation) => animation.id === animationId)
+          : animations.filter((animation) => animation.keyframes);
         if (anims.length === 0) return;
         void buildDomSelectionForTimelineElement(element).then(async (selection) => {
           if (!selection) return;

packages/studio/src/hooks/useGsapKeyframeOps.test.tsx

@@ -7,6 +7,8 @@ import { afterEach, describe, expect, it, vi } from "vitest";
 (globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
 
 import type { DomEditSelection } from "../components/editor/domEditingTypes";
+import type { KeyframeCacheEntry } from "../player/store/playerStore";
+import { usePlayerStore } from "../player/store/playerStore";
 import { useGsapKeyframeOps } from "./useGsapKeyframeOps";
 
 type HookApi = ReturnType<typeof useGsapKeyframeOps>;
@@ -30,6 +32,7 @@ function successfulCommitMutation() {
 
 function renderKeyframeOps(over: {
   commitMutation: (...args: unknown[]) => Promise<unknown>;
+  commitMutationSafely?: (...args: unknown[]) => Promise<void>;
   trackGsapSaveFailure: (...args: unknown[]) => void;
 }) {
   const captured: { api: HookApi | null } = { api: null };
@@ -41,7 +44,7 @@ function renderKeyframeOps(over: {
       // eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
       commitMutation: over.commitMutation as any,
       // eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
-      commitMutationSafely: (() => {}) as any,
+      commitMutationSafely: (over.commitMutationSafely ?? (async () => {})) as any,
       // eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
       trackGsapSaveFailure: over.trackGsapSaveFailure as any,
       sdkSession: null,
@@ -203,6 +206,41 @@ describe("useGsapKeyframeOps — keyframe transaction options", () => {
     );
   });
 
+  it("lets the successful commit refresh own delete-all cache invalidation", async () => {
+    let finishCommit: (() => void) | undefined;
+    const commitMutationSafely = vi.fn(
+      () =>
+        new Promise<void>((resolve) => {
+          finishCommit = resolve;
+        }),
+    );
+    const api = renderKeyframeOps({
+      commitMutation: successfulCommitMutation(),
+      commitMutationSafely,
+      trackGsapSaveFailure: vi.fn(),
+    });
+    const cached: KeyframeCacheEntry = {
+      format: "percentage",
+      keyframes: [
+        { percentage: 0, properties: { x: 0 } },
+        { percentage: 100, properties: { x: 200 } },
+      ],
+    };
+    usePlayerStore.setState({ keyframeCache: new Map([["index.html#box", cached]]) });
+
+    const pending = api.removeAllKeyframes(selection, "box-to-0-position");
+    expect(commitMutationSafely).toHaveBeenCalledWith(
+      selection,
+      { type: "remove-all-keyframes", animationId: "box-to-0-position" },
+      { label: "Remove all keyframes", softReload: true },
+    );
+    expect(usePlayerStore.getState().keyframeCache.get("index.html#box")).toBe(cached);
+
+    finishCommit?.();
+    await pending;
+    expect(usePlayerStore.getState().keyframeCache.get("index.html#box")).toBe(cached);
+  });
+
   it("threads one coalesce key through skipped convert reload and terminal batch edit", async () => {
     const commitMutation = successfulCommitMutation();
     const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure: vi.fn() });

packages/studio/src/hooks/useGsapKeyframeOps.ts

@@ -15,12 +15,7 @@ import {
 } from "../utils/sdkCutover";
 import type { KeyframeCacheEntry } from "../player/store/playerStore";
 import { commitKeyframeAtTimeImpl } from "./gsapKeyframeCommit";
-import { idFromSelector } from "./gsapShared";
-import {
-  clearKeyframeCacheForElement,
-  readKeyframeSnapshot,
-  writeKeyframeCache,
-} from "./gsapKeyframeCacheHelpers";
+import { readKeyframeSnapshot, writeKeyframeCache } from "./gsapKeyframeCacheHelpers";
 import type {
   CommitMutation,
   CommitMutationOptions,
@@ -336,11 +331,6 @@ export function useGsapKeyframeOps({
   const removeAllKeyframes = useCallback(
     async (selection: DomEditSelection, animationId: string) => {
       const targetPath = selection.sourceFile || activeCompPath || "index.html";
-      // remove-all-keyframes collapses the tween to a static hold and the commit
-      // path doesn't return parsed animations, so the keyframe cache is never
-      // refreshed — clear it here so the timeline diamonds disappear immediately.
-      const elementId = selection.id ?? idFromSelector(selection.selector);
-      if (elementId) clearKeyframeCacheForElement(targetPath, elementId);
       if (sdkSession && sdkDeps) {
         const handled = await sdkGsapRemoveAllKeyframesPersist(
           targetPath,
@@ -351,7 +341,7 @@ export function useGsapKeyframeOps({
         );
         if (cutoverCommittedOrThrow(handled)) return;
       }
-      commitMutationSafely(
+      await commitMutationSafely(
         selection,
         { type: "remove-all-keyframes", animationId },
         { label: "Remove all keyframes", softReload: true },

packages/studio/src/player/components/KeyframeDiamondContextMenu.test.tsx

@@ -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");
+  });
 });

packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx

@@ -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();
         }}
       >

packages/studio/src/player/components/Timeline.tsx

@@ -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}