fix(studio): scrub audio, panel reads and gestures act on the bound element, one scan per pass

Author: miguel-heygen

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

@@ -2,6 +2,8 @@
 import { expect, it, vi } from "vitest";
 import { createGsapLivePreview } from "./gsapLivePreview";
 import type { DomEditSelection } from "./domEditingTypes";
+import type { GsapAnimation } from "@hyperframes/parsers/gsap-parser";
+import { readGsapRuntimeValuesForPanel } from "./propertyPanelHelpers";
 
 it("previews on the selected element, not an earlier same-id copy in a sub-composition", () => {
   document.body.innerHTML =
@@ -24,3 +26,38 @@ it("previews on the copy in the selection's own file when a sub-composition repe
   preview({ id: "card", hfId: "hf-card", sourceFile: "index.html" } as DomEditSelection, { x: 10 });
   expect(set.mock.calls[0]?.[0]).toBe(document.querySelector(".root"));
 });
+
+it("previews on the selected node while it is mounted, even in a second mount of the same sub-composition", () => {
+  document.body.innerHTML =
+    '<div data-composition-id="strip" data-composition-src="compositions/strip.html"><div id="card" data-hf-id="hf-card"></div></div>' +
+    '<div data-composition-id="strip" data-composition-src="compositions/strip.html"><div id="card" data-hf-id="hf-card"></div></div>';
+  const second = document.querySelectorAll<HTMLElement>("#card")[1];
+  const selection = {
+    id: "card",
+    hfId: "hf-card",
+    sourceFile: "compositions/strip.html",
+    element: second,
+  };
+  const set = vi.fn();
+  const iframe = { contentWindow: { gsap: { set } }, contentDocument: document };
+  createGsapLivePreview({ current: iframe as unknown as HTMLIFrameElement })(
+    selection as DomEditSelection,
+    { x: 10 },
+  );
+  expect(set.mock.calls[0]?.[0]).toBe(second);
+});
+
+it("the panel reads GSAP values off the node the live preview moves", () => {
+  document.body.innerHTML =
+    '<div data-composition-id="strip" data-composition-src="compositions/strip.html"><div id="card"></div></div>' +
+    '<div id="card" class="root"></div>';
+  const root = document.querySelector<HTMLElement>(".root");
+  const getProperty = vi.fn(() => 5);
+  const iframe = { contentWindow: { gsap: { getProperty } }, contentDocument: document };
+  const selection = { id: "card", sourceFile: "index.html", element: root } as DomEditSelection;
+  const animations = [{ properties: { x: 5 } }] as unknown as GsapAnimation[];
+  readGsapRuntimeValuesForPanel("anim", animations, selection, {
+    current: iframe as unknown as HTMLIFrameElement,
+  });
+  expect(getProperty.mock.calls[0]?.[0]).toBe(root);
+});

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

@@ -9,13 +9,14 @@ import { findElementForSelection } from "./domEditingElement";
  * Extracted so the identical closure exists once — shared by the legacy
  * PropertyPanel Layout section and the flat Layout group (PropertyPanelFlat).
  */
-// hf-ids and ids repeat across flattened sub-compositions and a selector's first match may be a sibling, so look in
-// the selection's own file first, then anywhere.
-function resolvePreviewNode(
+// The selected node while it is still mounted; after a reload, the copy in the selection's own file (hf-ids and ids
+// repeat across flattened sub-compositions), then anywhere.
+export function findPreviewNode(
   doc: Document | null | undefined,
   el: DomEditSelection,
 ): Element | null {
   if (!doc) return null;
+  if (el.element?.isConnected && el.element.ownerDocument === doc) return el.element;
   return (
     findElementForSelection(doc, el) ??
     findElementForSelection(doc, { ...el, sourceFile: undefined })
@@ -29,7 +30,7 @@ export function createGsapLivePreview(iframeRef: { readonly current: HTMLIFrameE
       | { gsap?: { set: (t: Element, v: Record<string, number>) => void } }
       | null
       | undefined;
-    const node = resolvePreviewNode(iframe?.contentDocument, el);
+    const node = findPreviewNode(iframe?.contentDocument, el);
     if (win?.gsap && node) win.gsap.set(node, props);
   };
 }

packages/studio/src/components/editor/propertyPanelHelpers.ts

@@ -4,6 +4,7 @@ import type { DomEditSelection } from "./domEditing";
 import type { GsapAnimation } from "@hyperframes/parsers/gsap-parser";
 import type { TimelineElement } from "../../player";
 import { roundToCenti } from "../../utils/rounding";
+import { findPreviewNode } from "./gsapLivePreview";
 
 export type {
   BackgroundRemovalProgress,
@@ -470,16 +471,14 @@ export function readGsapRuntimeValuesForPanel(
   if (!gsapAnimId || gsapAnimations.length === 0) return null;
   const iframe = previewIframeRef?.current;
   if (!iframe?.contentWindow) return null;
-  const selector = element.id ? `#${element.id}` : element.selector;
-  if (!selector) return null;
   try {
     const gsap = (
       iframe.contentWindow as unknown as {
         gsap?: { getProperty: (el: Element, prop: string) => number | string };
       }
     ).gsap;
     if (!gsap?.getProperty) return null;
-    const el = iframe.contentDocument?.querySelector(selector);
+    const el = findPreviewNode(iframe.contentDocument, element);
     if (!el) return null;
     const propKeys = collectPanelPropKeys(gsapAnimations);
     const result: Record<string, number> = {};
@@ -508,10 +507,9 @@ export function readGsapBorderRadiusForPanel(
     if (!hasBRProp) return null;
   }
   const iframe = previewIframeRef?.current;
-  const selector = element.id ? `#${element.id}` : element.selector;
-  if (!iframe?.contentDocument || !selector) return null;
+  if (!iframe?.contentDocument) return null;
   try {
-    const el = iframe.contentDocument.querySelector(selector);
+    const el = findPreviewNode(iframe.contentDocument, element);
     if (!el || !iframe.contentWindow) return null;
     const cs = iframe.contentWindow.getComputedStyle(el);
     const parse = (v: string) => Number.parseFloat(v) || 0;

packages/studio/src/hooks/useGestureRecording.test.tsx

@@ -109,7 +109,7 @@ describe("useGestureRecording", () => {
     act(() => root.unmount());
 
     expect(cancelAnimationFrame).toHaveBeenCalledWith(17);
-    expect(set).toHaveBeenCalledWith("#card", {
+    expect(set).toHaveBeenCalledWith(element, {
       clearProps: "x,y,scale,scaleX,scaleY,rotation,rotationX,rotationY,opacity,z",
     });
     expect(element.style.visibility).toBe("hidden");

packages/studio/src/hooks/useGestureRecording.ts

@@ -30,8 +30,7 @@ interface BasePosition {
 
 interface GsapRuntime {
   timeline: { seek: (t: number) => void };
-  gsap: { set: (target: string, vars: Record<string, number | string>) => void };
-  selector: string;
+  gsap: { set: (target: Element, vars: Record<string, number | string>) => void };
   element: HTMLElement;
   startTime: number;
   maxSeekTime: number;
@@ -86,7 +85,7 @@ function connectGsapRuntime(
 ): GsapRuntime | null {
   try {
     const win = iframeEl.contentWindow as Window & {
-      gsap?: { set: (t: string, v: Record<string, number | string>) => void };
+      gsap?: { set: (t: Element, v: Record<string, number | string>) => void };
       __timelines?: Record<string, { seek: (t: number) => void; duration: () => number }>;
       __player?: { getTime: () => number };
     };
@@ -103,7 +102,6 @@ function connectGsapRuntime(
       return {
         timeline: tl,
         gsap: win.gsap,
-        selector,
         element,
         startTime: win.__player?.getTime() ?? 0,
         maxSeekTime:
@@ -126,7 +124,7 @@ function applyRuntimePreview(
   const seekTime = Math.min(runtime.startTime + time, runtime.maxSeekTime);
   runtime.timeline.seek(seekTime);
   runtime.element.style.setProperty("translate", "none");
-  runtime.gsap.set(runtime.selector, { ...properties });
+  runtime.gsap.set(runtime.element, { ...properties });
   runtime.element.style.visibility = "visible";
   liveTime.notify(seekTime);
   usePlayerStore.getState().setCurrentTime(seekTime);
@@ -256,7 +254,7 @@ function releaseRuntimePreview(r: RecordingRefs): void {
   element.style.visibility = savedVisibility;
   element.style.setProperty("translate", savedTranslate || "");
   try {
-    runtime.gsap.set(runtime.selector, {
+    runtime.gsap.set(runtime.element, {
       clearProps: "x,y,scale,scaleX,scaleY,rotation,rotationX,rotationY,opacity,z",
     });
   } catch {

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

@@ -17,7 +17,7 @@ import { HF_AUDIO_AUTOMATION_ATTR } from "@hyperframes/core/audio-automation";
 import { HF_AUDIO_FX_ATTR } from "@hyperframes/core/audio-fx";
 import { usePlayerStore, type TimelineElement } from "../store/playerStore";
 import { groupInfoFor } from "./timelineGroupInfo";
-import { findPreviewElement } from "./timelineElementHelpers";
+import { previewElementFinder } from "./timelineElementHelpers";
 
 /**
  * Re-read every element's automation and FX-chain attributes from the preview
@@ -55,14 +55,11 @@ function syncedFields(doc: Document, element: TimelineElement, node: Element) {
 
 export function syncStoredAutomationFromPreview(doc: Document | null | undefined): void {
   if (!doc) return;
+  const findNode = previewElementFinder(doc);
   usePlayerStore.setState((state) => {
     let changed = false;
     const elements = state.elements.map((element) => {
-      const node = findPreviewElement(doc, {
-        hfId: element.hfId,
-        id: element.domId ?? element.id,
-        sourceFile: element.sourceFile,
-      });
+      const node = findNode(element);
       if (!node) return element;
       const fields = syncedFields(doc, element, node);
       // Same array back when nothing moved: `elements` keys memos all over the

packages/studio/src/player/lib/playbackScrub.ts

@@ -12,5 +12,5 @@ export function scrubMusicAtSeek(iframe: HTMLIFrameElement | null, nextTime: num
   if (!music || s.audioMuted) return;
   const rel = nextTime - music.start;
   const audioFileTime = rel >= 0 && rel <= music.duration ? (music.playbackStart ?? 0) + rel : null;
-  scrubPreviewAudio(iframe, audioFileTime, music.domId ?? music.id, s.audioVolume);
+  scrubPreviewAudio(iframe, audioFileTime, music, s.audioVolume);
 }

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

@@ -458,23 +458,29 @@ function findInClipScope(
   return lone;
 }
 
-/** A preview element by `data-hf-id` then id, which both repeat across files, preferring one in its own file. */
-export function findPreviewElement(
-  doc: Document,
-  target: { hfId?: string; id?: string | null; sourceFile?: string },
-): Element | null {
-  const selectors = [
-    target.hfId && `[data-hf-id="${CSS.escape(target.hfId)}"]`,
-    target.id && `[id="${CSS.escape(target.id)}"]`,
-  ];
-  const matches = selectors.flatMap((selector) =>
-    selector ? Array.from(doc.querySelectorAll(selector)) : [],
-  );
-  return (
-    matches.find((node) => getTimelineElementSourceFile(node) === target.sourceFile) ??
-    matches[0] ??
-    null
-  );
+export type PreviewTarget = Pick<TimelineElement, "hfId" | "domId" | "id" | "sourceFile">;
+
+/** Finds a row's preview element by `data-hf-id`, then id, preferring one in the row's own file: both repeat across
+ * files. Indexes the document once, so a pass over every row costs one scan. */
+export function previewElementFinder(doc: Document): (target: PreviewTarget) => Element | null {
+  const byKey = new Map<string, Element[]>();
+  const add = (key: string, node: Element) => byKey.set(key, [...(byKey.get(key) ?? []), node]);
+  for (const node of doc.querySelectorAll("[data-hf-id], [id]")) {
+    const hfId = node.getAttribute("data-hf-id");
+    if (hfId) add(`hf:${hfId}`, node);
+    if (node.id) add(`id:${node.id}`, node);
+  }
+  return (target) => {
+    const matches = [
+      ...((target.hfId && byKey.get(`hf:${target.hfId}`)) || []),
+      ...(byKey.get(`id:${target.domId ?? target.id}`) ?? []),
+    ];
+    return (
+      matches.find((node) => getTimelineElementSourceFile(node) === target.sourceFile) ??
+      matches[0] ??
+      null
+    );
+  };
 }
 
 export function findClipElementById(doc: Document, clip: ClipManifestClip): Element | null {

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

@@ -130,21 +130,21 @@ describe("scrubPreviewAudio", () => {
     audio.pause = vi.fn();
     iframe.contentDocument.body.append(audio);
 
-    scrubPreviewAudio(iframe, 0.5, "music", 0.4);
+    scrubPreviewAudio(iframe, 0.5, { id: "music" }, 0.4);
 
     expect(audio.volume).toBeCloseTo(0.1);
     stopScrubPreviewAudio();
   });
 
   /**
    * The preview document is a different realm, so `instanceof HTMLAudioElement`
-   * is false for every node in it. That threw the `musicId` hint away and left
+   * is false for every node in it. That threw the `music` hint away and left
    * the first `<audio>` in the document as the only route — and the first
    * `<audio>` is often the voiceover, so scrubbing previewed the wrong track.
    * Two elements, music second, is what tells the two paths apart: with one
    * element the fallback reaches the right node by accident.
    */
-  it("previews the track named by musicId, not the first audio in the document", () => {
+  it("previews the track named by the music row, not the first audio in the document", () => {
     const iframe = document.createElement("iframe");
     document.body.append(iframe);
     const previewDoc = iframe.contentDocument;
@@ -165,13 +165,32 @@ describe("scrubPreviewAudio", () => {
     // The node really is cross-realm; this is the condition, not a contrivance.
     expect(music instanceof HTMLAudioElement).toBe(false);
 
-    scrubPreviewAudio(iframe, 0.5, "music-bed", 1);
+    scrubPreviewAudio(iframe, 0.5, { id: "music-bed" }, 1);
 
     expect(music.play).toHaveBeenCalled();
     expect(voiceover.play).not.toHaveBeenCalled();
     stopScrubPreviewAudio();
   });
 
+  it("previews the music row's own track when a sub-composition repeats its id", () => {
+    const iframe = document.createElement("iframe");
+    document.body.append(iframe);
+    const previewDoc = iframe.contentDocument;
+    if (!previewDoc?.body) throw new Error("expected an iframe document");
+    previewDoc.body.innerHTML =
+      '<div data-composition-id="strip" data-composition-src="compositions/strip.html"><audio id="music"></audio></div>' +
+      '<audio id="music" class="root"></audio>';
+    const [inner, root] = Array.from(previewDoc.querySelectorAll("audio"));
+    for (const audio of [inner, root])
+      Object.assign(audio!, { play: vi.fn(async () => {}), pause: vi.fn() });
+
+    scrubPreviewAudio(iframe, 0.5, { id: "music" }, 1);
+
+    expect(root!.play).toHaveBeenCalled();
+    expect(inner!.play).not.toHaveBeenCalled();
+    stopScrubPreviewAudio();
+  });
+
   /** A scrub audition is media running under a paused clock, which the runtime now
    *  stops on sight. So it borrows the element. That a leased element survives the
    *  tick is asserted runtime-side in core's `transportPark.test.ts`; here the
@@ -192,7 +211,7 @@ describe("scrubPreviewAudio", () => {
     const releasePausedMedia = vi.fn();
     (previewDoc.defaultView as IframeWindow).__hf = { leasePausedMedia, releasePausedMedia };
 
-    scrubPreviewAudio(iframe, 0.5, "music", 1);
+    scrubPreviewAudio(iframe, 0.5, { id: "music" }, 1);
 
     expect(leasePausedMedia).toHaveBeenCalledWith(music);
     expect(releasePausedMedia).not.toHaveBeenCalled();

packages/studio/src/player/lib/timelineIframeHelpers.ts

@@ -20,7 +20,8 @@ import {
   getTimelineElementDisplayLabel,
   buildTimelineElementIdentity,
   readTimelineElementZIndex,
-  findPreviewElement,
+  previewElementFinder,
+  type PreviewTarget,
 } from "./timelineElementHelpers";
 import { postRuntimeControlMessage } from "./runtimeProtocol";
 import { transitionLabelsForDocument } from "./timelineTransitionMetadata";
@@ -224,7 +225,7 @@ let scrubPrevVolume: number | null = null;
  * `doc` is the preview iframe's document, so its `<audio>` nodes are instances of
  * the IFRAME's `HTMLAudioElement`, never this module's. `instanceof
  * HTMLAudioElement` here is false for every one of them, which silently threw the
- * `musicId` hint away and fell through to "first `<audio>` in the document" — the
+ * `music` hint away and fell through to "first `<audio>` in the document" — the
  * very thing the comment above warns can be a voiceover. Ask what the node IS.
  * Same rule and same reasoning as packages/core/src/runtime/domRealm.ts.
  */
@@ -236,9 +237,9 @@ function isAudioNode(node: Element | null): node is HTMLAudioElement {
   );
 }
 
-function resolveScrubAudioEl(doc: Document, musicId?: string | null): HTMLAudioElement | null {
-  if (musicId) {
-    const byId = doc.getElementById(musicId);
+function resolveScrubAudioEl(doc: Document, music?: PreviewTarget | null): HTMLAudioElement | null {
+  if (music) {
+    const byId = previewElementFinder(doc)(music);
     if (isAudioNode(byId)) return byId;
   }
   return (
@@ -284,7 +285,7 @@ function applyScrub(el: HTMLAudioElement, audioFileTime: number, previewVolume:
 export function scrubPreviewAudio(
   iframe: HTMLIFrameElement | null,
   audioFileTime: number | null,
-  musicId?: string | null,
+  music?: PreviewTarget | null,
   previewVolume = 1,
 ): void {
   if (!iframe) return;
@@ -299,7 +300,7 @@ export function scrubPreviewAudio(
     return;
   }
   if (!doc) return;
-  const el = resolveScrubAudioEl(doc, musicId);
+  const el = resolveScrubAudioEl(doc, music);
   if (el) applyScrub(el, audioFileTime, previewVolume);
 }
 
@@ -513,14 +514,11 @@ export function buildMissingCompositionElements(
 
   // Patch existing elements that are missing compositionSrc
   let patched = false;
+  const findHost = previewElementFinder(doc);
   const updatedEls = (currentEls as TimelineElement[]).map((existing) => {
     if (existing.compositionSrc) return existing;
     const host =
-      findPreviewElement(doc, {
-        hfId: existing.hfId,
-        id: existing.domId ?? existing.id,
-        sourceFile: existing.sourceFile,
-      }) ?? doc.querySelector(`[data-composition-id="${CSS.escape(existing.id)}"]`);
+      findHost(existing) ?? doc.querySelector(`[data-composition-id="${CSS.escape(existing.id)}"]`);
     if (!host) return existing;
     const compSrc =
       host.getAttribute("data-composition-src") || host.getAttribute("data-composition-file");