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

[material-ui][joy-ui] Generate typography tokens #41703

Merged
merged 8 commits into from Apr 18, 2024

Conversation

siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Mar 29, 2024

Part of #40225

closes #39677

mui-typography-tokens.mp4

Typography is the last piece for generating CSS theme variables. It's almost perfect (the limitation is on the CSS side) with CSS font property which is a shorthand for font-family, font-size, font-style font-weight, line-height (except letter-spacing).

Now typography token can be accessed from theme.vars.typography.*.

How it works

MUI system provides a prepareTypographyTokens to convert typography objects into CSS font values.


@siriwatknp siriwatknp added package: material-ui Specific to @mui/material package: joy-ui Specific to @mui/joy labels Mar 29, 2024
@mui-bot
Copy link

mui-bot commented Mar 29, 2024

Netlify deploy preview

https://deploy-preview-41703--material-ui.netlify.app/

packages/material-ui/material-ui.production.min.js: parsed: +0.08% , gzip: +0.06%
@material-ui/core: parsed: +0.07% , gzip: +0.09%
@mui/joy/Container: parsed: +0.65% , gzip: +0.59%
@mui/joy/Link: parsed: +0.55% , gzip: +0.51%
@mui/joy/ListItemContent: parsed: +0.64% , gzip: +0.58%
@mui/joy/Stack: parsed: +0.64% , gzip: +0.58%
@mui/joy/Tabs: parsed: +0.60% , gzip: +0.53%
@mui/joy/ModalClose: parsed: +0.48% , gzip: +0.43%
@mui/joy/Checkbox: parsed: +0.52% , gzip: +0.45%
@mui/joy/Slider: parsed: +0.48% , gzip: +0.42%
@mui/joy/MenuButton: parsed: +0.47% , gzip: +0.42%
@mui/joy/ChipDelete: parsed: +0.47% , gzip: +0.41%
@mui/joy/Input: parsed: +0.56% , gzip: +0.47%
@mui/joy/Avatar: parsed: +0.59% , gzip: +0.49%
@mui/joy/CardActions: parsed: +0.62% , gzip: +0.51%
@mui/joy/ListItem: parsed: +0.60% , gzip: +0.50%
@mui/joy/Step: parsed: +0.61% , gzip: +0.50%
@mui/joy/Accordion: parsed: +0.58% , gzip: +0.48%
@mui/joy/CircularProgress: parsed: +0.56% , gzip: +0.47%
@mui/joy/List: parsed: +0.59% , gzip: +0.49%
and 64 more changes

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 06741dc

@siriwatknp siriwatknp marked this pull request as ready for review April 8, 2024 04:52
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Looks good! Just have one question:

@@ -675,6 +677,7 @@ export default function extendTheme(themeOptions?: CssVarsThemeOptions): Theme {
return createSpacing(spacing, createUnarySpacing(this));
};
theme.spacing = theme.generateSpacing();
theme.typography = mergedScales.typography as any; // cast to `any` to avoid internal module augmentation in the repo.
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me clarify, it is redefined to preserve the same behavior as the developers passed the typography in.

In line 608, the typography is configured by prepareTypographyVars so that the generated token is in the format of font: ….

prepareTypographyVars({ h1: { fontFamily: , fontSize: , lineHeight: , fontWeight:  }})
// { h1: "…valid CSS `font` value", …other typography }

At the end, before the theme is return, the original typography object is redefined back so that this behavior remains the same:

styled('div')({ theme }) => ({
  …theme.typography.h1,
})

Copy link
Member

Choose a reason for hiding this comment

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

So, line 608 adds the theme.vars.typography... values?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really, line 608 builds up the typography structure so that prepareCssVars at line 667 generates the vars.typography based on that structure.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the explanation!

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 17, 2024
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 17, 2024
Copy link
Member

@DiegoAndai DiegoAndai left a comment

Choose a reason for hiding this comment

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

Nicely done 🚀

@siriwatknp siriwatknp merged commit 5297ea5 into mui:next Apr 18, 2024
19 checks passed
@danilo-leal
Copy link
Contributor

danilo-leal commented Apr 18, 2024

Hey folks — I just realized some typography styles got a bit messed up on the website, particularly the medium and semiBold font weights, which are not working anymore (the dev tools declare font-weight: semiBold as an invalid property). In some other places, not only is the font weight messed up, but the size, too; check the footer out:

Broken Correct
Screenshot 2024-04-18 at 18 12 47 Screenshot 2024-04-18 at 18 15 02

As this hadn't happened before, I noticed this was the most recent PR touching typography-related stuff. I've tried reverting some of the changes introduced here, which fixed the footer up there, at least the font size. But not the font weight.

Any ideas whether the issue is within the changes added here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Typography CSS vars are not generated by the CssVarsProvider
4 participants