Skip to content

Add Google font imports (Gatsby) #924

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

Merged
merged 1 commit into from
Oct 15, 2020

Conversation

mikeesto
Copy link
Contributor

@mikeesto mikeesto commented Oct 5, 2020

The live website uses a handful of Google fonts. These are also used in the Gatsby site but they are not currently imported, as seen below:

Group 1 (1)

This PR imports the fonts!

Copy link
Member

@saihaj saihaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey 👋🏼 thank you for bringing this up. I forgot about the fonts. I think instead of importing them in our css files we should prefetch fonts using gatsby-plugin-prefetch-google-fonts

@mikeesto
Copy link
Contributor Author

@saihaj Yes, that's what I originally planned to do but I couldn't work out if it's still being maintained. The one you linked to is archived: https://github.com/escaladesports/legacy-gatsby-plugin-prefetch-google-fonts and the link to its supposed new home gives me a 404.

@saihaj
Copy link
Member

saihaj commented Oct 10, 2020

This looks like a community fork of that package. https://www.npmjs.com/package/@el7cosmos/gatsby-plugin-prefetch-google-fonts
Do you think we should use this instead? Or should we write something ourselves?
cc @IvanGoncharov

@carolstran carolstran added 👾 Gatsby Related to the Gatsby migration 🎨 Design labels Oct 11, 2020
@carolstran
Copy link
Member

Popping in hi 👋🏼 The community fork looks like the option for this case if we want to prefetch the fonts. But what's the performance difference between prefetching via a plugin and importing the fonts? If it's not a big difference, then we could always implement the proposed changes now and open a separate issue to look into the best plugin for prefetching so that it's not a blocker.

@carolstran
Copy link
Member

By the way, thank you so much for picking up some of these Gatsby issues @mikeesto - you seriously rule! 🥳

@mikeesto
Copy link
Contributor Author

Thanks @carolstran 😊 Although I feel like I've barely done anything!

Unfortunately I haven't had much luck with either the fork or the original plugin. Both give this error when trying to load the Rubik font: Error: ENOENT: no such file or directory, stat '.cache/google-fonts//fonts'. I found an open issue about it in the original repo. I wasn't able to find any alternative prefetching plugins. I'm thinking it's perhaps best to go with what we have for now, and look at prefetching options again in a few months time.

@carolstran
Copy link
Member

Agreed! My vote would be to merge this solution and then open an issue about prefetching so that it's documented/not forgotten 😊 (pinging @IvanGoncharov to merge)

@IvanGoncharov IvanGoncharov merged commit fd9c7d6 into graphql:gatsby Oct 15, 2020
@IvanGoncharov
Copy link
Member

IvanGoncharov commented Oct 15, 2020

Agree with @carolstran we can optimize performance in the next PR.

@carolstran
Copy link
Member

Sweet! I created #932 as a future reference.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 Design 👾 Gatsby Related to the Gatsby migration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants