[material-next] Update Material You typescale tokens #39514
Conversation
Netlify deploy previewhttps://deploy-preview-39514--material-ui.netlify.app/ @mui/material-next: parsed: +0.10% , gzip: +0.06% Bundle size reportDetails of bundle changes (Toolpad) |
| }, | ||
| medium: { | ||
| family: 'Roboto', | ||
| weight: '500', | ||
| lineHeight: 16, | ||
| // TODO: label.medium has an alternative font weight https://m3.material.io/styles/typography/type-scale-tokens |
There was a problem hiding this comment.
Not sure if it's better to have a weightProminent property or add a label.mediumProminent variant but we can worry about it later, referencing material-web.dev this is very rarely used: https://github.com/search?q=repo%3Amaterial-components%2Fmaterial-web%20prominent&type=code
There was a problem hiding this comment.
Not sure if it's better to have a weightProminent property or add a label.mediumProminent
I think adding weightProminent is more in line with how material names their tokens.
we can worry about it later
I agree
| size: 12, | ||
| tracking: 0.5, | ||
| lineHeight: 16, |
There was a problem hiding this comment.
It can be misleading for people using the theme to divide this value with the size to get the specific CSS line height value. What I mean is that this will be used in most cases like:
line-height: calc(theme.sys.typescale.body.small.lineHeight / theme.sys.typescale.body.small.size)
Should we specify in the name that it is a px value? If it was up to me I would define two values lineHeightPx that would be 16 in this case and lineHeight (the actual value that can be used in the lineHeight CSS prop - note this needs to be calculated based on the theme values :)).
There was a problem hiding this comment.
I will let @DiegoAndai decide. It may be useful to have a design perspective, cc @danilo-leal
There was a problem hiding this comment.
If it was up to me I would define two values
lineHeightPxthat would be 16 in this case andlineHeight
This makes perfect sense to me. I think it will avoid confusion as users would expect the lineHeight to be the unit-less CSS value, as that's the most common.
Also, adding the lineHeight = lineHeightPx / size in the theme will save us from doing line-height: calc(theme.sys.typescale.body.small.lineHeight / theme.sys.typescale.body.small.size) on each component, which we're doing right now 😅.
There was a problem hiding this comment.
@mnajdova @DiegoAndai Good idea ~ I've added a task in #38804 (let's make this change separately)
There was a problem hiding this comment.
@mnajdova @DiegoAndai Good idea ~ I've added a task in #38804 (let's make this change separately)
Sounds good too. My point was, if we plan to rename it, why do a breaking change for just one release, but it's experimental package so it should be fine, it shouldn't affect lots of people.
This PR adds some missing tokens from the material-next typescale.
While working on TextField (#39503, #39483) I noticed we haven't included the latest tokens yet