232 show album name in playlists and queue

[abegert]

Addresses #232

I inserted a column for the album name in the playlist view, as was described in the issue.

I'm not entirely happy with how the UI looks with it now. I think it would look best if we showed the album name beneath the artist, but I feel like that would be worse since it wastes so much vertical space.
I'm attaching a screenshot so we can discuss.

Since I am not experienced with both Rust and GTK I'd be happy for any pointers as to what could be improved here, even though this is a small change.

Thanks for the awesome spotify client!

[xou816]

Hi! Thank you for taking a stab at this :)

This is indeed a complicated feature UI wise, as it needs to fit on mobile devices as well :/

I don't have a specific idea of how to tackle this either, but perhaps existing designs could help:

• Lollypop for instance "groups" songs from the same album, which minimizes the added vertical space when lots of song coming from the same album are queued at once (this seems to answer the needs of op in 232 rather well!);
• the official Spotify mobile client does not show the album name, but a cover art thumbnail, which is good enough if all we need is a visual cue that songs belong to the same album or not.

That second option is probably easier to implement -- it would be interesting, however, to be smart about it and not show the cover art if browsing a specific album.

What do you think? :)

[abegert]

Hi!

Thanks for the reminder, I hadn't thought of mobile!

Grouping songs of the same album sounds reasonable, but aesthetically speaking I prefer the cover art.

I took a look at the official Desktop and iOS client again. It shows the cover art in playlists, but not albums (as you suggested). On Desktop, the album is shown as a separate column in playlists, on iOS the album is displayed underneath the tracks name, together with the artist, separated by a large dot. I don't really love that, it seems crowded, especially with artists with long names.

To sum it up, I'm going to try and implement option 2 for now, we'll see how it goes and if we like the result.

Thanks for the input!

[xou816]

Sounds good to me :) if it's easier/cleaner, it's fine to have two different song widgets instead of just one.

I think we'll have to rethink the layout of the one with the cover art as well -- again, horizontal space is precious. Maybe we should overlay the cover art with the selection checkbox/playback indicator and ditch the track number/move it to be part of the track name?

[abegert]

Hey,

Sorry for not updating this for a little while, I'm embarrassed how long it took me to get this far.

Here's a side-by-side comparison of the changes:

Before:

After:

Here's a summary of the UI changes:

• The index number of each song has been moved into the title
• The cover of each song is displayed
• The currently playing song is still marked with a little play icon. The cover is not visible here.

As you can see, the vertical space usage is exactly the same, horizontal space is slightly increased by having the index as part of the title.

In my opinion, it's quite functional this way and looks better than before.
I would still like to implement your suggestion to only show the cover in actual playlists, not in albums. I wanted to show this to you for feedback while I work on that.

I'm excited to hear what you think!

[xou816]

Hey, no worries about that, I can't say I'm being very active on the project either. Thank you so much for the work you've been putting into this!!

This looks really good, I think you're definitely headed in the right direction! If you end up working on having a different look for albums (no cover, only index) then we might even get rid of the index for this kind of view -- it does not make that much sense in a playlist context anyway.

Just tell me how far you're willing to go, if you want to merge as is I'll have a closer look/review, it's already quite good and functional after all!

Thanks a lot!

[abegert]

Hey,

Thanks for the positive feedback!
I just implemented not showing album covers for each song in album views.

Regarding the index in the different views:

I originally got rid of the index in the album view, then I reread your comment and realized that your suggestion was to get rid of the index in the actual playlist views but keeping it for albums, right? I understood the exact opposite when first reading it.
My reasoning was that albums generally don't have as many tracks as playlists, so albums don't really need an index. Playlists might have a lot of songs so it's good to know in which position a song is and how many songs in total the playlist has.
Your reasoning was that the index isn't very meaningful in a playlist, but is meaningful in an album?

I see the arguments in favor of the index for both cases now so I decided to keep it as is for now.

Actual playlists still look the same as in the screenshot above, albums now look like this:

I consider this ready for review now but I am open towards changes on the UI side of things.

If you are also happy with the UI, I will be looking forward to your code review.

Slightly related is issue #208 'Track titles aren't aligned in album view', which can be closed IMHO after merging this.

[xou816]

Hi! Sorry, I could have been clearer indeed! I see your point, but I think I'd prefer having the numbers for album tracks only -- in the context of a playlist, those numbers don't mean much and are subject to change everytime the playlist is updated, although I do understand that it helps to track progress. The other reason I prefer having numbers for album tracks are, well, aesthetic one, so that we don't have those empty spaces :p Again, if you prefer to stop here that's perfectly fine!

[abegert]

Hi,

I think it was me who misread what you wrote, but it doesn't matter!

I agree about the empty spaces, that is a really good reason for keeping them were they originally where.

To recap:

• The album view stays exactly the way it was (so no mini cover images, index numbers stay where they are, just like the before image from above)
• All other playlist views (now playing, actual playlists, etc.) will have miniature album covers and no index at all (like in the screenshot above from the playlist but without the index numbers at the beginning of each title)

Thanks for bearing with me. I appreciate that you are opinionated about UI details, I'd rather do it right than do it fast!

[abegert]

I just made the changes like I wrote above.
Album:

Playlist:

Looking at the screenshots I think it looks better than all the previous iterations.

It's ready for review from my side now, but if you have any suggestion for the UI it probably makes sense to discuss / implement changes before starting code review.

[ondras12345]

I'd prefer having the numbers for album tracks only -- in the context of a playlist, those numbers don't mean much and are subject to change everytime the playlist is updated, although I do understand that it helps to track progress.

It is also helpful to have the numbers when you scroll past a page boundary in a playlist and the next page refuses to load. I can't remember the details, but I have encountered that issue before. If it wasn't for the numbers, it would be much harder to see what happened.

[abegert]

I'd prefer having the numbers for album tracks only -- in the context of a playlist, those numbers don't mean much and are subject to change everytime the playlist is updated, although I do understand that it helps to track progress.

It is also helpful to have the numbers when you scroll past a page boundary in a playlist and the next page refuses to load. I can't remember the details, but I have encountered that issue before. If it wasn't for the numbers, it would be much harder to see what happened.

Ah interesting. That has not happened for me so far. I'm happy with either having the index numbers or keeping them out. What about you, @xou816 ?

[xou816]

Interested in learning more about the issue if anything :p but I wouldn't make a decision based on this alone! if better debug tools are needed that's another issue :)

[xou816]

@jannuary i'd be interested in your opinion on this too!

[xou816]

Actually I completely missed that you said it was ready for review, my bad @abegert ! Just saw the above screenshots, really like it, I'll have a closer look, sorry about that!! Do you mind targeting the development branch if it doesnt affect the PR too much?

[jannuary]

@jannuary i'd be interested in your opinion on this too!

Not a fan, unfortunately - seems quite a bit more crowded and messy than the status quo. What's the problem to be solved with album covers/names in playlists?

[abegert]

@jannuary i'd be interested in your opinion on this too!

Not a fan, unfortunately - seems quite a bit more crowded and messy than the status quo. What's the problem to be solved with album covers/names in playlists?

When you are building a playlist with multiple songs from the same artist, it can be useful to see which album each song is from. Aesthetically speaking, I understand your critique but I do like seeing the album covers and find them useful. Do you have a suggestion on how to change it while still keeping the cover images? Or is it not really possible to resolve because you prefer a more minimal aesthetic?

[jannuary]

I understand your critique but I do like seeing the album covers and find them useful. Do you have a suggestion on how to change it while still keeping the cover images? Or is it not really possible to resolve because you prefer a more minimal aesthetic?

Unsure. Maybe spacing the covers a bit more far apart would help?

[abegert]

I understand your critique but I do like seeing the album covers and find them useful. Do you have a suggestion on how to change it while still keeping the cover images? Or is it not really possible to resolve because you prefer a more minimal aesthetic?

Unsure. Maybe spacing the covers a bit more far apart would help?

Do you mean decreasing the size of the covers like this:

[jannuary]

Do you mean decreasing the size of the covers like this:

No, the size's probably fine - you should increase the margins.

[abegert]

Do you mean decreasing the size of the covers like this:

No, the size's probably fine - you should increase the margins.

With slightly increased margins to the right, it looks like this:

An improvement in my opinion.

[jannuary]

Hm, maybe 12px in each direction could work?

[jannuary]

Actually, maybe 6 for top, start and bottom margins, but 12 for column spacing:

[xou816]

There's probably room for small improvements (mostly to improve legibility when using selection mode) but I think it's fine to merge this for now ;) it definitely helps identifying where a track comes from in contexts other than that of an album while keeping the same space usage, and that's the way the official app does things too!
If needed we'll open other issues along the way! But for now I think the improvement is clear over the current situation (fixes a recent issues with track numbers in the artist screen as well)

[xou816]

Looks good to me, merging when I get to a computer :) thank you very much for your work @abegert !
I'm sorry I had someone else chime in last minute, I really value their input as far as design is concerned, but next time I'll have them involved earlier on 😁
Thank you both!

Oh and @jannuary if you need help rebasing after this gets merged feel free to reach out! Hope this won't break things too badly! (I'm sure not)

[abegert]

Looks good to me, merging when I get to a computer :) thank you very much for your work @abegert ! I'm sorry I had someone else chime in last minute, I really value their input as far as design is concerned, but next time I'll have them involved earlier on grin Thank you both!

Oh and @jannuary if you need help rebasing after this gets merged feel free to reach out! Hope this won't break things too badly! (I'm sure not)

Oh I don't mind at all, looking at their design overhaul PR, @jannuary is definitely more design savvy than I am. I was happy to get constructive feedback.
Thanks for merging and all the feedback to both of you!