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

Use NPM packages to provide fonts instead of Google Fonts #6274

Merged
merged 1 commit into from Mar 16, 2021

Conversation

SISheogorath
Copy link
Contributor

@SISheogorath SISheogorath commented Mar 16, 2021

This patch uses the fontsource libraries instead of using Google Fonts
in order to provide the fonts in the frontend. This way the web frontend
becomes more private, since it no longer shared visitors IPs with Google
and might even loads faster since modern web browsers isolate site
caches from each other and can re-use connections with HTTP/2 instead of
setting up a new connection to Google.[1]

It's basically the next iteration of my previous PR[2], that no longer
relies on a tool to download the fonts separately, but uses a library
instead. It also relates to issues that have been opened before[3] and
should be easily handled and upgraded by the regular NPM package
tooling[4], solving all related problems to that.

1: https://csswizardry.com/2019/05/self-host-your-static-assets/
2: #4864
3: #5883
4: dependencies Related to dependency updates

Note: I'm not sure what to put into the brackets for the PR title, but I hope it's fine this way.

@shields-ci
Copy link

shields-ci commented Mar 16, 2021

Messages
📖 ✨ Thanks for your contribution to Shields, @SISheogorath!

Generated by 🚫 dangerJS against 4aad35b

This patch uses the fontsource libraries instead of using Google Fonts
in order to provide the fonts in the frontend. This way the web frontend
becomes more private, since it no longer shared visitors IPs with Google
and might even loads faster since modern web browsers isolate site
caches from each other and can re-use connections with HTTP/2 instead of
setting up a new connection to Google.[1]

It's basically the next iteration of my previous PR[2], that no longer
relies on a tool to download the fonts separately, but uses a library
instead. It also relates to issues that have been opened before[3] and
should be easily handled and upgraded by the regular NPM package
tooling[4], solving all related problems to that.

[1]: https://csswizardry.com/2019/05/self-host-your-static-assets/
[2]: badges#4864
[3]: badges#5883
[4]: https://github.com/badges/shields/labels/dependencies
@calebcartwright calebcartwright added frontend The React app and the infrastructure that supports it needs-discussion A consensus is needed to move forward labels Mar 16, 2021
@PyvesB PyvesB removed the needs-discussion A consensus is needed to move forward label Mar 16, 2021
Copy link
Member

@PyvesB PyvesB left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @SISheogorath! 👍🏻

@badges/shields-ops briefly discussed this in a meeting, we have no fundamental concerns with the proposed approach.

I've checked things in the Heroku review app, and could not spot any regressions. The fontsource project seems quite well organised and in a good shape. I'm going to go ahead and merge this.

@PyvesB PyvesB merged commit 99fc7c1 into badges:master Mar 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
frontend The React app and the infrastructure that supports it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants