From cd898dbdc7a8cca88a9d149dfed26e04bdbe6dfc Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 8 Nov 2026 09:17:25 -0800 Subject: [PATCH 1/2] fix(Lightbox): stop a video when the viewer closes A mounted Lightbox keeps its video element after the dialog closes. The playback effect paused only a gallery neighbour, so Close, Escape, or a tap outside left the sound running. Closing now pauses every video. Opening again plays the current one when hasAutoPlay is set, including a single video that has no page. The new close cases failed before the effect watched isOpen. vitest run packages/core/src/Lightbox/Lightbox.test.tsx --project ui passed 91 tests after the fix. --- .changeset/lightbox-pause-on-close.md | 7 ++ packages/core/src/Lightbox/Lightbox.test.tsx | 79 ++++++++++++++++++++ packages/core/src/Lightbox/Lightbox.tsx | 24 +++++- 3 files changed, 106 insertions(+), 4 deletions(-) create mode 100644 .changeset/lightbox-pause-on-close.md diff --git a/.changeset/lightbox-pause-on-close.md b/.changeset/lightbox-pause-on-close.md new file mode 100644 index 0000000000000..23ca54743ca70 --- /dev/null +++ b/.changeset/lightbox-pause-on-close.md @@ -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 diff --git a/packages/core/src/Lightbox/Lightbox.test.tsx b/packages/core/src/Lightbox/Lightbox.test.tsx index 937f28911e5e5..396ea5202e588 100644 --- a/packages/core/src/Lightbox/Lightbox.test.tsx +++ b/packages/core/src/Lightbox/Lightbox.test.tsx @@ -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) => ( + {}} + 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) => ( + {}} 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) => ( + {}} + 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}, diff --git a/packages/core/src/Lightbox/Lightbox.tsx b/packages/core/src/Lightbox/Lightbox.tsx index a219aa915f650..6af1dac500014 100644 --- a/packages/core/src/Lightbox/Lightbox.tsx +++ b/packages/core/src/Lightbox/Lightbox.tsx @@ -623,11 +623,24 @@ 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. useEffect(() => { const container = containerRef.current; if (container == null) { return; } + if (!isOpen) { + for (const video of container.querySelectorAll( + 'video', + )) { + if (!video.paused) { + video.pause(); + } + } + return; + } for (const video of container.querySelectorAll( '[data-lightbox-page]:not([data-current]) video', )) { @@ -635,14 +648,17 @@ export function Lightbox({ video.pause(); } } - const current = container.querySelector( - '[data-lightbox-page][data-current] video', - ); + const onAPage = container.querySelector('[data-lightbox-page]') != null; + const current = + container.querySelector( + '[data-lightbox-page][data-current] video', + ) ?? + (onAPage ? null : container.querySelector('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 From 43c7d2a5a4ec17e952250f18f85601ccb2fc220c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 8 Nov 2026 09:54:18 -0800 Subject: [PATCH 2/2] fix(Lightbox): do not autoplay a video while the viewer is closed The closed dialog still mounts the video. hasAutoPlay set the autoPlay attribute even when isOpen was false, so a viewer mounted closed started playback once the file loaded. The attribute now follows isOpen. The close comment no longer restates the play check. The new attribute test failed before this change because the closed viewer rendered autoplay. The playback tests passed after it. --- .changeset/lightbox-pause-on-close.md | 2 +- packages/core/src/Lightbox/Lightbox.test.tsx | 18 ++++++++++++++++++ packages/core/src/Lightbox/Lightbox.tsx | 7 +++---- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.changeset/lightbox-pause-on-close.md b/.changeset/lightbox-pause-on-close.md index 23ca54743ca70..c3f3f1866407d 100644 --- a/.changeset/lightbox-pause-on-close.md +++ b/.changeset/lightbox-pause-on-close.md @@ -2,6 +2,6 @@ '@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. +[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 diff --git a/packages/core/src/Lightbox/Lightbox.test.tsx b/packages/core/src/Lightbox/Lightbox.test.tsx index 396ea5202e588..5f217cf6f0c3c 100644 --- a/packages/core/src/Lightbox/Lightbox.test.tsx +++ b/packages/core/src/Lightbox/Lightbox.test.tsx @@ -677,6 +677,24 @@ describe('Lightbox', () => { } }); + 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) => ( + {}} + 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}, diff --git a/packages/core/src/Lightbox/Lightbox.tsx b/packages/core/src/Lightbox/Lightbox.tsx index 6af1dac500014..485e340dbc463 100644 --- a/packages/core/src/Lightbox/Lightbox.tsx +++ b/packages/core/src/Lightbox/Lightbox.tsx @@ -623,9 +623,8 @@ 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. + // 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) { @@ -1339,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)} />