-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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: Use data or src file to define font collection data #57734
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ phpunit/tests/fonts/font-library/wpFontCollection/getConfig.php ❔ lib/experimental/fonts/font-library/class-wp-font-collection.php ❔ lib/experimental/fonts/font-library/class-wp-rest-font-collections-controller.php ❔ lib/experimental/fonts/font-library/font-library.php ❔ phpunit/tests/fonts/font-library/wpFontCollection/__construct.php ❔ phpunit/tests/fonts/font-library/wpFontCollection/getConfigAndData.php |
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 added a commit to avoid returning the data when getting the config of a font collection and added tests to ensure that's not happening.
How to test?Add this to some PHP code (theme/plugin): $chivo = array(
'name' => 'Chivo',
'fontFamily' => 'Chivo, sans-serif',
'slug' => 'chivo',
'fontFace' => array(
array(
'src' => 'https://fonts.gstatic.com/s/chivo/v18/va9b4kzIxd1KFppkaRKvDRPJVDf_vB_ul2DSFXjQiQ.ttf',
'fontWeight' => '400',
'fontStyle' => 'normal',
'fontFamily' => 'Chivo',
'preview' => 'https://s.w.org/images/fonts/16.7/previews/chivo/chivo-400-normal.svg'
),
),
'preview' => 'https://s.w.org/images/fonts/16.7/previews/chivo/chivo.svg'
);
$texturina = array(
'name' => 'Texturina',
'fontFamily' => 'Texturina, serif',
'slug' => 'texturina',
'fontFace' => array(
array(
'src' => 'https://fonts.gstatic.com/s/texturina/v28/c4mM1nxpEtL3pXiAulRTkY-HGmNEX1b9NspjMwhAgliHhVrXy2eYGvUg25riW1OD.ttf',
'fontWeight' => '400',
'fontStyle' => 'normal',
'fontFamily' => 'Texturina',
'preview' => 'https://s.w.org/images/fonts/16.7/previews/texturina/texturina-400-normal.svg'
),
),
'preview' => 'https://s.w.org/images/fonts/16.7/previews/texturina/texturina.svg'
);
$ubuntu = array(
'name' => 'Ubuntu',
'fontFamily' => 'Ubuntu, sans-serif',
'slug' => 'ubuntu',
);
$verdana = array(
'name' => 'Verdana',
'fontFamily' => 'Verdana, sans-serif',
'slug' => 'verdana',
);
$collection_data = array(
'fontFamilies' => [ $chivo, $texturina, $ubuntu, $verdana ],
);
$default_font_collection = array(
'id' => 'my-custom-collection',
'name' => 'My Custom Collection',
'description' => __( 'Custom fonts collection' ),
'data' => $collection_data,
);
wp_register_font_collection( $default_font_collection ); You should get a new font collection in your font library modal: |
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.
☑️ Font collections are working with both JSON files and PHP arrays as data.
LGTM
What?
This allows a Font Collection to be set by passing Font Collection data instead of a src .json file.
Why?
So that collection details can by dynamically generated.
How?
Internally the details of the Font Collection are kept in a
data
property. They are stored there once the.json
file referenced insrc
are fetched. This change just allows thatdata
property to be passed instead of asrc
value.Testing Instructions
Unit tests are provided that demonstrates that the data passed in is the data that comes out. I'm not sure what other testing scenarios to suggest.