amplitudejs icon indicating copy to clipboard operation
amplitudejs copied to clipboard

Playlist shuffle highlights wrong track

Open floft opened this issue 4 years ago • 1 comments

Issue description

When playlist shuffle is enabled, clicking next plays the next track in the shuffle list but highlights the next track sequentially (even if that's not the one playing).

Environment

  • What's My Browser Support link: https://www.whatsmybrowser.org/b/EZWH3
  • Amplitude.js Version: 5.2.0

Steps to reproduce the issue

  1. Visit https://521dimensions.com/open-source/amplitudejs/docs/examples/multiple-playlists.html
  2. Enable Shuffle on the first playlist/player
  3. Show the playlist/queue (by clicking on the top right icon of the player)
  4. Click play
  5. Click next

What is expected?

Track playing is some random track, e.g. Terrain. The bottom of the player shows the correct track is playing, but if you look at the queue it shows track two is playing. See screenshot.

Link to where issue can be reproduced

https://521dimensions.com/open-source/amplitudejs/docs/examples/multiple-playlists.html

Additional details / screenshots

image

In the above screenshot, Terrain is the correct random next track but notice that the highlighted track is track number two rather than Terrain.

Code

https://github.com/521dimensions/amplitudejs/blob/072fa4ee1f5d7d0711a50397a631bd63e043322c/src/visual/containerElements.js#L79-L94

Line 90 looks like the code that would highlight the correct track, but it won't be executed because if this else block is executed (all of the above code, starting on Line 79) then config.active_playlist != null && config.active_playlist != '' is always going to be true (because if it wasn't, the if block on Line 41 would be executed instead). Thus, Line 85 will always be executed and ends up selecting the wrong track if playlist shuffle is enabled. I'm going to guess (and in my tests this fixed the problem) that Line 84 should be if (... && direct) or just if (direct), following the same logic as in the non-playlist code a few lines earlier:

https://github.com/521dimensions/amplitudejs/blob/072fa4ee1f5d7d0711a50397a631bd63e043322c/src/visual/containerElements.js#L45-L57

floft avatar Nov 20 '20 01:11 floft

Hi @floft I want to work on this issue, can you please assign me this.

mahi072 avatar Jan 25 '23 04:01 mahi072