diff --git a/src/lib/utils/searchScope.test.ts b/src/lib/utils/searchScope.test.ts index 96b52110..81e822a1 100644 --- a/src/lib/utils/searchScope.test.ts +++ b/src/lib/utils/searchScope.test.ts @@ -7,6 +7,8 @@ import { normalizeGroupOrder, reorderGroups, resolveSearchScope, + searchRouteUrl, + shouldNavigateToSearch, scopeItemTypes, type SearchGroupId, } from "./searchScope"; @@ -370,4 +372,42 @@ describe("reorderGroups", () => { expect(reorderGroups(order, -1, 2)).toEqual(order); expect(reorderGroups(order, 0, 9)).toEqual(order); }); -}); \ No newline at end of file +}); + +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); + }); +}); diff --git a/src/lib/utils/searchScope.ts b/src/lib/utils/searchScope.ts index f1e57214..7bbc9164 100644 --- a/src/lib/utils/searchScope.ts +++ b/src/lib/utils/searchScope.ts @@ -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 // --------------------------------------------------------------------------- diff --git a/src/routes/library/+layout.svelte b/src/routes/library/+layout.svelte index 82ab9142..57db9bbe 100644 --- a/src/routes/library/+layout.svelte +++ b/src/routes/library/+layout.svelte @@ -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 = ""; } } @@ -74,14 +82,12 @@ {#snippet librarySearch()} + - {#if searchQuery.trim()} - - {/if} {/snippet} -
-
-

- Search results for "{searchQuery}" -

- -
- - -
- {:else if showInlineLibraryContent} + {#if showInlineLibraryContent}
diff --git a/src/routes/search/+page.svelte b/src/routes/search/+page.svelte index fcffbef5..a932299a 100644 --- a/src/routes/search/+page.svelte +++ b/src/routes/search/+page.svelte @@ -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(resolveSearchScope($page.url.pathname)); + let scope = $state( + 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()) {