Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 65 additions & 17 deletions app/client/control/components/SpotifyControls.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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 } =
Expand Down Expand Up @@ -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({
Expand All @@ -83,18 +89,38 @@ const SpotifyControls = () => {
}
}, [connectionStatus, sendData, spotifyServiceInitialized])

// 4. Sync selected device and volume with active device
useEffect(() => {
let timeoutId: ReturnType<typeof setTimeout> | 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)
}
Expand All @@ -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
Expand All @@ -124,7 +149,6 @@ const SpotifyControls = () => {
return
}

// Clear pending flag after grace period
if (
hasPendingSendRef.current &&
timeSinceLastVolumeSend >= VOLUME_SYNC_GRACE_PERIOD_MS
Expand All @@ -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 &&
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -316,7 +337,7 @@ const SpotifyControls = () => {
<SpotifySearchInput onTrackSelect={handleTrackSelect} />
</Box>

{hasSpotifyData ? (
{shouldShowControls ? (
<>
<Box
sx={{
Expand Down Expand Up @@ -421,8 +442,35 @@ const SpotifyControls = () => {
<Button
onClick={handleBrowseClick}
data-testid="spotify-select-music-button"
disabled={!spotifyServiceInitialized}
startIcon={
!spotifyServiceInitialized ? (
<CircularProgress
size={20}
color="inherit"
data-visualtest-hide="true"
/>
) : null
}
sx={{
width: '100%',
mt: 2,
backgroundColor: spotifyServiceInitialized
? SPOTIFY_BRAND_COLOR
: 'rgba(29, 185, 84, 0.5)',
color: 'white',
'&:hover': {
backgroundColor: '#1ed760',
},
'&.Mui-disabled': {
backgroundColor: 'rgba(29, 185, 84, 0.3)',
color: 'rgba(255, 255, 255, 0.5)',
},
}}
>
Select Music
{spotifyServiceInitialized
? 'Connect to HRM Web Player'
: 'Searching for Web Player...'}
</Button>
)}
</CardContent>
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -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(<SpotifyControls />)

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')
})
})
Loading