-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[Font API] Use registered fonts for Block Editor iframe. #49646
[Font API] Use registered fonts for Block Editor iframe. #49646
Conversation
de755bd
to
1647b01
Compare
Flaky tests detected in 1647b01. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4632308306
|
This looks good to me 👍 |
@aristath The fix is for 6.2+, as the iframe changes were introduced in 6.2. Currently, the break is in the 6.2 compat file. |
Thank you for the clarification! |
You're welcome. Good question and observation! And thanks for reviewing this PR @aristath |
To capture the site editor identification condition in one place. Why? To improve readability and maintability, especially if the identification logic changes in the future.
Thanks for the PR, @hellofromtonya! 🙌🏻 Test ReportEnvironment
Actual Results
|
@hellofromtonya, does this need to be backported into the next WP minor release? |
@Mamaduka For WP 6.2, no this area of the code is not in the 6.2 release. What about the next Gutenberg release? Let's wait, ie don't ship yet. Why? To avoid breaking sites that are using Google Fonts via Jetpack's package as there's a bug in that package preventing user-selected (global styles) fonts from enqueuing and rendering. I'm thinking its best to ship this PR's fix when the #40363 feature enhancement is also committed into |
I've removed this PR from backporting to WP 6.2 minor. Why? This code is not in 6.2 or WP Core. Why? The Fonts API is still experimental and has not yet been backported to WP Core. I also removed this PR from the next GB release. Why? It needs to be bundled with the #40363 feature enhancement which (when implemented) will automatically enqueue all user selected fonts, selected from the Site Editor's Global Typography UI. Why? One of the most popular Google Fonts implementation has a bug in it where its fonts will not enqueue. The bug was hidden until this PR fixed the Fonts API for the iframe assets. Delaying the shipment of this PR's fix avoids breaking sites. |
@hellofromtonya, thanks for the clarification. I'm double-checking PR marked for the next WP minor release, and PR had the label. As for the Gutenberg release, this was included in the next version's RC, so we'll need to exclude it manually. If the PR is blocked by #40363, then we can create "double-revert" PRs (one to remove from the trunk and one to add back) and use the label "Blocked." Cc-ing @ndiego and @bph as they're coordinating the release. |
)" This reverts commit d39f350.
Thank you @Mamaduka. Yes, I do see it was included in 15.6 RC1. #49863 will revert this PR from Sorry for the confusion and delays (was out sick last week). In hindsight, I should have added noted its dependency in the description and marked the PR as blocked to avoid it being committed. |
Note: The revert has been committed into |
What?
Closes #49645
Fixes which fonts get their
@font-face
styles generated the specific iframe:Why?
Outside of the Site Editor, only the enqueued fonts are to be used. Why? To avoid a potential performance impact when a site has one or more plugins or scripts that are registering fonts with the Fonts API.
In the Site Editor, all registered are to be used. Why? To allow users to preview typography choices before selecting and updating the global styles.
How?
Uses
$pagenow
to identify if the web page is the Site Editor. If yes, then modifies thequeue
to use the registered fonts and restores thequeue
after the@font-face
styles are generated.Testing Instructions
Global_Styles::enqueue_global_styles_fonts()
found in the/vendor/automattic/jetpack-google-fonts-provider/src/introspectors/class-global-styles.php
file. This change fixes a bug (by removing the admin check) for enqueuing the plugin's fonts in the editor's iframe.Inspector
tab.wp-fonts-jetpack-google-fonts
.The expected behavior:
<style id="wp-fonts-jetpack-google-fonts">
should not exist.<style id="wp-fonts-local">
should exist (these are the theme's defined fonts).wp-fonts-jetpack-google-fonts
.The expected behavior:
<style id="wp-fonts-jetpack-google-fonts">
should exist.<style id="wp-fonts-local">
should exist (these are the theme's defined fonts).