mirror of
https://github.com/heygen-com/hyperframes.git
synced 2026-09-03 04:38:33 +00:00
fix(studio): address PR review — add tests, strip GSAP transform for rotation, remove dead exports
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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user