diff --git a/app/client/control/components/SpotifyControls.tsx b/app/client/control/components/SpotifyControls.tsx index 8e1501b751..cc323e89f6 100644 --- a/app/client/control/components/SpotifyControls.tsx +++ b/app/client/control/components/SpotifyControls.tsx @@ -4,6 +4,7 @@ import MusicNote from '@mui/icons-material/MusicNote' import LibraryMusic from '@mui/icons-material/LibraryMusic' import Box from '@mui/material/Box' import Button from '@mui/material/Button' +import CircularProgress from '@mui/material/CircularProgress' import ControlCard from '@/components/shared/ControlCard' import CardContent from '@mui/material/CardContent' import FormControl from '@mui/material/FormControl' @@ -28,6 +29,12 @@ import { SPOTIFY_BRAND_COLOR } from '@/constants/spotify' const VOLUME_SLIDER_SX = { mt: 3, mb: 1 } +const INVALID_TRACK_NAMES = new Set([ + 'Awaiting Login...', + '', + 'No Track Playing', +]) + const SpotifyControls = () => { const router = useRouter() const { spotifyData, connectionStatus, sendData, spotifyServiceInitialized } = @@ -68,12 +75,11 @@ const SpotifyControls = () => { router.push('/client/spotify-selection') } - const hasSpotifyData = - spotifyData.playback.track.name !== 'Awaiting Login...' && - spotifyData.playback.track.name !== '' && - spotifyData.playback.track.name !== 'No Track Playing' + const shouldShowControls = + spotifyServiceInitialized && + (!INVALID_TRACK_NAMES.has(spotifyData.playback.track.name) || + devices.some((d) => d.is_active)) - // 3. Request devices on mount or connection useEffect(() => { if (connectionStatus === 'Connected' && spotifyServiceInitialized) { sendData({ @@ -83,18 +89,38 @@ const SpotifyControls = () => { } }, [connectionStatus, sendData, spotifyServiceInitialized]) - // 4. Sync selected device and volume with active device + useEffect(() => { + let timeoutId: ReturnType | null = null + + const handleFocus = () => { + if (timeoutId) { + clearTimeout(timeoutId) + } + + timeoutId = setTimeout(() => { + if (connectionStatus === 'Connected' && spotifyServiceInitialized) { + sendData({ type: 'SPOTIFY_COMMAND', command: 'GET_DEVICES' }) + } + }, 300) + } + + window.addEventListener('focus', handleFocus) + return () => { + window.removeEventListener('focus', handleFocus) + if (timeoutId) { + clearTimeout(timeoutId) + } + } + }, [connectionStatus, spotifyServiceInitialized, sendData]) + useEffect(() => { const activeDevice = devices.find((d) => d.is_active) const activeId = activeDevice?.id - // Helper: determine if device should be updated to activeId const shouldUpdateToActive = () => { - // Initial sync or active device changed externally if (!prevActiveIdRef.current || activeId !== prevActiveIdRef.current) { return Boolean(activeId) } - // Selected device no longer exists or no device selected const selectedStillExists = devices.some((d) => d.id === selectedDeviceId) return (!selectedDeviceId || !selectedStillExists) && Boolean(activeId) } @@ -104,7 +130,6 @@ const SpotifyControls = () => { } prevActiveIdRef.current = activeId - // Sync Volume (if not dragging and not within grace period after send) // We rely on the server as the source of truth for volume, but use a grace period // to prevent local sliders from "jumping" while the user is actively adjusting them. const playbackVolume = spotifyData.playback.volume_percent @@ -124,7 +149,6 @@ const SpotifyControls = () => { return } - // Clear pending flag after grace period if ( hasPendingSendRef.current && timeSinceLastVolumeSend >= VOLUME_SYNC_GRACE_PERIOD_MS @@ -139,9 +163,8 @@ const SpotifyControls = () => { } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [devices]) // Rely on devices update to trigger sync + }, [devices]) - // Auto-select HRM Web Player if no active device is available useEffect(() => { if ( devices.length > 0 && @@ -240,7 +263,6 @@ const SpotifyControls = () => { setVolume(val) if (connectionStatus !== 'Connected') { const now = Date.now() - // Throttle warning to once every 3 seconds to avoid spam during sliding if (now - lastWarningTimeRef.current > 3000) { showWarning('Changes not saved: Offline') lastWarningTimeRef.current = now @@ -255,7 +277,6 @@ const SpotifyControls = () => { if (connectionStatus !== 'Connected') return const targetDeviceId = resolveTargetDeviceId() - // Prevent sending volume command if no device is targeted if (!targetDeviceId) return const sanitized = clampVolume(value) @@ -316,7 +337,7 @@ const SpotifyControls = () => { - {hasSpotifyData ? ( + {shouldShowControls ? ( <> { )} diff --git a/tests/playwright/vrt-dashboard.spec.ts-snapshots/dashboard-tablet-chromium-linux.png b/tests/playwright/vrt-dashboard.spec.ts-snapshots/dashboard-tablet-chromium-linux.png new file mode 100644 index 0000000000..12520994e6 Binary files /dev/null and b/tests/playwright/vrt-dashboard.spec.ts-snapshots/dashboard-tablet-chromium-linux.png differ diff --git a/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/select-music-button-hover-chromium-linux.png b/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/select-music-button-hover-chromium-linux.png index 078ff62f28..8a4ed206ad 100644 Binary files a/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/select-music-button-hover-chromium-linux.png and b/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/select-music-button-hover-chromium-linux.png differ diff --git a/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/spotify-controls-logged-out-chromium-linux.png b/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/spotify-controls-logged-out-chromium-linux.png index 1babefc9bd..163b9fd541 100644 Binary files a/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/spotify-controls-logged-out-chromium-linux.png and b/tests/playwright/vrt-spotify-controls.spec.ts-snapshots/spotify-controls-logged-out-chromium-linux.png differ diff --git a/tests/unit/app/client/control/components/SpotifyControls.repro.test.tsx b/tests/unit/app/client/control/components/SpotifyControls.repro.test.tsx new file mode 100644 index 0000000000..557efa435e --- /dev/null +++ b/tests/unit/app/client/control/components/SpotifyControls.repro.test.tsx @@ -0,0 +1,80 @@ +/** + * @jest-environment jsdom + */ +import { render, screen } from '@testing-library/react' +import { useWebSocket } from '@/context/WebSocketContext' +import SpotifyControls from '@/app/client/control/components/SpotifyControls' +import { + createMockSpotifyData, + createMockSpotifyDevice, +} from '@/tests/test-utils' +import '@testing-library/jest-dom' + +jest.mock('next/navigation', () => ({ + useRouter: jest.fn(), +})) + +jest.mock('@/context/WebSocketContext', () => ({ + useWebSocket: jest.fn(), +})) + +// Mock the volume preference hook +jest.mock('@/hooks/useVolumePreference', () => ({ + __esModule: true, + default: jest.fn(() => ({ + volume: 50, + muted: false, + setVolume: jest.fn(), + toggleMute: jest.fn(), + })), + clampVolume: (v: number) => v, +})) + +// Mock the snackbar hook +jest.mock('@/hooks/useAppSnackbar', () => ({ + useAppSnackbar: jest.fn(() => ({ + showWarning: jest.fn(), + })), +})) + +describe('SpotifyControls Reproduction', () => { + it('shows controls when an active device exists even if track is not playing (bug reproduction)', () => { + ;(useWebSocket as jest.Mock).mockReturnValue({ + connectionStatus: 'Connected', + spotifyData: createMockSpotifyData({ + playback: { + ...createMockSpotifyData().playback, + track: { + id: null, + name: 'Awaiting Login...', + artist: '', + albumName: '', + albumArtUrl: '', + }, + is_playing: false, + }, + devices: [ + createMockSpotifyDevice({ + id: '1', + name: 'Device 1', + is_active: true, + volume_percent: 50, + }), + ], + }), + sendData: jest.fn(), + spotifyServiceInitialized: true, + }) + + render() + + const selectMusicButton = screen.queryByTestId( + 'spotify-select-music-button' + ) + const playButton = screen.queryByTestId('spotify-play-pause') + + expect(selectMusicButton).not.toBeInTheDocument() + expect(playButton).toBeInTheDocument() + expect(playButton).toHaveAttribute('aria-label', 'Play') + }) +})