mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 23:03:09 +00:00
fix(studio): fix array-form keyframe writes, diamond click-deselect, and nested video sync
- fs.watch's async 'error' event had no listener, crashing the preview
server on EMFILE (exhausted OS watch handles)
- moveKeyframeInScript/resizeKeyframedTweenInScript/removeAllKeyframesFromScript
required object-form keyframes: {"0%": {...}}, silently no-opping on
array-form keyframes: [{...}, {...}]
- a keyframe diamond click's auto-synthesized native click event bubbled
to the ancestor clip's onClick, which toggles selection off when the
clip is already selected (the state every diamond click happens in)
- the clip's trim-resize handles (z-index 4) visually and functionally
covered any keyframe diamond within their 14px edge strip
- synthesizeFlatTweenKeyframes didn't recognize a collapsed
duration:0 + immediateRender static hold (what remove-all-keyframes
produces) as non-animated, so it kept showing a phantom diamond after
Delete All Keyframes
- resolveMediaStartSeconds's fast path for elements with their own
data-start discarded the host composition's inherited start offset,
so a video nested inside a sub-composition played from the root
timeline's time instead of holding until its parent scene began
Fixes #1838
This commit is contained in:
@@ -1,6 +1,14 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { EventEmitter } from "node:events";
|
||||||
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
import { shouldWatchProjectFile } from "./fileWatcher.js";
|
const mockWatcher = new EventEmitter() as EventEmitter & { close: () => void };
|
||||||
|
mockWatcher.close = vi.fn();
|
||||||
|
|
||||||
|
vi.mock("node:fs", () => ({
|
||||||
|
watch: vi.fn(() => mockWatcher),
|
||||||
|
}));
|
||||||
|
|
||||||
|
const { shouldWatchProjectFile, createProjectWatcher } = await import("./fileWatcher.js");
|
||||||
|
|
||||||
describe("shouldWatchProjectFile", () => {
|
describe("shouldWatchProjectFile", () => {
|
||||||
it("watches files that can affect the project signature", () => {
|
it("watches files that can affect the project signature", () => {
|
||||||
@@ -17,3 +25,15 @@ describe("shouldWatchProjectFile", () => {
|
|||||||
expect(shouldWatchProjectFile(".hyperframes/cache.json")).toBe(false);
|
expect(shouldWatchProjectFile(".hyperframes/cache.json")).toBe(false);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("createProjectWatcher", () => {
|
||||||
|
// Regression: fs.watch can fail asynchronously (e.g. EMFILE from exhausted
|
||||||
|
// OS watch handles) via an 'error' event, not a thrown exception. An
|
||||||
|
// EventEmitter 'error' with no listener crashes the whole process — this
|
||||||
|
// must degrade gracefully instead, per the sibling synchronous-failure path.
|
||||||
|
it("does not crash the process when the underlying watcher emits 'error'", () => {
|
||||||
|
createProjectWatcher("/fake/project/dir");
|
||||||
|
expect(() => mockWatcher.emit("error", new Error("EMFILE"))).not.toThrow();
|
||||||
|
expect(mockWatcher.close).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -47,6 +47,15 @@ export function createProjectWatcher(projectDir: string): ProjectWatcher {
|
|||||||
}
|
}
|
||||||
}, DEBOUNCE_MS);
|
}, DEBOUNCE_MS);
|
||||||
});
|
});
|
||||||
|
// fs.watch can fail asynchronously too (e.g. EMFILE from exhausted OS watch
|
||||||
|
// handles) — that surfaces as an 'error' event, not a thrown exception. An
|
||||||
|
// EventEmitter 'error' with no listener crashes the whole process, so this
|
||||||
|
// listener is required for the same "degrade gracefully" the catch below
|
||||||
|
// already promises for the synchronous failure mode.
|
||||||
|
watcher.on("error", () => {
|
||||||
|
watcher?.close();
|
||||||
|
watcher = null;
|
||||||
|
});
|
||||||
} catch {
|
} catch {
|
||||||
// fs.watch may fail on some platforms — degrade gracefully (no auto-refresh)
|
// fs.watch may fail on some platforms — degrade gracefully (no auto-refresh)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -550,6 +550,56 @@ describe("initSandboxRuntimeModular", () => {
|
|||||||
expect(video.currentTime).toBe(9);
|
expect(video.currentTime).toBe(9);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression (#1838): a video authoring its OWN data-start (the normal case
|
||||||
|
// for a timed clip the studio positions on a track) took a fast literal-
|
||||||
|
// value path that skipped adding the host composition's start offset —
|
||||||
|
// unlike the no-own-data-start case above, which already went through
|
||||||
|
// resolveStartForElement and got the offset for free. The video played
|
||||||
|
// from the ROOT timeline's time instead of holding until its parent scene
|
||||||
|
// began, desyncing from the correctly-offset GSAP overlay in the same scene.
|
||||||
|
it("offsets a nested video's own data-start by its host composition's start", () => {
|
||||||
|
const root = document.createElement("div");
|
||||||
|
root.setAttribute("data-composition-id", "main");
|
||||||
|
root.setAttribute("data-root", "true");
|
||||||
|
root.setAttribute("data-width", "1920");
|
||||||
|
root.setAttribute("data-height", "1080");
|
||||||
|
document.body.appendChild(root);
|
||||||
|
|
||||||
|
const child = document.createElement("div");
|
||||||
|
child.setAttribute("data-composition-id", "scene-2");
|
||||||
|
child.setAttribute("data-start", "20");
|
||||||
|
child.setAttribute("data-duration", "16");
|
||||||
|
root.appendChild(child);
|
||||||
|
|
||||||
|
const video = document.createElement("video");
|
||||||
|
// Authored relative to scene-2's own local timeline, not the root's.
|
||||||
|
video.setAttribute("data-start", "0");
|
||||||
|
video.setAttribute("data-duration", "16");
|
||||||
|
child.appendChild(video);
|
||||||
|
Object.defineProperty(video, "duration", { value: 20, writable: true, configurable: true });
|
||||||
|
Object.defineProperty(video, "paused", { value: true, writable: true, configurable: true });
|
||||||
|
Object.defineProperty(video, "readyState", { value: 4, writable: true, configurable: true });
|
||||||
|
Object.defineProperty(video, "currentTime", { value: 0, writable: true, configurable: true });
|
||||||
|
video.load = () => {};
|
||||||
|
video.play = () => Promise.resolve();
|
||||||
|
|
||||||
|
window.__timelines = {
|
||||||
|
main: createMockTimeline(40),
|
||||||
|
"scene-2": createMockTimeline(16),
|
||||||
|
};
|
||||||
|
|
||||||
|
initSandboxRuntimeModular();
|
||||||
|
|
||||||
|
const player = window.__player;
|
||||||
|
expect(player).toBeDefined();
|
||||||
|
|
||||||
|
// Root t=25 is 5s into scene-2 (which starts at root t=20) — the video
|
||||||
|
// must be 5s into its own local playback, not 25s (root time).
|
||||||
|
player?.seek(25);
|
||||||
|
|
||||||
|
expect(video.currentTime).toBe(5);
|
||||||
|
});
|
||||||
|
|
||||||
it("updates visibility for timed elements inside nested compositions", () => {
|
it("updates visibility for timed elements inside nested compositions", () => {
|
||||||
const root = document.createElement("div");
|
const root = document.createElement("div");
|
||||||
root.setAttribute("data-composition-id", "main");
|
root.setAttribute("data-composition-id", "main");
|
||||||
|
|||||||
@@ -496,7 +496,16 @@ export function initSandboxRuntimeModular(): void {
|
|||||||
|
|
||||||
const resolveMediaStartSeconds = (element: Element, fallback = 0): number => {
|
const resolveMediaStartSeconds = (element: Element, fallback = 0): number => {
|
||||||
if (!element.hasAttribute("data-hf-auto-start") && element.hasAttribute("data-start")) {
|
if (!element.hasAttribute("data-hf-auto-start") && element.hasAttribute("data-start")) {
|
||||||
return Math.max(0, Number(element.getAttribute("data-start") ?? 0) || 0);
|
// `data-start` is authored relative to the media element's OWN sub-
|
||||||
|
// composition, not the root timeline — `fallback` carries the host
|
||||||
|
// composition's resolved absolute start (see syncMediaForCurrentState's
|
||||||
|
// inheritedStart), so it must be added, not discarded. Skipping it made
|
||||||
|
// a nested video play from root t=0 instead of holding until its
|
||||||
|
// parent scene began (issue #1838) — resolveStartForElement's own
|
||||||
|
// absolute-expression branch already adds this same host offset, this
|
||||||
|
// fast literal-value path just didn't.
|
||||||
|
const own = Math.max(0, Number(element.getAttribute("data-start") ?? 0) || 0);
|
||||||
|
return own + fallback;
|
||||||
}
|
}
|
||||||
return resolveStartForElement(element, fallback);
|
return resolveStartForElement(element, fallback);
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -2338,7 +2338,11 @@ export function moveKeyframeInScript(
|
|||||||
): string {
|
): string {
|
||||||
const loc = locateAnimationWithFallback(script, animationId);
|
const loc = locateAnimationWithFallback(script, animationId);
|
||||||
if (!loc) return script;
|
if (!loc) return script;
|
||||||
const kfNode = findKeyframesObjectNode(loc.target.call.varsArg);
|
// Array-form keyframes can't host an arbitrary destination percentage —
|
||||||
|
// normalize to object form in place first (mirrors addKeyframeToScript).
|
||||||
|
const kfNode =
|
||||||
|
findKeyframesObjectNode(loc.target.call.varsArg) ??
|
||||||
|
convertArrayKeyframesToObjectNode(loc.target.call.varsArg);
|
||||||
if (!kfNode) return script;
|
if (!kfNode) return script;
|
||||||
|
|
||||||
const match = findKeyframePropByPct(kfNode, fromPercentage);
|
const match = findKeyframePropByPct(kfNode, fromPercentage);
|
||||||
@@ -2395,7 +2399,11 @@ export function resizeKeyframedTweenInScript(
|
|||||||
): string {
|
): string {
|
||||||
const loc = locateAnimationWithFallback(script, animationId);
|
const loc = locateAnimationWithFallback(script, animationId);
|
||||||
if (!loc) return script;
|
if (!loc) return script;
|
||||||
const kfNode = findKeyframesObjectNode(loc.target.call.varsArg);
|
// Array-form keyframes can't host an arbitrary re-keyed percentage —
|
||||||
|
// normalize to object form in place first (mirrors addKeyframeToScript).
|
||||||
|
const kfNode =
|
||||||
|
findKeyframesObjectNode(loc.target.call.varsArg) ??
|
||||||
|
convertArrayKeyframesToObjectNode(loc.target.call.varsArg);
|
||||||
if (!kfNode) return script;
|
if (!kfNode) return script;
|
||||||
|
|
||||||
const seen = new Set<AstNode>();
|
const seen = new Set<AstNode>();
|
||||||
@@ -2595,7 +2603,13 @@ export function convertToKeyframesInScript(
|
|||||||
export function removeAllKeyframesFromScript(script: string, animationId: string): string {
|
export function removeAllKeyframesFromScript(script: string, animationId: string): string {
|
||||||
let loc = locateAnimationWithFallback(script, animationId);
|
let loc = locateAnimationWithFallback(script, animationId);
|
||||||
if (!loc) return script;
|
if (!loc) return script;
|
||||||
const kfNode = findKeyframesObjectNode(loc.target.call.varsArg);
|
// Array-form keyframes have no percentage-keyed props for
|
||||||
|
// filterPercentageProps to read — normalize to object form first (mirrors
|
||||||
|
// addKeyframeToScript/moveKeyframeInScript), otherwise this silently no-ops
|
||||||
|
// on every array-form tween.
|
||||||
|
const kfNode =
|
||||||
|
findKeyframesObjectNode(loc.target.call.varsArg) ??
|
||||||
|
convertArrayKeyframesToObjectNode(loc.target.call.varsArg);
|
||||||
if (!kfNode) return script;
|
if (!kfNode) return script;
|
||||||
|
|
||||||
const kfEntries = filterPercentageProps(kfNode)
|
const kfEntries = filterPercentageProps(kfNode)
|
||||||
|
|||||||
@@ -1216,6 +1216,34 @@ describe("parity: moveKeyframeInScript (recast vs acorn)", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: array-form `keyframes: [...]` has no explicit percentages, so
|
||||||
|
// locateWithKeyframes/findKeyframesObjectNode (which only match the object
|
||||||
|
// form) resolved to nothing and the move silently no-op'd — Studio's "Move to
|
||||||
|
// Playhead" and drag-to-retime did nothing on any array-authored tween. Both
|
||||||
|
// writers now normalize array → object form first (mirrors addKeyframeToScript).
|
||||||
|
describe("moveKeyframeInScript: array-form keyframes (recast + acorn parity)", () => {
|
||||||
|
for (const [label, move] of [
|
||||||
|
["acorn", moveKeyframeAcorn],
|
||||||
|
["recast", moveKeyframeRecast],
|
||||||
|
] as const) {
|
||||||
|
it(`${label}: normalizes the array then retimes the moved keyframe`, () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const out = move(KF_ADD_ARRAY_SCRIPT, id, 50, 75);
|
||||||
|
expect(out).not.toBe(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const kfs = shapeOf(out).keyframes?.keyframes ?? [];
|
||||||
|
expect(kfs.map((k) => k.percentage)).toEqual([0, 75, 100]);
|
||||||
|
expect(kfs.find((k) => k.percentage === 75)!.properties).toEqual({ x: 50, y: 80 });
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("parity: both writers reparse to the same model", () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
expect(modelOf(moveKeyframeAcorn(KF_ADD_ARRAY_SCRIPT, id, 50, 75))).toEqual(
|
||||||
|
modelOf(moveKeyframeRecast(KF_ADD_ARRAY_SCRIPT, id, 50, 75)),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ── resizeKeyframedTweenInScript (boundary drag: re-key + grow window) ────────
|
// ── resizeKeyframedTweenInScript (boundary drag: re-key + grow window) ────────
|
||||||
// Boundary drag-to-retime grows/shifts the tween window and RE-KEYS keyframes in
|
// Boundary drag-to-retime grows/shifts the tween window and RE-KEYS keyframes in
|
||||||
// place. Unlike replace-with-keyframes (array rebuild), it must preserve author
|
// place. Unlike replace-with-keyframes (array rebuild), it must preserve author
|
||||||
@@ -1272,6 +1300,67 @@ describe("resizeKeyframedTweenInScript: preserves author intent (acorn + recast)
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: same array-form gap as moveKeyframeInScript above — boundary
|
||||||
|
// drag-to-retime re-keys existing keyframes to arbitrary percentages, which an
|
||||||
|
// array can't host. Both writers now normalize array → object form first.
|
||||||
|
const RESIZE_ARRAY_REMAP = [
|
||||||
|
{ from: 0, to: 0 },
|
||||||
|
{ from: 50, to: 25 },
|
||||||
|
{ from: 100, to: 100 },
|
||||||
|
];
|
||||||
|
|
||||||
|
describe("resizeKeyframedTweenInScript: array-form keyframes (recast + acorn parity)", () => {
|
||||||
|
for (const [label, resize] of [
|
||||||
|
["acorn", resizeKeyframedTweenAcorn],
|
||||||
|
["recast", resizeKeyframedTweenRecast],
|
||||||
|
] as const) {
|
||||||
|
it(`${label}: normalizes the array then re-keys to the remapped percentages`, () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const out = resize(KF_ADD_ARRAY_SCRIPT, id, 0.2, 2, RESIZE_ARRAY_REMAP);
|
||||||
|
expect(out).not.toBe(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const kfs = shapeOf(out).keyframes?.keyframes ?? [];
|
||||||
|
expect(kfs.map((k) => k.percentage)).toEqual([0, 25, 100]);
|
||||||
|
expect(kfs.find((k) => k.percentage === 25)!.properties).toEqual({ x: 50, y: 80 });
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("parity: both writers reparse to the same model", () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
expect(
|
||||||
|
modelOf(resizeKeyframedTweenAcorn(KF_ADD_ARRAY_SCRIPT, id, 0.2, 2, RESIZE_ARRAY_REMAP)),
|
||||||
|
).toEqual(
|
||||||
|
modelOf(resizeKeyframedTweenRecast(KF_ADD_ARRAY_SCRIPT, id, 0.2, 2, RESIZE_ARRAY_REMAP)),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("removeAllKeyframesFromScript: array-form keyframes (recast + acorn parity)", () => {
|
||||||
|
// Regression: the recast writer required an object-form `keyframes` node
|
||||||
|
// before doing anything, so array-form tweens silently no-op'd — the studio
|
||||||
|
// clears its keyframe cache optimistically on delete-all, so the diamonds
|
||||||
|
// vanished from the UI while the script kept every keyframe untouched.
|
||||||
|
for (const [label, removeAll] of [
|
||||||
|
["acorn", removeAllAcorn],
|
||||||
|
["recast", removeAllRecast],
|
||||||
|
] as const) {
|
||||||
|
it(`${label}: normalizes the array then collapses to the last keyframe`, () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const out = removeAll(KF_ADD_ARRAY_SCRIPT, id);
|
||||||
|
expect(out).not.toBe(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
const shape = shapeOf(out);
|
||||||
|
expect(shape.keyframes).toBeUndefined();
|
||||||
|
expect(shape.properties).toEqual({ x: 100, y: 0 });
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
it("parity: both writers reparse to the same model", () => {
|
||||||
|
const id = acornId(KF_ADD_ARRAY_SCRIPT);
|
||||||
|
expect(modelOf(removeAllAcorn(KF_ADD_ARRAY_SCRIPT, id))).toEqual(
|
||||||
|
modelOf(removeAllRecast(KF_ADD_ARRAY_SCRIPT, id)),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
// ── addAnimationWithKeyframesToScript parity (recast vs acorn) ───────────────
|
// ── addAnimationWithKeyframesToScript parity (recast vs acorn) ───────────────
|
||||||
// WS-3.C add path: both writers insert a new keyframed tl.to() call. The
|
// WS-3.C add path: both writers insert a new keyframed tl.to() call. The
|
||||||
// inserted statement's authored model (selector, keyframes, duration, ease,
|
// inserted statement's authored model (selector, keyframes, duration, ease,
|
||||||
|
|||||||
@@ -1209,17 +1209,20 @@ export function moveKeyframeInScript(
|
|||||||
fromPercentage: number,
|
fromPercentage: number,
|
||||||
toPercentage: number,
|
toPercentage: number,
|
||||||
): string {
|
): string {
|
||||||
const located = locateWithKeyframes(script, animationId);
|
// ensureKeyframesNode (not locateWithKeyframes) so array-form `keyframes: [...]`
|
||||||
|
// tweens normalize to percentage-object form first — locateWithKeyframes alone
|
||||||
|
// bails on ArrayExpression, silently no-op'ing every move on an array-form tween.
|
||||||
|
const located = ensureKeyframesNode(script, animationId);
|
||||||
if (!located) return script;
|
if (!located) return script;
|
||||||
const { kfNode } = located;
|
const { script: src, kfNode } = located;
|
||||||
|
|
||||||
const match = findKfPropByPct(kfNode, fromPercentage);
|
const match = findKfPropByPct(kfNode, fromPercentage);
|
||||||
if (!match) return script;
|
if (!match) return src;
|
||||||
// No-op ONLY for a negligible move (matches the drag's NOOP_EPSILON). The old
|
// No-op ONLY for a negligible move (matches the drag's NOOP_EPSILON). The old
|
||||||
// `collision.prop === match.prop` guard dropped EVERY sub-PCT_TOLERANCE (2%)
|
// `collision.prop === match.prop` guard dropped EVERY sub-PCT_TOLERANCE (2%)
|
||||||
// retime, because findKfPropByPct resolves the destination back onto the
|
// retime, because findKfPropByPct resolves the destination back onto the
|
||||||
// from-keyframe — so a deliberate 1% drag committed nothing.
|
// from-keyframe — so a deliberate 1% drag committed nothing.
|
||||||
if (Math.abs(fromPercentage - toPercentage) < MOVE_NOOP_EPSILON_PCT) return script;
|
if (Math.abs(fromPercentage - toPercentage) < MOVE_NOOP_EPSILON_PCT) return src;
|
||||||
// A destination keyframe is only a real collision (overwrite) when it's a
|
// A destination keyframe is only a real collision (overwrite) when it's a
|
||||||
// DIFFERENT keyframe; resolving back onto the from-keyframe is not.
|
// DIFFERENT keyframe; resolving back onto the from-keyframe is not.
|
||||||
const dest = findKfPropByPct(kfNode, toPercentage);
|
const dest = findKfPropByPct(kfNode, toPercentage);
|
||||||
@@ -1234,15 +1237,15 @@ export function moveKeyframeInScript(
|
|||||||
if (collision && prop === collision.prop) continue;
|
if (collision && prop === collision.prop) continue;
|
||||||
const pct = percentageFromKey(propKeyName(prop) ?? "");
|
const pct = percentageFromKey(propKeyName(prop) ?? "");
|
||||||
if (Number.isNaN(pct)) continue;
|
if (Number.isNaN(pct)) continue;
|
||||||
entries.push({ pct, record: valueNodeToRecord(prop.value, script) });
|
entries.push({ pct, record: valueNodeToRecord(prop.value, src) });
|
||||||
}
|
}
|
||||||
entries.push({ pct: toPercentage, record: valueNodeToRecord(match.prop.value, script) });
|
entries.push({ pct: toPercentage, record: valueNodeToRecord(match.prop.value, src) });
|
||||||
entries.sort((a, b) => a.pct - b.pct);
|
entries.sort((a, b) => a.pct - b.pct);
|
||||||
|
|
||||||
const body = entries
|
const body = entries
|
||||||
.map((e) => `${JSON.stringify(`${e.pct}%`)}: ${recordToCode(e.record)}`)
|
.map((e) => `${JSON.stringify(`${e.pct}%`)}: ${recordToCode(e.record)}`)
|
||||||
.join(", ");
|
.join(", ");
|
||||||
const ms = new MagicString(script);
|
const ms = new MagicString(src);
|
||||||
ms.overwrite(kfNode.start, kfNode.end, `{ ${body} }`);
|
ms.overwrite(kfNode.start, kfNode.end, `{ ${body} }`);
|
||||||
return ms.toString();
|
return ms.toString();
|
||||||
}
|
}
|
||||||
@@ -1266,9 +1269,11 @@ export function resizeKeyframedTweenInScript(
|
|||||||
newDuration: number,
|
newDuration: number,
|
||||||
pctRemap: ReadonlyArray<{ from: number; to: number }>,
|
pctRemap: ReadonlyArray<{ from: number; to: number }>,
|
||||||
): string {
|
): string {
|
||||||
const located = locateWithKeyframes(script, animationId);
|
// ensureKeyframesNode (not locateWithKeyframes) so array-form `keyframes: [...]`
|
||||||
|
// tweens normalize to percentage-object form first — see moveKeyframeInScript.
|
||||||
|
const located = ensureKeyframesNode(script, animationId);
|
||||||
if (!located) return script;
|
if (!located) return script;
|
||||||
const { target, kfNode } = located;
|
const { script: src, target, kfNode } = located;
|
||||||
|
|
||||||
// Resolve every re-key against the ORIGINAL AST first (offsets stay stable),
|
// Resolve every re-key against the ORIGINAL AST first (offsets stay stable),
|
||||||
// then splice — distinct key nodes, so the overwrites never overlap. A Set
|
// then splice — distinct key nodes, so the overwrites never overlap. A Set
|
||||||
@@ -1282,7 +1287,7 @@ export function resizeKeyframedTweenInScript(
|
|||||||
edits.push({ keyNode: match.prop.key, to });
|
edits.push({ keyNode: match.prop.key, to });
|
||||||
}
|
}
|
||||||
|
|
||||||
const ms = new MagicString(script);
|
const ms = new MagicString(src);
|
||||||
for (const { keyNode, to } of edits) {
|
for (const { keyNode, to } of edits) {
|
||||||
ms.overwrite(keyNode.start, keyNode.end, JSON.stringify(`${to}%`));
|
ms.overwrite(keyNode.start, keyNode.end, JSON.stringify(`${to}%`));
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,55 @@
|
|||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
|
||||||
|
import { synthesizeFlatTweenKeyframes } from "./gsapTweenSynth";
|
||||||
|
|
||||||
|
function anim(overrides: Partial<GsapAnimation>): GsapAnimation {
|
||||||
|
return {
|
||||||
|
id: "a1",
|
||||||
|
targetSelector: "#title",
|
||||||
|
method: "to",
|
||||||
|
position: 0,
|
||||||
|
properties: {},
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("synthesizeFlatTweenKeyframes", () => {
|
||||||
|
it("returns null for a set() static hold", () => {
|
||||||
|
expect(synthesizeFlatTweenKeyframes(anim({ method: "set", properties: { x: 5 } }))).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression: removeAllKeyframesFromScript collapses a keyframed tween to
|
||||||
|
// `tl.to(..., { duration: 0, immediateRender: true })` — a static hold with
|
||||||
|
// the same "not an animation" semantics as set(), but a different method
|
||||||
|
// string. Before this fix, only position-only (x/y) collapses were treated
|
||||||
|
// as holds elsewhere; a collapse to scale/opacity (or anything else) still
|
||||||
|
// synthesized a phantom keyframe diamond after "Delete All Keyframes".
|
||||||
|
it("returns null for a to() collapsed to a zero-duration immediateRender hold", () => {
|
||||||
|
const collapsed = anim({
|
||||||
|
method: "to",
|
||||||
|
duration: 0,
|
||||||
|
// Both parsers encode a literal `immediateRender: true` as this raw
|
||||||
|
// source string, not a boolean — see gsapParser.ts/gsapParserAcorn.ts.
|
||||||
|
extras: { immediateRender: "__raw:true" },
|
||||||
|
properties: { scale: 1, opacity: 1 },
|
||||||
|
});
|
||||||
|
expect(synthesizeFlatTweenKeyframes(collapsed)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still synthesizes keyframes for a genuine animated to() tween", () => {
|
||||||
|
const out = synthesizeFlatTweenKeyframes(
|
||||||
|
anim({ method: "to", duration: 1, properties: { opacity: 1 } }),
|
||||||
|
);
|
||||||
|
expect(out).not.toBeNull();
|
||||||
|
expect(out?.keyframes.map((k) => k.percentage)).toEqual([0, 100]);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("still synthesizes keyframes for a duration:0 tween that isn't an immediateRender hold", () => {
|
||||||
|
// duration:0 alone isn't enough — only paired with immediateRender does it
|
||||||
|
// mean "this is a static hold, not an animation".
|
||||||
|
const out = synthesizeFlatTweenKeyframes(
|
||||||
|
anim({ method: "to", duration: 0, properties: { opacity: 1 } }),
|
||||||
|
);
|
||||||
|
expect(out).not.toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -23,12 +23,18 @@ export function deduplicateKeyframes(
|
|||||||
|
|
||||||
// fallow-ignore-next-line complexity
|
// fallow-ignore-next-line complexity
|
||||||
export function synthesizeFlatTweenKeyframes(anim: GsapAnimation): GsapKeyframesData | null {
|
export function synthesizeFlatTweenKeyframes(anim: GsapAnimation): GsapKeyframesData | null {
|
||||||
if (anim.method === "set") {
|
// Both parsers store extras as raw source text (`__raw:${code}`) so
|
||||||
// A `set` is a STATIC HOLD — a value applied at one point, not an animated
|
// non-editable config like `stagger: {...}` survives verbatim — a literal
|
||||||
// keyframe. It must NOT synthesize a keyframe, or the timeline + panel show a
|
// `immediateRender: true` prints as exactly this string, not a boolean.
|
||||||
// phantom diamond for a value that doesn't animate. This holds for a base
|
const hasImmediateRenderHold = anim.extras?.immediateRender === "__raw:true";
|
||||||
// `gsap.set` (off-timeline) AND an on-timeline `tl.set`, and aligns the AST
|
if (anim.method === "set" || (anim.duration === 0 && hasImmediateRenderHold)) {
|
||||||
// path with the runtime scan, which already skips every zero-duration set.
|
// A `set` — or a `to()`/`from()` collapsed to a zero-duration
|
||||||
|
// immediateRender hold (what removeAllKeyframesFromScript collapses a
|
||||||
|
// keyframed tween to) — is a STATIC HOLD: a value applied at one point,
|
||||||
|
// not an animated keyframe. It must NOT synthesize a keyframe, or the
|
||||||
|
// timeline + panel show a phantom diamond for a value that doesn't
|
||||||
|
// animate. This aligns the AST path with the runtime scan, which already
|
||||||
|
// skips every zero-duration set.
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
const toProps = anim.properties;
|
const toProps = anim.properties;
|
||||||
|
|||||||
@@ -446,6 +446,7 @@ export const TimelineCanvas = memo(function TimelineCanvas({
|
|||||||
onShiftClickKeyframe={onShiftClickKeyframe}
|
onShiftClickKeyframe={onShiftClickKeyframe}
|
||||||
onContextMenuKeyframe={onContextMenuKeyframe}
|
onContextMenuKeyframe={onContextMenuKeyframe}
|
||||||
onMoveKeyframe={onMoveKeyframe}
|
onMoveKeyframe={onMoveKeyframe}
|
||||||
|
suppressClickRef={suppressClickRef}
|
||||||
/>
|
/>
|
||||||
)}
|
)}
|
||||||
</TimelineClip>
|
</TimelineClip>
|
||||||
|
|||||||
@@ -70,4 +70,146 @@ describe("TimelineClipDiamonds", () => {
|
|||||||
expect(onClickKeyframe).not.toHaveBeenCalled();
|
expect(onClickKeyframe).not.toHaveBeenCalled();
|
||||||
act(() => root.unmount());
|
act(() => root.unmount());
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: once the clip is selected, canDrag arms on every diamond
|
||||||
|
// press. A real click's few px of mouse/trackpad jitter then resolves (via
|
||||||
|
// the neighbour clamp) back onto ~the same position — "noop", not "move" —
|
||||||
|
// which fell through neither branch and silently did nothing: no
|
||||||
|
// selection, no retime. It must still count as the click it was.
|
||||||
|
it("treats a drag-armed press that resolves to a no-op move as a click", () => {
|
||||||
|
const onClickKeyframe = vi.fn();
|
||||||
|
const onMoveKeyframe = vi.fn();
|
||||||
|
const host = document.createElement("div");
|
||||||
|
document.body.append(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
act(() => {
|
||||||
|
root.render(
|
||||||
|
<TimelineClipDiamonds
|
||||||
|
keyframesData={{
|
||||||
|
format: "percentage",
|
||||||
|
keyframes: [
|
||||||
|
{ percentage: 0, properties: { x: 0 } },
|
||||||
|
{ percentage: 50, properties: { x: 100 } },
|
||||||
|
],
|
||||||
|
}}
|
||||||
|
clipWidthPx={5000}
|
||||||
|
clipHeightPx={48}
|
||||||
|
accentColor="#4ba3d2"
|
||||||
|
isSelected
|
||||||
|
currentPercentage={0}
|
||||||
|
elementId="clip-1"
|
||||||
|
selectedKeyframes={new Set()}
|
||||||
|
onClickKeyframe={onClickKeyframe}
|
||||||
|
onMoveKeyframe={onMoveKeyframe}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
const diamond = host.querySelector<HTMLButtonElement>('button[title="50%"]');
|
||||||
|
expect(diamond).not.toBeNull();
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
diamond!.dispatchEvent(
|
||||||
|
pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 100 }),
|
||||||
|
);
|
||||||
|
// 4px of travel at a 5000px clip width is ~0.08 clip-% — above the drag
|
||||||
|
// threshold (so resolveKeyframeDrag doesn't short-circuit to "click"
|
||||||
|
// itself) but below the no-op epsilon once neighbour-clamped.
|
||||||
|
diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 104 }));
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(onClickKeyframe).toHaveBeenCalledWith(50);
|
||||||
|
expect(onMoveKeyframe).not.toHaveBeenCalled();
|
||||||
|
act(() => root.unmount());
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression: a genuine retime (drag far enough to actually move the
|
||||||
|
// keyframe) committed the move but never selected/parked on the result —
|
||||||
|
// the diamond it was just dragged looked exactly like one nothing happened
|
||||||
|
// to. Select it at its NEW position too.
|
||||||
|
it("selects the keyframe at its new position after a real drag-retime", () => {
|
||||||
|
const onClickKeyframe = vi.fn();
|
||||||
|
const onMoveKeyframe = vi.fn();
|
||||||
|
const host = document.createElement("div");
|
||||||
|
document.body.append(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
act(() => {
|
||||||
|
root.render(
|
||||||
|
<TimelineClipDiamonds
|
||||||
|
keyframesData={{
|
||||||
|
format: "percentage",
|
||||||
|
keyframes: [
|
||||||
|
{ percentage: 0, properties: { x: 0 } },
|
||||||
|
{ percentage: 50, properties: { x: 100 } },
|
||||||
|
],
|
||||||
|
}}
|
||||||
|
clipWidthPx={200}
|
||||||
|
clipHeightPx={48}
|
||||||
|
accentColor="#4ba3d2"
|
||||||
|
isSelected
|
||||||
|
currentPercentage={0}
|
||||||
|
elementId="clip-1"
|
||||||
|
selectedKeyframes={new Set()}
|
||||||
|
onClickKeyframe={onClickKeyframe}
|
||||||
|
onMoveKeyframe={onMoveKeyframe}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
const diamond = host.querySelector<HTMLButtonElement>('button[title="50%"]');
|
||||||
|
expect(diamond).not.toBeNull();
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
diamond!.dispatchEvent(
|
||||||
|
pointerEvent("pointerdown", { bubbles: true, button: 0, clientX: 100 }),
|
||||||
|
);
|
||||||
|
// 4px at a 200px clip width is 2 clip-% — well past the no-op epsilon,
|
||||||
|
// a real retime.
|
||||||
|
diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0, clientX: 104 }));
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(onMoveKeyframe).toHaveBeenCalledWith("clip-1", 50, 52);
|
||||||
|
expect(onClickKeyframe).toHaveBeenCalledWith(52);
|
||||||
|
act(() => root.unmount());
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression: onClickKeyframe's state updates can re-render the diamond
|
||||||
|
// button out from under the gesture before the browser auto-synthesizes the
|
||||||
|
// "click" event that follows a button's pointerdown+pointerup. That orphaned
|
||||||
|
// click then bubbles to the ancestor clip's onClick, which toggles selection
|
||||||
|
// off whenever the clip is already selected — the state a diamond click
|
||||||
|
// always happens in — so every keyframe click immediately deselected its
|
||||||
|
// own clip. suppressClickRef lets that ancestor ignore the stray click.
|
||||||
|
it("arms suppressClickRef synchronously on a keyframe click", () => {
|
||||||
|
const suppressClickRef = { current: false };
|
||||||
|
const host = document.createElement("div");
|
||||||
|
document.body.append(host);
|
||||||
|
const root = createRoot(host);
|
||||||
|
act(() => {
|
||||||
|
root.render(
|
||||||
|
<TimelineClipDiamonds
|
||||||
|
keyframesData={{
|
||||||
|
format: "percentage",
|
||||||
|
keyframes: [{ percentage: 50, properties: { x: 100 } }],
|
||||||
|
}}
|
||||||
|
clipWidthPx={200}
|
||||||
|
clipHeightPx={48}
|
||||||
|
accentColor="#4ba3d2"
|
||||||
|
isSelected
|
||||||
|
currentPercentage={0}
|
||||||
|
elementId="clip-1"
|
||||||
|
selectedKeyframes={new Set()}
|
||||||
|
onClickKeyframe={vi.fn()}
|
||||||
|
suppressClickRef={suppressClickRef}
|
||||||
|
/>,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
const diamond = host.querySelector<HTMLButtonElement>('button[title="50%"]');
|
||||||
|
expect(diamond).not.toBeNull();
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
diamond!.dispatchEvent(pointerEvent("pointerup", { bubbles: true, button: 0 }));
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(suppressClickRef.current).toBe(true);
|
||||||
|
act(() => root.unmount());
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -45,6 +45,10 @@ interface TimelineClipDiamondsProps {
|
|||||||
fromClipPercentage: number,
|
fromClipPercentage: number,
|
||||||
toClipPercentage: number,
|
toClipPercentage: number,
|
||||||
) => void;
|
) => void;
|
||||||
|
/** Set while resolving a diamond press so the ancestor clip's onClick (which
|
||||||
|
* toggles selection off when already selected) ignores the native "click"
|
||||||
|
* the browser auto-synthesizes after this button's pointerdown+pointerup. */
|
||||||
|
suppressClickRef?: React.RefObject<boolean>;
|
||||||
}
|
}
|
||||||
|
|
||||||
const DIAMOND_RATIO = 0.8;
|
const DIAMOND_RATIO = 0.8;
|
||||||
@@ -76,6 +80,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
onShiftClickKeyframe,
|
onShiftClickKeyframe,
|
||||||
onContextMenuKeyframe,
|
onContextMenuKeyframe,
|
||||||
onMoveKeyframe,
|
onMoveKeyframe,
|
||||||
|
suppressClickRef,
|
||||||
}: TimelineClipDiamondsProps) {
|
}: TimelineClipDiamondsProps) {
|
||||||
// Hooks must run before the early return below.
|
// Hooks must run before the early return below.
|
||||||
const dragRef = useRef<DragState | null>(null);
|
const dragRef = useRef<DragState | null>(null);
|
||||||
@@ -83,6 +88,21 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
// (that optimistic hold was the #1763 flake). The atomic move-keyframe commit
|
// (that optimistic hold was the #1763 flake). The atomic move-keyframe commit
|
||||||
// on drop re-keys the diamond from source.
|
// on drop re-keys the diamond from source.
|
||||||
const [preview, setPreview] = useState<{ kfKey: string; clipPct: number } | null>(null);
|
const [preview, setPreview] = useState<{ kfKey: string; clipPct: number } | null>(null);
|
||||||
|
// The button element can re-render (reposition/unmount) synchronously from
|
||||||
|
// the state updates onClickKeyframe/onMoveKeyframe trigger, before the
|
||||||
|
// browser gets to auto-synthesize the "click" event that normally follows
|
||||||
|
// pointerdown+pointerup on a button. That orphaned click then fires on
|
||||||
|
// whatever ancestor is still there — the clip wrapper — whose own onClick
|
||||||
|
// toggles selection off when the clip is already selected (the state a
|
||||||
|
// diamond click always happens in). Suppressing it here is the same fix
|
||||||
|
// already used for clip drag/resize in useTimelineClipDrag.ts.
|
||||||
|
const suppressNextClick = () => {
|
||||||
|
if (!suppressClickRef) return;
|
||||||
|
suppressClickRef.current = true;
|
||||||
|
requestAnimationFrame(() => {
|
||||||
|
suppressClickRef.current = false;
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
if (clipWidthPx < 20) return null;
|
if (clipWidthPx < 20) return null;
|
||||||
|
|
||||||
@@ -102,7 +122,19 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
const canDrag = isSelected && !!onMoveKeyframe;
|
const canDrag = isSelected && !!onMoveKeyframe;
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="absolute inset-0" style={{ zIndex: 3, pointerEvents: "none" }}>
|
<div
|
||||||
|
className="absolute inset-0"
|
||||||
|
style={{
|
||||||
|
// Above the clip's trim-handle strips (TimelineClip.tsx, z-index 4) so
|
||||||
|
// a keyframe sitting in the first/last ~14px of the clip stays
|
||||||
|
// clickable instead of being covered by the resize handle. This div
|
||||||
|
// establishes its own stacking context (position + z-index), so the
|
||||||
|
// diamonds' own z-index (1/2) can't escape it on their own — the bump
|
||||||
|
// has to happen here.
|
||||||
|
zIndex: 5,
|
||||||
|
pointerEvents: "none",
|
||||||
|
}}
|
||||||
|
>
|
||||||
{sorted.map((kf, i) => {
|
{sorted.map((kf, i) => {
|
||||||
if (i === 0) return null;
|
if (i === 0) return null;
|
||||||
const prev = sorted[i - 1]!;
|
const prev = sorted[i - 1]!;
|
||||||
@@ -179,6 +211,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
// No drag armed (canDrag false / non-primary press) → treat as a click.
|
// No drag armed (canDrag false / non-primary press) → treat as a click.
|
||||||
if (!d || d.kfKey !== kfKey) {
|
if (!d || d.kfKey !== kfKey) {
|
||||||
if (e.button !== 0) return;
|
if (e.button !== 0) return;
|
||||||
|
suppressNextClick();
|
||||||
if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage);
|
if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage);
|
||||||
else onClickKeyframe?.(kf.percentage);
|
else onClickKeyframe?.(kf.percentage);
|
||||||
return;
|
return;
|
||||||
@@ -187,6 +220,7 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
dragRef.current = null;
|
dragRef.current = null;
|
||||||
setPreview(null);
|
setPreview(null);
|
||||||
e.currentTarget.releasePointerCapture?.(e.pointerId);
|
e.currentTarget.releasePointerCapture?.(e.pointerId);
|
||||||
|
suppressNextClick();
|
||||||
const res = resolveKeyframeDrag({
|
const res = resolveKeyframeDrag({
|
||||||
pointerDownX: d.startX,
|
pointerDownX: d.startX,
|
||||||
pointerUpX: e.clientX,
|
pointerUpX: e.clientX,
|
||||||
@@ -195,11 +229,20 @@ export const TimelineClipDiamonds = memo(function TimelineClipDiamonds({
|
|||||||
draggedIndex: i,
|
draggedIndex: i,
|
||||||
sortedClipPcts,
|
sortedClipPcts,
|
||||||
});
|
});
|
||||||
if (res.kind === "click") {
|
if (res.kind === "click" || res.kind === "noop") {
|
||||||
|
// "noop" is a press with enough pointer jitter to arm a drag (canDrag
|
||||||
|
// is on for every diamond once the clip is selected) that resolved
|
||||||
|
// back onto ~the same position — no real retime, so treat it as the
|
||||||
|
// click it was. Otherwise a normal click with a few px of mouse/
|
||||||
|
// trackpad drift silently does nothing: no selection, no move.
|
||||||
if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage);
|
if (e.shiftKey) onShiftClickKeyframe?.(elementId, kf.percentage);
|
||||||
else onClickKeyframe?.(kf.percentage);
|
else onClickKeyframe?.(kf.percentage);
|
||||||
} else if (res.kind === "move" && res.toClipPct != null) {
|
} else if (res.kind === "move" && res.toClipPct != null) {
|
||||||
onMoveKeyframe?.(elementId, d.fromClipPct, res.toClipPct);
|
onMoveKeyframe?.(elementId, d.fromClipPct, res.toClipPct);
|
||||||
|
// A retime still targeted this exact diamond — park/select it at its
|
||||||
|
// new position, same as a plain click, or a drag that actually moved
|
||||||
|
// something looks identical to one that silently did nothing.
|
||||||
|
onClickKeyframe?.(res.toClipPct);
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user