-
Notifications
You must be signed in to change notification settings - Fork 124
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
base: redesign
Are you sure you want to change the base?
Conversation
- also removed old list tile code
- hopefully this doesn't have a huge performance impact...
I played with this and noticed a couple issues:
|
Hey, thanks for giving it a try!
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 feel like it would be unnecessary or a bad idea, please let me know! |
- also gets rid of duplicate gesture detectors and fixes incorrect menu themes
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. Font size if update and icon contrast is also fixed. So the only major issue left should be the fast scroller. |
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 |
There was a problem hiding this 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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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...
As for the card design, I think it's a bit busy, but I can't really think of an alternative idea right now |
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. |
@Komodo5197 sorry for not getting back earlier :)
Thanks for your thoughts about the provider stuff, I'll see how best to integrate it then. |
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 🙈 |
@UnnKrigul1 thanks for the input! I'll make some changes and upload some new screenshots soon :) |
- also wait with applying the theme until accent color is available
I've made a few changes now and updated the screenshots in the description. Let me know what you think! |
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
old screenshots