Skip to content

Commit 0c65504

Browse files
ariigoogle-labs-jules[bot]
arii
and
google-labs-jules[bot]
authored
Refactor: Enhance Spotify Optimistic UI Architecture (#9664)
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: arii <342438+arii@users.noreply.github.com>
1 parent 5384589 commit 0c65504

11 files changed

Lines changed: 192 additions & 353 deletions

File tree

‎app/client/control/components/SpotifyControls.tsx‎

Lines changed: 1 addition & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import Select from '@mui/material/Select'
1111
import Typography from '@mui/material/Typography'
1212
import { useRouter } from 'next/navigation'
1313
import { useCallback, useEffect, useMemo, useRef, useState } from 'react'
14-
import throttle from 'lodash.throttle'
1514
import { clampVolume } from '@/utils/audioManager'
1615
import { useWebSocket } from '@/context/WebSocketContext'
1716
import { useSpotifyCommand } from '@/hooks/useSpotifyCommand'
@@ -33,10 +32,6 @@ const SpotifyControls = () => {
3332
const { devices = [] } = spotifyData
3433
const lastSentVolumeRef = useRef<string | null>(null)
3534
const [selectedDeviceId, setSelectedDeviceId] = useState<string>('')
36-
const [isSliding, setIsSliding] = useState(false)
37-
const prevActiveIdRef = useRef<string | undefined>(undefined)
38-
const lastVolumeSyncTimeRef = useRef<number>(0)
39-
const hasPendingSendRef = useRef<boolean>(false)
4035
const [optimisticIsPlaying, setOptimisticIsPlaying] = useState<
4136
boolean | null
4237
>(null)
@@ -79,27 +74,6 @@ const SpotifyControls = () => {
7974
[connectionStatus, resolveTargetDeviceId, executeSpotify]
8075
)
8176

82-
const throttledSendVolume = useMemo(
83-
() =>
84-
throttle((val: number) => {
85-
sendVolumeCommand(val)
86-
}, 200),
87-
[sendVolumeCommand]
88-
)
89-
90-
useEffect(() => {
91-
return () => {
92-
throttledSendVolume.cancel()
93-
}
94-
}, [throttledSendVolume])
95-
96-
const handleThrottledVolumeChange = useCallback(
97-
(val: number) => {
98-
throttledSendVolume(val)
99-
},
100-
[throttledSendVolume]
101-
)
102-
10377
const {
10478
displayVolume,
10579
isMuted,
@@ -112,14 +86,6 @@ const SpotifyControls = () => {
11286
sendVolumeCommand
11387
)
11488

115-
const handleVolumeSlide = useCallback(
116-
(val: number) => {
117-
handleVolumeChange(val)
118-
handleThrottledVolumeChange(val)
119-
},
120-
[handleVolumeChange, handleThrottledVolumeChange]
121-
)
122-
12389
const handleTrackSelect = (uri: string) => {
12490
const targetDeviceId = resolveTargetDeviceId()
12591
executeSpotify('PLAY', {
@@ -212,14 +178,12 @@ const SpotifyControls = () => {
212178
command === 'NEXT' ||
213179
command === 'PREVIOUS'
214180
) {
215-
// Optimistic UI update for Play/Pause
216181
if (command === 'PLAY') {
217182
setOptimisticIsPlaying(true)
218183
} else if (command === 'PAUSE') {
219184
setOptimisticIsPlaying(false)
220185
}
221186

222-
// Clear existing timer if any
223187
if (playbackGraceTimerRef.current) {
224188
clearTimeout(playbackGraceTimerRef.current)
225189
}
@@ -235,7 +199,6 @@ const SpotifyControls = () => {
235199
[sendSpotifyCommand]
236200
)
237201

238-
// Cleanup timers on unmount
239202
useEffect(() => {
240203
return () => {
241204
if (playbackGraceTimerRef.current) {
@@ -244,55 +207,6 @@ const SpotifyControls = () => {
244207
}
245208
}, [])
246209

247-
const handleVolumeChange = useCallback(
248-
(val: number) => {
249-
setIsSliding(true)
250-
setVolume(val)
251-
if (connectionStatus !== 'Connected') {
252-
const now = Date.now()
253-
// Throttle warning to once every 3 seconds to avoid spam during sliding
254-
if (now - lastWarningTimeRef.current > 3000) {
255-
showWarning('Changes not saved: Offline')
256-
lastWarningTimeRef.current = now
257-
}
258-
}
259-
},
260-
[connectionStatus, showWarning, setVolume]
261-
)
262-
263-
const sendVolumeCommand = useCallback(
264-
(value: number) => {
265-
if (connectionStatus !== 'Connected') return
266-
const targetDeviceId = resolveTargetDeviceId()
267-
268-
// Prevent sending volume command if no device is targeted
269-
if (!targetDeviceId) return
270-
271-
const sanitized = clampVolume(value)
272-
const messageKey = `${targetDeviceId}:${sanitized}`
273-
if (lastSentVolumeRef.current === messageKey) return
274-
275-
hasPendingSendRef.current = true
276-
lastVolumeSyncTimeRef.current = Date.now()
277-
278-
executeSpotify('SET_VOLUME', {
279-
volume: sanitized,
280-
deviceId: targetDeviceId,
281-
})
282-
283-
lastSentVolumeRef.current = messageKey
284-
},
285-
[connectionStatus, resolveTargetDeviceId, executeSpotify]
286-
)
287-
288-
const handleVolumeChangeCommitted = useCallback(
289-
(val: number) => {
290-
setIsSliding(false)
291-
sendVolumeCommand(val)
292-
},
293-
[sendVolumeCommand]
294-
)
295-
296210
useEffect(() => {
297211
if (connectionStatus !== 'Connected') {
298212
lastSentVolumeRef.current = null
@@ -367,7 +281,7 @@ const SpotifyControls = () => {
367281
<VolumeSlider
368282
volume={displayVolume}
369283
muted={isMuted}
370-
onVolumeChange={handleVolumeSlide}
284+
onVolumeChange={handleVolumeChange}
371285
onVolumeChangeCommitted={handleVolumeChangeCommitted}
372286
onToggleMute={handleToggleMute}
373287
showValue

‎components/shared/VolumeSlider.tsx‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,15 @@ interface VolumeSliderProps {
2626
}
2727

2828
const StyledSlider = styled(Slider, {
29-
shouldForwardProp: (prop) => prop !== 'sliderColor' && prop !== 'scaleFactor',
30-
})<{ sliderColor?: string; scaleFactor: number }>(
31-
({ theme, sliderColor, scaleFactor }) => ({
29+
shouldForwardProp: (prop) => prop !== 'sliderColor' && prop !== 'size',
30+
})<{ sliderColor?: string; size?: 'small' | 'medium' }>(
31+
({ theme, sliderColor, size }) => ({
3232
color: sliderColor || theme.palette.primary.main,
33-
height: 8 * scaleFactor,
33+
height: theme.spacing(size === 'small' ? 0.75 : 1),
3434
'& .MuiSlider-thumb': {
3535
backgroundColor: 'white',
36-
width: 28 * scaleFactor,
37-
height: 28 * scaleFactor,
36+
width: theme.spacing(size === 'small' ? 2.5 : 3.5),
37+
height: theme.spacing(size === 'small' ? 2.5 : 3.5),
3838
boxShadow: '0 2px 4px rgba(0,0,0,0.3)',
3939
'&:hover, &.Mui-focusVisible': {
4040
boxShadow: sliderColor
@@ -51,7 +51,9 @@ const StyledSlider = styled(Slider, {
5151
transform: 'translate(-50%, -50%)',
5252
},
5353
},
54-
'& .MuiSlider-track, .MuiSlider-rail': { height: 8 * scaleFactor },
54+
'& .MuiSlider-track, .MuiSlider-rail': {
55+
height: theme.spacing(size === 'small' ? 0.75 : 1),
56+
},
5557
'& .MuiSlider-rail': { opacity: 0.3 },
5658
})
5759
)
@@ -84,8 +86,6 @@ const VolumeSlider: React.FC<VolumeSliderProps> = ({
8486
[onVolumeChangeCommitted]
8587
)
8688

87-
const SCALE_FACTOR = size === 'small' ? 0.75 : 1
88-
8989
return (
9090
<Stack
9191
direction="row"
@@ -107,7 +107,7 @@ const VolumeSlider: React.FC<VolumeSliderProps> = ({
107107
sx={{
108108
color: muted ? 'error.main' : 'grey.400',
109109
'&:hover': { color: 'white' },
110-
padding: `${12 * SCALE_FACTOR}px`,
110+
padding: (theme) => theme.spacing(size === 'small' ? 1 : 1.5),
111111
}}
112112
aria-label={muted ? 'Unmute' : 'Mute'}
113113
data-testid="volume-slider-mute-button"
@@ -123,7 +123,7 @@ const VolumeSlider: React.FC<VolumeSliderProps> = ({
123123
value={muted ? 0 : volume}
124124
onChange={handleVolumeChange}
125125
onChangeCommitted={handleVolumeChangeCommitted}
126-
scaleFactor={SCALE_FACTOR}
126+
size={size}
127127
disabled={disabled}
128128
sliderColor={sliderColor}
129129
aria-label="Volume control"

‎constants/spotify.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,5 @@ export const HRM_WEB_PLAYER_NAME = 'HRM Web Player'
88
export const SPOTIFY_DEFAULT_TOKEN_EXPIRY_S = 3600
99

1010
// Centralized constants for Spotify integration
11-
export const VOLUME_SYNC_GRACE_PERIOD_MS = 3000
1211
export const SPOTIFY_BRAND_COLOR = '#1DB954'
1312
export const SYNC_LOCK_DURATION = 2000

‎context/AudioContext.tsx‎

Lines changed: 2 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,7 @@
11
'use client'
22

3-
import {
4-
createContext,
5-
useContext,
6-
useCallback,
7-
useEffect,
8-
useRef,
9-
useState,
10-
} from 'react'
11-
import { audioManager, clampVolume } from '@/utils/audioManager'
3+
import { createContext, useContext } from 'react'
4+
import { useAudioPreference } from '@/hooks/useAudioPreference'
125

136
export interface AudioContextType {
147
volume: number
@@ -20,115 +13,6 @@ export interface AudioContextType {
2013

2114
const AudioContext = createContext<AudioContextType | undefined>(undefined)
2215

23-
const STORAGE_KEY_VOL = 'hrm-preferred-volume'
24-
const STORAGE_KEY_MUTE = 'hrm-muted'
25-
26-
const useAudioPreference = (defaultVolume = 70) => {
27-
const sanitizedDefault = clampVolume(defaultVolume)
28-
const lastVolumeRef = useRef(sanitizedDefault)
29-
30-
const [volume, setVolumeState] = useState(sanitizedDefault)
31-
const [muted, setMutedState] = useState(false)
32-
const [isLoaded, setIsLoaded] = useState(false)
33-
34-
useEffect(() => {
35-
try {
36-
const storedMute = window.localStorage.getItem(STORAGE_KEY_MUTE)
37-
const storedVol = window.localStorage.getItem(STORAGE_KEY_VOL)
38-
39-
const isMuted = storedMute === 'true'
40-
const preferredVolume =
41-
storedVol !== null ? clampVolume(Number(storedVol)) : sanitizedDefault
42-
lastVolumeRef.current = preferredVolume
43-
setMutedState(isMuted)
44-
setVolumeState(isMuted ? 0 : preferredVolume)
45-
} catch (error) {
46-
console.warn('Failed to read audio preferences from localStorage:', error)
47-
} finally {
48-
setIsLoaded(true)
49-
}
50-
}, [sanitizedDefault])
51-
52-
useEffect(() => {
53-
if (isLoaded) {
54-
audioManager.setMuted(muted)
55-
audioManager.setVolume(volume)
56-
}
57-
}, [volume, muted, isLoaded])
58-
59-
const setVolume = useCallback(
60-
(value: number) => {
61-
const sanitized = clampVolume(value)
62-
setVolumeState(sanitized)
63-
if (sanitized > 0) {
64-
lastVolumeRef.current = sanitized
65-
setMutedState(false)
66-
} else {
67-
setMutedState(true)
68-
}
69-
window.dispatchEvent(
70-
new CustomEvent('hrm:volumeChange', { detail: sanitized })
71-
)
72-
},
73-
[setMutedState]
74-
)
75-
76-
const toggleMute = useCallback(() => {
77-
const isMuting = !muted
78-
setMutedState(isMuting)
79-
try {
80-
window.localStorage.setItem(STORAGE_KEY_MUTE, String(isMuting))
81-
if (isMuting) {
82-
if (volume > 0) {
83-
lastVolumeRef.current = volume
84-
window.localStorage.setItem(STORAGE_KEY_VOL, String(volume))
85-
}
86-
setVolumeState(0)
87-
} else {
88-
setVolumeState(lastVolumeRef.current)
89-
}
90-
window.dispatchEvent(
91-
new CustomEvent('hrm:muteChange', { detail: isMuting })
92-
)
93-
} catch (error) {
94-
console.warn('Could not persist mute preference:', error)
95-
}
96-
}, [muted, volume])
97-
98-
useEffect(() => {
99-
const handleStorageChange = (e: StorageEvent) => {
100-
if (e.key === STORAGE_KEY_VOL && e.newValue !== null) {
101-
setVolumeState(clampVolume(Number(e.newValue)))
102-
}
103-
if (e.key === STORAGE_KEY_MUTE && e.newValue !== null) {
104-
setMutedState(e.newValue === 'true')
105-
}
106-
}
107-
108-
const handleLocalVolume = (e: Event) => {
109-
const customEvent = e as CustomEvent
110-
setVolumeState(customEvent.detail)
111-
}
112-
113-
const handleLocalMute = (e: Event) => {
114-
const customEvent = e as CustomEvent
115-
setMutedState(customEvent.detail)
116-
}
117-
118-
window.addEventListener('storage', handleStorageChange)
119-
window.addEventListener('hrm:volumeChange', handleLocalVolume)
120-
window.addEventListener('hrm:muteChange', handleLocalMute)
121-
122-
return () => {
123-
window.removeEventListener('storage', handleStorageChange)
124-
window.removeEventListener('hrm:volumeChange', handleLocalVolume)
125-
window.removeEventListener('hrm:muteChange', handleLocalMute)
126-
}
127-
}, [])
128-
129-
return { volume, setVolume, muted, toggleMute, isLoaded }
130-
}
131-
13216
export const AudioProvider = ({ children }: { children: React.ReactNode }) => {
13317
const volumePreference = useAudioPreference()
13418
return (

0 commit comments

Comments
 (0)