diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index 106fb9dd..6e0ead5f 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -30,7 +30,7 @@ export default tseslint.config( allow: ["layout"], // Existing CSS treatments are allowed only on their owning components. contracts: [ - { pattern: "^Button$", allow: ["layout", "artist-hero-back"] }, + { pattern: "^Button$", allow: ["layout"] }, { pattern: "^CoverArt$", allow: ["layout", "card-art", "mini-art", "detail-art", "hero-art", "share-preview-art", "artist-hero-avatar"], diff --git a/frontend/src/components/library/AlbumSleeve.tsx b/frontend/src/components/library/AlbumSleeve.tsx index 1e1c27d1..d24a150f 100644 --- a/frontend/src/components/library/AlbumSleeve.tsx +++ b/frontend/src/components/library/AlbumSleeve.tsx @@ -7,7 +7,6 @@ import { } from "react"; import { Link, useSearchParams } from "react-router-dom"; import { - ArrowLeft as ArrowLeftIcon, Pencil as PencilIcon, Play as PlayIcon, Shuffle as ShuffleIcon, @@ -17,6 +16,7 @@ import { startListPlayback } from "@music-library/core/player/play-list"; import type { TrackListItem } from "../../api"; import { usePlayer, useRemotePlayback } from "../../context/Player"; import { displayText } from "../../lib/format"; +import { withoutLibraryDrill } from "../../lib/libraryDrill"; import { Button } from "../Button"; import CoverArt from "../CoverArt"; import SearchInput from "../SearchInput"; @@ -65,7 +65,6 @@ export interface AlbumSleeveSearch { */ export function AlbumSleeve({ kind, - onBack, album, loadingLabel, versions, @@ -79,7 +78,6 @@ export function AlbumSleeve({ children, }: { kind: ReactNode; - onBack: () => void; album: AlbumSleeveAlbum | null; /** Announced while `album` is null. */ loadingLabel: string; @@ -127,15 +125,6 @@ export function AlbumSleeve({ return (
- {album ? ( { - const next = new URLSearchParams(params); - for (const key of ["album", "tidalAlbum", "tidalArtist", "artistName"]) next.delete(key); + const next = withoutLibraryDrill(params); next.set("artist", artistId); return { pathname: "/library", search: `?${next}` }; }; diff --git a/frontend/src/components/shell/Breadcrumbs.tsx b/frontend/src/components/shell/Breadcrumbs.tsx index 57736d3d..bf0b3596 100644 --- a/frontend/src/components/shell/Breadcrumbs.tsx +++ b/frontend/src/components/shell/Breadcrumbs.tsx @@ -2,40 +2,53 @@ import { useMemo } from "react"; import { Link, useLocation, useParams } from "react-router-dom"; import { ChevronRight as ChevronRightIcon } from "lucide-react"; import type { Playlist } from "../../api"; +import { hasLibraryDrill, withoutLibraryDrill } from "../../lib/libraryDrill"; + +type Crumb = { label: string; to?: string; replace?: boolean; current?: boolean }; export default function Breadcrumbs({ playlists }: { playlists: Playlist[] }) { const location = useLocation(); const params = useParams(); const crumbs = useMemo( - () => buildCrumbs(location.pathname, params, playlists), - [location.pathname, params, playlists], + () => buildCrumbs(location.pathname, location.search, params, playlists), + [location.pathname, location.search, params, playlists], ); return ( -
+
+ ); } function buildCrumbs( pathname: string, + search: string, params: Record, playlists: Playlist[], -): { label: string; to?: string; current?: boolean }[] { +): Crumb[] { if (pathname === "/") return [{ label: "Home", current: true }]; if (pathname.startsWith("/library")) { + // On an album or artist page it's the way back to the list (and the + // search) the page was opened from, like Library's own back step. + const query = new URLSearchParams(search); + if (hasLibraryDrill(query)) { + const rest = withoutLibraryDrill(query).toString(); + return [{ label: "Library", to: rest ? `/library?${rest}` : "/library", replace: true }]; + } return [{ label: "Library", current: true }]; } if (pathname.startsWith("/favorites")) { diff --git a/frontend/src/lib/libraryDrill.ts b/frontend/src/lib/libraryDrill.ts new file mode 100644 index 00000000..2f705fef --- /dev/null +++ b/frontend/src/lib/libraryDrill.ts @@ -0,0 +1,14 @@ +/** The search params that open an album or artist page over the library list. */ +const DRILL_PARAMS = ["album", "artist", "tidalAlbum", "tidalArtist"] as const; + +/** An empty value (`?album=`) opens nothing, as on the Library page. */ +export function hasLibraryDrill(params: URLSearchParams): boolean { + return DRILL_PARAMS.some((key) => Boolean(params.get(key))); +} + +/** The library list (tab and search) the album or artist page was opened from. */ +export function withoutLibraryDrill(params: URLSearchParams): URLSearchParams { + const next = new URLSearchParams(params); + for (const key of [...DRILL_PARAMS, "artistName"]) next.delete(key); + return next; +} diff --git a/frontend/src/pages/Library.tsx b/frontend/src/pages/Library.tsx index 7d94c511..c3f47cc1 100644 --- a/frontend/src/pages/Library.tsx +++ b/frontend/src/pages/Library.tsx @@ -13,6 +13,7 @@ import { type TrackListItem, } from "../api"; import { displayText } from "../lib/format"; +import { withoutLibraryDrill } from "../lib/libraryDrill"; import type { EntityTarget } from "@music-library/core/entity-target"; import TrackList from "../components/TrackList"; import CoverArt from "../components/CoverArt"; @@ -158,11 +159,7 @@ export default function Library() { setParams(next); }; - const clearDrill = () => { - const next = new URLSearchParams(params); - for (const key of ["album", "artist", "tidalAlbum", "tidalArtist", "artistName"]) next.delete(key); - setParams(next, { replace: true }); - }; + const clearDrill = () => setParams(withoutLibraryDrill(params), { replace: true }); if (albumID) { return ; @@ -173,14 +170,13 @@ export default function Library() { key={tidalAlbumID} id={tidalAlbumID} artistId={tidalArtistID} - onBack={clearDrill} onOpenAlbum={openAlbum} onOpenVersion={openVersion} /> ); } if (tidalArtistID) { - return ; + return ; } if (artistID) { return ; diff --git a/frontend/src/pages/library/ArtistDetail.tsx b/frontend/src/pages/library/ArtistDetail.tsx index 9e10cc5a..30b6270b 100644 --- a/frontend/src/pages/library/ArtistDetail.tsx +++ b/frontend/src/pages/library/ArtistDetail.tsx @@ -1,6 +1,5 @@ import { useEffect, useMemo, useState, type ReactNode } from "react"; import { - ArrowLeft as ArrowLeftIcon, Shuffle as ArrowsRightLeftIcon, Pause as PauseIcon, Play as PlayIcon, @@ -75,21 +74,13 @@ export function ArtistDetailView({ return ; } // A failed load leaves both artist and tracks empty; without this the page - // would sit on the loading state with no way back. A failed refresh of a - // cached artist keeps the page and shows the error in it. + // would sit on the loading state for good. The Library crumb is the way + // back. A failed refresh of a cached artist keeps the page and shows the + // error in it. if (error && (!artist || !tracks)) { return (
-
- -
); } @@ -102,8 +93,6 @@ export function ArtistDetailView({ imageUrl={null} kind="Artist" meta={["Loading artist…"]} - backLabel="Library" - onBack={onBack} />
@@ -116,8 +105,6 @@ export function ArtistDetailView({ imageUrl={artistImageUrl(artist, tracks)} kind="Artist" meta={artistMetaParts(artist)} - backLabel="Library" - onBack={onBack} /> void; onOpenAlbum: (target: EntityTarget) => void; }) { // Keyed on the id by the caller, so this only has to seed a revisit. @@ -217,8 +202,6 @@ export function TidalArtistDetailView({ } meta={data ? tidalArtistMetaParts(releases.length, tracks.length) : undefined} - backLabel="Back to search" - onBack={onBack} /> {(error || warning) && ( @@ -261,8 +244,6 @@ function ArtistHero({ imageUrl, kind, meta, - backLabel, - onBack, }: { /** Null while loading: a bar in place of the name. */ name: string | null; @@ -270,23 +251,10 @@ function ArtistHero({ kind: ReactNode; /** Undefined while loading; the row keeps its height either way. */ meta?: (string | false)[]; - backLabel: string; - onBack: () => void; }) { const metaItems = meta?.filter((item): item is string => Boolean(item)); return (
-
- -
{name === null ? ( // No initial to show yet; a "?" would flash before the real one. diff --git a/frontend/src/pages/library/LibraryDetail.tsx b/frontend/src/pages/library/LibraryDetail.tsx index 73a6f959..15dc19d7 100644 --- a/frontend/src/pages/library/LibraryDetail.tsx +++ b/frontend/src/pages/library/LibraryDetail.tsx @@ -89,13 +89,11 @@ export function AlbumDetailView({ return ; } if (!sleeve || !album || !tracks) { - // A failed first load: the way back and the error, not a frame that - // would never fill. - if (error) return ; + // A failed first load: the error, not a frame that would never fill. + if (error) return ; return ( setEditing(true) : undefined} @@ -148,7 +145,6 @@ export function AlbumDetailView({ export function TidalAlbumDetailView({ id, artistId, - onBack, onOpenAlbum, onOpenVersion, }: { @@ -158,7 +154,6 @@ export function TidalAlbumDetailView({ * Without one, the album's own artist is asked once it loads. */ artistId: string | null; - onBack: () => void; onOpenAlbum: (target: EntityTarget) => void; /** * Swaps the page for another version of the release (a TIDAL album id), @@ -248,8 +243,7 @@ export function TidalAlbumDetailView({ if (!album || !sleeve) { if (error) { return ( -
- +
void }) { +/** A failed first load. The Library crumb is the way back. */ +function DetailLoadError({ error }: { error: string }) { return ( -
- +
); } -function DetailBackRow({ onBack }: { onBack: () => void }) { - return ( -
- -
- ); -} - export function NotFound({ kind, onBack, diff --git a/frontend/src/styles/artist.css b/frontend/src/styles/artist.css index 9dcfd146..2c0bdc30 100644 --- a/frontend/src/styles/artist.css +++ b/frontend/src/styles/artist.css @@ -16,24 +16,15 @@ container-type: inline-size; display: flex; flex-direction: column; - justify-content: space-between; - gap: 24px; + justify-content: center; min-height: 280px; - padding: 12px 24px 24px; + padding: 24px; border-radius: var(--radius-xl); border: 1px solid var(--border); background: var(--card); overflow: hidden; } -.btn.artist-hero-back { - color: var(--foreground); - background: transparent; - border-color: transparent; -} -.btn.artist-hero-back:hover { - background: var(--ghost-hover); -} .artist-hero-main { display: flex; align-items: flex-end; diff --git a/frontend/src/styles/detail.css b/frontend/src/styles/detail.css index 587a2d2b..1c584497 100644 --- a/frontend/src/styles/detail.css +++ b/frontend/src/styles/detail.css @@ -198,16 +198,10 @@ top: var(--sleeve-top, var(--pad)); display: grid; grid-template-columns: minmax(0, 1fr); - grid-template-areas: "back" "art" "head" "details"; + grid-template-areas: "art" "head" "details"; gap: 16px; min-width: 0; } -.album-sleeve-back { - grid-area: back; - justify-self: start; - /* The arrow lines up with the cover's edge. */ - margin: 0 0 -4px -10px; -} .album-sleeve-art { grid-area: art; width: 100%; @@ -399,7 +393,6 @@ position: static; grid-template-columns: 176px minmax(0, 1fr); grid-template-areas: - "back back" "art head" "details details"; column-gap: 24px; @@ -428,7 +421,7 @@ @media (max-width: 760px) { .album-sleeve[data-stacked] .album-sleeve-side { grid-template-columns: minmax(0, 1fr); - grid-template-areas: "back" "art" "head" "details"; + grid-template-areas: "art" "head" "details"; align-items: stretch; } .album-sleeve[data-stacked] .album-sleeve-art { diff --git a/frontend/tests/album-sleeve.test.tsx b/frontend/tests/album-sleeve.test.tsx index 3b3cd9f3..a0e5bb91 100644 --- a/frontend/tests/album-sleeve.test.tsx +++ b/frontend/tests/album-sleeve.test.tsx @@ -43,7 +43,6 @@ function sleeve(props: SleeveProps = {}) {
{}} album={album} loadingLabel="Loading album…" search={search} diff --git a/frontend/tests/artist-play.test.tsx b/frontend/tests/artist-play.test.tsx index 9037a036..4f5393b0 100644 --- a/frontend/tests/artist-play.test.tsx +++ b/frontend/tests/artist-play.test.tsx @@ -47,7 +47,7 @@ afterEach(() => { async function show(tracks: TrackListItem[]) { mock.artist.mockResolvedValueOnce({ albums: [], tracks }); render( - {}} onOpenAlbum={() => {}} />, + {}} />, ); await act(async () => {}); } diff --git a/frontend/tests/breadcrumbs.test.tsx b/frontend/tests/breadcrumbs.test.tsx new file mode 100644 index 00000000..c098cbff --- /dev/null +++ b/frontend/tests/breadcrumbs.test.tsx @@ -0,0 +1,48 @@ +import { cleanup, fireEvent, render, screen, within } from "@testing-library/react"; +import { afterEach, expect, it } from "vitest"; +import { MemoryRouter, useLocation, useNavigationType } from "react-router-dom"; +import Breadcrumbs from "../src/components/shell/Breadcrumbs"; + +afterEach(cleanup); + +function Probe() { + const location = useLocation(); + const type = useNavigationType(); + return {`${type} ${location.pathname}${location.search}`}; +} + +function show(entries: string[]) { + render( + + + + , + ); +} + +it.each([ + ["/library?view=albums&q=rec&album=al-1", "/library?view=albums&q=rec"], + ["/library?artist=ar-1", "/library"], + ["/library?q=x&type=artist&tidalArtist=ta-1&artistName=Band", "/library?q=x&type=artist"], + ["/library?tidalArtist=ta-1&tidalAlbum=tal-1", "/library"], +])("on %s the Library crumb links back to %s", (entry, href) => { + show([entry]); + const trail = screen.getByRole("navigation", { name: "Breadcrumb" }); + expect(within(trail).getByRole("link", { name: "Library" }).getAttribute("href")).toBe(href); +}); + +it.each(["/library", "/library?view=albums&q=rec", "/library?view=albums&album="])( + "on the list (%s) Library is the current crumb, not a link", + (entry) => { + show([entry]); + const trail = screen.getByRole("navigation", { name: "Breadcrumb" }); + expect(within(trail).queryByRole("link", { name: "Library" })).toBeNull(); + expect(within(trail).getByText("Library").getAttribute("aria-current")).toBe("page"); + }, +); + +it("goes back to the list in place, so browser Back skips the album page", () => { + show(["/home", "/library?q=rec&album=al-1"]); + fireEvent.click(screen.getByRole("link", { name: "Library" })); + expect(screen.getByRole("status", { name: "where" }).textContent).toBe("REPLACE /library?q=rec"); +}); diff --git a/frontend/tests/library-artist.test.tsx b/frontend/tests/library-artist.test.tsx index f35cc7bd..61f2b7f2 100644 --- a/frontend/tests/library-artist.test.tsx +++ b/frontend/tests/library-artist.test.tsx @@ -1,4 +1,4 @@ -import { act, cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { act, cleanup, render, screen } from "@testing-library/react"; import { afterEach, expect, it, vi } from "vitest"; import { ArtistDetailView } from "../src/pages/library/ArtistDetail"; @@ -15,14 +15,11 @@ vi.mock("../src/context/Player", () => ({ afterEach(cleanup); -it("shows a failed load with a way back instead of loading forever", async () => { +it("shows a failed load instead of loading forever", async () => { mock.getArtist.mockRejectedValueOnce(new Error("upstream")); mock.listArtistTracks.mockResolvedValueOnce([]); - const onBack = vi.fn(); - render( {}} />); + render( {}} onOpenAlbum={() => {}} />); await act(async () => {}); expect(screen.getByRole("alert")).toBeTruthy(); expect(screen.queryByText("Loading library…")).toBeNull(); - fireEvent.click(screen.getByRole("button", { name: "Back to library" })); - expect(onBack).toHaveBeenCalledOnce(); }); diff --git a/frontend/tests/search-types.test.tsx b/frontend/tests/search-types.test.tsx index d8a33185..791473ee 100644 --- a/frontend/tests/search-types.test.tsx +++ b/frontend/tests/search-types.test.tsx @@ -8,6 +8,7 @@ import { import { afterEach, beforeEach, expect, it, vi } from "vitest"; import { MemoryRouter } from "react-router-dom"; import type { Page, SearchResult } from "../src/api"; +import Breadcrumbs from "../src/components/shell/Breadcrumbs"; import Library from "../src/pages/Library"; const mock = vi.hoisted(() => ({ @@ -35,18 +36,7 @@ vi.mock("../src/pages/library/LibraryDetail", () => ({ ), ArtistDetailView: () => null, - TidalAlbumDetailView: ({ - id, - onBack, - }: { - id: string; - onBack: () => void; - }) => ( - <> - Remote album {id} - - - ), + TidalAlbumDetailView: ({ id }: { id: string }) => Remote album {id}, })); beforeEach(() => { @@ -150,16 +140,19 @@ it("opens remote albums and keeps the query and filter when returning", async () mock.search.mockResolvedValue(page([album])); render( + , ); await act(async () => {}); + // The list itself is the current crumb, not a link. + expect(screen.queryByRole("link", { name: "Library" })).toBeNull(); await act(async () => { fireEvent.click(screen.getByRole("button", { name: /Found album/ })); }); expect(screen.getByText("Remote album 42")).toBeTruthy(); await act(async () => { - fireEvent.click(screen.getByRole("button", { name: "Back to results" })); + fireEvent.click(screen.getByRole("link", { name: "Library" })); }); expect(screen.getByRole("searchbox").getAttribute("value")).toBe("hello"); expect( diff --git a/frontend/tests/tidal-album-actions.test.tsx b/frontend/tests/tidal-album-actions.test.tsx index c3cf11bf..dc2880be 100644 --- a/frontend/tests/tidal-album-actions.test.tsx +++ b/frontend/tests/tidal-album-actions.test.tsx @@ -46,7 +46,6 @@ async function show(data: TidalAlbum, artistId: string | null = null) { {}} onOpenAlbum={onOpenAlbum} onOpenVersion={onOpenVersion} /> diff --git a/frontend/tests/tidal-artist.test.tsx b/frontend/tests/tidal-artist.test.tsx index 988ae053..87e5980a 100644 --- a/frontend/tests/tidal-artist.test.tsx +++ b/frontend/tests/tidal-artist.test.tsx @@ -43,7 +43,6 @@ const show = async () => { {}} onOpenAlbum={() => {}} />, ); @@ -217,7 +216,7 @@ it("opens a release's TIDAL page by its TIDAL id", async () => { ], }); const onOpenAlbum = vi.fn(); - render( {}} onOpenAlbum={onOpenAlbum} />); + render(); await act(async () => {}); fireEvent.click(screen.getByRole("button", { name: /^LP/ })); fireEvent.click(screen.getByRole("button", { name: /^EP/ }));