feat(ui): show lyrics in the mini-player - #602
Merged
Merged
Conversation
Closes #580, asked for in discussion #503. The mini-player could play a track but not show a word of it, so anyone using the widget as their whole surface had to restore the main window to read along. A Mic2 toggle in the top bar opens the lyrics in the same slot the up-next list uses. Synced LRC scrolls itself and centres the active line; clicking a line seeks to it; unsynced payloads and radio sessions render as plain scrollable text. Three decisions worth stating, because none is obvious from the diff: The overlay is mounted only while open. useTrackLyrics fetches on every track change, so leaving it mounted behind a closed overlay would fire a second fetch_lyrics per track from this webview on top of the main window's — this is a separate webview, so it is a genuinely separate fetch and not a second consumer of the main window's. Unmounting puts that cost behind the user's actual request, and the backend cache serves the overlap when both surfaces are open. The two overlays share one state slot rather than a boolean each. They both cover the whole content area, so two independent flags would let them stack. Whichever is open now marks the cover / title / seek subtree inert, which the up-next list previously did only for local playback. The active word takes the progressive karaoke fill, through the same useKaraokeWordFill the immersive column uses — so the sweep stays continuous between the 4 Hz player:position events instead of stepping every 250 ms. The side panel deliberately keeps the cheap discrete version because it is a strip beside something else; the mini-player is a dedicated reading surface, so it takes the fill despite being smaller. No editing surface: at 280x380, draggable down to 240x320, there is no room for the source label, provider picker or the import / refetch / clear actions. Those stay in the main window. Unlike up-next this is not gated on Spotify playback, matching the player bar's own lyrics toggle. One new string in all 17 locales; the button label and the header reuse the existing lyrics keys rather than duplicating them. Claude-Session: https://claude.ai/code/session_01Mvi54fX8T3MyxNX1asWsxd
The settings tab bar and the onboarding wizard both opened their doc comment by describing what they resemble before describing what they do. The comparison dated from when neither existed and carries nothing now that both are built and documented in docs/features/ui.md — a reader lands on the shape rather than the resemblance. Comments only; no behaviour change. Claude-Session: https://claude.ai/code/session_01Mvi54fX8T3MyxNX1asWsxd
InstaZDLL
added
scope: frontend
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (22)
Included review availability: 9 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 10 reviews per hour. 📝 WalkthroughWalkthroughLe mini-lecteur ajoute un overlay de paroles exclusif avec chargement, synchronisation, défilement automatique, recherche par ligne et rendu des paroles non synchronisées. Les contrôles, l’accessibilité, la documentation et les traductions sont mis à jour. ChangesParoles du mini-lecteur
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~30 minutes Merge Risk: ⚪ Minimal · up to No actionable merge-blocking risk is established in the current changes. Sequence Diagram(s)sequenceDiagram
participant Lecteur
participant OverlayParoles
participant useTrackLyrics
participant useKaraokeWordFill
Lecteur->>OverlayParoles: ouvrir les paroles
OverlayParoles->>useTrackLyrics: charger les paroles du morceau
useTrackLyrics-->>OverlayParoles: retourner le contenu et l’état
OverlayParoles->>useKaraokeWordFill: calculer le remplissage du mot actif
useKaraokeWordFill-->>OverlayParoles: retourner la progression
OverlayParoles-->>Lecteur: afficher et faire défiler les paroles
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Out of Scope Changes checkExplanation Les changements fonctionnels et la documentation sur les paroles restent liés à Full details: Docstring CoverageExplanation Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. (19 skipped: 19 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #580 — raised by @jo-el414, and originally in discussion #503.
The mini-player could play a track but not show a word of it. Anyone using the widget as their whole surface had to restore the main window just to read along.
What it does
A
Mic2toggle in the top bar opens the current track's lyrics in the same slot the up-next list uses.useKaraokeWordFillthe immersive column uses — so the sweep stays continuous between the 4 Hzplayer:positionevents instead of stepping every 250 ms.Three decisions that aren't obvious from the diff
The overlay is mounted only while open, and that is the point.
useTrackLyricsfetches on every track change. This is a second webview, so its hook instance is a genuinely separate fetch rather than a second consumer of the main window's — leaving it mounted behind a closed overlay would fire an extrafetch_lyricsper track for lyrics nobody asked to see. Unmounting puts that cost behind the user's actual request, and the backend cache serves the overlap when both surfaces are open at once.The two overlays now share one state slot (
"none" | "queue" | "lyrics") rather than a boolean each. They both cover the whole content area, so two independent flags would let them stack. A side effect worth noting: whichever overlay is open now marks the cover / title / seek subtreeinert, which the up-next list previously did only for local playback.No editing surface. At 280×380, draggable down to 240×320, there is no room for the side panel's source label, provider picker, or the import / refetch / clear actions — those stay in the main window. And unlike up-next, this is not gated on Spotify playback, matching the player bar's own lyrics toggle, which is likewise ungated.
Scope
lyrics.*keys rather than duplicating them.docs/features/ui.mdanddocs/features/integrations.mdupdated — "one lyrics fetch, two consumers" is now three, and the progressive fill is no longer immersive-only.SettingsView/OnboardingModal, no behaviour change.Checks
bun run typecheck·bun run lint·prettier --checkon every changed file — all green. No Rust touched.Not verified on a running app: this box has no display, so the karaoke fill and the auto-scroll at 280×380 want a look before merge.
#582 (the standalone floating lyrics window) stays separate and stalled.
Summary by CodeRabbit
Nouvelles fonctionnalités
Accessibilité
Documentation et traductions