From e32f2c576606e4f8962bb1fa2fe42c9febecf192 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 26 Oct 2026 01:52:06 +0100 Subject: [PATCH 1/2] feat(webapp): link each credited artist from song lists and the track menu A playlist, a mix or any song list showed the artist as plain text, so no list could lead to an artist. The song table now links every credited artist from the structured list the server already sent, and the track menu offers Go to album and Go to each artist, which is how a phone reaches them: its song table hides the artist column. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 107 +++++++++++++++++++++++--- webapp/src/api/catalog.ts | 7 ++ webapp/src/components/song-table.tsx | 30 +++++++- webapp/src/components/track-menu.tsx | 28 +++++++ webapp/src/lib/artist-credits.test.ts | 42 ++++++++++ webapp/src/lib/artist-credits.ts | 21 +++++ webapp/src/state/i18n.tsx | 4 + webapp/src/styles/styles.css | 8 ++ 8 files changed, 237 insertions(+), 10 deletions(-) create mode 100644 webapp/src/lib/artist-credits.test.ts create mode 100644 webapp/src/lib/artist-credits.ts diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 4e141d47..bbda32a0 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -2094,6 +2094,93 @@ test("puts the songs playback action beside the sort and names its scope", async await expect(page.locator(".player-track strong")).toHaveText("Army of Me"); }); +/** + * Every credited artist in a song list is a link to its own page. + * + * The column used to be the display string as plain text, so a playlist or a + * mix offered no way to reach an artist. A track credited to two artists names + * two pages; a track whose credits the server did not list still links through + * its primary credit; an artist the catalogue does not know stays text. + */ +test("links each credited artist in a song list to its page", async ({ + page, +}, testInfo) => { + await page.route("**/api/v2/songs?*", (route) => + route.fulfill({ + json: [ + { + ...track, + id: "song-duo", + title: "Soleil Bleu", + artist: "Bleu Soleil, LUIZA", + artist_id: "artist-1", + artists: [ + { id: "artist-1", name: "Bleu Soleil" }, + { id: "artist-2", name: "LUIZA" }, + ], + }, + { + ...track, + id: "song-primary", + title: "Second track", + artist: "Björk", + artist_id: "artist-1", + artists: [], + }, + { + ...track, + id: "song-unknown", + title: "Third track", + artist: "Nobody We Know", + artist_id: null, + artists: [], + }, + ], + }), + ); + await page.goto("/songs"); + const rows = page.locator(".songs tbody tr"); + await expect(rows).toHaveCount(3); + + // Every layout: the track's menu names each credited artist, which is how a + // phone, whose song table hides the artist column, reaches one. + await rows.nth(0).locator("button.song-title").focus(); + await page.keyboard.press("Shift+F10"); + const menu = page.getByRole("menu", { name: "Actions for Soleil Bleu" }); + await expect(menu.getByRole("menuitem", { name: /^Go to / })).toHaveText([ + "Go to album", + "Go to Bleu Soleil", + "Go to LUIZA", + ]); + await menu.getByRole("menuitem", { name: "Go to LUIZA" }).click(); + await expect(page).toHaveURL(/\/artists\/artist-2$/); + await page.goBack(); + await expect(rows).toHaveCount(3); + + // The column itself is drawn on a wide screen only. + if (testInfo.project.name !== "desktop") return; + const duo = rows.nth(0).locator(".song-artists"); + await expect(duo.getByRole("link")).toHaveCount(2); + await expect(duo).toHaveText("Bleu Soleil, LUIZA"); + await expect(duo.getByRole("link", { name: "LUIZA" })).toHaveAttribute( + "href", + "/artists/artist-2", + ); + + await expect( + rows.nth(1).locator(".song-artists").getByRole("link", { name: "Björk" }), + ).toHaveAttribute("href", "/artists/artist-1"); + await expect(rows.nth(2).locator(".song-artists").getByRole("link")).toHaveCount( + 0, + ); + await expect(rows.nth(2).locator(".song-artists")).toHaveText("Nobody We Know"); + + await expectNoWcagViolations(page); + + await duo.getByRole("link", { name: "Bleu Soleil" }).click(); + await expect(page).toHaveURL(/\/artists\/artist-1$/); +}); + /** * The right click on a track, and the properties it opens (RFC-012 decision 4). * @@ -2141,6 +2228,9 @@ test("opens a track's properties from its context menu", async ({ page }) => { "Download", // RFC-022: this browser supports Cache Storage, so it offers the copy. "Keep for offline", + // Where the track belongs, reachable from any list and on a phone. + "Go to album", + "Go to Skunk Anansie", "Properties", "Edit tags", "Move to folder…", @@ -2151,11 +2241,11 @@ test("opens a track's properties from its context menu", async ({ page }) => { "outline-style", "solid", ); - // Past "Keep for offline" (RFC-022), the fourth item. - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); + // Past "Keep for offline" (RFC-022) and the album and artist entries, the + // seventh item. + for (let press = 0; press < 6; press += 1) { + await page.keyboard.press("ArrowDown"); + } await expect(menu.getByRole("menuitem", { name: "Properties" })).toBeFocused(); await page.keyboard.press("Enter"); @@ -2182,10 +2272,9 @@ test("opens a track's properties from its context menu", async ({ page }) => { // The backdrop, outside the panel's box, does close it. await page.keyboard.press("Shift+F10"); - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); - await page.keyboard.press("ArrowDown"); + for (let press = 0; press < 6; press += 1) { + await page.keyboard.press("ArrowDown"); + } await page.keyboard.press("Enter"); await expect(panel).toBeVisible(); await page.mouse.click(2, 2); diff --git a/webapp/src/api/catalog.ts b/webapp/src/api/catalog.ts index e50e75fd..51498825 100644 --- a/webapp/src/api/catalog.ts +++ b/webapp/src/api/catalog.ts @@ -26,6 +26,13 @@ export type Song = { album: string | null; artist: string | null; artist_id: string | null; + /** + * Every credited artist, in tag order: the structured form of `artist`, + * which stays the display string. On the wire since `SongItem` gained it; + * declared here so each name can link to its own page. Optional, so a mock + * that leaves it out is a track credited by its display string alone. + */ + artists?: Array<{ id: string; name: string }>; artwork_hash: string | null; duration_ms: number; track: number | null; diff --git a/webapp/src/components/song-table.tsx b/webapp/src/components/song-table.tsx index 2e729aa4..d89ac745 100644 --- a/webapp/src/components/song-table.tsx +++ b/webapp/src/components/song-table.tsx @@ -3,6 +3,7 @@ import { Link } from "@tanstack/react-router"; import { type ReactNode, useState } from "react"; import { formatDuration, type Song, setFavorite, setRating } from "../api"; +import { creditedArtists } from "../lib/artist-credits"; import { usePlayer } from "../player/player"; import { useI18n } from "../state/i18n"; import { mayCorrectTracks, useLibraryScope } from "../state/library-scope"; @@ -12,6 +13,31 @@ import { useTrackMenu } from "./track-menu"; const RATING_STARS = [1, 2, 3, 4, 5]; +/** + * A track's artists, each a link to its own page. + * + * A track credited to two artists ("Bleu Soleil, LUIZA") names two pages, so + * the structured list is what is linked, in tag order, rather than the display + * string. Where the list is empty, the primary credit still links if it has an + * id, and an artist the catalogue does not know stays plain text. + */ +function ArtistLinks({ song }: { song: Song }) { + const credited = creditedArtists(song); + if (credited.length === 0) return <>{song.artist}; + return ( + <> + {credited.map((artist, index) => ( + + {index > 0 ? ", " : null} + + {artist.name} + + + ))} + + ); +} + /** * What a dragged row carries: the track ids, as JSON. A type of its own, so a * folder takes a drop from a song row and nothing else. @@ -224,7 +250,9 @@ export function SongTable({ {variant === "default" ? ( - {song.artist} + + + ) : null} {formatDuration(song.duration_ms)} diff --git a/webapp/src/components/track-menu.tsx b/webapp/src/components/track-menu.tsx index 3d269d6e..620ff01f 100644 --- a/webapp/src/components/track-menu.tsx +++ b/webapp/src/components/track-menu.tsx @@ -12,6 +12,7 @@ import { import { createPortal } from "react-dom"; import { placeTracks, type Song, streamUrl } from "../api"; +import { creditedArtists } from "../lib/artist-credits"; import { downloadUrl, saveFrom } from "../lib/download"; import { forgetOffline, @@ -429,6 +430,33 @@ function TrackMenu({ }, ] : []), + // Where the track belongs, from every list it appears in. The artist + // column of a song table is hidden on a narrow screen, so this is how a + // phone reaches an artist from a playlist or a mix. + ...(song.album_id + ? [ + { + key: "album", + icon: "albums" as const, + label: t("trackMenu.goToAlbum"), + run: () => + void navigate({ + to: "/albums/$albumId", + params: { albumId: song.album_id as string }, + }), + }, + ] + : []), + ...creditedArtists(song).map((artist) => ({ + key: `artist-${artist.id}`, + icon: "artists" as const, + label: t("trackMenu.goToArtist", { name: artist.name }), + run: () => + void navigate({ + to: "/artists/$artistId", + params: { artistId: artist.id }, + }), + })), { key: "properties", icon: "info", diff --git a/webapp/src/lib/artist-credits.test.ts b/webapp/src/lib/artist-credits.test.ts new file mode 100644 index 00000000..82adf1aa --- /dev/null +++ b/webapp/src/lib/artist-credits.test.ts @@ -0,0 +1,42 @@ +import { describe, expect, it } from "vitest"; + +import { creditedArtists } from "./artist-credits"; + +describe("creditedArtists", () => { + it("uses the structured list, in tag order, when the server sends one", () => { + expect( + creditedArtists({ + artist: "Bleu Soleil, LUIZA", + artist_id: "artist-1", + artists: [ + { id: "artist-1", name: "Bleu Soleil" }, + { id: "artist-2", name: "LUIZA" }, + ], + }), + ).toEqual([ + { id: "artist-1", name: "Bleu Soleil" }, + { id: "artist-2", name: "LUIZA" }, + ]); + }); + + it("falls back to the primary credit when the list is missing or empty", () => { + const primary = [{ id: "artist-1", name: "Björk" }]; + expect(creditedArtists({ artist: "Björk", artist_id: "artist-1" })).toEqual( + primary, + ); + expect( + creditedArtists({ artist: "Björk", artist_id: "artist-1", artists: [] }), + ).toEqual(primary); + }); + + it("links nothing for an artist the catalogue does not know", () => { + expect( + creditedArtists({ + artist: "Nobody We Know", + artist_id: null, + artists: [], + }), + ).toEqual([]); + expect(creditedArtists({ artist: null, artist_id: null })).toEqual([]); + }); +}); diff --git a/webapp/src/lib/artist-credits.ts b/webapp/src/lib/artist-credits.ts new file mode 100644 index 00000000..6c8c8902 --- /dev/null +++ b/webapp/src/lib/artist-credits.ts @@ -0,0 +1,21 @@ +import type { Song } from "../api"; + +/** One artist a track can link to. */ +export type CreditedArtist = { id: string; name: string }; + +/** + * The artists a track can link to, in tag order. + * + * The structured list the server sends, where it has one. Otherwise the + * primary credit, if the catalogue knows it by an id, and nothing at all for + * an artist it does not know: a name with no page behind it is not a link. + */ +export function creditedArtists( + song: Pick, +): CreditedArtist[] { + if (song.artists && song.artists.length > 0) return song.artists; + if (song.artist_id && song.artist) { + return [{ id: song.artist_id, name: song.artist }]; + } + return []; +} diff --git a/webapp/src/state/i18n.tsx b/webapp/src/state/i18n.tsx index 4e43c976..04b1db8c 100644 --- a/webapp/src/state/i18n.tsx +++ b/webapp/src/state/i18n.tsx @@ -132,6 +132,8 @@ const en = { "trackMenu.forgetOffline": "Remove the offline copy", "trackMenu.downloadFailed": "The download could not start. Try again.", "trackMenu.dismiss": "Dismiss", + "trackMenu.goToAlbum": "Go to album", + "trackMenu.goToArtist": "Go to {name}", "trackMenu.properties": "Properties", "trackMenu.edit": "Edit tags", "properties.kicker": "Properties", @@ -1264,6 +1266,8 @@ const fr: Record = { "trackMenu.downloadFailed": "Le téléchargement n’a pas pu démarrer. Réessayez.", "trackMenu.dismiss": "Fermer", + "trackMenu.goToAlbum": "Aller à l’album", + "trackMenu.goToArtist": "Aller à {name}", "trackMenu.properties": "Propriétés", "trackMenu.edit": "Corriger les tags", "properties.kicker": "Propriétés", diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index 1941b341..c713a613 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -1192,6 +1192,14 @@ main { vertical-align: middle; } +/* Each credited artist is a link; at rest it reads as the muted text it was, + and says it can be followed only when pointed at or focused. */ +.song-artists a:hover, +.song-artists a:focus-visible { + color: var(--text); + text-decoration: underline; +} + .songs .index { width: 2.5rem; color: var(--muted); From 49a0fa0e5d6af10893bdc6f9965d315e2952935c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 26 Oct 2026 02:07:08 +0100 Subject: [PATCH 2/2] fix(webapp): cap a long track menu at the window and let it scroll One Go-to entry per credited artist can make the menu taller than a short window, leaving its last entries off screen. It is now capped at the window and scrolls on its own; the wheel over a menu that has rows to scroll scrolls it instead of closing it, and the wheel elsewhere still closes it. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 60 ++++++++++++++++++++++++++++ webapp/src/components/track-menu.tsx | 15 +++++-- webapp/src/styles/styles.css | 5 +++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index bbda32a0..64717012 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -2181,6 +2181,66 @@ test("links each credited artist in a song list to its page", async ({ await expect(page).toHaveURL(/\/artists\/artist-1$/); }); +/** + * A track menu taller than the window stays on screen and scrolls itself. + * + * One "Go to" entry per credited artist means a compilation track can carry a + * menu longer than a short window. It is capped at the window and scrolls on + * its own; the wheel over it scrolls it rather than closing it, and the wheel + * over the page still closes it. + */ +test("keeps a long track menu on screen and lets it scroll", async ({ + page, +}) => { + await page.setViewportSize({ width: 900, height: 420 }); + const many = Array.from({ length: 30 }, (_, index) => ({ + id: `artist-${index + 1}`, + name: `Guest ${index + 1}`, + })); + await page.route("**/api/v2/songs?*", (route) => + route.fulfill({ + json: [ + { + ...track, + id: "song-crowd", + title: "Crowded", + artist: many.map((artist) => artist.name).join(", "), + artist_id: "artist-1", + artists: many, + }, + ], + }), + ); + await page.goto("/songs"); + await page.locator(".songs tbody tr button.song-title").first().focus(); + await page.keyboard.press("Shift+F10"); + const menu = page.getByRole("menu", { name: "Actions for Crowded" }); + await expect(menu).toBeVisible(); + + const box = await menu.boundingBox(); + expect(box).not.toBeNull(); + expect((box?.y ?? 0) + (box?.height ?? 0)).toBeLessThanOrEqual(420); + expect(await menu.evaluate((node) => node.scrollHeight > node.clientHeight)).toBe( + true, + ); + + // The wheel over the menu scrolls it and leaves it open. + await menu.hover(); + await page.mouse.wheel(0, 400); + await expect + .poll(() => menu.evaluate((node) => node.scrollTop)) + .toBeGreaterThan(0); + await expect(menu).toBeVisible(); + const last = menu.getByRole("menuitem", { name: "Go to Guest 30" }); + await last.scrollIntoViewIfNeeded(); + await expect(last).toBeInViewport(); + + // The wheel anywhere else still closes it. + await page.mouse.move(880, 400); + await page.mouse.wheel(0, 100); + await expect(menu).toHaveCount(0); +}); + /** * The right click on a track, and the properties it opens (RFC-012 decision 4). * diff --git a/webapp/src/components/track-menu.tsx b/webapp/src/components/track-menu.tsx index 620ff01f..c3b57844 100644 --- a/webapp/src/components/track-menu.tsx +++ b/webapp/src/components/track-menu.tsx @@ -342,10 +342,17 @@ function TrackMenu({ const away = (event: Event) => { if (!list.current?.contains(event.target as Node)) closeRef.current(); }; - // The wheel closes it wherever it turns, over the menu too: the menu does - // not scroll, so a wheel over it scrolls the page beneath and leaves the - // menu where its row used to be. - const wheel = () => closeRef.current(); + // The wheel closes it, over the menu too, unless the menu has rows to + // scroll: a track credited to many artists makes it taller than a short + // window, and it is then capped and scrolls on its own. A menu with nothing + // to scroll would hand the wheel to the page beneath and leave the menu + // where its row used to be, so that one still closes. + const wheel = (event: Event) => { + const menu = list.current; + const inside = menu?.contains(event.target as Node) ?? false; + if (inside && menu && menu.scrollHeight > menu.clientHeight) return; + closeRef.current(); + }; const leave = () => closeRef.current(); // A long press opens the menu with the finger still down, and that finger // drifts as it lifts: its moves are the gesture that opened the menu, not diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index c713a613..393bb08d 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -2848,6 +2848,11 @@ button.danger { z-index: 40; display: grid; min-width: 13rem; + /* A track credited to many artists lists one entry each: capped at the + window, the menu scrolls on its own rather than running off the screen. */ + max-height: calc(100dvh - 2rem); + overflow-y: auto; + overscroll-behavior: contain; padding: var(--space-1); border: 1px solid var(--line-strong); border-radius: 0.75rem;