Skip to content
Open
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
7 changes: 7 additions & 0 deletions .changeset/lightbox-pause-on-close.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] Closing a Lightbox stops a video that was playing. The viewer stays mounted, so the picture and its sound would otherwise continue after Close, Escape, or a tap outside. A viewer that is already closed does not start playback either. Opening it again plays the current video when `hasAutoPlay` is set.

@SebTardif
97 changes: 97 additions & 0 deletions packages/core/src/Lightbox/Lightbox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -598,6 +598,103 @@ describe('Lightbox', () => {
expect(pause).toHaveBeenCalledTimes(1);
});

it('pauses a playing video when the viewer closes', () => {
const items = [
{src: '/clip.mp4', alt: 'A clip', type: 'video' as const},
{src: '/b.jpg', alt: 'Image B'},
];
const view = (isOpen: boolean) => (
<Lightbox
isOpen={isOpen}
onOpenChange={() => {}}
media={items}
index={0}
/>
);
const {rerender} = render(view(true));
const video = document.querySelector('video')!;
// Started by someone: playing, with its sound. Closing keeps the
// element mounted inside the closed dialog.
const pause = vi.fn();
video.pause = pause;
Object.defineProperty(video, 'paused', {
configurable: true,
get: () => pause.mock.calls.length > 0,
});
rerender(view(false));
expect(document.querySelector('video')).toBe(video);
expect(pause).toHaveBeenCalledTimes(1);
});

it('pauses a single playing video when the viewer closes', () => {
const media = {src: '/clip.mp4', alt: 'A clip', type: 'video' as const};
const view = (isOpen: boolean) => (
<Lightbox isOpen={isOpen} onOpenChange={() => {}} media={media} />
);
const {rerender} = render(view(true));
const video = document.querySelector('video')!;
const pause = vi.fn();
video.pause = pause;
Object.defineProperty(video, 'paused', {
configurable: true,
get: () => pause.mock.calls.length > 0,
});
rerender(view(false));
expect(document.querySelector('video')).toBe(video);
expect(pause).toHaveBeenCalledTimes(1);
});

it('plays a single video again when the viewer reopens, with hasAutoPlay', () => {
const play = vi
.spyOn(HTMLMediaElement.prototype, 'play')
.mockImplementation(async () => {});
try {
const media = {src: '/clip.mp4', alt: 'A clip', type: 'video' as const};
const view = (isOpen: boolean) => (
<Lightbox
isOpen={isOpen}
onOpenChange={() => {}}
media={media}
hasAutoPlay
/>
);
const {rerender} = render(view(true));
expect(play).toHaveBeenCalledTimes(1);
const video = document.querySelector('video')!;
const pause = vi.fn();
video.pause = pause;
Object.defineProperty(video, 'paused', {
configurable: true,
get: () => pause.mock.calls.length > 0,
});
rerender(view(false));
expect(pause).toHaveBeenCalledTimes(1);
rerender(view(true));
expect(play).toHaveBeenCalledTimes(2);
expect(play.mock.contexts.at(-1)).toBe(video);
} finally {
play.mockRestore();
}
});

it('withholds autoplay while the viewer is mounted closed', () => {
const media = {src: '/clip.mp4', alt: 'A clip', type: 'video' as const};
const view = (isOpen: boolean) => (
<Lightbox
isOpen={isOpen}
onOpenChange={() => {}}
media={media}
hasAutoPlay
/>
);
const {rerender} = render(view(false));
// The element stays mounted. The attribute would start playback once
// the file loads, after the close effect has already run.
expect(document.querySelector('video')).not.toHaveAttribute('autoplay');
rerender(view(true));
expect(document.querySelector('video')).toHaveAttribute('autoplay');
});

it("keeps a paged-away video's element and time once the viewer pages back", () => {
const items = [
{src: '/clip.mp4', alt: 'A clip', type: 'video' as const},
Expand Down
25 changes: 20 additions & 5 deletions packages/core/src/Lightbox/Lightbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -623,26 +623,41 @@ export function Lightbox({
// A video left on another page stops: the pager keeps the neighbours
// mounted, and a playing one would go on, with its sound, off screen. With
// `hasAutoPlay`, the video paged to plays again, as a newly shown one does.
// The closed dialog keeps them mounted too, so closing stops a playing
// video. A single video has no page element.
useEffect(() => {
const container = containerRef.current;
if (container == null) {
return;
}
if (!isOpen) {
for (const video of container.querySelectorAll<HTMLVideoElement>(
'video',
)) {
if (!video.paused) {
video.pause();
}
}
return;
}
Comment thread
astracat-bot[bot] marked this conversation as resolved.
Resolved
for (const video of container.querySelectorAll<HTMLVideoElement>(
'[data-lightbox-page]:not([data-current]) video',
)) {
if (!video.paused) {
video.pause();
}
}
const current = container.querySelector<HTMLVideoElement>(
'[data-lightbox-page][data-current] video',
);
const onAPage = container.querySelector('[data-lightbox-page]') != null;
const current =
container.querySelector<HTMLVideoElement>(
'[data-lightbox-page][data-current] video',
) ??
(onAPage ? null : container.querySelector<HTMLVideoElement>('video'));
if (hasAutoPlay && current != null && current.paused) {
// A browser can refuse playback without a user's gesture.
void current.play()?.catch(() => {});
}
}, [index, hasAutoPlay]);
}, [index, hasAutoPlay, isOpen]);

// The backdrop, the controls and the pages a dismissal faded, to restore
// exactly those. A page faded as a neighbour may be the current one when
Expand Down Expand Up @@ -1323,7 +1338,7 @@ export function Lightbox({
src={src}
aria-label={alt}
controls
autoPlay={isCurrent && hasAutoPlay}
autoPlay={isOpen && isCurrent && hasAutoPlay}
preload={isCurrent ? 'auto' : 'metadata'}
{...stylex.props(styles.video)}
/>
Expand Down