Player.jsx Component - Comprehensive Architecture Improvement

This commit is contained in:
Xavier Araque 2025-11-07 15:25:00 +01:00
parent b0e2457e61
commit b342035884
8 changed files with 101 additions and 51 deletions

View File

@ -61,10 +61,16 @@ const Player = () => {
dispatchClearQueue, dispatchClearQueue,
} = usePlayerState() } = usePlayerState()
const { startTime, scrobbled, onAudioProgress, onAudioPlayTrackChange, onAudioEnded } = const {
useScrobbling(playerState, dispatch, dataProvider) 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( const { audioInstance, setAudioInstance, onAudioPlay } = useAudioInstance(
isMobilePlayer, isMobilePlayer,
@ -81,7 +87,6 @@ const Player = () => {
enableCoverAnimation: config.enableCoverAnimation, enableCoverAnimation: config.enableCoverAnimation,
}) })
const defaultOptions = useMemo( const defaultOptions = useMemo(
() => ({ () => ({
theme: playerTheme, theme: playerTheme,
@ -134,12 +139,24 @@ const Player = () => {
autoPlay: playerState.clear || playerState.playIndex === 0, autoPlay: playerState.clear || playerState.playIndex === 0,
clearPriorAudioLists: playerState.clear, clearPriorAudioLists: playerState.clear,
extendsContent: ( extendsContent: (
<PlayerToolbar id={currentTrack.trackId} isRadio={currentTrack.isRadio} /> <PlayerToolbar
id={currentTrack.trackId}
isRadio={currentTrack.isRadio}
/>
), ),
defaultVolume: isMobilePlayer ? 1 : playerState.volume, defaultVolume: isMobilePlayer ? 1 : playerState.volume,
showMediaSession: !currentTrack.isRadio, 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( const onAudioListsChange = useCallback(
(_, audioLists, audioInfo) => dispatchSyncQueue(audioInfo, audioLists), (_, audioLists, audioInfo) => dispatchSyncQueue(audioInfo, audioLists),
@ -202,14 +219,9 @@ const Player = () => {
[audioInstance, playerState], [audioInstance, playerState],
) )
return ( return (
<ThemeProvider theme={createMuiTheme(theme)}> <ThemeProvider theme={createMuiTheme(theme)}>
<div <div role="region" aria-label="Audio Player" aria-live="polite">
role="region"
aria-label="Audio Player"
aria-live="polite"
>
<ReactJkMusicPlayer <ReactJkMusicPlayer
{...options} {...options}
className={classes.player} className={classes.player}

View File

@ -26,7 +26,9 @@ jest.mock('../config', () => ({
jest.mock('./AudioTitle', () => ({ jest.mock('./AudioTitle', () => ({
__esModule: true, __esModule: true,
default: ({ audioInfo }) => <div data-testid="audio-title">{audioInfo?.song?.title || 'No song'}</div>, default: ({ audioInfo }) => (
<div data-testid="audio-title">{audioInfo?.song?.title || 'No song'}</div>
),
})) }))
jest.mock('./PlayerToolbar', () => ({ jest.mock('./PlayerToolbar', () => ({
@ -101,9 +103,18 @@ describe('Player Component', () => {
{ {
player: { player: {
queue: [ 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, playIndex: 0,
mode: 'single', mode: 'single',
volume: 0.8, volume: 0.8,
@ -115,7 +126,7 @@ describe('Player Component', () => {
replayGain: { replayGain: {
gainMode: 'track', gainMode: 'track',
}, },
} },
) )
const renderPlayer = () => { const renderPlayer = () => {
@ -124,7 +135,7 @@ describe('Player Component', () => {
<ThemeProvider theme={createMuiTheme()}> <ThemeProvider theme={createMuiTheme()}>
<Player /> <Player />
</ThemeProvider> </ThemeProvider>
</Provider> </Provider>,
) )
} }
@ -149,7 +160,8 @@ describe('Player Component', () => {
expect(container.firstChild).toBeNull() expect(container.firstChild).toBeNull()
// Reset mock // Reset mock
const { useAuthState: originalUseAuthState } = jest.requireMock('react-admin') const { useAuthState: originalUseAuthState } =
jest.requireMock('react-admin')
originalUseAuthState.mockReturnValue({ authenticated: true }) originalUseAuthState.mockReturnValue({ authenticated: true })
}) })
@ -164,7 +176,7 @@ describe('Player Component', () => {
player: { queue: [] }, player: { queue: [] },
settings: { notifications: true }, settings: { notifications: true },
replayGain: { gainMode: 'track' }, replayGain: { gainMode: 'track' },
} },
) )
const { container } = render( const { container } = render(
@ -172,7 +184,7 @@ describe('Player Component', () => {
<ThemeProvider theme={createMuiTheme()}> <ThemeProvider theme={createMuiTheme()}>
<Player /> <Player />
</ThemeProvider> </ThemeProvider>
</Provider> </Provider>,
) )
expect(container.firstChild).toBeNull() expect(container.firstChild).toBeNull()
@ -221,7 +233,7 @@ describe('Player Component', () => {
player: { queue: [] }, player: { queue: [] },
settings: { notifications: true }, settings: { notifications: true },
replayGain: { gainMode: 'track' }, replayGain: { gainMode: 'track' },
} },
) )
render( render(
@ -229,7 +241,7 @@ describe('Player Component', () => {
<ThemeProvider theme={createMuiTheme()}> <ThemeProvider theme={createMuiTheme()}>
<Player /> <Player />
</ThemeProvider> </ThemeProvider>
</Provider> </Provider>,
) )
// Document title should be reset when player is not visible // Document title should be reset when player is not visible
@ -240,6 +252,8 @@ describe('Player Component', () => {
renderPlayer() renderPlayer()
// ThemeProvider should wrap the component // ThemeProvider should wrap the component
expect(screen.getByTestId('react-jk-music-player').parentElement).toBeInTheDocument() expect(
screen.getByTestId('react-jk-music-player').parentElement,
).toBeInTheDocument()
}) })
}) })

View File

@ -95,7 +95,11 @@ export const useAudioInstance = (isMobilePlayer, context) => {
if (showNotifications) { if (showNotifications) {
try { try {
sendNotification(song.title, `${song.artist} - ${song.album}`, info.cover) sendNotification(
song.title,
`${song.artist} - ${song.album}`,
info.cover,
)
} catch (error) { } catch (error) {
// eslint-disable-next-line no-console // eslint-disable-next-line no-console
console.error('Notification error:', error) console.error('Notification error:', error)

View File

@ -108,7 +108,10 @@ describe('usePreloading', () => {
result.current.preloadNextSong() 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 expect(result.current.preloaded).toBe(false) // Should remain false on error
consoleSpy.mockRestore() consoleSpy.mockRestore()
@ -134,7 +137,10 @@ describe('usePreloading', () => {
result.current.preloadNextSong() result.current.preloadNextSong()
}) })
expect(consoleSpy).toHaveBeenCalledWith('Preloading error:', expect.any(Event)) expect(consoleSpy).toHaveBeenCalledWith(
'Preloading error:',
expect.any(Event),
)
consoleSpy.mockRestore() consoleSpy.mockRestore()
}) })

View File

@ -37,7 +37,7 @@ describe('useReplayGain', () => {
it('should initialize with null context and gainNode', () => { it('should initialize with null context and gainNode', () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useReplayGain(null, { current: {} }, { gainMode: 'track' }) useReplayGain(null, { current: {} }, { gainMode: 'track' }),
) )
expect(result.current.context).toBeNull() expect(result.current.context).toBeNull()
@ -52,7 +52,7 @@ describe('useReplayGain', () => {
const mockGainInfo = { gainMode: 'track' } const mockGainInfo = { gainMode: 'track' }
const { result } = renderHook(() => const { result } = renderHook(() =>
useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo),
) )
expect(global.AudioContext).toHaveBeenCalled() expect(global.AudioContext).toHaveBeenCalled()
@ -69,11 +69,17 @@ describe('useReplayGain', () => {
mockCalculateGain.mockReturnValue(0.8) mockCalculateGain.mockReturnValue(0.8)
const { result } = renderHook(() => const { result } = renderHook(() =>
useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo),
) )
expect(mockCalculateGain).toHaveBeenCalledWith(mockGainInfo, mockPlayerState.current.song) expect(mockCalculateGain).toHaveBeenCalledWith(
expect(result.current.gainNode.gain.setValueAtTime).toHaveBeenCalledWith(0.8, 0) mockGainInfo,
mockPlayerState.current.song,
)
expect(result.current.gainNode.gain.setValueAtTime).toHaveBeenCalledWith(
0.8,
0,
)
}) })
it('should handle Web Audio API errors gracefully', () => { it('should handle Web Audio API errors gracefully', () => {
@ -89,12 +95,12 @@ describe('useReplayGain', () => {
const mockGainInfo = { gainMode: 'track' } const mockGainInfo = { gainMode: 'track' }
const { result } = renderHook(() => const { result } = renderHook(() =>
useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo),
) )
expect(consoleSpy).toHaveBeenCalledWith( expect(consoleSpy).toHaveBeenCalledWith(
'Error initializing Web Audio API for replay gain:', 'Error initializing Web Audio API for replay gain:',
expect.any(Error) expect.any(Error),
) )
expect(result.current.context).toBeNull() expect(result.current.context).toBeNull()
@ -128,10 +134,13 @@ describe('useReplayGain', () => {
})) }))
const { result } = renderHook(() => 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() consoleSpy.mockRestore()
}) })
@ -142,7 +151,7 @@ describe('useReplayGain', () => {
const mockGainInfo = { gainMode: 'off' } const mockGainInfo = { gainMode: 'off' }
const { result } = renderHook(() => const { result } = renderHook(() =>
useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo) useReplayGain(mockAudioInstance, mockPlayerState, mockGainInfo),
) )
expect(global.AudioContext).not.toHaveBeenCalled() expect(global.AudioContext).not.toHaveBeenCalled()

View File

@ -35,7 +35,7 @@ describe('useScrobbling', () => {
it('should initialize with default state', () => { it('should initialize with default state', () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
expect(result.current.startTime).toBeNull() expect(result.current.startTime).toBeNull()
@ -47,7 +47,7 @@ describe('useScrobbling', () => {
it('should handle audio progress and scrobble when conditions are met', () => { it('should handle audio progress and scrobble when conditions are met', () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
const mockInfo = { const mockInfo = {
@ -68,7 +68,7 @@ describe('useScrobbling', () => {
it('should not scrobble radio streams', () => { it('should not scrobble radio streams', () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
const mockInfo = { const mockInfo = {
@ -87,7 +87,7 @@ describe('useScrobbling', () => {
it('should reset scrobbling state on track change', () => { it('should reset scrobbling state on track change', () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
// Set initial state // Set initial state
@ -114,7 +114,7 @@ describe('useScrobbling', () => {
it('should handle audio ended and perform keepalive', async () => { it('should handle audio ended and perform keepalive', async () => {
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
const mockInfo = { trackId: 'track1' } const mockInfo = { trackId: 'track1' }
@ -125,7 +125,9 @@ describe('useScrobbling', () => {
expect(result.current.scrobbled).toBe(false) expect(result.current.scrobbled).toBe(false)
expect(result.current.startTime).toBeNull() 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', () => { it('should handle scrobbling errors gracefully', () => {
@ -136,7 +138,7 @@ describe('useScrobbling', () => {
}) })
const { result } = renderHook(() => const { result } = renderHook(() =>
useScrobbling(mockPlayerState, mockDispatch, mockDataProvider) useScrobbling(mockPlayerState, mockDispatch, mockDataProvider),
) )
const mockInfo = { const mockInfo = {
@ -150,7 +152,10 @@ describe('useScrobbling', () => {
result.current.onAudioProgress(mockInfo) 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 expect(result.current.scrobbled).toBe(true) // Still sets to true despite error
consoleSpy.mockRestore() consoleSpy.mockRestore()