-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Block support: Add server-side processing for anchor #10655
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
base: trunk
Are you sure you want to change the base?
Block support: Add server-side processing for anchor #10655
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
| * | ||
| * @param WP_Block_Type $block_type Block Type. | ||
| */ | ||
| function wp_register_anchor_support( $block_type ) { |
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.
It doesn't seem this function has any test coverage?
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.
Tests added in 8fee9e1.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
…unction Co-authored-by: Weston Ruter <westonruter@gmail.com>
…unction Co-authored-by: Weston Ruter <westonruter@gmail.com>
Co-authored-by: Weston Ruter <westonruter@gmail.com>
westonruter
left a comment
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.
This looks good to me. A couple other tiny nits/suggestions which aren't necessary for merge.
| return; | ||
| } | ||
|
|
||
| if ( ! $block_type->attributes ) { |
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.
Since the value can only be null or array, this would seem more precise (although I know other block supports are doing this as well):
| if ( ! $block_type->attributes ) { | |
| if ( ! isset( $block_type->attributes ) ) { |
This is slightly better from a type perspective, because with PHPStan's strict rules, specifically booleansInConditions, using anything other than a bool in the condition is flagged as an issue. Granted, WordPress core does this all over the place, but maybe we can start moving away from it with Core-64238 and the like.
| if ( ! isset( $block_attributes['anchor'] ) || ! is_string( $block_attributes['anchor'] ) ) { | ||
| return array(); | ||
| } | ||
|
|
||
| if ( '' === $block_attributes['anchor'] ) { | ||
| return array(); | ||
| } |
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.
Could be combined. Not critical.
| if ( ! isset( $block_attributes['anchor'] ) || ! is_string( $block_attributes['anchor'] ) ) { | |
| return array(); | |
| } | |
| if ( '' === $block_attributes['anchor'] ) { | |
| return array(); | |
| } | |
| if ( ! isset( $block_attributes['anchor'] ) || ! is_string( $block_attributes['anchor'] ) || '' === $block_attributes['anchor'] ) { | |
| return array(); | |
| } |
Trac ticket: https://core.trac.wordpress.org/ticket/64449
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.