-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Apply an unelevated card style to stats cards #11470
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
Apply an unelevated card style to stats cards #11470
Conversation
|
You can test the changes on this Pull Request by downloading the APK here. |
|
@renanferrari @mattmiklic in dark theme, do you think the cards should be elevated to 1dp, similar to what’s shown in this example on the Material guidelines? Or are we assuming the cards are on surface level (0dp)? |
planarvoid
left a comment
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.
Looks great! Good job 👍 Feel free to merge this PR once you address @iamthomasbishop question
| icon = R.drawable.ic_plus_white_24dp, | ||
| textResource = R.string.stats_management_add_new_stats_card, | ||
| navigationAction = Companion.create(this::onClick), | ||
| showDivider = false, |
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.
good catch 👍
|
@iamthomasbishop @mattmiklic this looks fine to me! But just curious if we go away from cards and elevation would we consider going away from boxes too? If I was designing this page from scratch with no page background colour, I would probably do it with horizontal lines dividing the blocks of content, but not specifically put borders around the boxes, if that makes sense? Its a little bit more work so it could be a later step to be considered. Here's a quick hack |
Hi @iamthomasbishop 👋 Yes, in this case we're assuming the cards are on surface level.
Thank you for your input on this, @osullivanchris! From a development perspective, both options (with and without boxes) are quite simple to achieve, but having the same look on both phones and tablets is even simpler, which is the main reason I leaned towards the option presented here. @mattmiklic and @SylvesterWilmott also considered these options and they may have some thoughts to share about them, from a design perspective. @planarvoid Thanks a lot for the review! 🙇♂️ |
What's seen above is consistent with the Material Design rules for outlined cards. This felt like the most appropriate treatment for this usage, where there are many cards together on the same screen. Outlined cards have 0 elevation. So this is still a standard card treatment, just one that has a little bit more subtle visual style than elevated cards. Re: @osullivanchris's mockup; that works for single-column views and it's consistent with how we use cards in the Reader and the Blog Posts screen. But since Stats includes a two-column view (on tablets, or on a phone when in landscape) this feels like the better approach to me. |
That makes sense, thanks @mattmiklic ! |
|
Yep sounds good to me too! |

Fixes #11468
This PR creates a new unelevated card style and applies it to the stats cards.
To test
Open the stats screen on a tablet and make sure it looks like the provided mockup (in the linked issue). Test on both light and dark modes.
PR submission checklist:
RELEASE-NOTES.txtif necessary.