Skip to content

Add FontFace component - #77

Open
stevenbenisek wants to merge 3 commits into
jaredpalmer:masterfrom
stevenbenisek:feat/fontface
Open

Add FontFace component#77
stevenbenisek wants to merge 3 commits into
jaredpalmer:masterfrom
stevenbenisek:feat/fontface

Conversation

@stevenbenisek

@stevenbenisek stevenbenisek commented Mar 17, 2019

Copy link
Copy Markdown

✨ Add FontFace resource
📚 Document <FontFace \> and useFontFace
👉 Add <FontFace /> example

Resolves #76

@stevenbenisek

Copy link
Copy Markdown
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

swyxio commented Mar 18, 2019

Copy link
Copy Markdown

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
Copy Markdown
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

swyxio commented Mar 18, 2019

Copy link
Copy Markdown

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

@stevenbenisek

Copy link
Copy Markdown
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