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

Remove webfontloader and simplify web fonts #1815

Merged
merged 1 commit into from
Dec 12, 2024

Conversation

adamzap
Copy link
Member

@adamzap adamzap commented Dec 10, 2024

This patch removes the webfontloader JavaScript library in favor of using link tags to load web font assets. webfontloader had its last release in mid-2017, and we were using it in a very simple way.

This patch also makes the following related changes:

  • Remove all font assets other than woff, which is widely supported: https://caniuse.com/woff
  • Flatten font assets into djangoproject/static/fonts/
  • Remove the FiraMono-Medium font variant assets since this variant was not being used
  • Update outdated font-related references in the styleguide.html template

@bmispelon This is a particularly exciting PR for me because it drops ~7.7 megabytes from the repository. The diff is huge, but the actual changes are small.

If this gets merged, some nice follow-up changes could be:

  1. Update fonts to their latest versions
  2. Use .woff2 font assets instead of .woff (it's safe)
  3. Self-host the Roboto font since Google open-sourced it? Would this be ok license-wise? Remove Google Fonts #1003 is kind of related.

If there's anything I can do to help test this, please let me know. It looks good to me locally.

This patch removes the `webfontloader` JavaScript library in favor of
using `link` tags to load web font assets. `webfontloader` had its last
release in mid-2017, and we were using it in a very simple way.

This patch also makes the following related changes:

- Remove all font assets other than `woff`, which is widely supported:
  https://caniuse.com/woff
- Flatten font assets into `djangoproject/static/fonts/`
- Remove the `FiraMono-Medium` font variant assets since this variant
  was not being used
- Update outdated font-related references in the `styleguide.html`
  template
Copy link
Member

@bmispelon bmispelon left a comment

Choose a reason for hiding this comment

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

What can I say, I wish all PRs were like this 💚

20k lines deleted, clear instructions on how to verify the changes, links to reference documentation: this is a pleasure and a breeze to review.

A+, would merge again 💯

@bmispelon bmispelon merged commit 7d21f20 into django:main Dec 12, 2024
4 checks passed
@bmispelon
Copy link
Member

If this gets merged, some nice follow-up changes could be:

1. Update fonts to their latest versions

2. Use `.woff2` font assets instead of `.woff` ([it's safe](https://caniuse.com/woff2))

3. Self-host the Roboto font since [Google open-sourced it](https://opensource.googleblog.com/2015/05/roboto-googles-signature-font-is-now.html)? Would this be ok license-wise?

Yes, yes, and yes (please) 😁

@adamzap adamzap deleted the remove-webfont-loader branch December 12, 2024 22:02
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