Add favorites

[nt4f04uNd]

Also fixes #46

New button in player route	Home route	Home route - favorites filter on

Add to favorites selection action	Search route	Search route - favorites filter on

[nt4f04uNd]

@Abestanis would you like to review this? I understand this is a lot of code changes, but I thought it could be interesting to you.

[Abestanis]

I love this feature! ❤️

I didn't look at the code yet, but just playing around with it in an emulator, and I noticed two things.

• When you favor or un-favor a song, there is a 1 second where an empty dialog with a progress bar is displayed. Would be nice if that could be avoided.
• The heart icon widget needs some keys, if you have a filled heart in the all songs list and you click on a song which isn't a favorite, then it will show a filled heart. This will be fixed by the suggestion I left in lib/widgets/buttons.dart

Also, a suggestion:

• How about moving the heart in the home route next to the search icon. At first, I thought there was a new category, it was surprising to see that it was just a filter.

[Abestanis]

When you favor or un-favor a song, there is a 1 second where an empty dialog with a progress bar is displayed.

Ok, according to the documentation, the system is supposed to show a prompt. But, the prompt is immediately dismissed on API 30, and there is no prompt on API 32 (both are emulators).

Since there is no way to get rid of the prompt, I think we should at least add a setting to disable synchronizing favorites with the Android media store.

[Abestanis]

Sorry, it took me a bit to go through everything. It looks really good, apart from one bug, I just left a couple of suggestions.

One final suggestion is that tapping on the hearts in the songs list should remove their favorite state. Either that or it could be smaller, so it doesn't look like a touch target.

[nt4f04uNd]

Thank you for this review!

How about moving the heart in the home route next to the search icon. At first, I thought there was a new category, it was surprising to see that it was just a filter.

I'll look into this (and the other bug you mentioned about synchronisation with the MediaStore) a bit later, I agree that it would be better to move it out from the bottom tab panel.

[nt4f04uNd]

Sorry, it took me a bit to go through everything.

You have done very well, by the way! I can imagine how hard it was to review so many unfamiliar code 💀

[nt4f04uNd]

@Abestanis you can check it out now

[Abestanis]

I noticed that there is currently no way to filter for favorites in the playlist and album detail view, nor in the album list of an artist.

Also, have you noticed how awesome the image comparison is on GitHub?

[nt4f04uNd]

I noticed that there is currently no way to filter for favorites in the playlist and album detail view, nor in the album list of an artist.

If we make it comply with the global favorite filter button, it will be confusing, because you can open an album that you makred as favorite, and then it will not have any songs marked as favorite, so it will appear entirely empty and you will need to constantly switch it on and off.

So the way I could see this work is it would be a per-page favorites filter, so when you close album/playlist/artist route, it would be reset. But it seems like it would be a little bit confusing as to how the button would act if you press it. I think I'm going to keep it as-is right now.

[nt4f04uNd]

@Abestanis thank you for the detailed review again!

I think this is good to merge now, the only thing here left I would want to is to make sure the translation is OK, could you check this comment? :-) #34 (comment)

[Abestanis]

I think this is good to merge now

Me too, I have a couple of additional thoughts but if we want to act on them we can do it in a second iteration and another pull request.
For some reason I can't make any of the conversations as resolved, but (once you change the German translation) they are resolved from my end.

If we make it comply with the global favorite filter button, it will be confusing [...]

Yeah, I see what you mean. Ultimately, I feel like this would be better as a per page filter and not a global filter. Maybe synchronize the favorite filter state in the main tabs, but keep it separate for any nested pages. But it's good as it is right now, this could be an enhancement in the future.

It would be nice to hear what other people think. I must admit, I'm not an UI designer and I'm just going with my gut feeling. 😅

[nt4f04uNd]

The movement is definitely gone, but it still looks a bit weird that the select all button is animating to the left

When I designed the actions, I had played with a few variations of how the actions should animate, and ultimately decided that it will always be just from the left. If you have a suggestion how it would look better, definitely open a new issue.

I'ts definitely better, but it looks like there is still more spacing in the back than in the front because of the padding here:

Thanks, removed 8 pixel padding.

I also noticed that there is now more empty space to the right, even if the favorite indicator is not shown: (focus on the second and fourth list entry.)

Nope, that's a side effect of a refactor that I didn't notice, sharp eyes! Fixed by converting ListTile to custom code (as with other tiles)