From f64115871bc318a224cc9b63708444bb1619ee2d Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Thu, 4 Jun 2026 19:43:11 -0400 Subject: [PATCH] fix(slideshow): rebuild buffer sequentially when pausing a shuffle run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After a shuffle slideshow stops, the swiper buffer is still in random order, so the forward/back buttons walked that leftover shuffle instead of the current image's real sequential neighbors. pauseSlideshow() only halts autoplay and never rebuilds the buffer, so the stale shuffled slides surrounding the current one drove manual navigation. Rebuild the buffer in album order around the current slide when a shuffle run is paused via the play/pause button (or spacebar). Sequential runs already leave an in-order buffer, so only shuffle needs this. Note: navigating *during* a running shuffle slideshow still walks the shuffle order — that is intended. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../frontend/static/javascript/slideshow.js | 11 +++++ tests/frontend/slideshow.test.js | 46 ++++++++++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/photomap/frontend/static/javascript/slideshow.js b/photomap/frontend/static/javascript/slideshow.js index 11ba50a4..716db373 100644 --- a/photomap/frontend/static/javascript/slideshow.js +++ b/photomap/frontend/static/javascript/slideshow.js @@ -94,8 +94,19 @@ export async function toggleSlideshowWithIndicator(e) { if (slideShowRunning()) { // pause + const wasShuffling = state.mode === "random"; try { state.single_swiper.pauseSlideshow(); + // A shuffle run fills the swiper buffer with slides in random order. Once + // stopped, the forward/back buttons just walk that buffer, so the user + // would see the shuffled neighborhood instead of the current image's real + // sequential neighbors. Rebuild the buffer in album order around the + // current slide so manual navigation behaves sequentially again. + // (Sequential runs already leave an in-order buffer, so only shuffle needs + // this.) + if (wasShuffling) { + await state.single_swiper.resetAllSlides(); + } } catch (err) { console.warn("pauseSlideshow failed:", err); } diff --git a/tests/frontend/slideshow.test.js b/tests/frontend/slideshow.test.js index 38215c06..2632e2b8 100644 --- a/tests/frontend/slideshow.test.js +++ b/tests/frontend/slideshow.test.js @@ -38,8 +38,13 @@ jest.unstable_mockModule("../../photomap/frontend/static/javascript/umap.js", () })); // Now import the module we want to test -const { slideShowRunning, updateSlideshowButtonIcon, showPlayPauseIndicator, removeExistingIndicator } = - await import("../../photomap/frontend/static/javascript/slideshow.js"); +const { + slideShowRunning, + updateSlideshowButtonIcon, + showPlayPauseIndicator, + removeExistingIndicator, + toggleSlideshowWithIndicator, +} = await import("../../photomap/frontend/static/javascript/slideshow.js"); const { state } = await import("../../photomap/frontend/static/javascript/state.js"); @@ -165,6 +170,43 @@ describe("slideshow.js", () => { }); }); + describe("toggleSlideshowWithIndicator (pause path)", () => { + beforeEach(() => { + document.body.innerHTML = ` +
+ + `; + }); + + it("rebuilds the buffer sequentially when pausing a shuffle run", async () => { + // Regression: after stopping a shuffle slideshow, the swiper buffer is + // still in random order, so forward/back navigation would walk the + // leftover shuffle instead of the current image's sequential neighbors. + const resetAllSlides = jest.fn(() => Promise.resolve()); + const pauseSlideshow = jest.fn(); + state.single_swiper = { swiper: { autoplay: { running: true } }, pauseSlideshow, resetAllSlides }; + state.mode = "random"; + + await toggleSlideshowWithIndicator(); + + expect(pauseSlideshow).toHaveBeenCalled(); + expect(resetAllSlides).toHaveBeenCalled(); + }); + + it("does not rebuild the buffer when pausing a sequential run", async () => { + // Sequential runs already leave an in-order buffer, so no rebuild needed. + const resetAllSlides = jest.fn(() => Promise.resolve()); + const pauseSlideshow = jest.fn(); + state.single_swiper = { swiper: { autoplay: { running: true } }, pauseSlideshow, resetAllSlides }; + state.mode = "chronological"; + + await toggleSlideshowWithIndicator(); + + expect(pauseSlideshow).toHaveBeenCalled(); + expect(resetAllSlides).not.toHaveBeenCalled(); + }); + }); + describe("showPlayPauseIndicator", () => { it("should create indicator element when showing play", () => { state.mode = "chronological";