mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): target one element when adding a keyframe at the playhead
"Add keyframe at playhead" on an element with no id authored the bare class
buildStableSelector hands back, so one add on a `.group` wrote
`tl.to(".group", ...)`: a tween that animates all five siblings and that
resolveSelectorElementIds reads back as all five, collapsing their timeline
rows into one. It survived a reload, so the written file stayed un-editable.
writeTargetSelector is the write-side counterpart to selectorFromSelection
(which must keep returning the exact string findTweenAtTime compares against).
It resolves the element's own identity to a selector that addresses exactly
one element: `#id`, else `[data-hf-id="..."]`, else the selection's selector
when it is already unique, else a `:nth-child` path anchored on the nearest
identifiable ancestor (the selector + selectorIndex pair, resolved through the
DOM the index was counted in).
Applied to the two paths that author a NEW tween: the no-animation branch of
useEnableKeyframes and commitKeyframeAtTimeImpl. replace-with-keyframes still
writes the selection's own selector, since retargeting a tween the author
aimed at a whole group is a different decision from adding a keyframe.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import type { GsapAnimation } from "@hyperframes/core/gsap-parser";
|
||||
import type { DomEditSelection } from "../components/editor/domEditingTypes";
|
||||
import { absoluteToPercentageForAnimation, findTweenAtTime } from "../utils/globalTimeCompiler";
|
||||
import { PROPERTY_DEFAULTS, selectorFromSelection } from "./gsapShared";
|
||||
import { PROPERTY_DEFAULTS, selectorFromSelection, writeTargetSelector } from "./gsapShared";
|
||||
import { roundToCenti } from "../utils/rounding";
|
||||
|
||||
type CommitFn = (
|
||||
@@ -22,6 +22,9 @@ export async function commitKeyframeAtTimeImpl(
|
||||
properties: Record<string, number | string>,
|
||||
commitMutation: CommitFn,
|
||||
): Promise<void> {
|
||||
// Matching an authored tween is a string compare against what the author
|
||||
// wrote, so it keeps using the selection's own selector; the NEW tween below
|
||||
// is authored with the one-element form instead.
|
||||
const selector = selectorFromSelection(selection);
|
||||
if (!selector) return;
|
||||
|
||||
@@ -65,7 +68,7 @@ export async function commitKeyframeAtTimeImpl(
|
||||
selection,
|
||||
{
|
||||
type: "add-with-keyframes" as const,
|
||||
targetSelector: selector,
|
||||
targetSelector: writeTargetSelector(selection) ?? selector,
|
||||
position: absoluteTime,
|
||||
duration: defaultDuration,
|
||||
keyframes: [
|
||||
|
||||
@@ -181,6 +181,86 @@ export function selectorFromSelection(selection: DomEditSelection): string | nul
|
||||
return null;
|
||||
}
|
||||
|
||||
/** `[name="value"]`, with the quote/backslash escaping a CSS string needs. */
|
||||
function attributeSelector(name: string, value: string): string {
|
||||
return `[${name}="${value.replace(/(["\\])/g, "\\$1")}"]`;
|
||||
}
|
||||
|
||||
function matchesExactlyOne(doc: Document, selector: string, element: Element): boolean {
|
||||
try {
|
||||
const matches = doc.querySelectorAll(selector);
|
||||
return matches.length === 1 && matches[0] === element;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A structural address for an element that carries no identity of its own:
|
||||
* `:nth-child` steps up to the nearest ancestor that IS uniquely addressable
|
||||
* (an id or a data-hf-id). This is the `selector` + `selectorIndex` pair the
|
||||
* selection already carries, resolved through the live DOM the index was
|
||||
* counted in — an index can't be spelled in CSS, but the element's position can.
|
||||
*/
|
||||
function structuralSelector(element: Element): string | null {
|
||||
const doc = element.ownerDocument;
|
||||
if (!doc) return null;
|
||||
const parts: string[] = [];
|
||||
for (let node: Element | null = element; node; node = node.parentElement) {
|
||||
if (node !== element) {
|
||||
const id = node instanceof HTMLElement ? node.id : "";
|
||||
const hfId = node.getAttribute("data-hf-id");
|
||||
if (id) {
|
||||
parts.unshift(idSelector(id));
|
||||
break;
|
||||
}
|
||||
if (hfId) {
|
||||
parts.unshift(attributeSelector("data-hf-id", hfId));
|
||||
break;
|
||||
}
|
||||
}
|
||||
const parent = node.parentElement;
|
||||
if (!parent) break;
|
||||
const index = Array.prototype.indexOf.call(parent.children, node) + 1;
|
||||
if (index < 1) return null;
|
||||
parts.unshift(`${node.tagName.toLowerCase()}:nth-child(${index})`);
|
||||
}
|
||||
if (parts.length === 0) return null;
|
||||
const selector = parts.join(" > ");
|
||||
return matchesExactlyOne(doc, selector, element) ? selector : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The selector to author a NEW tween with. Distinct from
|
||||
* {@link selectorFromSelection}, which must keep returning the exact string an
|
||||
* already-authored tween is string-matched against (findTweenAtTime): this one
|
||||
* has to ADDRESS ONE ELEMENT.
|
||||
*
|
||||
* `buildStableSelector` hands back a bare class for any element without an id,
|
||||
* so "add keyframe at playhead" on one of five `.group` siblings wrote
|
||||
* `tl.set(".group", …)` — a tween that animates all five and that
|
||||
* {@link resolveSelectorElementIds} reads back as all five, collapsing their
|
||||
* timeline rows into one. Every rung below resolves to exactly one element.
|
||||
*
|
||||
* ponytail: the last rung returns the bare selector unchanged rather than null.
|
||||
* Refusing to write would turn a mis-targeted add into a silently dead button;
|
||||
* it is only reachable with no live DOM to disambiguate against.
|
||||
*/
|
||||
export function writeTargetSelector(selection: DomEditSelection): string | null {
|
||||
if (selection.id) return idSelector(selection.id);
|
||||
if (selection.hfId) return attributeSelector("data-hf-id", selection.hfId);
|
||||
const element = selection.element;
|
||||
const doc = element?.ownerDocument;
|
||||
if (element && doc) {
|
||||
if (selection.selector && matchesExactlyOne(doc, selection.selector, element)) {
|
||||
return selection.selector;
|
||||
}
|
||||
const structural = structuralSelector(element);
|
||||
if (structural) return structural;
|
||||
}
|
||||
return selection.selector ?? null;
|
||||
}
|
||||
|
||||
// ── Percentage computation ────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
// @vitest-environment jsdom
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { parseGsapScript } from "@hyperframes/core/gsap-parser";
|
||||
import { addAnimationWithKeyframesToScript } from "@hyperframes/parsers/gsap-writer-acorn";
|
||||
import type { DomEditSelection } from "../components/editor/domEditingTypes";
|
||||
import { buildStableSelector, getSelectorIndex } from "../components/editor/domEditingDom";
|
||||
import { resolveSelectorElementIds, writeTargetSelector } from "./gsapShared";
|
||||
import { commitKeyframeAtTimeImpl } from "./gsapKeyframeCommit";
|
||||
|
||||
afterEach(() => {
|
||||
document.body.innerHTML = "";
|
||||
});
|
||||
|
||||
/**
|
||||
* A selection built the way production builds it (getDomLayerPatchTarget), so
|
||||
* the class-only case under test is the real one: buildStableSelector hands back
|
||||
* a BARE class for an element with no id / hf-id / composition id.
|
||||
*/
|
||||
function selectionFor(el: HTMLElement): DomEditSelection {
|
||||
const selector = buildStableSelector(el);
|
||||
return {
|
||||
element: el,
|
||||
id: el.id || undefined,
|
||||
hfId: el.getAttribute("data-hf-id") || undefined,
|
||||
selector,
|
||||
selectorIndex: getSelectorIndex(document, el, selector, "index.html", null),
|
||||
sourceFile: "index.html",
|
||||
dataAttributes: { start: "0", duration: "2" },
|
||||
} as unknown as DomEditSelection;
|
||||
}
|
||||
|
||||
/** Five class-only siblings — the shape that made one add collapse the timeline. */
|
||||
function mountGroupSiblings(): HTMLElement[] {
|
||||
document.body.innerHTML = `
|
||||
<div id="scene" class="clip" data-start="0" data-duration="2">
|
||||
<div class="group"></div>
|
||||
<div class="group"></div>
|
||||
<div class="group"></div>
|
||||
<div class="group"></div>
|
||||
<div class="group"></div>
|
||||
</div>
|
||||
`;
|
||||
return Array.from(document.querySelectorAll<HTMLElement>(".group"));
|
||||
}
|
||||
|
||||
const BASE_SCRIPT = `
|
||||
const tl = gsap.timeline({ paused: true });
|
||||
tl.to("#scene", { opacity: 1, duration: 0.5 }, 0);
|
||||
`.trim();
|
||||
|
||||
const KEYFRAMES = [
|
||||
{ percentage: 0, properties: { x: 0 } },
|
||||
{ percentage: 100, properties: { x: 40 } },
|
||||
];
|
||||
|
||||
/** Author the selector into a real script and read the tween's targets back. */
|
||||
function roundTripTargets(selector: string): { script: string; targets: string[] } {
|
||||
const { script } = addAnimationWithKeyframesToScript(BASE_SCRIPT, selector, 0, 1, KEYFRAMES);
|
||||
const added = parseGsapScript(script).animations.find((a) => a.targetSelector === selector);
|
||||
if (!added) throw new Error(`written selector did not re-parse: ${selector}`);
|
||||
return { script, targets: resolveSelectorElementIds(added.targetSelector, document) };
|
||||
}
|
||||
|
||||
describe("writeTargetSelector", () => {
|
||||
it("addresses exactly one element when the only identity is a shared class", () => {
|
||||
const groups = mountGroupSiblings();
|
||||
const selection = selectionFor(groups[2]);
|
||||
|
||||
// Precondition: this is the defect's input — a bare class hitting all five.
|
||||
expect(selection.selector).toBe(".group");
|
||||
expect(document.querySelectorAll(".group")).toHaveLength(5);
|
||||
|
||||
const written = writeTargetSelector(selection);
|
||||
|
||||
expect(written).toBeTruthy();
|
||||
expect(document.querySelectorAll(written!)).toHaveLength(1);
|
||||
expect(document.querySelector(written!)).toBe(groups[2]);
|
||||
});
|
||||
|
||||
it("keeps a unique #id target", () => {
|
||||
document.body.innerHTML = `<div id="box" class="card"></div>`;
|
||||
const el = document.querySelector<HTMLElement>("#box")!;
|
||||
|
||||
expect(writeTargetSelector(selectionFor(el))).toBe("#box");
|
||||
});
|
||||
|
||||
it("prefers data-hf-id over a generated structural selector", () => {
|
||||
document.body.innerHTML = `
|
||||
<div id="scene"><div class="group"></div><div class="group" data-hf-id="hf-42"></div></div>
|
||||
`;
|
||||
const el = document.querySelectorAll<HTMLElement>(".group")[1]!;
|
||||
|
||||
const written = writeTargetSelector(selectionFor(el));
|
||||
|
||||
expect(written).toBe('[data-hf-id="hf-42"]');
|
||||
expect(document.querySelector(written!)).toBe(el);
|
||||
});
|
||||
|
||||
it("keeps an already-unique class selector as authored", () => {
|
||||
document.body.innerHTML = `<div id="scene"><div class="header"></div></div>`;
|
||||
const el = document.querySelector<HTMLElement>(".header")!;
|
||||
|
||||
expect(writeTargetSelector(selectionFor(el))).toBe(".header");
|
||||
});
|
||||
});
|
||||
|
||||
describe("writeTargetSelector — write/read round trip", () => {
|
||||
it("re-parses and attributes the new tween to the one element it targeted", () => {
|
||||
const groups = mountGroupSiblings();
|
||||
// Ids let resolveSelectorElementIds name the attributed elements; the
|
||||
// SELECTION still has none, so the write path still faces the bare class.
|
||||
groups.forEach((el, i) => el.setAttribute("id", `group-${i}`));
|
||||
const selection = { ...selectionFor(groups[2]), id: undefined } as DomEditSelection;
|
||||
|
||||
const written = writeTargetSelector(selection);
|
||||
const { targets } = roundTripTargets(written!);
|
||||
|
||||
expect(targets).toEqual(["group-2"]);
|
||||
});
|
||||
|
||||
it("does not widen the add to the element's four class siblings", () => {
|
||||
const groups = mountGroupSiblings();
|
||||
groups.forEach((el, i) => el.setAttribute("id", `group-${i}`));
|
||||
const selection = { ...selectionFor(groups[0]), id: undefined } as DomEditSelection;
|
||||
|
||||
// The old bare-class write attributed the one add to every sibling — the
|
||||
// attribution blow-up behind "one add collapsed the timeline to a single row".
|
||||
expect(roundTripTargets(".group").targets).toHaveLength(5);
|
||||
|
||||
expect(roundTripTargets(writeTargetSelector(selection)!).targets).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe("commitKeyframeAtTimeImpl — new-tween target", () => {
|
||||
it("authors a one-element selector when no tween exists at the playhead", async () => {
|
||||
const groups = mountGroupSiblings();
|
||||
const selection = selectionFor(groups[3]);
|
||||
const commitMutation = vi.fn(async () => undefined);
|
||||
|
||||
await commitKeyframeAtTimeImpl(selection, 1, [], { x: 12 }, commitMutation);
|
||||
|
||||
const mutation = commitMutation.mock.calls[0]?.[1] as { targetSelector: string };
|
||||
expect(mutation.targetSelector).not.toBe(".group");
|
||||
expect(document.querySelectorAll(mutation.targetSelector)).toHaveLength(1);
|
||||
expect(document.querySelector(mutation.targetSelector)).toBe(groups[3]);
|
||||
});
|
||||
});
|
||||
@@ -466,3 +466,47 @@ describe("useEnableKeyframes — flat tween transaction", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("useEnableKeyframes — new tween on a class-only element", () => {
|
||||
it("targets the selected sibling alone, not every element sharing its class", async () => {
|
||||
window.location.hash = "#/project/test-project";
|
||||
usePlayerStore.setState({ currentTime: 1 });
|
||||
vi.stubGlobal(
|
||||
"fetch",
|
||||
vi.fn(async () => ({ ok: true, json: async () => ({ animations: [] }) })),
|
||||
);
|
||||
const scene = document.body.appendChild(document.createElement("div"));
|
||||
scene.id = "scene";
|
||||
scene.innerHTML = '<i class="group"></i>'.repeat(5);
|
||||
const groups = Array.from(scene.querySelectorAll<HTMLElement>(".group"));
|
||||
const commitMutation = vi.fn(async () => undefined);
|
||||
const enable = renderEnableKeyframes({
|
||||
// A bare `.group` here writes `tl.to(".group", …)`, which animates all five
|
||||
// siblings and reads back as all five — one add wiped the timeline.
|
||||
domEditSelection: {
|
||||
selector: ".group",
|
||||
selectorIndex: 3,
|
||||
sourceFile: "index.html",
|
||||
element: groups[3],
|
||||
dataAttributes: { start: "0", duration: "2" },
|
||||
} as unknown as DomEditSelection,
|
||||
selectedGsapAnimations: [],
|
||||
previewIframeRef: {
|
||||
current: {
|
||||
contentWindow: { gsap: { getProperty: () => 7 } },
|
||||
} as unknown as HTMLIFrameElement,
|
||||
},
|
||||
handleGsapAddAnimation: vi.fn(),
|
||||
handleGsapConvertToKeyframes: vi.fn(),
|
||||
handleGsapRemoveKeyframe: vi.fn(),
|
||||
commitMutation,
|
||||
});
|
||||
|
||||
await act(async () => enable());
|
||||
|
||||
const mutation = commitMutation.mock.calls[0]?.[0] as { targetSelector: string };
|
||||
expect(mutation?.targetSelector).toBeTruthy();
|
||||
expect(document.querySelectorAll(mutation.targetSelector)).toHaveLength(1);
|
||||
expect(document.querySelector(mutation.targetSelector)).toBe(groups[3]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
KEYFRAME_PCT_MATCH,
|
||||
isInstantHold,
|
||||
resolveEditableTweenDuration,
|
||||
writeTargetSelector,
|
||||
} from "./gsapShared";
|
||||
import {
|
||||
absoluteToPercentage,
|
||||
@@ -523,7 +524,10 @@ export function useEnableKeyframes(
|
||||
sel.dataAttributes?.duration,
|
||||
t,
|
||||
);
|
||||
const selector = selectorFromSelection(sel);
|
||||
// A brand-new tween: author it against the one element the user selected.
|
||||
// The bare class selectorFromSelection hands back for an id-less element
|
||||
// animates every sibling sharing the class (see writeTargetSelector).
|
||||
const selector = writeTargetSelector(sel);
|
||||
|
||||
if (!selector) {
|
||||
session.handleGsapAddAnimation("to");
|
||||
|
||||
Reference in New Issue
Block a user