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

Add FontFace component #77

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

stevenbenisek
Copy link

@stevenbenisek stevenbenisek commented Mar 17, 2019

✨ Add FontFace resource
πŸ“š Document <FontFace \> and useFontFace
πŸ‘‰ Add <FontFace /> example

Resolves #76

@stevenbenisek
Copy link
Author

When reviewing especially keep an eye on:

  • Typescript definitions (I'm by no means an expert, would love to get feedback)
  • Error handling if the browser doesn't support window.FontFace (https://caniuse.com/#feat=font-loading). It throws a TypeError that can be caught by adding an error boundary (not sure if this is the right approach?).

@swyxio
Copy link

swyxio commented Mar 18, 2019

interesting use of suspense. why not offer url and format as props? just a random comment, obviously make the api however you like

@stevenbenisek
Copy link
Author

No, it's absolutely a valid comment. In fact, I thought about url and format as props myself. The reason I didn't do it is because with source you can add a comma separated list of url and format pairs (like the CSS @font-face-rule); e.g.:

<FontFace
  family="Name"
  source="url(path/to/name.woff2) format('woff2'), url(path/to/name.woff) format('woff')" />

This is great to provide fallbacks if the preferred format is not supported or to refer to a local font as the primary option.

An alternative API could be using source as an array:

<FontFace
  family="Name"
  source={[
    {
      url: 'path/to/name.woff2',
      format: 'woff2',
    },
    {
      url: 'path/to/name.woff',
      format: 'woff',
    },
  ]}
/>

@swyxio
Copy link

swyxio commented Mar 18, 2019

hmm, yea i see. idk, you can also leave it as a string and ask for OSS to write validation regex :)

@stevenbenisek
Copy link
Author

You do get limited runtime feedback. If you call window.FontFace with an invalid source it will throw a DOMException. You're not completely left in the blind. Might just stick with a string. The reason I opted for a string in the first place was because it aligns with the spec. It's also the same as the @font-face-rule so it's well known and well documented.

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.

2 participants