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

Add a client-side font face resolver in the editor #65019

Open
wants to merge 20 commits into
base: trunk
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
6219a6d
add editor font face resolver component
matiasbenedetto Sep 3, 2024
1e8ef55
remove code not needed
matiasbenedetto Sep 4, 2024
65f3754
default value when font families aren't defined
matiasbenedetto Sep 5, 2024
77f7638
comments formatting
matiasbenedetto Sep 9, 2024
0bed01d
use callback for loadFontFaceAsset
matiasbenedetto Sep 9, 2024
48bfe65
improve syntax
matiasbenedetto Sep 9, 2024
65d147a
Merge branch 'trunk' into add/editor-font-face-resolver
matiasbenedetto Sep 11, 2024
c6a32f9
Move EditorFontsResolver inside EditorStyles, use fontFamilies data f…
matiasbenedetto Sep 13, 2024
5a9e2d0
use a ref to reference the current document
matiasbenedetto Sep 13, 2024
27b5e64
currentTheme default to empty object
matiasbenedetto Sep 16, 2024
463fa95
revert changes on useDarkThemeBodyClassName, refactor useEditorFontsR…
matiasbenedetto Sep 18, 2024
9bd3378
revert not needed change
matiasbenedetto Sep 18, 2024
5cecd18
Merge branch 'trunk' into add/editor-font-face-resolver
matiasbenedetto Sep 20, 2024
c262896
Merge branch 'trunk' into add/editor-font-face-resolver
matiasbenedetto Sep 20, 2024
8ced871
try adding currentTheme to the editor settings
matiasbenedetto Sep 20, 2024
ee98b6e
add theme fonts uris
matiasbenedetto Sep 26, 2024
1461989
use _links in the font resolver
matiasbenedetto Sep 27, 2024
8172039
Merge branch 'trunk' into add/editor-font-face-resolver
matiasbenedetto Sep 30, 2024
7cb4c01
Fix PHP linting
getdave Oct 7, 2024
20a00e3
Use correct alias
getdave Oct 7, 2024
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
50 changes: 50 additions & 0 deletions lib/class-wp-theme-json-resolver-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -831,6 +831,50 @@
return $variations;
}

/**
* Resolves relative paths in theme.json typography to theme absolute paths
* and returns them in an array that can be embedded
* as the value of `_link` object in REST API responses.
*
* @since 6.6.0
*
* @param

Check failure on line 841 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Whitespace found at end of line
* @return array An array of resolved paths.
*/
private static function get_resolved_fonts_theme_uris( $theme_json_data ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think this could be part of get_resolved_theme_uris - it's not that much code, and it could reuse $placeholder etc.

Example diff
diff --git a/lib/class-wp-theme-json-resolver-gutenberg.php b/lib/class-wp-theme-json-resolver-gutenberg.php
index 9c64e09b7e..c128a3c6ea 100644
--- a/lib/class-wp-theme-json-resolver-gutenberg.php
+++ b/lib/class-wp-theme-json-resolver-gutenberg.php
@@ -838,30 +838,30 @@ class WP_Theme_JSON_Resolver_Gutenberg {
 	 *
 	 * @since 6.6.0
 	 *
-	 * @param 
+	 * @param
 	 * @return array An array of resolved paths.
 	 */
 	private static function get_resolved_fonts_theme_uris( $theme_json_data ) {
 		$resolved_theme_uris = array();
 
-		if ( !empty( $theme_json_data['settings']['typography']['fontFamilies'] ) ) {
+		if ( ! empty( $theme_json_data['settings']['typography']['fontFamilies'] ) ) {
 
 			$font_families = ( $theme_json_data['settings']['typography']['fontFamilies']['theme'] ?? array() )
 				+ ( $theme_json_data['settings']['typography']['fontFamilies']['custom'] ?? array() )
 				+ ( $theme_json_data['settings']['typography']['fontFamilies']['default'] ?? array() );
 
 			foreach ( $font_families as $font_family ) {
-				if ( !empty( $font_family['fontFace'] ) ) {
+				if ( ! empty( $font_family['fontFace'] ) ) {
 					foreach ( $font_family['fontFace'] as $font_face ) {
-						if ( !empty( $font_face['src'] ) ) {
-							$sources = is_string( $font_face['src'] ) 
+						if ( ! empty( $font_face['src'] ) ) {
+							$sources = is_string( $font_face['src'] )
 								? array( $font_face['src'] )
 								: $font_face['src'];
 							foreach ( $sources as $source ) {
 								if ( str_starts_with( $source, 'file:' ) ) {
 									$resolved_theme_uris[] = array(
-										'name' => $source,
-										'href' => sanitize_url( get_theme_file_uri( str_replace( 'file:./', '', $source ) ) ),
+										'name'   => $source,
+										'href'   => sanitize_url( get_theme_file_uri( str_replace( 'file:./', '', $source ) ) ),
 										'target' => "typography.fontFamilies.{$font_family['slug']}.fontFace.src",
 									);
 								}
@@ -896,12 +896,6 @@ class WP_Theme_JSON_Resolver_Gutenberg {
 
 		$theme_json_data = $theme_json->get_raw_data();
 
-		// Add font URIs.
-		$resolved_theme_uris = array_merge(
-			$resolved_theme_uris,
-			static::get_resolved_fonts_theme_uris( $theme_json_data )
-		);
-
 		// Using the same file convention when registering web fonts. See: WP_Font_Face_Resolver:: to_theme_file_uri.
 		$placeholder = 'file:./';
 
@@ -952,6 +946,31 @@ class WP_Theme_JSON_Resolver_Gutenberg {
 			}
 		}
 
+		// Add font URIs.
+		if ( ! empty( $theme_json_data['settings']['typography']['fontFamilies'] ) ) {
+			$font_families = array_merge(
+				$theme_json_data['settings']['typography']['fontFamilies']['theme'] ?? array(),
+				$theme_json_data['settings']['typography']['fontFamilies']['custom'] ?? array(),
+				$theme_json_data['settings']['typography']['fontFamilies']['default'] ?? array()
+			);
+			foreach ( $font_families as $font_family ) {
+				if ( ! empty( $font_family['fontFace'] ) ) {
+					foreach ( $font_family['fontFace'] as $font_face ) {
+						$sources = (array) ( $font_face['src'] ?? array() );
+						foreach ( $sources as $source ) {
+							if ( is_string( $source ) && strpos( $source, $placeholder ) === 0 ) {
+								$resolved_theme_uris[] = array(
+									'name'   => $source,
+									'href'   => sanitize_url( get_theme_file_uri( substr( $source, strlen( $placeholder ) ) ) ),
+									'target' => "typography.fontFamilies.{$font_family['slug']}.fontFace.src",
+								);
+							}
+						}
+					}
+				}
+			}
+		}
+
 		return $resolved_theme_uris;
 	}

$resolved_theme_uris = array();

if ( !empty( $theme_json_data['settings']['typography']['fontFamilies'] ) ) {

Check failure on line 847 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Expected 1 space after "!"; 0 found

$font_families = ( $theme_json_data['settings']['typography']['fontFamilies']['theme'] ?? array() )
+ ( $theme_json_data['settings']['typography']['fontFamilies']['custom'] ?? array() )
Copy link
Member

Choose a reason for hiding this comment

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

Was the union operator + intentional? I think array_merge() is more readable and will reindex the new array, avoid potential lost data.

See: https://stitcher.io/blog/array-merge-vs+

Unit test coverage would help test that.

+ ( $theme_json_data['settings']['typography']['fontFamilies']['default'] ?? array() );

foreach ( $font_families as $font_family ) {
if ( !empty( $font_family['fontFace'] ) ) {

Check failure on line 854 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Expected 1 space after "!"; 0 found
foreach ( $font_family['fontFace'] as $font_face ) {
if ( !empty( $font_face['src'] ) ) {

Check failure on line 856 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Expected 1 space after "!"; 0 found
$sources = is_string( $font_face['src'] )

Check failure on line 857 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Whitespace found at end of line
? array( $font_face['src'] )
: $font_face['src'];
foreach ( $sources as $source ) {
if ( str_starts_with( $source, 'file:' ) ) {
$resolved_theme_uris[] = array(
'name' => $source,

Check warning on line 863 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Array double arrow not aligned correctly; expected 3 space(s) between "'name'" and double arrow, but found 1.
'href' => sanitize_url( get_theme_file_uri( str_replace( 'file:./', '', $source ) ) ),

Check warning on line 864 in lib/class-wp-theme-json-resolver-gutenberg.php

View workflow job for this annotation

GitHub Actions / PHP coding standards

Array double arrow not aligned correctly; expected 3 space(s) between "'href'" and double arrow, but found 1.
'target' => "typography.fontFamilies.{$font_family['slug']}.fontFace.src",
);
}
}
}
}
}
}
}

return $resolved_theme_uris;
}


/**
* Resolves relative paths in theme.json styles to theme absolute paths
Expand All @@ -852,6 +896,12 @@

$theme_json_data = $theme_json->get_raw_data();

// Add font URIs.
$resolved_theme_uris = array_merge(
$resolved_theme_uris,
static::get_resolved_fonts_theme_uris( $theme_json_data )
);

// Using the same file convention when registering web fonts. See: WP_Font_Face_Resolver:: to_theme_file_uri.
$placeholder = 'file:./';

Expand Down
4 changes: 4 additions & 0 deletions packages/block-editor/src/components/editor-styles/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { useSelect } from '@wordpress/data';
import transformStyles from '../../utils/transform-styles';
import { store as blockEditorStore } from '../../store';
import { unlock } from '../../lock-unlock';
import useEditorFontsResolver from '../use-editor-fonts-resolver';

extend( [ namesPlugin, a11yPlugin ] );

Expand Down Expand Up @@ -105,6 +106,9 @@ function EditorStyles( { styles, scope, transformOptions } ) {
<style
ref={ useDarkThemeBodyClassName( transformedStyles, scope ) }
/>

<style ref={ useEditorFontsResolver() } />

{ transformedStyles.map( ( css, index ) => (
<style key={ index }>{ css }</style>
) ) }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* WordPress dependencies
*/
import { useState, useMemo, useCallback } from '@wordpress/element';
import { useSelect } from '@wordpress/data';

/**
* Internal dependencies
*/
import { getDisplaySrcFromFontFace, loadFontFaceInBrowser } from './utils';
import { store as editorStore } from '../../store';
Copy link
Member

Choose a reason for hiding this comment

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

Let's use blockEditorStore to be specific and differentiate from the editor package's store.

Suggested change
import { store as editorStore } from '../../store';
import { store as blockEditorStore } from '../../store';


function useEditorFontsResolver() {
const [ loadedFontUrls, setLoadedFontUrls ] = useState( new Set() );

const { currentTheme = {}, fontFamilies = [] } = useSelect( ( select ) => {
return {
currentTheme:
select( editorStore ).getSettings()?.__experimentalFeatures
?.currentTheme,
fontFamilies:
select( editorStore ).getSettings()?.__experimentalFeatures
?.typography?.fontFamilies,
};
Copy link
Member

Choose a reason for hiding this comment

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

Opportunity to call getSettings() once, e.g.,

		const { __experimentalFeatures } =
			select( blockEditorStore ).getSettings();
		return {
			currentTheme: __experimentalFeatures?.currentTheme,
			fontFamilies: __experimentalFeatures?.typography?.fontFamilies,
		};

}, [] );

const fontFaces = useMemo( () => {
return Object.values( fontFamilies )
.flat()
.map( ( family ) => family.fontFace )
.filter( Boolean )
.flat();
}, [ fontFamilies ] );

const loadFontFaceAsset = useCallback(
async ( fontFace, ownerDocument ) => {
if ( ! fontFace.src ) {
return;
}

const src = getDisplaySrcFromFontFace(
fontFace.src,
currentTheme?.stylesheet_uri
);

if ( ! src || loadedFontUrls.has( src ) ) {
return;
}

loadFontFaceInBrowser( fontFace, src, ownerDocument );
setLoadedFontUrls( ( prevUrls ) => new Set( prevUrls ).add( src ) );
},
[ currentTheme, loadedFontUrls ]
);

return useCallback(
( node ) => {
if ( ! node ) {
return;
}

const { ownerDocument } = node;
fontFaces.forEach( ( fontFace ) =>
loadFontFaceAsset( fontFace, ownerDocument )
);
},
[ fontFaces, loadFontFaceAsset ]
);
}

export default useEditorFontsResolver;
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
/*
* Format the font face name to use in the font-family property of a font face.
*
* The input can be a string with the font face name or a string with multiple font face names separated by commas.
* It removes the leading and trailing quotes from the font face name.
*
* @param {string} input - The font face name.
* @return {string} The formatted font face name.
*
* Example:
* formatFontFaceName("Open Sans") => "Open Sans"
* formatFontFaceName("'Open Sans', sans-serif") => "Open Sans"
* formatFontFaceName(", 'Open Sans', 'Helvetica Neue', sans-serif") => "Open Sans"
*/
function formatFontFaceName( input ) {
if ( ! input ) {
return '';
}

let output = input.trim();
if ( output.includes( ',' ) ) {
output = output
.split( ',' )
// Finds the first item that is not an empty string.
.find( ( item ) => item.trim() !== '' )
.trim();
}
Comment on lines +21 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we describe why we need to do this? I can see what but at a casual glance I'm not sure as to why.

// Removes leading and trailing quotes.
output = output.replace( /^["']|["']$/g, '' );

// Firefox needs the font name to be wrapped in double quotes meanwhile other browsers don't.
if ( window.navigator.userAgent.toLowerCase().includes( 'firefox' ) ) {
output = `"${ output }"`;
}
Comment on lines +31 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if we just let all browsers have the wrapped version?

return output;
}

/*
* Loads the font face from a URL and adds it to the browser.
* It also adds it to the iframe document.
*/
export async function loadFontFaceInBrowser( fontFace, source, documentRef ) {
if ( typeof source !== 'string' ) {
return;
}
const dataSource = `url(${ source })`;
const newFont = new window.FontFace(
formatFontFaceName( fontFace.fontFamily ),
dataSource,
{
style: fontFace.fontStyle,
weight: fontFace.fontWeight,
}
);

const loadedFace = await newFont.load();

// Add the font to the ref document.
documentRef.fonts.add( loadedFace );

// Add the font to the window document.
if ( documentRef !== window.document ) {
window.document.fonts.add( loadedFace );
}
}

function isUrlEncoded( url ) {
if ( typeof url !== 'string' ) {
return false;
}
return url !== decodeURIComponent( url );
}

/*
* Retrieves the display source from a font face src.
*
* @param {string|string[]} fontSrc - The font face src.
* @param {string} baseUrl - The base URL to resolve the src.
* @return {string|undefined} The display source or undefined if the input is invalid.
*/
export function getDisplaySrcFromFontFace( fontSrc, baseUrl ) {
if ( ! fontSrc ) {
return;
}

let src;
if ( Array.isArray( fontSrc ) ) {
src = fontSrc[ 0 ];
} else {
src = fontSrc;
}

if ( ! isUrlEncoded( src ) ) {
src = encodeURI( src );
}

// If baseUrl is provided, use it to resolve the src.
if ( src.startsWith( 'file:.' ) ) {
src = baseUrl + '/' + src.replace( 'file:./', '' );
}

return src;
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { useEffect } from '@wordpress/element';
import { useSelect, useDispatch } from '@wordpress/data';
import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor';
import { store as coreDataStore } from '@wordpress/core-data';

/**
* Internal dependencies
Expand All @@ -15,12 +16,18 @@ import { TEMPLATE_POST_TYPE } from '../../utils/constants';
const { useGlobalStylesOutput } = unlock( blockEditorPrivateApis );

function useGlobalStylesRenderer() {
const postType = useSelect( ( select ) => {
return select( editSiteStore ).getEditedPostType();
const { postType, currentTheme } = useSelect( ( select ) => {
return {
postType: select( editSiteStore ).getEditedPostType(),
currentTheme: select( coreDataStore ).getCurrentTheme(),
};
} );
const [ styles, settings ] = useGlobalStylesOutput(
postType !== TEMPLATE_POST_TYPE
);

settings.currentTheme = currentTheme;

const { getSettings } = useSelect( editSiteStore );
const { updateSettings } = useDispatch( editSiteStore );

Expand Down
Loading