jpmorganchase / salt-ds

React UI components built with a focus on accessibility, customization and ease-of-use
https://www.saltdesignsystem.com
Apache License 2.0
112 stars 85 forks source link

What's the right way to use next/font with Salt #2162

Open origami-z opened 11 months ago

origami-z commented 11 months ago

To use Salt, we need load font. In theory, we can use next/font's variable option with --salt-typography-fontFamily pointing to next's font.

import { Open_Sans } from 'next/font/google';

const openSans = Open_Sans({
  subsets: ['latin'],
  variable: '--salt-typography-fontFamily',
  display: 'swap',
});

return (
    <SaltProvider>
      <div className={openSans.variable}> 
          ..........
      </div>
    </SaltProvider>
)
.__variable_19b540 {
    --salt-typography-fontFamily: '__Open_Sans_19b540', '__Open_Sans_Fallback_19b540';
}

However, css variable inheritance made it not working. e.g. when using Text component , font-family resolved to "Open Sans" which is not available if you don't have the font installed locally. The css variable resolution surprised me a bit, with my attempt to express it in a table (same order of Chrome dev tool styles tab) :

DOM / Class Variable
.saltText font-family: var(--saltText-fontFamily, var(--salt-text-fontFamily));
.__variable_19b540 --salt-typography-fontFamily: '__Open_Sans_19b540', '__Open_Sans_Fallback_19b540';
.saltTheme --salt-text-fontFamily: var(--salt-typography-fontFamily);
.saltTheme --salt-typography-fontFamily: "Open Sans";

I thought font-family would resolve to __Open_Sans_19b540 but not. So I need to brush up my CSS variable inheritance knowledge, and also need to find a way to use next/font

Sample StackBlitz

https://stackblitz.com/edit/stackblitz-starters-jbfoge?file=pages%2F_app.js

Reference implmentation

james-nash commented 11 months ago

So, I did a bit of tinkering and reading and I've figured out why it behaves as it does and what you can do to get things working with Next.js...

The behaviour you observed is down to when CSS property values are resolved. In your code, the CSS classes added by <SaltProvider> will include something like:

.salt-theme {

  --salt-typography-fontFamily: "Open Sans";

  font-family: var(--salt-typography-fontFamily);

}

...and as that font-family rule is applied, its computed value resolves to Open Sans.

So, when your nested <div className={openSans.variable}> applies the CSS that Next.js generated along the lines of:

--salt-typography-fontFamily: '__Open_Sans_19b540', '__Open_Sans_Fallback_19b540';

...it doesn't affect the value of font-family, as that's already been computed.

(If some elements nested within were to use font-family: var(--salt-typography-fontFamily), they would pick up the Next.js value ('__Open_Sans_19b540', '__Open_Sans_Fallback_19b540'), but there don't seem to be any. So, text inside just inherits the computed font-family value coming from <SaltProvider>.)

Btw, other --salt-* variables that refer to --salt-typography-fontFamily (e.g. stuff like --salt-text-display1-fontFamily: var(--salt-typography-fontFamily);) also get computed and thus "fixed" at the same time, so changing --salt-typography-fontFamily in nested elements won't affect those either.

FWIW, I threw together a little CodePen to do a simplified reproduction of this behaviour: https://codepen.io/cirrus/pen/MWZYEWV

With the help of looking at how this was solved for the Salt website, here's a modified version of your StackBlitz that works. Here's what I did:

  1. Updated the Next font functions to use different CSS var names for their font family stacks (could be anything, as long as it's not a --salt-* variable name)
  2. Defined a new .next-fonts CSS class, which sets --salt-typography-fontFamily and --salt-typography-fontFamily-code to the CSS custom properties names I defined in 1)
  3. Use the <SaltProvider>'s theme prop to apply my .next-fonts class and also openSans.variable and ptSans.variable. This appends those classnames to the <div> that it renders (aside: makes me think theme could have just been called className)

Therefore, those 3 classes will be applied together with the .salt-theme one. Therefore, when the font-family values are computed, our re-assignment of the salt-typography-fontFamily* custom properties does take effect.

There was one gotcha though, the source order of the CSS output seems to be something like this:

.__variable_03547c {
  --next-open-sans: '__Playfair_Display_03547c', '__Playfair_Display_Fallback_03547c';
}

.__variable_af8203 {
  --next-pt-mono: '__PT_Mono_af8203', '__PT_Mono_Fallback_af8203';
}

.next-fonts {
  --salt-typography-fontFamily: var(--next-open-sans);
  --salt-typography-fontFamily-code: var(--next-pt-mono);
}

.salt-theme {

  --salt-typography-fontFamily: "Open Sans";

  font-family: var(--salt-typography-fontFamily);

}

Since the .next-fonts and .salt-theme selectors have the same specificity, the source order meant that the assignment of --salt-typography-fontFamily in .salt-theme took precedence. To resolve that, I used the following hack in global.css to double the specificity of my .next-fonts class:

.next-fonts.next-fonts {
  /* ... */
}

A bit dirty, but it works :-)

james-nash commented 11 months ago

FYI, we had to change our approach slightly on the Salt site today. See #2283

Also, given that there's another issue about documenting this on the Salt site (#2142), is this issue still needed, or can we close it?

origami-z commented 11 months ago

Keep this open, at least until all Salt dev is aware the problem and solution

DavieReid commented 10 months ago

Nice work @james-nash

ZarkoPernar commented 10 months ago

I think I found a simpler way. This is in nextjs app router but the approach should work in the legacy pages router as well.

import React, { CSSProperties } from 'react';
import { Open_Sans } from 'next/font/google';
import '@salt-ds/theme/index.css';
import { SaltProvider } from '@salt-ds/core';

const openSans = Open_Sans({
  subsets: ['latin'],
  variable: '--next-open-sans',
  display: 'swap'
});

export default function RootLayout({
  children
}: {
  children: React.ReactNode;
}) {
  return (
    <html
      lang="en"
      className="salt-theme salt-density-medium"
      data-mode="light"
      style={
        {
          '--salt-typography-fontFamily': openSans.style.fontFamily
        } as CSSProperties
      }
    >
      <body>
        <SaltProvider>{children}</SaltProvider>
      </body>
    </html>
  );
}
ZarkoPernar commented 10 months ago

@DavieReid you think above would work in mosaic?

origami-z commented 3 days ago

Next font(s) can be applied to theme of SaltProvider from 1.32.0

https://github.com/jpmorganchase/mosaic/pull/631