Repository navigation
Icons: Allow icons to ship to WordPress Core without exposing them in the Icon block #82634
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
Changes from all commits
fb610a2
beed60f
1fa4107
1aab75b
a2cdadc
f19b044
9bcbb52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| https://github.com/WordPress/wordpress-develop/pull/13453 | ||
|
|
||
| * https://github.com/WordPress/gutenberg/pull/82634 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -116,6 +116,9 @@ public function get_items( $request ) { | |
| $icons = WP_Icons_Registry::get_instance()->get_registered_icons( $search ); | ||
|
|
||
| foreach ( $icons as $icon ) { | ||
| if ( false === ( $icon['public'] ?? true ) ) { | ||
| continue; | ||
| } | ||
| if ( null !== $collection && ( ! isset( $icon['collection'] ) || $icon['collection'] !== $collection ) ) { | ||
| continue; | ||
| } | ||
|
|
@@ -126,6 +129,33 @@ public function get_items( $request ) { | |
| return rest_ensure_response( $response ); | ||
| } | ||
|
|
||
| /** | ||
| * Retrieves a specific icon from the registry. | ||
| * | ||
| * Icons registered as non-public are reported as not found. Icons registered | ||
| * without an explicit `public` property are public. | ||
| * | ||
| * @param string $name Icon name. | ||
| * @return array|WP_Error Icon data on success, or WP_Error object on failure. | ||
| */ | ||
| public function get_icon( $name ) { | ||
| $icon = parent::get_icon( $name ); | ||
|
|
||
| if ( ! is_wp_error( $icon ) && false === ( $icon['public'] ?? true ) ) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Icon block loads the icons through this endpoint in the editor, but uses
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh.... this is a good point. Hmm. I'd vote to also skip rendering the block on frontend: 1aab75b. What do you think?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to be consistent, so not rendering the block might be best. I think that's a fine decision to make after landing, in another PR.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. OK, I removed it from this PR from now in the latest commit, will raise another later!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extracted here: #82774 |
||
| return new WP_Error( | ||
| 'rest_icon_not_found', | ||
| sprintf( | ||
| // translators: %s is the name of any user-provided name | ||
| __( 'Icon not found: "%s".', 'gutenberg' ), | ||
| $name | ||
| ), | ||
| array( 'status' => 404 ) | ||
| ); | ||
| } | ||
|
|
||
| return $icon; | ||
| } | ||
|
|
||
| /** | ||
| * Prepare a raw icon before it gets output in a REST API response. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,7 +17,7 @@ public function set_up() { | |
| if ( ! WP_Icon_Collections_Registry::get_instance()->is_registered( 'core' ) ) { | ||
| gutenberg_register_default_icon_collections(); | ||
| } | ||
| if ( empty( WP_Icons_Registry::get_instance()->get_registered_icons() ) ) { | ||
| if ( empty( WP_Icons_Registry_Gutenberg::get_instance()->get_registered_icons() ) ) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we make the same change in
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes sense, this is also done via latest commit: 1aab75b |
||
| gutenberg_register_default_icons(); | ||
| } | ||
| } | ||
|
|
@@ -52,7 +52,8 @@ public function test_wp_get_icon_custom_size() { | |
|
|
||
| public function test_wp_get_icon_size_null_leaves_dimensions_untouched() { | ||
| $output = wp_get_icon( 'core/plus', array( 'size' => null ) ); | ||
| $this->assertStringNotContainsString( 'width=', $output ); | ||
| // Leading space to avoid matching `stroke-width`. | ||
| $this->assertStringNotContainsString( ' width=', $output ); | ||
| $this->assertStringNotContainsString( 'height=', $output ); | ||
| } | ||
|
|
||
|
|
||
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.
Do we need better validation here? A "public": "false" typo would keep the SVG in the core zip .
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.
Makes sense. Validating here instead: 1fa4107