[Menu][discussion] Menu height/padding changes in 4.5 should be under a dense flag

[Anwardo]

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.

[oliviertassinari]

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.

[pheeria]

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

• Multiple choice

[pheeria]

@oliviertassinari do you have any insights about my question?

[oliviertassinari]

@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.

[cristianoccazinsp]

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?

[oliviertassinari]

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

[cristianoccazinsp]

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?

[oliviertassinari]

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