Webfonts API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

zaguiini · 2022-03-09T22:22:59Z

Part of #39332. Read #39332 (comment) to gather more context about the whole picture regarding the implementation.

What?

Currently, the webfonts API automatically enqueues all registered webfonts. In this PR, we separate webfont registration from webfont enqueuing into two different processes.

Why?

By introducing these changes, we more closely match the behavior of wp scripts and styles and accommodate functionality that WordPress developers are already familiar with.
We allow API consumers more flexibility in how and when they enqueue webfonts, rather than forcing them to enqueue all fonts indiscriminately.

How?

Create state to track registered webfonts and enqueued webfonts
Create methods to access and manually enqueue webfonts

We're also adding the possibility to register custom ids now. We're actually enforcing it when registering through wp_register_webfont or wp_enqueue_webfont.

Naming what you're registering and enqueueing makes the whole API more predictable. You know exactly which fonts you are registering and you can be sure that the font you want to enqueue by id is the exact same font you registered. That's the same principle defined in wp_scripts and wp_styles, for example, where you must pass the $handle.

Testing Instructions

Register a webfont via theme.json or programmatically through the wp_register_webfont php function (See below for examples)
Enqueue the webfont by calling wp_enqueue_webfont
Activate the theme in the local WordPress development environment
Verify that the webfont is selectable in block settings for certain blocks (button, site title, site tag, etc.) in the post editor and template editor
Verify that the webfont is available in the global styles menu for the full site editor
Verify that the webfont is displayed in the frontend of the site*

*For now, we're not enqueueing any font if solely picked in the editor without programmatically enqueueing, so no fonts will be loaded in the front-end, even if selected. To test this, you need to programmatically enqueue the font until #39399 gets merged.

For the examples below, the Roboto font can be downloaded from https://fonts.google.com/specimen/Roboto. Unzip the package and place the .ttf file in the bennett/assets/fonts theme directory.

Testing a webfont loaded via `theme.json`:

Using the Bennett theme as an example, we edit its theme.json file and replace fontFamilies with the following:

"fontFamilies": [
    {
        "id": "roboto-regular",
        "fontFamily": "Roboto",
        "fontFace": [
            {
                "fontFamily": "Roboto",
                "fontWeight": "900",
                "fontStyle": "normal",
                "fontStretch": "normal",
                "src": [ "file:./assets/fonts/Roboto-Regular.ttf" ]
            },
        ]
    }
],

Then, we create a functions.php in the bennett theme directory and add the following:

<?php

add_action( 'after_setup_theme', function() {
	wp_enqueue_webfont( 'roboto-regular' );
} );

Testing a webfont programmatically through PHP:

Next, to test the PHP registration using the wp_register_webfont function, in functions.php add the following:

add_action( 'after_setup_theme', function() {
	wp_register_webfont(
		'roboto-regular',
		array(
			'font-family'  => 'Roboto',
			'font-style'   => 'normal',
			'font-stretch' => 'normal',
			"font-weight" => "900",
			'src'          => array( 'file:./assets/fonts/Roboto-Regular.ttf' ),
		)
	);

        wp_enqueue_webfont( 'roboto-regular' );
} );

Like WordPress, we can also directly enqueue a webfont without registering it like so:

add_action( 'after_setup_theme', function() {
	wp_enqueue_webfont(
		'roboto-regular',
		array(
			'font-family'  => 'Roboto',
			'font-style'   => 'normal',
			'font-stretch' => 'normal',
			"font-weight" => "900",
			'src'          => array( 'file:./assets/fonts/Roboto-Regular.ttf' ),
		)
	);
} );

jeyip · 2022-03-10T02:44:19Z

@aristath As Luis and I were refactoring the WP_Webfonts class, we were wondering why it doesn't extend the WP_Dependencies class.

Webfonts resemble scripts and styles, and we can't see why we wouldn't use it. Could you elaborate?

aristath · 2022-03-10T07:16:02Z

@aristath As Luis and I were refactoring the WP_Webfonts class, we were wondering why it doesn't extend the WP_Dependencies class.

The initial implementation of the Webfonts API was actually using a class extending WP_Styles (you can see it in the initial commits in that PR). However, as the development of the feature moved forward and it starting taking shape, extending the WP_Styles class became less and less necessary.
It became clear that we needed to use providers to allow extending the API for 3rd-party providers, code started to be moved around and the structure started to take shape. At some point, we were extending the WP_Styles class without actually using anything from it.
True, webfonts are basically styles. However, they don't work like other WP styles because they don't have a stylesheet but instead use inline styles. Extending WP_Styles just for the sake of making it look like we're using the same API didn't make a lot of sense. Extending WP_Dependencies didn't make a lot of sense either, because well... webfonts don't have dependencies.
Other than a generic conceptual similarity (both WP_Styles and webfonts add styles), there is no common ground between these 2 APIs.

It's exactly the same for global-styles... Conceptually, global-styles should extend WP_Styles or WP_Dependencies. In practice however, doing that would not improve the global-styles API.

Both global-styles and webfonts are standalone APIs that use styles, but don't need to extend some other API.

zaguiini · 2022-03-10T12:33:58Z

Thanks, @aristath, and that makes a lot of sense!

One question, though:

webfonts don't have dependencies.

Do you think it's true? One might want to include font B only if font A has been registered... That might happen. Also, fonts definitely have dependents so I wonder if the _WP_Dependency model works here.

aristath · 2022-03-10T13:11:44Z

One might want to include font B only if font A has been registered... That might happen.

Also, fonts definitely have dependents so I wonder if the _WP_Dependency model works here.

🤔 I can't think of a scenario where we wouldn't want to load font B if font A has not been loaded... Or a scenario where something would depend on whether a webfont has been loaded or not. Can you please elaborate? A practical example might help me understand!

The way I see it, users should be able to get the content and continue being able to use the page as if everything is fine, even if a webfont for some reason doesn't load. Worst-case scenario, they will see the content with a system-font and not the designer's font of choice... But that's not necessarily a bad thing: If a webfont doesn't load it will be because for example they are on an extremely slow connection, or force their browser to ignore all custom font-families and use their own due to a11y.

zaguiini · 2022-03-10T16:11:57Z

I can't think of a scenario where we wouldn't want to load font B if font A has not been loaded

Maybe I went too far with it 😅 This could be achieved in the future if needed, anyways!

Thanks, @aristath! Any observations about this PR?

We thought about making the API look more WP-ish by adding wp_(register|enqueue)_webfonts as separate methods so instead of filtering out, we could make the user/developer opt IN on which fonts they'd like to have on their pages. This way we'd achieve more predictable behavior and as a consequence, wouldn't hurt extensibility.

aristath · 2022-03-15T08:14:31Z

Question: Scripts & styles have enqueue and register methods, because they have dependencies. So the concept is that we register everything, and then enqueue things as needed based on the dependencies tree.
How does that relate with webfonts? Why do we need enqueue/register for something that does not have dependencies? 🤔

zaguiini · 2022-03-15T17:14:09Z

Hey @aristath, thanks for the messages!

Why was the get_font_id method removed from the WP_Webfonts class? 🤔 I see we removed it in this PR, then added a top-level gutenberg_generate_font_id function, and then add a trigger_error when enqueueing a webfont in case there is no ID. I also see that we're calling the gutenberg_generate_font_id function when adding a webfont from theme.json, but not when adding the webfont using the PHP functions.

If all fonts are required to have an ID, then why not have the method as part of the WP_Webfonts class like it was before? Would that not cover all cases and ensure that a webfont always has an ID, instead of adding another point of possible failure? Or am I missing something obvious? Can you please explain the rationale? I don't see the benefit here... Please note that I did not test the implementation, I was just looking at the code, so there's a big chance my concerns are invalid and I just missed something in the logic.

The idea is that developers provide their own hashing mechanism so they can understand how creating an ID works -- since they're creating their own.

I do believe that adding the gutenberg_generate_font_id function is confusing. The reason why it's there is that no theme specifies the ID when registering a font through theme.json so we wanted to maintain it backward compatible. That's why this function is used there.

So, now that we're separating registration from enqueueing, I do think it makes sense for the developers to come with their own IDs, so that they can register and enqueue the fonts as they need instead of relying on an abstract mechanism.

Question: Scripts & styles have enqueue and register methods, because they have dependencies. So the concept is that we register everything, and then enqueue things as needed based on the dependencies tree.
How does that relate with webfonts? Why do we need enqueue/register for something that does not have dependencies? 🤔

As we discussed previously, yes, there are no dependencies between fonts. The reason we decided to go that route is solely based on API similarity: we are registering resources and then enqueueing them as needed.

I do not see a reason to treat webfonts differently: we are registering so the users can pick them up in the editor, and enqueueing them as they're picked.

The API is vastly similar to what WP developers are already used to: wp_register_webfont and wp_enqueue_webfont, just like we have with styles (wp_register_style and wp_enqueue_style) and scripts (wp_register_script and wp_enqueue_script). This similarity will help in API adoption and will keep familiar lingo across WordPress codebases. The only things we're picking from previous experiences are the name and part of the method signature.

jeyip · 2022-03-16T04:41:41Z

@aristath Thanks for the prompt replies. As we continued working on this PR, we were wondering about a few more things:

Why we do parse the font-face src URL in register-webfonts-from-theme-json.php, and not in the local Provider? l40-43 below seem like logic specific to locally hosted fonts.

gutenberg/lib/compat/wordpress-6.0/register-webfonts-from-theme-json.php

Lines 33 to 45 in 05b8a7d

    
           foreach ( $font_family['fontFace'] as $font_face ) { 
        
           	// Check if webfonts have a "src" param, and if they do account for the use of "file:./". 
        
           	if ( ! empty( $font_face['src'] ) ) { 
        
           		$font_face['src'] = (array) $font_face['src']; 
        
           		foreach ( $font_face['src'] as $src_key => $url ) { 
        
           			// Tweak the URL to be relative to the theme root. 
        
           			if ( 0 !== strpos( $url, 'file:./' ) ) { 
        
           				continue; 
        
           			} 
        
           			$font_face['src'][ $src_key ] = get_theme_file_uri( str_replace( 'file:./', '', $url ) ); 
        
           		} 
        
           	}

jeyip · 2022-03-16T04:50:01Z

@carolinan @creativecoder @spacedmonkey @TimothyBJacobs

We'd love your input on this PR -- it stands to solve potential performance problems users may have in the future and has significant ramifications on how the webfonts API will evolve.

zaguiini · 2022-03-17T00:24:00Z

We're now placing the logic to transform file:./ into the asset URL inside the local provider.

Apart from being simple separation of concerns, there is an implication: a webfont enqueueing might happen after its theme.json registration.

Referencing #39399's use case, we want to enqueue the webfonts that are used in the content, and that's pretty late in the process -- almost before rendering. Because of that, the URL transformation wouldn't get applied.

So it's been moved to the provider. The reason why it's done here and not in the other PR is that I don't want to deliver this PR in a broken state where enqueued webfonts are not shown in the front-end because of the wrong URL.

mattwiebe

This approach makes sense, both because 1) it tracks with the existing WP mental model of scripts and styles having separate register and enqueue functionality, and 2) it promotes better performance by not trying to render fonts that are not in use.

I've left some smaller feedback on a couple of items as well to get this into a mergeable state.

mattwiebe · 2022-03-17T16:08:54Z

lib/compat/wordpress-6.0/class-wp-webfonts.php

-	public function register_font( $font ) {
+	public function register_font( $id, $font ) {
+		if ( ! $id ) {
+			trigger_error( __( 'An ID is necessary when registering a webfont.', 'gutenberg' ) );


I think that using the WP internal _doing_it_wrong here would be better. Plugins and themes do all kinds of weird things and we generally don't want a users' site to WSOD on account of it.

_doing_it_wrong has the virtue of throwing actual errors when WP_DEBUG is defined, which is typical for developers, who will get the more aggressive treatment.

I see several other uses of trigger_error in here and would replace all of them.

mattwiebe · 2022-03-17T16:20:21Z

lib/compat/wordpress-6.0/webfonts.php

+ * <code>
+ * wp_enqueue_webfonts(
+ *      array(
+ *          'some-already-registered-font', // This requires the font to be registered beforehand.


I would tweak this slightly to comment on both the already-registered font and the new font. The latter isn't explicitly commented-on here. I would change the example to something like:

wp_enqueue_webfonts( array( // Pass an ID for an already-registered font: 'some-already-registered-font', // Register and enqueue some new fonts: array( 'id' => 'source-serif-200-900-normal', 'provider' => 'local', 'font_family' => 'Source Serif Pro', 'font_weight' => '200 900', 'font_style' => 'normal', 'src' => get_theme_file_uri( 'assets/fonts/source-serif-pro/SourceSerif4Variable-Roman.ttf.woff2' ), ), array( 'id' => 'source-serif-200-900-italic', 'provider' => 'local', 'font_family' => 'Source Serif Pro', 'font_weight' => '200 900', 'font_style' => 'italic', 'src' => get_theme_file_uri( 'assets/fonts/source-serif-pro/SourceSerif4Variable-Italic.ttf.woff2' ), ), ) );

zaguiini · 2022-03-17T17:48:19Z

lib/compat/wordpress-6.0/register-webfonts-from-theme-json.php

@@ -58,7 +55,8 @@ function gutenberg_register_webfonts_from_theme_json() {
 		}
 	}
 	foreach ( $webfonts as $webfont ) {
-		wp_webfonts()->register_font( $webfont );
+		$id = isset( $webfont['id'] ) ? $webfont['id'] : gutenberg_generate_font_id( $webfont );


The sole reason we're doing this ternary is that it might break themes already using the Webfont API and registering webfonts, but without specifying the font id.

The thing is, the Webfont API hasn't been made public yet, so we can do whatever we want with the signature... Including breaking changes such as this one.

"Why are we making the id key required?"

The idea is that developers provide their own hashing mechanism so they can register IDs that works for them, since they're creating their own.

I do believe that adding the gutenberg_generate_font_id function is confusing. The reason why it's there is that no theme specifies the ID when registering a font through theme.json so we wanted to maintain it backward compatible. That's why this function is used there.

So, now that we're separating registration from enqueueing, I do think it makes sense for the developers to come with their own IDs, so that they can register and enqueue the fonts as they need instead of relying on an abstract mechanism that they're not aware of. Explicit is better than implicit.

aristath · 2022-03-18T07:56:04Z

Why we do parse the font-face src URL in register-webfonts-from-theme-json.php, and not in the local Provider? l40-43 below seem like logic specific to locally hosted fonts.

Yeah, that one was a bit weird... The problem was that the file:./ logic would only work in a theme since it was getting the file relative to the theme root's folder. But we have both theme.json and PHP (wp_register_webfonts) implementations. Plugins can use the wp_register_webfonts function to register a webfont, so if done in a plugin, the file:./ reference would be expected to work relatively to the file where the function call was made. But doing that would increase the complexity A LOT, without any real reason. The file:./ reference is only necessary in a theme.json context because in JSON we can't use something like get_theme_file_uri(), while in PHP we can.
Since it was something only needed for theme.json and not the wp_register_webfonts function, it was deemed better to not place that piece of code in the local provider.
I hope that makes some sense... 👍

zaguiini · 2022-03-22T19:23:02Z

Closed in detriment of #39559

jeyip mentioned this pull request Mar 10, 2022

Webfonts API: Filter out enqueued webfonts not used in frontend [DO NOT MERGE] #39343

Draft

zaguiini mentioned this pull request Mar 10, 2022

Webfonts: remove unused font-face declarations #39332

Closed

zaguiini force-pushed the try/expose-enqueue-webfont-method branch 2 times, most recently from ceaf7e9 to 6bcd1b0 Compare March 10, 2022 19:41

jeyip assigned zaguiini Mar 10, 2022

jeyip added the [Feature] Webfonts label Mar 10, 2022

zaguiini mentioned this pull request Mar 10, 2022

Webfonts: change class properties from static to instance members #39361

Merged

zaguiini force-pushed the try/expose-enqueue-webfont-method branch 5 times, most recently from 74b8d4d to da0cfe6 Compare March 11, 2022 18:52

zaguiini changed the title ~~Webfonts API: expose enqueueing method instead of directly enqueue webfont on registration~~ Webfonts: expose enqueueing method instead of directly enqueueing on registration Mar 11, 2022

zaguiini marked this pull request as ready for review March 11, 2022 18:59

zaguiini requested review from TimothyBJacobs and spacedmonkey as code owners March 11, 2022 18:59

zaguiini force-pushed the try/expose-enqueue-webfont-method branch from da0cfe6 to 8a53646 Compare March 11, 2022 19:09

zaguiini changed the title ~~Webfonts: expose enqueueing method instead of directly enqueueing on registration~~ Webfonts: expose enqueueing method instead of directly enqueueing fonts on registration Mar 11, 2022

zaguiini force-pushed the try/expose-enqueue-webfont-method branch from 8a53646 to 953d6e9 Compare March 11, 2022 21:04

zaguiini mentioned this pull request Mar 11, 2022

Webfonts: scan content for webfonts and enqueue them #39399

Closed

1 task

zaguiini requested review from jeyip, mattwiebe, aristath, Addison-Stavlo and carolinan March 11, 2022 22:37

zaguiini force-pushed the try/expose-enqueue-webfont-method branch from be66425 to 7b4900a Compare March 15, 2022 17:44

zaguiini force-pushed the try/expose-enqueue-webfont-method branch 3 times, most recently from f763e10 to e497a0c Compare March 17, 2022 00:08

zaguiini and others added 14 commits March 16, 2022 21:49

Separate between registered and enqueued webfonts

86ece5f

Expose webfont enqueueing method

b084fd0

Register fonts from theme.json instead of directly enqueueing them

a9bf4b6

Expose bulk webfont enqueueing method

91dbeba

Remove misleading comment regarding arguments list

ab36072

Enhance wp_register_webfonts and wp_register_webfont tests

53ab9d1

Add wp_enqueue_webfonts and wp_enqueue_webfont tests

0888132

Make sure wp-block-library is enqueued before adding inline styles to it

6a0cf04

Remove static variable declaration for registered and enqueued webfonts

59ef049

Reference the registered_webfonts as an instance variable

e0a573d

Reference the enqueued_webfonts as an instance variable

391b0b0

Add test for generate_and_enqueue_styles

2df17bf

Add test for generate_and_enqueue_editor_styles

7a12067

Move src attribute processing to the local provider

5ab6da7

zaguiini force-pushed the try/expose-enqueue-webfont-method branch from e497a0c to 5ab6da7 Compare March 17, 2022 00:49

mattwiebe reviewed Mar 17, 2022

View reviewed changes

zaguiini commented Mar 17, 2022

View reviewed changes

zaguiini closed this Mar 22, 2022

zaguiini deleted the try/expose-enqueue-webfont-method branch October 20, 2023 17:16

Directory

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webfonts API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

Webfonts API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

zaguiini commented Mar 9, 2022 •

edited

jeyip commented Mar 10, 2022

aristath commented Mar 10, 2022

zaguiini commented Mar 10, 2022

aristath commented Mar 10, 2022

zaguiini commented Mar 10, 2022

aristath commented Mar 15, 2022

zaguiini commented Mar 15, 2022

jeyip commented Mar 16, 2022 •

edited

jeyip commented Mar 16, 2022

zaguiini commented Mar 17, 2022 •

edited

mattwiebe left a comment

mattwiebe Mar 17, 2022

mattwiebe Mar 17, 2022

zaguiini Mar 17, 2022 •

edited

aristath commented Mar 18, 2022

zaguiini commented Mar 22, 2022

Directory

Webfonts API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

Webfonts API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

Conversation

zaguiini commented Mar 9, 2022 • edited

What?

Why?

How?

Testing Instructions

Testing a webfont loaded via theme.json:

Testing a webfont programmatically through PHP:

jeyip commented Mar 10, 2022

aristath commented Mar 10, 2022

zaguiini commented Mar 10, 2022

aristath commented Mar 10, 2022

zaguiini commented Mar 10, 2022

aristath commented Mar 15, 2022

zaguiini commented Mar 15, 2022

jeyip commented Mar 16, 2022 • edited

jeyip commented Mar 16, 2022

zaguiini commented Mar 17, 2022 • edited

mattwiebe left a comment

Choose a reason for hiding this comment

mattwiebe Mar 17, 2022

Choose a reason for hiding this comment

mattwiebe Mar 17, 2022

Choose a reason for hiding this comment

zaguiini Mar 17, 2022 • edited

Choose a reason for hiding this comment

"Why are we making the id key required?"

aristath commented Mar 18, 2022

zaguiini commented Mar 22, 2022

zaguiini commented Mar 9, 2022 •

edited

Testing a webfont loaded via `theme.json`:

jeyip commented Mar 16, 2022 •

edited

zaguiini commented Mar 17, 2022 •

edited

zaguiini Mar 17, 2022 •

edited

"Why are we making the `id` key required?"