Skip to content

fix: feed refresh spinner leak + lyrics glow offset on complex scripts - #317

Merged
ajisth69 merged 3 commits into
mainfrom
fix/feed-stuck-refresh-and-lyrics-glow
Oct 13, 2026
Merged

ajisth69 merged 3 commits into
mainfrom
fix/feed-stuck-refresh-and-lyrics-glow

Conversation

@ajisth69

@ajisth69 ajisth69 commented Oct 13, 2026 •

Copy link
Copy Markdown
Member

Three commits — two feature fixes, one repair found by CI on this PR.

1. Feed: pull-to-refresh spinner spins forever over loaded content — 68665aa

FeedViewModel.fetchFeed has two exit paths that never cleared isRefreshing:

  • the ensureActive() after loadFeed — fires when a newer session/YT-auth emission cancels the job mid-load, and
  • the changed-identity guard's return@launch — fires when the connection or username changed while the load was in flight.

Both left isRefreshing = true, so PullToRefreshBox kept its indicator spinning over fully-loaded content, indefinitely.

Fix: a finally clears the flag on every exit, guarded by feedJob === self so a newer feed job that owns the state is never reset underneath.

Note — the other half of the report ("home page takes ages to load") was the discovery-gating bug fixed in #315. This PR fixes the spinner half on top of it.

2. Lyrics: glow offset behind non-Latin lines — af70b97

The active line's glow was a TextStyle.shadow (50 % white, 22 px). The platform positions that shadow through its per-glyph shadow pass, which lands the halo off the shaped glyph positions for complex-script shaping — Gurmukhi reads as a second, offset image behind the text, while Devanagari and Latin look fine.

Fix: LyricGlowText draws the glow as a blurred copy of the very Text rendered on top of it — same text, style, layout direction, width — so the halo's bounds are identical to the glyph run for every script by construction. Wired behind the active line in both the word-sync and line-sync branches. Below API 31 (Modifier.blur is a no-op) the original shadow stays as the fallback.

Tests: LyricsRtlTest gains Indic-script cases (Gurmukhi, Devanagari, Tamil must stay LTR), so a future "non-Latin means RTL" mistake can't silently mirror a line.

3. 844a187 — repairs two compile breaks, and main itself

CI on this PR surfaced something bigger than my own bugs:

  • main currently does not compile. The fix: feed first-paint latency, MMAP 44.1 fallback, canvas/static cover handover #315 merge landed the feed commit ea6a5e8 without its fixup a732c68, so the add(it to ...) typo inside the forEachIndexed lambdas is live on main — every build from main right now fails at FeedRepository.kt. Restored here (add(t to ...) in the indexed blocks; the unnamed forEach blocks below keep add(it to seedScore(...))).
  • Modifier.blur has no clip parameter in this Compose version; the glow now uses edgeTreatment = BlurredEdgeTreatment.Unbounded.

If a build of yours failed mysteriously today, this was it.

Status

CI green on 844a187 (unit tests + release build). Device pass still worth doing: pull-to-refresh the feed mid-load and confirm the indicator always clears; play a Punjabi track and confirm the glow tracks the glyphs.

Not addressed here: "every other page takes ages to load" — the Home/Discover/Playlist/NewReleases loading paths all reset their flags correctly in code, so that half needs a logcat pass on the specific page rather than a guess.

…ed content

fetchFeed() exits without clearing isRefreshing on two paths: the
ensureActive() after loadFeed (fires when a newer session or YT auth
emission cancels the job mid-load) and the changed-identity guard's
return@launch (connection or username changed while loading). Both left
isRefreshing true, so PullToRefreshBox kept its indicator spinning over
fully-loaded content indefinitely.

A finally now clears the flag on every exit, guarded by feedJob === self
so a newer feed job that owns the state is never reset underneath.
The active line's glow was a TextStyle.shadow (50% white, 22px), which the
platform positions through its per-glyph shadow pass. For complex-script
shaping that pass lands the halo off the shaped glyph positions, so
Gurmukhi lines read as a second, offset image behind the text while
Devanagari and Latin look fine.

LyricGlowText renders the glow as a blurred copy of the very Text drawn
on top of it — same text, style, layout direction and width — so the
bounds are identical to the glyph run for every script by construction.
Wired behind the active line in both the word-sync and line-sync
branches; below API 31, where Modifier.blur is a no-op, the original
shadow remains as the fallback.

LyricsRtlTest gains Indic-script cases (Gurmukhi, Devanagari, Tamil must
stay LTR) so a future "non-Latin means RTL" mistake cannot silently
mirror a line and offset the whole lyric.
1. FeedRepository: the #315 merge landed ea6a5e8 without its fixup
   commit a732c68, so the add(it to ...) typo inside the
   forEachIndexed lambdas is live on main and the app does not compile
   from it. Restored here: forEachIndexed blocks use add(t to ...), the
   unnamed forEach blocks below keep add(it to seedScore(...)).

2. ModernLyricsView: Modifier.blur has no clip parameter in this Compose
   version — its second parameter is edgeTreatment. The glow uses
   BlurredEdgeTreatment.Unbounded so the halo is not cut at the Text's
   own bounds.
ajisth69 added a commit that referenced this pull request Oct 13, 2026
Identical to the repair in #317: the #315 merge landed the feed change without its fixup commit, so main does not compile from add(it to ...) inside the forEachIndexed lambdas. This branch needs main to compile; once #317 merges, the two sides carry identical content for these lines.
@ajisth69
ajisth69 merged commit c1b8635 into main Oct 13, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant