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

fix: make useResolvedFontFamily consistent with documented setup for custom fonts #5756

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevindice
Copy link
Contributor

Summary

This PR fixes an inconsistency between useResolvedFontFamily and the documentation & examples.

To use a custom font, the documentation suggests that a property needs to be added to theme.fontConfig mapping all the font weights and styles to specific font files.

The useResolvedFontFamily implementation, however, on line 23, suggests that a property also needs to be added to theme.fonts which is inconsistent with the documentation.

As documented, if I wanted to add SF Pro Display as a custom font, I would do:

const fonts = {
  heading: 'SF Pro Display',
  body: 'SF Pro Display',
  monospace: undefined,
}

const fontConfig = {
  'SF Pro Display': {
    400: {
      normal: 'SF Pro Display Regular',
      itallc: 'SF Pro Display Regular Italic',
    },
    ....
  }
}

const theme = extendTheme({
  fonts,
  fontConfig,
});

But nowhere does it suggest I need to update fonts, adding a property with SF Pro Display as a key. Unless I were to do that, line 23 of useResolvedFontFamily always evaluates to false and this hook never properly does its job to map fonts to the correct value for Android and iOS.

Changelog

General Fixed - Correct an inconsistency between useResolvedFontFamily and the documentation for using custom fonts

Test Plan

@auto-assign auto-assign bot requested a review from surajahmed June 24, 2023 17:12
@vercel
Copy link

vercel bot commented Jun 24, 2023

@kevindice is attempting to deploy a commit to the Geekyants Team Team on Vercel.

A member of the Team first needs to authorize it.

@kevindice kevindice changed the title fix: make useResolvedFonts consistent with documentation fix: make useResolvedFontFamily consistent with documentation Jun 24, 2023
@kevindice kevindice changed the title fix: make useResolvedFontFamily consistent with documentation fix: make useResolvedFontFamily consistent with documentation & examples Jun 24, 2023
@kevindice kevindice changed the title fix: make useResolvedFontFamily consistent with documentation & examples fix: make useResolvedFontFamily consistent with documented setup for custom fonts Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant