From b1c1c7cfa40c68d8a4168b987c137495f3cbada1 Mon Sep 17 00:00:00 2001 From: Carlos Alcaraz <193642530+calcarazgre646@users.noreply.github.com> Date: Sun, 14 Jun 2026 22:20:42 -0300 Subject: [PATCH] fix(studio): delete only the active element's selected keyframes Pressing Delete with keyframes multi-selected removed keyframes from the wrong element. selectedKeyframes holds ":" 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. --- packages/studio/src/App.tsx | 13 +++--- .../src/utils/keyframeSelection.test.ts | 45 +++++++++++++++++++ .../studio/src/utils/keyframeSelection.ts | 29 ++++++++++++ 3 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 packages/studio/src/utils/keyframeSelection.test.ts create mode 100644 packages/studio/src/utils/keyframeSelection.ts diff --git a/packages/studio/src/App.tsx b/packages/studio/src/App.tsx index 1bd308838..39c9cc597 100644 --- a/packages/studio/src/App.tsx +++ b/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, diff --git a/packages/studio/src/utils/keyframeSelection.test.ts b/packages/studio/src/utils/keyframeSelection.test.ts new file mode 100644 index 000000000..6d8f91e77 --- /dev/null +++ b/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]); + }); +}); diff --git a/packages/studio/src/utils/keyframeSelection.ts b/packages/studio/src/utils/keyframeSelection.ts new file mode 100644 index 000000000..3ecaa36b9 --- /dev/null +++ b/packages/studio/src/utils/keyframeSelection.ts @@ -0,0 +1,29 @@ +/** + * Resolves which keyframe percentages a bulk operation should act on. + * + * `selectedKeyframes` holds `":"` 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, + 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; +}