Skip to content

[material-next] Update Material You typescale tokens #39514

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Oct 25, 2023
Merged

[material-next] Update Material You typescale tokens #39514

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
232e2d7
Update Material You typescale tokens
mj12albert Oct 19, 2023
72b2de4
Update comment
mj12albert Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 39 additions & 4 deletions packages/mui-material-next/src/styles/typescale.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,84 +3,119 @@ const mdSysTypescale = {
small: {
family: 'Roboto',
weight: '500',
lineHeight: 16,
size: 11,
tracking: 0.5,
lineHeight: 16,
},
medium: {
family: 'Roboto',
weight: '500',
lineHeight: 16,
// TODO: label.medium has an alternative font weight https://m3.material.io/styles/typography/type-scale-tokens
Copy link
Member Author

@mj12albert mj12albert Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

// md.sys.typescale.label-medium.weight.prominent -> md.sys.typescale.label-medium.weight.prominent -> '700'
size: 12,
tracking: 0.5,
lineHeight: 16,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 :)).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will let @DiegoAndai decide. It may be useful to have a design perspective, cc @danilo-leal

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it was up to me I would define two values lineHeightPx that would be 16 in this case and lineHeight

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mnajdova @DiegoAndai Good idea ~ I've added a task in #38804 (let's make this change separately)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
large: {
family: 'Roboto',
weight: '500',
lineHeight: 20,
// TODO: label.large has an alternative font weight https://m3.material.io/styles/typography/type-scale-tokens
// md.sys.typescale.label-large.weight.prominent -> md.sys.typescale.label-large.weight.prominent -> '700'
size: 14,
tracking: 0.1,
lineHeight: 20,
},
},
body: {
small: {
family: 'Roboto',
weight: '400',
size: 12,
tracking: 0.4,
lineHeight: 16,
},
medium: {
family: 'Roboto',
weight: '400',
size: 14,
tracking: 0.25,
lineHeight: 20,
},
large: {
family: 'Roboto',
weight: '400',
tracking: 0.15,
size: 16,
tracking: 0.5,
lineHeight: 24,
},
},
title: {
small: {
family: 'Roboto',
weight: '500',
size: 14,
tracking: 0.1,
lineHeight: 20,
},
medium: {
family: 'Roboto',
weight: '500',
size: 16,
tracking: 0.15,
lineHeight: 24,
},
large: {
family: 'Roboto',
weight: '400',
size: 22,
tracking: 0,
lineHeight: 28,
},
},
headline: {
small: {
family: 'Roboto',
weight: '400',
size: 24,
tracking: 0,
lineHeight: 32,
},
medium: {
family: 'Roboto',
weight: '400',
size: 28,
tracking: 0,
lineHeight: 36,
},
large: {
family: 'Roboto',
weight: '400',
size: 32,
tracking: 0,
lineHeight: 40,
},
},
display: {
small: {
family: 'Roboto',
weight: '400',
size: 36,
tracking: 0,
lineHeight: 44,
},
medium: {
family: 'Roboto',
weight: '400',
size: 45,
tracking: 0,
lineHeight: 52,
},
large: {
family: 'Roboto',
weight: '400',
size: 57,
tracking: -0.25,
lineHeight: 64,
},
},
};
Expand Down