Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -317,17 +317,44 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
});

it("deletes all keyframes through the clicked non-selected element's identity", async () => {
const { circle, selection } = arrangeClickedCircle();
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 scaleAnimation: GsapAnimation = {
...otherKeyframedAnimation,
id: "circle-to-0-scale",
properties: {},
propertyGroup: "scale",
keyframes: {
format: "percentage",
keyframes: [
{ percentage: 0, properties: { scale: 1 } },
{ percentage: 100, properties: { scale: 2 } },
],
},
};
usePlayerStore.setState({
elements: [element, circle],
gsapAnimations: new Map([
["scenes/main.html#circle", [otherKeyframedAnimation, scaleAnimation]],
]),
});
mocks.actions.buildDomSelectionForTimelineElement.mockResolvedValue(circleSelection);
const view = renderCallbacks();

await act(async () => {
view.callbacks.onDeleteAllKeyframes?.(circle);
view.callbacks.onDeleteAllKeyframes?.(circle, scaleAnimation.id);
await Promise.resolve();
});

expect(mocks.actions.handleGsapRemoveAllKeyframes).toHaveBeenCalledWith(
otherKeyframedAnimation.id,
selection,
scaleAnimation.id,
circleSelection,
);
view.unmount();
});
Expand Down Expand Up @@ -360,6 +387,19 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
view.unmount();
});

it("does not delete a different lane when an explicit animation identity is stale", async () => {
const { circle } = arrangeClickedCircle();
const view = renderCallbacks();

await act(async () => {
view.callbacks.onDeleteAllKeyframes?.(circle, "missing-animation-id");
await Promise.resolve();
});

expect(mocks.actions.handleGsapRemoveAllKeyframes).not.toHaveBeenCalled();
view.unmount();
});

it("aborts every mutation when the clicked element resolves no selection", async () => {
const { circle } = arrangeClickedCircle();
mocks.actions.buildDomSelectionForTimelineElement.mockResolvedValue(null);
Expand Down Expand Up @@ -391,6 +431,40 @@ describe("useTimelineEditCallbacks — flat tween keyframe lanes", () => {
view.unmount();
});

it("does not delete a different keyframe when its explicit animation identity is stale", () => {
const view = renderCallbacks();

act(() => {
view.callbacks.onDeleteKeyframe?.("box", {
percentage: 100,
propertyGroup: "position",
tweenPercentage: 100,
animationId: "missing-animation-id",
});
});

expect(mocks.actions.handleGsapDeleteAnimation).not.toHaveBeenCalled();
expect(mocks.actions.handleGsapRemoveKeyframe).not.toHaveBeenCalled();
view.unmount();
});

it("does not move a different keyframe when its explicit animation identity is stale", async () => {
const view = renderCallbacks();

await act(async () => {
view.callbacks.onMoveKeyframeToPlayhead?.(element, {
percentage: 100,
propertyGroup: "position",
tweenPercentage: 100,
animationId: "missing-animation-id",
});
await Promise.resolve();
});

expect(mocks.actions.handleGsapMoveKeyframeToPlayhead).not.toHaveBeenCalled();
view.unmount();
});

it("moves a keyframe to the playhead through the clicked non-selected element's identity", async () => {
const { circle, selection } = arrangeClickedCircle();
const view = renderCallbacks();
Expand Down
19 changes: 12 additions & 7 deletions packages/studio/src/components/nle/useTimelineEditCallbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,17 +189,22 @@ export function useTimelineEditCallbacks({
onSplitElement: handleTimelineElementSplit,
onRazorSplit: handleRazorSplit,
onRazorSplitAll: handleRazorSplitAll,
onDeleteAllKeyframes: (element) => {
onDeleteAllKeyframes: (element, animationId) => {
// Hold the element where it is (collapse keyframes to a static set) rather
// than deleting the whole animation — deleting strands a stale GSAP base
// that the next drag adds to, flinging the element off-screen.
const elementKey = getTimelineElementIdentity(element);
// Every keyframed tween on the layer, not just the first: a layer with
// position AND opacity keyframes left the second one keyframed, so
// "Delete All Keyframes" visibly did half the job.
const anims = resolveElementAnimations(elementKey).filter(
(animation) => animation.keyframes,
);
// An explicit animation id scopes the delete to the lane whose menu was
// opened; without one this is the layer-wide action, and that means
// EVERY keyframed tween, not just the first. A layer with position AND
// opacity keyframes used to leave the second one keyframed, so "Delete
// All Keyframes" visibly did half the job. A stale id matches nothing
// and deletes nothing, which is the point: it never falls back to a
// lane the user did not click.
const animations = resolveElementAnimations(elementKey);
const anims = animationId
? animations.filter((animation) => animation.id === animationId)
: animations.filter((animation) => animation.keyframes);
if (anims.length === 0) return;
void buildDomSelectionForTimelineElement(element).then(async (selection) => {
if (!selection) return;
Expand Down
40 changes: 39 additions & 1 deletion packages/studio/src/hooks/useGsapKeyframeOps.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import { afterEach, describe, expect, it, vi } from "vitest";
(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

import type { DomEditSelection } from "../components/editor/domEditingTypes";
import type { KeyframeCacheEntry } from "../player/store/playerStore";
import { usePlayerStore } from "../player/store/playerStore";
import { useGsapKeyframeOps } from "./useGsapKeyframeOps";

type HookApi = ReturnType<typeof useGsapKeyframeOps>;
Expand All @@ -30,6 +32,7 @@ function successfulCommitMutation() {

function renderKeyframeOps(over: {
commitMutation: (...args: unknown[]) => Promise<unknown>;
commitMutationSafely?: (...args: unknown[]) => Promise<void>;
trackGsapSaveFailure: (...args: unknown[]) => void;
}) {
const captured: { api: HookApi | null } = { api: null };
Expand All @@ -41,7 +44,7 @@ function renderKeyframeOps(over: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
commitMutation: over.commitMutation as any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
commitMutationSafely: (() => {}) as any,
commitMutationSafely: (over.commitMutationSafely ?? (async () => {})) as any,
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- test doubles
trackGsapSaveFailure: over.trackGsapSaveFailure as any,
sdkSession: null,
Expand Down Expand Up @@ -203,6 +206,41 @@ describe("useGsapKeyframeOps — keyframe transaction options", () => {
);
});

it("lets the successful commit refresh own delete-all cache invalidation", async () => {
let finishCommit: (() => void) | undefined;
const commitMutationSafely = vi.fn(
() =>
new Promise<void>((resolve) => {
finishCommit = resolve;
}),
);
const api = renderKeyframeOps({
commitMutation: successfulCommitMutation(),
commitMutationSafely,
trackGsapSaveFailure: vi.fn(),
});
const cached: KeyframeCacheEntry = {
format: "percentage",
keyframes: [
{ percentage: 0, properties: { x: 0 } },
{ percentage: 100, properties: { x: 200 } },
],
};
usePlayerStore.setState({ keyframeCache: new Map([["index.html#box", cached]]) });

const pending = api.removeAllKeyframes(selection, "box-to-0-position");
expect(commitMutationSafely).toHaveBeenCalledWith(
selection,
{ type: "remove-all-keyframes", animationId: "box-to-0-position" },
{ label: "Remove all keyframes", softReload: true },
);
expect(usePlayerStore.getState().keyframeCache.get("index.html#box")).toBe(cached);

finishCommit?.();
await pending;
expect(usePlayerStore.getState().keyframeCache.get("index.html#box")).toBe(cached);
});

it("threads one coalesce key through skipped convert reload and terminal batch edit", async () => {
const commitMutation = successfulCommitMutation();
const api = renderKeyframeOps({ commitMutation, trackGsapSaveFailure: vi.fn() });
Expand Down
14 changes: 2 additions & 12 deletions packages/studio/src/hooks/useGsapKeyframeOps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@ import {
} from "../utils/sdkCutover";
import type { KeyframeCacheEntry } from "../player/store/playerStore";
import { commitKeyframeAtTimeImpl } from "./gsapKeyframeCommit";
import { idFromSelector } from "./gsapShared";
import {
clearKeyframeCacheForElement,
readKeyframeSnapshot,
writeKeyframeCache,
} from "./gsapKeyframeCacheHelpers";
import { readKeyframeSnapshot, writeKeyframeCache } from "./gsapKeyframeCacheHelpers";
import type {
CommitMutation,
CommitMutationOptions,
Expand Down Expand Up @@ -336,11 +331,6 @@ export function useGsapKeyframeOps({
const removeAllKeyframes = useCallback(
async (selection: DomEditSelection, animationId: string) => {
const targetPath = selection.sourceFile || activeCompPath || "index.html";
// remove-all-keyframes collapses the tween to a static hold and the commit
// path doesn't return parsed animations, so the keyframe cache is never
// refreshed — clear it here so the timeline diamonds disappear immediately.
const elementId = selection.id ?? idFromSelector(selection.selector);
if (elementId) clearKeyframeCacheForElement(targetPath, elementId);
if (sdkSession && sdkDeps) {
const handled = await sdkGsapRemoveAllKeyframesPersist(
targetPath,
Expand All @@ -351,7 +341,7 @@ export function useGsapKeyframeOps({
);
if (cutoverCommittedOrThrow(handled)) return;
}
commitMutationSafely(
await commitMutationSafely(
selection,
{ type: "remove-all-keyframes", animationId },
{ label: "Remove all keyframes", softReload: true },
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// @vitest-environment happy-dom
import { act } from "react";
import { createRoot } from "react-dom/client";
import { describe, expect, it, vi } from "vitest";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { TimelineElement } from "../store/playerStore";
import {
KeyframeDiamondContextMenu,
Expand All @@ -10,6 +10,10 @@ import {

(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;

afterEach(() => {
document.body.innerHTML = "";
});

const element = { id: "box", start: 0, duration: 2, track: 0 } as unknown as TimelineElement;

const state: KeyframeDiamondContextMenuState = {
Expand Down Expand Up @@ -87,4 +91,15 @@ describe("KeyframeDiamondContextMenu", () => {
act(() => root.unmount());
host.remove();
});

// The layer-wide delete takes every keyframed tween. Opened from a diamond it
// has to stay on that diamond's own lane, or right-clicking the opacity lane
// silently clears position too.
it("deletes all keyframes from the animation that opened the menu", () => {
const onDeleteAll = vi.fn();

clickMenuItem("Delete All Keyframes", { onDeleteAll });

expect(onDeleteAll).toHaveBeenCalledExactlyOnceWith(element, "box-to-1-position");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import type { TimelineKeyframeTarget } from "./timelineKeyframeIdentity";
export interface KeyframeDiamondContextMenuState {
x: number;
y: number;
/** Timeline project session that created this portaled target. */
sessionEpoch?: number;
element: TimelineElement;
elementId: string;
percentage: number;
Expand All @@ -23,7 +25,7 @@ interface KeyframeDiamondContextMenuProps {
* floor in removeMotionPathPointInScript): an entry that silently no-ops is
* worse than no entry. */
onDelete?: (elementId: string, keyframe: TimelineKeyframeTarget) => void;
onDeleteAll: (element: TimelineElement) => void;
onDeleteAll: (element: TimelineElement, animationId?: string) => void;
/** Retime the keyframe to the current playhead, preserving its value + ease. */
onMoveToPlayhead?: (element: TimelineElement, keyframe: TimelineKeyframeTarget) => void;
}
Expand Down Expand Up @@ -93,7 +95,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={() => {
onDeleteAll(state.element);
onDeleteAll(state.element, state.animationId);
onClose();
}}
>
Expand Down
17 changes: 10 additions & 7 deletions packages/studio/src/player/components/Timeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { TimelineEmptyState } from "./TimelineEmptyState";
import { TimelineCanvas } from "./TimelineCanvas";
import { type KeyframeDiamondContextMenuState } from "./KeyframeDiamondContextMenu";
import { useTimelineClipDrag } from "./useTimelineClipDrag";
import { TimelineOverlays } from "./TimelineOverlays";
import { TimelineOverlays, type ClipContextMenuState } from "./TimelineOverlays";
import { useTimelineEditPinning } from "./useTimelineEditPinning";
import { useTimelineStackingSync } from "./useTimelineStackingSync";
import { useTimelineGeometry } from "./useTimelineGeometry";
Expand Down Expand Up @@ -141,11 +141,7 @@ export const Timeline = memo(function Timeline({
const shiftHeld = useTimelineShiftModifier();
const [showPopover, setShowPopover] = useState(false);
const [kfContextMenu, setKfContextMenu] = useState<KeyframeDiamondContextMenuState | null>(null);
const [clipContextMenu, setClipContextMenu] = useState<{
x: number;
y: number;
element: TimelineElement;
} | null>(null);
const [clipContextMenu, setClipContextMenu] = useState<ClipContextMenuState | null>(null);
const setContainerRef = useCallback((el: HTMLDivElement | null) => {
containerRef.current = el;
}, []);
Expand Down Expand Up @@ -557,7 +553,12 @@ export const Timeline = memo(function Timeline({
setSelectedElementId(el.key ?? el.id);
onSelectElement?.(el);
dismissGapMenu();
setClipContextMenu({ x: e.clientX, y: e.clientY, element: el });
setClipContextMenu({
x: e.clientX,
y: e.clientY,
element: el,
sessionEpoch: usePlayerStore.getState().timelineSessionEpoch,
});
}}
onContextMenuLane={(e, track, time) => {
if (draggedClip?.started || resizingClip) return;
Expand All @@ -568,6 +569,8 @@ export const Timeline = memo(function Timeline({
{activeTool === "razor" && razorGuideX !== null && <TimelineRazorGuide x={razorGuideX} />}
</div>
<TimelineOverlays
elements={expandedElements}
elementsRef={expandedElementsRef}
theme={theme}
showShortcutHint={showShortcutHint}
showPopover={showPopover}
Expand Down
Loading
Loading