diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx index cf240afa9..dec72f7b1 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.test.tsx @@ -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(); + }); }); diff --git a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts index 0ac9a3274..fa80561bb 100644 --- a/packages/studio/src/components/nle/useTimelineEditCallbacks.ts +++ b/packages/studio/src/components/nle/useTimelineEditCallbacks.ts @@ -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; diff --git a/packages/studio/src/player/components/TimelineClipDiamonds.tsx b/packages/studio/src/player/components/TimelineClipDiamonds.tsx index 1a066d5d0..09320bdc5 100644 --- a/packages/studio/src/player/components/TimelineClipDiamonds.tsx +++ b/packages/studio/src/player/components/TimelineClipDiamonds.tsx @@ -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; /** 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 } diff --git a/packages/studio/src/player/components/TimelineLanes.tsx b/packages/studio/src/player/components/TimelineLanes.tsx index 13707c05c..8437bb64e 100644 --- a/packages/studio/src/player/components/TimelineLanes.tsx +++ b/packages/studio/src/player/components/TimelineLanes.tsx @@ -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; onContextMenuClip?: (e: React.MouseEvent, element: TimelineElement) => void; diff --git a/packages/studio/src/player/components/TimelineOverlays.tsx b/packages/studio/src/player/components/TimelineOverlays.tsx index bd0896e0b..44e312074 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, 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) => { diff --git a/packages/studio/src/player/components/timelineCallbacks.ts b/packages/studio/src/player/components/timelineCallbacks.ts index ced618e71..204302e4b 100644 --- a/packages/studio/src/player/components/timelineCallbacks.ts +++ b/packages/studio/src/player/components/timelineCallbacks.ts @@ -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; onRazorSplit?: (element: TimelineElement, splitTime: number) => Promise | void; onRazorSplitAll?: (splitTime: number) => Promise | 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; onToggleKeyframeAtPlayhead?: (element: TimelineElement) => void; onTogglePropertyGroupKeyframe?: ( diff --git a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts index a5e31e71b..d0ccaa268 100644 --- a/packages/studio/src/player/hooks/useExpandedTimelineElements.ts +++ b/packages/studio/src/player/hooks/useExpandedTimelineElements.ts @@ -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 | null { let current = parentMap.get(id); @@ -19,18 +19,13 @@ function findTopLevelAncestor(id: string, parentMap: Map): 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 | 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); diff --git a/packages/studio/src/player/lib/timelineElementHelpers.ts b/packages/studio/src/player/lib/timelineElementHelpers.ts index b3d6482ae..323248291 100644 --- a/packages/studio/src/player/lib/timelineElementHelpers.ts +++ b/packages/studio/src/player/lib/timelineElementHelpers.ts @@ -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;