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

Next.js next/font does not apply to components with presentation role #37080

Closed
2 tasks done
RayHughes opened this issue Apr 28, 2023 · 7 comments
Closed
2 tasks done
Assignees
Labels
duplicate This issue or pull request already exists package: system Specific to @mui/system support: question Community support but can be turned into an improvement

Comments

@RayHughes
Copy link

RayHughes commented Apr 28, 2023

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Steps to reproduce 🕹

Live Version: I was unable to get next/font to work in Codesandbox.

Related: vercel/next.js#45433

Steps:

  1. Configure MUI to use the next/font package (https://nextjs.org/docs/basic-features/font-optimization)
import { Montserrat, Readex_Pro } from 'next/font/google'

const montserrat = Montserrat({ subsets: ['latin'], variable: '--font-montserrat' })
const readexPro = Readex_Pro({ subsets: ['latin'], variable: '--font-readex-pro'})
  1. Add font vars to __app.tsx
    <div className={cx(montserrat.variable, readexPro.variable)}>
      <StyledEngineProvider injectFirst>
        <ThemeProvider theme={myTheme}>
          <CssBaseline />
          <SSRProvider>
              <Component {...pageProps} {...router} />
        </ThemeProvider>
      </StyledEngineProvider>
    </div>
  1. Use font vars on theme.
const baseThemeOptions: ThemeOptions = {
  typography: {
    fontFamily: ['var(--font-montserrat)', 'helvetica neue', 'sans-serif'].join(','),
 }
  1. Add to global.scss
body {
  font-family: var(--font-montserrat), arial, helvetica, sans-serif;
 }
  1. Add component with presentation role
      <Drawer
          anchor="top"
          onClose={() => setIsDrawerOpen(false)}
          open={isDrawerOpen}
      >
        <Typography>Some next</<Typography>
      </Drawer>

Current behavior 😯

The next/font styles are applying correctly to all components that do not have the presentation role. The font is probably added to the css output but the font is not found, causing the browser to use a fallback.

Expected behavior 🤔

next/font declarations are applied to all MUI components

Context 🔦

We are attempting to use next/font to remove the CLS from Google Fonts and optimizer our page load. The fonts are being applied correctly to all elements except those with presentation role. See related issue from NextJS

Your environment 🌎

npx @mui/envinfo
  System:
    OS: macOS 13.2.1
  Binaries:
    Node: 18.6.0 - /opt/homebrew/bin/node
    Yarn: 1.22.19 - /opt/homebrew/bin/yarn
    npm: 8.13.2 - /opt/homebrew/bin/npm
  Browsers:
    Chrome: 112.0.5615.137
    Safari: 16.3
  npmPackages:
    @emotion/react: ^11.10.6 => 11.10.6 
    @emotion/styled: ^11.10.6 => 11.10.6 
    @mui/base:  5.0.0-alpha.126 
    @mui/core-downloads-tracker:  5.12.1 
    @mui/lab: ^5.0.0-alpha.127 => 5.0.0-alpha.127 
    @mui/material: ^5.12.1 => 5.12.1 
    @mui/private-theming:  5.12.0 
    @mui/styled-engine:  5.12.0 
    @mui/system:  5.12.1 
    @mui/types:  7.2.4 
    @mui/utils:  5.12.0 
    @mui/x-data-grid:  5.17.26 
    @mui/x-data-grid-pro: ^5.17.26 => 5.17.26 
    @mui/x-license-pro:  5.17.12 
    @types/react: ^18.0.26 => 18.0.26 
    react: ^18.2.0 => 18.2.0 
    react-dom: ^18.2.0 => 18.2.0 
    typescript: ^4.6.4 => 4.6.4 
@RayHughes RayHughes added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Apr 28, 2023
@RayHughes
Copy link
Author

RayHughes commented Apr 28, 2023

The two workarounds are below. Each method has a drawback.

  1. Add the style to the html element globally in the App component as it does not respect being set in CSS or with SX. This does not solve the case where multiple fonts are needed.
const montserrat = Montserrat({ subsets: ['latin'], variable: '--font-montserrat' })
const readexPro = Readex_Pro({ subsets: ['latin'], variable: '--font-readex-pro' })

const MyApp = ({ Component, pageProps, router }: AppPropsWithLayout) => {

//other bootstrap code


  return (
    <>
      <style global jsx>
        {`
          html {
            font-family: ${montserrat.style.fontFamily};
          }
        `}
      </style>
      <div className={cx(montserrat.variable, readexPro.variable)}>
        <StyledEngineProvider injectFirst>
          <ThemeProvider theme={sharpTheme}>
            <CssBaseline />
            <SSRProvider>
               <Component {...pageProps} {...router} />
            </SSRProvider>
          </ThemeProvider>
        </StyledEngineProvider>
      </div>
    </>
  );
  }
  1. Import the styles directly in your theme file and inject the font family. This method prevents you from having direct CSS variables.
const montserrat = Montserrat({ subsets: ['latin'] })
const readexPro = Readex_Pro({ subsets: ['latin'] })

const baseThemeOptions: ThemeOptions = {
  typography: {
      fontFamily: [montserrat.style.fontFamily, 'helvetica neue', 'sans-serif'].join(','),
      fontWeightBold: 700,
      body1: {
        fontFamily: [readexPro.style.fontFamily, 'arial', 'sans-serif'].join(','),
        fontSize: 13,
        fontWeight: 300,
        textTransform: 'none',
        letterSpacing: 0.65,
      },
   }
 }

@zannager zannager added the package: system Specific to @mui/system label May 1, 2023
@mj12albert
Copy link
Member

Linking this to the Next.js 13 umbrella issue: #34905

@mnajdova mnajdova added bug 🐛 Something doesn't work and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels May 29, 2023
@mj12albert mj12albert assigned mj12albert and unassigned mnajdova Jul 21, 2023
@oliviertassinari oliviertassinari changed the title next/font does not apply to components with presentation role Next.js next/font does not apply to components with presentation role Jul 24, 2023
@oliviertassinari oliviertassinari added support: question Community support but can be turned into an improvement and removed bug 🐛 Something doesn't work labels Jul 24, 2023
@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 24, 2023

It's not a bug, but a question/docs problem. The behavior described looks expected to me (based on how it was configured. It surfaces the need for #38082. A CSS variable is applied to a div, which only impacts its children, when a component that uses a portal is used, it's not under the parent CSS variable, so doesn't know about the font family.

In #38095, I pushed a commit, I think it's the right way to do it, hopefully if developers are using this as a starting point, it will already solve an good chunk of the problem, see below for an even better fix ⬇️

@oliviertassinari
Copy link
Member

Duplicate of #38082

@oliviertassinari oliviertassinari marked this as a duplicate of #38082 Jul 24, 2023
@github-actions github-actions bot added the duplicate This issue or pull request already exists label Jul 24, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 24, 2023
@NiceDay608088
Copy link

NiceDay608088 commented Sep 17, 2023

I have the same issue when using "Syncopate" google font with nextjs 13.4.19.

@JTtime
Copy link

JTtime commented Nov 6, 2023

My issue is exactly same, with TextareaAutoSize component. Its showing text with monospace font under this component, where as we had put Montserrat as global font and this global font is applicable to all components except TextareaAutoSize component in our project

@lmf-git
Copy link

lmf-git commented Feb 20, 2024

This is really lame.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists package: system Specific to @mui/system support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

8 participants