From 84d2b22d95c27811f13b6e00bbf1f6574a39ff64 Mon Sep 17 00:00:00 2001 From: Jeff Ong Date: Mon, 27 Nov 2023 15:18:59 -0500 Subject: [PATCH] Font Library: fix fonts not displaying correctly (#55393) * Add kebab casing and font family wrapping in sanitization function. Remove commented code. * Simplify how kebab casing and quote wrapping is done. * Ensure incoming font is returned. * Add kebab casing to slug in form data. * Add tests for wpKebabCase * remove redundant test * test 2 different inputs on first wpKebabCase case * Move formatting functions to utils. * reduce the scope of the function * fix the live preview (without reloading the page) of the just installed fonts in the editor * format php * Add test for format_font_family. * Format php. * Add more test cases and format. * Remove accidental badKey. * Format. --------- Co-authored-by: Vicente Canales Co-authored-by: Sarah Norris <1645628+mikachan@users.noreply.github.com> Co-authored-by: Sarah Norris Co-authored-by: Matias Benedetto --- .../class-wp-font-family-utils.php | 32 ++++++++++ .../font-library/class-wp-font-family.php | 6 +- .../utils/get-intersecting-font-faces.js | 2 +- .../font-library-modal/utils/index.js | 29 ++++++--- .../test/getIntersectingFontFaces.spec.js | 61 ++++++++++++++----- .../utils/test/wpKebabCase.spec.js | 28 +++++++++ .../wpFontFamilyUtils/formatFontFamily.php | 59 ++++++++++++++++++ 7 files changed, 191 insertions(+), 26 deletions(-) create mode 100644 packages/edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js create mode 100644 phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php diff --git a/lib/experimental/fonts/font-library/class-wp-font-family-utils.php b/lib/experimental/fonts/font-library/class-wp-font-family-utils.php index 8a8ee1d4ddb5f..7d954e79e96a3 100644 --- a/lib/experimental/fonts/font-library/class-wp-font-family-utils.php +++ b/lib/experimental/fonts/font-library/class-wp-font-family-utils.php @@ -90,4 +90,36 @@ public static function has_font_mime_type( $filepath ) { return in_array( $filetype['type'], $allowed_mime_types, true ); } + + /** + * Format font family to make it valid CSS. + * + * @since 6.5.0 + * + * @param string $font_family Font family attribute. + * @return string The formatted font family attribute. + */ + public static function format_font_family( $font_family ) { + if ( $font_family ) { + $font_families = explode( ',', $font_family ); + $wrapped_font_families = array_map( + function ( $family ) { + $trimmed = trim( $family ); + if ( ! empty( $trimmed ) && strpos( $trimmed, ' ' ) !== false && strpos( $trimmed, "'" ) === false && strpos( $trimmed, '"' ) === false ) { + return "'" . $trimmed . "'"; + } + return $trimmed; + }, + $font_families + ); + + if ( count( $wrapped_font_families ) === 1 ) { + $font_family = $wrapped_font_families[0]; + } else { + $font_family = implode( ', ', $wrapped_font_families ); + } + } + + return $font_family; + } } diff --git a/lib/experimental/fonts/font-library/class-wp-font-family.php b/lib/experimental/fonts/font-library/class-wp-font-family.php index 309b1c8b7902b..58d4f476e834d 100644 --- a/lib/experimental/fonts/font-library/class-wp-font-family.php +++ b/lib/experimental/fonts/font-library/class-wp-font-family.php @@ -308,6 +308,7 @@ private function sanitize() { ), ), ); + // Creates a new WP_Theme_JSON object with the new fonts to // leverage sanitization and validation. $fonts_json = WP_Theme_JSON_Gutenberg::remove_insecure_properties( $fonts_json ); @@ -316,7 +317,10 @@ private function sanitize() { $sanitized_font = ! empty( $theme_data['settings']['typography']['fontFamilies'] ) ? $theme_data['settings']['typography']['fontFamilies'][0] : array(); - $this->data = $sanitized_font; + + $sanitized_font['slug'] = _wp_to_kebab_case( $sanitized_font['slug'] ); + $sanitized_font['fontFamily'] = WP_Font_Family_Utils::format_font_family( $sanitized_font['fontFamily'] ); + $this->data = $sanitized_font; return $this->data; } diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js index a3ffd31db2288..e21e72c58ed53 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js @@ -47,7 +47,7 @@ export default function getIntersectingFontFaces( incoming, existing ) { } ); } ); - matches.push( { ...existingFont, fontFace: matchingFaces } ); + matches.push( { ...incomingFont, fontFace: matchingFaces } ); } else { matches.push( incomingFont ); } diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js index d0a57978bcce9..f5723f5814e98 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js @@ -1,8 +1,12 @@ +/** + * External dependencies + */ +import { paramCase as kebabCase } from 'change-case'; + /** * Internal dependencies */ import { FONT_WEIGHTS, FONT_STYLES } from './constants'; -import { formatFontFamily } from './preview-styles'; export function setUIValuesNeeded( font, extraValues = {} ) { if ( ! font.name && ( font.fontFamily || font.slug ) ) { @@ -85,14 +89,10 @@ export async function loadFontFaceInBrowser( fontFace, source, addTo = 'all' ) { } // eslint-disable-next-line no-undef - const newFont = new FontFace( - formatFontFamily( fontFace.fontFamily ), - dataSource, - { - style: fontFace.fontStyle, - weight: fontFace.fontWeight, - } - ); + const newFont = new FontFace( fontFace.fontFamily, dataSource, { + style: fontFace.fontStyle, + weight: fontFace.fontWeight, + } ); const loadedFace = await newFont.load(); @@ -129,9 +129,20 @@ export function getDisplaySrcFromFontFace( input, urlPrefix ) { return src; } +// This function replicates one behavior of _wp_to_kebab_case(). +// Additional context: https://github.com/WordPress/gutenberg/issues/53695 +export function wpKebabCase( str ) { + // If a string contains a digit followed by a number, insert a dash between them. + return kebabCase( str ).replace( + /([a-zA-Z])(\d)|(\d)([a-zA-Z])/g, + '$1$3-$2$4' + ); +} + export function makeFormDataFromFontFamilies( fontFamilies ) { const formData = new FormData(); const newFontFamilies = fontFamilies.map( ( family, familyIndex ) => { + family.slug = wpKebabCase( family.slug ); if ( family?.fontFace ) { family.fontFace = family.fontFace.map( ( face, faceIndex ) => { if ( face.file ) { diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js index 91ae5f45d66da..9899005ad65b8 100644 --- a/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js @@ -5,7 +5,7 @@ import getIntersectingFontFaces from '../get-intersecting-font-faces'; describe( 'getIntersectingFontFaces', () => { it( 'returns matching font faces for matching font family', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'lobster', fontFace: [ @@ -30,15 +30,15 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); - expect( result ).toEqual( intendedFontsFamilies ); + expect( result ).toEqual( incomingFontFamilies ); } ); it( 'returns empty array when there is no match', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'lobster', fontFace: [ @@ -63,7 +63,7 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); @@ -71,7 +71,7 @@ describe( 'getIntersectingFontFaces', () => { } ); it( 'returns matching font faces', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'lobster', fontFace: [ @@ -129,7 +129,7 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); @@ -137,7 +137,7 @@ describe( 'getIntersectingFontFaces', () => { } ); it( 'returns empty array when the first list is empty', () => { - const intendedFontsFamilies = []; + const incomingFontFamilies = []; const existingFontFamilies = [ { @@ -152,7 +152,7 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); @@ -160,7 +160,7 @@ describe( 'getIntersectingFontFaces', () => { } ); it( 'returns empty array when the second list is empty', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'lobster', fontFace: [ @@ -175,7 +175,7 @@ describe( 'getIntersectingFontFaces', () => { const existingFontFamilies = []; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); @@ -183,7 +183,7 @@ describe( 'getIntersectingFontFaces', () => { } ); it( 'returns intersecting font family when there are no fonfaces', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'piazzolla', fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ], @@ -200,7 +200,7 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); @@ -208,7 +208,7 @@ describe( 'getIntersectingFontFaces', () => { } ); it( 'returns intersecting if there is an intended font face and is not present in the returning it should not be returned', () => { - const intendedFontsFamilies = [ + const incomingFontFamilies = [ { slug: 'piazzolla', fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ], @@ -226,7 +226,7 @@ describe( 'getIntersectingFontFaces', () => { ]; const result = getIntersectingFontFaces( - intendedFontsFamilies, + incomingFontFamilies, existingFontFamilies ); const expected = [ @@ -237,4 +237,35 @@ describe( 'getIntersectingFontFaces', () => { ]; expect( result ).toEqual( expected ); } ); + + it( 'updates font family definition using the incoming data', () => { + const incomingFontFamilies = [ + { + slug: 'gothic-a1', + fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ], + fontFamily: "'Gothic A1', serif", + }, + ]; + + const existingFontFamilies = [ + { + slug: 'gothic-a1', + fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ], + fontFamily: 'Gothic A1, serif', + }, + ]; + + const result = getIntersectingFontFaces( + incomingFontFamilies, + existingFontFamilies + ); + const expected = [ + { + slug: 'gothic-a1', + fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ], + fontFamily: "'Gothic A1', serif", + }, + ]; + expect( result ).toEqual( expected ); + } ); } ); diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js new file mode 100644 index 0000000000000..d296117ff3a49 --- /dev/null +++ b/packages/edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js @@ -0,0 +1,28 @@ +/** + * Internal dependencies + */ +import { wpKebabCase } from '../index'; + +describe( 'wpKebabCase', () => { + it( 'should insert a dash between a letter and a digit', () => { + const input = 'abc1def'; + const expectedOutput = 'abc-1def'; + expect( wpKebabCase( input ) ).toEqual( expectedOutput ); + + const input2 = 'abc1def2ghi'; + const expectedOutput2 = 'abc-1def-2ghi'; + expect( wpKebabCase( input2 ) ).toEqual( expectedOutput2 ); + } ); + + it( 'should not insert a dash between two letters', () => { + const input = 'abcdef'; + const expectedOutput = 'abcdef'; + expect( wpKebabCase( input ) ).toEqual( expectedOutput ); + } ); + + it( 'should not insert a dash between a digit and a hyphen', () => { + const input = 'abc1-def'; + const expectedOutput = 'abc-1-def'; + expect( wpKebabCase( input ) ).toEqual( expectedOutput ); + } ); +} ); diff --git a/phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php b/phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php new file mode 100644 index 0000000000000..4f247c5219feb --- /dev/null +++ b/phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php @@ -0,0 +1,59 @@ +assertSame( + $expected, + WP_Font_Family_Utils::format_font_family( + $font_family + ) + ); + } + + /** + * Data provider. + * + * @return array[] + */ + public function data_should_format_font_family() { + return array( + 'data_families_with_spaces_and_numbers' => array( + 'font_family' => 'Rock 3D , Open Sans,serif', + 'expected' => "'Rock 3D', 'Open Sans', serif", + ), + 'data_single_font_family' => array( + 'font_family' => 'Rock 3D', + 'expected' => "'Rock 3D'", + ), + 'data_no_spaces' => array( + 'font_family' => 'Rock3D', + 'expected' => 'Rock3D', + ), + 'data_many_spaces_and_existing_quotes' => array( + 'font_family' => 'Rock 3D serif, serif,sans-serif, "Open Sans"', + 'expected' => "'Rock 3D serif', serif, sans-serif, \"Open Sans\"", + ), + 'data_empty_family' => array( + 'font_family' => ' ', + 'expected' => '', + ), + ); + } +}