fix(search): route the library header search to /search

Typing in the desktop header search bar ran library.search() in place and
relied on /library rendering the results inline. On every other /library/**
route nothing rendered them, so the search bar looked broken: results were
fetched and never shown.

Make /search the single surface that renders results. The header bar becomes a
navigator — it hands the query and route-derived scope to /search via ?q= and
?scope=, which seed the page and run the search on arrival. The inline result
block and the header's scope chips are removed; the chips live on /search,
which owns the results. The empty `all` scope is omitted from the URL, and
typing while already on /search does not push a history entry per keystroke.
This commit is contained in:
2026-07-25 15:13:45 +02:00
parent 5927299c0f
commit 124da29fc7
5 changed files with 130 additions and 42 deletions
+40
View File
@@ -7,6 +7,8 @@ import {
normalizeGroupOrder,
reorderGroups,
resolveSearchScope,
searchRouteUrl,
shouldNavigateToSearch,
scopeItemTypes,
type SearchGroupId,
} from "./searchScope";
@@ -371,3 +373,41 @@ describe("reorderGroups", () => {
expect(reorderGroups(order, 0, 9)).toEqual(order);
});
});
describe("searchRouteUrl", () => {
it("encodes the query and the scope", () => {
expect(searchRouteUrl("miles davis", "music")).toBe("/search?q=miles%20davis&scope=music");
});
it("omits the scope key for the default `all` scope", () => {
expect(searchRouteUrl("dune", "all")).toBe("/search?q=dune");
});
it("targets bare /search for an empty query so the page shows its empty state", () => {
expect(searchRouteUrl("", "all")).toBe("/search");
expect(searchRouteUrl(" ", "music")).toBe("/search");
});
});
describe("shouldNavigateToSearch", () => {
it("navigates from any library page, which cannot render results itself", () => {
// The bug: the header search bar shows on every /library/** route but only
// /library rendered $library.searchResults, so typing did nothing on
// /library/music, /library/tv, /library/movies and detail pages.
expect(shouldNavigateToSearch("/library", "jazz")).toBe(true);
expect(shouldNavigateToSearch("/library/music", "jazz")).toBe(true);
expect(shouldNavigateToSearch("/library/tv", "jazz")).toBe(true);
expect(shouldNavigateToSearch("/library/movies", "jazz")).toBe(true);
expect(shouldNavigateToSearch("/library/abc123", "jazz")).toBe(true);
});
it("stays put when already on /search, so typing does not re-push history", () => {
expect(shouldNavigateToSearch("/search", "jazz")).toBe(false);
expect(shouldNavigateToSearch("/search?q=old", "jazz")).toBe(false);
});
it("does not navigate on an empty query", () => {
expect(shouldNavigateToSearch("/library/music", "")).toBe(false);
expect(shouldNavigateToSearch("/library/music", " ")).toBe(false);
});
});
+36
View File
@@ -62,6 +62,42 @@ export function resolveSearchScope(pathname: string): SearchScope {
return "all";
}
/**
* The URL of the single search surface for a query + scope.
*
* `/search` is the *only* route that renders results, so every other search
* affordance (the desktop header bar) is a navigator to this URL rather than a
* second result renderer. The `all` scope is the page's own default, so it is
* omitted to keep shared/back-navigated URLs clean.
*
* TRACES: UR-049 | DR-063
*/
export function searchRouteUrl(query: string, scope: SearchScope): string {
const trimmed = query.trim();
if (!trimmed) return "/search";
const params = new URLSearchParams({ q: trimmed });
if (scope !== "all") params.set("scope", scope);
// URLSearchParams renders spaces as "+", valid in a query but noisier to
// read; %20 is equally valid and matches how the app builds other links.
return `/search?${params.toString().replace(/\+/g, "%20")}`;
}
/**
* Whether a search typed on `pathname` must navigate to `/search` to be seen.
*
* True for every route except `/search` itself: no other page renders
* `searchResults`, so a search performed there is invisible. Guarding on
* `/search` keeps typing from pushing a history entry per keystroke.
*
* TRACES: UR-049 | DR-063
*/
export function shouldNavigateToSearch(pathname: string, query: string): boolean {
if (!query.trim()) return false;
const path = pathname.split(/[?#]/)[0].replace(/\/+$/, "") || "/";
return path !== "/search";
}
// ---------------------------------------------------------------------------
// Result groups
// ---------------------------------------------------------------------------
+20 -14
View File
@@ -6,8 +6,12 @@
import { library } from "$lib/stores/library";
import { useScrollGuard } from "$lib/composables/useScrollGuard";
import Search from "$lib/components/Search.svelte";
import SearchScopeChips from "$lib/components/search/SearchScopeChips.svelte";
import { resolveSearchScope, type SearchScope } from "$lib/utils/searchScope";
import {
resolveSearchScope,
searchRouteUrl,
shouldNavigateToSearch,
type SearchScope,
} from "$lib/utils/searchScope";
import AppHeader from "$lib/components/AppHeader.svelte";
import BottomUi from "$lib/components/BottomUi.svelte";
import SleepTimerModal from "$lib/components/player/SleepTimerModal.svelte";
@@ -48,18 +52,22 @@
}
});
// The header bar is a *navigator*, not a second results surface: /search is
// the only route that renders searchResults, so searching here routes there
// with the query + route-derived scope in the URL. Previously this ran
// library.search() in place, which was invisible on every /library/** page
// except /library itself.
// TRACES: UR-049 | DR-063
async function handleSearch(query: string) {
if (query.trim()) {
await library.search(query, searchScope);
} else {
if (!query.trim()) {
library.clearSearch();
return;
}
}
async function handleScopeChange(next: SearchScope) {
searchScope = next;
if (searchQuery.trim()) {
await library.search(searchQuery, next);
if (shouldNavigateToSearch($page.url.pathname, query)) {
await goto(searchRouteUrl(query, searchScope));
// The query now lives in the URL; clear the header input so returning to
// a library page does not leave a stale term sitting in the box.
searchQuery = "";
}
}
</script>
@@ -74,14 +82,12 @@
<AppHeader search={librarySearch} />
{#snippet librarySearch()}
<!-- Scope chips live on /search, which owns the results. -->
<Search
bind:value={searchQuery}
placeholder="Search your library..."
onSearch={handleSearch}
/>
{#if searchQuery.trim()}
<SearchScopeChips scope={searchScope} onChange={handleScopeChange} />
{/if}
{/snippet}
<!-- Main content. The BottomUi below is an in-flow flex sibling, so this
+4 -24
View File
@@ -12,8 +12,9 @@
// Scroll guard from layout - prevents accidental taps during scrolling (Android)
const scrollGuard = getContext<ReturnType<typeof useScrollGuard>>("scrollGuard");
let searchResults = $derived($library.searchResults);
let searchQuery = $derived($library.searchQuery);
// Search results are rendered exclusively by /search — this page used to
// render them inline, which made the header search bar appear broken on every
// other /library/** route. TRACES: UR-049 | DR-063
const isMusicLibrary = $derived($currentLibrary?.collectionType === "music");
@@ -169,28 +170,7 @@
</script>
<div class="space-y-8">
{#if searchQuery}
<!-- Search results -->
<div>
<div class="flex items-center justify-between mb-4">
<h1 class="text-2xl font-bold text-white">
Search results for "{searchQuery}"
</h1>
<button
onclick={() => library.clearSearch()}
class="text-sm text-gray-400 hover:text-white"
>
Clear search
</button>
</div>
<LibraryGrid
items={searchResults}
loading={$isLibraryLoading}
onItemClick={handleItemClick}
/>
</div>
{:else if showInlineLibraryContent}
{#if showInlineLibraryContent}
<!-- Library content (live TV / channels / other inline-rendered types) -->
<div class="space-y-6">
<div class="flex items-center gap-4">
+29 -3
View File
@@ -5,15 +5,41 @@
import Search from "$lib/components/Search.svelte";
import SearchResults from "$lib/components/search/SearchResults.svelte";
import SearchScopeChips from "$lib/components/search/SearchScopeChips.svelte";
import { resolveSearchScope, type SearchScope } from "$lib/utils/searchScope";
import { resolveSearchScope, SEARCH_SCOPES, type SearchScope } from "$lib/utils/searchScope";
import type { MediaItem } from "$lib/api/types";
let searchQuery = $state("");
// `?q=` / `?scope=` seed the page so the desktop header search bar can hand
// a query over by navigating here — /search is the only surface that renders
// results, so every other search affordance routes into it.
// TRACES: UR-049 | DR-063
const initialQuery = $page.url.searchParams.get("q") ?? "";
const initialScope = $page.url.searchParams.get("scope");
let searchQuery = $state(initialQuery);
// Route resolves the *initial* scope only. Deriving it reactively would snap
// a user who widened to All back to the route's scope on any navigation.
// TRACES: UR-049 | DR-064
let scope = $state<SearchScope>(resolveSearchScope($page.url.pathname));
let scope = $state<SearchScope>(
SEARCH_SCOPES.includes(initialScope as SearchScope)
? (initialScope as SearchScope)
: resolveSearchScope($page.url.pathname)
);
// A query arriving in the URL must actually run — mounting with a seeded
// input alone would render the empty state with a filled box.
$effect(() => {
const q = $page.url.searchParams.get("q") ?? "";
if (!q.trim()) return;
const urlScope = $page.url.searchParams.get("scope");
const nextScope = SEARCH_SCOPES.includes(urlScope as SearchScope)
? (urlScope as SearchScope)
: "all";
if (q === $library.searchQuery && nextScope === scope) return;
searchQuery = q;
scope = nextScope;
library.search(q, nextScope);
});
async function handleSearch(query: string) {
if (query.trim()) {