Skip to content

Commit 57781d4

Browse files
committed
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.
1 parent ea7b955 commit 57781d4

3 files changed

Lines changed: 92 additions & 30 deletions

File tree

packages/studio/src/components/editor/MotionPathOverlay.tsx

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
KeyframeDiamondContextMenu,
1313
type KeyframeDiamondContextMenuState,
1414
} from "../../player/components/KeyframeDiamondContextMenu";
15+
import type { TimelineKeyframeTarget } from "../../player/components/timelineKeyframeIdentity";
1516
import {
1617
commitAddKeyframe,
1718
commitAddWaypoint,
@@ -96,9 +97,13 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
9697
const [draft, setDraft] = useState<Draft | null>(null);
9798
const [ghost, setGhost] = useState<{ x: number; y: number; segIndex: number } | null>(null);
9899
const [hoverNode, setHoverNode] = useState<number | null>(null);
99-
// Right-click context menu on a keyframe node — same delete actions as the
100-
// timeline keyframe diamond.
101-
const [kfMenu, setKfMenu] = useState<KeyframeDiamondContextMenuState | null>(null);
100+
// Right-click context menu on a path node — same delete actions as the
101+
// timeline keyframe diamond. The node it was opened on rides along, because
102+
// which entries apply depends on whether it is a keyframe or a waypoint.
103+
const [kfMenu, setKfMenu] = useState<{
104+
state: KeyframeDiamondContextMenuState;
105+
ref: MotionNodeRef;
106+
} | null>(null);
102107
// The keyframe % selected by clicking its node — highlighted, and the next drag
103108
// modifies it rather than adding a keyframe.
104109
const activeKeyframePct = usePlayerStore((s) => s.activeKeyframePct);
@@ -436,21 +441,47 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
436441
};
437442

438443
const elementId = selection?.id ?? null;
439-
// Right-click a keyframe node → the timeline's keyframe context menu (delete
440-
// this keyframe / delete all), so motion-path keyframes are removable in place.
444+
// Right-click any path node → the timeline's keyframe context menu (delete this
445+
// one / delete all), so path nodes are removable in place. Waypoints open it
446+
// too: returning early for them let the browser's own context menu open over
447+
// the editor overlay, which is never what a right-click on a node should do.
441448
const onNodeContextMenu = (e: React.MouseEvent, ref: MotionNodeRef) => {
442-
if (ref.type !== "keyframe" || !animId || !elementId || !timelineElement) return;
449+
if (!animId || !elementId || !timelineElement) return;
443450
e.preventDefault();
444451
e.stopPropagation();
445452
setKfMenu({
446-
x: e.clientX,
447-
y: e.clientY,
448-
element: timelineElement,
449-
elementId,
450-
percentage: ref.pct,
451-
tweenPercentage: ref.pct,
453+
ref,
454+
state: {
455+
x: e.clientX,
456+
y: e.clientY,
457+
element: timelineElement,
458+
elementId,
459+
// A waypoint carries no percentage of its own: one tween-level ease owns
460+
// every segment, and its index is the identity the delete acts on. The
461+
// menu never reads this for a waypoint, because the only entry that
462+
// would (Move to Playhead) is hidden below.
463+
percentage: ref.type === "keyframe" ? ref.pct : 0,
464+
tweenPercentage: ref.type === "keyframe" ? ref.pct : 0,
465+
},
452466
});
453467
};
468+
const menuRef = kfMenu?.ref;
469+
// Deleting one node: by tween-% for a keyframe, by path index for a waypoint.
470+
// A waypoint delete is offered on the same condition as the hover x badge,
471+
// because removeMotionPathPointInScript refuses to drop an arc below two
472+
// anchors and the entry would silently do nothing.
473+
const onMenuDelete =
474+
menuRef === undefined || !animId
475+
? undefined
476+
: menuRef.type === "keyframe"
477+
? (_elId: string, keyframe: TimelineKeyframeTarget) => {
478+
handleGsapRemoveKeyframe(animId, keyframe.percentage);
479+
}
480+
: removable
481+
? () => {
482+
void commitRemoveWaypoint(animId, menuRef.index, commitMutation);
483+
}
484+
: undefined;
454485

455486
return (
456487
<>
@@ -532,14 +563,17 @@ export const MotionPathOverlay = memo(function MotionPathOverlay({
532563
</svg>
533564
{kfMenu && (
534565
<KeyframeDiamondContextMenu
535-
state={kfMenu}
566+
state={kfMenu.state}
536567
onClose={() => setKfMenu(null)}
537-
onDelete={(_elId, keyframe) =>
538-
animId && handleGsapRemoveKeyframe(animId, keyframe.percentage)
539-
}
568+
onDelete={onMenuDelete}
540569
onDeleteAll={() => animId && handleGsapRemoveAllKeyframes(animId)}
541-
onMoveToPlayhead={(_element, keyframe) =>
542-
animId && handleGsapMoveKeyframeToPlayhead(animId, keyframe.percentage)
570+
// Retiming needs a percentage of this node's own, which a waypoint
571+
// does not have.
572+
onMoveToPlayhead={
573+
kfMenu.ref.type === "keyframe"
574+
? (_element, keyframe) =>
575+
animId && handleGsapMoveKeyframeToPlayhead(animId, keyframe.percentage)
576+
: undefined
543577
}
544578
/>
545579
)}

packages/studio/src/player/components/KeyframeDiamondContextMenu.test.tsx

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,25 @@ describe("KeyframeDiamondContextMenu", () => {
6666
expect(onDelete).toHaveBeenCalledWith("box", target);
6767
expect(onMoveToPlayhead).toHaveBeenCalledWith(element, target);
6868
});
69+
70+
// An arc waypoint on a two-anchor path cannot be dropped on its own, so the
71+
// caller withholds onDelete rather than offering an entry that does nothing.
72+
it("hides the single-node delete when no handler is given", () => {
73+
const host = document.createElement("div");
74+
document.body.appendChild(host);
75+
const root = createRoot(host);
76+
act(() =>
77+
root.render(
78+
<KeyframeDiamondContextMenu state={state} onClose={() => {}} onDeleteAll={vi.fn()} />,
79+
),
80+
);
81+
82+
const labels = Array.from(document.body.querySelectorAll("button")).map(
83+
(button) => button.textContent,
84+
);
85+
expect(labels).toEqual(["Delete All Keyframes"]);
86+
87+
act(() => root.unmount());
88+
host.remove();
89+
});
6990
});

packages/studio/src/player/components/KeyframeDiamondContextMenu.tsx

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@ export interface KeyframeDiamondContextMenuState {
1919
interface KeyframeDiamondContextMenuProps {
2020
state: KeyframeDiamondContextMenuState;
2121
onClose: () => void;
22-
onDelete: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
22+
/** Omitted where this node cannot be deleted on its own (see the arc-waypoint
23+
* floor in removeMotionPathPointInScript): an entry that silently no-ops is
24+
* worse than no entry. */
25+
onDelete?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
2326
onDeleteAll: (element: TimelineElement) => void;
2427
/** Retime the keyframe to the current playhead, preserving its value + ease. */
2528
onMoveToPlayhead?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void;
@@ -43,7 +46,9 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe
4346
};
4447

4548
const menuWidth = 200;
46-
const menuHeight = onMoveToPlayhead ? 100 : 70;
49+
// Measured off the rendered rows, so the flip-up test below stays right as
50+
// optional entries drop out.
51+
const menuHeight = 10 + (1 + (onMoveToPlayhead ? 1 : 0) + (onDelete ? 1 : 0)) * 30;
4752
const overflowY = state.y + menuHeight - window.innerHeight;
4853
const adjustedX = state.x + menuWidth > window.innerWidth ? state.x - menuWidth : state.x;
4954
const adjustedY = overflowY > 0 ? state.y - overflowY - 8 : state.y;
@@ -71,16 +76,18 @@ export const KeyframeDiamondContextMenu = memo(function KeyframeDiamondContextMe
7176
)}
7277

7378
{/* Delete */}
74-
<button
75-
type="button"
76-
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"
77-
onClick={() => {
78-
onDelete(state.elementId, keyframe);
79-
onClose();
80-
}}
81-
>
82-
Delete Keyframe
83-
</button>
79+
{onDelete && (
80+
<button
81+
type="button"
82+
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"
83+
onClick={() => {
84+
onDelete(state.elementId, keyframe);
85+
onClose();
86+
}}
87+
>
88+
Delete Keyframe
89+
</button>
90+
)}
8491

8592
<button
8693
type="button"

0 commit comments

Comments
 (0)