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

Webfonts: Fix webfont loading in the Site Editor #40262

Merged
merged 2 commits into from
Apr 18, 2022
Merged

Conversation

scruffian
Copy link
Contributor

What?

This ensures that all Webfonts from all variations are loaded in the Site Editor

Why?

We need to load all Webfonts for every variation so that when a user previews the variation they see their fonts.

How?

The merging of Webfonts assumed that they fontFamilies were stored under settings > typography > fontFamilies but they are actually under settings > typography > fontFamilies > theme`.

Testing Instructions

Screenshot 2022-04-12 at 10 24 11

  • Confirm that you see the different font files load in the network tab

Screenshot 2022-04-12 at 10 24 05

Copy link
Contributor

@jffng jffng left a comment

Choose a reason for hiding this comment

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

Tests well for me:

Screen Shot 2022-04-12 at 10 48 24 AM

It looks like the preview tiles of the style variations still do not load the fonts, but that's probably a separate issue.

@kjellr
Copy link
Contributor

kjellr commented Apr 12, 2022

+1, this fixes the issue for me! As noted by @jffng, the previews do not get the correct font though.

@kjellr
Copy link
Contributor

kjellr commented Apr 12, 2022

This may be unrelated, but I'm not seeing the secondary font weights loaded in (on both the front end or the back end). This is not a problem for Inter or Source Serif (since those are variable fonts), but it's an issue for DM Sans:

Screen Shot 2022-04-12 at 11 37 17 AM

^ In this example, the post title should be bold, but that 700 weight is never loaded in.

@jffng
Copy link
Contributor

jffng commented Apr 13, 2022

@gziolo since this fixes a bug with style variations, can this be marked for backporting to Core?

@scruffian
Copy link
Contributor Author

I am AFK the next few days so if someone wants to finish this off please be my guest...

@gziolo
Copy link
Member

gziolo commented Apr 14, 2022

@gziolo since this fixes a bug with style variations, can this be marked for backporting to Core?

The changes are applied to the PHP file which is in the experiemental folder. It doesn’t look like Webfonts API made it to WP 6.0. @priethor and @annezazu, can you confirm that?

@jasmussen
Copy link
Contributor

It would be problematic for TwentyTwentyTwo and style variants without this.

@jasmussen
Copy link
Contributor

@gziolo there's a section on webfonts in the dev notes tracking issue: #39654 and from what I can tell, #37140 should be in 6, right?

@gziolo
Copy link
Member

gziolo commented Apr 14, 2022

@gziolo there's a section on webfonts in the dev notes tracking issue: #39654 and from what I can tell, #37140 should be in 6, right?

See #39889 and the note from @hellofromtonya:

Webfonts are still in development. PR #39559 moves the files into the lib/experimentation folder until the API is ready for WordPress Core commit.

The code is stil in the lib/experimental folder and we didn't get any confirmation that it's ready to backport to WordPress core.

@bph
Copy link
Contributor

bph commented Apr 14, 2022

Sorry @jasmussen the section was added without knowing that WebfontsAPI hasn't merged into the Beta 1.

@jffng
Copy link
Contributor

jffng commented Apr 14, 2022

Thanks everyone for the comments. Since this fixes the issue described and the tests are passing, any objections to merging this?

It sounds like there is still the decision to be made whether Webfonts makes it in to 6.0, which means TT2s style variations would lack custom fonts. We could enqueue fonts for the style variations the old way, but I don't think that's a good idea or precedent to set.

@kjellr
Copy link
Contributor

kjellr commented Apr 14, 2022

It sounds like there is still the decision to be made whether Webfonts makes it in to 6.0, which means TT2s style variations would lack custom fonts. We could enqueue fonts for the style variations the old way, but I don't think that's a good idea or precedent to set.

Yeah, I agree there. Part of the allure of styles is the ability to swap out a variety of styles: fonts, colors, image treatments, settings etc. We can technically build in more font support manually, but it would be an unfortunate way to do things since this is so close otherwise.

The feature is still listed as a must-have for RC: #40139

@jffng jffng merged commit b86f98f into trunk Apr 18, 2022
@jffng jffng deleted the fix/webfonts-in-editor branch April 18, 2022 14:09
@github-actions github-actions bot added this to the Gutenberg 13.1 milestone Apr 18, 2022
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.

7 participants