-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
fix(list): ensure multi-line lists expand to fill space #1466
Conversation
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.
LGTM aside from minor comments
@@ -54,6 +54,11 @@ $md-dense-three-line-height: 76px; | |||
height: $three-line-height; | |||
} | |||
|
|||
&.md-multi-line .md-list-item { |
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.
Add comment explaining what the md-multi-line
class means.
@@ -18,6 +18,7 @@ describe('MdList', () => { | |||
ListWithItemWithCssClass, | |||
ListWithDynamicNumberOfLines, | |||
ListWithMultipleItems, | |||
ListWithManyLines |
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.
Trailing comma
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.
ye with the trailing commas. done!
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The Material Design spec only recommends a maximum of three lines in list items, so this is what we formally support. However, if you decide to go off-spec and add more than three lines to a list item, all the lines are currently squeezed into a shorter height (which looks awful).
This PR allows list items with 4+ lines to expand to match the height of the contained text so they will look a bit saner.
r: @jelbourn
Closes #1391