Skip to content
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

[List] Doesn't meet guidelines #6098

Closed
kybarg opened this issue Feb 9, 2017 · 8 comments
Closed

[List] Doesn't meet guidelines #6098

kybarg opened this issue Feb 9, 2017 · 8 comments
Assignees
Labels
component: list This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process priority: important This change can make a difference
Milestone

Comments

@kybarg
Copy link
Contributor

kybarg commented Feb 9, 2017

Problem description

According to Material guidelines ALL(with controls also) single line list items must be 48px height and 40px when in dense mode. Only exception is when avatar element is present it can be 56px height.
https://material.io/guidelines/components/lists.html#lists-specs

Link to minimally-working code that reproduces the issue

Versions

  • Material-UI: next
  • React: 15.4.0
  • Browser: Chrome 56
@kybarg kybarg changed the title [list] List doesn't meet guidelines [List] List doesn't meet guidelines Feb 9, 2017
@oliviertassinari oliviertassinari added component: list This is the name of the generic UI component, not the React module! next labels Jun 5, 2017
@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Jun 24, 2017
@oliviertassinari oliviertassinari added this to the v1.0.0-prerelease milestone Jul 4, 2017
@oliviertassinari oliviertassinari added design: material This is about Material Design, please involve a visual or UX designer in the process and removed bug 🐛 Something doesn't work labels Jul 8, 2017
@oliviertassinari oliviertassinari removed this from the v1.0.0-prerelease milestone Jul 8, 2017
@ArcanisCz
Copy link
Contributor

Only case i am seeing is when there are some custom components in list-item, such as buttons (https://material-ui-1dab0.firebaseapp.com/component-demos/lists#checkbox).

Should ListItem force its size even though children are taller than 48/40 px?

@skirunman
Copy link
Contributor

skirunman commented Sep 20, 2017

Also, list is missing secondary text line for three-line lists, which is supported in v0.x?
#9205

@zdrummond
Copy link

This problem is hitting me now. I am trying to use a ListItem with a Checkbox, and the list must be dense for this UI. It works fine, until I put the Checkbox in, as noted above.

I am new to material-ui and I having trouble figure out how to force the checkbox to be smaller. Any guidance would be appreciated.

@oliviertassinari oliviertassinari changed the title [List] List doesn't meet guidelines [List] Doesn't meet guidelines Feb 11, 2018
@oliviertassinari oliviertassinari added this to the post v1 milestone Feb 11, 2018
@rvolgers
Copy link

rvolgers commented Apr 11, 2018

I just ran into this. While I'm really excited about v1.0, it's kind of a shame this isn't scheduled to be fixed for the big release!

I'm currently using the following wrapper component around Checkbox to resolve this:

// Workaround for https://github.com/mui-org/material-ui/issues/6098
const DenserPrimaryAction = withStyles(
    theme => ({
        root: {margin: '-12px 4px -12px -12px'},
    }),
)(
    props => (
        <div className={props.classes.root}>
            {props.children}
        </div>
    ),
);

I'm only using it inside a non-dense ListItem around a Checkbox currently, and it's pixel perfect for that usecase. Ymmv for other uses but I think it should work for any 48x48 component that has a centered 24x24 thing inside it surrounded by empty space.

Using the negative margins is a bit of a hack but it seemed like the least invasive change I could make.

@Zuurkern
Copy link

Zuurkern commented Apr 12, 2018

I am working on a solution, also adding support for three-line items (#9205). I'll open a PR when I feel it is robust and elegant enough.

@oliviertassinari oliviertassinari modified the milestones: post v1.0.0, v2 May 20, 2018
@oliviertassinari oliviertassinari removed this from the v2 milestone Jul 28, 2018
@oliviertassinari oliviertassinari added this to the v4 milestone Mar 9, 2019
@oliviertassinari oliviertassinari added the priority: important This change can make a difference label Mar 13, 2019
@oliviertassinari oliviertassinari self-assigned this Mar 30, 2019
@jedwards1211
Copy link
Contributor

I'm confused, do the specs say anything about a dense mode? Now at least, there's nothing in the OP link about dense mode and no image in the specs where the item is 40px tall.

@mbrookes
Copy link
Member

@jedwards1211 You're commenting on a closed issue from 2017.

@jedwards1211
Copy link
Contributor

I guess I should have tagged the OP in the new issue that resulted from this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: list This is the name of the generic UI component, not the React module! design: material This is about Material Design, please involve a visual or UX designer in the process priority: important This change can make a difference
Projects
None yet
Development

No branches or pull requests

9 participants