fix: polish Spotify Track Selection UI optimization PR (#9547)

- Keep `duration` field alongside `duration_ms` for backward compatibility in API responses and types.
- Standardize `SpotifyPlaylistItem` type by simplifying `artists` to a `string` and removing redundant `imageUrl`.
- Remove redundant Play/Pause `IconButton` in `PlaylistTracksDisplay` and handle clicks on the row directly.
- Migrate index-based playback to URI-based playback in `PlaylistTracksDisplay`.
- Fix TypeScript error in `socketManager.ts` by replacing `as any` cast.
- Resolve unused imports linting issues.
- Update related unit tests to match architecture changes.

Co-authored-by: arii <342438+arii@users.noreply.github.com>

Author: google-labs-jules[bot]

app/api/spotify/playlists/[playlistId]/tracks/route.ts

@@ -104,6 +104,7 @@ async function getPlaylistTracks(
           images: track.album.images,
         },
         duration_ms: track.duration_ms,
+        duration: track.duration_ms, // Alias for backward compatibility
         uri: track.uri,
       }
     })

components/Playlist/PlaylistTracksDisplay.tsx

@@ -7,9 +7,6 @@ import Box from '@mui/material/Box'
 import Typography from '@mui/material/Typography'
 import CircularProgress from '@mui/material/CircularProgress'
 import Alert from '@mui/material/Alert'
-import IconButton from '@mui/material/IconButton'
-import PlayArrowIcon from '@mui/icons-material/PlayArrow'
-import PauseIcon from '@mui/icons-material/Pause'
 import List from '@mui/material/List'
 import Paper from '@mui/material/Paper'
 import Button from '@mui/material/Button'
@@ -60,10 +57,10 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => {
     fetchTracks(offset)
   }, [fetchTracks, offset])
 
-  const handlePlayTrack = (playlistUri: string, position: number) => {
+  const handlePlayTrack = (playlistUri: string, uri: string) => {
     executeSpotify('PLAY', {
       contextUri: playlistUri,
-      offset: { position },
+      offset: { uri },
     })
   }
 
@@ -111,7 +108,7 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => {
     <Box>
       <Paper>
         <List dense sx={{ width: '100%', bgcolor: 'background.paper', p: 0 }}>
-          {tracks.map((track, index) => {
+          {tracks.map((track) => {
             const isPlaying =
               spotifyData.playback.is_playing &&
               spotifyData.playback.track.id === track.id
@@ -125,32 +122,19 @@ const PlaylistTracksDisplay = ({ playlistId }: PlaylistTracksDisplayProps) => {
                 onClick={() =>
                   isPlaying
                     ? handlePause()
-                    : handlePlayTrack(playlistUri, index)
+                    : handlePlayTrack(playlistUri, track.uri)
                 }
                 secondaryAction={
-                  <Box sx={{ display: 'flex', alignItems: 'center' }}>
-                    <Typography
-                      variant="caption"
-                      sx={{ color: 'text.secondary', mr: 1 }}
-                    >
-                      {!!track.duration_ms &&
-                        formatDuration(track.duration_ms, {
-                          unit: 'milliseconds',
-                          format: 'MM:SS',
-                        })}
-                    </Typography>
-                    <IconButton
-                      onClick={() =>
-                        isPlaying
-                          ? handlePause()
-                          : handlePlayTrack(playlistUri, offset + index)
-                      }
-                      aria-label={isPlaying ? 'Pause' : 'Play'}
-                      size="small"
-                    >
-                      {isPlaying ? <PauseIcon /> : <PlayArrowIcon />}
-                    </IconButton>
-                  </Box>
+                  <Typography
+                    variant="caption"
+                    sx={{ color: 'text.secondary', mr: 1 }}
+                  >
+                    {!!track.duration_ms &&
+                      formatDuration(track.duration_ms, {
+                        unit: 'milliseconds',
+                        format: 'MM:SS',
+                      })}
+                  </Typography>
                 }
               />
             )

lib/spotify.ts

@@ -66,9 +66,5 @@ export async function refreshSpotifyToken(refreshToken: string) {
 }
 
 export const getArtistNames = (artists: SpotifyPlaylistItem['artists']) => {
-  if (!Array.isArray(artists)) return artists ?? 'Unknown'
-  return artists
-    .map((a) => (typeof a === 'string' ? a : a?.name))
-    .filter(Boolean)
-    .join(', ')
+  return artists ?? 'Unknown'
 }

tests/unit/app/api/spotify/playlists/[playlistId]/tracks/route.test.ts

@@ -59,6 +59,7 @@ describe('GET /api/spotify/playlists/[playlistId]/tracks', () => {
         name: 'Album 1',
       },
       duration_ms: 180000,
+      duration: 180000,
       uri: 'spotify:track:t1',
     })
   })

tests/unit/components/Playlist/PlaylistTracksDisplay.test.tsx

@@ -122,14 +122,14 @@ describe('PlaylistTracksDisplay', () => {
       </WebSocketContext.Provider>
     )
 
-    const playButton = await screen.findByRole('button', { name: /play/i })
-    fireEvent.click(playButton)
+    const trackButton = await screen.findByText('Track 1')
+    fireEvent.click(trackButton)
 
     expect(executeMock).toHaveBeenCalledWith(
       'PLAY',
       expect.objectContaining({
         contextUri: 'spotify:playlist:123',
-        offset: { position: 0 },
+        offset: { uri: 'spotify:track:t1' },
       })
     )
   })
@@ -173,8 +173,8 @@ describe('PlaylistTracksDisplay', () => {
       </WebSocketContext.Provider>
     )
 
-    const pauseButton = await screen.findByRole('button', { name: /pause/i })
-    fireEvent.click(pauseButton)
+    const trackButton = await screen.findByText('Track 1')
+    fireEvent.click(trackButton)
 
     expect(executeMock).toHaveBeenCalledWith('PAUSE')
   })

tests/unit/components/Spotify/PlaylistDetails.test.tsx

@@ -9,30 +9,23 @@ const mockFetch = jest.fn()
 global.fetch = mockFetch
 
 describe('PlaylistDetails', () => {
-  const mockTracksAsArray: Track[] = [
+  const mockTracksAsString: Track[] = [
     {
       id: '1',
       name: 'Track 1',
       uri: 'spotify:track:1',
-      artists: [{ name: 'Artist 1' }],
-      album: { name: 'Album 1' },
-      imageUrl: '',
+      artists: 'Artist 1',
+      album: { name: 'Album 1', images: [] },
     },
     {
       id: '2',
       name: 'Track 2',
       uri: 'spotify:track:2',
-      artists: [{ name: 'Artist 2' }],
-      album: { name: 'Album 2' },
-      imageUrl: '',
+      artists: 'Artist 2',
+      album: { name: 'Album 2', images: [] },
     },
   ]
 
-  const mockTracksAsString = mockTracksAsArray.map((track) => ({
-    ...track,
-    artists: track.artists.map((a) => a.name).join(', '),
-  })) as unknown as Track[]
-
   beforeEach(() => {
     jest.clearAllMocks()
   })
@@ -45,7 +38,7 @@ describe('PlaylistDetails', () => {
             () =>
               resolve({
                 ok: true,
-                json: () => Promise.resolve({ tracks: mockTracksAsArray }),
+                json: () => Promise.resolve({ tracks: mockTracksAsString }),
               }),
             100
           )
@@ -59,34 +52,6 @@ describe('PlaylistDetails', () => {
     await waitFor(() => expect(screen.queryByRole('progressbar')).toBeNull())
   })
 
-  it('displays the track list and handles play clicks when artists is an array', async () => {
-    mockFetch.mockResolvedValue({
-      ok: true,
-      json: () => Promise.resolve({ tracks: mockTracksAsArray }),
-    })
-    const onTrackPlay = jest.fn()
-
-    render(
-      <PlaylistDetails
-        playlistId="test-playlist-id"
-        onTrackPlay={onTrackPlay}
-      />
-    )
-
-    await waitFor(() => {
-      expect(screen.getByText('Track 1')).toBeInTheDocument()
-    })
-    expect(
-      screen.getByText('Artist 1 • Album 1', { exact: false })
-    ).toBeInTheDocument()
-    expect(screen.getByText('Track 2')).toBeInTheDocument()
-    expect(
-      screen.getByText('Artist 2 • Album 2', { exact: false })
-    ).toBeInTheDocument()
-
-    fireEvent.click(screen.getByText('Track 1'))
-    expect(onTrackPlay).toHaveBeenCalledWith('spotify:track:1')
-  })
 
   it('displays the track list and handles play clicks when artists is a string', async () => {
     mockFetch.mockResolvedValue({

types/core.ts

@@ -218,13 +218,13 @@ export interface SpotifyPlaylistItem {
   id: string
   name: string
   uri: string
-  artists?: { name: string }[] | string
+  artists?: string
   album?: {
     name: string
     images: { url: string; height: number; width: number }[]
   }
-  imageUrl?: string | null
   duration_ms?: number
+  duration?: number
 }
 
 /**

utils/socketManager.ts

@@ -447,7 +447,8 @@ const handleIncomingMessage = (
           spotifyCommandParams.contextUri = commandMsg.contextUri
         if (commandMsg.uri) spotifyCommandParams.uri = commandMsg.uri
         if (commandMsg.offset) {
-          spotifyCommandParams.offset = commandMsg.offset as any
+          spotifyCommandParams.offset =
+            commandMsg.offset as SpotifyCommandParameters['offset']
         }
 
         spotifyService.handleCommand(commandMsg.command, spotifyCommandParams)