fix(studio): pass the whole keyframe identity out of the diamond menu

The context menu handed its actions loose positional arguments, so the
overlay adapters forwarded the percentage alone and dropped the property
group, tween percentage, and animation id the menu had resolved. The target
then fell back to first-match-by-percentage and deleted or retimed the wrong
animation whenever two collide at one percentage. Every action now carries a
TimelineKeyframeTarget, which leaves no adapter shape that can drop it.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-28 00:25:41 +02:00
parent f5d3c7a3d1
commit eddc5e9224
4 changed files with 92 additions and 32 deletions
@@ -520,9 +520,13 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
<KeyframeDiamondContextMenu
state={kfMenu}
onClose={() => 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)
}
/>
)}
</>
@@ -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<Record<string, unknown>>) {
const host = document.createElement("div");
document.body.appendChild(host);
const root = createRoot(host);
act(() =>
root.render(
<KeyframeDiamondContextMenu
state={state}
onClose={() => {}}
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);
});
});
@@ -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();
}}
>
@@ -106,12 +106,12 @@ export function TimelineOverlays({
<KeyframeDiamondContextMenu
state={kfContextMenu}
onClose={() => 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) => {