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 API: expose enqueueing method instead of directly enqueueing fonts on registration #39327

Closed
wants to merge 14 commits into from
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Register fonts from theme.json instead of directly enqueueing them
  • Loading branch information
zaguiini committed Mar 17, 2022
commit a9bf4b6ec62ec6aa89fdec79e0d24dffec1f4104
13 changes: 12 additions & 1 deletion lib/compat/wordpress-6.0/register-webfonts-from-theme-json.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@
* @package gutenberg
*/

/**
* Generates a font id based on the provided font object.
*
* @param array $font The font object.
* @return string
*/
function gutenberg_generate_font_id( $font ) {
return sanitize_title( "{$font['font-family']}-{$font['font-weight']}-{$font['font-style']}-{$font['provider']}" );
}

/**
* Register webfonts defined in theme.json.
*/
Expand Down Expand Up @@ -58,7 +68,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 );
Copy link
Member Author

@zaguiini zaguiini Mar 17, 2022

Choose a reason for hiding this comment

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

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.

wp_register_webfont( $id, $webfont );
}
}

Expand Down