fix(studio): open the path node menu on arc waypoints

Right-clicking a motionPath waypoint in the preview overlay opened Chrome's
own context menu on top of the editor: the handler returned before
preventDefault for every node that was not an x/y keyframe. Both node kinds
now open Studio's menu. A waypoint has no percentage of its own, so Move to
Playhead is hidden and Delete acts on the path index, matching the hover x
badge; Delete is withheld entirely on a two-anchor arc, where the writer
refuses the removal and the entry would silently do nothing.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-29 03:44:31 +02:00
parent 659e22656e
commit adb7de5358
3 changed files with 92 additions and 30 deletions
@@ -12,6 +12,7 @@ import {
KeyframeDiamondContextMenu,
type KeyframeDiamondContextMenuState,
} from "../../player/components/KeyframeDiamondContextMenu";
import type { TimelineKeyframeTarget } from "../../player/components/timelineKeyframeIdentity";
import {
commitAddKeyframe,
commitAddWaypoint,
@@ -96,9 +97,13 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
const [draft, setDraft] = useState<Draft | null>(null);
const [ghost, setGhost] = useState<{ x: number; y: number; segIndex: number } | null>(null);
const [hoverNode, setHoverNode] = useState<number | null>(null);
// Right-click context menu on a keyframe node — same delete actions as the
// timeline keyframe diamond.
const [kfMenu, setKfMenu] = useState<KeyframeDiamondContextMenuState | null>(null);
// Right-click context menu on a path node — same delete actions as the
// timeline keyframe diamond. The node it was opened on rides along, because
// which entries apply depends on whether it is a keyframe or a waypoint.
const [kfMenu, setKfMenu] = useState<{
state: KeyframeDiamondContextMenuState;
ref: MotionNodeRef;
} | null>(null);
// The keyframe % selected by clicking its node — highlighted, and the next drag
// modifies it rather than adding a keyframe.
const activeKeyframePct = usePlayerStore((s) => s.activeKeyframePct);
@@ -436,21 +441,47 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
};
const elementId = selection?.id ?? null;
// Right-click a keyframe node → the timeline's keyframe context menu (delete
// this keyframe / delete all), so motion-path keyframes are removable in place.
// Right-click any path node → the timeline's keyframe context menu (delete this
// one / delete all), so path nodes are removable in place. Waypoints open it
// too: returning early for them let the browser's own context menu open over
// the editor overlay, which is never what a right-click on a node should do.
const onNodeContextMenu = (e: React.MouseEvent, ref: MotionNodeRef) => {
if (ref.type !== "keyframe" || !animId || !elementId || !timelineElement) return;
if (!animId || !elementId || !timelineElement) return;
e.preventDefault();
e.stopPropagation();
setKfMenu({
x: e.clientX,
y: e.clientY,
element: timelineElement,
elementId,
percentage: ref.pct,
tweenPercentage: ref.pct,
ref,
state: {
x: e.clientX,
y: e.clientY,
element: timelineElement,
elementId,
// A waypoint carries no percentage of its own: one tween-level ease owns
// every segment, and its index is the identity the delete acts on. The
// menu never reads this for a waypoint, because the only entry that
// would (Move to Playhead) is hidden below.
percentage: ref.type === "keyframe" ? ref.pct : 0,
tweenPercentage: ref.type === "keyframe" ? ref.pct : 0,
},
});
};
const menuRef = kfMenu?.ref;
// Deleting one node: by tween-% for a keyframe, by path index for a waypoint.
// A waypoint delete is offered on the same condition as the hover x badge,
// because removeMotionPathPointInScript refuses to drop an arc below two
// anchors and the entry would silently do nothing.
const onMenuDelete =
menuRef === undefined || !animId
? undefined
: menuRef.type === "keyframe"
? (_elId: string, keyframe: TimelineKeyframeTarget) => {
handleGsapRemoveKeyframe(animId, keyframe.percentage);
}
: removable
? () => {
void commitRemoveWaypoint(animId, menuRef.index, commitMutation);
}
: undefined;
return (
<>
@@ -532,14 +563,17 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
</svg>
{kfMenu && (
<KeyframeDiamondContextMenu
state={kfMenu}
state={kfMenu.state}
onClose={() => setKfMenu(null)}
onDelete={(_elId, keyframe) =>
animId && handleGsapRemoveKeyframe(animId, keyframe.percentage)
}
onDelete={onMenuDelete}
onDeleteAll={() => animId && handleGsapRemoveAllKeyframes(animId)}
onMoveToPlayhead={(_element, keyframe) =>
animId && handleGsapMoveKeyframeToPlayhead(animId, keyframe.percentage)
// Retiming needs a percentage of this node's own, which a waypoint
// does not have.
onMoveToPlayhead={
kfMenu.ref.type === "keyframe"
? (_element, keyframe) =>
animId && handleGsapMoveKeyframeToPlayhead(animId, keyframe.percentage)
: undefined
}
/>
)}
@@ -66,4 +66,25 @@ describe("KeyframeDiamondContextMenu", () => {
expect(onDelete).toHaveBeenCalledWith("box", target);
expect(onMoveToPlayhead).toHaveBeenCalledWith(element, target);
});
// An arc waypoint on a two-anchor path cannot be dropped on its own, so the
// caller withholds onDelete rather than offering an entry that does nothing.
it("hides the single-node delete when no handler is given", () => {
const host = document.createElement("div");
document.body.appendChild(host);
const root = createRoot(host);
act(() =>
root.render(
<KeyframeDiamondContextMenu state={state} onClose={() => {}} onDeleteAll={vi.fn()} />,
),
);
const labels = Array.from(document.body.querySelectorAll("button")).map(
(button) => button.textContent,
);
expect(labels).toEqual(["Delete All Keyframes"]);
act(() => root.unmount());
host.remove();
});
});
@@ -19,7 +19,10 @@ export interface KeyframeDiamondContextMenuState {
interface KeyframeDiamondContextMenuProps {
state: KeyframeDiamondContextMenuState;
onClose: () => void;
onDelete: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
/** Omitted where this node cannot be deleted on its own (see the arc-waypoint
* floor in removeMotionPathPointInScript): an entry that silently no-ops is
* worse than no entry. */
onDelete?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
onDeleteAll: (element: TimelineElement) => void;
/** Retime the keyframe to the current playhead, preserving its value + ease. */
onMoveToPlayhead?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void;
@@ -43,7 +46,9 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe
};
const menuWidth = 200;
const menuHeight = onMoveToPlayhead ? 100 : 70;
// Measured off the rendered rows, so the flip-up test below stays right as
// optional entries drop out.
const menuHeight = 10 + (1 + (onMoveToPlayhead ? 1 : 0) + (onDelete ? 1 : 0)) * 30;
const overflowY = state.y + menuHeight - window.innerHeight;
const adjustedX = state.x + menuWidth > window.innerWidth ? state.x - menuWidth : state.x;
const adjustedY = overflowY > 0 ? state.y - overflowY - 8 : state.y;
@@ -71,16 +76,18 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe
)}
{/* Delete */}
<button
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, keyframe);
onClose();
}}
>
Delete Keyframe
</button>
{onDelete && (
<button
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, keyframe);
onClose();
}}
>
Delete Keyframe
</button>
)}
<button
type="button"