Skip to content

Commit

Permalink
Use slug instead of id for Font Collection (#57735)
Browse files Browse the repository at this point in the history
* Add ability to use data rather than a file to define font collection

* Use slug instead of id for Font Collections

* format php

* merge file changes

* format php

---------

Co-authored-by: Matias Benedetto <matias.benedetto@gmail.com>
  • Loading branch information
pbking and matiasbenedetto authored Jan 11, 2024
1 parent 2762ec4 commit 8599e15
Show file tree
Hide file tree
Showing 14 changed files with 63 additions and 63 deletions.
10 changes: 5 additions & 5 deletions lib/experimental/fonts/font-library/class-wp-font-collection.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ public function __construct( $config ) {
throw new Exception( 'Font Collection config options is required as a non-empty array.' );
}

if ( empty( $config['id'] ) || ! is_string( $config['id'] ) ) {
throw new Exception( 'Font Collection config ID is required as a non-empty string.' );
if ( empty( $config['slug'] ) || ! is_string( $config['slug'] ) ) {
throw new Exception( 'Font Collection config slug is required as a non-empty string.' );
}

if ( empty( $config['name'] ) || ! is_string( $config['name'] ) ) {
Expand All @@ -66,14 +66,14 @@ public function __construct( $config ) {
* @return array {
* An array of font collection config.
*
* @type string $id The font collection's unique ID.
* @type string $slug The font collection's unique slug.
* @type string $name The font collection's name.
* @type string $description The font collection's description.
* }
*/
public function get_config() {
return array(
'id' => $this->config['id'],
'slug' => $this->config['slug'],
'name' => $this->config['name'],
'description' => $this->config['description'] ?? '',
);
Expand All @@ -90,7 +90,7 @@ public function get_config() {
* @return array {
* An array of font collection config and data.
*
* @type string $id The font collection's unique ID.
* @type string $slug The font collection's unique ID.
* @type string $name The font collection's name.
* @type string $description The font collection's description.
* @type array $data The font collection's data as a PHP array.
Expand Down
36 changes: 18 additions & 18 deletions lib/experimental/fonts/font-library/class-wp-font-library.php
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ public static function get_expected_font_mime_types_per_php_version( $php_versio
*/
public static function register_font_collection( $config ) {
$new_collection = new WP_Font_Collection( $config );
if ( self::is_collection_registered( $config['id'] ) ) {
if ( self::is_collection_registered( $config['slug'] ) ) {
$error_message = sprintf(
/* translators: %s: Font collection id. */
__( 'Font collection with id: "%s" is already registered.', 'default' ),
$config['id']
/* translators: %s: Font collection slug. */
__( 'Font collection with slug: "%s" is already registered.', 'default' ),
$config['slug']
);
_doing_it_wrong(
__METHOD__,
Expand All @@ -76,7 +76,7 @@ public static function register_font_collection( $config ) {
);
return new WP_Error( 'font_collection_registration_error', $error_message );
}
self::$collections[ $config['id'] ] = $new_collection;
self::$collections[ $config['slug'] ] = $new_collection;
return $new_collection;
}

Expand All @@ -85,20 +85,20 @@ public static function register_font_collection( $config ) {
*
* @since 6.5.0
*
* @param string $collection_id Font collection ID.
* @param string $collection_slug Font collection slug.
* @return bool True if the font collection was unregistered successfully and false otherwise.
*/
public static function unregister_font_collection( $collection_id ) {
if ( ! self::is_collection_registered( $collection_id ) ) {
public static function unregister_font_collection( $slug ) {
if ( ! self::is_collection_registered( $slug ) ) {
_doing_it_wrong(
__METHOD__,
/* translators: %s: Font collection id. */
sprintf( __( 'Font collection "%s" not found.', 'default' ), $collection_id ),
/* translators: %s: Font collection slug. */
sprintf( __( 'Font collection "%s" not found.', 'default' ), $slug ),
'6.5.0'
);
return false;
}
unset( self::$collections[ $collection_id ] );
unset( self::$collections[ $slug ] );
return true;
}

Expand All @@ -107,11 +107,11 @@ public static function unregister_font_collection( $collection_id ) {
*
* @since 6.5.0
*
* @param string $collection_id Font collection ID.
* @param string $slug Font collection slug.
* @return bool True if the font collection is registered and false otherwise.
*/
private static function is_collection_registered( $collection_id ) {
return array_key_exists( $collection_id, self::$collections );
private static function is_collection_registered( $slug ) {
return array_key_exists( $slug, self::$collections );
}

/**
Expand All @@ -130,12 +130,12 @@ public static function get_font_collections() {
*
* @since 6.5.0
*
* @param string $id Font collection id.
* @param string $slug Font collection slug.
* @return array List of font collections.
*/
public static function get_font_collection( $id ) {
if ( array_key_exists( $id, self::$collections ) ) {
return self::$collections[ $id ];
public static function get_font_collection( $slug ) {
if ( array_key_exists( $slug, self::$collections ) ) {
return self::$collections[ $slug ];
}
return new WP_Error( 'font_collection_not_found', 'Font collection not found.' );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function register_routes() {

register_rest_route(
$this->namespace,
'/' . $this->rest_base . '/(?P<id>[\/\w-]+)',
'/' . $this->rest_base . '/(?P<slug>[\/\w-]+)',
array(
array(
'methods' => WP_REST_Server::READABLE,
Expand All @@ -70,8 +70,8 @@ public function register_routes() {
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
*/
public function get_font_collection( $request ) {
$id = $request->get_param( 'id' );
$collection = WP_Font_Library::get_font_collection( $id );
$slug = $request->get_param( 'slug' );
$collection = WP_Font_Library::get_font_collection( $slug );
// If the collection doesn't exist returns a 404.
if ( is_wp_error( $collection ) ) {
$collection->add_data( array( 'status' => 404 ) );
Expand Down
2 changes: 1 addition & 1 deletion lib/experimental/fonts/font-library/font-library.php
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function wp_unregister_font_collection( $collection_id ) {
}

$default_font_collection = array(
'id' => 'default-font-collection',
'slug' => 'default-font-collection',
'name' => 'Google Fonts',
'description' => __( 'Add from Google Fonts. Fonts are copied to and served from your site.', 'gutenberg' ),
'src' => 'https://s.w.org/images/fonts/16.7/collections/google-fonts-with-preview.json',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public function test_should_initialize_data() {
$property->setAccessible( true );

$config = array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'src' => 'my-collection-data.json',
Expand Down Expand Up @@ -55,7 +55,7 @@ public function data_should_throw_exception() {
'description' => 'My collection description',
'src' => 'my-collection-data.json',
),
'Font Collection config ID is required as a non-empty string.',
'Font Collection config slug is required as a non-empty string.',
),

'no config' => array(
Expand All @@ -80,7 +80,7 @@ public function data_should_throw_exception() {

'missing src' => array(
array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
),
Expand Down
12 changes: 6 additions & 6 deletions phpunit/tests/fonts/font-library/wpFontCollection/getConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,39 +34,39 @@ public function data_should_get_config() {
return array(
'with a file' => array(
'config' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'src' => $mock_file,
),
'expected_data' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
),
),
'with a url' => array(
'config' => array(
'id' => 'my-collection-with-url',
'slug' => 'my-collection-with-url',
'name' => 'My Collection with URL',
'description' => 'My collection description',
'src' => 'https://localhost/fonts/mock-font-collection.json',
),
'expected_data' => array(
'id' => 'my-collection-with-url',
'slug' => 'my-collection-with-url',
'name' => 'My Collection with URL',
'description' => 'My collection description',
),
),
'with data' => array(
'config' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'data' => array( 'this is mock data' => true ),
),
'expected_data' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,27 +69,27 @@ public function data_should_get_config_and_data() {
return array(
'with a file' => array(
'config' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'src' => $mock_file,
),
'expected_data' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'data' => array( 'this is mock data' => true ),
),
),
'with a url' => array(
'config' => array(
'id' => 'my-collection-with-url',
'slug' => 'my-collection-with-url',
'name' => 'My Collection with URL',
'description' => 'My collection description',
'src' => 'https://localhost/fonts/mock-font-collection.json',
),
'expected_data' => array(
'id' => 'my-collection-with-url',
'slug' => 'my-collection-with-url',
'name' => 'My Collection with URL',
'description' => 'My collection description',
'data' => array(
Expand All @@ -100,13 +100,13 @@ public function data_should_get_config_and_data() {
),
'with data' => array(
'config' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'data' => array( 'this is mock data' => true ),
),
'expected_data' => array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My collection description',
'data' => array( 'this is mock data' => true ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Tests_Fonts_WpFontLibrary_GetFontCollection extends WP_Font_Library_UnitTe

public function test_should_get_font_collection() {
$my_font_collection_config = array(
'id' => 'my-font-collection',
'slug' => 'my-font-collection',
'name' => 'My Font Collection',
'description' => 'Demo about how to a font collection to your WordPress Font Library.',
'src' => path_join( __DIR__, 'my-font-collection-data.json' ),
Expand All @@ -24,7 +24,7 @@ public function test_should_get_font_collection() {
$this->assertInstanceOf( 'WP_Font_Collection', $font_collection );
}

public function test_should_get_no_font_collection_if_the_id_is_not_registered() {
public function test_should_get_no_font_collection_if_the_slug_is_not_registered() {
$font_collection = WP_Font_Library::get_font_collection( 'not-registered-font-collection' );
$this->assertWPError( $font_collection );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ public function test_should_get_an_empty_list() {

public function test_should_get_mock_font_collection() {
$my_font_collection_config = array(
'id' => 'my-font-collection',
'slug' => 'my-font-collection',
'name' => 'My Font Collection',
'description' => 'Demo about how to a font collection to your WordPress Font Library.',
'src' => path_join( __DIR__, 'my-font-collection-data.json' ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Tests_Fonts_WpFontLibrary_RegisterFontCollection extends WP_Font_Library_U

public function test_should_register_font_collection() {
$config = array(
'id' => 'my-collection',
'slug' => 'my-collection',
'name' => 'My Collection',
'description' => 'My Collection Description',
'src' => 'my-collection-data.json',
Expand All @@ -23,20 +23,20 @@ public function test_should_register_font_collection() {
$this->assertInstanceOf( 'WP_Font_Collection', $collection );
}

public function test_should_return_error_if_id_is_missing() {
public function test_should_return_error_if_slug_is_missing() {
$config = array(
'name' => 'My Collection',
'description' => 'My Collection Description',
'src' => 'my-collection-data.json',
);
$this->expectException( 'Exception' );
$this->expectExceptionMessage( 'Font Collection config ID is required as a non-empty string.' );
$this->expectExceptionMessage( 'Font Collection config slug is required as a non-empty string.' );
WP_Font_Library::register_font_collection( $config );
}

public function test_should_return_error_if_name_is_missing() {
$config = array(
'id' => 'my-collection',
'slug' => 'my-collection',
'description' => 'My Collection Description',
'src' => 'my-collection-data.json',
);
Expand All @@ -52,15 +52,15 @@ public function test_should_return_error_if_config_is_empty() {
WP_Font_Library::register_font_collection( $config );
}

public function test_should_return_error_if_id_is_repeated() {
public function test_should_return_error_if_slug_is_repeated() {
$config1 = array(
'id' => 'my-collection-1',
'slug' => 'my-collection-1',
'name' => 'My Collection 1',
'description' => 'My Collection 1 Description',
'src' => 'my-collection-1-data.json',
);
$config2 = array(
'id' => 'my-collection-1',
'slug' => 'my-collection-1',
'name' => 'My Collection 2',
'description' => 'My Collection 2 Description',
'src' => 'my-collection-2-data.json',
Expand All @@ -72,7 +72,7 @@ public function test_should_return_error_if_id_is_repeated() {

// Expects a _doing_it_wrong notice.
$this->setExpectedIncorrectUsage( 'WP_Font_Library::register_font_collection' );
// Try to register a second collection with same id.
// Try to register a second collection with same slug.
$collection2 = WP_Font_Library::register_font_collection( $config2 );
$this->assertWPError( $collection2, 'A WP_Error should be returned.' );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ class Tests_Fonts_WpFontLibrary_UnregisterFontCollection extends WP_Font_Library
public function test_should_unregister_font_collection() {
// Registers two mock font collections.
$config = array(
'id' => 'mock-font-collection-1',
'slug' => 'mock-font-collection-1',
'name' => 'Mock Collection to be unregistered',
'description' => 'A mock font collection to be unregistered.',
'src' => 'my-collection-data.json',
);
WP_Font_Library::register_font_collection( $config );

$config = array(
'id' => 'mock-font-collection-2',
'slug' => 'mock-font-collection-2',
'name' => 'Mock Collection',
'description' => 'A mock font collection.',
'src' => 'my-mock-data.json',
Expand Down
Loading

0 comments on commit 8599e15

Please sign in to comment.