-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Face: to generate and print font-face styles for theme.json fonts #51770
Merged
Merged
Changes from all commits
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
3ffdfa1
Add WP_Font_Face_Resolver.
hellofromtonya fe8106c
Add WP_Font_Face.
hellofromtonya 017862e
Add wp_print_font_faces().
hellofromtonya f91f244
Relocate iframed editor assets font printing.
hellofromtonya aaeb453
Load files & guard usage of Fonts API.
hellofromtonya 0cf9f63
Fix wp_print_font_faces().
hellofromtonya 5e189d4
Fixed property typo
hellofromtonya 5aed83c
Test for Test WP_Font_Face::generate_and_print.
anton-vlasenko 2f5f107
Fix class name.
anton-vlasenko ab03732
Fix the test.
anton-vlasenko 9048aaf
Don't load WP_Fonts_TestCase
anton-vlasenko 771934c
Don't print the last \n character
anton-vlasenko 458b1dd
Add a comment.
anton-vlasenko 818259d
Add more data to the dataprovider.
anton-vlasenko 5f172e9
Rename files.
anton-vlasenko ac45ee2
Fix fatal error.
anton-vlasenko bd8b39c
Fix package name.
anton-vlasenko 610f403
Add a draft test for WP_Font_Face_Resolver.
anton-vlasenko 2986f7e
Commit draft test.
anton-vlasenko 3a1af83
Return an empty array if no fonts are defined. This method should alw…
anton-vlasenko 2fc983c
Improve the test.
anton-vlasenko d9a4e36
CS fix
aristath 265b5e0
Fix font names.
anton-vlasenko b2e0a09
Fix package name.
anton-vlasenko 3f90a70
Remove unrelated tests.
anton-vlasenko 5c23eab
Add a test for placeholders.
anton-vlasenko 674187e
Fix property names.
anton-vlasenko cf95e34
Fix DocBlocks.
anton-vlasenko a59bb60
Fix CS.
anton-vlasenko b878840
Fix test class names.
anton-vlasenko e392dca
Tests for wp_print_font_faces().
anton-vlasenko 41860aa
Improve the test and kick off CI checks.
anton-vlasenko 7fedb60
Fix the failing test.
anton-vlasenko 33f65a2
Quick and dirty fix for non-iframed editor. See https://wordpress.sla…
anton-vlasenko c6746db
Revert "Quick and dirty fix for non-iframed editor. See https://wordp…
anton-vlasenko 8ef75f1
Test reorganization.
hellofromtonya 6359091
Add wp_print_fonts() tests.
hellofromtonya 6f8e601
Bail out if no fonts to process.
hellofromtonya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
<?php | ||
/** | ||
* WP_Font_Face_Resolver class. | ||
* | ||
* @package WordPress | ||
* @subpackage Fonts | ||
* @since X.X.X | ||
*/ | ||
|
||
if ( class_exists( 'WP_Font_Face_Resolver' ) ) { | ||
return; | ||
} | ||
|
||
/** | ||
* The Font Face Resolver abstracts the processing of different data sources | ||
* (such as theme.json) for processing within the Font Face. | ||
* | ||
* This class is for internal core usage and is not supposed to be used by | ||
* extenders (plugins and/or themes). | ||
* | ||
* @access private | ||
*/ | ||
class WP_Font_Face_Resolver { | ||
|
||
/** | ||
* Gets fonts defined in theme.json. | ||
* | ||
* @since X.X.X | ||
* | ||
* @return array Returns the font-families, each with their font-face variations. | ||
*/ | ||
public static function get_fonts_from_theme_json() { | ||
$settings = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data()->get_settings(); | ||
|
||
// Bail out early if there are no font settings. | ||
if ( empty( $settings['typography'] ) || empty( $settings['typography']['fontFamilies'] ) ) { | ||
return array(); | ||
} | ||
|
||
return static::parse_settings( $settings ); | ||
} | ||
|
||
/** | ||
* Parse theme.json settings to extract font definitions with variations grouped by font-family. | ||
* | ||
* @since X.X.X | ||
* | ||
* @param array $settings Font settings to parse. | ||
* @return array Returns an array of fonts, grouped by font-family. | ||
*/ | ||
private static function parse_settings( array $settings ) { | ||
$fonts = array(); | ||
|
||
foreach ( $settings['typography']['fontFamilies'] as $font_families ) { | ||
foreach ( $font_families as $definition ) { | ||
|
||
// Skip if font-family "name" is not defined. | ||
if ( empty( $definition['name'] ) ) { | ||
continue; | ||
} | ||
|
||
// Skip if "fontFace" is not defined, meaning there are no variations. | ||
if ( empty( $definition['fontFace'] ) ) { | ||
continue; | ||
} | ||
|
||
$font_family = $definition['name']; | ||
|
||
// Prepare the fonts array structure for this font-family. | ||
if ( ! array_key_exists( $font_family, $fonts ) ) { | ||
$fonts[ $font_family ] = array(); | ||
} | ||
|
||
$fonts[ $font_family ] = static::convert_font_face_properties( $definition['fontFace'], $font_family ); | ||
} | ||
} | ||
|
||
return $fonts; | ||
} | ||
|
||
/** | ||
* Converts font-face properties from theme.json format. | ||
* | ||
* @since X.X.X | ||
* | ||
* @param array $font_face_definition The font-face definitions to convert. | ||
* @param string $font_family_property The value to store in the font-face font-family property. | ||
* @return array Converted font-face properties. | ||
*/ | ||
private static function convert_font_face_properties( array $font_face_definition, $font_family_property ) { | ||
$converted_font_faces = array(); | ||
|
||
foreach ( $font_face_definition as $font_face ) { | ||
// Add the font-family property to the font-face. | ||
$font_face['font-family'] = $font_family_property; | ||
|
||
// Converts the "file:./" src placeholder into a theme font file URI. | ||
if ( ! empty( $font_face['src'] ) ) { | ||
$font_face['src'] = static::to_theme_file_uri( (array) $font_face['src'] ); | ||
} | ||
|
||
// Convert camelCase properties into kebab-case. | ||
$font_face = static::to_kebab_case( $font_face ); | ||
|
||
$converted_font_faces[] = $font_face; | ||
} | ||
|
||
return $converted_font_faces; | ||
} | ||
|
||
/** | ||
* Converts each 'file:./' placeholder into a URI to the font file in the theme. | ||
* | ||
* The 'file:./' is specified in the theme's `theme.json` as a placeholder to be | ||
* replaced with the URI to the font file's location in the theme. When a "src" | ||
* beings with this placeholder, it is replaced, converting the src into a URI. | ||
* | ||
* @since X.X.X | ||
* | ||
* @param array $src An array of font file sources to process. | ||
* @return array An array of font file src URI(s). | ||
*/ | ||
private static function to_theme_file_uri( array $src ) { | ||
$placeholder = 'file:./'; | ||
|
||
foreach ( $src as $src_key => $src_url ) { | ||
// Skip if the src doesn't start with the placeholder, as there's nothing to replace. | ||
if ( ! str_starts_with( $src_url, $placeholder ) ) { | ||
continue; | ||
} | ||
|
||
$src_file = str_replace( $placeholder, '', $src_url ); | ||
$src[ $src_key ] = get_theme_file_uri( $src_file ); | ||
} | ||
|
||
return $src; | ||
} | ||
|
||
/** | ||
* Converts all first dimension keys into kebab-case. | ||
* | ||
* @since X.X.X | ||
* | ||
* @param array $data The array to process. | ||
* @return array Data with first dimension keys converted into kebab-case. | ||
*/ | ||
private static function to_kebab_case( array $data ) { | ||
foreach ( $data as $key => $value ) { | ||
$kebab_case = _wp_to_kebab_case( $key ); | ||
$data[ $kebab_case ] = $value; | ||
if ( $kebab_case !== $key ) { | ||
unset( $data[ $key ] ); | ||
} | ||
} | ||
|
||
return $data; | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is in Core already, but it relates to a feature being built for 6.6: relative paths to background images in theme.json styles
For consistency, and after a nudge from @creativecoder, I decided to follow the
file:./
dotslash path convention to represent paths to these images.@noisysocks then pointed out that dotslash indicates the current directory, and might imply other dot-style paths, e.g.,
../
So, if web fonts can be used in theme variations, e.g.,
my-theme/styles/variation-with-fonts.json
, the./
would indicate a path relative to the styles directory.It might also imply that paths with
../../some/path.file
could also be resolved. But I'm not sure ifget_theme_file_uri()
supports such things as it looks for files relative to the active or parent theme root. Maybe I'm missing something.https://github.com/WordPress/gutenberg/blob/trunk/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php#L147C27-L147C44
I was wondering if we should enforce a convention. Either:
What do folks think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean... the syntax and behavior seem mismatched:
file:./
seems to imply a reference to the current directory, but really it's a placeholder for the theme root looking at how it's coded.Updating
file:./
to apply to the current directory (e.g.theme-root/styles
) for theme variations and/or adding resolution of../
paths would make the syntax clearer. But I worry about backward compatibility (I imagine that some theme variations are already usingfile:./
?). And some care is needed to prevent directory traversal outside the theme directory.So I favor keeping things simple, and continuing to use
file:./
simply as a placeholder to the theme directory root. But it's not a strong opinion.(Note that either way, we should add to and update the theme handbook docs to make it clearer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick reply @creativecoder
👍🏻
I went with this approach for now given that folks might be using to it already.
True. Backwards compatibility is now a requirement regardless. And I considered this in #61271 and contemplated supporting
file:./
(with the./
for folks who are now used to it) and also afile:
prefix.But I agree that it's simpler to go with what's there until folks come up with a viable alternative. Even if it's "Enforce that all paths must be relative to the theme root" - that way, we could update the docs now and make the
./
optional later without adding any complex../
support etc. 🤷🏻