diff --git a/photomap/frontend/static/javascript/swiper.js b/photomap/frontend/static/javascript/swiper.js index 376896b3..3eb4207d 100644 --- a/photomap/frontend/static/javascript/swiper.js +++ b/photomap/frontend/static/javascript/swiper.js @@ -29,6 +29,15 @@ class SwiperManager { this.isAppending = false; this.isInternalSlideChange = false; + // Shuffle-mode "bag": every image is dealt once per cycle in random order, + // then the bag is refilled and reshuffled for a fresh order. This is what + // lets shuffle run indefinitely even on tiny albums — avoiding + // already-loaded slides would otherwise leave nothing to pick once all + // images are in the buffer, stalling the slideshow on the last slide. + this.shuffleBag = []; // iteration indices not yet dealt this cycle + this.shuffleBagPool = 0; // pool size the current bag was built for + this.lastShuffleIterIndex = null; // last index dealt, to avoid an immediate repeat across cycles + // Single-flight gate for resetAllSlides. albumChanged, searchResultsChanged, // and swiperModeChanged can all fire in quick succession (e.g. switching // album while a search is in flight). Without coordination, two concurrent @@ -83,6 +92,15 @@ class SwiperManager { delay: state.currentDelay * 1000, disableOnInteraction: true, enabled: false, + // Backstop for the "start the slideshow while already parked on the + // last slide" case, where no slideNextTransitionStart fires for us to + // intercept. Swiper's autoplay otherwise defaults this to false and, on + // reaching the end with loop off, calls slideTo(0) — jumping to the + // first slide of the in-memory buffer (a windowed subset, not the + // album's first image). The primary end-of-list handling lives in the + // slideNextTransitionStart handler below, which stops autoplay the + // instant resolveOffset(+1) reports there is no next slide. + stopOnLastSlide: true, }, loop: false, touchEventsTarget: "container", @@ -180,21 +198,45 @@ class SwiperManager { this.isAppending = true; this.swiper.allowSlideNext = false; + const finishAppend = () => { + this.isAppending = false; + this.swiper.allowSlideNext = true; + }; + + // Shuffle mode has no "end of list": the next slide is a random pick, + // not the one after the current index. addSlideByIndex(null, null) + // selects a random slide internally when the slideshow is running in + // random mode. We must NOT consult resolveOffset here — it reports null + // whenever the current random slide happens to be the last album index, + // which would otherwise stop the shuffle slideshow prematurely. + const isRandom = state.mode === "random" && slideShowRunning(); + if (isRandom) { + const finishRandomAppend = () => { + finishAppend(); + // Re-dealing images across shuffle cycles would grow the DOM without + // bound, so drop the oldest slides once we exceed the high-water + // mark. Deferred so it runs after the in-progress transition settles. + setTimeout(() => this.trimShuffleBacklog(), 500); + }; + this.addSlideByIndex(null, null).then(finishRandomAppend).catch(finishRandomAppend); + return; + } + const { globalIndex: nextGlobal, searchIndex: nextSearch } = slideState.resolveOffset(+1); if (nextGlobal !== null) { - this.addSlideByIndex(nextGlobal, nextSearch) - .then(() => { - this.isAppending = false; - this.swiper.allowSlideNext = true; - }) - .catch(() => { - this.isAppending = false; - this.swiper.allowSlideNext = true; - }); + this.addSlideByIndex(nextGlobal, nextSearch).then(finishAppend).catch(finishAppend); } else { - this.isAppending = false; - this.swiper.allowSlideNext = true; + finishAppend(); + // resolveOffset(+1) returned null: in linear mode we have just landed + // on the genuine last item with wrap navigation off (it only returns + // null at the end of the list — wrap mode always resolves to a real + // index). Nothing gets appended, so the active slide is now the last + // loaded one and Swiper considers itself at the end. Stop autoplay + // here so the slideshow rests on this final slide. If we left autoplay + // running, its next tick would see isEnd and call slideTo(0), snapping + // back to the first slide still held in the windowed buffer (~10 back). + this.pauseSlideshow(); } } }); @@ -349,38 +391,55 @@ class SwiperManager { } /** - * Select a random slide index that doesn't already exist in the swiper. - * Tries multiple random selections to avoid duplicates. + * Deal the next slide for shuffle mode from a reshuffling "bag". + * + * Each image is dealt exactly once per cycle in a random order; when the bag + * empties it is refilled and reshuffled, so every pass through the album uses + * a fresh order and the slideshow never runs out of slides to show. The + * index is an "iteration index": a search-results index in search mode, or a + * global album index otherwise — matching slideState.getCurrentIndex(). + * * @returns {{globalIndex: number|null, searchIndex: number|null}} The selected indices */ selectRandomSlideIndex() { - const totalPool = slideState.isSearchMode ? slideState.searchResults.length : slideState.totalAlbumImages; - - const existingIndices = new Set(Array.from(this.swiper.slides).map((el) => parseInt(el.dataset.globalIndex, 10))); - - // Try to find a random slide that doesn't already exist in the swiper - // Limit attempts to avoid infinite loop when all slides are already loaded - const MAX_RANDOM_ATTEMPTS = 50; - const maxAttempts = Math.min(totalPool, MAX_RANDOM_ATTEMPTS); + const pool = slideState.isSearchMode ? slideState.searchResults.length : slideState.totalAlbumImages; + if (!pool || pool <= 0) { + return { globalIndex: null, searchIndex: null }; + } - let globalIndex = null; - let searchIndex = null; + // Refill + reshuffle when the bag empties (new cycle) or the album/search + // pool changes underneath us (album switch, search results changed). + const poolChanged = this.shuffleBagPool !== pool; + if (this.shuffleBag.length === 0 || poolChanged) { + if (poolChanged) { + this.lastShuffleIterIndex = null; + } + this.shuffleBag = Array.from({ length: pool }, (_, i) => i); + this.shuffleBagPool = pool; - for (let attempt = 0; attempt < maxAttempts; attempt++) { - if (slideState.isSearchMode) { - searchIndex = Math.floor(Math.random() * totalPool); - globalIndex = slideState.searchToGlobal(searchIndex); - } else { - globalIndex = Math.floor(Math.random() * totalPool); - searchIndex = null; + // Fisher-Yates shuffle. + for (let i = this.shuffleBag.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)); + [this.shuffleBag[i], this.shuffleBag[j]] = [this.shuffleBag[j], this.shuffleBag[i]]; } - // Check for valid globalIndex and that it doesn't already exist - if (globalIndex !== null && !existingIndices.has(globalIndex)) { - break; + + // Avoid showing the same image twice in a row across a cycle boundary. The + // bag is dealt from the end (pop), so if the last entry repeats the image + // currently on screen, swap it to the front of the bag (dealt last). + const avoidIter = this.lastShuffleIterIndex !== null ? this.lastShuffleIterIndex : slideState.getCurrentIndex(); + const lastPos = this.shuffleBag.length - 1; + if (pool > 1 && this.shuffleBag[lastPos] === avoidIter) { + [this.shuffleBag[0], this.shuffleBag[lastPos]] = [this.shuffleBag[lastPos], this.shuffleBag[0]]; } } - return { globalIndex, searchIndex }; + const iterIndex = this.shuffleBag.pop(); + this.lastShuffleIterIndex = iterIndex; + + if (slideState.isSearchMode) { + return { globalIndex: slideState.searchToGlobal(iterIndex), searchIndex: iterIndex }; + } + return { globalIndex: iterIndex, searchIndex: null }; } async addSlideByIndex(globalIndex, searchIndex = null, prepend = false, random = null) { @@ -395,14 +454,13 @@ class SwiperManager { const selected = this.selectRandomSlideIndex(); globalIndex = selected.globalIndex; searchIndex = selected.searchIndex; - // Shuffle mode must not show duplicates: if selectRandomSlideIndex - // exhausted its attempts in a small pool, the index might already be loaded. - // Linear navigation deliberately allows duplicates so wrap-around can - // re-append a globalIndex that appeared earlier in the swiper. - const exists = Array.from(this.swiper.slides).some((el) => parseInt(el.dataset.globalIndex, 10) === globalIndex); - if (exists) { + if (globalIndex === null) { return; } + // No buffer-duplicate guard here: the shuffle bag already guarantees each + // image is dealt once per cycle, and re-dealing an image on a later cycle + // (so it can be shown again) is the whole point. An image already in the + // windowed buffer from an earlier cycle is therefore an intentional repeat. } let currentScore, currentCluster, currentColor; @@ -611,6 +669,23 @@ class SwiperManager { } } + /** + * Keep the shuffle buffer bounded. The active slide and its single look-ahead + * live at the tail, so dropping the oldest (front) slides never disturbs what + * is on screen or about to be shown. Unlike enforceHighWaterMark this does not + * stop/start autoplay, so it won't flicker the play/pause icon on every + * advance — important because shuffle trims on (nearly) every slide. + */ + trimShuffleBacklog() { + if (!this.swiper) { + return; + } + const maxSlides = state.highWaterMark || 50; + while (this.swiper.slides.length > maxSlides) { + this.swiper.removeSlide(0); + } + } + enforceHighWaterMark(backward = false) { const maxSlides = state.highWaterMark || 50; const swiper = this.swiper; diff --git a/tests/frontend/swiper-shuffle.test.js b/tests/frontend/swiper-shuffle.test.js index 39a4bd2f..fa2dfec4 100644 --- a/tests/frontend/swiper-shuffle.test.js +++ b/tests/frontend/swiper-shuffle.test.js @@ -95,6 +95,9 @@ const mockSlideState = { totalCount: mockSlideState.totalAlbumImages, isSearchMode: false, })), + getCurrentIndex: jest.fn(() => + mockSlideState.isSearchMode ? mockSlideState.currentSearchIndex : mockSlideState.currentGlobalIndex + ), searchToGlobal: jest.fn((idx) => mockSlideState.searchResults[idx]?.index ?? null), }; @@ -170,74 +173,52 @@ describe("swiper.js shuffle mode", () => { }); describe("random slide selection", () => { - it("should try multiple random indices until finding a unique one", async () => { - // This test verifies that when random slides are selected, the algorithm - // tries multiple times if the first random selection is already in the swiper + it("deals every image once per cycle, then reshuffles for a fresh order", async () => { + // The shuffle bag is a deck: across one cycle of `pool` deals every image + // index appears exactly once; the next cycle is a fresh permutation. + mockSlideState.totalAlbumImages = 5; - // Create mock slides that already exist in swiper (indices 0, 1, 2) - const existingSlides = [createMockSlide(0), createMockSlide(1), createMockSlide(2)]; - mockSwiper.slides = existingSlides; - - // Track which indices are requested via Math.random - // The algorithm uses Math.floor(Math.random() * totalPool) where totalPool = 10 - // We return values that map to existing indices first, then a non-existing one - let callCount = 0; - const originalRandom = Math.random; - Math.random = jest.fn(() => { - callCount++; - // Return 0.05 (maps to 0), 0.15 (maps to 1), 0.25 (maps to 2), then 0.55 (maps to 5) - // These values ensure: floor(0.05*10)=0, floor(0.15*10)=1, floor(0.25*10)=2, floor(0.55*10)=5 - if (callCount <= 3) { - return (callCount - 1) * 0.1 + 0.05; - } - return 0.55; - }); - - try { - // Import the module (needs to be done after mocks are set up) - const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); - - const manager = await initializeSingleSwiper(); - - // Simulate adding a slide in random mode - await manager.addSlideByIndex(0, null, false, true); - - // Should have called Math.random multiple times to find a unique index - expect(Math.random).toHaveBeenCalled(); - expect(callCount).toBeGreaterThan(1); - - // The slide that was added should NOT be one of the existing indices (0, 1, 2) - // Check that fetchImageByIndex was called with a non-existing index - const lastCallArg = mockFetchImageByIndex.mock.calls[mockFetchImageByIndex.mock.calls.length - 1]?.[0]; - expect([3, 4, 5, 6, 7, 8, 9]).toContain(lastCallArg); - } finally { - Math.random = originalRandom; + const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); + const manager = await initializeSingleSwiper(); + // Force a clean bag (the SwiperManager singleton persists across tests). + manager.shuffleBag = []; + manager.shuffleBagPool = 0; + manager.lastShuffleIterIndex = null; + + const cycle1 = []; + for (let i = 0; i < 5; i++) { + cycle1.push(manager.selectRandomSlideIndex().globalIndex); + } + const cycle2 = []; + for (let i = 0; i < 5; i++) { + cycle2.push(manager.selectRandomSlideIndex().globalIndex); } + + // Each cycle is a permutation of every album index — nothing missed, nothing repeated. + expect([...cycle1].sort((a, b) => a - b)).toEqual([0, 1, 2, 3, 4]); + expect([...cycle2].sort((a, b) => a - b)).toEqual([0, 1, 2, 3, 4]); + // No immediate repeat across the cycle boundary. + expect(cycle2[0]).not.toBe(cycle1[cycle1.length - 1]); }); - it("should not get stuck in infinite loop when all slides exist", async () => { - // With only 3 total images and all 3 already loaded, - // the algorithm should give up after max attempts + it("appends a fresh slide even when every image is already loaded (no stall)", async () => { + // Regression: a small album whose every image is already in the buffer must + // still advance — the bag re-deals images rather than refusing to append. mockSlideState.totalAlbumImages = 3; const existingSlides = [createMockSlide(0), createMockSlide(1), createMockSlide(2)]; mockSwiper.slides = existingSlides; const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); - const manager = await initializeSingleSwiper(); + manager.shuffleBag = []; + manager.shuffleBagPool = 0; + manager.lastShuffleIterIndex = null; - // This should not hang - it should return early after max attempts - const startTime = Date.now(); await manager.addSlideByIndex(0, null, false, true); - const elapsed = Date.now() - startTime; - - // Should complete quickly (within 1 second), not hang - expect(elapsed).toBeLessThan(1000); - // Since all slides already exist, no new slide should be added - // (fetchImageByIndex might be called but appendSlide won't add a duplicate) - expect(mockSwiper.slides.length).toBe(3); + // A slide was appended despite all three images already being present. + expect(mockSwiper.slides.length).toBe(4); }); it("should handle search mode with small result sets", async () => { @@ -264,6 +245,112 @@ describe("swiper.js shuffle mode", () => { expect(mockSlideState.searchToGlobal).toHaveBeenCalled(); }); }); + + describe("autoplay end-of-list behavior", () => { + // Regression tests for the linear-slideshow bug where reaching the last + // slide jumped back ~10 slides instead of stopping. Swiper's autoplay, + // on reaching the end with loop off, calls slideTo(0) — the first slide in + // the windowed buffer, not the album start. The primary defense is that our + // slideNextTransitionStart handler stops autoplay the moment resolveOffset + // reports no next slide; stopOnLastSlide is a config-level backstop. + it("configures Swiper autoplay with stopOnLastSlide enabled and loop disabled", async () => { + const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); + await initializeSingleSwiper(); + + // new Swiper(selector, config) — grab the config it was constructed with. + const swiperConfig = global.Swiper.mock.calls[0][1]; + expect(swiperConfig.loop).toBe(false); + expect(swiperConfig.autoplay.stopOnLastSlide).toBe(true); + }); + + it("stops autoplay and appends nothing at the genuine last image (wrap off)", async () => { + // resolveOffset(+1) returning null is how slide-state signals "no next + // slide" at the end with wrap off. The handler must then leave the buffer + // untouched AND stop autoplay so Swiper's next tick can't slideTo(0). + mockState.mode = "chronological"; // linear, not shuffle + mockSlideState.resolveOffset = jest.fn(() => ({ globalIndex: null, searchIndex: null })); + + // Capture the slideNextTransitionStart handler registered on the swiper. + const handlers = {}; + mockSwiper.on = jest.fn((event, cb) => { + handlers[event] = cb; + }); + + const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); + const manager = await initializeSingleSwiper(); + + // Sit on the last loaded slide with autoplay running. + mockSwiper.slides = [createMockSlide(8), createMockSlide(9)]; + mockSwiper.activeIndex = mockSwiper.slides.length - 1; + mockSwiper.autoplay.running = true; + const slidesBefore = mockSwiper.slides.length; + + await handlers.slideNextTransitionStart.call(manager); + + // No slide appended past the end, forward navigation re-enabled, and + // autoplay halted so the slideshow rests on the final slide. + expect(mockSwiper.slides.length).toBe(slidesBefore); + expect(mockSwiper.allowSlideNext).toBe(true); + expect(mockSwiper.autoplay.stop).toHaveBeenCalled(); + }); + + it("keeps autoplay running and appends the wrapped slide at the end (wrap on)", async () => { + // With wrap on, resolveOffset(+1) returns a real index (the first image), + // so the handler appends it ahead and must NOT stop autoplay. + mockState.mode = "chronological"; // linear, not shuffle + mockSlideState.resolveOffset = jest.fn(() => ({ globalIndex: 0, searchIndex: null })); + + const handlers = {}; + mockSwiper.on = jest.fn((event, cb) => { + handlers[event] = cb; + }); + + const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); + const manager = await initializeSingleSwiper(); + + mockSwiper.slides = [createMockSlide(8), createMockSlide(9)]; + mockSwiper.activeIndex = mockSwiper.slides.length - 1; + mockSwiper.autoplay.running = true; + + handlers.slideNextTransitionStart.call(manager); + // The append is async (fetchImageByIndex); let its promise chain settle. + await new Promise((resolve) => setTimeout(resolve, 0)); + + // Wrapped slide appended ahead; autoplay left running to advance into it. + expect(mockSwiper.slides.length).toBe(3); + expect(mockSwiper.autoplay.stop).not.toHaveBeenCalled(); + }); + + it("keeps shuffling and never stops at the last index in random mode", async () => { + // In shuffle mode there is no end of list. resolveOffset(+1) returns null + // whenever the current random slide is the last album index, but that must + // NOT stop the slideshow — the handler should append another random slide. + mockState.mode = "random"; + mockSlideShowRunning.mockReturnValue(true); + mockSlideState.resolveOffset = jest.fn(() => ({ globalIndex: null, searchIndex: null })); + + const handlers = {}; + mockSwiper.on = jest.fn((event, cb) => { + handlers[event] = cb; + }); + + const { initializeSingleSwiper } = await import("../../photomap/frontend/static/javascript/swiper.js"); + const manager = await initializeSingleSwiper(); + + // Parked on the last loaded slide, which is the last album index (9). + mockSwiper.slides = [createMockSlide(8), createMockSlide(9)]; + mockSwiper.activeIndex = mockSwiper.slides.length - 1; + mockSwiper.autoplay.running = true; + const slidesBefore = mockSwiper.slides.length; + + handlers.slideNextTransitionStart.call(manager); + await new Promise((resolve) => setTimeout(resolve, 0)); + + // A random slide was appended and autoplay kept running — no premature stop. + expect(mockSwiper.slides.length).toBe(slidesBefore + 1); + expect(mockSwiper.autoplay.stop).not.toHaveBeenCalled(); + }); + }); }); // Helper function to create mock slide elements