fix(studio): retime the dragged element's own keyframe

Drag-to-retime resolved the dragged diamond against the selected element's
animations and committed through the selected element's DOM selection, so
dragging a diamond on a non-selected clip retimed the wrong tween. It now
resolves against the clicked element's animations and commits through that
element's selection, matching the delete path.

The three diamond callbacks also take the TimelineKeyframeTarget they already
had instead of five positional fields, and the two copies of the
sourceFile#domId split share splitTimelineElementKey.
This commit is contained in:
Miguel Angel Simon Sierra
2026-07-27 19:52:07 +02:00
parent 3f93794fa6
commit 8f66b06d12
8 changed files with 166 additions and 95 deletions
@@ -167,7 +167,16 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
act(() => {
view.callbacks.onMoveKeyframe?.("box", 0, 25, "position", 0, flatAnimation.id);
view.callbacks.onMoveKeyframe?.(
"box",
{
percentage: 0,
propertyGroup: "position",
tweenPercentage: 0,
animationId: flatAnimation.id,
},
25,
);
});
expect(mocks.actions.handleGsapMoveKeyframe).not.toHaveBeenCalled();
@@ -189,13 +198,12 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
await act(async () => {
view.callbacks.onDeleteKeyframe?.(
"scenes/main.html#circle",
0,
"position",
0,
otherFlatAnimation.id,
);
view.callbacks.onDeleteKeyframe?.("scenes/main.html#circle", {
percentage: 0,
propertyGroup: "position",
tweenPercentage: 0,
animationId: otherFlatAnimation.id,
});
await Promise.resolve();
await Promise.resolve();
});
@@ -223,13 +231,12 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
await act(async () => {
view.callbacks.onDeleteKeyframe?.(
"scenes/main.html#circle",
100,
"position",
100,
otherKeyframedAnimation.id,
);
view.callbacks.onDeleteKeyframe?.("scenes/main.html#circle", {
percentage: 100,
propertyGroup: "position",
tweenPercentage: 100,
animationId: otherKeyframedAnimation.id,
});
await Promise.resolve();
await Promise.resolve();
});
@@ -287,7 +294,7 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
act(() => {
view.callbacks.onMoveKeyframeToPlayhead?.("scenes/main.html#circle", 100);
view.callbacks.onMoveKeyframeToPlayhead?.("scenes/main.html#circle", { percentage: 100 });
});
expect(mocks.actions.handleGsapMoveKeyframeToPlayhead).toHaveBeenCalledWith(
@@ -301,7 +308,12 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
act(() => {
view.callbacks.onDeleteKeyframe?.("box", 0, "position", 0, flatAnimation.id);
view.callbacks.onDeleteKeyframe?.("box", {
percentage: 0,
propertyGroup: "position",
tweenPercentage: 0,
animationId: flatAnimation.id,
});
});
expect(mocks.actions.handleGsapDeleteAnimation).toHaveBeenCalledWith(flatAnimation.id);
@@ -371,7 +383,12 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
const view = renderCallbacks();
act(() => {
view.callbacks.onDeleteKeyframe?.("box", 50, "position", 50, flatAnimation.id);
view.callbacks.onDeleteKeyframe?.("box", {
percentage: 50,
propertyGroup: "position",
tweenPercentage: 50,
animationId: flatAnimation.id,
});
});
expect(mocks.actions.handleGsapRemoveKeyframe).toHaveBeenCalledWith(flatAnimation.id, 50);
@@ -379,16 +396,74 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
view.unmount();
});
it("keeps an authored interior drag on the per-keyframe move path", () => {
mocks.animations = [authoredInteriorAnimation()];
it("keeps an authored interior drag on the per-keyframe move path", async () => {
const authored = authoredInteriorAnimation();
mocks.animations = [authored];
usePlayerStore.setState({ gsapAnimations: new Map([["box", [authored]]]) });
const view = renderCallbacks();
act(() => {
view.callbacks.onMoveKeyframe?.("box", 50, 75, "position", 50, flatAnimation.id);
await act(async () => {
await view.callbacks.onMoveKeyframe?.(
"box",
{
percentage: 50,
propertyGroup: "position",
tweenPercentage: 50,
animationId: authored.id,
},
75,
);
});
expect(mocks.actions.handleGsapMoveKeyframe).toHaveBeenCalledWith(flatAnimation.id, 50, 75);
expect(mocks.actions.handleGsapMoveKeyframe).toHaveBeenCalledWith(
authored.id,
50,
75,
mocks.selection,
);
expect(mocks.actions.handleGsapResizeKeyframedTween).not.toHaveBeenCalled();
view.unmount();
});
// A drag starts on whatever diamond the pointer is over, which need not be the
// selected element. Resolving against the selection would retime the selected
// element's tween and commit it through the selected element's file.
it("retimes a non-selected element's keyframe through that element's own selection", async () => {
const circle: TimelineElement = {
...element,
id: "circle",
key: "scenes/main.html#circle",
domId: "circle",
sourceFile: "scenes/main.html",
};
const circleSelection = { id: "circle", selector: "#circle", sourceFile: "scenes/main.html" };
const circleAnimation = { ...authoredInteriorAnimation(), id: "circle-to-0-position" };
usePlayerStore.setState({
elements: [element, circle],
gsapAnimations: new Map([["scenes/main.html#circle", [circleAnimation]]]),
});
mocks.actions.buildDomSelectionForTimelineElement.mockResolvedValue(circleSelection);
const view = renderCallbacks();
await act(async () => {
await view.callbacks.onMoveKeyframe?.(
"scenes/main.html#circle",
{
percentage: 50,
propertyGroup: "position",
tweenPercentage: 50,
animationId: circleAnimation.id,
},
75,
);
});
expect(mocks.actions.handleGsapMoveKeyframe).toHaveBeenCalledWith(
circleAnimation.id,
50,
75,
circleSelection,
);
view.unmount();
});
});
@@ -14,6 +14,8 @@ import { resolveClipTimingBasis } from "../../hooks/useGsapTweenCache";
import { resolveKeyframeRetime } from "../editor/keyframeRetime";
import type { DomEditSelection } from "../editor/domEditingTypes";
import type { TimelineMoveOperation } from "../../hooks/timelineMoveAdapter";
import { splitTimelineElementKey } from "../../player/lib/timelineElementHelpers";
import type { TimelineKeyframeTarget } from "../../player/components/timelineKeyframeIdentity";
export interface TimelineEditCallbackDeps {
handleTimelineElementMove: (
@@ -117,14 +119,12 @@ export function useTimelineEditCallbacks({
const resolveElementAnimations = useCallback(
(elementKey: string): GsapAnimation[] => {
const { gsapAnimations } = usePlayerStore.getState();
const hashIndex = elementKey.lastIndexOf("#");
const elementId = hashIndex === -1 ? elementKey : elementKey.slice(hashIndex + 1);
const sourceFile =
hashIndex === -1 ? (activeCompPath ?? "index.html") : elementKey.slice(0, hashIndex);
const { sourceFile, domId } = splitTimelineElementKey(elementKey);
const scope = sourceFile ?? activeCompPath ?? "index.html";
return (
gsapAnimations.get(`${sourceFile}#${elementId}`) ??
gsapAnimations.get(`index.html#${elementId}`) ??
gsapAnimations.get(elementId) ??
gsapAnimations.get(`${scope}#${domId}`) ??
gsapAnimations.get(`index.html#${domId}`) ??
gsapAnimations.get(domId) ??
[]
);
},
@@ -136,19 +136,15 @@ export function useTimelineEditCallbacks({
// diamond reports a clip-% but the script ops key on the tween-%. Prefers the
// anim in the keyframe's property group, falling back to the first keyframed one.
const resolveKeyframeTarget = useCallback(
// fallow-ignore-next-line complexity
(
pct: number,
propertyGroup?: string,
tweenPercentage?: number,
animationId?: string,
target: TimelineKeyframeTarget,
animations: GsapAnimation[] = selectedGsapAnimations,
elementKey?: string,
): { animId: string; tweenPct: number } | null => {
const explicitTarget =
propertyGroup !== undefined || tweenPercentage !== undefined || animationId !== undefined
? [{ percentage: pct, propertyGroup, tweenPercentage, animationId }]
: undefined;
const carriesIdentity =
target.propertyGroup !== undefined ||
target.tweenPercentage !== undefined ||
target.animationId !== undefined;
// The clicked element's own cache when the caller knows it: the diamond
// context menu can open on an element that is not the selected one, and
// reading the selection's cache there resolves against the wrong element.
@@ -156,8 +152,8 @@ export function useTimelineEditCallbacks({
.getState()
.keyframeCache.get(elementKey ?? domEditSelection?.id ?? "");
return resolveTimelineKeyframeTarget(
pct,
explicitTarget ?? cached?.keyframes ?? [],
target.percentage,
carriesIdentity ? [target] : (cached?.keyframes ?? []),
animations,
);
},
@@ -202,9 +198,9 @@ export function useTimelineEditCallbacks({
if (!anim) return;
handleGsapRemoveAllKeyframes(anim.id);
},
onDeleteKeyframe: (elId, pct, group, tweenPct, animationId) => {
onDeleteKeyframe: (elId, keyframe) => {
const animations = resolveElementAnimations(elId);
const target = resolveKeyframeTarget(pct, group, tweenPct, animationId, animations, elId);
const target = resolveKeyframeTarget(keyframe, animations, elId);
if (!target) return;
const element = usePlayerStore.getState().elements.find((el) => (el.key ?? el.id) === elId);
if (!element) {
@@ -219,15 +215,8 @@ export function useTimelineEditCallbacks({
});
},
// Retime the keyframe to the playhead, preserving its value + ease.
onMoveKeyframeToPlayhead: (elId, pct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(
pct,
group,
tweenPct,
animationId,
resolveElementAnimations(elId),
elId,
);
onMoveKeyframeToPlayhead: (elId, keyframe) => {
const target = resolveKeyframeTarget(keyframe, resolveElementAnimations(elId), elId);
if (target) handleGsapMoveKeyframeToPlayhead(target.animId, target.tweenPct);
},
// Drag-to-retime. The diamond reports clip-%s; resolveKeyframeTarget gives
@@ -238,11 +227,17 @@ export function useTimelineEditCallbacks({
// resizes the tween — position/duration grow so the dragged keyframe lands at
// the drop while every other keyframe keeps its absolute time (value+ease too).
// fallow-ignore-next-line complexity
onMoveKeyframe: async (_elId, fromClipPct, toClipPct, group, tweenPct, animationId) => {
const target = resolveKeyframeTarget(fromClipPct, group, tweenPct, animationId);
const sel = domEditSelection;
if (!target || !sel) return false;
const anim = selectedGsapAnimations.find((a) => a.id === target.animId);
onMoveKeyframe: async (elId, keyframe, toClipPct) => {
const animations = resolveElementAnimations(elId);
const target = resolveKeyframeTarget(keyframe, animations, elId);
if (!target) return false;
// The dragged diamond's OWN element, not the selected one: a drag on a
// non-selected clip has to read that clip's animations and commit
// through that clip's selection, or it retimes whatever is selected.
const element = usePlayerStore.getState().elements.find((el) => (el.key ?? el.id) === elId);
const sel = element ? await buildDomSelectionForTimelineElement(element) : domEditSelection;
if (!sel) return false;
const anim = animations.find((a) => a.id === target.animId);
const tweenStart = anim ? resolveTweenStart(anim) : null;
if (!anim || tweenStart === null) return false;
// Synthesized flat endpoints are clip boundaries, not authored keyframes.
@@ -267,7 +262,7 @@ export function useTimelineEditCallbacks({
dropAbsTime,
});
if (decision.kind === "move" && decision.toTweenPct != null) {
handleGsapMoveKeyframe(target.animId, target.tweenPct, decision.toTweenPct);
handleGsapMoveKeyframe(target.animId, target.tweenPct, decision.toTweenPct, sel);
} else if (
decision.kind === "resize" &&
decision.pctRemap &&
@@ -280,15 +275,17 @@ export function useTimelineEditCallbacks({
decision.position,
decision.duration,
decision.pctRemap,
sel,
);
} else {
// resize-keyframed-tween requires an authored `keyframes` AST node
// and intentionally no-ops for a flat tween. Update its real tween
// window through the metadata writer (and SDK cutover path) instead.
handleGsapUpdateMeta(target.animId, {
position: decision.position,
duration: decision.duration,
});
handleGsapUpdateMeta(
target.animId,
{ position: decision.position, duration: decision.duration },
sel,
);
}
} else {
return false;
@@ -48,12 +48,13 @@ interface TimelineClipDiamondsProps {
onShiftClickKeyframe?: (elementId: string, percentage: number) => void;
onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void;
/** Drag-to-retime: move a keyframe to a new time, preserving its value + ease.
* Both percentages are clip-relative: `fromClipPercentage` identifies the
* dragged keyframe, `toClipPercentage` is the neighbour-clamped drop position.
* The handler decides move (within the tween) vs resize (past its boundary). */
* `keyframe` identifies the dragged keyframe (clip-relative percentage plus
* whatever animation identity the row carries); `toClipPercentage` is the
* neighbour-clamped drop position, also clip-relative. The handler decides
* move (within the tween) vs resize (past its boundary). */
onMoveKeyframe?: (
elementId: string,
fromClipPercentage: number,
keyframe: TimelineKeyframeTarget,
toClipPercentage: number,
) => Promise<boolean>;
/** Open the segment ease editor for the hovered mid-point button — available on
@@ -549,7 +550,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds(
onMoveKeyframe={
props.onMoveKeyframe
? (target, toClipPercentage) =>
props.onMoveKeyframe?.(props.elementId, target.percentage, toClipPercentage) ??
props.onMoveKeyframe?.(props.elementId, target, toClipPercentage) ??
Promise.resolve(false)
: undefined
}
@@ -3,6 +3,7 @@ import { Eye, EyeSlash } from "@phosphor-icons/react";
import { BeatStrip, BeatBackgroundLines } from "./BeatStrip";
import { TimelineClip } from "./TimelineClip";
import { TimelineClipDiamonds } from "./TimelineClipDiamonds";
import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
import type { MusicBeatAnalysis } from "@hyperframes/core/beats";
import { getTimelineEditCapabilities, resolveBlockedTimelineEditIntent } from "./timelineEditing";
import type { TimelineTheme } from "./timelineTheme";
@@ -76,7 +77,7 @@ export interface TimelineLaneBaseProps {
onContextMenuKeyframe?: (e: React.MouseEvent, elementId: string, percentage: number) => void;
onMoveKeyframe?: (
elementId: string,
fromClipPercentage: number,
keyframe: TimelineKeyframeTarget,
toClipPercentage: number,
) => Promise<boolean>;
onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void;
@@ -106,12 +106,12 @@ export function TimelineOverlays({
<KeyframeDiamondContextMenu
state={kfContextMenu}
onClose={() => setKfContextMenu(null)}
onDelete={(elId, pct) => onDeleteKeyframe?.(elId, pct)}
onDelete={(elId, pct) => onDeleteKeyframe?.(elId, { percentage: pct })}
onDeleteAll={(elId) => onDeleteAllKeyframes?.(elId)}
onChangeEase={(elId, pct, ease) => onChangeKeyframeEase?.(elId, pct, ease)}
onMoveToPlayhead={
onMoveKeyframeToPlayhead
? (elId, pct) => onMoveKeyframeToPlayhead(elId, pct)
? (elId, pct) => onMoveKeyframeToPlayhead(elId, { percentage: pct })
: undefined
}
onCopyProperties={(elId, pct) => {
@@ -4,6 +4,7 @@ import type { TimelineElement } from "../store/playerStore";
import type { TimelineMoveOperation } from "../../hooks/timelineMoveAdapter";
import type { BlockedTimelineEditIntent } from "./timelineEditing";
import type { PropertyGroupName } from "@hyperframes/core/gsap-parser";
import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
export interface TimelinePropertyGroupKeyframeToggle {
animationId: string;
@@ -71,29 +72,16 @@ export interface TimelineEditCallbacks {
onSplitElement?: (element: TimelineElement, splitTime: number) => Promise<void> | void;
onRazorSplit?: (element: TimelineElement, splitTime: number) => Promise<void> | void;
onRazorSplitAll?: (splitTime: number) => Promise<void> | void;
onDeleteKeyframe?: (
elementId: string,
percentage: number,
propertyGroup?: string,
tweenPercentage?: number,
animationId?: string,
) => void;
onDeleteKeyframe?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
onDeleteAllKeyframes?: (elementId: string) => void;
onChangeKeyframeEase?: (elementId: string, percentage: number, ease: string) => void;
onMoveKeyframeToPlayhead?: (
elementId: string,
percentage: number,
propertyGroup?: string,
tweenPercentage?: number,
animationId?: string,
) => void;
onMoveKeyframeToPlayhead?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
/** Drag-to-retime: `keyframe` identifies the dragged keyframe (its percentage
* is clip-relative), `toClipPercentage` is the neighbour-clamped drop. */
onMoveKeyframe?: (
elementId: string,
fromClipPercentage: number,
keyframe: TimelineKeyframeTarget,
toClipPercentage: number,
propertyGroup?: string,
tweenPercentage?: number,
animationId?: string,
) => Promise<boolean>;
onToggleKeyframeAtPlayhead?: (element: TimelineElement) => void;
onTogglePropertyGroupKeyframe?: (
@@ -2,7 +2,7 @@ import { useMemo } from "react";
import { usePlayerStore, type TimelineElement, type DomClipChild } from "../store/playerStore";
import type { ClipManifestClip } from "../lib/playbackTypes";
import { createTimelineElementFromManifestClip } from "../lib/timelineDOM";
import { buildTimelineElementKey } from "../lib/timelineElementHelpers";
import { buildTimelineElementKey, splitTimelineElementKey } from "../lib/timelineElementHelpers";
function findTopLevelAncestor(id: string, parentMap: Map<string, string>): string | null {
let current = parentMap.get(id);
@@ -19,18 +19,13 @@ function findTopLevelAncestor(id: string, parentMap: Map<string, string>): strin
return current;
}
function extractDomId(key: string): string {
const hashIdx = key.lastIndexOf("#");
return hashIdx >= 0 ? key.slice(hashIdx + 1) : key;
}
function resolveRawId(
selectedId: string | null,
manifest: ClipManifestClip[],
parentMap: Map<string, string>,
): string | null {
if (!selectedId) return null;
const rawId = extractDomId(selectedId);
const rawId = splitTimelineElementKey(selectedId).domId;
if (parentMap.has(rawId)) return rawId;
if (parentMap.has(selectedId)) return selectedId;
const clip = manifest.find((c) => c.label === selectedId || c.label === rawId);
@@ -298,6 +298,20 @@ export function buildTimelineElementKey(params: {
return `${scope}:${params.id}:${params.fallbackIndex}`;
}
/**
* Inverse of {@link buildTimelineElementKey} for the `sourceFile#domId` form.
* A key with no `#` is a bare dom id and carries no source file of its own, so
* the caller supplies the scope it wants to look that id up in.
*/
export function splitTimelineElementKey(key: string): {
sourceFile: string | null;
domId: string;
} {
const hashIndex = key.lastIndexOf("#");
if (hashIndex < 0) return { sourceFile: null, domId: key };
return { sourceFile: key.slice(0, hashIndex), domId: key.slice(hashIndex + 1) };
}
export function buildTimelineElementIdentity(params: {
preferredId?: string | null;
label: string;