GUI: click a synced lyric line to jump there
I maintainer di solito rispondono entro 1 giorno
Una pull request collegata è già stata integrata.
- #853 di @niukanen1 — integrata
Valutazione
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Idoneità per principianti
- 65/100
- Tipo di issue
- Funzionalità
- Chiarezza
- Specificata chiaramente
- Stato di attività
- Attiva
- Stack tecnologico
- react, typescript
- Ambito
- frontend
Direzione di ricerca
Start with the line rendering in gui/src/LyricsPage.tsx (lines 56-72) and the Lyrics function in gui/src/Room.tsx (lines 306-342), then the Transport.tsx seek bar, which already sends SeekTo. Add a small LyricText.tsx that renders a real button for synced lines only, wire connected and send through Shell.tsx and Room.tsx, and keep aria-current on the li. Done when clicking or pressing Enter on a synced line sends { SeekTo: at_ms }, unsynced lines stay plain text, and the updated lyricsPage.test.tsx and new lyricText.test.tsx pass.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Descrizione
What
The GUI shows the lyrics of the playing song in two places: the Lyrics page (press l, or the Lyrics button in the player bar) and the full-screen Room (press r, then l). When the lyrics are synced, the current line lights up as the song plays. But the lines are plain text, so clicking one does nothing.
To see it:
- Play a song that has synced lyrics.
- Press
lto open the Lyrics page. - Click a line further down. Nothing happens. The song keeps playing where it was.
What should happen: clicking a synced line seeks the song to the start of that line, the same way clicking the seek bar does. Keyboard users can do it too: Tab reaches the lyrics once, on the line being sung, and Enter or Space seeks there.
Only synced lyrics should be clickable. When the lyrics are not synced, the line times are only estimates (the Lyrics page says "These lyrics are not synced", the Room says "Timing estimated"), so a seek there would land in the wrong place. Those lines stay plain text.
Arrow-key browsing between the lines is out of scope. Please don't add it here.
Where to look
gui/src/LyricsPage.tsx,LyricsPage(lines 10-77). Its props are onlyitem,lyrics,positionandactive(lines 15-20), so it can't send anything yet. The lines are rendered at lines 56-72:
The auto-scroll at lines 28-36 finds the line with<ol className={synced ? "lyrics-lines synced" : "lyrics-lines"}> {lines.map((line, row) => ( <li key={`${row}-${line.at_ms}`} aria-current={row === current ? "true" : undefined} className={ ... "past" : "future" } > {line.text || "♪"} </li> ))} </ol>querySelector('[aria-current="true"]').gui/src/Room.tsx, the privateLyricsfunction (lines 306-342). It takes only{ lyrics, ms }. Its<ol ref={panel}>(lines 329-339) is the same list with a different class scheme: a line before the current one getssung(line 334). Its scroll effect is at lines 312-320.Roomitself already hasconnectedandsend(lines 29 and 36) and renders<Lyrics lyrics={lyrics} ms={ms} />at line 196. TheSidestracklist right above it (lines 250-304) is a good model: each row is a real<button>that callssend(...).gui/src/Shell.tsx, thelyrics:entry in thepagesobject (lines 357-364):
Most pages in that object getlyrics: ( <LyricsPage item={playback?.item ?? null} lyrics={channels.lyrics?.payload ?? null} position={state.position} active={page === "lyrics"} /> ),send={send}, and some (Home and Discover, for example) also getconnected={state.connected}. The<Room ... />element (lines 572-584) already gets both.gui/src/Transport.tsx, line 95. The seek bar already sends the message you need:send({ SeekTo: Math.round(ratio * duration) }). Ingui/src/bindings/Action.tsit's{ "SeekTo": number }, in milliseconds.- The line data:
gui/src/bindings/LyricLine.tsis{ at_ms: number, text: string }, andLyricsPayloadhassynced: boolean. The server builds it insrc/gui/protocol.rs,fn lyrics(lines 916-936), with the raw line start times. - How the seek works on the Rust side (you don't need to change any of it):
Action::SeekTogoes toApp::seek_to(src/core/action/apply.rsline 43), andseek_toinsrc/core/app/seek.rs(lines 169-226) handles Apple Music, the decoded sources (Local Files, Subsonic, YouTube, Qobuz) and Spotify, and clamps to the track length. Radio can't seek:seek_todoes nothing when the decoded source has no position (lines 179-187), so a click there is harmless. OneSeekToworks for every source, exactly like the seek bar. gui/src/Shell.tsx,describe()(line 614). Line 626 matters here:interactive: target?.closest("button, a[href], [role=slider]") != null,gui/src/areas.tsline 58 only toggles playback on Space when the target is not interactive. So on a real button, Space goes to the button and clicks it.gui/src/style.csslines 55-64: everybuttoninherits font and color, and adisabledbutton getsopacity: 0.4.gui/src/LyricsPage.cssline 39: the page's list setsletter-spacing: -0.01em.gui/src/Room.csslines 256-271 color the Room's lines on the<li>.gui/src/testTree.ts:byNamefinds an element by its text and lets a test call itsonClick. It can only render components without hooks.gui/src/bars.test.tsxlines 182-183 and 202 show the pattern.- Tests that assert on the lyric markup today:
gui/src/lyricsPage.test.tsxlines 41-43 check exact strings:'<li class="past">There's a fire','<li aria-current="true">Reaching a fever pitch'and'<li class="future">♪</li>'. These will break once a button sits inside the<li>, and that's expected. The render helper at lines 28-36 will also need the two new props.gui/src/room.test.tsx, "shows estimated lyrics without a current line" (lines 110-130), usessynced: false. It should keep passing unchanged. Line 89 (class="now" aria-current="true") is the tracklist button, not lyrics.
- The Playwright scenes:
gui/e2e/scenes.shot.ts, theroomscene (lines 222-228) and thelyricsscene (lines 230-236), with synced lines from the lyrics message ingui/e2e/fixtures.ts(lines 563-580).
Possible solution
The whole change is about 100 lines, half of it tests.
- One small shared component,
gui/src/LyricText.tsx. It renders what goes inside each<li>: a seek button for a synced line, plain text otherwise. It has no hooks, so the Lyrics page and the Room can both use it and a test can click it.
Enter and Space need no code: a real button clicks itself.import type { Action } from "./bindings/Action"; import type { LyricLine } from "./bindings/LyricLine"; import "./LyricText.css"; /** A lyric line: a button that seeks to its start when synced, plain text when not. */ export function LyricText({ line, synced, tabStop, connected, send, }: { line: LyricLine; synced: boolean; tabStop: boolean; connected: boolean; send: (action: Action) => void; }) { const text = line.text || "♪"; if (!synced) return text; return ( <button type="button" className="lyric-seek" tabIndex={tabStop ? 0 : -1} onClick={() => { if (connected) send({ SeekTo: line.at_ms }); }} > {text} </button> ); } - Use it in both places. In each
lines.map, replace{line.text || "♪"}with:<LyricText line={line} synced={synced} tabStop={row === Math.max(current, 0)} connected={connected} send={send} />LyricsPage.tsx: addconnected: booleanandsend: (action: Action) => voidto the props. InShell.tsx, passconnected={state.connected}andsend={send}in thelyrics:entry.Room.tsx: giveLyricsthe same two props and pass them fromRoomat line 196.
- CSS, once. Add
gui/src/LyricText.csswith a.lyric-seekrule that makes the button look exactly like the text it replaces:padding: 0; border: 0; background: none; letter-spacing: inherit; text-align: left;. The browser resetsletter-spacingand centers text on buttons, so both lines matter. Font and color already come fromstyle.css. A light hover cue (an underline, say) is fine. Keep the browser's focus ring so keyboard users can see where they are. - CHANGELOG. Add one short bullet under
## [Unreleased]>### AddedinCHANGELOG.md, for example "Click a synced lyric line to jump there in the desktop app, on the Lyrics page and in full screen." That section changes often, so if it conflicts, rebase on main and keep both entries.
Traps:
- Use a real
<button>inside the<li>, notrole="button"on the<li>. Shell's Space handling (describe(), line 626) only treatsbutton,a[href]and[role=slider]as interactive. On arole="button"element, Space would toggle playback instead of seeking, and you'd have to handle Enter and Space yourself. - One tab stop, not one per line. A song can have 60 lines, and 60 tab stops would make the page painful to Tab through. Only the line with
row === Math.max(current, 0)getstabIndex={0}; every other line gets-1. Before the first line starts,currentis-1, so the first line holds the tab stop. Don't add a cursor or anyuseStatefor this. - Keep
aria-currentand the line classes on the<li>. Both auto-scroll effects look up[aria-current="true"], and the CSS colors the<li>. The button inherits the color. - Don't use
disabledon the buttons when the socket is down.style.cssline 61 fades every disabled button to 40%, which would dim all the lyrics. Guard insideonClickinstead, as in the sketch. - Send
line.at_msas it is. The terminal app has a lyric timing nudge (timing_offset_msinsrc/core/app/lyrics.rsline 26), but that's terminal-only view state. The GUI never sets it, andprotocol.rssends the raw times. Don't add an offset. - Don't put
data-focuson the lines.Shell.tsxfocuses the first[data-focus]of a page (focusScreen(), line 640) and of the Room (the.room-layer [data-focus]lookup at line 179). That should stay the page or Room container. - Don't add a local "optimistic" position. The seek bar doesn't have one either. The server pushes the new position on the next tick, and the highlight follows.
- Don't change the scroll effects (
LyricsPage.tsxlines 28-36,Room.tsxlines 312-320). #839 edits those same effects and the imports at the top of both files. Whichever PR merges second rebases and keeps both changes. If that one has merged first, keep itsbehavior: scrollBehavior()in both effects when you rebase. - Synced lines stop being easy to select and copy. Text inside a button can't be drag-selected well. That's accepted, as in Spotify. Don't work around it with
user-select: a drag that ends on the same line would click it and seek. - After a mouse click, the line keeps focus, so Space seeks to that line again instead of pausing. Every other button in the GUI behaves the same way, so leave it.
- Leave the Rust side alone: no change to
protocol.rs,seek.rsorAction. Also leave the terminal lyrics view alone.
Tests to add (name tests as behavior sentences):
- A new
gui/src/lyricText.test.tsx:import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it } from "vitest"; import type { Action } from "./bindings/Action"; import { LyricText } from "./LyricText"; import { byName } from "./testTree"; const line = { at_ms: 4_000, text: "Reaching a fever pitch" }; const click = (connected: boolean) => { const sent: Action[] = []; const button = byName( <LyricText line={line} synced tabStop connected={connected} send={(action) => sent.push(action)} />, "Reaching a fever pitch", ); (button.props.onClick as () => void)(); return sent; }; describe("LyricText", () => { it("seeks to the start of a synced line when you click it", () => { expect(click(true)).toEqual([{ SeekTo: 4_000 }]); }); it("sends nothing while the connection is down", () => { expect(click(false)).toEqual([]); }); it("leaves an estimated line as plain text", () => { const html = renderToStaticMarkup( <LyricText line={line} synced={false} tabStop connected send={() => {}} />, ); expect(html).toBe("Reaching a fever pitch"); }); }); - In
gui/src/lyricsPage.test.tsx: passconnectedandsend={() => {}}in the render helper. Update the three strings at lines 41-43 to the new markup, keeping what they check (past, current, future). Add a test likeit("puts the one tab stop on the sung line", ...): the position is 5 000 ms, so the current line is "Reaching a fever pitch". Expect exactly onetabindex="0"(html.match(/tabindex="0"/g)has length 1) and expect it on that line. Count onlytabindex="0": the pagedivalready hastabindex="-1". In the unsynced test (lines 47-52), addexpect(html).not.toContain("<button"). - In
gui/src/room.test.tsx: add a short synced case, for exampleit("lets you click synced lyrics in the Room", ...), usingshowLyrics: trueandsynced: true, and expect aclass="lyric-seek"button with a lyric line in it. Don't assert "no<button" in the Room: it has other buttons (Exit full screen, the transport). The existing unsynced test should pass unchanged. renderToStaticMarkupprints attributes in the order you write the props. Run the test once and copy the real markup into yourtoContainstrings.- Leave tools/gates.count alone. New tests need no counter change.
Done when
- In
gui/, the full GUI gate passes:npm ci,npm run lint,npm run format:check,npm run typecheck,npm testandnpm run build. npm test -- lyricText,npm test -- lyricsPageandnpm test -- room.testrun your new tests and they pass. Check that the output shows your test ran; a filter that matches nothing runs nothing.- Screenshots, no Spotify and no Rust needed: install the browsers once with
npx playwright install chromium firefox webkit, then runnpm run shotsingui/. It renders the page against the fake bridge ingui/e2e/and writes one PNG per scene and browser togui/shots/. Opengui/shots/chromium/lyrics.pngand check that the lines look the same as before: no button borders, no gray backgrounds, same font size and spacing. If you like, add aroom lyricsscene togui/e2e/scenes.shot.ts, directly after thelyricsscene. Copy theroomscene, then also pressland wait for "But I set fire to the rain", so the Room's lyrics get a screenshot too. - Optional manual click check with the real app, no Spotify Premium needed. Lyrics for local files come from LRCLIB, so you need an internet connection and a well-known song with title and artist tags. Lyrics must be on in Settings (they are by default).
- On Linux, install the prerequisites in docs/installation.md (Building from Source > Prerequisites) first.
- In
gui/, runnpm run build(the Rust build embedsgui/dist). - Close any other running spotatui, since only one can run at a time. From the repo root, run:
cargo run --no-default-features --features telemetry,gui,streaming,discord-rpc,self-update,scripting,mcp-server,mpris,local-files,subsonic,internet-radio,youtube,qobuz --bin spotatui-gui
It opens the page in your browser. - Pick Local Files in the first-launch questions and point it at a folder with the song. If you've used spotatui before, there are no first-launch questions: set Settings > Sources > Music Folder to its folder, and pick Local Files in the Source picker at the top of Your Library. Play the song.
- Press
l, click a line further down, and check that the song jumps there and that line lights up. Press Tab until you reach the lyrics: you should land on the line being sung. Press Enter, then Space, and check that both seek instead of pausing. - Press
r, thenl, and do the same in the Room. - Optional: if you find a song whose lyrics say "not synced", check that its lines can't be clicked or tabbed to. The tests already cover this case.
- Put a before/after screenshot or a short clip in the PR, as CONTRIBUTING asks for UI changes.
- Lingua principale
- Rust
- Stelle
- 1.4k
- Fork
- 238
- Merge medio
- 9h 21m
- PR unite (30g)
- 193
Preparare l'ambiente
- Nessun Dockerfile né file Docker Compose
- Ha un modello di pull request
- Leggi la guida per i contributori
Come iniziare
- Leggi tutta la issue e poi la guida ai contributi del progetto.
- Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
- Fai un fork del repository e lavora su un branch.
- Apri una pull request che faccia riferimento al numero della issue.
Altre issue di LargeModGames/spotatui
-
enhancement good first issue javascript
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
LargeModGames/spotatui#841 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
documentation good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 82/100
LargeModGames/spotatui#832 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
bug good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
LargeModGames/spotatui#829 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
bug good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
LargeModGames/spotatui#827 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
bug enhancement good first issue
Difficoltà 2/5 1-3 ore Idoneità per principianti 85/100
LargeModGames/spotatui#825 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
Tutte le issue di LargeModGames/spotatui
Issue simili
-
agent:triaged bug bughunt pm:pipenv priority:p1
Difficoltà 2/5 1-3 ore Idoneità per principianti 78/100
SocketDev/socket-patch#1608 · 1 commento ·
I maintainer di solito rispondono entro 1 giorno
-
Difficoltà 2/5 1-3 ore Idoneità per principianti 62/100
adorsys/status-list-server#698 ·
I maintainer di solito rispondono entro 2 giorni
-
Expose the dot grid colourAperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 70/100
nannou-org/egui_graph#92 ·
I maintainer di solito rispondono entro 1 giorno
-
documentation good first issue
Difficoltà 2/5 Mezza giornata Idoneità per principianti 72/100
sujithputta02/Esapay#13 ·
-
Deduplicate normalized_f64_bits (hash_table.rs and bloom_filter.rs)Forse già presa Una pull request collegata a questa issue è aperta o già unita. Aperta
Difficoltà 2/5 1-3 ore Idoneità per principianti 75/100
tursodatabase/turso#10002 ·
I maintainer di solito rispondono entro 1 giorno