Material-ui: [Menu][discussion] Menu height/padding changes in 4.5 should be under a `dense` flag

Created on 4 Oct 2019  路  8Comments  路  Source: mui-org/material-ui

In the 4.5 version of Material-UI the height and padding of menu items changed "to better match Google's products" in #17332. I feel like the change was unnecessary to implement as the default just because that's how Google does it. The Material Design menu documentation specifically mentions both a version with more padding as well as a dense option. The Material Design documentation should take precedence over the look of Google's products (which is pretty inconsistent). I don't think the goal of Material-UI should be to clone the look of Google's products but rather be an implementation of Material Design.

It would be a lot more logical to implement these changes under a dense prop this way it's an opt-in functionality and wont mess up the current styles of peoples menu's. Because in my opinion it's not a change that looks better, especially when using a different font than Roboto.

Menu material design

Most helpful comment

Any "migration" guide for this? This change has screwed up most of my current UI; I now have different sizes everywhere.

@cristianoccazinsp Sure, for people that want to revert the change (to be honest, it's was definitely borderline for a minor), you can apply this patch:

.MuiMenuItem-root {
  padding-top: 4px;
  padding-bottom: 4px;
  min-height: 48;
}

or with the theme:

const theme = createMuiTheme()

theme.overrides = {
  MuiMenuItem: {
    root: {
      paddingTop: 4,
      paddingBottom: 4,
      [theme.breakpoints.up('sm')]: {
        minHeight: 48,
      },
    },
  },
};

Or at least, how can I make and components behave like these menu items? Custom components relying on will now have a smaller size than regular menu/select fields... Very annoying.

You can apply:

.my-class {
  padding-top: 6px;
  padding-bottom: 6px;
  min-height: 48;
}

/* theme.breakpoints.up('sm') */
@media (min-width: 600px) {
  .my-class {
    min-height: auto;
  }
}

Additionally, I also need to know what's the item height going to be (due to some custom component that needs to decide on it's height). Where can get this info now that the height might adjust based on the screen size?

We use this approach in the demos:
https://github.com/mui-org/material-ui/blob/f23c7a9bdcf3fc0b7e0eb351b7540bb3054fbc7c/docs/src/pages/components/autocomplete/Virtualize.tsx#L27-L29

All 8 comments

This is one of the most controversial changes we have introduced since v4.0.0. I wouldn't be surprised if we get similar feedback for this change in the future. The component still has a dense mode. It will reduce the font size and apply the same small height it has on the desktop to the mobile environment. As far as I have explored the specification, the instructions are inconsistent.
I believe that both Google products and material.io try to provide a consistent user experience with normalized patterns, but also have to deal with limited resources and prioritize. I have run a benchmark with many Google products as well as other UI frameworks. I believe it's one of the best tradeoffs possible.

I think that it's significantly better than the height we had on desktop prior to v4.5.0.

So, the new menu items look differently depending on screen size (bigger on smaller screens), as expected/described.
But why selects and checkboxes look differently on the same screen size? Is this done on purpose?

  • Single choice

image

  • Multiple choice

image

@oliviertassinari do you have any insights about my question?

@pheeria I have faced it a few hours ago. I went with small checkboxes. It could make sense to support a size="small" on the checkbox, similar to #14129.

Capture d鈥檈虂cran 2019-10-11 a虁 16 27 33

Any "migration" guide for this? This change has screwed up most of my current UI; I now have different sizes everywhere.

Or at least, how can I make and components behave like these menu items? Custom components relying on will now have a smaller size than regular menu/select fields... Very annoying.

Additionally, I also need to know what's the item height going to be (due to some custom component that needs to decide on it's height). Where can get this info now that the height might adjust based on the screen size?

Any "migration" guide for this? This change has screwed up most of my current UI; I now have different sizes everywhere.

@cristianoccazinsp Sure, for people that want to revert the change (to be honest, it's was definitely borderline for a minor), you can apply this patch:

.MuiMenuItem-root {
  padding-top: 4px;
  padding-bottom: 4px;
  min-height: 48;
}

or with the theme:

const theme = createMuiTheme()

theme.overrides = {
  MuiMenuItem: {
    root: {
      paddingTop: 4,
      paddingBottom: 4,
      [theme.breakpoints.up('sm')]: {
        minHeight: 48,
      },
    },
  },
};

Or at least, how can I make and components behave like these menu items? Custom components relying on will now have a smaller size than regular menu/select fields... Very annoying.

You can apply:

.my-class {
  padding-top: 6px;
  padding-bottom: 6px;
  min-height: 48;
}

/* theme.breakpoints.up('sm') */
@media (min-width: 600px) {
  .my-class {
    min-height: auto;
  }
}

Additionally, I also need to know what's the item height going to be (due to some custom component that needs to decide on it's height). Where can get this info now that the height might adjust based on the screen size?

We use this approach in the demos:
https://github.com/mui-org/material-ui/blob/f23c7a9bdcf3fc0b7e0eb351b7540bb3054fbc7c/docs/src/pages/components/autocomplete/Virtualize.tsx#L27-L29

Thanks @oliviertassinari , that's very useful!

I'm also wondering, when using dense, the element height does not change based on the device size, is this intended? Shouldn't it behave similarly?

@cristianoccazinsp The dense props behavior is intended, it reduces the font size and is dense on mobile too.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kybarg picture kybarg  路  164Comments

gndplayground picture gndplayground  路  54Comments

darkowic picture darkowic  路  62Comments

illogikal picture illogikal  路  75Comments

iceafish picture iceafish  路  62Comments