fix(player): restart on re-tap + kill loop-all Now Playing flicker - #28
Merged
Merged
Conversation
The load effect was keyed on [index, current?.id], so re-selecting the track already playing (same index) never reloaded it — tapping a finished/playing track did nothing instead of restarting. next()/prev() on a single-track loop-all queue had the same dead-end. And the loop-all wrap used setIndex(-1)+requestAnimationFrame(()=>setIndex(0)), which made `current` null for a frame and flashed the Now Playing screen closed (losing lyrics scroll) every cycle. Add a playToken nonce to the load effect's deps and bump it whenever a restart is intended: - playQueue bumps it, so re-tapping the current track restarts it. - next()/prev() bump it (instead of a no-op setIndex) when the target index equals the current one (single-track loop-all). - onEnded's loop-all wrap sets index 0 and bumps the token instead of the setIndex(-1) bounce, so `current` never becomes null — no more flicker. next()/prev() now read `index` from the render closure (added to deps) so the same-index restart case is detectable. Verified in-browser: playing a track, seeking to 5s, and re-tapping it restarts to ~0; switching to a different track still works; no errors. Fixes #9. Fixes #14. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkDKh1Uo1a4D7n5wCKdcKF
Review follow-up: instead of reading `index` from the closure (which could collapse two synchronous next()/prev() calls into a single advance), special- case only the single-track queue — where a restart needs a playToken bump because the index can't change — and keep the functional setIndex updater for the multi-track advance so it stays atomic. Also drops `index` from the deps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FkDKh1Uo1a4D7n5wCKdcKF
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.
Fixes #9. Fixes #14.
Problem
The load effect was keyed on
[index, current?.id], so:next()/prev()on a single-track loop-all queue hit the same dead end.setIndex(-1)+requestAnimationFrame(() => setIndex(0)), which madecurrentnull for a frame and flashed the Now Playing screen closed (losing lyrics scroll) every cycle.Fix
Add a
playTokennonce to the load effect's deps, bumped whenever a restart is intended:playQueuebumps it → re-tapping the current track restarts it.next()/prev()bump it (instead of a no-opsetIndex) when the target index equals the current one (single-track loop-all). They now readindexfrom the render closure so that case is detectable.onEnded's loop-all wrap sets index0and bumps the token instead of thesetIndex(-1)bounce —currentnever becomes null, so no more flicker.Verification
Built clean; behavioral test:
0.64(restarted) ✅"Express Track"✅The flicker fix is verified by construction (the
setIndex(-1)null-frame is gone); the single-track loop-all + on-device Now Playing behavior are best confirmed on your phone.🤖 Generated with Claude Code
Generated by Claude Code