diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 8681e3ca..b9ef1845 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -4999,6 +4999,91 @@ 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); + + // 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. + 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. * @@ -7661,6 +7746,24 @@ 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); + // 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 ` + ) : ( + line.value || BLANK_LINE + )} + + ); + })} + + ); +} + +/** + * 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/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 825d2ebf..44428c4c 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; @@ -3944,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; @@ -3954,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;