Skip to content
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.

Sign up for GitHub

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

Blocks: Introduce filter to allow easy addition of hooked blocks #5271

Closed
wants to merge 1 commit into from
Closed

Blocks: Introduce filter to allow easy addition of hooked blocks #5271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 67 additions & 44 deletions src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,20 +744,25 @@ function get_dynamic_block_names() {
*
* @since 6.4.0
*
* @param string $name Block type name including namespace.
* @param string $name Block type name including namespace.
* @param string $relative_position Optional. Relative position of the hooked block. Default empty string.
* @return array Associative array of `$block_type_name => $position` pairs.
*/
function get_hooked_blocks( $name ) {
function get_hooked_blocks( $name, $relative_position = '' ) {
$block_types = WP_Block_Type_Registry::get_instance()->get_all_registered();
$hooked_blocks = array();
foreach ( $block_types as $block_type ) {
if ( ! property_exists( $block_type, 'block_hooks' ) || ! is_array( $block_type->block_hooks ) ) {
continue;
}
foreach ( $block_type->block_hooks as $anchor_block_type => $relative_position ) {
if ( $anchor_block_type === $name ) {
$hooked_blocks[ $block_type->name ] = $relative_position;
foreach ( $block_type->block_hooks as $anchor_block_type => $position ) {
if ( $anchor_block_type !== $name ) {
continue;
}
if ( $relative_position && $relative_position !== $position ) {
continue;
}
$hooked_blocks[ $block_type->name ] = $position;
}
}
return $hooked_blocks;
Expand Down Expand Up @@ -797,34 +802,48 @@ function make_before_block_visitor( $context ) {

if ( $parent && ! $prev ) {
// Candidate for first-child insertion.
$hooked_blocks_for_parent = get_hooked_blocks( $parent['blockName'] );
foreach ( $hooked_blocks_for_parent as $hooked_block_type => $relative_position ) {
if ( 'first_child' === $relative_position ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $parent, $context );
}
$relative_position = 'first_child';
$anchor_block_type = $parent['blockName'];
$hooked_block_types = array_keys( get_hooked_blocks( $anchor_block_type, $relative_position ) );
/**
* Filters the list of hooked block types for a given anchor block type and relative position.
*
* @since 6.4.0
*
* @param string[] $hooked_block_types The list of hooked block types.
* @param string $relative_position The relative position of the hooked blocks.
* Can be one of 'before', 'after', 'first_child', or 'last_child'.
* @param string $anchor_block_type The anchor block type.
* @param WP_Block_Template|array $context The block template, template part, or pattern that the anchor block belongs to.
*/
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we call the filter just hooked_blocks?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like hooked_block_types personally, but defer to you all. It makes it clear to me that you are filtering the "block types" and not the entire block markup or something else.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with Nick here although it does make me wonder how consistent the codebase is in differentiating between "blocks" and "block types".

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how consistent the codebase is in differentiating between "blocks" and "block types".

😬 I support any effort to draw those distinctions where they haven't already been blurred.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hooked_block_types sounds good 👍🏻

foreach ( $hooked_block_types as $hooked_block_type ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $parent, $context );
}
}

$hooked_blocks = get_hooked_blocks( $block['blockName'] );
foreach ( $hooked_blocks as $hooked_block_type => $relative_position ) {
if ( 'before' === $relative_position ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/**
* Filters the serialized markup of a hooked block.
*
* @since 6.4.0
*
* @param string $hooked_block_markup The serialized markup of the hooked block.
* @param string $hooked_block_type The type of the hooked block.
* @param string $relative_position The relative position of the hooked block.
* Can be one of 'before', 'after', 'first_child', or 'last_child'.
* @param array $block The anchor block.
* @param WP_Block_Template|array $context The block template, template part, or pattern that the anchor block belongs to.
*/
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $block, $context );
}
$relative_position = 'before';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor, but I wonder if the various $relative_position values should be constants (something like WP_Block_Hooks::RELATIVE_POSITION_BEFORE, WP_Block_Hooks::RELATIVE_POSITION_FIRST_CHILD ...etc). This could reduce some variable declaration and make it more straightforward/clear for extension use.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, good point. I think it hadn't occurred to me since we need to use the position values verbatim in block.json -- but it might make sense to add constants in the code 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we update the block.json schema so that these values can be drawn from an auto-completed list with inline documentation?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we have in the schema:

https://github.com/WordPress/gutenberg/blob/aa938166f28bcfff6c7b8d1caac5a95f78852f97/schemas/json/block.json#L686-L695

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Speaking of schema, I opened #5278 to align the schemas between Gutenberg and WordPress core.

$anchor_block_type = $block['blockName'];
$hooked_block_types = array_keys( get_hooked_blocks( $anchor_block_type, $relative_position ) );
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/**
* Filters the serialized markup of a hooked block.
*
* @since 6.4.0
*
* @param string $hooked_block_markup The serialized markup of the hooked block.
* @param string $hooked_block_type The type of the hooked block.
* @param string $relative_position The relative position of the hooked block.
* Can be one of 'before', 'after', 'first_child', or 'last_child'.
* @param array $block The anchor block.
* @param WP_Block_Template|array $context The block template, template part, or pattern that the anchor block belongs to.
*/
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $block, $context );
Comment on lines +834 to +846
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gziolo We might consider removing the inject_hooked_block_markup filter in a follow-up; it might be a bit low-level (and less useful?) after all.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think a bit more about it, but at first glance, it looks like they are mostly the same.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inject_hooked_block_markup operates on the markup level, so it basically allows setting any string as the return value. The difference in the params available in the filter is that hooked_block_types uses the block name as the anchor, in contrast to the block type object that inject_hooked_block_markup offers for the parent or sibling block. I would be in favor of keeping only hooked_block_types which is a nice abstraction as it's just a list of block names.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. We can either land this PR and remove inject_hooked_block_markup in a follow-up, or do it as part of this PR.

}

return $markup;
Expand Down Expand Up @@ -860,24 +879,28 @@ function make_after_block_visitor( $context ) {
return function( &$block, $parent = null, $next = null ) use ( $context ) {
$markup = '';

$hooked_blocks = get_hooked_blocks( $block['blockName'] );
foreach ( $hooked_blocks as $hooked_block_type => $relative_position ) {
if ( 'after' === $relative_position ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $block, $context );
}
$relative_position = 'after';
$anchor_block_type = $block['blockName'];
$hooked_block_types = array_keys( get_hooked_blocks( $anchor_block_type, $relative_position ) );
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $block, $context );
}

if ( $parent && ! $next ) {
// Candidate for last-child insertion.
$hooked_blocks_for_parent = get_hooked_blocks( $parent['blockName'] );
foreach ( $hooked_blocks_for_parent as $hooked_block_type => $relative_position ) {
if ( 'last_child' === $relative_position ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $parent, $context );
}
$relative_position = 'last_child';
$anchor_block_type = $parent['blockName'];
$hooked_block_types = array_keys( get_hooked_blocks( $anchor_block_type, $relative_position ) );
/** This filter is documented in wp-includes/blocks.php */
$hooked_block_types = apply_filters( 'hooked_block_types', $hooked_block_types, $relative_position, $anchor_block_type, $context );
foreach ( $hooked_block_types as $hooked_block_type ) {
$hooked_block_markup = get_comment_delimited_block_content( $hooked_block_type, array(), '' );
/** This filter is documented in wp-includes/blocks.php */
$markup .= apply_filters( 'inject_hooked_block_markup', $hooked_block_markup, $hooked_block_type, $relative_position, $parent, $context );
}
}

Expand Down
29 changes: 23 additions & 6 deletions tests/phpunit/tests/blocks/blockHooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,19 +49,21 @@ public function test_get_hooked_blocks_matches_found() {
'tests/injected-one',
array(
'block_hooks' => array(
'tests/hooked-at-before' => 'before',
'tests/hooked-at-after' => 'after',
'tests/hooked-at-before' => 'before',
'tests/hooked-at-after' => 'after',
'tests/hooked-at-before-and-after' => 'before',
),
)
);
register_block_type(
'tests/injected-two',
array(
'block_hooks' => array(
'tests/hooked-at-before' => 'before',
'tests/hooked-at-after' => 'after',
'tests/hooked-at-first-child' => 'first_child',
'tests/hooked-at-last-child' => 'last_child',
'tests/hooked-at-before' => 'before',
'tests/hooked-at-after' => 'after',
'tests/hooked-at-before-and-after' => 'after',
'tests/hooked-at-first-child' => 'first_child',
'tests/hooked-at-last-child' => 'last_child',
),
)
);
Expand Down Expand Up @@ -96,5 +98,20 @@ public function test_get_hooked_blocks_matches_found() {
get_hooked_blocks( 'tests/hooked-at-last-child' ),
'block hooked at the last child position'
);
$this->assertSame(
array(
'tests/injected-one' => 'before',
'tests/injected-two' => 'after',
),
get_hooked_blocks( 'tests/hooked-at-before-and-after' ),
'block hooked before one block and after another'
);
$this->assertSame(
array(
'tests/injected-one' => 'before',
),
get_hooked_blocks( 'tests/hooked-at-before-and-after', 'before' ),
'block hooked before one block and after another, filtered for before'
);
}
}