react-native-track-player icon indicating copy to clipboard operation
react-native-track-player copied to clipboard

Next button disappears on last track in queue

Open austinried opened this issue 3 years ago • 13 comments

Describe the Bug Using the latest v2.2.0-rc3 on Android, when playing a queue with multiple tracks and reaching the last track, the next button disappears in the notification. This happens in both the compact and normal notifications, and the next button will reappear if you skip to a previous track.

Related discussion on this issue can be found here: https://github.com/doublesymmetry/react-native-track-player/discussions/1264#discussioncomment-2589155

Steps To Reproduce

  1. Run the example app
  2. Comment out the line that sets the repeat mode
  3. Skip to the last song

Environment Info:

Paste the exact react-native-track-player version you are using

v2.2.0-rc3

Real device? Or simulator?

Real device (Pixel 4) and simulator (Pixel 3)

What OS are you running?

I've tried it on Android 12.1 and Android 9

How I can Help

Have you investigated the underlying JS or Swift/Android code causing this bug?

I've tried to look into KotlinAudio and the changes made in the DoubleSymmetry branch of ExoPlayer a little bit, but I have not yet found anything obvious that would be causing this.

Can you create a Pull Request with a fix?

Possibly with some pointing in the right direction as to where the issue might be.

austinried avatar Apr 23 '22 12:04 austinried

@mpivchev this looks like a rewrite issue.

jspizziri avatar Apr 23 '22 17:04 jspizziri

@austinried what would you do with that button? RNTP does not do anything when you call skip to next on the last track. So is not having the button an issue?

Does the button appear if the mode is set to repeat queue or track?

dcvz avatar Apr 23 '22 23:04 dcvz

It appears to stay there in repeat queue mode, I haven't tested repeat track.

Right now in my app I just catch the error and do nothing on the last track, what I probably should be doing is the same thing that I do when the last track ends without skipping, which is loop back to the first track and pause. What I really don't want though is the current behavior, which is that it disappears. To remove the button is confusing, and to slide another button over in its place makes the UI inconsistent and causes misclicks.

If it's possible to reduce the button's opacity/make it visually disabled, that's much better I think. Or letting the app decide when to disable the button is even better, but just reverting to the previous behavior would be okay too.

austinried avatar Apr 23 '22 23:04 austinried

I've done some more tests: when there is a single track in the queue, regardless of repeat mode, there is no next button. When there are multiple tracks in the queue, regardless of repeat track, next takes you to the next track in the queue until you're at the end, and then it disappears.

austinried avatar Apr 24 '22 12:04 austinried

So to summarize this:

  1. If you don't have repeat mode, the next button disappears (which is intended)
  2. If you DO have repeat mode on, the next button still disappears (when it shouldn't).

The issue you are having is with the 2nd point. Correct?

mpivchev avatar Apr 26 '22 08:04 mpivchev

No not quite sorry, I was just describing current behavior. My issue is that I don't want it to disappear ever, regardless of repeat mode and even when it's on the last track in the queue.

Also, I thought this disappearing was not intended? Is it something that KotlinAudio is controlling?

austinried avatar Apr 26 '22 08:04 austinried

No, it's ExoPlayer that controls this. And it is intended.

mpivchev avatar Apr 26 '22 09:04 mpivchev

I see, well I've looked into the ExoPlayer repo and found a few issues related to this pretty suspect decision, mainly this one that also talks about how to override it: https://github.com/google/ExoPlayer/issues/7221#issuecomment-613492175

I think I already made clear above why I think this is bad UX, but I would also put forward that this should be overriden for this library in particular since it changes behavior in a way that could be considered breaking. I believe there are also a number of people who use this library with one track in the queue at all times in order to get better control over the queue in their own code, and this would be an even bigger change for that use case.

austinried avatar Apr 26 '22 10:04 austinried

IMO since we're doing the refactor in as a minor release we should try to mirror the functionality as before. It sounds like this could be considered a breaking change between the versions which we don't want.

jspizziri avatar Apr 26 '22 10:04 jspizziri

@austinried point 2, definitely sounds like a bug.. point 1 is a behaviour that makes sense IMO but I understand its different than current behaviour.

Right now we're tackling a crucial bug in the module - @austinried if you've found a workaround or way to keep it enabled, would you consider opening a PR for it? That would help out a lot.

dcvz avatar Apr 26 '22 10:04 dcvz

Sure, I've just found those methods in that ExoPlayer thread so far but I can give them a shot and see if I can get one working.

austinried avatar Apr 26 '22 11:04 austinried

@austinried superman

jspizziri avatar Apr 26 '22 11:04 jspizziri

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

github-actions[bot] avatar Jul 30 '22 02:07 github-actions[bot]