-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[Lists] Added initial MaterialListItem implementation #1350
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
base: master
Are you sure you want to change the base?
Conversation
MaterialListItem is an adaptive view holder for RecyclerView lists that makes list items easy. After initialization, developers are given a pre-built list item. From here, they can enable any element (visual, text, meta etc.) using setVisibility() and set its contents as usual. Internally, MaterialListItem splits the list item into three separate parts. Each part is implemented as a custom View and handles certain functionality for the list item: * Visual - A dynamically-shaped visual (image) seen at the start of some list items. * TextCollection - A View that controls the overline, primary and secondary texts. * SecondaryContent - Controls the secondary action of the list, either a piece of metadata (text) or a customizable FrameLayout In addition, MaterialListItem automatically changes depending on whether it is a single-line, two line or three line item. Here is the process that takes place to do such: 1. TextCollection dynamically calculates the "total lines" of text it uses (single, two or three line typically). 2. Using a TotalLinesListener, its parent MaterialListItem then observes the total lines and is able to share the total lines with its three child elements. 3. All three views adjust their view properties to match the Material spec for single-line, two and three line items.
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.
Thanks for the PR!
I've left specific some comments but also have some overall thoughts.
My current feelings are:
- This feels a bit too fragmented and that there are too many classes involved. Bear in mind that the engineering team will have to maintain this if it gets approved and this makes it a lot more difficult. I'd prefer to see a similar approach to what some other MDC widgets do: the public class which is used by devs and exposes the API (eg.
MaterialButton
) and a restricted helper class (eg.MaterialButtonHelper
) for the various bits of configuration, updating of layout params, etc. - As mentioned in my one comment, I think this is missing a fair bit in terms of themes/styles. A
<declare-styleable
, a default style (Widget.MaterialComponents.MaterialViewHolder
for example) and a theme attr (materialViewHolderStyle
for example). All other MDC widgets/classes allow you to configure things from an app theme level and this should not be exempt from that. - As you mentioned, class/method javadocs and testing will be needed.
lib/java/com/google/android/material/lists/res/color/mtrl_list_item_color_primary_text.xml
Show resolved
Hide resolved
lib/java/com/google/android/material/lists/res/color/mtrl_list_item_color_secondary_text.xml
Show resolved
Hide resolved
lib/java/com/google/android/material/lists/res/layout/material_list_item_overline_text.xml
Show resolved
Hide resolved
return layoutMargins; | ||
} | ||
|
||
private ShapeAppearanceModel calculateShapeAppearanceModel() { |
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.
This feels a bit too inflexible and will not respond fully to theming. What happens if someone wants slightly rounded corners on a square/rectangle? (eg. The Crane example here). And what about cut corners? It feels like this needs a <declare-styleable>
and a default style and should be using:
shapeAppearance
=shapeAppearanceSmallComponent
shapeAppearanceOverlay
= Custom overlay that sets corner sizes to 0dp- If type == CIRCLE (not the biggest fan of this word as it could technically be a diamond), then use
ShapeAppearanceModel.PILL
to set corners sizes to 50% regardless of size
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 definitely agree that Material Lists should respect styling and theming. Infact, it will help shift view holder customization to XML (where it should be) and be easier for devs to understand. I will include cohesive XML style and Java APIs in the next update 🖇️
@maratkalibek I am hoping that Material Lists will go through the |
Thanks! Waiting for this functionality! |
Based on @dsn5ft's recommendations from #1092, Lists now has a single, adaptive MaterialListItem (yay). I spent a lot of time optimizing the internals and tweaking the list item to fit the Material spec so I hope this approach is best for the component!
Example: https://drive.google.com/file/d/1IXp38AiJsrVEWbKO8ql1tVZj0KlE9O7K/view?usp=sharing
API Usage:
This PR is the start of an amazing journey, and I do have improvements planned for MaterialListItem. However, I am not in sync with your team's roadmap, so feel free to tell me if I should add anything else to this pull request. I intentionally left some parts out and believe they are better reserved for their own PRs:
VISIBLE
if it was displaying text, and automatically beGONE
if it has no text. This would remove all of thesetVisibility()
calls and make adoption a bit easier.If you feel that one of these features should be included in this PR, I am happy to add it in 😉