Add Play Next & Add to Queue for songs and albums. #481
Conversation
|
Regarding shuffle, yeah it's a mess, mostly due to the way the queue is handled in just_audio/audio_service. Given that it's added functionality it's probably fine if there are issues. Would you mind posting screenshots of how the user interface for this looks? Can't build it at the moment :) |
|
@Chaphasilor Here's what it looks like on my device. |
|
@ray-kast so it's only an option for albums when long-pressing on the album in a list of albums, correct? I think that's fine for the moment :) |
|
Okay, after delving into Specifically, I just want to make sure the use case of shuffling all songs then selecting Play Next on an item doesn't just send the inserted items into queue purgatory, since I think that particular use case is a significant portion of the added value of a Play Next button. |
|
Okay so it turns out I'm blind and just now noticed this discussion. I bandaid-fixed shuffle breaking everything and added all the new strings to |
|
Not sure if that PR is still active. You could clone it and test it yourself, but if that implementation isn't significantly better, I think this one should be merged |
Checked out the branch, it doesn't merge onto the current upstream. Combed through it and identified a handful of things it does that this PR doesn't:
The latter two seem better suited for separate UX tickets, especially in the case of handling a paused/stopped player since most actions currently don't attempt to resume playback. |
|
Thanks for working on this! I've closed my PR (#142) since I don't have time to revive it, but I hope that at least the discussions there were valuable. |
For sure! I read through the comments and largely agree with the general sentiment there (and I'd like to think the code of this PR does too, given that we both landed on the same |
| @Deprecated("Use addQueueItems instead") | ||
| Future<void> addQueueItem(BaseItemDto item) async { | ||
| await addQueueItems([item]); | ||
| } |
There was a problem hiding this comment.
Should this be marked deprecated? It would be more ergonomic to allow adding single items, and would remove the need to change everything that adds a single item
There was a problem hiding this comment.
My initial train of thought was the few extra characters for one less function call on the stack and a slightly more concise API surface was a worthwhile tradeoff, but I don't really want to die on that hill (and truth be told I don't know much about inlining in modern Dart) so I can drop the deprecated attributes if desired.
| @Deprecated("Use addQueueItems instead") | ||
| Future<void> addQueueItem(MediaItem mediaItem) async { | ||
| addQueueItems([mediaItem]); | ||
| } |
There was a problem hiding this comment.
Same as the note on AudioServiceHelper for completeness 🙃
|
Just gave this a test, it works nicely. UX will be improved in the redesign, where a more complex implementation is being made (#484). Merging :) |
|
Awesome! I use this app daily so I'd be more than happy to get involved with future improvements. |
I've seen that the UI redesign project intends to cover this, but I thought I'd offer this as a quality-of-life backport for the current UI until that gets released. This partially covers #221, and maybe covers #92 depending on opinion. Apologies for any style errors (and for the faux pas of swinging in out of the blue with a PR), my Dart is pretty rusty.
This has been tested on iOS Simulator and an iPhone 11 running iOS 16.6.
Open questions and future work:
Iconsdoesn't really offer anything better thanqueue_musicThe existing Add to Queue behaves badly with shuffle enabled (is there an open issue for this?), and the added ones behave worsesee linked commentDoes it make sense (and is it worth the effort) to calculate the preimage ofshuffleIndiceswhen modifying the queue with shuffle enabled?