feat(web): sort the genre index A–Z as well as by count — #2468
test-web / test (push) Successful in 38s
test-web / test (push) Successful in 38s
Operator decision: the taxonomy this task proposed is cancelled. With their repaired library measured — 391 genres, 90,774 tag applications over ~24,185 tracks, so ~3.7 genres per track — multi-membership already puts each track under everything it claims, and grouping would add nothing while destroying real specificity (Neurofunk, Wassoulou, Soukous are not noise). The task's premise was also wrong. It argued from case variants, "Alt. Rock" abbreviations and a junk tail; none exist. That apparent mess was the scanner welding multi-value tags (#2499) plus ghost rows from deleted files (#2523), both ours, both now fixed. A category system would have papered over both. So the ask reduces to sorting and search. The search box already existed (QuickFilter, with its own no-matches state), so this adds only the sort: count-first by default — the server's order, and the right default since the head is where you're going — or A–Z for when you can already name the thing but can't find it among 391 rows. The filtered count now reads "12 of 391" so a filter's effect is visible. Client-side only: /api/library/genres is unpaged and already returns the whole set (~12KB), so neither control needs a round trip or a server change. Two things the tests pin down: - The sort COPIES before sorting. With no filter applied the derived list is the very array held by the query cache, and Array.sort mutates in place — sorting it directly would reorder cached data under every other consumer. - Count mode passes the server's order through rather than re-sorting. The fixture is deliberately not in count order so the test asserts pass-through instead of coincidence. Verified locally: svelte-check 0 errors, 110 files / 788 tests.
This commit is contained in:
@@ -30,6 +30,31 @@
|
||||
return genres.filter((g: GenreCount) => g.genre.toLowerCase().includes(q));
|
||||
});
|
||||
|
||||
// Count-first by default because that's what the server returns and it's the
|
||||
// right default: the head of this list is genuinely where you're going. But a
|
||||
// real library runs to several hundred genres with a long tail of one-offs
|
||||
// (391 / ~3.7 per track on the operator's), and at that size "I know roughly
|
||||
// what it's called" needs A-Z as much as filtering does.
|
||||
//
|
||||
// View state only, not a query parameter — same as `filter` above. Neither is
|
||||
// worth making shareable, and putting one in the URL and not the other would
|
||||
// be the inconsistent choice.
|
||||
let sortMode = $state<'count' | 'name'>('count');
|
||||
|
||||
const visibleGenres = $derived.by(() => {
|
||||
// Copy before sorting. With no filter applied `filteredGenres` IS the array
|
||||
// held by the query cache, and Array.sort mutates in place — sorting it
|
||||
// directly would reorder cached data for every other consumer.
|
||||
const list = [...filteredGenres];
|
||||
if (sortMode === 'name') {
|
||||
// sensitivity 'base' so case and accents don't split neighbours apart.
|
||||
return list.sort((a: GenreCount, b: GenreCount) =>
|
||||
a.genre.localeCompare(b.genre, undefined, { sensitivity: 'base' })
|
||||
);
|
||||
}
|
||||
return list; // server order: count DESC, then name
|
||||
});
|
||||
|
||||
let albums = $state<AlbumRef[]>([]);
|
||||
let total = $state(0);
|
||||
let loading = $state(false);
|
||||
@@ -153,12 +178,30 @@
|
||||
<h1 class="font-display text-2xl font-medium text-text-primary">Genres</h1>
|
||||
{#if !index.isPending && !index.isError}
|
||||
<p class="text-sm text-text-secondary">
|
||||
{genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags
|
||||
{#if filter.trim()}
|
||||
{visibleGenres.length} of {genres.length} genres
|
||||
{:else}
|
||||
{genres.length} {genres.length === 1 ? 'genre' : 'genres'}, straight from your file tags
|
||||
{/if}
|
||||
</p>
|
||||
{/if}
|
||||
</div>
|
||||
{#if genres.length > 0}
|
||||
<QuickFilter bind:value={filter} placeholder="Filter genres" />
|
||||
<div class="flex flex-wrap items-center gap-2">
|
||||
<QuickFilter bind:value={filter} placeholder="Filter genres" />
|
||||
<label class="flex items-center gap-1.5 text-xs text-text-secondary">
|
||||
Sort
|
||||
<select
|
||||
bind:value={sortMode}
|
||||
aria-label="Sort genres"
|
||||
class="rounded border border-border bg-surface px-2 py-1 text-sm text-text-primary
|
||||
focus-visible:outline focus-visible:outline-2 focus-visible:outline-accent"
|
||||
>
|
||||
<option value="count">Most tracks</option>
|
||||
<option value="name">A–Z</option>
|
||||
</select>
|
||||
</label>
|
||||
</div>
|
||||
{/if}
|
||||
</header>
|
||||
|
||||
@@ -180,16 +223,17 @@
|
||||
</a>
|
||||
{/snippet}
|
||||
</EmptyState>
|
||||
{:else if filter.trim() && filteredGenres.length === 0}
|
||||
{:else if filter.trim() && visibleGenres.length === 0}
|
||||
<p class="text-text-secondary">
|
||||
No genres match <span class="font-medium">'{filter.trim()}'</span>.
|
||||
</p>
|
||||
{:else}
|
||||
<!-- Ordered by track count, not alphabetically: raw tags carry a long
|
||||
tail of one-offs, so alphabetical would bury the handful of genres
|
||||
you actually have a library's worth of. -->
|
||||
<!-- Defaults to track count rather than alphabetical: raw tags carry a
|
||||
long tail of one-offs, so A-Z would bury the handful of genres you
|
||||
actually have a library's worth of. The Sort control lets you ask for
|
||||
A-Z when you already know roughly what you're looking for. -->
|
||||
<ul class="grid grid-cols-1 gap-2 sm:grid-cols-2 lg:grid-cols-3">
|
||||
{#each filteredGenres as g (g.genre)}
|
||||
{#each visibleGenres as g (g.genre)}
|
||||
<li>
|
||||
<a
|
||||
href={genreHref(g.genre)}
|
||||
|
||||
@@ -98,6 +98,57 @@ describe('/library/genres index', () => {
|
||||
expect(screen.getByRole('link', { name: /^rock 2$/ })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
// #2468: the operator's real library is 391 genres at ~3.7 per track, so the
|
||||
// count-first default buries anything you can already name. A–Z is the ask.
|
||||
test('A–Z sort reorders the list without mutating the source array', async () => {
|
||||
const data = [
|
||||
{ genre: 'Rock', track_count: 8974 },
|
||||
{ genre: 'Ambient', track_count: 646 },
|
||||
{ genre: 'jazz', track_count: 1406 }
|
||||
];
|
||||
asMock(createGenresQuery).mockReturnValue(mockQuery({ data }));
|
||||
render(GenresPage);
|
||||
|
||||
const order = () =>
|
||||
screen.getAllByRole('link').map((a) => a.textContent?.trim().split(/\s+/)[0]);
|
||||
|
||||
// Count mode PRESERVES the server's order rather than re-sorting client
|
||||
// side — the server already returns count DESC, name ASC, and duplicating
|
||||
// that here would be two orderings to keep in step. The fixture is
|
||||
// deliberately not in count order so this asserts pass-through, not luck.
|
||||
expect(order()).toEqual(['Rock', 'Ambient', 'jazz']);
|
||||
|
||||
await fireEvent.change(screen.getByLabelText('Sort genres'), { target: { value: 'name' } });
|
||||
|
||||
// Case-insensitive, so 'jazz' sorts between Ambient and Rock rather than
|
||||
// after both.
|
||||
expect(order()).toEqual(['Ambient', 'jazz', 'Rock']);
|
||||
|
||||
// The query cache's own array must not have been reordered in place —
|
||||
// Array.sort mutates, and with no filter applied the derived list IS that
|
||||
// array.
|
||||
expect(data.map((d) => d.genre)).toEqual(['Rock', 'Ambient', 'jazz']);
|
||||
});
|
||||
|
||||
test('filtering reports the visible subset against the total', async () => {
|
||||
asMock(createGenresQuery).mockReturnValue(
|
||||
mockQuery({
|
||||
data: [
|
||||
{ genre: 'Rock', track_count: 10 },
|
||||
{ genre: 'Ska Punk', track_count: 5 },
|
||||
{ genre: 'Jazz', track_count: 3 }
|
||||
]
|
||||
})
|
||||
);
|
||||
render(GenresPage);
|
||||
expect(screen.getByText(/3 genres, straight from your file tags/)).toBeInTheDocument();
|
||||
|
||||
await fireEvent.input(screen.getByLabelText('Filter genres'), { target: { value: 'punk' } });
|
||||
|
||||
// QuickFilter debounces by 120ms.
|
||||
await waitFor(() => expect(screen.getByText('1 of 3 genres')).toBeInTheDocument());
|
||||
});
|
||||
|
||||
test('empty library explains where genres come from', () => {
|
||||
asMock(createGenresQuery).mockReturnValue(mockQuery({ data: [] }));
|
||||
render(GenresPage);
|
||||
|
||||
Reference in New Issue
Block a user