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.

Sign up for GitHub

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

[Fonts API] Relocate which fonts to print into wp_print_fonts() #50151

Merged
merged 7 commits into from
May 8, 2023
Merged

[Fonts API] Relocate which fonts to print into wp_print_fonts() #50151

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0af2d56
Relocate from 6.2 script-loader
hellofromtonya May 1, 2023
6147c9b
Add flag to 6.3-script-loader call
hellofromtonya May 1, 2023
6dd2c35
Add tests
hellofromtonya May 2, 2023
a64362c
Assign registered to $handles.
hellofromtonya May 2, 2023
d95eb1c
Set $handles to true
hellofromtonya May 2, 2023
fd7ce33
Combine ifs into if/elseif + micro-optimization
hellofromtonya May 3, 2023
dfd8657
Restore overwriting queue and done.
hellofromtonya May 8, 2023
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
27 changes: 5 additions & 22 deletions lib/compat/wordpress-6.2/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,28 +129,11 @@ function gutenberg_resolve_assets_override() {

$scripts = ob_get_clean();

/*
* Generate font @font-face styles for the site editor iframe.
* Use the registered font families for printing.
*/
if ( class_exists( 'WP_Fonts' ) ) {
$wp_fonts = wp_fonts();
$registered = $wp_fonts->get_registered_font_families();
if ( ! empty( $registered ) ) {
$queue = $wp_fonts->queue;
$done = $wp_fonts->done;

$wp_fonts->done = array();
$wp_fonts->queue = $registered;

ob_start();
$wp_fonts->do_items();
$styles .= ob_get_clean();

// Reset the Web Fonts API.
$wp_fonts->done = $done;
$wp_fonts->queue = $queue;
}
// Generate font @font-face styles.
if ( function_exists( 'wp_print_fonts' ) ) {
ob_start();
wp_print_fonts( true );
$styles .= ob_get_clean();
}

return array(
Expand Down
2 changes: 1 addition & 1 deletion lib/compat/wordpress-6.3/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function _gutenberg_get_iframed_editor_assets() {

ob_start();
wp_print_styles();
wp_print_fonts();
wp_print_fonts( true );
$styles = ob_get_clean();

ob_start();
Expand Down
12 changes: 9 additions & 3 deletions lib/experimental/fonts-api/fonts-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,10 @@ function wp_register_font_provider( $name, $classname ) {
*
* @since X.X.X
*
* @param string|string[]|false $handles Optional. Items to be processed: queue (false),
* single item (string), or multiple items (array of strings).
* Default false.
* @param string|string[]|bool $handles Optional. Items to be processed: queue (false),
* for iframed editor assets (true), single item (string),
* or multiple items (array of strings).
* Default false.
* @return array|string[] Array of font handles that have been processed.
* An empty array if none were processed.
*/
Expand All @@ -197,6 +198,11 @@ function wp_print_fonts( $handles = false ) {

_wp_scripts_maybe_doing_it_wrong( __FUNCTION__ );

// Print all registered fonts for the iframed editor.
if ( true === $handles ) {
$handles = $registered;
}

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this condition be moved above the if ( empty( $handles ) ) { check a few lines above? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good observation @aristath! Yes, I agree with you.

In looking further at this area of code, there's a tiny tiny micro-optimization opportunity which will benefit the front-end, i.e. when false then skip over these decision-making reassignment conditionals as they are unnecessary.

Changes done here fd7ce33

return $wp_fonts->do_items( $handles );
}
}
Expand Down
66 changes: 66 additions & 0 deletions phpunit/fonts-api/wpPrintFonts-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,70 @@ private function setup_integrated_deps( array $setup, $wp_fonts, $enqueue = true
$wp_fonts->enqueue( $setup['enqueued'] );
}
}

/**
* @dataProvider data_should_print_all_registered_fonts_for_iframed_editor
*
* @param string $fonts Fonts to register.
* @param array $expected Expected results.
*/
public function test_should_print_all_registered_fonts_for_iframed_editor( $fonts, $expected ) {
wp_register_fonts( $fonts );

$this->expectOutputString( $expected['output'] );
$actual_done = wp_print_fonts( true );
$this->assertSameSets( $expected['done'], $actual_done, 'All registered font-family handles should be returned' );
}

/**
* Data provider.
*
* @return array
*/
public function data_should_print_all_registered_fonts_for_iframed_editor() {
$local_fonts = $this->get_registered_local_fonts();
$font_faces = $this->get_registered_fonts_css();

return array(
'Merriweather with 1 variation' => array(
'fonts' => array( 'merriweather' => $local_fonts['merriweather'] ),
'expected' => array(
'done' => array( 'merriweather', 'merriweather-200-900-normal' ),
'output' => sprintf(
"<style id='wp-fonts-local' type='text/css'>\n%s\n</style>\n",
$font_faces['merriweather-200-900-normal']
),
),
),
'Source Serif Pro with 2 variations' => array(
'fonts' => array( 'Source Serif Pro' => $local_fonts['Source Serif Pro'] ),
'expected' => array(
'done' => array( 'source-serif-pro', 'Source Serif Pro-300-normal', 'Source Serif Pro-900-italic' ),
'output' => sprintf(
"<style id='wp-fonts-local' type='text/css'>\n%s%s\n</style>\n",
$font_faces['Source Serif Pro-300-normal'],
$font_faces['Source Serif Pro-900-italic']
),
),
),
'all fonts' => array(
'fonts' => $local_fonts,
'expected' => array(
'done' => array(
'merriweather',
'merriweather-200-900-normal',
'source-serif-pro',
'Source Serif Pro-300-normal',
'Source Serif Pro-900-italic',
),
'output' => sprintf(
"<style id='wp-fonts-local' type='text/css'>\n%s%s%s\n</style>\n",
$font_faces['merriweather-200-900-normal'],
$font_faces['Source Serif Pro-300-normal'],
$font_faces['Source Serif Pro-900-italic']
),
),
),
);
}
}