Move Blockbase Google Fonts to Locally Hosted ( DO NOT MERGE )

[creativecoder]

Changes proposed in this Pull Request:

• Migrate custom font settings to be more compatible with Global Styles (solves Blockbase & children: Google fonts not appearing in Global Styles for some sites wp-calypso#63229)
• Add Google Fonts to theme.json to populate Font family options in the editor
• Include Jetpack package for loading Google fonts from the editor settings, so that Jetpack plugin is not required
• Remove Customizer font settings and add deprecation message that links to Global styles
• Disable Jetpack Google Fonts module, if active, so it doesn't conflict with Blockbase fonts

Note: This migrates the Heading font from the Customizer to the Global Styles settings for the Post Title and Heading blocks. The Heading block typography settings require Gutenberg 13.3, which has not yet been released, so this PR should wait to be merged until then.

Testing Instructions

• Activate a blockbase theme ( Quadrat, Bennett, Blockbase, etc. )
• Before applying this branch, set custom fonts from Customizer > Fonts

• Navigate to Appearance > Editor ( beta )
• Verify that custom google fonts are not selectable in the global styles typography dropdown of the site editor

• Apply this branch and reload a page to migrate the settings
• You should see the body font you selected in Global Styles > Typography

• You should see the heading font you selected in Global Styles > Blocks > Heading > Typography (currently requires Gutenberg trunk) and Global Styles > Blocks > Post Title > Typography

• You should see all the fonts that Blockbase provides in the list of font families provided by Global Styles and Block Settings

• When you navigate back to the customizer, you should see a deprecation notice

Note:

• To test repeatedly, delete the blockbase_legacy_font_settings from wp_options database table to trigger the settings migration (migration only runs when this option is not present).

[MaggieCabrera]

Thanks for this! the team is in a meetup this week, I added this PR to our board so we'll review as soon as possible

[jeyip]

Testing

Requirements

Custom fonts in the site editor

• All the fonts that Blockbase provided in the customizer should now be available in Global Styles and Block Settings
• Font settings that were previously set in the customizer should be applied automatically to global styles and the site editor
• Font settings that were previously set should still be displayed in the frontend
• The blockbase fonts customizer feature should display a deprecation notice once legacy data is migrated

Google Fonts Jetpack module

• If Blockbase or a Blockbase child theme is active, the google fonts jetpack module should not allow toggling. It should immediately be disabled to prevent duplicating fonts in the editor font family settings (blocks and Global Styles)
• If a Blockbase related theme is not active, the google fonts jetpack module should allow toggling.

Browsers

• Edge
• Firefox
• Chrome

[creativecoder]

Noting that the "Use Site Editor" link in the Fonts section of the Customizer is meant to open the Site editor with the Global Styles panel open.

This doesn't currently work with the WP 6.0 release candidate... tracking that in https://core.trac.wordpress.org/ticket/55752.

[jeyip]

🎉 🎉 🎉 Tested for dotcom simple sites. I can confirm that a site with legacy blockbase custom fonts was migrated successfully. One thing to note is that I couldn't confirm the heading font was mapped to global styles heading blocks because typography settings won't be exposed until Gutenberg v13.3. Fonts did map successfully to the global styles post title block though.

To save others the trouble, I navigated to the themes/blockbase directory in my local machine from the terminal. Then I ran this command rsync -azP . sandbox_username@sandbox_address:~/public_html/wp-content/themes/pub/blockbase where sandbox information should be replaced with your respective credentials.

I'll test this for atomic sites tomorrow.

[MaggieCabrera]

I tested this locally, with Gutenberg installed but not Jetpack on a site running Quadrat with custom fonts selected via the customizer. As soon as I checked out this branch, the fonts stop working, they are not being loaded at all. I don't see the font-face rules either for them, but the CSS is requesting the correct font family name.

[jeyip]

I tested this locally, with Gutenberg installed but not Jetpack on a site running Quadrat with custom fonts selected via the customizer. As soon as I checked out this branch, the fonts stop working, they are not being loaded at all.

I have a hunch that this is related to the testing environment and not the code itself -- things had to be run in a specific way when I was testing the PR alongside @creativecoder. Taking a look at your issues today.

[jeyip]

@MaggieCabrera thanks for the patience in testing. It turns out that what you found was, indeed, a bug 😅

I've been mistakenly thinking that the behavior we've been seeing has been a caching issue, because, when we select the font in global styles and refresh the page, the font does load properly.

The issue was that @creativecoder and I didn't realize that font enqueueing for the editor and the front share the same url generation method blockbase_fonts_url. We added font filtering to the function to avoid enqueuing fonts that aren't used in the frontend, and were inadvertently filtering out enqueuing those fonts in the editor as well.

We now enqueue all google fonts options in the editor. The issue should be fixed now 🎉

Note:
Before testing again, you'll want to remove the blockbase_legacy_font_settings from the options table to completely reset the environment

[MaggieCabrera]

@MaggieCabrera thanks for the patience in testing. It turns out that what you found was, indeed, a bug 😅

I've been mistakenly thinking that the behavior we've been seeing has been a caching issue, because, when we select the font in global styles and refresh the page, the font does load properly.

The issue was that @creativecoder and I didn't realize that font enqueueing for the editor and the front share the same url generation method blockbase_fonts_url. We added font filtering to the function to avoid enqueuing fonts that aren't used in the frontend, and were inadvertently filtering out enqueuing those fonts in the editor as well.

We now enqueue all google fonts options in the editor. The issue should be fixed now 🎉

Note: Before testing again, you'll want to remove the blockbase_legacy_font_settings from the options table to completely reset the environment

I'm sorry but something is still broken for me. I tested on a fresh site, my setup is basically a fresh install using Local by Flywheel with a symlink to my themes and plugin folder (using Gutenberg trunk). I'm testing again with Quadrat, first selecting fonts in the customizer while on trunk, then switch to this branch and check the frontend. In this case I see that the CSS variable that should be applied is not defined:

Debugging this a bit I see that when running blockbase_fonts_url() $blockbase_block_font_families is an empty array. $fonts_to_load is showing the fonts I selected in the customizer and $font_families are showing the original font families from the Quadrat theme theme.json file (not the user selected ones). Hope that helps narrow it down? Let me know if I can help with this more.

[jeyip]

I'm sorry but something is still broken for me. I tested on a fresh site, my setup is basically a fresh install using Local by Flywheel with a symlink to my themes and plugin folder (using Gutenberg trunk). I'm testing again with Quadrat, first selecting fonts in the customizer while on trunk, then switch to this branch and check the frontend

Thanks again for the patience @MaggieCabrera! I think we've got the issues sorted this time. I tested our changes for Quadrat in the editor + frontend and things should be working as expected.

We, ultimately, had to do a noticeable amount of refactoring behind the scenes. We're effectively copying the functionality for custom google fonts from the dotcom Jetpack custom fonts implementation. The end result, however, is still the same, and delivers the same functionality that we agreed upon earlier paYE8P-1B3-p2#comment-1526.

• No dependencies on the Jetpack plugin
• The user will update fonts in the site editor
• The Blockbase fonts feature in the customizer will be deprecated
• There will no longer be conflicts when Blockbase is activated on a WordPress installation where the Jetpack custom google fonts feature is enabled

This PR is ready for review again 👍

[pbking]

Considering that there will be themes in the wild that we don't have control of that handle fonts in the same way that Quadrat does I'm extra keen to solve this in a way that doesn't require changes to the child theme. I'm going to tinker with that today and see if I can come up with any suggestions.

The 'heading-font' and 'body-font' is the part I mentioned earlier where some themes were depending on those values in their patterns/settings. It's not guaranteed to be consistent across themes how each of those values are used, only that they had been available to use.

[jeyip]

@pbking

After more consideration, @creativecoder and I took a closer look and realized that we could automatically enqueue default Blockbase theme google fonts. See our latest commit a6bdb08.

It appears as if the bug is fixed, and it seems like the solution satisfies all of the constraints you mentioned earlier:

• Doesn't require changes to child themes
• Leaves the heading-font and body-font values the same

Thoughts?

[pbking]

After more consideration, @creativecoder and I took a closer look and realized that we could automatically enqueue default Blockbase theme google fonts. See our latest commit a6bdb08.

Brilliant! Love it, it's the perfect fix for that situation IMO. Now a child theme doesn't have to be updated to continue to work as expected.

That said a few child themes do have reference to heading-font and body-font outside of the defaults being set in the migration logic. I think the best bet for those (few) cases are to go ahead and commit those changes to configuration.

Meraki (another)
A Blockbase Pattern

...and I think that's it for the Blockbase children. So only Meraki really NEEDS to be updated with this change to maintain design. (Note: I haven't looked for the same in any of the /premium blockbase children).

Just a few more (housekeeping) changes I suggest;

• The color customizer files (aside from the code being added) have no use now and could be removed in this PR. (There's a similar one for colors).
• Can we move the font-related additions from ./functions.php to ./inc/customize-fonts.php? (Renamed to be more appropriate if you feel that's needed since most of it isn't for the Customizer anymore).

[jeyip]

@pbking

Addressed the comments. I removed the unnecessary fonts files and moved fonts logic into theinc directory. I also updated Meraki's configuration. I can make a follow up for changes to the blockbase pattern.

Hopefully everything looks good now. As far as the update to premium themes, let me know how I can coordinate the deploy of this PR with your changes to premium logic.

[pbking]

let me know how I can coordinate the deploy of this PR with your changes to premium logic.

I have no intentions of updating any premium themes, I just wanted to point out that they should. :)

I understand the changes to Blockbase will be pulled into the "premium" version at which point any changes like those for Meraki will need to happen then. I'm not sure who will coordinate that.

As to the rest of the changes I think we're solid! Thanks for working so hard on this! I'mma approve these changes though I'm also going to stick a "do not merge" tag on this change as well. I intend for this change and the color customization change to ship together in a major point release later this week.

[pbking]

A LOT OF WORK HERE!

A good change and improvement to Blockbase.

Approving, requesting that it wait to be merged.

[jeyip]

I have no intentions of updating any premium themes, I just wanted to point out that they should. :)

Gotcha 👍 Thanks for clarifying!

A LOT OF WORK HERE!

A good change and improvement to Blockbase.

Approving, requesting that it wait to be merged.

Thanks for sticking with us through the reviews -- it's been much appreciated 🙏

As to the rest of the changes I think we're solid! Thanks for working so hard on this! I'mma approve these changes though I'm also going to stick a "do not merge" tag on this change as well. I intend for this change and the color customization change to ship together in a major point release later this week.

To make sure we're on the same page, does that mean the themes team will be handling deployment of this PR? To set expectations, I'm planning to head off on a vacation soon. Grant is also on a separate team now, so he'll be prioritizing separate work.

[pbking]

To make sure we're on the same page, does that mean the themes team will be handling deployment of this PR?

Yes, I would like to coordinate the release with a few other things. Since it's a fairly fundamental change to how Blockbase is managed I think it needs a major release. I'll merge the change in the next few days.

[pbking]

I just noticed this, and I'm not sure why, but with this branch applied the site-title doesn't obey the font size values assigned via theme.json Global Styles:

(expected)

Rebasing to trunk didn't help.
The theme.json settings for core/site-title remain unchanged in the branch.
I am quite confused...

--- [UPDATE] ---

So far I've only been able to trace this happening upon the action in the /inc/custom-google-fonts.php:

	add_action( 'init', '\Automattic\Jetpack\Fonts\Introspectors\Global_Styles::enqueue_global_styles_fonts' );
}

With that bit removed the site-title global styles attributes are again applied. I can't find any reason why the above logic should prevent those styles (or any others) from being applied but this is as far as I was able to trace the behavior.

Note that this is effecting child themes as well (as I would have expected):

Found an error to be addressed before this can ship.

[Addison-Stavlo]

I just noticed this, and I'm not sure why, but with this branch applied the site-title doesn't obey the font size values assigned via theme.json Global Styles:

Im trying to repro but am unable. If I have font size set by the block setting, that value is reflected. If i remove that and set a font size in GS -> Blocks -> Site Title -> Typography, that value is reflected.

Do you have more info on how to repro? I tested this by syncing this build to my sandbox and testing on a simple site.

[jeyip]

Im trying to repro but am unable. If I have font size set by the block setting, that value is reflected. If i remove that and set a font size in GS -> Blocks -> Site Title -> Typography, that value is reflected.

Do you have more info on how to repro? I tested this by syncing this build to my sandbox and testing on a simple site.

After further testing, Addie and I were able to reproduce the issue.

One other bug to note is that saving block level global styles changes doesn't work either. The saved values appear to revert to the original body font ( e.g. global styles < blocks < post title < typography < font family modifications don't persist and revert to the body font )

We did some preliminary investigation, and the culprit has something to do with the newest Gutenberg version (v13.5). We tested this PR alongside Gutenberg v13.3 and the site title renders properly in that instance. The saving bug noted above is also fixed.

After running a git bisect on Gutenberg, the problem appears to be this commit from this PR

Found an error to be addressed before this can ship.

@pbking hoping to clarify expectations here. Is the themes team going to handle the error and push a fix? This is looking like a rabbit hole, and Team Cylon has shifted focus to work on the help center.

Happy to discuss this further 👍

cc @Addison-Stavlo @MaggieCabrera

[jeyip]

Putting this PR on hold until we settle on a path forward with GDPR concerns

p1655712801370349-slack-C029FM1EH

[pbking]

Is the themes team going to handle the error and push a fix? This is looking like a rabbit hole, and Team Cylon has shifted focus to work on the help center.

@jeyip yes I'm currently pouring all of my time into the Blockbase font management issue, and while I'm particularly focused on the GDPR concerns mentioned I'm also planning to work through the site-title issue raised. Tracking down the change to that commit looks like it will be particularly helpful so thank you very much!

[jeyip]

Closing this in favor of #6123 for now