Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Redesign track list tile #829

Open
wants to merge 17 commits into
base: redesign
Choose a base branch
from
Open

Conversation

Chaphasilor
Copy link
Collaborator

@Chaphasilor Chaphasilor commented Jul 31, 2024

Finally got around to redesign the track/song list tile to make it consistent, fit two lines of text and use a track-based accent color.
There's now also a favorite indicator / playlist selector button, just like in the design.

Some features, like the download indicator, are still missing. But those require some other changes and a finalized concept anyway, so I left them out for now.
I also updated the dismiss indicator and added text that describes what happens when dismissed, and allocated a dedicated space for the fast scroller instead of overlaying it

If this redesign doesn't have at least feature parity (all features that the old design had), please let me know!

TODO

  • Fix contrast of heart icon in light mode
  • Update queue list tiles
  • Make theme transition less janky? (help please!)
old screenshots

@Chaphasilor Chaphasilor added design Design changes redesign-beta Issues related to the beta/redsigned version of Finamp labels Jul 31, 2024
@Chaphasilor Chaphasilor requested a review from jmshrv July 31, 2024 14:18
@Komodo5197
Copy link

I played with this and noticed a couple issues:

  • The add to playlist menu is using the currently playing image/theme instead of the clicked song.
  • The fast scroller spacing has issues. Going into split-screen mode or scaling the screen really tall can both make the scroller wide enough to stick out past the fixed space given to it and infringe on the song tiles.
  • I'm not really sure why, but there seems to be performance issues. Scrolling deep into the song list results in extreme lag, which does not occur with the current track list.
  • There's not enough contrast between the song title and the artist subtitle. This makes it harder to scan through titles compared to the old design or the album list screen. This is especially annoying in albums with only one artist, where this info is useless.
  • The currently playing visualizer bars are way less visible now that they use the theme color and are placed on top of and in line with the album images. This is fine for the tracks list, where the the theming makes the currently playing song stand out, but will probably be more of an issue on the album screen, because eventually that screen is supposed to be fully themed.
  • This isn't an issue per-se, but I don't particularly like the aesthetics of having songs in little pills all the time.

@Chaphasilor
Copy link
Collaborator Author

Hey, thanks for giving it a try!

  1. I can't seem to repro that. What are the exact steps? Here's how it works for me:
    https://github.com/user-attachments/assets/176aa662-a995-4d97-9439-f6116b0df31c

  2. I'm honestly not surprised the fast scroller has issues. It also is unusable in landscape mode, but I think that has always been the case. I'll see if I can make it more consistent.

  3. I modified the condition that checks if the rendered track is the currently playing track. Maybe that introduced issues? I also noticed the (regular) menu lags a bit befor opening.

  4. I could decrease the font size for the artist again, but didn't think it was such a big deal. Do you have the setting enabled that only shows artists in the album lists if they're different from the album artist?

  5. I mean the visualizer isn't super important anyway, and I'd much prefer to save some horizontal space. But even in the redesigned album screen, the tracks will be grey except for the currently playing track, so I think it should be obvious enough?

  6. You're not the first to mention that. Is it just for aesthetic reasons or would you also prefer a more "compact" layout?

Also, the original plan was to add a download indicator on each track item. I'm not sure if I'll still do that, but either way it might be comfortable to have a provider for getting and updating the download status of items, including albums, etc. Similar to what you added for the favorite status.
If you have the time and feel like it, I'd be glad if you could make a small PR that adds such a download state provider. Right now this is handled by the download button (which already uses a provider for reading), but there might be cases where that specific download button isn't suitable.

If you feel like it would be unnecessary or a bad idea, please let me know!

@Chaphasilor
Copy link
Collaborator Author

Okay, I think I figured some of the issues out. I didn't notice that I still had two gesture detected that did the same thing, with only one being properly connected to the menu theme calculation. That was probably the reason why you saw the wrong theme being used, a simple race condition. Probably also the reason for the laggy menu opening, and might've otherwise negatively impacted performance.
I also made of the widget stateless, since we didn't need two stateful widgets with different keepalive mixins. From my limited testing it seems like performance is much better now, could you please confirm that?

Font size if update and icon contrast is also fixed.

So the only major issue left should be the fast scroller.

@jmshrv
Copy link
Owner

jmshrv commented Aug 2, 2024

ooo nice :)

only visual suggestion I can think of is that having a like button everywhere seems a bit overkill, but does make sense when looking at an album/maybe searching

Copy link
Owner

@jmshrv jmshrv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good :D

fontSize: 16,
height: 1.0),
overflow: TextOverflow.ellipsis,
maxLines: 2,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would maxLines: 1 maybe look better here? Songs with long titles look a bit "compressed" right now (or maybe the line height is wrong? I prefer to use provided fonts like "headline" instead of defining them myself. I'm pretty sure you can dig around and find what ListTile uses)

Screenshot of Infinity on High, which includes a lot of long titles

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was aiming to fit two rows onto a tile instead of just one. I agree that it looks off. I'll think about it...

@jmshrv
Copy link
Owner

jmshrv commented Aug 2, 2024

As for the card design, I think it's a bit busy, but I can't really think of an alternative idea right now

@Komodo5197
Copy link

  1. I'm talking about pressing the new favorite/add to playlist button on the right of the tracks. The song menu is correctly themed, but pressing that isn't.
  2. The performance does seem to have been fixed.
  3. The smaller artist font is slightly better. I do have the setting to hide matching artists enabled, it seems like it isn't working?
  4. Horizontal space is important. If the tracks are going to stay grey, it's probably fine I guess? If we're relying on theming, I'd like the background color to be a bit less subtle.
  5. The density seems about the same as before, and I think it's fine. The album covers may even be bigger at the same density? This is mostly aesthetics. Honestly, I liked the way the actual item lists looked before well enough, with no separators at all, and my instinct would be to keep it like that, matching the background color so that you only see the card when highlighting or on the currently playing track.

Regarding a downloads provider, I'm not sure there's really anything I can do there. As you noted, reading is already cleanly exposed with the download status provider, so that only leaves updating. The guts of that are already packed into downloadsService.deleteDownload(stub: item) and DownloadDialog.show(context, item, viewId), so the remaining logic is pretty tied into the exact UI. Any changes that match the downloadbutton UI that exactly would probably be better off as more parameters in that class.

Also, I noticed that if one song is playing, and then you play a new one with the different theme, the new song has the old theme for a second during the transition. I think we probably shouldn't animate theme transitions at all for the song tiles, because unlike the player screen components each tile only ever has one theme, it's own.

@Chaphasilor
Copy link
Collaborator Author

@Komodo5197 sorry for not getting back earlier :)

  1. Ah, I see. It's not the theme that's wrong though, it's the actual track. The currently playing track is used instead of the selected track. I'll fix it.
  2. Nice!
  3. You're right, I didn't add that functionality back in yet 😅
  4. I'll see if I can increase the contrast in a nice way, either for the visualizer or the background/accent color. Don't want things to be too saturated though
  5. Let me think of a design that isn't too busy but stays consistent with the queue :)

Thanks for your thoughts about the provider stuff, I'll see how best to integrate it then.
I also noticed the weird transition. My issue was that the accent color is based on the current album cover (of the currently playing track, in full resolution). That's the only cover we calculate the accent color for, and it takes a bit before the cover has loaded. This means the old color would be shown even without the transition. I'll definitely try it out though. But I don't really have an idea how to instantly get the accent color for the currently playing track, at least not using the full-resolution cover (which is important because lower-res covers might end up producing different accent colors). Do you have any ideas?

@Chaphasilor
Copy link
Collaborator Author

Now that I think about it, we should already be pre-caching the full resolution covers of upcoming tracks, so it is possible to get the full-resolution cover right-away 🙈

@Uno-Muno
Copy link

Uno-Muno commented Oct 2, 2024

Gonna chip in some designs.

I like tiles, but after thinking about them for a week I see more and more problems with them. Although they do have some pros too.

  • They take extra space leaving less room for the content,
  • They add more clutter as they draw more lines,
  • They limit scaling vertically (for example when we want to use multiple lines and keep the padding same, then we would have to increase the size of the image as well, but that's not a good option),
  • On the bright side they do make drag-and-drop look and feel really nice.

So I went back to the roots and played around with some no background tiles :)
Group 26
Without background the album/artist/song title can easily be 3 lines or even 4 but that's not very nice (and is far fetched anyways).
The overflow menu can be omitted and replaced by a gesture/long press leaving more room for other stuff, but personally I like visual indication, although the target audience is quite tech savvy and can probably figure it out.
In the song tiles I wasn't too sure about how to wrap multiple elements (the artist and the album name) so I took inspiration from Spotify and added marquee/slider thingy. There are probably better solutions to this.

I didn't pay any attention to the dressing, just the songs, albums and artists.

@Chaphasilor
Copy link
Collaborator Author

@UnnKrigul1 thanks for the input!
I'm willing to let go of tiles for most stuff, it really seem to be too much of a hassle to get right.
We could think about reserving the tiled look only for the currently playing track, like in your non-tiled queue mockup. This way we could have a clear and obvious visual indication, we could have consistency in how the current track is shown (possibly even showing the progress of a track in the track list too), but would make better use of space for all other items.

I'll make some changes and upload some new screenshots soon :)

@Chaphasilor
Copy link
Collaborator Author

I've made a few changes now and updated the screenshots in the description. Let me know what you think!
If you approve, I could move ahead and apply the same treatment to the queue list tiles.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design Design changes redesign-beta Issues related to the beta/redsigned version of Finamp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants