From 7380209dbc901af9fdf5a72abf3ef08d566d4c42 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sun, 25 Oct 2026 22:55:41 +0100 Subject: [PATCH 1/4] feat(webapp): let the lyrics strip follow the sung line and seek from it The strip beside the cover marked the sung line but left it to scroll out of its own box, and no line could be pressed. It now shares the lyrics view's follow hook and line list: the sung line is held in view, and a synced line is a button that seeks to it. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 72 ++++++++++ webapp/src/pages/playing-page.tsx | 224 +++++++++++++++++------------ webapp/src/styles/styles.css | 5 +- 3 files changed, 205 insertions(+), 96 deletions(-) diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 8681e3ca..8cb383e7 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -4999,6 +4999,78 @@ test("offers no seeking on a sheet that carries no timings", async ({ await expect(view.locator("[aria-current]")).toHaveCount(0); }); +/** + * The strip beside the cover reads the way the lyrics view does. + * + * It used to mark the sung line and leave it to scroll out of its own box, and + * no line could be pressed. Now it holds the sung line in view, and a synced + * line is a button that seeks to it. A sheet without timings still gets no + * buttons. + */ +test("follows the sung line in the strip beside the cover, and seeks from it", async ({ + page, +}) => { + lyricSheets.set("song-1", [ + { + displayArtist: "Rue Delacour", + displayTitle: "Nocturne", + lang: "eng", + synced: true, + line: Array.from({ length: 40 }, (_, index) => ({ + start: index * 5_000, + value: `strip line ${index}`, + })), + }, + ]); + + await page.goto("/playing"); + const strip = page.locator(".playing .lyrics"); + const column = strip.locator("ol"); + await expect(column).toBeVisible(); + // Every timed line is a button, named by its words and then the time. + await expect(strip.getByRole("button")).toHaveCount(40); + await expect( + strip.getByRole("button", { + name: "strip line 1, jump to 0:05", + exact: true, + }), + ).toBeVisible(); + await expect.poll(() => column.evaluate((list) => list.scrollTop)).toBe(0); + + // The head moves well down the sheet: the sung line is marked, and the strip + // has scrolled itself to it rather than leaving it below its own fold. + for (let press = 0; press < 20; press += 1) { + await page.keyboard.press("ArrowRight"); + } + await expect( + strip.getByRole("listitem").filter({ hasText: "strip line 20" }), + ).toHaveAttribute("aria-current", "true"); + await expect + .poll(() => column.evaluate((list) => list.scrollTop)) + .toBeGreaterThan(0); + // The strip scrolls itself and nothing else: the page stays where it was. + expect(await page.evaluate(() => window.scrollY)).toBe(0); + + await expectNoWcagViolations(page); + + // Without timings, the strip offers no seeking, as the lyrics view does not. + lyricSheets.set("song-1", [ + { + displayArtist: null, + displayTitle: "Nocturne", + lang: "eng", + synced: false, + line: [ + { value: "the lamps come on along the quay" }, + { value: "and the water keeps the light" }, + ], + }, + ]); + await page.reload(); + await expect(strip.getByRole("listitem")).toHaveCount(2); + await expect(strip.getByRole("button")).toHaveCount(0); +}); + /** * The immersive view, and the two things that make it a mode rather than a trap. * diff --git a/webapp/src/pages/playing-page.tsx b/webapp/src/pages/playing-page.tsx index 495552cb..7f1b4a72 100644 --- a/webapp/src/pages/playing-page.tsx +++ b/webapp/src/pages/playing-page.tsx @@ -1,6 +1,12 @@ import { useQuery } from "@tanstack/react-query"; import { Link, useNavigate, useSearch } from "@tanstack/react-router"; -import { useLayoutEffect, useMemo, useRef, useState } from "react"; +import { + type RefObject, + useLayoutEffect, + useMemo, + useRef, + useState, +} from "react"; import { deleteBookmark, @@ -146,66 +152,23 @@ function useLyricSheet(trackId: string) { }; } -/** The words beside the cover, as the detail view has always shown them. */ -function LyricsStrip({ trackId }: { trackId: string }) { - const { t } = useI18n(); - const { sheet, active, keys, state } = useLyricSheet(trackId); - - if (state === "waiting") - return

{t("common.loading")}

; - if (state === "none" || !sheet) - return

{t("lyrics.none")}

; - return ( -
-

- {sheet.displayTitle} - {sheet.displayArtist ? {sheet.displayArtist} : null} -

-
    - {sheet.line.map((line, index) => ( -
  1. - {line.value || BLANK_LINE} -
    2. - ))} -
- {sheet.source === "lrclib" ? ( -

- {t("lyrics.fromLrclib")} -

- ) : null} -
- ); -} - /** - * The words as the subject of the screen. + * Holds the sung line in the middle of `column`, and only `column`. * - * Two things the strip cannot do, and they are why this view exists rather - * than being the strip with a larger font. It **holds the sung line in view**, - * so a listener reads along without touching anything — which is the whole - * point of a server that supplies timings. And where those timings exist, a - * line is a **button that seeks to it**: the sheet becomes a way to move - * through the track, not only a thing to read. - * - * A sheet without timings gets neither, and says so by having no buttons at - * all. Rendering an unsynced line as something pressable would promise a seek - * the sheet cannot compute. + * Shared by the strip beside the cover and the lyrics view, so the two follow + * the words the same way: the strip used to bold the sung line and leave it to + * scroll out of its own box, which marked a line nobody could see. */ -function LyricsView({ song }: { song: Song }) { - const { t } = useI18n(); - const player = usePlayer(); +function useFollowSungLine( + column: RefObject, + sheet: StructuredLyrics | undefined, + active: number, +) { const reduced = usePrefersReducedMotion(); - const { sheet, active, keys, state } = useLyricSheet(song.id); - const column = useRef(null); - // **Before paint, and only when there is something to do.** `useEffect` // would let the new line show at the bottom of the column for a frame before // the scroll caught up. And a progress tick that stays on the same line - // changes none of the three dependencies, so it costs nothing. + // changes none of the dependencies, so it costs nothing. useLayoutEffect(() => { // `sheet` is read here and not only named below: there is no column // without one, and a dependency the body never touches is a decorative @@ -230,8 +193,8 @@ function LyricsView({ song }: { song: Song }) { (lineBox.top - listBox.top) - (listBox.height - lineBox.height) / 2, // A sheet that jumps rather than glides, for whoever asked for less - // motion. The following itself is not decoration — it is how the view - // works — so it is the gliding that goes, not the following. + // motion. The following itself is not decoration — it is how the words + // are read — so it is the gliding that goes, not the following. behavior: reduced ? "auto" : "smooth", }); // `sheet` as well as `active`, for the track change where the two @@ -240,7 +203,115 @@ function LyricsView({ song }: { song: Song }) { // the previous track's scroll and the opening line of the new one is above // the fold. The object is react-query's and stable between renders, so // naming it costs no extra run. - }, [active, reduced, sheet]); + }, [active, column, reduced, sheet]); +} + +/** + * A sheet's lines, the sung one marked. + * + * Where the sheet carries timings, each line is a **button that seeks to it**, + * in the strip as in the lyrics view. A sheet without timings gets no buttons + * at all: rendering an unsynced line as something pressable would promise a + * seek the sheet cannot compute. + */ +function LyricLines({ + sheet, + active, + keys, + column, +}: { + sheet: StructuredLyrics; + active: number; + keys: string[]; + column: RefObject; +}) { + const { t } = useI18n(); + const player = usePlayer(); + return ( +
    + {sheet.line.map((line, index) => { + const at = line.start; + const on = index === active; + return ( +
  1. + {sheet.synced && at !== undefined ? ( + + ) : ( + line.value || BLANK_LINE + )} +
    2. + ); + })} +
+ ); +} + +/** + * The words beside the cover, in the detail view. + * + * Smaller than the lyrics view and inside a box of its own, but reading the + * same way: the sung line is held in view and, where the sheet is synced, a + * line seeks to itself. + */ +function LyricsStrip({ trackId }: { trackId: string }) { + const { t } = useI18n(); + const { sheet, active, keys, state } = useLyricSheet(trackId); + const column = useRef(null); + useFollowSungLine(column, sheet, active); + + if (state === "waiting") + return

{t("common.loading")}

; + if (state === "none" || !sheet) + return

{t("lyrics.none")}

; + return ( +
+

+ {sheet.displayTitle} + {sheet.displayArtist ? {sheet.displayArtist} : null} +

+ + {sheet.source === "lrclib" ? ( +

+ {t("lyrics.fromLrclib")} +

+ ) : null} +
+ ); +} + +/** + * The words as the subject of the screen: one wide column, centred, with the + * names and the hint above it. It follows and seeks as the strip does; what it + * adds is the room. + */ +function LyricsView({ song }: { song: Song }) { + const { t } = useI18n(); + const { sheet, active, keys, state } = useLyricSheet(song.id); + const column = useRef(null); + useFollowSungLine(column, sheet, active); // Inside the view's own frame, like the empty state below it: a column that // appears out of nowhere once the words land is a jump the wrapper absorbs. @@ -275,44 +346,7 @@ function LyricsView({ song }: { song: Song }) {

) : null} -
    - {sheet.line.map((line, index) => { - const at = line.start; - const on = index === active; - return ( -
  1. - {sheet.synced && at !== undefined ? ( - - ) : ( - line.value || BLANK_LINE - )} -
    2. - ); - })} -
+ ); } diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index 825d2ebf..ca31cf51 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -2096,7 +2096,9 @@ button.danger { } /* A seekable line is a button and has to look like the text it is, not like a - control: what makes it pressable is already said by its label. */ + control: what makes it pressable is already said by its label. The strip + beside the cover seeks the same way, so it shares the rule. */ +.lyrics .link, .lyrics-view .link { display: block; width: 100%; @@ -2109,6 +2111,7 @@ button.danger { cursor: pointer; } +.lyrics .link:hover, .lyrics-view .link:hover { color: var(--text); text-decoration: underline; From df7012a7be62b4d4cc5c0e49865c0470bcf2c9f3 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sun, 25 Oct 2026 22:57:08 +0100 Subject: [PATCH 2/4] feat(webapp): show covers and portraits in the statistics lists The top tracks, albums and artists named only their titles. The server already sends each one's artwork, so each entry now leads with its cover, the artist's as a round portrait like the artists page. Genres have no artwork and stay as they were. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 14 +++++++++ webapp/src/pages/stats-page.tsx | 46 +++++++++++++++++++++++------- webapp/src/styles/styles.css | 25 +++++++++++++++- 3 files changed, 73 insertions(+), 12 deletions(-) diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 8cb383e7..31ba9a72 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -7733,6 +7733,20 @@ test("shows this account's listening, period by period", async ({ page }) => { await expect(columns.getByRole("link", { name: "Art Pop" })).toBeVisible(); await expect(columns.getByRole("link", { name: /Vespertine/ })).toBeVisible(); await expect(page.locator(".stats-moments")).toContainText("Cocoon, "); + // Every track, artist and album entry carries its cover (or the lettered + // placeholder when there is none); a genre has no artwork to show. + const entries = columns.locator(".stats-entry"); + await expect(columns.locator(".stats-entry .stats-art")).toHaveCount( + (await entries.count()) - 1, + ); + await expect( + columns + .getByRole("link", { name: "Björk", exact: true }) + .locator(".stats-art-round"), + ).toHaveCount(1); + await expect( + columns.getByRole("link", { name: "Art Pop" }).locator(".stats-art"), + ).toHaveCount(0); await expectNoWcagViolations(page); await page.getByRole("combobox", { name: "Period" }).selectOption("7d"); diff --git a/webapp/src/pages/stats-page.tsx b/webapp/src/pages/stats-page.tsx index ffcec980..d08e6560 100644 --- a/webapp/src/pages/stats-page.tsx +++ b/webapp/src/pages/stats-page.tsx @@ -4,6 +4,7 @@ import { type CSSProperties, useState } from "react"; import type { StatsPeriod } from "../api"; import { statsQuery } from "../api/queries"; +import { Artwork } from "../components/artwork"; import { EmptyState, Loading, PageHeader } from "../components/page-shell"; import { barShares, listeningTime } from "../lib/mixes"; import { usePlayer } from "../player/player"; @@ -147,9 +148,18 @@ export function StatsPage() { ) } > - {entry.song.title} - - {entry.song.artist ?? t("common.unknownArtist")} + + + + {entry.song.title} + + + {entry.song.artist ?? t("common.unknownArtist")} + @@ -169,9 +179,16 @@ export function StatsPage() { to="/artists/$artistId" params={{ artistId: entry.artist.id }} > - - {entry.artist.name} - + + + + {entry.artist.name} + + {t("stats.playCount", { count: entry.plays })} @@ -190,11 +207,18 @@ export function StatsPage() { to="/albums/$albumId" params={{ albumId: entry.album.id }} > - - {entry.album.title} - - - {entry.album.artist ?? t("common.variousArtists")} + + + + {entry.album.title} + + + {entry.album.artist ?? t("common.variousArtists")} + diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index ca31cf51..44428c4c 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -3947,8 +3947,11 @@ button.danger { min-width: 0; } +/* The cover, then the names stacked beside it. */ .stats-entry { - display: grid; + display: flex; + align-items: center; + gap: var(--space-3); min-width: 0; padding: 0; border: 0; @@ -3957,6 +3960,26 @@ button.danger { text-align: left; } +.stats-art { + width: 2.75rem; + flex: 0 0 auto; + border-radius: 0.45rem; +} + +/* An artist is a portrait, round as on the artists page. */ +.stats-art-round { + border-radius: 50%; +} + +.stats-art span { + font-size: var(--text-base); +} + +.stats-text { + display: grid; + min-width: 0; +} + .stats-name, .stats-sub { overflow: hidden; From 4ad71b5fa10ffe76a0f8e0721a5d6f7c910feb5e Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sun, 25 Oct 2026 23:09:43 +0100 Subject: [PATCH 3/4] test(webapp): press a strip line and check the head moved to it The strip test counted the seek buttons and named one, but never pressed a line, so it passed with a button that seeks nowhere. It now presses a line and requires the sung-line mark to land on it. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 31ba9a72..336f9448 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -5051,6 +5051,19 @@ test("follows the sung line in the strip beside the cover, and seeks from it", a // The strip scrolls itself and nothing else: the page stays where it was. expect(await page.evaluate(() => window.scrollY)).toBe(0); + // Pressing a line moves the head to it. The sung line is derived from the + // player's position, so the mark landing on the pressed line is the head + // having moved there — the arrow keys above go through the same `seek`. + await strip + .getByRole("button", { name: "strip line 5, jump to 0:25", exact: true }) + .click(); + await expect( + strip.getByRole("listitem").filter({ hasText: /^strip line 5$/ }), + ).toHaveAttribute("aria-current", "true"); + await expect( + strip.getByRole("listitem").filter({ hasText: /^strip line 20$/ }), + ).not.toHaveAttribute("aria-current", "true"); + await expectNoWcagViolations(page); // Without timings, the strip offers no seeking, as the lyrics view does not. From 38e957189ed645c69ec8b6b8972d0a3f78f5b937 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Sun, 25 Oct 2026 23:24:47 +0100 Subject: [PATCH 4/4] fix(webapp): draw the cover placeholder as a span so it fits inside a button A top track in the statistics is a button that plays it, and the cover's lettered placeholder was a div, which a button may not hold. It is a span now; .cover already sets display: grid, so nothing drawn changes. Signed-off-by: InstaZDLL --- webapp/e2e/studio-nocturne.spec.ts | 4 ++++ webapp/src/components/artwork.tsx | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 336f9448..b9ef1845 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -7760,6 +7760,10 @@ test("shows this account's listening, period by period", async ({ page }) => { await expect( columns.getByRole("link", { name: "Art Pop" }).locator(".stats-art"), ).toHaveCount(0); + // A top track is a button, and a button holds phrasing content only: the + // lettered placeholder inside it must not be a `div`. + await expect(columns.locator("button .cover-fallback")).toHaveCount(2); + await expect(columns.locator("button div")).toHaveCount(0); await expectNoWcagViolations(page); await page.getByRole("combobox", { name: "Period" }).selectOption("7d"); diff --git a/webapp/src/components/artwork.tsx b/webapp/src/components/artwork.tsx index d31cad60..87cc079b 100644 --- a/webapp/src/components/artwork.tsx +++ b/webapp/src/components/artwork.tsx @@ -97,13 +97,16 @@ export function Artwork({ artworkId, title, className = "" }: ArtworkProps) { const classes = `cover ${className}`.trim(); if (src) return ; + // A `span`, not a `div`: a cover can sit inside a `