Font Library #5285
Font Library #5285
Conversation
src/wp-includes/fonts/font-library/class-wp-rest-font-library-controller.php
Outdated
Show resolved
Hide resolved
This reverts commit d4bce59.
| 'callback' => array( $this, 'get_font_collections' ), | ||
| 'permission_callback' => array( $this, 'update_font_library_permissions_check' ), | ||
| ), | ||
| 'schema' => array( $this, 'get_items_schema' ), |
There was a problem hiding this comment.
There idea of schema, is the desciptions an item in the response in the request. The shape of the repsonse should match between items and item endpoints. Items should just be a array of item. That way you can reuse the same prepare_items_for_response. See this example.
| $collections[] = $this->prepare_item_for_response( $collection->get_config(), null ); | ||
| } | ||
|
|
||
| return new WP_REST_Response( $collections, 200 ); |
There was a problem hiding this comment.
| $collections[] = $this->prepare_item_for_response( $collection->get_config(), null ); | |
| } | |
| return new WP_REST_Response( $collections, 200 ); | |
| $font_collection = $this->prepare_item_for_response( $collection->get_config(), null ); | |
| $collections[] = $this->prepare_response_for_collection( $font_collection ); | |
| } | |
| return rest_ensure_response( $collections ); |
There was a problem hiding this comment.
| 'context' => array( 'view', 'edit', 'embed' ), | ||
| ), | ||
| 'data' => array( | ||
| 'description' => __( 'Data of the font collection.' ), |
There was a problem hiding this comment.
This seems like it is embedded font family object no? This should use links to link to font object.
https://developer.wordpress.org/rest-api/using-the-rest-api/linking-and-embedding/
|
@matiasbenedetto My review was requested. Please ensure before requesting my reviewing the following things are complete, before I review.
|
| * | ||
| * @since 6.4.0 | ||
| */ | ||
| class WP_REST_Font_Families_Controller extends WP_REST_Controller { |
There was a problem hiding this comment.
@matiasbenedetto Sorry if I am missing this, but why is this a custom controller? Doesn't this map to the post contoller for font CPT?
| function wp_register_font_collection( $config ) { | ||
| return WP_Font_Library::register_font_collection( $config ); | ||
| } | ||
|
|
There was a problem hiding this comment.
how do a unregister a font collection.
There was a problem hiding this comment.
That's not currently possible but that work is already started: WordPress/gutenberg#54701
There was a problem hiding this comment.
| public function update_font_library_permissions_check() { | ||
| if ( ! current_user_can( 'edit_theme_options' ) ) { | ||
| return new WP_Error( | ||
| 'rest_cannot_update_font_library', | ||
| __( 'Sorry, you are not allowed to update the Font Library on this site.' ), | ||
| array( | ||
| 'status' => rest_authorization_required_code(), | ||
| ) | ||
| ); | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Can we not use a meta capabilities for the font post type instead of just using edit_theme_options? Example
https://core.trac.wordpress.org/ticket/54516
There was a problem hiding this comment.
This received detailed discussion over on WordPress/gutenberg#55280.
|
Sharing @spacedmonkey observations from Make/Core slack in the
Some links to help:
See WordPress/gutenberg#54697 to add
|
…est_prepare_item.
|
Closing this PR due to the inclusion of this feature was punted to 6.5: |
|
Tracking the required changes to make Font Library ready for 6.5 in this Gutenberg issue: WordPress/gutenberg#54169 |
Trac ticket: https://core.trac.wordpress.org/ticket/59166
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.