fix(lyrics): restore spacing between word-synced lyric tokens#160
Merged
Conversation
Reported: when an Enhanced LRC track reached an active line, the words collapsed into a single run — "Meet me in the crowd, people, people" rendered as "Meetmeinthecrowd,people,people". Other lines on the same track stayed correctly spaced. Two ingredients combined to produce the bug: 1. Each word in the active line is rendered as its own `display: inline-block` `<span>` so the karaoke highlight can scale/opacity-animate per word. Inline-block sibling boxes do not inherit JSX-level whitespace — `<span>a</span><span>b</span>` renders as `ab`, not `a b`. The non-active lines escape the bug because they fall through to `line.text` (already space-joined by the parser). 2. Many Enhanced LRC sources omit spaces between word stamps (`<00:01>Meet<00:02>me<00:03>in…`). The parser's `body.slice(start, end)` then yields word texts with no leading or trailing whitespace, so even falling back on intra-text space wouldn't help. Fix: wrap each word `<span>` in a `Fragment` together with a literal `" "` text node when the word isn't the last in the line. The text node restores the inter-box gap; if the source did carry whitespace inside `word.text`, `white-space: normal` collapses the pair to a single space. Applied identically in `FullscreenLyrics` (Apple-Music- style fullscreen) and `LyricsPanel` (side panel).
📝 WalkthroughRésuméCette PR corrige le rendu des paroles synchronisées en deux composants : ChangementsRestauration de l'espacement inter-mots dans les paroles synchronisées
Effort d'examen estimé🎯 2 (Simple) | ⏱️ ~12 minutes Labels suggérés
Poésie
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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
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.
Bug
On a track with Enhanced LRC word-level timestamps, the active line collapsed into a single run with no spaces between words:
Other lines on the same track render correctly because they only paint when not active.
Cause
Two ingredients combined:
Non-active lines escape both because they fall through to `line.text` (already space-joined by the parser).
Fix
Wrap each word `` in a `Fragment` with a literal `" "` text node when it isn't the last. The text node provides the inter-box gap; if the source DID carry trailing whitespace in `word.text`, `white-space: normal` collapses the pair back to one space. Same idiom applied identically in FullscreenLyrics and LyricsPanel.
Test plan
Summary by CodeRabbit