bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

feat(web): trim main nav; pair Discover+Requests; move chrome to user menu

Browse Source 
The main nav was carrying surfaces that didn't belong on it:

- Search led to an empty page when clicked (you only ever want to land
  there from the global SearchInput typing into /search?q=...). Drop it.
- Requests is downstream of Discover (you discover, then you request).
  Lift both onto a shared horizontal tab strip; keep their URLs so
  bookmarks survive. New DiscoverTabs component used by both pages.
- Settings + Admin are operator chrome, not primary surfaces. Move them
  into the username dropdown alongside Log out, with Admin gated on
  is_admin. Users see Settings + Log out; admins see Settings + Admin
  + Log out.

Final main nav: Home / Artists / Albums / Liked / Discover / Playlists.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Van Deusen
2026-05-01 22:23:56 -04:00
parent 14c1895beb
commit 22ac86f200
6 changed files with 136 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/components/DiscoverTabs.svelte
+36
View File
@@ -0,0 +1,36 @@
<script lang="ts">
import { page } from '$app/state';
type Item = { href: string; label: string };
// Discover and Requests are paired: discover an artist → request it. The
// tab strip lifts both surfaces onto the same horizontal axis so the
// operator can move between them without round-tripping through the
// main nav.
const items: Item[] = [
{ href: '/discover', label: 'Discover' },
{ href: '/requests', label: 'Requests' }
];
function isActive(href: string): boolean {
return page.url.pathname.startsWith(href);
}
</script>
<nav aria-label="Discover sections" class="border-b border-border">
<ul class="flex gap-2">
{#each items as item (item.href)}
<li>
<a
href={item.href}
aria-current={isActive(item.href) ? 'page' : undefined}
class="block border-b-2 px-3 py-2 text-sm transition-colors {isActive(item.href)
? 'border-accent text-text-primary'
: 'border-transparent text-text-secondary hover:text-text-primary'}"
>
{item.label}
</a>
</li>
{/each}
</ul>
</nav>
web/src/lib/components/DiscoverTabs.test.ts
+35
View File
@@ -0,0 +1,35 @@
import { describe, expect, test, vi } from 'vitest';
import { render, screen } from '@testing-library/svelte';
const state = vi.hoisted(() => ({
pageUrl: new URL('http://localhost/discover')
}));
vi.mock('$app/state', () => ({
page: { get url() { return state.pageUrl; } }
}));
import DiscoverTabs from './DiscoverTabs.svelte';
describe('DiscoverTabs', () => {
test('Discover tab is current on /discover', () => {
state.pageUrl = new URL('http://localhost/discover');
render(DiscoverTabs);
expect(screen.getByRole('link', { name: /discover/i })).toHaveAttribute('aria-current', 'page');
expect(screen.getByRole('link', { name: /requests/i })).not.toHaveAttribute('aria-current');
});
test('Requests tab is current on /requests', () => {
state.pageUrl = new URL('http://localhost/requests');
render(DiscoverTabs);
expect(screen.getByRole('link', { name: /requests/i })).toHaveAttribute('aria-current', 'page');
expect(screen.getByRole('link', { name: /discover/i })).not.toHaveAttribute('aria-current');
});
test('renders both tabs in order', () => {
state.pageUrl = new URL('http://localhost/discover');
render(DiscoverTabs);
const labels = screen.getAllByRole('link').map((l) => l.textContent?.trim());
expect(labels).toEqual(['Discover', 'Requests']);
});
});
web/src/lib/components/Shell.svelte
+26 -14
View File
@@ -21,24 +21,17 @@
return page.url.pathname.startsWith(href);
}
// Search reaches /search via the global SearchInput in the header (not the
// nav). Requests is a tab inside Discover. Settings + Admin live in the
// username dropdown — they're per-operator chrome, not primary surfaces.
const navItems = [
{ href: '/', label: 'Home' },
{ href: '/library/artists', label: 'Artists' },
{ href: '/library/albums', label: 'Albums' },
{ href: '/library/liked', label: 'Liked' },
{ href: '/search', label: 'Search' },
{ href: '/discover', label: 'Discover' },
{ href: '/requests', label: 'Requests' },
{ href: '/playlists', label: 'Playlists' },
{ href: '/settings', label: 'Settings' }
{ href: '/playlists', label: 'Playlists' }
];
// Admin link sits between Playlists and Settings, only visible to admins.
const visibleNavItems = $derived(
user.value?.is_admin
? [...navItems.slice(0, -1), { href: '/admin', label: 'Admin' }, navItems[navItems.length - 1]]
: navItems
);
</script>
<svelte:window onclick={() => (menuOpen = false)} onkeydown={(e) => e.key === 'Escape' && (menuOpen = false)} />
@@ -59,15 +52,34 @@
</button>
{#if menuOpen}
<div
class="absolute right-0 mt-1 w-32 rounded border border-border bg-surface shadow"
class="absolute right-0 mt-1 w-40 rounded border border-border bg-surface shadow"
onclick={(e) => e.stopPropagation()}
onkeydown={(e) => e.stopPropagation()}
role="menu"
tabindex="-1"
>
<a
href="/settings"
role="menuitem"
class="block px-3 py-2 text-sm hover:bg-surface-hover"
onclick={() => (menuOpen = false)}
>
Settings
</a>
{#if user.value?.is_admin}
<a
href="/admin"
role="menuitem"
class="block px-3 py-2 text-sm hover:bg-surface-hover"
onclick={() => (menuOpen = false)}
>
Admin
</a>
{/if}
<button
type="button"
class="block w-full px-3 py-2 text-left hover:bg-surface-hover"
role="menuitem"
class="block w-full border-t border-border px-3 py-2 text-left text-sm hover:bg-surface-hover"
onclick={handleLogout}
>
Log out
@@ -79,7 +91,7 @@
<nav class="hidden w-48 border-r border-border bg-surface md:block">
<ul class="p-2">
{#each visibleNavItems as item}
{#each navItems as item}
<li>
<a
href={item.href}
web/src/lib/components/Shell.test.ts
+31 -29
View File
@@ -39,54 +39,56 @@ describe('Shell', () => {
expect(screen.getByText('alice')).toBeInTheDocument();
});
test('renders nav items in expected order', () => {
test('main nav renders the six primary surfaces in order', () => {
render(Shell);
const labels = ['Home', 'Artists', 'Albums', 'Liked', 'Search',
'Discover', 'Requests', 'Playlists', 'Settings'];
const labels = ['Home', 'Artists', 'Albums', 'Liked', 'Discover', 'Playlists'];
for (const label of labels) {
expect(screen.getByRole('link', { name: label })).toBeInTheDocument();
}
expect(screen.getByRole('link', { name: 'Home' })).toHaveAttribute('href', '/');
expect(screen.getByRole('link', { name: 'Artists' })).toHaveAttribute('href', '/library/artists');
expect(screen.getByRole('link', { name: 'Albums' })).toHaveAttribute('href', '/library/albums');
expect(screen.getByRole('link', { name: 'Search' })).toHaveAttribute('href', '/search');
expect(screen.getByRole('link', { name: 'Discover' })).toHaveAttribute('href', '/discover');
expect(screen.getByRole('link', { name: 'Requests' })).toHaveAttribute('href', '/requests');
expect(screen.getByRole('link', { name: 'Playlists' })).toHaveAttribute('href', '/playlists');
});
test('Hidden link is not in the main nav', () => {
render(Shell);
expect(screen.queryByRole('link', { name: 'Hidden' })).not.toBeInTheDocument();
});
test('non-admin users do not see the Admin nav link', () => {
userState.current = { id: '1', username: 'alice', is_admin: false };
render(Shell);
expect(screen.queryByRole('link', { name: 'Admin' })).not.toBeInTheDocument();
});
test('admin users see the Admin nav link between Playlists and Settings', () => {
test('main nav omits Search, Requests, Hidden, Settings, and Admin', () => {
userState.current = { id: '1', username: 'alice', is_admin: true };
render(Shell);
const link = screen.getByRole('link', { name: 'Admin' });
expect(link).toHaveAttribute('href', '/admin');
// Order check: Playlists then Admin then Settings.
const labels = screen
// Search reaches /search via the global SearchInput, Requests is a
// tab inside Discover, Hidden lives under Settings, Settings + Admin
// live in the username dropdown — none belong on the main nav.
const navLinks = screen
.getAllByRole('link')
.map((el) => el.textContent?.trim())
.filter(Boolean);
const idxPlaylists = labels.indexOf('Playlists');
const idxAdmin = labels.indexOf('Admin');
const idxSettings = labels.indexOf('Settings');
expect(idxPlaylists).toBeLessThan(idxAdmin);
expect(idxAdmin).toBeLessThan(idxSettings);
.filter((el) => el.closest('nav') !== null)
.map((el) => el.textContent?.trim());
for (const label of ['Search', 'Requests', 'Hidden', 'Settings', 'Admin']) {
expect(navLinks).not.toContain(label);
}
});
test('user-menu shows Settings + Log out for non-admin users', async () => {
userState.current = { id: '1', username: 'alice', is_admin: false };
render(Shell);
await fireEvent.click(screen.getByRole('button', { name: /alice/i }));
expect(screen.getByRole('menuitem', { name: 'Settings' })).toHaveAttribute('href', '/settings');
expect(screen.getByRole('menuitem', { name: /log out/i })).toBeInTheDocument();
expect(screen.queryByRole('menuitem', { name: 'Admin' })).not.toBeInTheDocument();
});
test('user-menu adds Admin between Settings and Log out for admin users', async () => {
userState.current = { id: '1', username: 'alice', is_admin: true };
render(Shell);
await fireEvent.click(screen.getByRole('button', { name: /alice/i }));
const items = screen.getAllByRole('menuitem').map((el) => el.textContent?.trim());
expect(items).toEqual(['Settings', 'Admin', 'Log out']);
expect(screen.getByRole('menuitem', { name: 'Admin' })).toHaveAttribute('href', '/admin');
});
test('user-menu "Log out" calls logout() and navigates to /login', async () => {
render(Shell);
await fireEvent.click(screen.getByRole('button', { name: /alice/i }));
await fireEvent.click(screen.getByRole('button', { name: /log out/i }));
await fireEvent.click(screen.getByRole('menuitem', { name: /log out/i }));
expect(logout).toHaveBeenCalledTimes(1);
expect(goto).toHaveBeenCalledWith('/login', { replaceState: true });
});
web/src/routes/discover/+page.svelte
+4 -1
View File
@@ -2,6 +2,7 @@
import { createLidarrSearchQuery } from '$lib/api/lidarr';
import { createRequest } from '$lib/api/requests';
import DiscoverResultCard from '$lib/components/DiscoverResultCard.svelte';
import DiscoverTabs from '$lib/components/DiscoverTabs.svelte';
import ApiErrorBanner from '$lib/components/ApiErrorBanner.svelte';
import SuggestionFeed from '$lib/components/SuggestionFeed.svelte';
import type {
@@ -114,7 +115,9 @@
];
</script>
<div class="space-y-6">
<DiscoverTabs />
<div class="space-y-6 pt-6">
<input
type="search"
aria-label="Search Lidarr"
web/src/routes/requests/+page.svelte
+4 -1
View File
@@ -5,6 +5,7 @@
import { qk } from '$lib/api/queries';
import StatusPill from '$lib/components/StatusPill.svelte';
import ApiErrorBanner from '$lib/components/ApiErrorBanner.svelte';
import DiscoverTabs from '$lib/components/DiscoverTabs.svelte';
import type { LidarrRequest, LidarrRequestKind } from '$lib/api/types';
const queryStore = createMyRequestsQuery();
@@ -60,7 +61,9 @@
}
</script>
<div class="space-y-6">
<DiscoverTabs />
<div class="space-y-6 pt-6">
<header class="space-y-1">
<h2 class="font-display text-2xl font-medium text-text-primary">
Your requests