Hacktoberfest 2026: le issue che i maintainer hanno segnato per ottobre, aperte e adatte ai principianti. Sfoglia le issue Hacktoberfest

GUI: click a synced lyric line to jump there

Chiusa
#844 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

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

enhancement good first issue javascript

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:

  1. Play a song that has synced lyrics.
  2. Press l to open the Lyrics page.
  3. 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 only item, lyrics, position and active (lines 15-20), so it can't send anything yet. The lines are rendered at lines 56-72:
    <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>
    
    The auto-scroll at lines 28-36 finds the line with querySelector('[aria-current="true"]').
  • gui/src/Room.tsx, the private Lyrics function (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 gets sung (line 334). Its scroll effect is at lines 312-320. Room itself already has connected and send (lines 29 and 36) and renders <Lyrics lyrics={lyrics} ms={ms} /> at line 196. The Sides tracklist right above it (lines 250-304) is a good model: each row is a real <button> that calls send(...).
  • gui/src/Shell.tsx, the lyrics: entry in the pages object (lines 357-364):
    lyrics: (
      <LyricsPage
        item={playback?.item ?? null}
        lyrics={channels.lyrics?.payload ?? null}
        position={state.position}
        active={page === "lyrics"}
      />
    ),
    
    Most pages in that object get send={send}, and some (Home and Discover, for example) also get connected={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) }). In gui/src/bindings/Action.ts it's { "SeekTo": number }, in milliseconds.
  • The line data: gui/src/bindings/LyricLine.ts is { at_ms: number, text: string }, and LyricsPayload has synced: boolean. The server builds it in src/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::SeekTo goes to App::seek_to (src/core/action/apply.rs line 43), and seek_to in src/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_to does nothing when the decoded source has no position (lines 179-187), so a click there is harmless. One SeekTo works 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.ts line 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.css lines 55-64: every button inherits font and color, and a disabled button gets opacity: 0.4.
  • gui/src/LyricsPage.css line 39: the page's list sets letter-spacing: -0.01em. gui/src/Room.css lines 256-271 color the Room's lines on the <li>.
  • gui/src/testTree.ts: byName finds an element by its text and lets a test call its onClick. It can only render components without hooks. gui/src/bars.test.tsx lines 182-183 and 202 show the pattern.
  • Tests that assert on the lyric markup today:
    • gui/src/lyricsPage.test.tsx lines 41-43 check exact strings: '<li class="past">There&#x27;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), uses synced: 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, the room scene (lines 222-228) and the lyrics scene (lines 230-236), with synced lines from the lyrics message in gui/e2e/fixtures.ts (lines 563-580).

Possible solution

The whole change is about 100 lines, half of it tests.

  1. 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.
    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>
      );
    }
    
    Enter and Space need no code: a real button clicks itself.
  2. 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: add connected: boolean and send: (action: Action) => void to the props. In Shell.tsx, pass connected={state.connected} and send={send} in the lyrics: entry.
    • Room.tsx: give Lyrics the same two props and pass them from Room at line 196.
  3. CSS, once. Add gui/src/LyricText.css with a .lyric-seek rule 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 resets letter-spacing and centers text on buttons, so both lines matter. Font and color already come from style.css. A light hover cue (an underline, say) is fine. Keep the browser's focus ring so keyboard users can see where they are.
  4. CHANGELOG. Add one short bullet under ## [Unreleased] > ### Added in CHANGELOG.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>, not role="button" on the <li>. Shell's Space handling (describe(), line 626) only treats button, a[href] and [role=slider] as interactive. On a role="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) gets tabIndex={0}; every other line gets -1. Before the first line starts, current is -1, so the first line holds the tab stop. Don't add a cursor or any useState for this.
  • Keep aria-current and 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 disabled on the buttons when the socket is down. style.css line 61 fades every disabled button to 40%, which would dim all the lyrics. Guard inside onClick instead, as in the sketch.
  • Send line.at_ms as it is. The terminal app has a lyric timing nudge (timing_offset_ms in src/core/app/lyrics.rs line 26), but that's terminal-only view state. The GUI never sets it, and protocol.rs sends the raw times. Don't add an offset.
  • Don't put data-focus on the lines. Shell.tsx focuses 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.tsx lines 28-36, Room.tsx lines 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 its behavior: 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.rs or Action. 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: pass connected and send={() => {}} 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 like it("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 one tabindex="0" (html.match(/tabindex="0"/g) has length 1) and expect it on that line. Count only tabindex="0": the page div already has tabindex="-1". In the unsynced test (lines 47-52), add expect(html).not.toContain("<button").
  • In gui/src/room.test.tsx: add a short synced case, for example it("lets you click synced lyrics in the Room", ...), using showLyrics: true and synced: true, and expect a class="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.
  • renderToStaticMarkup prints attributes in the order you write the props. Run the test once and copy the real markup into your toContain strings.
  • 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 test and npm run build.
  • npm test -- lyricText, npm test -- lyricsPage and npm test -- room.test run 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 run npm run shots in gui/. It renders the page against the fake bridge in gui/e2e/ and writes one PNG per scene and browser to gui/shots/. Open gui/shots/chromium/lyrics.png and check that the lines look the same as before: no button borders, no gray backgrounds, same font size and spacing. If you like, add a room lyrics scene to gui/e2e/scenes.shot.ts, directly after the lyrics scene. Copy the room scene, then also press l and 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).
    1. On Linux, install the prerequisites in docs/installation.md (Building from Source > Prerequisites) first.
    2. In gui/, run npm run build (the Rust build embeds gui/dist).
    3. 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.
    4. 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.
    5. 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.
    6. Press r, then l, and do the same in the Room.
    7. 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

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di LargeModGames/spotatui

Tutte le issue di LargeModGames/spotatui

Issue simili

Altre issue su Rust

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.