From b34203588496b7016c9c1a8e0ed6866e7fbe37f2 Mon Sep 17 00:00:00 2001 From: Xavier Araque Date: Fri, 7 Nov 2025 15:25:00 +0100 Subject: [PATCH] Player.jsx Component - Comprehensive Architecture Improvement --- ui/src/audioplayer/Player.jsx | 36 ++++++++++++------ ui/src/audioplayer/Player.test.jsx | 38 +++++++++++++------ ui/src/audioplayer/hooks/useAudioInstance.js | 8 +++- ui/src/audioplayer/hooks/usePlayerState.js | 2 +- .../audioplayer/hooks/usePlayerState.test.js | 2 +- .../audioplayer/hooks/usePreloading.test.js | 12 ++++-- .../audioplayer/hooks/useReplayGain.test.js | 31 +++++++++------ .../audioplayer/hooks/useScrobbling.test.js | 23 ++++++----- 8 files changed, 101 insertions(+), 51 deletions(-) diff --git a/ui/src/audioplayer/Player.jsx b/ui/src/audioplayer/Player.jsx index 02ca2cddd..a8a508cfa 100644 --- a/ui/src/audioplayer/Player.jsx +++ b/ui/src/audioplayer/Player.jsx @@ -61,10 +61,16 @@ const Player = () => { dispatchClearQueue, } = usePlayerState() - const { startTime, scrobbled, onAudioProgress, onAudioPlayTrackChange, onAudioEnded } = - useScrobbling(playerState, dispatch, dataProvider) + const { + startTime, + scrobbled, + onAudioProgress, + onAudioPlayTrackChange, + onAudioEnded, + } = useScrobbling(playerState, dispatch, dataProvider) - const { preloaded, preloadNextSong, resetPreloading } = usePreloading(playerState) + const { preloaded, preloadNextSong, resetPreloading } = + usePreloading(playerState) const { audioInstance, setAudioInstance, onAudioPlay } = useAudioInstance( isMobilePlayer, @@ -81,7 +87,6 @@ const Player = () => { enableCoverAnimation: config.enableCoverAnimation, }) - const defaultOptions = useMemo( () => ({ theme: playerTheme, @@ -134,12 +139,24 @@ const Player = () => { autoPlay: playerState.clear || playerState.playIndex === 0, clearPriorAudioLists: playerState.clear, extendsContent: ( - + ), defaultVolume: isMobilePlayer ? 1 : playerState.volume, showMediaSession: !currentTrack.isRadio, } - }, [defaultOptions, audioLists, playerState.playIndex, playerState.clear, playerState.volume, isMobilePlayer, currentTrack.trackId, currentTrack.isRadio]) + }, [ + defaultOptions, + audioLists, + playerState.playIndex, + playerState.clear, + playerState.volume, + isMobilePlayer, + currentTrack.trackId, + currentTrack.isRadio, + ]) const onAudioListsChange = useCallback( (_, audioLists, audioInfo) => dispatchSyncQueue(audioInfo, audioLists), @@ -202,14 +219,9 @@ const Player = () => { [audioInstance, playerState], ) - return ( -
+
({ jest.mock('./AudioTitle', () => ({ __esModule: true, - default: ({ audioInfo }) =>
{audioInfo?.song?.title || 'No song'}
, + default: ({ audioInfo }) => ( +
{audioInfo?.song?.title || 'No song'}
+ ), })) jest.mock('./PlayerToolbar', () => ({ @@ -101,9 +103,18 @@ describe('Player Component', () => { { player: { queue: [ - { uuid: '1', musicSrc: 'song1.mp3', title: 'Song 1', artist: 'Artist 1' }, + { + uuid: '1', + musicSrc: 'song1.mp3', + title: 'Song 1', + artist: 'Artist 1', + }, ], - current: { uuid: '1', trackId: 'track1', song: { title: 'Song 1', artist: 'Artist 1' } }, + current: { + uuid: '1', + trackId: 'track1', + song: { title: 'Song 1', artist: 'Artist 1' }, + }, playIndex: 0, mode: 'single', volume: 0.8, @@ -115,7 +126,7 @@ describe('Player Component', () => { replayGain: { gainMode: 'track', }, - } + }, ) const renderPlayer = () => { @@ -124,7 +135,7 @@ describe('Player Component', () => { - + , ) } @@ -149,7 +160,8 @@ describe('Player Component', () => { expect(container.firstChild).toBeNull() // Reset mock - const { useAuthState: originalUseAuthState } = jest.requireMock('react-admin') + const { useAuthState: originalUseAuthState } = + jest.requireMock('react-admin') originalUseAuthState.mockReturnValue({ authenticated: true }) }) @@ -164,7 +176,7 @@ describe('Player Component', () => { player: { queue: [] }, settings: { notifications: true }, replayGain: { gainMode: 'track' }, - } + }, ) const { container } = render( @@ -172,7 +184,7 @@ describe('Player Component', () => { - + , ) expect(container.firstChild).toBeNull() @@ -221,7 +233,7 @@ describe('Player Component', () => { player: { queue: [] }, settings: { notifications: true }, replayGain: { gainMode: 'track' }, - } + }, ) render( @@ -229,7 +241,7 @@ describe('Player Component', () => { - + , ) // Document title should be reset when player is not visible @@ -240,6 +252,8 @@ describe('Player Component', () => { renderPlayer() // ThemeProvider should wrap the component - expect(screen.getByTestId('react-jk-music-player').parentElement).toBeInTheDocument() + expect( + screen.getByTestId('react-jk-music-player').parentElement, + ).toBeInTheDocument() }) -}) \ No newline at end of file +}) diff --git a/ui/src/audioplayer/hooks/useAudioInstance.js b/ui/src/audioplayer/hooks/useAudioInstance.js index 7adfa6f99..ebb42e2b2 100644 --- a/ui/src/audioplayer/hooks/useAudioInstance.js +++ b/ui/src/audioplayer/hooks/useAudioInstance.js @@ -95,7 +95,11 @@ export const useAudioInstance = (isMobilePlayer, context) => { if (showNotifications) { try { - sendNotification(song.title, `${song.artist} - ${song.album}`, info.cover) + sendNotification( + song.title, + `${song.artist} - ${song.album}`, + info.cover, + ) } catch (error) { // eslint-disable-next-line no-console console.error('Notification error:', error) @@ -123,4 +127,4 @@ export const useAudioInstance = (isMobilePlayer, context) => { setAudioInstance, onAudioPlay, } -} \ No newline at end of file +} diff --git a/ui/src/audioplayer/hooks/usePlayerState.js b/ui/src/audioplayer/hooks/usePlayerState.js index 3deee4a96..02199f272 100644 --- a/ui/src/audioplayer/hooks/usePlayerState.js +++ b/ui/src/audioplayer/hooks/usePlayerState.js @@ -81,4 +81,4 @@ export const usePlayerState = () => { dispatchSyncQueue, dispatchClearQueue, } -} \ No newline at end of file +} diff --git a/ui/src/audioplayer/hooks/usePlayerState.test.js b/ui/src/audioplayer/hooks/usePlayerState.test.js index d8581bf57..11797dc2b 100644 --- a/ui/src/audioplayer/hooks/usePlayerState.test.js +++ b/ui/src/audioplayer/hooks/usePlayerState.test.js @@ -101,4 +101,4 @@ describe('usePlayerState', () => { expect(useDispatch).toHaveBeenCalled() expect(useSelector).toHaveBeenCalledWith(expect.any(Function)) }) -}) \ No newline at end of file +}) diff --git a/ui/src/audioplayer/hooks/usePreloading.test.js b/ui/src/audioplayer/hooks/usePreloading.test.js index 14f46a0e9..d0e8cf4e5 100644 --- a/ui/src/audioplayer/hooks/usePreloading.test.js +++ b/ui/src/audioplayer/hooks/usePreloading.test.js @@ -108,7 +108,10 @@ describe('usePreloading', () => { result.current.preloadNextSong() }) - expect(consoleSpy).toHaveBeenCalledWith('Error during preloading:', expect.any(Error)) + expect(consoleSpy).toHaveBeenCalledWith( + 'Error during preloading:', + expect.any(Error), + ) expect(result.current.preloaded).toBe(false) // Should remain false on error consoleSpy.mockRestore() @@ -134,8 +137,11 @@ describe('usePreloading', () => { result.current.preloadNextSong() }) - expect(consoleSpy).toHaveBeenCalledWith('Preloading error:', expect.any(Event)) + expect(consoleSpy).toHaveBeenCalledWith( + 'Preloading error:', + expect.any(Event), + ) consoleSpy.mockRestore() }) -}) \ No newline at end of file +}) diff --git a/ui/src/audioplayer/hooks/useReplayGain.test.js b/ui/src/audioplayer/hooks/useReplayGain.test.js index b8471f23c..0ba8638b8 100644 --- a/ui/src/audioplayer/hooks/useReplayGain.test.js +++ b/ui/src/audioplayer/hooks/useReplayGain.test.js @@ -37,7 +37,7 @@ describe('useReplayGain', () => { it('should initialize with null context and gainNode', () => { const { result } = renderHook(() => - useReplayGain(null, { current: {} }, { gainMode: 'track' }) + useReplayGain(null, { current: {} }, { gainMode: 'track' }), ) expect(result.current.context).toBeNull() @@ -52,7 +52,7 @@ describe('useReplayGain', () => { const mockGainInfo = { gainMode: 'track' } const { result } = renderHook(() => - useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) + useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo), ) expect(global.AudioContext).toHaveBeenCalled() @@ -69,11 +69,17 @@ describe('useReplayGain', () => { mockCalculateGain.mockReturnValue(0.8) const { result } = renderHook(() => - useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) + useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo), ) - expect(mockCalculateGain).toHaveBeenCalledWith(mockGainInfo, mockPlayerState.current.song) - expect(result.current.gainNode.gain.setValueAtTime).toHaveBeenCalledWith(0.8, 0) + expect(mockCalculateGain).toHaveBeenCalledWith( + mockGainInfo, + mockPlayerState.current.song, + ) + expect(result.current.gainNode.gain.setValueAtTime).toHaveBeenCalledWith( + 0.8, + 0, + ) }) it('should handle Web Audio API errors gracefully', () => { @@ -89,12 +95,12 @@ describe('useReplayGain', () => { const mockGainInfo = { gainMode: 'track' } const { result } = renderHook(() => - useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) + useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo), ) expect(consoleSpy).toHaveBeenCalledWith( 'Error initializing Web Audio API for replay gain:', - expect.any(Error) + expect.any(Error), ) expect(result.current.context).toBeNull() @@ -128,10 +134,13 @@ describe('useReplayGain', () => { })) const { result } = renderHook(() => - useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) + useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo), ) - expect(consoleSpy).toHaveBeenCalledWith('Error applying replay gain:', expect.any(Error)) + expect(consoleSpy).toHaveBeenCalledWith( + 'Error applying replay gain:', + expect.any(Error), + ) consoleSpy.mockRestore() }) @@ -142,10 +151,10 @@ describe('useReplayGain', () => { const mockGainInfo = { gainMode: 'off' } const { result } = renderHook(() => - useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) + useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo), ) expect(global.AudioContext).not.toHaveBeenCalled() expect(result.current.context).toBeNull() }) -}) \ No newline at end of file +}) diff --git a/ui/src/audioplayer/hooks/useScrobbling.test.js b/ui/src/audioplayer/hooks/useScrobbling.test.js index ca9b7190c..85763222f 100644 --- a/ui/src/audioplayer/hooks/useScrobbling.test.js +++ b/ui/src/audioplayer/hooks/useScrobbling.test.js @@ -35,7 +35,7 @@ describe('useScrobbling', () => { it('should initialize with default state', () => { const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) expect(result.current.startTime).toBeNull() @@ -47,7 +47,7 @@ describe('useScrobbling', () => { it('should handle audio progress and scrobble when conditions are met', () => { const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) const mockInfo = { @@ -68,7 +68,7 @@ describe('useScrobbling', () => { it('should not scrobble radio streams', () => { const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) const mockInfo = { @@ -87,7 +87,7 @@ describe('useScrobbling', () => { it('should reset scrobbling state on track change', () => { const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) // Set initial state @@ -114,7 +114,7 @@ describe('useScrobbling', () => { it('should handle audio ended and perform keepalive', async () => { const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) const mockInfo = { trackId: 'track1' } @@ -125,7 +125,9 @@ describe('useScrobbling', () => { expect(result.current.scrobbled).toBe(false) expect(result.current.startTime).toBeNull() - expect(mockDataProvider.getOne).toHaveBeenCalledWith('keepalive', { id: 'track1' }) + expect(mockDataProvider.getOne).toHaveBeenCalledWith('keepalive', { + id: 'track1', + }) }) it('should handle scrobbling errors gracefully', () => { @@ -136,7 +138,7 @@ describe('useScrobbling', () => { }) const { result } = renderHook(() => - useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) + useScrobbling(mockPlayerState, mockDispatch, mockDataProvider), ) const mockInfo = { @@ -150,9 +152,12 @@ describe('useScrobbling', () => { result.current.onAudioProgress(mockInfo) }) - expect(consoleSpy).toHaveBeenCalledWith('Scrobbling error:', expect.any(Error)) + expect(consoleSpy).toHaveBeenCalledWith( + 'Scrobbling error:', + expect.any(Error), + ) expect(result.current.scrobbled).toBe(true) // Still sets to true despite error consoleSpy.mockRestore() }) -}) \ No newline at end of file +})