fix(studio): address PR review — add tests, strip GSAP transform for rotation, remove dead exports

miguel-heygen · miguel-heygen · commit f7d63fbebd89 · 2026-05-19T16:05:37.000-04:00
Addresses review feedback from Rames and Vai:

1. Add 7 new tests for createStudioPositionSeekReapplyScript:
   box-size reapplication, GSAP translate stripping (identity removal,
   scale+translate preservation, transform:none no-op), and rotation-
   only elements with GSAP-baked translate.

2. Add pinning test for the PiP-over-sub-composition selection bug:
   elementsFromPoint returns [pipVideo, subCompRoot, sfChromeImg] as
   siblings — assert the topmost (pipVideo) wins.

3. Apply stripGsapTranslateFromTransform to rotation-only elements
   too, not just path-offset elements. A rotation-only element with
   a GSAP-animated translate would have its position clobbered.

4. Remove dead exports: getPreviewLocalPointer,
   buildRasterClickSelectionContext, getPreviewPlayer,
   seekStudioPreview, PreviewPlayerCompat, PreviewLocalPointer from
   studioPreviewHelpers.ts. Unexport resolvePreviewLocalPointer.
diff --git a/.filesize-allowlist b/.filesize-allowlist
@@ -9,3 +9,4 @@ packages/studio/src/utils/sourcePatcher.test.ts
 packages/studio/src/App.tsx
 packages/studio/src/player/components/Timeline.tsx
 packages/studio/src/player/components/timelineEditing.test.ts
+packages/studio/src/components/editor/domEditing.test.ts
diff --git a/packages/core/src/studio-api/helpers/manualEditsRenderScript.test.ts b/packages/core/src/studio-api/helpers/manualEditsRenderScript.test.ts
@@ -1,6 +1,9 @@
 import { describe, expect, it } from "vitest";
 import { Window } from "happy-dom";
-import { createStudioManualEditsRenderBodyScript } from "./manualEditsRenderScript";
+import {
+  createStudioManualEditsRenderBodyScript,
+  createStudioPositionSeekReapplyScript,
+} from "./manualEditsRenderScript";
 
 function runScript(
   window: Window,
@@ -380,3 +383,182 @@ describe("createStudioManualEditsRenderBodyScript", () => {
     expect(card.style.getPropertyValue("translate")).toContain("--hf-studio-offset-x");
   });
 });
+
+describe("createStudioPositionSeekReapplyScript", () => {
+  function runPositionScript(
+    window: Window,
+    timers: {
+      setInterval?: typeof globalThis.setInterval;
+      clearInterval?: typeof globalThis.clearInterval;
+    } = {},
+  ): void {
+    Object.assign(window, { SyntaxError });
+    const script = createStudioPositionSeekReapplyScript();
+    const execute = new Function(
+      "window",
+      "document",
+      "HTMLElement",
+      "DOMMatrix",
+      "setInterval",
+      "clearInterval",
+      script,
+    );
+    execute(
+      window,
+      window.document,
+      window.HTMLElement,
+      globalThis.DOMMatrix,
+      timers.setInterval ??
+        (((callback: TimerHandler) => {
+          void callback;
+          return 0 as never;
+        }) as typeof globalThis.setInterval),
+      timers.clearInterval ?? globalThis.clearInterval,
+    );
+  }
+
+  it("reapplies box-size after seek", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-box-size="true"
+        style="--hf-studio-width: 200px; --hf-studio-height: 100px; width: 200px; height: 100px">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    const originalSeek = () => {
+      card.style.removeProperty("width");
+      card.style.removeProperty("height");
+    };
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: originalSeek };
+
+    runPositionScript(window);
+    const wrappedSeek = (window as unknown as { __hf: { seek: (t: number) => void } }).__hf.seek;
+    wrappedSeek(1);
+
+    expect(card.style.getPropertyValue("width")).toBe("200px");
+    expect(card.style.getPropertyValue("height")).toBe("100px");
+  });
+
+  it("strips GSAP translate from transform after reapplying path offset", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-path-offset="true"
+        data-hf-studio-original-translate=""
+        style="--hf-studio-offset-x: 50px; --hf-studio-offset-y: 30px; translate: var(--hf-studio-offset-x, 0px) var(--hf-studio-offset-y, 0px)">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    const originalSeek = () => {
+      card.style.setProperty("transform", "matrix(1, 0, 0, 1, 120, 60)");
+    };
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: originalSeek };
+
+    runPositionScript(window);
+    const wrappedSeek = (window as unknown as { __hf: { seek: (t: number) => void } }).__hf.seek;
+    wrappedSeek(1);
+
+    expect(card.style.getPropertyValue("translate")).toContain("--hf-studio-offset-x");
+    const transform = card.style.getPropertyValue("transform");
+    if (transform && transform !== "none") {
+      const m = new DOMMatrix(transform);
+      expect(m.m41).toBe(0);
+      expect(m.m42).toBe(0);
+    }
+  });
+
+  it("preserves non-translate components when stripping GSAP transform", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-path-offset="true"
+        data-hf-studio-original-translate=""
+        style="--hf-studio-offset-x: 10px; --hf-studio-offset-y: 20px; translate: var(--hf-studio-offset-x, 0px) var(--hf-studio-offset-y, 0px)">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    const originalSeek = () => {
+      card.style.setProperty("transform", "matrix(0.5, 0, 0, 0.5, 80, 40)");
+    };
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: originalSeek };
+
+    runPositionScript(window);
+    const wrappedSeek = (window as unknown as { __hf: { seek: (t: number) => void } }).__hf.seek;
+    wrappedSeek(1);
+
+    const transform = card.style.getPropertyValue("transform");
+    expect(transform).toBeTruthy();
+    expect(transform).not.toContain("80");
+    expect(transform).not.toContain("40");
+  });
+
+  it("removes transform entirely when it becomes identity after stripping translate", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-path-offset="true"
+        data-hf-studio-original-translate=""
+        style="--hf-studio-offset-x: 10px; --hf-studio-offset-y: 20px; translate: var(--hf-studio-offset-x, 0px) var(--hf-studio-offset-y, 0px)">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    const originalSeek = () => {
+      card.style.setProperty("transform", "matrix(1, 0, 0, 1, 50, 25)");
+    };
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: originalSeek };
+
+    runPositionScript(window);
+    const wrappedSeek = (window as unknown as { __hf: { seek: (t: number) => void } }).__hf.seek;
+    wrappedSeek(1);
+
+    const transform = card.style.getPropertyValue("transform");
+    expect(!transform || transform === "none" || transform === "").toBe(true);
+  });
+
+  it("no-ops when transform is 'none'", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-path-offset="true"
+        data-hf-studio-original-translate=""
+        style="--hf-studio-offset-x: 10px; --hf-studio-offset-y: 20px; translate: var(--hf-studio-offset-x, 0px) var(--hf-studio-offset-y, 0px); transform: none">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: () => {} };
+    runPositionScript(window);
+
+    expect(card.style.getPropertyValue("transform")).toBe("none");
+  });
+
+  it("strips GSAP translate for rotation-only elements", () => {
+    const window = new Window();
+    window.document.body.innerHTML = `
+      <div id="card"
+        data-hf-studio-rotation="true"
+        data-hf-studio-original-rotate=""
+        style="--hf-studio-rotation: 45deg; rotate: var(--hf-studio-rotation, 0deg)">
+      </div>
+    `;
+    const card = window.document.getElementById("card") as unknown as HTMLElement;
+
+    const originalSeek = () => {
+      card.style.setProperty("transform", "matrix(1, 0, 0, 1, 100, 50)");
+    };
+    (window as unknown as { __hf: Record<string, unknown> }).__hf = { seek: originalSeek };
+
+    runPositionScript(window);
+    const wrappedSeek = (window as unknown as { __hf: { seek: (t: number) => void } }).__hf.seek;
+    wrappedSeek(1);
+
+    expect(card.style.getPropertyValue("rotate")).toContain("--hf-studio-rotation");
+    const transform = card.style.getPropertyValue("transform");
+    expect(!transform || transform === "none" || transform === "").toBe(true);
+  });
+});
diff --git a/packages/core/src/studio-api/helpers/manualEditsRenderScript.ts b/packages/core/src/studio-api/helpers/manualEditsRenderScript.ts
@@ -251,6 +251,7 @@ function studioPositionSeekReapplyRuntime(): void {
       const rot = el.style.getPropertyValue(ROTATION_PROP);
       if (rot) {
         el.style.setProperty("rotate", composeRotation(el, "var(" + ROTATION_PROP + ", 0deg)"));
+        stripGsapTranslateFromTransform(el);
       }
     }
     reapplyMotionTimeline();
diff --git a/packages/studio/src/components/editor/domEditing.test.ts b/packages/studio/src/components/editor/domEditing.test.ts
@@ -321,6 +321,29 @@ describe("resolveVisualDomEditSelectionTarget", () => {
     expect(visualTarget).toBe(headline);
     expect(explicitSelection?.id).toBe("container");
   });
+
+  it("prefers the visually-on-top sibling over a deeper element in a separate visual layer", () => {
+    const document = createDocument(`
+      <div id="comp-root">
+        <div id="sub-comp" class="sub-comp">
+          <img id="sf-chrome" class="sf-chrome" style="width:100%;height:100%" />
+        </div>
+        <video id="pip-studio" class="pip-studio" style="position:absolute;z-index:15" />
+      </div>
+    `);
+    const pipStudio = document.getElementById("pip-studio") as HTMLElement;
+    const sfChrome = document.getElementById("sf-chrome") as HTMLElement;
+    const subComp = document.getElementById("sub-comp") as HTMLElement;
+    setElementRect(pipStudio, { left: 50, top: 50, width: 320, height: 320 });
+    setElementRect(sfChrome, { left: 0, top: 0, width: 1920, height: 1080 });
+    setElementRect(subComp, { left: 0, top: 0, width: 1920, height: 1080 });
+
+    expect(
+      resolveVisualDomEditSelectionTarget([pipStudio, subComp, sfChrome], {
+        activeCompositionPath: "index.html",
+      }),
+    ).toBe(pipStudio);
+  });
 });
 
 describe("isLargeRasterDomEditSelection", () => {
diff --git a/packages/studio/src/utils/studioPreviewHelpers.ts b/packages/studio/src/utils/studioPreviewHelpers.ts
@@ -1,24 +1,18 @@
-import type { DomEditViewport, DomEditSelection } from "../components/editor/domEditing";
+import type { DomEditViewport } from "../components/editor/domEditing";
 import { resolveVisualDomEditSelectionTarget } from "../components/editor/domEditing";
 import {
   getDomLayerPatchTarget,
   isElementComputedVisible,
 } from "../components/editor/domEditingElement";
-import { usePlayerStore, liveTime } from "../player";
 import { getEventTargetElement } from "./studioHelpers";
 
-export interface PreviewLocalPointer {
+interface PreviewLocalPointer {
   x: number;
   y: number;
   viewport: DomEditViewport;
 }
 
-export interface PreviewPlayerCompat {
-  getTime: () => number;
-  renderSeek: (timeSeconds: number) => void;
-}
-
-export function resolvePreviewLocalPointer(
+function resolvePreviewLocalPointer(
   iframe: HTMLIFrameElement,
   doc: Document,
   win: Window,
@@ -42,24 +36,6 @@ export function resolvePreviewLocalPointer(
   };
 }
 
-export function getPreviewLocalPointer(
-  iframe: HTMLIFrameElement,
-  clientX: number,
-  clientY: number,
-): PreviewLocalPointer | null {
-  let doc: Document | null = null;
-  let win: Window | null = null;
-  try {
-    doc = iframe.contentDocument;
-    win = iframe.contentWindow;
-  } catch {
-    return null;
-  }
-  if (!doc || !win) return null;
-
-  return resolvePreviewLocalPointer(iframe, doc, win, clientX, clientY);
-}
-
 const POINTER_EVENTS_OVERRIDE_ID = "__hf_studio_pointer_events_override__";
 
 function forcePointerEventsAuto(doc: Document): HTMLStyleElement | null {
@@ -122,21 +98,6 @@ export function getPreviewTargetFromPointer(
   }
 }
 
-export function buildRasterClickSelectionContext(
-  selection: DomEditSelection,
-  localPointer: PreviewLocalPointer,
-): string {
-  return [
-    "The user clicked a large raster/background element in the Studio preview.",
-    `Preview click: x=${Math.round(localPointer.x)}px, y=${Math.round(localPointer.y)}px in a ${Math.round(
-      localPointer.viewport.width,
-    )}x${Math.round(localPointer.viewport.height)} composition.`,
-    `Selected target: <${selection.tagName}> ${selection.selector ?? selection.id ?? selection.label}.`,
-    "Visible copy or artwork at that point may be baked into the selected image/background rather than a selectable DOM text layer.",
-    "If the request mentions text seen at the click location, inspect or replace the image asset, or recreate that visible copy as editable DOM.",
-  ].join("\n");
-}
-
 function objectLike(value: unknown): object | null {
   return value && (typeof value === "object" || typeof value === "function") ? value : null;
 }
@@ -162,33 +123,6 @@ function readPlaybackTime(target: object | null, key: string): number | null {
   }
 }
 
-export function getPreviewPlayer(win: Window | null | undefined): PreviewPlayerCompat | null {
-  const player = objectLike(win ? Reflect.get(win, "__player") : null);
-  if (!player) return null;
-  const getTime = Reflect.get(player, "getTime");
-  const renderSeek = Reflect.get(player, "renderSeek");
-  if (typeof getTime !== "function" || typeof renderSeek !== "function") return null;
-  return {
-    getTime: () => {
-      const value = getTime.call(player);
-      return typeof value === "number" && Number.isFinite(value) ? value : 0;
-    },
-    renderSeek: (timeSeconds: number) => {
-      renderSeek.call(player, timeSeconds);
-    },
-  };
-}
-
-export function seekStudioPreview(iframe: HTMLIFrameElement | null, timeSeconds: number): boolean {
-  const player = getPreviewPlayer(iframe?.contentWindow);
-  if (!player) return false;
-  const nextTime = Math.max(0, timeSeconds);
-  player.renderSeek(nextTime);
-  usePlayerStore.getState().setCurrentTime(nextTime);
-  liveTime.notify(nextTime);
-  return true;
-}
-
 export function pauseStudioPreviewPlayback(iframe: HTMLIFrameElement | null): number | null {
   const win = iframe?.contentWindow;
   if (!win) return null;

Original file line number	Diff line number	Diff line change
`@@ -251,6 +251,7 @@ function studioPositionSeekReapplyRuntime(): void {`
`251`	`251`	`const rot = el.style.getPropertyValue(ROTATION_PROP);`
`252`	`252`	`if (rot) {`
`253`	`253`	`el.style.setProperty("rotate", composeRotation(el, "var(" + ROTATION_PROP + ", 0deg)"));`
	`254`	`+ stripGsapTranslateFromTransform(el);`
`254`	`255`	`}`
`255`	`256`	`}`
`256`	`257`	`reapplyMotionTimeline();`