mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.52k stars 31.89k forks source link

[theming] @font-face at-rule broken in styleOverrides #24966

Open ZLevine opened 3 years ago

ZLevine commented 3 years ago

Current Behavior 😯

When specifying multiple @font-face at-rules via an array in MuiCssBaseline.styleOverrides, a single @font-face block is output rather than multiple @font-face declarations. The final rule thus takes precedence, ignoring all previous rules.

Expected Behavior 🤔

Specifying multiple @font-face at-rules should result in multiple @font-face declarations, 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:

  1. Build any theme using createMuiTheme
  2. Specify @font-face with several values in MuiCssBaseline.styleOverrides as designated below:
const theme = createMuiTheme({
  MuiCssBaseline: {
    styleOverrides: {
      "@font-face": [
        {
          fontFamily: "Inter",
          fontStyle: "normal",
          fontDisplay: "swap",
          fontWeight: 300,
          src:
            "url('/fonts/inter/Inter-Light.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Light.woff?v=3.15') format('woff')"
        },
        {
          fontFamily: "Inter",
          fontStyle: "italic",
          fontDisplay: "swap",
          fontWeight: 300,
          src:
            "url('/fonts/inter/Inter-LightItalic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-LightItalic.woff?v=3.15') format('woff')"
        },
        {
          fontFamily: "Inter",
          fontStyle: "normal",
          fontDisplay: "swap",
          fontWeight: 400,
          src:
            "url('/fonts/inter/Inter-Regular.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Regular.woff?v=3.15') format('woff')"
        },
        {
          fontFamily: "Inter",
          fontStyle: "italic",
          fontDisplay: "swap",
          fontWeight: 400,
          src:
            "url('/fonts/inter/Inter-Italic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Italic.woff?v=3.15') format('woff')"
        },
      ]
    }
  }
});
  1. Use the CssBaseline component inside your ThemeProvider:
<ThemeProvider theme={theme}>
  <CssBaseline />
  {/* eslint-disable-next-line react/jsx-props-no-spreading */}
  <Component {...pageProps} />
</ThemeProvider>
  1. Observe that the corresponding injected <style> tag contains the following:
<style data-emotion="css-global" data-s="">
@font-face{font-family:Inter;font-style:normal;font-display:swap;font-weight:300;src:url('/fonts/inter/Inter-Light.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Light.woff?v=3.15') format('woff');font-family:Inter;font-style:italic;font-display:swap;font-weight:300;src:url('/fonts/inter/Inter-LightItalic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-LightItalic.woff?v=3.15') format('woff');font-family:Inter;font-style:normal;font-display:swap;font-weight:400;src:url('/fonts/inter/Inter-Regular.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Regular.woff?v=3.15') format('woff');font-family:Inter;font-style:italic;font-display:swap;font-weight:400;src:url('/fonts/inter/Inter-Italic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Italic.woff?v=3.15') format('woff');}
</style>

Notice that there's only a single @font-face at-rule containing each rule rather than a @font-face at-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:

<style data-emotion="css-global" data-s="">
@font-face{font-family:Inter;font-style:normal;font-display:swap;font-weight:300;src:url('/fonts/inter/Inter-Light.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Light.woff?v=3.15') format('woff');}
@font-face{font-family:Inter;font-style:italic;font-display:swap;font-weight:300;src:url('/fonts/inter/Inter-LightItalic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-LightItalic.woff?v=3.15') format('woff');}
@font-face{font-family:Inter;font-style:normal;font-display:swap;font-weight:400;src:url('/fonts/inter/Inter-Regular.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Regular.woff?v=3.15') format('woff');}
@font-face{font-family:Inter;font-style:italic;font-display:swap;font-weight:400;src:url('/fonts/inter/Inter-Italic.woff2?v=3.15') format('woff2'),url('/fonts/inter/Inter-Italic.woff?v=3.15') format('woff');}
</style>

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` ``` System: OS: macOS 11.2 Binaries: Node: 14.4.0 - /usr/local/bin/node Yarn: Not Found npm: 6.14.11 - /usr/local/bin/npm Browsers: Chrome: 88.0.4324.150 Edge: Not Found Firefox: 84.0.2 Safari: 14.0.3 npmPackages: @emotion/react: ^11.1.5 => 11.1.5 @emotion/styled: ^11.1.5 => 11.1.5 @material-ui/core: ^5.0.0-alpha.25 => 5.0.0-alpha.25 @material-ui/icons: ^5.0.0-alpha.24 => 5.0.0-alpha.24 @material-ui/styled-engine: 5.0.0-alpha.25 @material-ui/styles: 5.0.0-alpha.25 @material-ui/system: 5.0.0-alpha.25 @material-ui/types: 5.1.7 @material-ui/unstyled: 5.0.0-alpha.25 @material-ui/utils: 5.0.0-alpha.25 @types/react: ^17.0.2 => 17.0.2 react: ^17.0.1 => 17.0.1 react-dom: ^17.0.1 => 17.0.1 typescript: ^4.1.5 => 4.1.5 ```

Using with Next.js though that shouldn't matter. :)

oliviertassinari commented 3 years ago

@ZLevine Thanks for opening the issue. There are multiple aspects to it:

  1. The JavaScript syntax can't work with emotion and styled-components. The same key will override the other.
  2. The documentation needs to be updated to showcase the CSS approach.
  3. One solution is:
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>
  );
}
  1. CssBaseline doesn't allow to mix CSS and JavaScript, it needs to be updated:
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 import { css } from @material-ui/styled-engine;

  1. We need to do something about TypeScript, not sure why:
Capture d’écran 2021-02-16 à 19 49 58

cc @mnajdova

mnajdova commented 3 years ago

@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 MuiCSSBaseline, as currently we expect CSSProperties there. This changes should be enough I think:

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.

ghost commented 2 years ago

I cant get this to work at all. Any ideas?


const RecoletaBold = {
  fontFamily: "Recoleta-Bold",
  src: `url("/src/utils/Fonts/Recoleta-Bold.woff2") format("woff2")`,
};

 components: {
    MuiCssBaseline: {
      styleOverrides: {
        "@font-face": [RecoletaBold],
        html: {
          fontSize: "62.5%",
        },
      },
    },
  },
oliviertassinari commented 2 years ago

@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;
eric-burel commented 2 years ago

It seems that it also affects us even when using adaptV4Theme during migration, it doesn't adapt the @global wrapper. Removing the @global works fine (I am using only one font) even when using adaptV4Theme.

daniel-rabe commented 2 years ago

for multiple font-faces the fallbacks syntax works fine;

MuiCssBaseline: {
  styleOverrides: {
    '@font-face': {
      /* open-sans-300 - latin_cyrillic */
      fontFamily: '"Open Sans"',
      fontDisplay: 'swap',
      fontStyle: 'normal',
      fontWeight: 300,
      src: `local('Open Sans Light'), local('OpenSans-Light'),
            url(${normal300woff2}) format('woff2'),
            url(${normal300woff}) format('woff'),
            url(${normal300ttf}) format('truetype')`
    },
    fallbacks: [
        {
            /* open-sans-300italic - latin_cyrillic */
            '@font-face': {
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'italic',
                fontWeight: 300,
                src: `local('Open Sans Light Italic'), local('OpenSans-LightItalic'),
                    url(${italic300woff2}) format('woff2'),
                    url(${italic300woff}) format('woff'),
                    url(${italic300ttf}) format('truetype')`
            }
        },
        {
            '@font-face': {
                /* open-sans-regular - latin_cyrillic */
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'normal',
                fontWeight: 400,
                src: `local('Open Sans Regular'), local('OpenSans-Regular'),
                    url(${normal400woff2}) format('woff2'),
                    url(${normal400woff}) format('woff'),
                    url(${normal400ttf}) format('truetype')`
            },
        }
    ]
}}
keepeek-rd commented 2 years ago

for multiple font-faces the fallbacks syntax works fine;

MuiCssBaseline: {
  styleOverrides: {
    '@font-face': {
      /* open-sans-300 - latin_cyrillic */
      fontFamily: '"Open Sans"',
      fontDisplay: 'swap',
      fontStyle: 'normal',
      fontWeight: 300,
      src: `local('Open Sans Light'), local('OpenSans-Light'),
            url(${normal300woff2}) format('woff2'),
            url(${normal300woff}) format('woff'),
            url(${normal300ttf}) format('truetype')`
    },
    fallbacks: [
        {
            /* open-sans-300italic - latin_cyrillic */
            '@font-face': {
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'italic',
                fontWeight: 300,
                src: `local('Open Sans Light Italic'), local('OpenSans-LightItalic'),
                    url(${italic300woff2}) format('woff2'),
                    url(${italic300woff}) format('woff'),
                    url(${italic300ttf}) format('truetype')`
            }
        },
        {
            '@font-face': {
                /* open-sans-regular - latin_cyrillic */
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'normal',
                fontWeight: 400,
                src: `local('Open Sans Regular'), local('OpenSans-Regular'),
                    url(${normal400woff2}) format('woff2'),
                    url(${normal400woff}) format('woff'),
                    url(${normal400ttf}) format('truetype')`
            },
        }
    ]
}}

The solution of going through the string template does not suit me because I need to keep CSSINJS. @daniel-rabe So your solution with the fallbacks syntax interests me but can you explain to me where this syntax of fallbacks comes from? Is this something supported by MUI? emotions? Is this solution sustainable over time?

Thank you :)

daniel-rabe commented 2 years ago

@keepeek-rd its in jss: https://cssinjs.org/jss-syntax/?v=v10.9.0 so as long mui uses jss we are fine :)

LazzyLizzard commented 2 years ago

Should I import all fonts before I use them or I can do

src: `url(${FONTS_ASSETS_PATH}${fontName}) format('${format}')` 
ncesar commented 2 years ago

for multiple font-faces the fallbacks syntax works fine;

MuiCssBaseline: {
  styleOverrides: {
    '@font-face': {
      /* open-sans-300 - latin_cyrillic */
      fontFamily: '"Open Sans"',
      fontDisplay: 'swap',
      fontStyle: 'normal',
      fontWeight: 300,
      src: `local('Open Sans Light'), local('OpenSans-Light'),
            url(${normal300woff2}) format('woff2'),
            url(${normal300woff}) format('woff'),
            url(${normal300ttf}) format('truetype')`
    },
    fallbacks: [
        {
            /* open-sans-300italic - latin_cyrillic */
            '@font-face': {
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'italic',
                fontWeight: 300,
                src: `local('Open Sans Light Italic'), local('OpenSans-LightItalic'),
                    url(${italic300woff2}) format('woff2'),
                    url(${italic300woff}) format('woff'),
                    url(${italic300ttf}) format('truetype')`
            }
        },
        {
            '@font-face': {
                /* open-sans-regular - latin_cyrillic */
                fontFamily: '"Open Sans"',
                fontDisplay: 'swap',
                fontStyle: 'normal',
                fontWeight: 400,
                src: `local('Open Sans Regular'), local('OpenSans-Regular'),
                    url(${normal400woff2}) format('woff2'),
                    url(${normal400woff}) format('woff'),
                    url(${normal400ttf}) format('truetype')`
            },
        }
    ]
}}

Nice solution, thank you!