diff --git a/packages/studio/src/components/editor/MotionPathOverlay.tsx b/packages/studio/src/components/editor/MotionPathOverlay.tsx index 81a10a8d1..8af9f3129 100644 --- a/packages/studio/src/components/editor/MotionPathOverlay.tsx +++ b/packages/studio/src/components/editor/MotionPathOverlay.tsx @@ -520,9 +520,13 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({ setKfMenu(null)} - onDelete={(_elId, pct) => animId && handleGsapRemoveKeyframe(animId, pct)} + onDelete={(_elId, target) => + animId && handleGsapRemoveKeyframe(animId, target.percentage) + } onDeleteAll={() => animId && handleGsapRemoveAllKeyframes(animId)} - onMoveToPlayhead={(_elId, pct) => animId && handleGsapMoveKeyframeToPlayhead(animId, pct)} + onMoveToPlayhead={(_element, target) => + animId && handleGsapMoveKeyframeToPlayhead(animId, target.percentage) + } /> )} diff --git a/packages/studio/src/player/components/KeyframeDiamondContextMenu.test.tsx b/packages/studio/src/player/components/KeyframeDiamondContextMenu.test.tsx new file mode 100644 index 000000000..f802da680 --- /dev/null +++ b/packages/studio/src/player/components/KeyframeDiamondContextMenu.test.tsx @@ -0,0 +1,69 @@ +// @vitest-environment happy-dom +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import { describe, expect, it, vi } from "vitest"; +import type { TimelineElement } from "../store/playerStore"; +import { + KeyframeDiamondContextMenu, + type KeyframeDiamondContextMenuState, +} from "./KeyframeDiamondContextMenu"; + +(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +const element = { id: "box", start: 0, duration: 2, track: 0 } as unknown as TimelineElement; + +const state: KeyframeDiamondContextMenuState = { + x: 10, + y: 10, + element, + elementId: "box", + percentage: 50, + tweenPercentage: 25, + propertyGroup: "position", + animationId: "box-to-1-position", +}; + +function clickMenuItem(label: string, props: Partial>) { + const host = document.createElement("div"); + document.body.appendChild(host); + const root = createRoot(host); + act(() => + root.render( + {}} + onDelete={vi.fn()} + onDeleteAll={vi.fn()} + {...props} + />, + ), + ); + const button = Array.from(document.body.querySelectorAll("button")).find( + (candidate) => candidate.textContent === label, + ); + act(() => button?.click()); + act(() => root.unmount()); + host.remove(); +} + +describe("KeyframeDiamondContextMenu", () => { + // Two animations can carry a keyframe at the same clip percentage. Dropping the + // property group / tween percentage / animation id here sends the mutation back + // to first-match-by-percentage, which retimes or deletes the wrong tween. + it("hands every action the full keyframe identity, not just the percentage", () => { + const onDelete = vi.fn(); + const onMoveToPlayhead = vi.fn(); + + clickMenuItem("Delete Keyframe", { onDelete }); + clickMenuItem("Move to Playhead", { onMoveToPlayhead }); + + const target = { + percentage: 50, + tweenPercentage: 25, + propertyGroup: "position", + animationId: "box-to-1-position", + }; + expect(onDelete).toHaveBeenCalledWith("box", target); + expect(onMoveToPlayhead).toHaveBeenCalledWith(element, target); + }); +}); diff --git a/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx b/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx index 1e8275511..9dc70df9d 100644 --- a/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx +++ b/packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx @@ -2,6 +2,7 @@ import { memo } from "react"; import { createPortal } from "react-dom"; import { useContextMenuDismiss } from "../../hooks/useContextMenuDismiss"; import type { TimelineElement } from "../store/playerStore"; +import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity"; export interface KeyframeDiamondContextMenuState { x: number; @@ -18,24 +19,12 @@ export interface KeyframeDiamondContextMenuState { interface KeyframeDiamondContextMenuProps { state: KeyframeDiamondContextMenuState; onClose: () => void; - onDelete: ( - elementId: string, - percentage: number, - propertyGroup?: string, - tweenPercentage?: number, - animationId?: string, - ) => void; + onDelete: (elementId: string, target: TimelineKeyframeTarget) => void; onDeleteAll: (element: TimelineElement) => void; onChangeEase?: (elementId: string, percentage: number, ease: string) => void; onCopyProperties?: (elementId: string, percentage: number) => void; /** Retime the keyframe to the current playhead, preserving its value + ease. */ - onMoveToPlayhead?: ( - element: TimelineElement, - fromPercentage: number, - propertyGroup?: string, - tweenPercentage?: number, - animationId?: string, - ) => void; + onMoveToPlayhead?: (element: TimelineElement, target: TimelineKeyframeTarget) => void; } export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMenu({ @@ -46,6 +35,16 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe onMoveToPlayhead, }: KeyframeDiamondContextMenuProps) { const menuRef = useContextMenuDismiss(onClose); + // One target object for every action: passing the identity as loose positional + // arguments let an adapter forward the percentage alone, which drops the menu + // back to first-match-by-percentage and picks the wrong animation whenever two + // collide at the same percentage. + const target: TimelineKeyframeTarget = { + percentage: state.percentage, + tweenPercentage: state.tweenPercentage, + propertyGroup: state.propertyGroup, + animationId: state.animationId, + }; const menuWidth = 200; const menuHeight = onMoveToPlayhead ? 100 : 70; @@ -67,13 +66,7 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe // Pass clip-% — resolveKeyframeTarget keys the cache lookup on clip-% // and returns the tween-% for the mutation. Passing tween-% here would // miss the lookup on any tween whose window is shorter than the clip. - onMoveToPlayhead( - state.element, - state.percentage, - state.propertyGroup, - state.tweenPercentage, - state.animationId, - ); + onMoveToPlayhead(state.element, target); onClose(); }} > @@ -86,13 +79,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={() => { - onDelete( - state.elementId, - state.percentage, - state.propertyGroup, - state.tweenPercentage, - state.animationId, - ); + onDelete(state.elementId, target); onClose(); }} > diff --git a/packages/studio/src/player/components/TimelineOverlays.tsx b/packages/studio/src/player/components/TimelineOverlays.tsx index 44e312074..cfc2527b4 100644 --- a/packages/studio/src/player/components/TimelineOverlays.tsx +++ b/packages/studio/src/player/components/TimelineOverlays.tsx @@ -106,12 +106,12 @@ export function TimelineOverlays({ setKfContextMenu(null)} - onDelete={(elId, pct) => onDeleteKeyframe?.(elId, { percentage: pct })} + onDelete={(elId, target) => onDeleteKeyframe?.(elId, target)} onDeleteAll={(elId) => onDeleteAllKeyframes?.(elId)} onChangeEase={(elId, pct, ease) => onChangeKeyframeEase?.(elId, pct, ease)} onMoveToPlayhead={ onMoveKeyframeToPlayhead - ? (elId, pct) => onMoveKeyframeToPlayhead(elId, { percentage: pct }) + ? (element, target) => onMoveKeyframeToPlayhead(element, target) : undefined } onCopyProperties={(elId, pct) => {