Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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'));
Comment thread
astracat-bot[bot] marked this conversation as resolved.
Resolved
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
Loading