Refactor and improve song bottom sheet

[Lurux]

Intended changes:

• Reorder menu items to be in a more logical order
• Unify content of online, local, and player sheets
• Move some advanced player option to a submenu
• Use disabled buttons when artists or albums are unavailable instead of hiding them, so the menu stays consistent to improve muscle memory

Side effects:

When doing this I noticed the SongMenu, YoutubeSongMenu, and PlayerMenu were all essentially the same. They were refactored and merged into a single menu that takes a MediaMetadata as parameter.

The parts of the menu related to the current playback (aka. volume, pitch, etc) are displayed only in the player base on the inPlayer parameter.

This will reduce code complexity and ensure these menus are consistent moving forwards.

Side note: I only did the song menu for now because I don't love refactoring, but you can expect me to do stuff like this when I work on your project. I hope this helps !

[nvllz]

Hi. What's the "Related" thing about? It's grayed out for all the songs I checked. I also miss a "Start radio" in the menu grid inside the player.

This PR would fix a mess I couldn't figure out for hours, when tried to map songs without any result..

I think making a different .kt file for a player song menu would be better than checking the state of "inPlayer" variable. I would then get a new menu based on metadata (man I spent literal hours and couldn't map songs basing on this.. eh) for queue items, and stitch it with my Queue Improvements PR, which would allow for a much better menu than the actual one (see my latest comment there).

For anyone wondering, the new song menu inside player looks as follows:

[Lurux]

Hi. What's the "Related" thing about? It's grayed out for all the songs I checked. I also miss a "Start radio" in the menu grid inside the player.

Oh yeah sorry I forgot about that, it's related to another feature that I'm working on. Basically when showing the app around people complained it didn't have Youtube-style recommendations, so I intend this to take the results from "start radio", but show them as page that people can explore (similar to a playlist) instead of putting them in the queue.

Anyways, I had already added it to highlight the logic of the new button order when they show up as three columns, but I forgot to remove it before this PR.

As for the start radio button in the player, I didn't include it here because having it at the start would change the layout when opening the menu in the player, which is something I explicitly didn't want to do.

We can add it back to the hidden icons though, and/or this may change anyways if we decide to revamp the player at some point and we need to put more stuff in the menu (this is actually an idea that had been on my mind recently, as the current player seems a bit disorganized to me).

This PR would fix a mess I couldn't figure out for hours, when tried to map songs without any result..

When searching around while trying to understand the codebase, I stumbled upon some conversion functions in the models/MediaMetadata.kt file, but I have to admit I don't understand why there are have so many ways to represent videos, haha :sweat_smile:

I think making a different .kt file for a player song menu would be better than checking the state of "inPlayer" variable. I would then get a new menu based on metadata (man I spent literal hours and couldn't map songs basing on this.. eh) for queue items, and stitch it with my Queue Improvements PR, which would allow for a much better menu than the actual one (see my latest comment there).

I think it's better to have at least the common part of the menu in a single file, that will avoid code duplication and potential inconsistencies in the future. I agree it would be better to separate the "normal" and "player" specialization parts though, I just didn't bother doing it here because I don't know if it's going to be merged.

[nvllz]

I think it's better to have at least the common part of the menu in a single file, that will avoid code duplication and potential inconsistencies in the future. I agree it would be better to separate the "normal" and "player" specialization parts though, I just didn't bother doing it here because I don't know if it's going to be merged.

Sticking to this may only limit the possibilities. For queue song menu I'd like to have a button to play next, start radio would be useful too. This is really a PR that would save me like 10 hours total if you came up with this a month ago, hahah.. I can't imagine it won't even be considered merging. But I think the menu needs to be split among multiple files for beter handling. Having multiple 'if' clauses for player, playlist, queue and other sources of calling it will make the code not as readable as it may be when tailored to each menu needs. Or maybe there's some easier way to deal with multiple different layouts in a single file?

It will look so bad when applied all the if (isPlayer || isQueue && !isPlaylist) multiple times in different states, and we still need "Play next" when called from the queue, but not in the player ;/

[Lurux]

Uh, I think the items in the queue can just use the same menu as in the library, can't they ? Also, yeah, I'll find a better way to handle the modes.

[nvllz]

For a song menu in a queue I'd prefer to also have the "Play Next" and "Start Radio" items. And a "Delete" for a song inside a playlist.

[Lurux]

(I mean, the library menu do have those... (?)) Good point for the delete button in playlists, I didn't realize it was missing xD

[Lurux]

Just so you know, I'm currently improving the menu so the code is much cleaner and the UI is further simplified, will convert to draft for now