[Fonts API] Automatically enqueue user-selected global fonts #50529
Conversation
When invoked as a hooked callback, it receives an empty string. Empty is the same as false.
hellofromtonya
requested review from
oandregal,
aristath and
anton-vlasenko
and removed request for
spacedmonkey
hellofromtonya
added
[Feature] Fonts API
[Type] Enhancement
|
FYI: Tests are timing out in the setup process. This is likely due to GitHub queue delay issues https://www.githubstatus.com/. Once the issues are resolved, the failed CI jobs can be restarted. |
|
Flaky tests detected in fe2bd0d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4939695096
|
|
@oandregal @anton-vlasenko @aristath round 2 for this enhancement. What's different in this PR vs the reverted PR? This PR :
I've tested this PR and it works as expected. Would appreciate a code review of the new bits, a manual test report, plus approval if you dig it. Thanks! |
| */ | ||
| public static function enqueue_user_selected_fonts() { | ||
| $user_selected_fonts = array(); | ||
| $user_global_styles = WP_Theme_JSON_Resolver_Gutenberg::get_user_data()->get_raw_data(); |
There was a problem hiding this comment.
I've learned a bit more how the fonts API works. This is my suggestion for the way forward: #40363 (comment) The first step we should address is #50576 and we can revisit what this PR is trying to achieve after.
There was a problem hiding this comment.
There are too many threads! For clarity and transparency, #50576 shouldn't block this work.
I'd also like to bring attention to this thread #40363 (comment) (perhaps better comment there):
When a plugin registers more fonts, these are available in any typography panel control: both in the "global styles sidebar" AND in the block inspector of any block that supports font family. If core wants to help plugins, it's not enough to enqueue the fonts picked by the user in the "global styles sidebar", core also needs to enqueue the fonts picked by the user for a given block via its block inspector. It sounds like (this PR) addresses the former, but not the latter.
There was a problem hiding this comment.
I have tested this PR and approved it. I did manual testing, and it worked as described.
PHPUnit tests also passed after rebasing the branch to the latest trunk.
Overall, the code looks good to me.
However, since @oandregal has raised a question here, I am not merging the PR at this time to allow for further discussion.
|
Update: @oandregal has removed the block of this PR. Why? The Fonts API and Theme JSON data structures are not the same. Theme JSON's What about the needs of the Fonts Library? What about concerns of a user-selected font (saved in the database) that is no longer available on the site due to the theme or plugin no longer being activated? |
hellofromtonya
deleted the
try/automatic-enqueue-user-selected-global-fonts
branch
Closes #40363
Follow-up to #50297, #50499, and #50512.
What?
Adds a new enhancement to the Fonts API: automatic enqueue all user-selected fonts.
What's different in this PR vs the reverted PR?
PR #50297 introduced this enhancement using
wp_get_global_styles(). But after #50366 modified the style value structure, the tests failed. This revealed a problem.wp_get_global_styles()fetched the combined/merged core, theme, and user data, whereas the global styles this enhancement needs is only the user data. Whoopsie. This PR usesWP_Theme_JSON_Resolver_Gutenberg::get_merged_data()instead ofwp_get_global_styles()and the new style value structure from #50297. Props to @oandregal.Why?
Prior to this PR, the API automatically registers and enqueues all fonts defined in a theme's
theme.jsonfile, but not fonts from plugins. Instead, plugins were required to handle the enqueuing.With this PR, the API now handles enqueuing those fonts that users select in the Site Editor > Styles > Typography.
Why the change?
How?
tl;dr
A Resolver parses the user data global styles to collect the font-families. Then enqueues those found before printing happens.
This PR is a combination of #50297 and #50499. Thus it is co-authored by @oandregal.
Longer explanation:
User selections are made in the Site Editor > Styles > Typography UI. That data is available within the global styles custom post type (CPT), which is fetchable using
WP_Theme_JSON_Resolver_Gutenberg::get_merged_data().If user selected, each of the following "Elements" are in the
wp_get_global_styles()returned array:$global_styles['typography']['fontFamily']$global_styles['elements']['link']['typography']['fontFamily']$global_styles['elements']['heading']['typography']['fontFamily']$global_styles['elements']['caption']['typography']['fontFamily']$global_styles['elements']['button']['typography']['fontFamily']Since the global styles have a specific and known structure, the Resolver knowns how to iterate through the styles to find any that are defined.
This PR adds a resolver called
WP_Fonts_Resolverfor processing different data sources and interactions. Currently, it's processing the user-selected fonts that are available within the global styles. Later, other functionality can be added to it, such as (a) reconciling any missing fonts in the theme.json and registering and (b) enqueuing the theme's theme.json defined fonts.A property (
WP_Fonts_Resolver::$global_styles_font_family_structure) exists to define the nested key structure within the global styles data array. Iterating through that structure,_wp_array_get()searches to find each font-family if it exists.If the parser finds a user-selected font, then the font-family is parsed from the style, which has a format of
var(--wp-preset--font-family--<font-family-handle>).WP_Fonts_Resolver::get_value_from_style()handles this process.Code Design notes:
WP_Fonts_Resolver::$global_styles_font_family_structurea protected static property instead of aconst?A
constis public. Once introduced into Core, it must be maintained for BC. By making it as a property, the BC concern is removed providing more flexibility for future changes.By having it as
$protectedrather than$private, it gives the Gutenberg plugin the means to modify that structure if needed for future changes (i.e. once the API is in Core).WP_Fonts_Resolver::get_value_from_style()generic for any preset type?Currently only the
font-familypreset type is needed. But in the future, other preset types might be needed, such as extracting the variation details, e.g.font-weight,font-style. So I designed this method to be generic for parsing any preset type from the given style.wp_print_fonts()? Why so late?My thinking is:
TODO
Testing Instructions
Automated Tests
Manually Testing
The key to testing is to select and save one or more fonts and then check that only those fonts are in the
<style id="wp-fonts-jetpack-google-fonts">element on the front-end.Set up:
On your test site:
Testing Instructions:
Playfair Displayfont and set Appearance toBold Italic.Poppinsfont and set the Appearance toRegular.wp-fonts-jetpack-google-fonts. Expected: It should exist in the<head>.<style>element and inspect the styles. Expected: It should only contain Poppins and Playfair Display, i.e. no other font-families should be in that element. (Note: there will be a lot of variations for each of these font families.)Tip: Copy the element's styles and then search for
font-family. Check each one.Google Fonts: font-face styles
Screenshots or screencast