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

Font Library: fix fonts not displaying correctly #55393

Merged
merged 18 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export default function getIntersectingFontFaces( incoming, existing ) {
} );
}
);
matches.push( { ...existingFont, fontFace: matchingFaces } );
matches.push( { ...incomingFont, fontFace: matchingFaces } );
} else {
matches.push( incomingFont );
}
Expand Down
Original file line number Diff line number Diff line change
@@ -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 ) ) {
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: [
Expand All @@ -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: [
Expand All @@ -63,15 +63,15 @@ describe( 'getIntersectingFontFaces', () => {
];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);

expect( result ).toEqual( [] );
} );

it( 'returns matching font faces', () => {
const intendedFontsFamilies = [
const incomingFontFamilies = [
{
slug: 'lobster',
fontFace: [
Expand Down Expand Up @@ -129,15 +129,15 @@ describe( 'getIntersectingFontFaces', () => {
];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);

expect( result ).toEqual( expectedOutput );
} );

it( 'returns empty array when the first list is empty', () => {
const intendedFontsFamilies = [];
const incomingFontFamilies = [];

const existingFontFamilies = [
{
Expand All @@ -152,15 +152,15 @@ describe( 'getIntersectingFontFaces', () => {
];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);

expect( result ).toEqual( [] );
} );

it( 'returns empty array when the second list is empty', () => {
const intendedFontsFamilies = [
const incomingFontFamilies = [
{
slug: 'lobster',
fontFace: [
Expand All @@ -175,15 +175,15 @@ describe( 'getIntersectingFontFaces', () => {
const existingFontFamilies = [];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);

expect( result ).toEqual( [] );
} );

it( 'returns intersecting font family when there are no fonfaces', () => {
const intendedFontsFamilies = [
const incomingFontFamilies = [
{
slug: 'piazzolla',
fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
Expand All @@ -200,15 +200,15 @@ describe( 'getIntersectingFontFaces', () => {
];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);

expect( result ).toEqual( existingFontFamilies );
} );

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' } ],
Expand All @@ -226,7 +226,7 @@ describe( 'getIntersectingFontFaces', () => {
];

const result = getIntersectingFontFaces(
intendedFontsFamilies,
incomingFontFamilies,
existingFontFamilies
);
const expected = [
Expand All @@ -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 );
} );
} );
Original file line number Diff line number Diff line change
@@ -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 );
} );
} );
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php
/**
* Test WP_Font_Family_Utils::format_font_family().
*
* @package WordPress
* @subpackage Font Library
*
* @group fonts
* @group font-library
*
* @covers WP_Font_Family_Utils::format_font_family
*/
class Tests_Fonts_WpFontsFamilyUtils_FormatFontFamily extends WP_UnitTestCase {

/**
* @dataProvider data_should_format_font_family
*
* @param string $font_family Font family.
* @param string $expected Expected family.
*/
public function test_should_format_font_family( $font_family, $expected ) {
$this->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' => '',
),
);
}
}
Loading