fix(studio): delete only the active element's selected keyframes (#1453)

Pressing Delete with keyframes multi-selected removed keyframes from the
wrong element. selectedKeyframes holds "<elementId>:<percentage>" keys and
can outlive the element it was built on (a clip click, keyframe click, layers
selection, or keyframe context menu changes the active element without clearing
it, and a shift-selection can span elements). deleteSelectedKeyframes parsed
only the percentage from each key and applied it to the active animation,
ignoring which element each selected keyframe belonged to, so a stale selection
deleted keyframes the user never targeted on the active element.

Extract selectedKeyframePercentagesForElement, which keeps only the percentages
whose key matches the active element id, and route the delete through it. The
common case (all selected keyframes on the active element) is unchanged; stale
cross-element keys are skipped instead of mis-applied.

Co-authored-by: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com>

packages/studio/src/App.tsx

@@ -17,6 +17,7 @@ import { useBlockHandlers } from "./hooks/useBlockHandlers";
 import { useAppHotkeys } from "./hooks/useAppHotkeys";
 import { useClipboard } from "./hooks/useClipboard";
 import { readStudioUiPreferences, writeStudioUiPreferences } from "./utils/studioUiPreferences";
+import { selectedKeyframePercentagesForElement } from "./utils/keyframeSelection";
 import { useCaptionDetection } from "./hooks/useCaptionDetection";
 import { useRenderClipContent } from "./hooks/useRenderClipContent";
 import { useConsoleErrorCapture } from "./hooks/useConsoleErrorCapture";
@@ -305,13 +306,13 @@ export function StudioApp() {
   resetKeyframesRef.current = domEditSession.handleResetSelectedElementKeyframes;
   invalidateGsapCacheRef.current = domEditSession.invalidateGsapCache;
   deleteSelectedKeyframesRef.current = () => {
-    const sk = usePlayerStore.getState().selectedKeyframes;
+    const { selectedKeyframes, selectedElementId } = usePlayerStore.getState();
     const a = domEditSession.selectedGsapAnimations.find((x) => x.keyframes);
-    if (!a || sk.size === 0) return;
-    sk.forEach((k) => {
-      const p = Number(k.split(":")[1]);
-      if (Number.isFinite(p)) domEditSession.handleGsapRemoveKeyframe(a.id, p);
-    });
+    if (!a) return;
+    // Only the active element's keyframes; a stale cross-element selection must not delete here.
+    for (const p of selectedKeyframePercentagesForElement(selectedKeyframes, selectedElementId)) {
+      domEditSession.handleGsapRemoveKeyframe(a.id, p);
+    }
   };
   useCaptionDetection({
     projectId,

packages/studio/src/utils/keyframeSelection.test.ts

@@ -0,0 +1,45 @@
+import { describe, it, expect } from "vitest";
+import { selectedKeyframePercentagesForElement } from "./keyframeSelection";
+
+describe("selectedKeyframePercentagesForElement", () => {
+  it("returns the percentages of keyframes on the active element", () => {
+    const selected = new Set(["comp#a:25", "comp#a:75"]);
+    expect(selectedKeyframePercentagesForElement(selected, "comp#a")).toEqual([25, 75]);
+  });
+
+  it("drops keyframes that belong to other elements", () => {
+    // The bug: a stale shift-selection on `comp#b` would otherwise have its
+    // percentages applied to the now-active `comp#a`, deleting the wrong keyframes.
+    const selected = new Set(["comp#a:25", "comp#b:50", "comp#b:80"]);
+    expect(selectedKeyframePercentagesForElement(selected, "comp#a")).toEqual([25]);
+  });
+
+  it("returns nothing when no key belongs to the active element", () => {
+    const selected = new Set(["comp#b:50"]);
+    expect(selectedKeyframePercentagesForElement(selected, "comp#a")).toEqual([]);
+  });
+
+  it("returns nothing when there is no active element", () => {
+    const selected = new Set(["comp#a:25"]);
+    expect(selectedKeyframePercentagesForElement(selected, null)).toEqual([]);
+  });
+
+  it("returns nothing for an empty selection", () => {
+    expect(selectedKeyframePercentagesForElement(new Set(), "comp#a")).toEqual([]);
+  });
+
+  it("splits on the final colon so element ids containing ':' still match", () => {
+    const selected = new Set(["a:b:40"]);
+    expect(selectedKeyframePercentagesForElement(selected, "a:b")).toEqual([40]);
+  });
+
+  it("skips keys without a percentage separator", () => {
+    const selected = new Set(["comp#a"]);
+    expect(selectedKeyframePercentagesForElement(selected, "comp#a")).toEqual([]);
+  });
+
+  it("skips keys whose percentage is not a finite number", () => {
+    const selected = new Set(["comp#a:abc", "comp#a:NaN", "comp#a:30"]);
+    expect(selectedKeyframePercentagesForElement(selected, "comp#a")).toEqual([30]);
+  });
+});

packages/studio/src/utils/keyframeSelection.ts

@@ -0,0 +1,29 @@
+/**
+ * Resolves which keyframe percentages a bulk operation should act on.
+ *
+ * `selectedKeyframes` holds `"<elementId>:<percentage>"` keys and can contain
+ * keyframes from more than one element — e.g. a shift-selection made before the
+ * active element changed (via a keyframe click, a clip click, the layers panel,
+ * or the keyframe context menu). A bulk delete only targets the active
+ * element's animation, so keys belonging to other elements must be dropped;
+ * otherwise their percentages get applied to the active element and remove
+ * keyframes the user never selected on it.
+ *
+ * The element id is everything before the final `:` so element ids that happen
+ * to contain `:` are handled correctly.
+ */
+export function selectedKeyframePercentagesForElement(
+  selectedKeyframes: ReadonlySet<string>,
+  activeElementId: string | null,
+): number[] {
+  if (!activeElementId) return [];
+  const percentages: number[] = [];
+  for (const key of selectedKeyframes) {
+    const separator = key.lastIndexOf(":");
+    if (separator < 0) continue;
+    if (key.slice(0, separator) !== activeElementId) continue;
+    const percentage = Number(key.slice(separator + 1));
+    if (Number.isFinite(percentage)) percentages.push(percentage);
+  }
+  return percentages;
+}