Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,47 @@ function mount(onSelect = vi.fn(), onAddToTimeline = vi.fn()) {
}

describe("composition card drag", () => {
it("uses a cached image instead of eagerly mounting a live preview iframe", () => {
const { host } = mount();
const thumbnail = host.querySelector<HTMLImageElement>('img[src*="/thumbnail/"]');
expect(thumbnail).not.toBeNull();
expect(new URL(thumbnail?.src ?? "").searchParams.get("t")).toBe("3.00");
expect(host.querySelector("iframe")).toBeNull();
});

it("shows a fallback when the cached thumbnail fails", () => {
const { host } = mount();
const thumbnail = host.querySelector<HTMLImageElement>('img[src*="/thumbnail/"]');
if (!thumbnail) throw new Error("composition thumbnail did not render");

act(() => thumbnail.dispatchEvent(new Event("error")));

expect(host.textContent).toContain("Preview unavailable");
expect(host.querySelector('img[src*="/thumbnail/"]')).toBeNull();
});

it("mounts one live preview only after sustained hover and removes it on leave", () => {
vi.useFakeTimers();
const consoleError = vi.spyOn(console, "error").mockImplementation(() => {});
try {
const { host, card } = mount();
act(() => {
card.dispatchEvent(new Event("pointerover", { bubbles: true }));
vi.advanceTimersByTime(300);
});
expect(host.querySelectorAll("iframe")).toHaveLength(1);

act(() => {
card.dispatchEvent(new Event("pointerout", { bubbles: true }));
});
expect(host.querySelector("iframe")).toBeNull();
expect(vi.getTimerCount()).toBe(0);
} finally {
consoleError.mockRestore();
vi.useRealTimers();
}
});

it("keeps ordinary click navigation", () => {
const { card, onSelect } = mount();
act(() => card.click());
Expand Down
98 changes: 66 additions & 32 deletions packages/studio/src/components/sidebar/CompositionsTab.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { memo, useCallback, useEffect, useRef, useState } from "react";
import { setPreviewMediaMuted } from "../../player/lib/timelineIframeHelpers";
import { buildCompositionThumbnailUrl } from "../../player/components/CompositionThumbnail";
import { TIMELINE_COMPOSITION_MIME } from "../../utils/timelineCompositionDrop";

interface CompositionsTabProps {
Expand Down Expand Up @@ -130,6 +131,8 @@ function CompCard({
}) {
const [hovered, setHovered] = useState(false);
const [stageSize, setStageSize] = useState(DEFAULT_PREVIEW_STAGE);
const [livePreviewLoaded, setLivePreviewLoaded] = useState(false);
const [thumbnailFailed, setThumbnailFailed] = useState(false);
const iframeRef = useRef<HTMLIFrameElement | null>(null);
const hoverTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
const syncTimer = useRef<ReturnType<typeof setTimeout> | null>(null);
Expand Down Expand Up @@ -158,10 +161,21 @@ function CompCard({
clearTimeout(hoverTimer.current);
hoverTimer.current = null;
}
if (syncTimer.current) {
clearTimeout(syncTimer.current);
syncTimer.current = null;
}
setHovered(false);
setLivePreviewLoaded(false);
};
const name = comp.replace(/^compositions\//, "").replace(/\.html$/, "");
const previewUrl = `/api/projects/${projectId}/preview/comp/${comp}`;
const thumbnailUrl = buildCompositionThumbnailUrl({
previewUrl,
seekTime: THUMBNAIL_SEEK_TIME_SECONDS,
duration: 0,
origin: window.location.origin,
});
const previewScale = resolveCompositionPreviewScale({
cardWidth: CARD_W,
cardHeight: CARD_H,
Expand All @@ -172,7 +186,7 @@ function CompCard({
const thumbnailOffsetY = (CARD_H - stageSize.height * previewScale) / 2;

useEffect(() => {
requestIframePlaybackSync(hovered);
if (hovered) requestIframePlaybackSync(true);
}, [hovered, requestIframePlaybackSync]);

useEffect(() => {
Expand Down Expand Up @@ -216,36 +230,56 @@ function CompCard({
}`}
>
<div className="w-20 h-[45px] rounded overflow-hidden bg-neutral-900 flex-shrink-0 relative">
<iframe
ref={iframeRef}
src={previewUrl}
sandbox="allow-scripts allow-same-origin"
loading="lazy"
className="absolute border-none pointer-events-none"
style={{
transformOrigin: "0 0",
width: stageSize.width,
height: stageSize.height,
left: thumbnailOffsetX,
top: thumbnailOffsetY,
transform: `scale(${previewScale})`,
}}
onLoad={(e) => {
try {
const iframe = e.currentTarget;
const root = iframe.contentDocument?.querySelector("[data-composition-id]");
const width = Number(root?.getAttribute("data-width")) || DEFAULT_PREVIEW_STAGE.width;
const height =
Number(root?.getAttribute("data-height")) || DEFAULT_PREVIEW_STAGE.height;
setStageSize({ width, height });
requestIframePlaybackSync(hovered);
} catch {
setStageSize(DEFAULT_PREVIEW_STAGE);
}
}}
title={`${name} preview`}
tabIndex={-1}
/>
{thumbnailFailed ? (
<div className="absolute inset-0 flex items-center justify-center px-1 text-center text-[8px] leading-tight text-neutral-600">
Preview unavailable
</div>
) : (
<img
src={thumbnailUrl}
alt=""
draggable={false}
loading="lazy"
decoding="async"
onError={() => setThumbnailFailed(true)}
className={`absolute inset-0 h-full w-full object-contain transition-opacity ${
livePreviewLoaded ? "opacity-0" : "opacity-100"
}`}
/>
)}
{hovered && (
<iframe
ref={iframeRef}
src={previewUrl}
sandbox="allow-scripts allow-same-origin"
className="absolute border-none pointer-events-none"
style={{
transformOrigin: "0 0",
width: stageSize.width,
height: stageSize.height,
left: thumbnailOffsetX,
top: thumbnailOffsetY,
transform: `scale(${previewScale})`,
}}
onLoad={(e) => {
try {
const iframe = e.currentTarget;
const root = iframe.contentDocument?.querySelector("[data-composition-id]");
const width =
Number(root?.getAttribute("data-width")) || DEFAULT_PREVIEW_STAGE.width;
const height =
Number(root?.getAttribute("data-height")) || DEFAULT_PREVIEW_STAGE.height;
setStageSize({ width, height });
setLivePreviewLoaded(true);
requestIframePlaybackSync(true);
} catch {
setStageSize(DEFAULT_PREVIEW_STAGE);
}
}}
title={`${name} preview`}
tabIndex={-1}
/>
)}
</div>
<div
className="min-w-0 flex-1"
Expand Down Expand Up @@ -331,7 +365,7 @@ export const CompositionsTab = memo(function CompositionsTab({
<div className="flex-1 overflow-y-auto">
{compositions.map((comp) => (
<CompCard
key={comp}
key={`${projectId}:${comp}`}
projectId={projectId}
comp={comp}
isActive={activeComposition === comp}
Expand Down
155 changes: 143 additions & 12 deletions packages/studio/src/player/components/Player.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,146 @@
// @vitest-environment happy-dom

import { describe, expect, it } from "vitest";
import { hasUnloadedAssets, shouldShowCompositionLoadingOverlay } from "./Player";
import { act, createElement } from "react";
import { createRoot, type Root } from "react-dom/client";
import { afterEach, describe, expect, it, vi } from "vitest";
import {
hasUnloadedAssets,
Player,
readPreviewErrorMessage,
shouldShowCompositionLoadingOverlay,
} from "./Player";

vi.mock("@hyperframes/player", () => ({}));

Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });

let root: Root | null = null;
let lifecycleLog: string[] = [];

class TestHyperframesPlayer extends HTMLElement {
readonly iframeElement = document.createElement("iframe");

constructor() {
super();

const addIframeListener = this.iframeElement.addEventListener.bind(this.iframeElement);
this.iframeElement.addEventListener = ((type, listener, options) => {
lifecycleLog.push(`iframe:${type}`);
addIframeListener(type, listener, options);
}) as typeof this.iframeElement.addEventListener;

const addPlayerListener = this.addEventListener.bind(this);
this.addEventListener = ((type, listener, options) => {
lifecycleLog.push(`player:${type}`);
addPlayerListener(type, listener, options);
}) as typeof this.addEventListener;

const setPlayerAttribute = this.setAttribute.bind(this);
this.setAttribute = (name, value) => {
if (name === "src") lifecycleLog.push("src");
setPlayerAttribute(name, value);
};
}
}

if (!customElements.get("hyperframes-player")) {
customElements.define("hyperframes-player", TestHyperframesPlayer);
}

afterEach(() => {
if (root) act(() => root?.unmount());
root = null;
lifecycleLog = [];
document.body.innerHTML = "";
});

async function mountPlayer() {
const host = document.createElement("div");
document.body.append(host);
root = createRoot(host);
await act(async () => {
root?.render(
createElement(Player, {
directUrl: "/api/projects/demo/preview",
onLoad: vi.fn(),
suppressLoadingOverlay: true,
}),
);
await Promise.resolve();
});

const player = host.querySelector<TestHyperframesPlayer>("hyperframes-player");
if (!player) throw new Error("player did not mount");
return { host, player };
}

function createAudioIframe() {
const iframe = document.createElement("iframe");
document.body.appendChild(iframe);
const audio = iframe.contentDocument?.createElement("audio");
expect(audio).toBeDefined();
iframe.contentDocument?.body.appendChild(audio!);
return { audio: audio!, iframe };
}

describe("preview errors", () => {
it("reads the player probe error for the visible retry state", () => {
expect(
readPreviewErrorMessage(
new CustomEvent("error", {
detail: { message: "Composition timeline not found after 8s" },
}),
),
).toBe("Composition timeline not found after 8s");
});

it("falls back when the player emits an unstructured error", () => {
expect(readPreviewErrorMessage(new Event("error"))).toBe(
"The composition preview did not become ready.",
);
});

it("attaches lifecycle listeners before navigating the player", async () => {
await mountPlayer();
const srcIndex = lifecycleLog.indexOf("src");

expect(srcIndex).toBeGreaterThan(-1);
for (const listener of [
"iframe:load",
"player:click",
"player:shadertransitionstate",
"player:ready",
"player:error",
]) {
expect(lifecycleLog.indexOf(listener)).toBeGreaterThan(-1);
expect(lifecycleLog.indexOf(listener)).toBeLessThan(srcIndex);
}
});

it("retries a failed preview with a fresh player URL", async () => {
const { host, player } = await mountPlayer();

act(() => {
player.dispatchEvent(
new CustomEvent("error", {
detail: { message: "Composition timeline not found after 8s" },
}),
);
});

expect(host.querySelector('[data-testid="composition-preview-error"]')).not.toBeNull();
const retry = Array.from(host.querySelectorAll("button")).find(
(button) => button.textContent === "Retry preview",
);
if (!retry) throw new Error("retry action did not render");

act(() => retry.click());

const retryUrl = new URL(player.getAttribute("src") ?? "", window.location.origin);
expect(retryUrl.searchParams.get("_hfStudioRetry")).toBe("1");
expect(host.querySelector('[data-testid="composition-preview-error"]')).toBeNull();
});
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The PR body says this PR adds "focused regression coverage for listener ordering" — but the only new tests in this file cover readPreviewErrorMessage message parsing. There is no test that asserts the invariant the fix hinges on: that load / click / shadertransitionstate / ready / error listeners attach BEFORE player.setAttribute("src", src) and container.appendChild(player). The race the PR is closing ("a cached preview loads before listeners are attached") is inherently a code-ordering invariant, so ordering is what a regression test would need to observe.

A reasonable pin: mount the Player under jsdom / happy-dom, spy on player.addEventListener and player.setAttribute, then assert the recorded call order has all four addEventListener("ready" | "error" | ...) calls precede the setAttribute("src", ...) call. Reordering them back later would then fail CI instead of shipping.

Not a blocker — the fix looks correct at HEAD — but the marquee invariant this PR is buying isn't defended by a test that would fire when it regresses.

— Rames D Jusso


describe("composition loading overlay", () => {
it("shows while the composition is loading", () => {
Expand All @@ -13,10 +152,7 @@ describe("composition loading overlay", () => {
});

it("keeps the asset overlay up while media is still buffering", () => {
const iframe = document.createElement("iframe");
document.body.appendChild(iframe);
const audio = iframe.contentDocument?.createElement("audio");
expect(audio).toBeDefined();
const { audio, iframe } = createAudioIframe();
Object.defineProperty(audio, "readyState", {
value: 0,
configurable: true,
Expand All @@ -25,18 +161,14 @@ describe("composition loading overlay", () => {
value: 2,
configurable: true,
});
iframe.contentDocument?.body.appendChild(audio!);

expect(hasUnloadedAssets(iframe, false)).toBe(true);

iframe.remove();
});

it("does not keep the asset overlay stuck on failed media sources", () => {
const iframe = document.createElement("iframe");
document.body.appendChild(iframe);
const audio = iframe.contentDocument?.createElement("audio");
expect(audio).toBeDefined();
const { audio, iframe } = createAudioIframe();
Object.defineProperty(audio, "error", {
value: { code: 4, message: "format error" },
configurable: true,
Expand All @@ -49,7 +181,6 @@ describe("composition loading overlay", () => {
value: 3,
configurable: true,
});
iframe.contentDocument?.body.appendChild(audio!);

expect(hasUnloadedAssets(iframe, false)).toBe(false);

Expand Down
Loading
Loading