Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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. Opening it again plays the current video when `hasAutoPlay` is set.

@SebTardif
79 changes: 79 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,85 @@ 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("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
24 changes: 20 additions & 4 deletions packages/core/src/Lightbox/Lightbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -623,26 +623,42 @@ 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.
// Closing keeps those elements mounted in the closed dialog, so a playing
// video stops then too. Opening again plays the current one when
// `hasAutoPlay` is set. A single video has no page element.
Comment thread
astracat-bot[bot] marked this conversation as resolved.
Resolved
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