material-ui
material-ui copied to clipboard
[Typography] Apply font properties to typography inherit variant
- [X] I have followed (at least) the PR section of the contributing guide.
Ensure that Typography components actually inherit font properties when inherit variant is used.
Since the Link component is built on top of the Typography component, this also fixes #32942.
const theme = createTheme({
typography: {
fontFamily: "Inconsolata, Arial"
}
});
Before:
<ThemeProvider theme={theme}>
<Typography variant="body1">
<Link>This is good (it's Inconsolata)</Link>
<br />
<Link component="button">This is not good (it's Arial)</Link>
</Typography>
</ThemeProvider>
Now:
<ThemeProvider theme={theme}>
<Typography variant="body1">
<Link>This is good (it's Inconsolata)</Link>
<br />
<Link component="button">This is good too (it's Inconsolata)</Link>
</Typography>
</ThemeProvider>
Fixes #32942
Netlify deploy preview
https://deploy-preview-33621--material-ui.netlify.app/
@material-ui/core: parsed: +0.10% , gzip: +0.09% @material-ui/lab: parsed: +0.19% , gzip: +0.16% @material-ui/system: parsed: +0.54% , gzip: +0.48% @mui/material-next: parsed: +0.37% , gzip: +0.32% @mui/joy: parsed: +0.10% , gzip: +0.11%
Bundle size report
Generated by :no_entry_sign: dangerJS against 8b6f44a66f57efff078df86c5535f9f4ecbc31a6
The visual regression tests detected another issue. The sx prop doesn't override the CSS properties from the variant. Compare https://mui.com/material-ui/react-tree-view/#gmail-clone with https://deploy-preview-33621--material-ui.netlify.app/material-ui/react-tree-view/#gmail-clone.
This has worked fine before, so I wonder if it's related to the latest changes to Emotion support in the System (cc @garronej, @mnajdova).
@michaldudak I don't think the problem is with the order of how things are defined, but how this variant is defined. In the demo that you linked, if you change the fontWeight with font-weight it works as expected, see https://codesandbox.io/s/xenodochial-banach-z7sojt. There is something wrong with how the variant is defined or how the value is fetched (haven't looked into too much details, just wanted to confirm it is not related to the changes done by @garronej as that would have a much bigger impact).
Thanks @mnajdova for figuring this out. I am on vacation so I wasn't able to look at it.
@mnajdova Thanks, I'll take a deeper look at this, then.
@ZeeshanTamboli I changed a bit the fix for the sx prop, by providing the necessary changes to the sx config instead of the styleFunctionSx. The changes are related to the default theme, for e.g. they are not related to Joy UI, so it makes sense for them not to live in the styleFucntionSx intself.
@ZeeshanTamboli I changed a bit the fix for the sx prop, by providing the necessary changes to the sx config instead of the
styleFunctionSx. The changes are related to the default theme, for e.g. they are not related to Joy UI, so it makes sense for them not to live in thestyleFucntionSxintself.
Nice! Makes sense.
It appears that this PR may have broken the sx handling of font props? The documentation says that you can do something like this:
sx={{
fontWeight: 'fontWeightMedium'
}}
But the code in this PR appears to only look for the prop without the fontWeight:
sx={{
fontWeight: 'medium'
}}
Here's a codesandbox that reproduces the issue: https://codesandbox.io/s/vibrant-brook-572rnc?file=/src/App.js
You can verify that this is a regression by changing the version of @mui/material that is pulled in to 5.11.12 or earlier, and notice that using fontWeightMedium properly changes the font weight.
Here's the documentation that refers to being able to use fontWeightLight (as an example):
https://mui.com/system/getting-started/the-sx-prop/#typography
I created an issue for this: #36542