From e08d4bef16586256e70640d2f1f6078f1bb5bdf3 Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 9 Mar 2026 12:44:19 -0400 Subject: [PATCH] fix(ui): preserve pending track selection through queue sync and premature callbacks When clicking a song while another was playing, PLAYER_SYNC_QUEUE and PLAYER_CURRENT would fire before the music player switched tracks, wiping the playIndex set by PLAYER_PLAY_TRACKS. This caused the player to stay on the old track instead of switching to the clicked one. Now reduceSyncQueue and reduceCurrent preserve a pending playIndex until the music player confirms it actually reached the requested track. --- ui/src/reducers/playerReducer.js | 17 +++-- ui/src/reducers/playerReducer.test.js | 95 ++++++++++++++++++++++++++- 2 files changed, 107 insertions(+), 5 deletions(-) diff --git a/ui/src/reducers/playerReducer.js b/ui/src/reducers/playerReducer.js index b7086e6c..d3291633 100644 --- a/ui/src/reducers/playerReducer.js +++ b/ui/src/reducers/playerReducer.js @@ -173,8 +173,11 @@ const reduceSyncQueue = (state, { data: { audioInfo, audioLists } }) => { return { ...state, queue: audioLists, - clear: false, - playIndex: undefined, + // Keep clear and playIndex alive so the music player can still + // pick up a pending track selection set by PLAYER_PLAY_TRACKS. + // They will be consumed by the next PLAYER_CURRENT dispatch. + clear: state.playIndex != null ? state.clear : false, + playIndex: state.playIndex != null ? state.playIndex : undefined, } } @@ -183,11 +186,17 @@ const reduceCurrent = (state, { data }) => { const savedPlayIndex = state.queue.findIndex( (item) => item.uuid === current.uuid, ) + // When a track selection is pending (playIndex is set), keep it alive + // until the music player confirms it actually switched to the requested + // track. Without this, a premature onAudioPlay callback for the + // still-playing old track would overwrite the pending selection. + const pending = state.playIndex != null && savedPlayIndex !== state.playIndex return { ...state, current, - playIndex: undefined, - savedPlayIndex, + playIndex: pending ? state.playIndex : undefined, + clear: pending ? state.clear : false, + savedPlayIndex: pending ? state.savedPlayIndex : savedPlayIndex, volume: data.volume, } } diff --git a/ui/src/reducers/playerReducer.test.js b/ui/src/reducers/playerReducer.test.js index 9e3b03b1..10e9512d 100644 --- a/ui/src/reducers/playerReducer.test.js +++ b/ui/src/reducers/playerReducer.test.js @@ -1,8 +1,101 @@ import { describe, it, expect } from 'vitest' import { playerReducer } from './playerReducer' -import { PLAYER_REFRESH_QUEUE } from '../actions' +import { + PLAYER_SYNC_QUEUE, + PLAYER_CURRENT, + PLAYER_REFRESH_QUEUE, +} from '../actions' describe('playerReducer', () => { + describe('pending track selection survives SYNC_QUEUE and premature CURRENT', () => { + // Simulates the real sequence when clicking a new song while one is playing: + // 1. PLAYER_PLAY_TRACKS sets playIndex and clear + // 2. PLAYER_SYNC_QUEUE fires when music player syncs its internal queue + // 3. PLAYER_CURRENT fires for the OLD still-playing track + // 4. PLAYER_CURRENT fires for the NEW track (player switched) + const stateAfterPlayTracks = { + queue: [ + { trackId: 's1', uuid: 'aaa', name: 'Song 1' }, + { trackId: 's2', uuid: 'bbb', name: 'Song 2' }, + { trackId: 's3', uuid: 'ccc', name: 'Song 3' }, + ], + current: { uuid: 'ccc', name: 'Song 3' }, + playIndex: 0, // user clicked Song 1 + savedPlayIndex: 2, // Song 3 was playing + clear: true, + volume: 1, + } + + it('SYNC_QUEUE preserves pending playIndex and clear', () => { + const newQueue = [ + { trackId: 's1', uuid: 'xxx', name: 'Song 1' }, + { trackId: 's2', uuid: 'yyy', name: 'Song 2' }, + { trackId: 's3', uuid: 'zzz', name: 'Song 3' }, + ] + const action = { + type: PLAYER_SYNC_QUEUE, + data: { audioInfo: {}, audioLists: newQueue }, + } + const result = playerReducer(stateAfterPlayTracks, action) + expect(result.playIndex).toBe(0) + expect(result.clear).toBe(true) + expect(result.queue).toBe(newQueue) + }) + + it('SYNC_QUEUE clears playIndex when no pending selection', () => { + const stateNoPending = { ...stateAfterPlayTracks, playIndex: undefined } + const action = { + type: PLAYER_SYNC_QUEUE, + data: { audioInfo: {}, audioLists: stateNoPending.queue }, + } + const result = playerReducer(stateNoPending, action) + expect(result.playIndex).toBeUndefined() + expect(result.clear).toBe(false) + }) + + it('CURRENT for old track preserves pending playIndex', () => { + // After SYNC_QUEUE, queue has new UUIDs. The old track's UUID (zzz) + // is at index 2, but playIndex is 0. This is a premature callback. + const stateAfterSync = { + ...stateAfterPlayTracks, + queue: [ + { trackId: 's1', uuid: 'xxx', name: 'Song 1' }, + { trackId: 's2', uuid: 'yyy', name: 'Song 2' }, + { trackId: 's3', uuid: 'zzz', name: 'Song 3' }, + ], + } + const action = { + type: PLAYER_CURRENT, + data: { uuid: 'zzz', name: 'Song 3', volume: 1 }, + } + const result = playerReducer(stateAfterSync, action) + expect(result.playIndex).toBe(0) + expect(result.clear).toBe(true) + expect(result.savedPlayIndex).toBe(2) // preserved from before + }) + + it('CURRENT for correct track consumes pending playIndex', () => { + const stateAfterSync = { + ...stateAfterPlayTracks, + queue: [ + { trackId: 's1', uuid: 'xxx', name: 'Song 1' }, + { trackId: 's2', uuid: 'yyy', name: 'Song 2' }, + { trackId: 's3', uuid: 'zzz', name: 'Song 3' }, + ], + } + // Player switched to Song 1 (uuid 'xxx', index 0 == playIndex) + const action = { + type: PLAYER_CURRENT, + data: { uuid: 'xxx', name: 'Song 1', volume: 1 }, + } + const result = playerReducer(stateAfterSync, action) + expect(result.playIndex).toBeUndefined() + expect(result.clear).toBe(false) + expect(result.savedPlayIndex).toBe(0) + expect(result.current.name).toBe('Song 1') + }) + }) + describe('PLAYER_REFRESH_QUEUE', () => { it('clamps negative savedPlayIndex to 0', () => { const state = {