[theming] @font-face at-rule broken in styleOverrides #24966
Labels
bug 🐛
Something doesn't work
docs
Improvements or additions to the documentation
new feature
New feature or request
ready to take
Help wanted. Guidance available. There is a high chance the change will be accepted
v5.x migration
Comments
ZLevine
added
the
status: waiting for maintainer
oliviertassinari
added
docs
oliviertassinari
changed the title
|
@ZLevine Thanks for opening the issue. There are multiple aspects to it:
import * as React from 'react';
import { ThemeProvider, createMuiTheme, CssBaseline, Box } from '@material-ui/core';
const theme = createMuiTheme({
typography: {
fontFamily: 'Raleway, Arial',
},
components: {
MuiCssBaseline: {
styleOverrides: `
@font-face {
font-family: 'DroidSerif';
src: url('https://rawgit.com/google/fonts/master/ufl/ubuntu/Ubuntu-Bold.ttf') format('truetype');
font-weight: normal;
font-style: normal;
}
@font-face {
font-family: 'DroidSerif';
src: url('https://rawgit.com/google/fonts/master/ufl/ubuntumono/UbuntuMono-Italic.ttf') format('truetype');
font-weight: bold;
font-style: normal;
}
`,
},
},
});
export default function App() {
return (
<ThemeProvider theme={theme}>
<CssBaseline />
<Box sx={{
fontFamily: 'DroidSerif',
}}>
Hello
</Box>
</ThemeProvider>
);
}
diff --git a/packages/material-ui/src/CssBaseline/CssBaseline.js b/packages/material-ui/src/CssBaseline/CssBaseline.js
index 4b5c9c8980..23475eb9b8 100644
--- a/packages/material-ui/src/CssBaseline/CssBaseline.js
+++ b/packages/material-ui/src/CssBaseline/CssBaseline.js
@@ -1,6 +1,5 @@
import * as React from 'react';
import PropTypes from 'prop-types';
-import { deepmerge } from '@material-ui/utils';
import useThemeProps from '../styles/useThemeProps';
import GlobalStyles from '../GlobalStyles';
@@ -25,7 +24,7 @@ export const body = (theme) => ({
});
export const styles = (theme) => {
- const defaultStyles = {
+ let defaultStyles = {
html,
'*, *::before, *::after': {
boxSizing: 'inherit',
@@ -46,7 +45,7 @@ export const styles = (theme) => {
const themeOverrides = theme.components?.MuiCssBaseline?.styleOverrides;
if (themeOverrides) {
- return deepmerge(defaultStyles, themeOverrides);
+ defaultStyles = [defaultStyles, themeOverrides];
}
return defaultStyles;This works with emotion and styled-components. Without the change, it only works with emotion once wrapped with
cc @mnajdova |
oliviertassinari
added
bug 🐛
2 tasks
|
@oliviertassinari proposed changes look good to me. If we go with this approach, we will need to update the typings for the styleOverrides for the diff --git a/packages/material-ui/src/styles/overrides.d.ts b/packages/material-ui/src/styles/overrides.d.ts
index 7f3e97c561..e563331032 100644
--- a/packages/material-ui/src/styles/overrides.d.ts
+++ b/packages/material-ui/src/styles/overrides.d.ts
@@ -1,4 +1,4 @@
-import { CSSProperties, StyleRules } from './withStyles';
+import { StyleRules } from './withStyles';
import { AccordionActionsClassKey } from '../AccordionActions';
import { AccordionClassKey } from '../Accordion';
import { AccordionDetailsClassKey } from '../AccordionDetails';
@@ -113,7 +113,7 @@ import { TypographyClassKey } from '../Typography';
export type ComponentsOverrides = {
[Name in keyof ComponentNameToClassKey]?: Partial<StyleRules<ComponentNameToClassKey[Name]>>;
} & {
- MuiCssBaseline?: CSSProperties;
+ MuiCssBaseline?: string;
};
export interface ComponentNameToClassKey {This is why you had the TS error. |
|
I cant get this to work at all. Any ideas? |
|
@chaserda The answer is in this issue. What we might be able to do is: diff --git a/docs/src/pages/guides/migration-v4/migration-v4.md b/docs/src/pages/guides/migration-v4/migration-v4.md
index e955c305aa..0a4f0cd531 100644
--- a/docs/src/pages/guides/migration-v4/migration-v4.md
+++ b/docs/src/pages/guides/migration-v4/migration-v4.md
@@ -1096,6 +1096,32 @@ As the core components use emotion as their style engine, the props used by emot
(Note that this will also affect use of the Typography component with the default `body1` variant).
+- The `'@font-face'` [array API of JSS](https://cssinjs.org/jss-syntax/#font-face) is not supported by emotion nor styled-components.
+ You should use the CSS template string syntax instead.
+
+ ```diff
+ const theme = createTheme({
+ components: {
+ MuiCssBaseline: {
+ - styleOverrides: {
+ - '@global': {
+ - '@font-face': [raleway1, raleway2],
+ - },
+ - },
+ + styleOverrides: `
+ + @font-face {
+ + //
+ + }
+ +
+ + @font-face {
+ + //
+ + }
+ + `
+ },
+ },
+ });
+ ```
+
### Dialog
- The onE\* transition props were removed. Use TransitionProps instead.
diff --git a/packages/material-ui/src/CssBaseline/CssBaseline.js b/packages/material-ui/src/CssBaseline/CssBaseline.js
index 699b4e2058..fa748c4bde 100644
--- a/packages/material-ui/src/CssBaseline/CssBaseline.js
+++ b/packages/material-ui/src/CssBaseline/CssBaseline.js
@@ -46,6 +46,15 @@ export const styles = (theme) => {
const themeOverrides = theme.components?.MuiCssBaseline?.styleOverrides;
if (themeOverrides) {
defaultStyles = [defaultStyles, themeOverrides];
+
+ if (process.env.NODE_ENV !== 'production') {
+ if (Array.isArray(themeOverrides['@font-face'])) {
+ console.warn([
+ 'Material-UI: The @font-face: [font1, font2] API is no longer supported.',
+ 'Follow the upgrade guide on https://material-ui.com/r/migration-v4#cssbaseline.',
+ ].join('\n'));
+ }
+ }
}
return defaultStyles; |
|
It seems that it also affects us even when using |
|
for multiple font-faces the fallbacks syntax works fine; |
The solution of going through the string template does not suit me because I need to keep CSSINJS. Thank you :) |
|
@keepeek-rd |
|
Should I |
Nice solution, thank you! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Current Behavior 😯
When specifying multiple
@font-faceat-rules via an array inMuiCssBaseline.styleOverrides, a single@font-faceblock is output rather than multiple@font-facedeclarations. The final rule thus takes precedence, ignoring all previous rules.Expected Behavior 🤔
Specifying multiple
@font-faceat-rules should result in multiple@font-facedeclarations, allowing us to specify multiple faces for different font variants.Steps to Reproduce 🕹
This is tough to build on CodeSandbox since it involves theming and it's hard to get that set up on there to render correctly. But you should be able to reproduce with the dummy code below in ANY theme in your local environment.
Steps:
createMuiTheme@font-facewith several values inMuiCssBaseline.styleOverridesas designated below:CssBaselinecomponent inside yourThemeProvider:<style>tag contains the following:Notice that there's only a single
@font-faceat-rule containing each rule rather than a@font-faceat-rule for EACH object specified. This results in every font rendering using whatever the last rule is (in this case, italic).What SHOULD happen is this:
Context 🔦
Trying to define multiple @font-face declarations in order to render multiple font variations of a single typeface.
Your Environment 🌎
`npx @material-ui/envinfo`
Using with Next.js though that shouldn't matter. :)
The text was updated successfully, but these errors were encountered: