Skip to content

Commit 60c02c1

Browse files
juliusmarmingeclaude
juliusmarminge
and
claude
committed
refactor(web): video previews attach listeners on their element and share one visibility observer
Problem: MediaVideoPlayer stored its <video> element in state only so two effects could bind to it. That cost every video an extra render on mount, gave each video its own IntersectionObserver, and tracked "which element is playing" to work around remounts. Fix: the video's ref callback owns the visibility/fullscreen listeners and pauses on detach (also resetting the playing flag, since a detached element's pause event never fires). The preload trigger moves to the wrapper's ref callback and uses the shared observeIntersection helper, so all video previews share one observer. Playing state becomes a boolean. Effects: MediaVideoPlayer 2 -> 0. Both were element lifetime work, which a ref callback with cleanup models directly. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent dd5b540 commit 60c02c1

2 files changed

Lines changed: 64 additions & 33 deletions

File tree

‎apps/web/src/components/media/MediaVideoPlayer.browser.test.tsx‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, it } from "vite-plus/test";
1+
import { describe, expect, it, vi } from "vite-plus/test";
22

33
import { act, render, stubGlobal, type Screen } from "../../../test/render";
44
import { MediaVideoPlayer } from "./MediaVideoPlayer";
@@ -10,6 +10,18 @@ class HiddenIntersectionObserver {
1010
disconnect() {}
1111
}
1212

13+
class VisibleIntersectionObserver {
14+
constructor(private readonly callback: IntersectionObserverCallback) {}
15+
observe(target: Element) {
16+
this.callback(
17+
[{ target, isIntersecting: true } as IntersectionObserverEntry],
18+
this as unknown as IntersectionObserver,
19+
);
20+
}
21+
unobserve() {}
22+
disconnect() {}
23+
}
24+
1325
function expectVideoSrc(screen: Screen, src: string) {
1426
return expect.element(screen.getByLabelText("clip")).toHaveAttribute("src", src);
1527
}
@@ -55,4 +67,27 @@ describe("MediaVideoPlayer", () => {
5567
await screen.rerender(<MediaVideoPlayer src={null} label="clip" revision="mutation-2" />);
5668
await expectVideoSrc(screen, "/asset/first/clip.mp4");
5769
});
70+
71+
it("starts loading metadata once the player scrolls near the viewport", async () => {
72+
using _observer = stubGlobal("IntersectionObserver", VisibleIntersectionObserver);
73+
await using screen = await render(<MediaVideoPlayer src="/asset/clip.mp4" label="clip" />);
74+
75+
await expect.element(screen.getByLabelText("clip")).toHaveAttribute("preload", "metadata");
76+
});
77+
78+
it("keeps a playing video playing when the player rerenders", async () => {
79+
using _observer = stubGlobal("IntersectionObserver", HiddenIntersectionObserver);
80+
await using screen = await render(
81+
<MediaVideoPlayer src="/asset/clip.mp4" label="clip" className="w-40" />,
82+
);
83+
const video = screen.getByLabelText("clip").element() as HTMLVideoElement;
84+
using pause = vi.spyOn(video, "pause");
85+
await act(() => {
86+
video.dispatchEvent(new Event("play"));
87+
});
88+
89+
await screen.rerender(<MediaVideoPlayer src="/asset/clip.mp4" label="clip" className="w-48" />);
90+
91+
expect(pause).not.toHaveBeenCalled();
92+
});
5893
});

‎apps/web/src/components/media/MediaVideoPlayer.tsx‎

Lines changed: 28 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import { settlePromise } from "@t3tools/client-runtime/state/runtime";
22
import { AsyncResult } from "effect/reactivity";
33
import { PlayIcon, RotateCwIcon, TriangleAlertIcon } from "lucide-react";
4-
import { useEffect, useState, type CSSProperties } from "react";
4+
import { useState, type CSSProperties } from "react";
55

6+
import { observeIntersection } from "../../lib/observeIntersection";
67
import { cn } from "../../lib/utils";
78
import { prepareVideoFirstFrame } from "../../lib/videoFirstFrame";
89
import { Button } from "../ui/button";
@@ -44,13 +45,11 @@ export function MediaVideoPlayer({
4445
onRetry,
4546
actionsSource,
4647
}: MediaVideoPlayerProps) {
47-
// Use state so effects rebind when retry or failure remounts the video.
48-
const [video, setVideo] = useState<HTMLVideoElement | null>(null);
4948
const [playbackSource, setPlaybackSource] = useState<{
5049
src: string;
5150
revision: string | null;
5251
} | null>(null);
53-
const [playingVideo, setPlayingVideo] = useState<HTMLVideoElement | null>(null);
52+
const [playing, setPlaying] = useState(false);
5453
const [failedSrc, setFailedSrc] = useState<string | null>(null);
5554
const [retrying, setRetrying] = useState(false);
5655
const [loadAttempt, setLoadAttempt] = useState(0);
@@ -65,33 +64,15 @@ export function MediaVideoPlayer({
6564
playbackSource !== null &&
6665
playbackSource.revision !== revision &&
6766
(latestSrc !== null || sourceFailed) &&
68-
(video === null || playingVideo !== video)
67+
!playing
6968
) {
7069
setPlaybackSource(
7170
revision !== null && latestSrc !== null ? { src: latestSrc, revision } : null,
7271
);
7372
}
7473

75-
useEffect(() => {
76-
if (!video || preload === "metadata" || preloaded) return;
77-
let active = true;
78-
const observer = new IntersectionObserver(
79-
(entries) => {
80-
if (!active || !entries.some((entry) => entry.isIntersecting)) return;
81-
setPreloadedSrc(src);
82-
observer.disconnect();
83-
},
84-
{ rootMargin: "200px" },
85-
);
86-
observer.observe(video);
87-
return () => {
88-
active = false;
89-
observer.disconnect();
90-
};
91-
}, [video, src, preload, preloaded]);
92-
93-
useEffect(() => {
94-
if (!video) return;
74+
// Retry and failure remount the element, so its listeners live and die with it.
75+
const attachVideo = (video: HTMLVideoElement) => {
9576
const pauseWhenHidden = () => {
9677
// Native fullscreen can hide the inline page while this video is still visible.
9778
const fullscreen =
@@ -107,8 +88,22 @@ export function MediaVideoPlayer({
10788
document.removeEventListener("fullscreenchange", pauseWhenHidden);
10889
video.removeEventListener("webkitendfullscreen", pauseWhenHidden);
10990
video.pause();
91+
// A detached element's pause event never reaches onPause.
92+
setPlaying(false);
11093
};
111-
}, [video]);
94+
};
95+
96+
// On the wrapper, not the video: re-running the video's ref callback would pause playback.
97+
const observePreload = (player: HTMLSpanElement) => {
98+
if (src === null || failed || preload === "metadata" || preloaded) return;
99+
return observeIntersection(
100+
player,
101+
(entry) => {
102+
if (entry.isIntersecting) setPreloadedSrc(src);
103+
},
104+
{ rootMargin: "200px" },
105+
);
106+
};
112107

113108
const retry = async () => {
114109
if (retrying) return;
@@ -128,6 +123,7 @@ export function MediaVideoPlayer({
128123

129124
const player = (
130125
<span
126+
ref={observePreload}
131127
className={cn("relative inline-block align-middle", className)}
132128
style={style}
133129
data-markdown-copy={copyMarkdown}
@@ -162,7 +158,7 @@ export function MediaVideoPlayer({
162158
) : src !== null && !failed ? (
163159
<video
164160
key={loadAttempt}
165-
ref={setVideo}
161+
ref={attachVideo}
166162
src={src}
167163
aria-label={label || "Video preview"}
168164
aria-hidden={onOpen ? true : undefined}
@@ -181,14 +177,14 @@ export function MediaVideoPlayer({
181177
prepareVideoFirstFrame(event.currentTarget);
182178
if (sourceRevision !== null) setPlaybackSource({ src, revision: sourceRevision });
183179
}}
184-
onPlay={(event) => {
185-
setPlayingVideo(event.currentTarget);
180+
onPlay={() => {
181+
setPlaying(true);
186182
setPlaybackSource({ src, revision: sourceRevision });
187183
}}
188-
onPause={() => setPlayingVideo(null)}
189-
onEnded={() => setPlayingVideo(null)}
184+
onPause={() => setPlaying(false)}
185+
onEnded={() => setPlaying(false)}
190186
onError={() => {
191-
setPlayingVideo(null);
187+
setPlaying(false);
192188
setPlaybackSource(null);
193189
if (latestSrc === null || src === latestSrc) setFailedSrc(src);
194190
}}

0 commit comments

Comments
 (0)