From 0f9ddbca97175c7d3ddb9717d8fcea2931a05c24 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 8 Oct 2026 16:55:33 -0400 Subject: [PATCH] feat(web): every paged list loads as you scroll; Liked becomes tabs (#5416) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preference 172: no "Load more" buttons. Eight remained, on Liked (three sections), the three Search overflow pages, Genres and Years. - ListContinuation: the one bottom-of-list block. It loads the next page ahead of the reader, announces "Loading more…" through aria-live, shows an optional end line, and on a failed page shows the error with Try again, dropping the sentinel so a dead endpoint isn't re-hit on every scroll. Used on all ten paged lists, including the four that already autoloaded. - A failed next page no longer replaces the list. TanStack sets isError for it, so the page-level error branches now apply only to a failed first load. Genres and Years did the same with their own loader; a later page's failure now keeps the grid. - Liked: Artists | Albums | Tracks tabs (operator's choice). Stacked, a long Artists list loading as you scroll would bury the other two. It opens on the first tab that has likes. - TabStrip: the in-page tab strip, now shared by Liked, Playback errors and Requests. - test-utils/intersectionObserver: a stand-in so tests can scroll to the end. Co-Authored-By: Claude Opus 5.5 --- .../lib/components/ListContinuation.svelte | 49 +++++ .../lib/components/ListContinuation.test.ts | 43 +++++ web/src/lib/components/TabStrip.svelte | 47 +++++ web/src/lib/components/TabStrip.test.ts | 25 +++ .../routes/admin/playback-errors/+page.svelte | 29 +-- web/src/routes/admin/requests/+page.svelte | 38 +--- .../routes/admin/suspect-sources/+page.svelte | 19 +- web/src/routes/library/albums/+page.svelte | 24 +-- web/src/routes/library/artists/+page.svelte | 24 +-- web/src/routes/library/genres/+page.svelte | 34 ++-- web/src/routes/library/genres/genres.test.ts | 42 ++++- web/src/routes/library/history/+page.svelte | 22 +-- web/src/routes/library/liked/+page.svelte | 174 +++++++++--------- web/src/routes/library/liked/liked.test.ts | 137 ++++++++------ web/src/routes/library/years/+page.svelte | 36 ++-- web/src/routes/library/years/years.test.ts | 41 ++++- web/src/routes/search/albums/+page.svelte | 21 +-- web/src/routes/search/albums/albums.test.ts | 14 +- web/src/routes/search/artists/+page.svelte | 21 +-- web/src/routes/search/artists/artists.test.ts | 20 +- web/src/routes/search/tracks/+page.svelte | 21 +-- web/src/routes/search/tracks/tracks.test.ts | 14 +- web/src/test-utils/intersectionObserver.ts | 40 ++++ web/src/test-utils/query.ts | 2 + 24 files changed, 593 insertions(+), 344 deletions(-) create mode 100644 web/src/lib/components/ListContinuation.svelte create mode 100644 web/src/lib/components/ListContinuation.test.ts create mode 100644 web/src/lib/components/TabStrip.svelte create mode 100644 web/src/lib/components/TabStrip.test.ts create mode 100644 web/src/test-utils/intersectionObserver.ts diff --git a/web/src/lib/components/ListContinuation.svelte b/web/src/lib/components/ListContinuation.svelte new file mode 100644 index 00000000..9c0b6d4c --- /dev/null +++ b/web/src/lib/components/ListContinuation.svelte @@ -0,0 +1,49 @@ + + +{#if failed} + +{:else if hasMore} + +{/if} + +

+ {#if !failed && hasMore && loading}Loading more…{:else if !hasMore && endLabel}{endLabel}{/if} +

diff --git a/web/src/lib/components/ListContinuation.test.ts b/web/src/lib/components/ListContinuation.test.ts new file mode 100644 index 00000000..45d2b9d4 --- /dev/null +++ b/web/src/lib/components/ListContinuation.test.ts @@ -0,0 +1,43 @@ +import { describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/svelte'; +import ListContinuation from './ListContinuation.svelte'; + +// InfiniteScrollSentinel renders diff --git a/web/src/routes/library/artists/+page.svelte b/web/src/routes/library/artists/+page.svelte index 0d88e61b..e38808d1 100644 --- a/web/src/routes/library/artists/+page.svelte +++ b/web/src/routes/library/artists/+page.svelte @@ -4,7 +4,7 @@ import ArtistCard from '#lib/components/ArtistCard.svelte'; import AlphabeticalGrid from '#lib/components/AlphabeticalGrid.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; - import InfiniteScrollSentinel from '#lib/components/InfiniteScrollSentinel.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import QuickFilter from '#lib/components/QuickFilter.svelte'; import EmptyState from '#lib/components/EmptyState.svelte'; import { useDelayed } from '#lib/utils/useDelayed.svelte.js'; @@ -30,7 +30,7 @@

Artists

- {#if !query.isPending && !query.isError} + {#if !query.isPending && !(query.isError && !query.isFetchNextPageError)}

{total} {total === 1 ? 'artist' : 'artists'}

@@ -41,7 +41,7 @@ {/if}
- {#if query.isError} + {#if query.isError && !query.isFetchNextPageError} {:else if showSkeleton.value && artists.length === 0}

Loading…

@@ -79,16 +79,12 @@ {/snippet} - {#if query.hasNextPage} - query.fetchNextPage()} - /> - {#if query.isFetchingNextPage} -

Loading more…

- {/if} - {:else if artists.length > 0} -

End of library

- {/if} + query.fetchNextPage()} + endLabel="End of library" + /> {/if} diff --git a/web/src/routes/library/genres/+page.svelte b/web/src/routes/library/genres/+page.svelte index 3a4ccfdf..e8adffb1 100644 --- a/web/src/routes/library/genres/+page.svelte +++ b/web/src/routes/library/genres/+page.svelte @@ -11,6 +11,7 @@ import AlbumCard from '#lib/components/AlbumCard.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; import EmptyState from '#lib/components/EmptyState.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import QuickFilter from '#lib/components/QuickFilter.svelte'; import type { AlbumRef } from '#lib/api/types.js'; @@ -59,6 +60,9 @@ let total = $state(0); let loading = $state(false); let failed = $state(false); + // A later page failed: the albums already shown stay, and the list offers a + // retry at its end instead of loading again on every scroll. + let moreFailed = $state(false); // Plain `let`, deliberately not $state: it's read inside the fetch path, and // as reactive state that read would make this effect depend on its own @@ -76,6 +80,7 @@ albums = []; total = 0; failed = false; + moreFailed = false; if (!genre) return; await fetchPage(genre, 0, requestToken); } @@ -88,13 +93,18 @@ albums = offset === 0 ? p.items : [...albums, ...p.items]; total = p.total; } catch { - if (token === requestToken) failed = true; + if (token === requestToken) { + if (offset === 0) failed = true; + else moreFailed = true; + } } finally { if (token === requestToken) loading = false; } } function loadMore() { + if (loading) return; + moreFailed = false; void fetchPage(selected, albums.length, requestToken); } @@ -154,21 +164,13 @@ {/each} - {#if albums.length < total} -
- -
- {:else} -

End of genre

- {/if} + {/if} {:else} diff --git a/web/src/routes/library/genres/genres.test.ts b/web/src/routes/library/genres/genres.test.ts index f24be753..5e3b81ac 100644 --- a/web/src/routes/library/genres/genres.test.ts +++ b/web/src/routes/library/genres/genres.test.ts @@ -1,6 +1,7 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { render, screen, waitFor, fireEvent } from '@testing-library/svelte'; import { mockQuery } from '#test-utils/query.js'; +import { installIntersectionObserverMock } from '#test-utils/intersectionObserver.js'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; import { apiClientMock } from '#test-utils/mocks/client.js'; import { emptyLikesMock } from '#test-utils/mocks/likes.js'; @@ -33,6 +34,11 @@ import { createGenresQuery, listAlbumsByGenre } from '#lib/api/browse.js'; const asMock = (fn: unknown) => fn as ReturnType; +let io: ReturnType; +beforeEach(() => { + io = installIntersectionObserverMock(); +}); + function album(id: string, title: string): AlbumRef { return { id, @@ -187,7 +193,7 @@ describe('/library/genres drill-down', () => { expect(screen.getByRole('heading', { name: 'Rock/Pop' })).toBeInTheDocument(); }); - test('load more appends the next page and then reports the end', async () => { + test('scrolling to the end loads the next page, then reports the end', async () => { pageState.pageUrl = new URL('http://localhost/library/genres?g=Rock'); asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); asMock(listAlbumsByGenre) @@ -201,10 +207,13 @@ describe('/library/genres drill-down', () => { render(GenresPage); await screen.findByText('One'); - const more = await screen.findByRole('button', { name: /Load more \(1 left\)/ }); - await fireEvent.click(more); + // No button (#5416): reaching the bottom of the grid loads the next page. + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + await waitFor(() => { + io.scrollToEnd(); + expect(listAlbumsByGenre).toHaveBeenLastCalledWith('Rock', 2, 2); + }); - await waitFor(() => expect(listAlbumsByGenre).toHaveBeenLastCalledWith('Rock', 2, 2)); expect(await screen.findByText('Three')).toBeInTheDocument(); // Earlier pages are appended, not replaced. expect(screen.getByText('One')).toBeInTheDocument(); @@ -219,4 +228,27 @@ describe('/library/genres drill-down', () => { expect(await screen.findByText(/Couldn't load albums for this genre/i)).toBeInTheDocument(); }); + + // A failed later page keeps what is shown and offers a retry at the end, + // rather than replacing the grid or re-requesting on every scroll. + test('a failed later page keeps the albums and offers Try again', async () => { + pageState.pageUrl = new URL('http://localhost/library/genres?g=Rock'); + asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByGenre) + .mockResolvedValueOnce({ items: [album('a1', 'One'), album('a2', 'Two')], total: 3, limit: 2, offset: 0 }) + .mockRejectedValueOnce(new Error('nope')) + .mockResolvedValueOnce({ items: [album('a3', 'Three')], total: 3, limit: 2, offset: 2 }); + render(GenresPage); + + await screen.findByText('One'); + await waitFor(() => { + io.scrollToEnd(); + expect(screen.getByRole('alert')).toHaveTextContent("Couldn't load more."); + }); + expect(screen.getByText('One')).toBeInTheDocument(); + expect(screen.queryByText(/Couldn't load albums for this genre/i)).not.toBeInTheDocument(); + + await fireEvent.click(screen.getByRole('button', { name: 'Try again' })); + expect(await screen.findByText('Three')).toBeInTheDocument(); + }); }); diff --git a/web/src/routes/library/history/+page.svelte b/web/src/routes/library/history/+page.svelte index 44893c19..be509f10 100644 --- a/web/src/routes/library/history/+page.svelte +++ b/web/src/routes/library/history/+page.svelte @@ -5,7 +5,7 @@ import { groupByDay } from '#lib/utils/dayGroup.js'; import HistoryRow from '#lib/components/HistoryRow.svelte'; import TrackList from '#lib/components/TrackList.svelte'; - import InfiniteScrollSentinel from '#lib/components/InfiniteScrollSentinel.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import LibrarySkeleton from '#lib/components/LibrarySkeleton.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; import QuickFilter from '#lib/components/QuickFilter.svelte'; @@ -40,7 +40,7 @@ {#if query.isPending} - {:else if query.isError} + {:else if query.isError && !query.isFetchNextPageError} {:else if flatEvents.length === 0} {/each} - {#if query.hasNextPage} - query.fetchNextPage()} - /> - {#if query.isFetchingNextPage} -

Loading more…

- {/if} - {:else if flatEvents.length > 0} -

End of history

- {/if} + query.fetchNextPage()} + endLabel="End of history" + /> {/if} diff --git a/web/src/routes/library/liked/+page.svelte b/web/src/routes/library/liked/+page.svelte index 29a67cd1..f71cee22 100644 --- a/web/src/routes/library/liked/+page.svelte +++ b/web/src/routes/library/liked/+page.svelte @@ -12,6 +12,8 @@ import TrackList from '#lib/components/TrackList.svelte'; import QuickFilter from '#lib/components/QuickFilter.svelte'; import EmptyState from '#lib/components/EmptyState.svelte'; + import TabStrip from '#lib/components/TabStrip.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; const artistsStore = $derived(createLikedArtistsInfiniteQuery()); const albumsStore = $derived(createLikedAlbumsInfiniteQuery()); @@ -29,6 +31,14 @@ const albumsTotal = $derived(albumsQuery?.data?.pages?.[0]?.total ?? 0); const tracksTotal = $derived(tracksQuery?.data?.pages?.[0]?.total ?? 0); + type Tab = 'artists' | 'albums' | 'tracks'; + // Opens on the first tab with anything in it; once the reader picks a tab, + // that choice sticks. + let chosenTab = $state(null); + const tab = $derived( + chosenTab ?? (artistsTotal > 0 ? 'artists' : albumsTotal > 0 ? 'albums' : tracksTotal > 0 ? 'tracks' : 'artists') + ); + let filter = $state(''); const q = $derived(filter.trim().toLowerCase()); const fArtists = $derived(q ? artists.filter((a) => a.name.toLowerCase().includes(q)) : artists); @@ -41,8 +51,10 @@ t.artist_name.toLowerCase().includes(q) || (t.album_title ?? '').toLowerCase().includes(q)) : tracks); + // The filter applies to the open tab's loaded rows. const filterMatchesNothing = $derived( - !!q && fArtists.length === 0 && fAlbums.length === 0 && fTracks.length === 0 + !!q && + (tab === 'artists' ? fArtists : tab === 'albums' ? fAlbums : fTracks).length === 0 ); const allEmpty = $derived( !q && artistsTotal === 0 && albumsTotal === 0 && tracksTotal === 0 @@ -82,99 +94,79 @@ {/if} - {#if !allEmpty && (!q || fArtists.length > 0)} -
-

Artists

- {#if artistsTotal === 0} - - {#snippet actions()} - Browse artists → - {/snippet} - - {:else} -
- {#each fArtists as a (a.id)} - - {/each} -
- {#if artistsQuery?.hasNextPage} - - {/if} - {/if} -
- {/if} + {#if !allEmpty} + + tab, (v) => (chosenTab = v)} + ariaLabel="Liked" + items={[ + { id: 'artists', label: 'Artists', count: artistsTotal }, + { id: 'albums', label: 'Albums', count: albumsTotal }, + { id: 'tracks', label: 'Tracks', count: tracksTotal } + ]} + /> - {#if !allEmpty && (!q || fAlbums.length > 0)} -
-

Albums

- {#if albumsTotal === 0} - - {#snippet actions()} - Browse albums → - {/snippet} - + {#if tab === 'artists'} + {#if artistsTotal === 0} + + {#snippet actions()} + Browse artists → + {/snippet} + + {:else} +
+ {#each fArtists as a (a.id)} + + {/each} +
+ artistsQuery.fetchNextPage()} + /> + {/if} + {:else if tab === 'albums'} + {#if albumsTotal === 0} + + {#snippet actions()} + Browse albums → + {/snippet} + + {:else} +
+ {#each fAlbums as al (al.id)} + + {/each} +
+ albumsQuery.fetchNextPage()} + /> + {/if} {:else} -
- {#each fAlbums as al (al.id)} - - {/each} -
- {#if albumsQuery?.hasNextPage} - + {#if tracksTotal === 0} + + {#snippet actions()} + Find something new → + {/snippet} + + {:else} + + {#each fTracks as t, i (t.id)} + + {/each} + + tracksQuery.fetchNextPage()} + /> {/if} {/if} -
- {/if} - - {#if !allEmpty && (!q || fTracks.length > 0)} -
-

Tracks

- {#if tracksTotal === 0} - - {#snippet actions()} - Find something new → - {/snippet} - - {:else} - - {#each fTracks as t, i (t.id)} - - {/each} - - {#if tracksQuery?.hasNextPage} - - {/if} - {/if} -
{/if} diff --git a/web/src/routes/library/liked/liked.test.ts b/web/src/routes/library/liked/liked.test.ts index 19616208..242f8849 100644 --- a/web/src/routes/library/liked/liked.test.ts +++ b/web/src/routes/library/liked/liked.test.ts @@ -38,82 +38,99 @@ function page(items: T[], total: number, offset = 0, limit = 50): Page { afterEach(() => vi.clearAllMocks()); +const ar: ArtistRef = { id: 'a1', name: 'Miles', sort_name: 'Miles', album_count: 1, cover_url: '' }; +const al: AlbumRef = { + id: 'al1', title: 'Kind of Blue', sort_title: 'Kind of Blue', artist_id: 'a1', artist_name: 'Miles', + year: 1959, track_count: 1, duration_sec: 100, cover_url: '/x', cover_art_source: null +}; +const tr: TrackRef = makeTrack({ + title: 'So What', album_id: 'al1', album_title: 'Kind of Blue', + artist_id: 'a1', artist_name: 'Miles', duration_sec: 100 +}); + +function setup(opts: { + artists?: Page; + albums?: Page; + tracks?: Page; + tracksQuery?: Parameters[0]; +}) { + (createLikedArtistsInfiniteQuery as ReturnType).mockReturnValue( + mockInfiniteQuery({ pages: [opts.artists ?? page([], 0)] }) + ); + (createLikedAlbumsInfiniteQuery as ReturnType).mockReturnValue( + mockInfiniteQuery({ pages: [opts.albums ?? page([], 0)] }) + ); + (createLikedTracksInfiniteQuery as ReturnType).mockReturnValue( + mockInfiniteQuery({ pages: [opts.tracks ?? page([], 0)], ...opts.tracksQuery }) + ); +} + describe('liked library page', () => { - test('renders three sections when each has items', () => { - const ar: ArtistRef = { id: 'a1', name: 'X', sort_name: 'X', album_count: 1, cover_url: '' }; - const al: AlbumRef = { - id: 'al1', title: 'Y', sort_title: 'Y', artist_id: 'a1', artist_name: 'X', - year: 2020, track_count: 1, duration_sec: 100, cover_url: '/x', cover_art_source: null - }; - const tr: TrackRef = makeTrack({ - title: 'Z', album_id: 'al1', album_title: 'Y', - artist_id: 'a1', artist_name: 'X', duration_sec: 100 - }); - (createLikedArtistsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([ar], 1)] }) - ); - (createLikedAlbumsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([al], 1)] }) - ); - (createLikedTracksInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([tr], 1)] }) - ); + // #5416: one list at a time. Stacked, a long Artists list that loads as you + // scroll would bury Albums and Tracks. + test('tabs carry each count and show one list at a time', async () => { + setup({ artists: page([ar], 1), albums: page([al], 1), tracks: page([tr], 1) }); render(LikedPage); - expect(screen.getByRole('heading', { name: /artists/i })).toBeInTheDocument(); - expect(screen.getByRole('heading', { name: /albums/i })).toBeInTheDocument(); - expect(screen.getByRole('heading', { name: /tracks/i })).toBeInTheDocument(); + + const tabs = screen.getAllByRole('tab'); + expect(tabs.map((t) => t.textContent?.replace(/\s+/g, ' ').trim())).toEqual([ + 'Artists 1', + 'Albums 1', + 'Tracks 1' + ]); + expect(screen.getByText('Miles')).toBeInTheDocument(); + expect(screen.queryByText('So What')).not.toBeInTheDocument(); + + await fireEvent.click(screen.getByRole('tab', { name: /albums/i })); + expect(screen.getByText('Kind of Blue')).toBeInTheDocument(); + + await fireEvent.click(screen.getByRole('tab', { name: /tracks/i })); + expect(screen.getByText('So What')).toBeInTheDocument(); + expect(screen.getByRole('tab', { name: /tracks/i })).toHaveAttribute('aria-selected', 'true'); }); - test('all three sections empty renders the whole-tab onboarding card', () => { - (createLikedArtistsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - (createLikedAlbumsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - (createLikedTracksInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); + test('opens on the first tab that has likes', () => { + setup({ tracks: page([tr], 1) }); + render(LikedPage); + expect(screen.getByRole('tab', { name: /tracks/i })).toHaveAttribute('aria-selected', 'true'); + expect(screen.getByText('So What')).toBeInTheDocument(); + }); + + test('all three empty renders the whole-page onboarding card and no tabs', () => { + setup({}); render(LikedPage); - // The whole-tab EmptyState replaces the three per-section hints - // when every section is empty — single heading + action affordances. expect(screen.getByRole('heading', { name: /no likes yet/i })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /explore home/i })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /browse albums/i })).toBeInTheDocument(); + expect(screen.queryByRole('tablist')).not.toBeInTheDocument(); }); - test('one populated section + two empty subsections renders inline hints', () => { - (createLikedArtistsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([{ id: 'a1', name: 'Miles' } as ArtistRef], 1)] }) - ); - (createLikedAlbumsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - (createLikedTracksInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); + test('an empty tab shows its inline hint', async () => { + setup({ artists: page([ar], 1) }); render(LikedPage); + await fireEvent.click(screen.getByRole('tab', { name: /albums/i })); expect(screen.getByText(/no liked albums yet/i)).toBeInTheDocument(); + await fireEvent.click(screen.getByRole('tab', { name: /tracks/i })); expect(screen.getByText(/no liked tracks yet/i)).toBeInTheDocument(); - // Inline variant uses link affordances, not the big card heading. expect(screen.queryByRole('heading', { name: /no likes yet/i })).not.toBeInTheDocument(); }); - test('Load more calls fetchNextPage on each section that has more', async () => { - const fetchTracks = vi.fn(); - (createLikedTracksInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 100)], hasNextPage: true, fetchNextPage: fetchTracks }) - ); - (createLikedAlbumsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - (createLikedArtistsInfiniteQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); + // Preference 172: the list continues itself; there is no button to press. + test('more tracks load as you scroll, with no Load more button', () => { + setup({ tracks: page([tr], 100), tracksQuery: { hasNextPage: true } }); + const { container } = render(LikedPage); + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + expect(container.querySelector('div[aria-hidden="true"].h-px')).not.toBeNull(); + }); + + test('a failed page offers Try again, which fetches it', async () => { + const fetchNextPage = vi.fn(); + setup({ + tracks: page([tr], 100), + tracksQuery: { hasNextPage: true, isFetchNextPageError: true, fetchNextPage } + }); render(LikedPage); - const buttons = screen.getAllByRole('button', { name: /load more/i }); - expect(buttons).toHaveLength(1); - await fireEvent.click(buttons[0]); - expect(fetchTracks).toHaveBeenCalledTimes(1); + await fireEvent.click(screen.getByRole('button', { name: 'Try again' })); + expect(fetchNextPage).toHaveBeenCalledTimes(1); }); }); diff --git a/web/src/routes/library/years/+page.svelte b/web/src/routes/library/years/+page.svelte index 5d1469f7..fb5ab4fe 100644 --- a/web/src/routes/library/years/+page.svelte +++ b/web/src/routes/library/years/+page.svelte @@ -11,6 +11,7 @@ import AlbumCard from '#lib/components/AlbumCard.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; import EmptyState from '#lib/components/EmptyState.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import type { AlbumRef } from '#lib/api/types.js'; const indexStore = createAlbumYearsQuery(); @@ -48,6 +49,9 @@ let total = $state(0); let loading = $state(false); let failed = $state(false); + // A later page failed: the albums already shown stay, and the list offers a + // retry at its end instead of loading again on every scroll. + let moreFailed = $state(false); // Plain `let`, not $state — see the note in the genres page: as reactive // state, reading it in the fetch path would make the effect below depend on @@ -65,6 +69,7 @@ albums = []; total = 0; failed = false; + moreFailed = false; if (year === null) return; await fetchPage(year, 0, requestToken); } @@ -77,14 +82,19 @@ albums = offset === 0 ? p.items : [...albums, ...p.items]; total = p.total; } catch { - if (token === requestToken) failed = true; + if (token === requestToken) { + if (offset === 0) failed = true; + else moreFailed = true; + } } finally { if (token === requestToken) loading = false; } } function loadMore() { - if (selected !== null) void fetchPage(selected, albums.length, requestToken); + if (selected === null || loading) return; + moreFailed = false; + void fetchPage(selected, albums.length, requestToken); } @@ -136,21 +146,13 @@ {/each} - {#if albums.length < total} -
- -
- {:else} -

End of year

- {/if} + {/if} {:else} diff --git a/web/src/routes/library/years/years.test.ts b/web/src/routes/library/years/years.test.ts index cd0f2436..eedfab16 100644 --- a/web/src/routes/library/years/years.test.ts +++ b/web/src/routes/library/years/years.test.ts @@ -1,6 +1,7 @@ -import { afterEach, describe, expect, test, vi } from 'vitest'; +import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; import { render, screen, waitFor, fireEvent } from '@testing-library/svelte'; import { mockQuery } from '#test-utils/query.js'; +import { installIntersectionObserverMock } from '#test-utils/intersectionObserver.js'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; import { apiClientMock } from '#test-utils/mocks/client.js'; import { emptyLikesMock } from '#test-utils/mocks/likes.js'; @@ -33,6 +34,11 @@ import { createAlbumYearsQuery, listAlbumsByYear } from '#lib/api/browse.js'; const asMock = (fn: unknown) => fn as ReturnType; +let io: ReturnType; +beforeEach(() => { + io = installIntersectionObserverMock(); +}); + function album(id: string, title: string): AlbumRef { return { id, @@ -146,7 +152,7 @@ describe('/library/years drill-down', () => { expect(screen.getByRole('heading', { level: 1, name: 'Years' })).toBeInTheDocument(); }); - test('load more appends and then reports the end', async () => { + test('scrolling to the end loads the next page, then reports the end', async () => { pageState.pageUrl = new URL('http://localhost/library/years?y=1995'); asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); asMock(listAlbumsByYear) @@ -160,9 +166,13 @@ describe('/library/years drill-down', () => { render(YearsPage); await screen.findByText('One'); - await fireEvent.click(await screen.findByRole('button', { name: /Load more \(1 left\)/ })); + // No button (#5416): reaching the bottom of the grid loads the next page. + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + await waitFor(() => { + io.scrollToEnd(); + expect(listAlbumsByYear).toHaveBeenLastCalledWith(1995, 2, 2); + }); - await waitFor(() => expect(listAlbumsByYear).toHaveBeenLastCalledWith(1995, 2, 2)); expect(await screen.findByText('Three')).toBeInTheDocument(); expect(screen.getByText('One')).toBeInTheDocument(); expect(await screen.findByText('End of year')).toBeInTheDocument(); @@ -176,4 +186,27 @@ describe('/library/years drill-down', () => { expect(await screen.findByText(/Couldn't load albums for 1995/i)).toBeInTheDocument(); }); + + // A failed later page keeps what is shown and offers a retry at the end, + // rather than replacing the grid or re-requesting on every scroll. + test('a failed later page keeps the albums and offers Try again', async () => { + pageState.pageUrl = new URL('http://localhost/library/years?y=1995'); + asMock(createAlbumYearsQuery).mockReturnValue(mockQuery({ data: [] })); + asMock(listAlbumsByYear) + .mockResolvedValueOnce({ items: [album('a1', 'One'), album('a2', 'Two')], total: 3, limit: 2, offset: 0 }) + .mockRejectedValueOnce(new Error('nope')) + .mockResolvedValueOnce({ items: [album('a3', 'Three')], total: 3, limit: 2, offset: 2 }); + render(YearsPage); + + await screen.findByText('One'); + await waitFor(() => { + io.scrollToEnd(); + expect(screen.getByRole('alert')).toHaveTextContent("Couldn't load more."); + }); + expect(screen.getByText('One')).toBeInTheDocument(); + expect(screen.queryByText(/Couldn't load albums for 1995/i)).not.toBeInTheDocument(); + + await fireEvent.click(screen.getByRole('button', { name: 'Try again' })); + expect(await screen.findByText('Three')).toBeInTheDocument(); + }); }); diff --git a/web/src/routes/search/albums/+page.svelte b/web/src/routes/search/albums/+page.svelte index 97287a25..35a28baa 100644 --- a/web/src/routes/search/albums/+page.svelte +++ b/web/src/routes/search/albums/+page.svelte @@ -5,6 +5,7 @@ import AlbumCard from '#lib/components/AlbumCard.svelte'; import LibrarySkeleton from '#lib/components/LibrarySkeleton.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import { useDelayed } from '#lib/utils/useDelayed.svelte.js'; const q = $derived((page.url.searchParams.get('q') ?? '').trim()); @@ -24,14 +25,14 @@ ← Back to search

Albums matching '{q}'

- {#if !query?.isPending && !query?.isError && q} + {#if q && !query?.isPending && !(query?.isError && albums.length === 0)}

{total} {total === 1 ? 'album' : 'albums'}

{/if} {#if !q}

No query — return to search to start typing.

- {:else if query?.isError} + {:else if query?.isError && albums.length === 0} {:else if showSkeleton.value && albums.length === 0} @@ -43,15 +44,11 @@ {/each} - {#if query?.hasNextPage} - - {/if} + query?.fetchNextPage()} + /> {/if} diff --git a/web/src/routes/search/albums/albums.test.ts b/web/src/routes/search/albums/albums.test.ts index 2ad8d960..516ceb46 100644 --- a/web/src/routes/search/albums/albums.test.ts +++ b/web/src/routes/search/albums/albums.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; +import { render, screen, waitFor } from '@testing-library/svelte'; import { mockInfiniteQuery } from '../../../test-utils/query'; +import { installIntersectionObserverMock } from '#test-utils/intersectionObserver.js'; import { emptyLikesMock } from '../../../test-utils/mocks/likes'; import { apiClientMock } from '../../../test-utils/mocks/client'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; @@ -47,7 +48,9 @@ describe('search albums overflow', () => { expect(screen.getByRole('link', { name: /Kind of Blue/ })).toHaveAttribute('href', '/albums/al1'); }); - test('Load more calls fetchNextPage', async () => { + // #5416: no button; reaching the end of the list loads the next page. + test('scrolling to the end loads the next page', async () => { + const io = installIntersectionObserverMock(); const fetchNextPage = vi.fn(); (createSearchAlbumsInfiniteQuery as ReturnType).mockReturnValue( mockInfiniteQuery({ @@ -57,8 +60,11 @@ describe('search albums overflow', () => { }) ); render(AlbumsOverflow); - await fireEvent.click(screen.getByRole('button', { name: /load more/i })); - expect(fetchNextPage).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + await waitFor(() => { + io.scrollToEnd(); + expect(fetchNextPage).toHaveBeenCalled(); + }); }); test('empty q shows a prompt', () => { diff --git a/web/src/routes/search/artists/+page.svelte b/web/src/routes/search/artists/+page.svelte index d6503cc0..9e0d7967 100644 --- a/web/src/routes/search/artists/+page.svelte +++ b/web/src/routes/search/artists/+page.svelte @@ -5,6 +5,7 @@ import ArtistCard from '#lib/components/ArtistCard.svelte'; import LibrarySkeleton from '#lib/components/LibrarySkeleton.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import { useDelayed } from '#lib/utils/useDelayed.svelte.js'; const q = $derived((page.url.searchParams.get('q') ?? '').trim()); @@ -24,14 +25,14 @@ ← Back to search

Artists matching '{q}'

- {#if !query?.isPending && !query?.isError && q} + {#if q && !query?.isPending && !(query?.isError && artists.length === 0)}

{total} {total === 1 ? 'artist' : 'artists'}

{/if} {#if !q}

No query — return to search to start typing.

- {:else if query?.isError} + {:else if query?.isError && artists.length === 0} {:else if showSkeleton.value && artists.length === 0} @@ -43,15 +44,11 @@ {/each} - {#if query?.hasNextPage} - - {/if} + query?.fetchNextPage()} + /> {/if} diff --git a/web/src/routes/search/artists/artists.test.ts b/web/src/routes/search/artists/artists.test.ts index d604f734..7f8bb42f 100644 --- a/web/src/routes/search/artists/artists.test.ts +++ b/web/src/routes/search/artists/artists.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; +import { render, screen, waitFor } from '@testing-library/svelte'; import { mockInfiniteQuery } from '../../../test-utils/query'; +import { installIntersectionObserverMock } from '#test-utils/intersectionObserver.js'; import { emptyLikesMock } from '../../../test-utils/mocks/likes'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; import type { ArtistRef, Page } from '#lib/api/types.js'; @@ -39,7 +40,9 @@ describe('search artists overflow', () => { expect(screen.getByRole('link', { name: /Miles Mosley/ })).toBeInTheDocument(); }); - test('Load more calls fetchNextPage when hasNextPage', async () => { + // #5416: no button; reaching the end of the list loads the next page. + test('scrolling to the end loads the next page', async () => { + const io = installIntersectionObserverMock(); const fetchNextPage = vi.fn(); (createSearchArtistsInfiniteQuery as ReturnType).mockReturnValue( mockInfiniteQuery({ @@ -49,16 +52,19 @@ describe('search artists overflow', () => { }) ); render(ArtistsOverflow); - await fireEvent.click(screen.getByRole('button', { name: /load more/i })); - expect(fetchNextPage).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + await waitFor(() => { + io.scrollToEnd(); + expect(fetchNextPage).toHaveBeenCalled(); + }); }); - test('Load more is hidden when hasNextPage is false', () => { + test('nothing more to load: no sentinel', () => { (createSearchArtistsInfiniteQuery as ReturnType).mockReturnValue( mockInfiniteQuery({ pages: [page([{ id: 'a', name: 'A', sort_name: 'A', album_count: 1, cover_url: '' }], 1)] }) ); - render(ArtistsOverflow); - expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + const { container } = render(ArtistsOverflow); + expect(container.querySelector('div[aria-hidden="true"].h-px')).toBeNull(); }); test('empty q shows a prompt and fires no query', () => { diff --git a/web/src/routes/search/tracks/+page.svelte b/web/src/routes/search/tracks/+page.svelte index 95ef8014..2be2f018 100644 --- a/web/src/routes/search/tracks/+page.svelte +++ b/web/src/routes/search/tracks/+page.svelte @@ -7,6 +7,7 @@ import TrackList from '#lib/components/TrackList.svelte'; import LibrarySkeleton from '#lib/components/LibrarySkeleton.svelte'; import ApiErrorBanner from '#lib/components/ApiErrorBanner.svelte'; + import ListContinuation from '#lib/components/ListContinuation.svelte'; import { useDelayed } from '#lib/utils/useDelayed.svelte.js'; import { playRadio } from '#lib/player/store.svelte.js'; import type { TrackRef } from '#lib/api/types.js'; @@ -32,14 +33,14 @@ ← Back to search

Tracks matching '{q}'

- {#if !query?.isPending && !query?.isError && q} + {#if q && !query?.isPending && !(query?.isError && tracks.length === 0)}

{total} {total === 1 ? 'track' : 'tracks'}

{/if} {#if !q}

No query — return to search to start typing.

- {:else if query?.isError} + {:else if query?.isError && tracks.length === 0} {:else if showSkeleton.value && tracks.length === 0} @@ -51,15 +52,11 @@ {/each} - {#if query?.hasNextPage} - - {/if} + query?.fetchNextPage()} + /> {/if} diff --git a/web/src/routes/search/tracks/tracks.test.ts b/web/src/routes/search/tracks/tracks.test.ts index c624b5f4..6b081352 100644 --- a/web/src/routes/search/tracks/tracks.test.ts +++ b/web/src/routes/search/tracks/tracks.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; import { mockInfiniteQuery } from '../../../test-utils/query'; +import { installIntersectionObserverMock } from '#test-utils/intersectionObserver.js'; import { emptyLikesMock } from '../../../test-utils/mocks/likes'; import { emptyQuarantineMock } from '../../../test-utils/mocks/quarantine'; import { pageUrlModule } from '#test-utils/mocks/appState.js'; @@ -59,7 +60,9 @@ describe('search tracks overflow', () => { expect(playRadio).toHaveBeenCalledWith('t1'); }); - test('Load more calls fetchNextPage', async () => { + // #5416: no button; reaching the end of the list loads the next page. + test('scrolling to the end loads the next page', async () => { + const io = installIntersectionObserverMock(); const fetchNextPage = vi.fn(); (createSearchTracksInfiniteQuery as ReturnType).mockReturnValue( mockInfiniteQuery({ @@ -69,8 +72,11 @@ describe('search tracks overflow', () => { }) ); render(TracksOverflow); - await fireEvent.click(screen.getByRole('button', { name: /load more/i })); - expect(fetchNextPage).toHaveBeenCalledTimes(1); + expect(screen.queryByRole('button', { name: /load more/i })).not.toBeInTheDocument(); + await waitFor(() => { + io.scrollToEnd(); + expect(fetchNextPage).toHaveBeenCalled(); + }); }); test('empty q shows a prompt', () => { diff --git a/web/src/test-utils/intersectionObserver.ts b/web/src/test-utils/intersectionObserver.ts new file mode 100644 index 00000000..57450dbb --- /dev/null +++ b/web/src/test-utils/intersectionObserver.ts @@ -0,0 +1,40 @@ +import { vi } from 'vitest'; + +// jsdom doesn't ship IntersectionObserver, so a list that loads as you scroll +// (InfiniteScrollSentinel, ListContinuation) never fires in a test. This +// installs a stand-in that records each observer, so a test can play +// "the reader scrolled to the bottom" with scrollToEnd(). +type Observer = { cb: IntersectionObserverCallback; self: IntersectionObserver; connected: boolean }; + +export function installIntersectionObserverMock() { + const observers: Observer[] = []; + + class MockIntersectionObserver { + observe = vi.fn(); + unobserve = vi.fn(); + disconnect = vi.fn(() => { + const o = observers.find((x) => x.self === (this as unknown as IntersectionObserver)); + if (o) o.connected = false; + }); + takeRecords = vi.fn(() => []); + constructor(cb: IntersectionObserverCallback) { + observers.push({ cb, self: this as unknown as IntersectionObserver, connected: true }); + } + } + + vi.stubGlobal('IntersectionObserver', MockIntersectionObserver); + + return { + /** Reports every connected observer's target as on screen. */ + scrollToEnd() { + for (const o of observers) { + if (!o.connected) continue; + o.cb([{ isIntersecting: true } as IntersectionObserverEntry], o.self); + } + }, + /** How many observers are currently watching. */ + connectedCount() { + return observers.filter((o) => o.connected).length; + } + }; +} diff --git a/web/src/test-utils/query.ts b/web/src/test-utils/query.ts index 6e24f72e..39f2e50c 100644 --- a/web/src/test-utils/query.ts +++ b/web/src/test-utils/query.ts @@ -14,6 +14,7 @@ export function mockInfiniteQuery(opts: { error?: unknown; hasNextPage?: boolean; isFetchingNextPage?: boolean; + isFetchNextPageError?: boolean; fetchNextPage?: () => void; refetch?: () => void; } = {}) { @@ -24,6 +25,7 @@ export function mockInfiniteQuery(opts: { error: opts.error, hasNextPage: opts.hasNextPage ?? false, isFetchingNextPage: opts.isFetchingNextPage ?? false, + isFetchNextPageError: opts.isFetchNextPageError ?? false, fetchNextPage: opts.fetchNextPage ?? (() => {}), refetch: opts.refetch ?? (() => {}) });