Skip to content

Commit 9a2ce03

Browse files
committed
fix(studio): preview lookups prefer the element's own file, and a pasted element gets its own hf-id
1 parent 121b39e commit 9a2ce03

12 files changed

Lines changed: 137 additions & 55 deletions

‎packages/studio/src/components/editor/gsapLivePreview.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,3 +13,14 @@ it("previews on the selected element, not an earlier same-id copy in a sub-compo
1313
preview({ id: "card", hfId: "hf-root" } as DomEditSelection, { x: 10 });
1414
expect(set.mock.calls[0]?.[0]).toBe(document.querySelector('[data-hf-id="hf-root"]'));
1515
});
16+
17+
it("previews on the copy in the selection's own file when a sub-composition repeats its hf-id", () => {
18+
document.body.innerHTML =
19+
'<div data-composition-id="strip" data-composition-src="compositions/strip.html">' +
20+
'<div id="card" data-hf-id="hf-card"></div></div><div id="card" data-hf-id="hf-card" class="root"></div>';
21+
const set = vi.fn();
22+
const iframe = { contentWindow: { gsap: { set } }, contentDocument: document };
23+
const preview = createGsapLivePreview({ current: iframe as unknown as HTMLIFrameElement });
24+
preview({ id: "card", hfId: "hf-card", sourceFile: "index.html" } as DomEditSelection, { x: 10 });
25+
expect(set.mock.calls[0]?.[0]).toBe(document.querySelector(".root"));
26+
});

‎packages/studio/src/components/editor/gsapLivePreview.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { DomEditSelection } from "./domEditingTypes";
2-
import { findPreviewElement } from "../../player/lib/timelineElementHelpers";
2+
import { findElementForSelection } from "./domEditingElement";
33

44
/**
55
* Build the "live preview" callback the 3D-transform sub-view fires while a
@@ -9,17 +9,17 @@ import { findPreviewElement } from "../../player/lib/timelineElementHelpers";
99
* Extracted so the identical closure exists once — shared by the legacy
1010
* PropertyPanel Layout section and the flat Layout group (PropertyPanelFlat).
1111
*/
12-
// Resolve by id when unique, otherwise by selector + selectorIndex — a bare
13-
// querySelector(selector) always hits the FIRST match, so dragging on the
14-
// second of two same-selector siblings would animate the wrong element.
12+
// hf-ids and ids repeat across flattened sub-compositions and a selector's first match may be a sibling, so look in
13+
// the selection's own file first, then anywhere.
1514
function resolvePreviewNode(
1615
doc: Document | null | undefined,
1716
el: DomEditSelection,
1817
): Element | null {
1918
if (!doc) return null;
20-
if (el.hfId || el.id) return findPreviewElement(doc, el);
21-
if (!el.selector) return null;
22-
return doc.querySelectorAll(el.selector)[el.selectorIndex ?? 0] ?? null;
19+
return (
20+
findElementForSelection(doc, el) ??
21+
findElementForSelection(doc, { ...el, sourceFile: undefined })
22+
);
2323
}
2424

2525
export function createGsapLivePreview(iframeRef: { readonly current: HTMLIFrameElement | null }) {

‎packages/studio/src/hooks/useClipboard.test.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
// @vitest-environment jsdom
22
import { describe, expect, it } from "vitest";
3-
import { pasteTimelineClips, resolveFreeTrack, type PlacedClip } from "./useClipboard";
3+
import {
4+
pasteDomElement,
5+
pasteTimelineClips,
6+
resolveFreeTrack,
7+
type PlacedClip,
8+
} from "./useClipboard";
49
import { extendRootDurationInSource } from "../utils/rootDuration";
510
import type { TimelineClipboardClip } from "../utils/clipboardPayload";
611
import type { TimelineElement } from "../player";
@@ -232,3 +237,13 @@ describe("pasteTimelineClips + extendRootDurationInSource", () => {
232237
expect(extended).toContain('data-duration="10"');
233238
});
234239
});
240+
241+
describe("pasteDomElement", () => {
242+
it("strips the copied data-hf-id so the pasted element does not share its original's", () => {
243+
const source = '<div data-composition-id="main"><p id="card" data-hf-id="hf-card">Hi</p></div>';
244+
const html = '<p id="card" data-hf-id="hf-card">Hi</p>';
245+
const content = pasteDomElement(source, { html, originSelector: "#card" });
246+
expect(content.match(/data-hf-id/g)).toHaveLength(1);
247+
expect(content).toContain('id="card-2"');
248+
});
249+
});

‎packages/studio/src/hooks/useClipboard.ts‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,14 @@ function stripHfIds(html: string, parser: DOMParser): string {
117117
return root.outerHTML;
118118
}
119119

120+
export function pasteDomElement(
121+
content: string,
122+
payload: { html: string; originSelector?: string; originSelectorIndex?: number },
123+
): string {
124+
const html = deduplicateIds(stripHfIds(payload.html, new DOMParser()), collectHtmlIds(content));
125+
return insertAsSibling(content, html, payload.originSelector, payload.originSelectorIndex);
126+
}
127+
120128
/** Shared insertion path for paste and duplicate, anchored at the playhead or
121129
* the selection's end respectively. Returns the final ids so the caller can
122130
* select what it just placed, and the furthest end any clip lands at so the
@@ -280,13 +288,7 @@ export function useClipboard({
280288
patchedContent = extendRootDurationInSource(pasted.content, pasted.requiredEnd);
281289
pastedIds = pasted.ids;
282290
} else {
283-
const deduped = deduplicateIds(payload.html, collectHtmlIds(originalContent));
284-
patchedContent = insertAsSibling(
285-
originalContent,
286-
deduped,
287-
payload.originSelector,
288-
payload.originSelectorIndex,
289-
);
291+
patchedContent = pasteDomElement(originalContent, payload);
290292
}
291293

292294
const label =

‎packages/studio/src/player/lib/automationStoreSync.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,18 @@ describe("syncStoredAutomationFromPreview", () => {
4545
expect(usePlayerStore.getState().elements[0]?.automation).toBe(RESTORED);
4646
});
4747

48+
it("reads the copy in the element's own file when a sub-composition repeats its hf-id", () => {
49+
usePlayerStore.setState({ elements: [el({ hfId: "hf-bgm" })] });
50+
const doc = document.implementation.createHTMLDocument("preview");
51+
doc.body.innerHTML =
52+
'<div data-composition-id="strip" data-composition-src="compositions/strip.html">' +
53+
'<audio id="bgm" data-hf-id="hf-bgm"></audio></div><audio id="bgm" data-hf-id="hf-bgm"></audio>';
54+
doc.querySelectorAll("audio")[0]?.setAttribute("data-automation", TWO_POINTS);
55+
doc.querySelectorAll("audio")[1]?.setAttribute("data-automation", RESTORED);
56+
syncStoredAutomationFromPreview(doc);
57+
expect(usePlayerStore.getState().elements[0]?.automation).toBe(RESTORED);
58+
});
59+
4860
it("reads back an envelope an undo restored on the preview", () => {
4961
// The bug: a soft undo patches the preview document and re-runs the timeline, but
5062
// the store keeps its own copy of the attributes and that copy is what a lane

‎packages/studio/src/player/lib/automationStoreSync.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ export function syncStoredAutomationFromPreview(doc: Document | null | undefined
5858
usePlayerStore.setState((state) => {
5959
let changed = false;
6060
const elements = state.elements.map((element) => {
61-
const node = findPreviewElement(doc, { hfId: element.hfId, id: element.domId ?? element.id });
61+
const node = findPreviewElement(doc, {
62+
hfId: element.hfId,
63+
id: element.domId ?? element.id,
64+
sourceFile: element.sourceFile,
65+
});
6266
if (!node) return element;
6367
const fields = syncedFields(doc, element, node);
6468
// Same array back when nothing moved: `elements` keys memos all over the

‎packages/studio/src/player/lib/timelineDOM.test.ts‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,34 @@ describe("createTimelineElementFromManifestClip — source-scoped selector ident
285285
});
286286
});
287287

288+
it("keeps the host it was given when a sub-composition is mounted twice", () => {
289+
const doc = makeDoc(`
290+
<div data-composition-id="root" data-composition-file="index.html">
291+
<div id="a" data-hf-id="hf-a" data-composition-id="card" data-composition-src="card.html"></div>
292+
<div id="b" data-hf-id="hf-b" data-composition-id="card" data-composition-src="card.html"></div>
293+
</div>
294+
`);
295+
const element = createTimelineElementFromManifestClip({
296+
clip: {
297+
id: "b",
298+
label: "Card",
299+
kind: "composition",
300+
tagName: "div",
301+
start: 4,
302+
duration: 4,
303+
track: 0,
304+
compositionId: "card",
305+
parentCompositionId: "root",
306+
compositionSrc: null,
307+
assetUrl: null,
308+
},
309+
fallbackIndex: 1,
310+
doc,
311+
hostEl: doc.getElementById("b"),
312+
});
313+
expect(element).toMatchObject({ domId: "b", hfId: "hf-b", compositionSrc: "card.html" });
314+
});
315+
288316
it("ignores an index.html duplicate when indexing a scene.html selector", () => {
289317
const doc = makeDoc(`
290318
<div data-composition-id="root" data-composition-file="index.html">

‎packages/studio/src/player/lib/timelineDOM.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,8 +171,10 @@ export function createTimelineElementFromManifestClip(params: {
171171
entry.playbackRate ??= 1;
172172
let resolvedSrc = clip.compositionSrc;
173173
if (!resolvedSrc) {
174-
hostEl =
175-
doc?.querySelector(`[data-composition-id="${CSS.escape(clip.compositionId)}"]`) ?? hostEl;
174+
if (hostEl?.getAttribute("data-composition-id") !== clip.compositionId) {
175+
hostEl =
176+
doc?.querySelector(`[data-composition-id="${CSS.escape(clip.compositionId)}"]`) ?? hostEl;
177+
}
176178
resolvedSrc =
177179
hostEl?.getAttribute("data-composition-src") ??
178180
hostEl?.getAttribute("data-composition-file") ??

‎packages/studio/src/player/lib/timelineElementHelpers.ts‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -458,15 +458,23 @@ function findInClipScope(
458458
return lone;
459459
}
460460

461-
/** A preview element by the `data-hf-id` the timeline bound it with, then by id, which can repeat across files. */
461+
/** A preview element by `data-hf-id` then id, which both repeat across files, preferring one in its own file. */
462462
export function findPreviewElement(
463463
doc: Document,
464-
target: { hfId?: string; id?: string },
464+
target: { hfId?: string; id?: string | null; sourceFile?: string },
465465
): Element | null {
466-
const byHfId = target.hfId
467-
? doc.querySelector(`[data-hf-id="${CSS.escape(target.hfId)}"]`)
468-
: null;
469-
return byHfId ?? (target.id ? doc.getElementById(target.id) : null);
466+
const selectors = [
467+
target.hfId && `[data-hf-id="${CSS.escape(target.hfId)}"]`,
468+
target.id && `[id="${CSS.escape(target.id)}"]`,
469+
];
470+
const matches = selectors.flatMap((selector) =>
471+
selector ? Array.from(doc.querySelectorAll(selector)) : [],
472+
);
473+
return (
474+
matches.find((node) => getTimelineElementSourceFile(node) === target.sourceFile) ??
475+
matches[0] ??
476+
null
477+
);
470478
}
471479

472480
export function findClipElementById(doc: Document, clip: ClipManifestClip): Element | null {

‎packages/studio/src/player/lib/timelineIframeHelpers.hostLookup.test.ts‎

Lines changed: 0 additions & 29 deletions
This file was deleted.

0 commit comments

Comments
 (0)