fix(material/list): list item truncates on narrow screen widths #29033
Conversation
andrewseguin
added
the
dev-app preview
|
Deployed dev-app for b302644 to: https://ng-dev-previews-comp--pr-angular-components-29033-dev-eoyhezyg.web.app Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt. |
src/material/list/list.scss
Outdated
| line-height: normal; | ||
| } | ||
| .mat-mdc-list-item-title.mdc-list-item__primary-text { | ||
| height: 45px; |
There was a problem hiding this comment.
I don't understand why this is added. It's moving the secondary text closer to the primary text, and I don't see how it improves readability
There was a problem hiding this comment.
Hi @andrewseguin! Removed previous attempt to improve readability and revised by increasing the list items with 3 lines from 88px to min-height 90px so that the bottom letters of the 3rd line do not get cut off. See screenshots for reference:
- Before screenshot
- After screenshot
There was a problem hiding this comment.
Hardcoding the heights won't work if the text needs to wrap to several lines. Try adding more secondary text to see that it only shows two lines.
Ideally the text container grows as necessary and doesn't set height at all. This might require making more significant changes to the list item styles
For example, we might want to get rid of height: var(--mdc-list-list-item-three-line-container-height); altogether
There was a problem hiding this comment.
Hi @andrewseguin! Thanks for the rec. I did some further digging and it looks like that variable (value of 88px) is coming from _list.js theming for density (screenshot here) . Would it be better to override the value by adding in a height: auto in the list.scss or remove the specific values from the theming? Honestly I haven't worked directly with affecting overall theming so wanted to check before moving in that direction.
essjay05
force-pushed
the
fix-list-item-truncate-small-screens
branch
4 times, most recently
from
d12043b
to
9e08fcc
Compare
essjay05
force-pushed
the
fix-list-item-truncate-small-screens
branch
2 times, most recently
from
1d9b95a
to
4023349
Compare
essjay05
force-pushed
the
fix-list-item-truncate-small-screens
branch
2 times, most recently
from
dff207f
to
91b2390
Compare
src/material/list/list.scss
Outdated
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-three-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-three-lines { | ||
| height: auto; | ||
| padding-bottom: 12px; |
There was a problem hiding this comment.
Change to 16px to match previous padding
src/material/list/list.scss
Outdated
| // if the list item has acquired three lines. We unset these styles for line elements. | ||
| // https://github.com/material-components/material-components-web/blob/348665978ce73694ad4518626dd70cdf5b984113/packages/mdc-list/_evolution-mixins.scss#L205-L206. | ||
| // TODO: Consider removing once MDC supports the explicit tertiary line list variant. | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-avatar.mdc-list-item--with-two-lines |
There was a problem hiding this comment.
I see this helps center it if the text wrap to two lines, but fails if its more. I'd just take this out
Fixes issue with Angular Components List component where the list item truncates on narrow screen widths (ie. mobile screens). Removes white-space wrap style and adds some height to primary lines for readability. Fixes b/291296866
Updates Angular Components List item component height if the list item has 3 lines. With the previous list item height of 88px the bottom lines letters like 'g' get cut off and less readable. Fixes b/247881646
Fixes Angular Components List item component with and without a leading avatar if it is has multiple lines for the height to auto adjust based on the content. Added padding-bottom for list items with a leading icon/avatar/checkbox to improve readability. Fixes b/291296866
Fixes bottom padding beneath Antular Components List item with leading avatar to provide a similar space as exists above the list item. Fixes b/291296866
essjay05
force-pushed
the
fix-list-item-truncate-small-screens
branch
from
91b2390
to
c1a0a27
Compare
Updates fix for Angular Components List component where the list item truncates on narrow screen widths (ie. mobile screens). Updates the padding to be more cohesive with previous styling. Fixes b/291296866
Updates the padding for the previous fix to Angular Components List component for the list item truncating on narrow screen widths. Fixes b/291296866
src/material/list/list.scss
Outdated
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-three-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-three-lines { | ||
| height: auto; | ||
| padding-bottom: 16px; |
There was a problem hiding this comment.
I think 16px looks good, but 8px will match the previous style and probably produce far less screenshot test failures
Updates the previous Angular Components List component fix to more closely align with previous screenshots and avoid additional screenshot test failures. Fixes b/291296866
| // Increased list item height if it has multiple lines so bottom line doesn't get | ||
| // cut off. Also added padding-bottom so there's space btw the text and the divider. | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-avatar.mdc-list-item--with-two-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-two-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-two-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-avatar.mdc-list-item--with-three-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-three-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-three-lines { | ||
| height: auto; | ||
| padding-bottom: 8px; | ||
| } | ||
|
|
||
| .mat-mdc-list-item.mdc-list-item--with-three-lines, | ||
| .mat-mdc-list-item.mdc-list-item--with-two-lines { | ||
| height: auto; | ||
| padding-bottom: 13px; |
There was a problem hiding this comment.
Just realized we can make this a little cleaner by nesting the special case right under where you set the padding-bottom:
| // Increased list item height if it has multiple lines so bottom line doesn't get | |
| // cut off. Also added padding-bottom so there's space btw the text and the divider. | |
| .mat-mdc-list-item.mdc-list-item--with-leading-avatar.mdc-list-item--with-two-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-two-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-two-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-leading-avatar.mdc-list-item--with-three-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-leading-checkbox.mdc-list-item--with-three-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-leading-icon.mdc-list-item--with-three-lines { | |
| height: auto; | |
| padding-bottom: 8px; | |
| } | |
| .mat-mdc-list-item.mdc-list-item--with-three-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-two-lines { | |
| height: auto; | |
| padding-bottom: 13px; | |
| .mat-mdc-list-item.mdc-list-item--with-three-lines, | |
| .mat-mdc-list-item.mdc-list-item--with-two-lines { | |
| height: auto; | |
| padding-bottom: 13px; | |
| &.mdc-list-item--with-leading-avatar, | |
| &.mdc-list-item--with-leading-checkbox, | |
| &.mdc-list-item--with-leading-icon { | |
| padding-bottom: 8px; | |
| } | |
| ... |
Fixes issue with Angular Components List component where the list item truncates on narrow screen widths (ie. mobile screens). Removes white-space wrap style and adds some height to primary lines for readability.
Before screenshot
After screenshot
Fixes b/291296866