Typography.js plugin overrideStyles not called

[LeonardoGentile]

It seems that Typography.js overrideStyles function defined inside the themes is not called when calling toTheme.

Example

import { toTheme } from '@theme-ui/typography'
import wp2016 from 'typography-theme-wordpress-2016'

const theme = toTheme(wp2016)

wp2016 defines an overrideStyles function that will modify part of the theme when instantiated with plain typography package.

For example, for the wp2016 theme :

overrideStyles: ({ adjustFontSizeTo, scale, rhythm }, options) => ({
    h1: {
      fontFamily: ["Montserrat", "sans-serif"].join(","),
    },
   // ...

but this function is never called by toTheme and so h1 will result to have the default

headerFontFamily: ["Merriweather", "Georgia", "serif"],

not the Montserrat family as defined by overrideStyles.

This means that Typography.js themes will be slightly different when used in theme-ui, isn't it? Or am I missing somethings?

[hasparus]

Hey @LeonardoGentile :wave: Thanks for the issue!

Yeah, that makes sense. @theme-ui/typography doesn't get as much attention as other packages. I just went through the Git history from the point the file was created and we never did call overrideStyles :(

If don't have bandwidth to work on this one, but if somebody wants to pick this up:

I made a small branch to test what we'd get from calling overrideStyles, and the result looks quite useful.

  // typography.js types are outdated
  const overrideStyles = options?.overrideStyles as any
  if (overrideStyles) {
    const overrides = overrideStyles(
      {
        adjustFontSizeTo: rhythm.adjustFontSizeTo,
        rhythm: rhythm.rhythm,
        scale: getScale(opts),
      },
      options
    )

    console.log({ overrides })
  }

see output from overrideStyles

``` console.log { overrides: { blockquote: { color: 'hsla(0,0%,0%,0.59)', fontStyle: 'italic', paddingLeft: '26px', marginLeft: '-2px', borderLeft: '6px solid hsla(0,0%,0%,0.64)' }, 'blockquote > :last-child': { marginBottom: 0 }, 'blockquote cite': { fontSize: '20px', lineHeight: '32px', color: 'hsla(0,0%,0%,0.9)', fontWeight: 200 }, 'blockquote cite:before': { content: '"— "' }, ul: { listStyle: 'disc' }, 'ul,ol': { marginBottom: '32px' }, '@media only screen and (max-width:480px)': { 'ul,ol': [Object], blockquote: [Object] }, h1: { marginTop: '64px' }, 'h2,h3,h4,h5,h6': { marginTop: '32px', marginBottom: '32px', fontFamily: 'Merriweather, Georgia, SourceHanSerifCN-Regular, san-serif' }, h4: { letterSpacing: 0, textTransform: 'uppercase' }, a: { boxShadow: 'none', color: '#007acc', textDecoration: 'none' }, 'a:hover,a:active': { boxShadow: 'none' }, 'mark,ins': { background: '#007acc', color: 'white', padding: '2px 4px', textDecoration: 'none' } } } at toTheme (packages/typography/src/to-theme.ts:243:13) ```

We could

• use it to return better theme.styles
• (a simpler way) just stick it in theme.styles.root and let it cascade
• or maybe even return it for the user to do whatever they like

@lachlanjc What do you think?

[lachlanjc]

@LeonardoGentile Curious—what do you use Typography.js for in concert with Theme UI? I’ve never seen the appeal or used it so never contributed to it personally.

[LeonardoGentile]

@lachlanjc simply put I wanted to use a typography.js theme. Not being a designer I wanted the various spaces and font-sizes to be already determined for me for a defined set of fonts that work/look well together.

Given that on theme-ui front page it's claimed that it will work out of the box with typography.js I wanted to try. But as I've understood not many people uses the typography.js package so I was wondering if it's a good idea to use it at all.

[hasparus]

@LeonardoGentile, well... if you're courageous, we'll gladly welcome another contributor :)

The claim it'll work out of the box is probably as old as the integration, and frankly speaking, this might be a first issue with a bug about it.

If you're not building an app where Typography.js is selected by user, copying the Typography theme you like straight to your Theme UI theme might suffice.