Add Block Hooks (a.k.a. Auto-inserting Blocks) #5158
Conversation
ockham
force-pushed
the
add/block-hooks
branch
from
017929d
to
f7465b9
Compare
| /** | ||
| * Injects the active theme's stylesheet as a `theme` attribute | ||
| * into a given template part block. | ||
| * | ||
| * @since 6.4.0 | ||
| * @access private | ||
| * | ||
| * @param array $block a parsed block. | ||
| * @return array Updated block. | ||
| */ | ||
| function _inject_theme_attribute_in_template_part_block( $block ) { | ||
| if ( | ||
| 'core/template-part' === $block['blockName'] && | ||
| ! isset( $block['attrs']['theme'] ) | ||
| ) { | ||
| $block['attrs']['theme'] = get_stylesheet(); | ||
| } | ||
| return $block; | ||
| } | ||
|
|
There was a problem hiding this comment.
The idea is to change this to a more general function (e.g. called _visit_block) that is called for each block during serialization (see below).
We can then extend that function to also take care of automatic insertion:
diff --git a/src/wp-includes/block-template-utils.php b/src/wp-includes/block-template-utils.php
index 460372b8cc..97126341d5 100644
--- a/src/wp-includes/block-template-utils.php
+++ b/src/wp-includes/block-template-utils.php
@@ -523,13 +523,15 @@ function _inject_theme_attribute_in_block_template_content( $template_content )
* @param array $block a parsed block.
* @return array Updated block.
*/
-function _inject_theme_attribute_in_template_part_block( $block ) {
+function _visit_parsed_block( $block ) {
if (
'core/template-part' === $block['blockName'] &&
! isset( $block['attrs']['theme'] )
) {
$block['attrs']['theme'] = get_stylesheet();
}
+
+ $block = insert_hooked_blocks( $block );
return $block;
}Pending implementation insert_hooked_blocks (and adapting the WP_Block_Type class and friends to include a new $block_hooks field etc), this should give us parity with what we're currently doing in Gutenberg through some filter magic.
We can then introduce a way to limit automatic insertion to certain template types; off-the-cuff, I'd say this should be possible via binding inside of _build_block_template_result_from_file (but maybe we can make it more elegant).
diff --git a/src/wp-includes/block-template-utils.php b/src/wp-includes/block-template-utils.php
index 460372b8cc..88fedf939a 100644
--- a/src/wp-includes/block-template-utils.php
+++ b/src/wp-includes/block-template-utils.php
@@ -514,23 +514,31 @@ function _inject_theme_attribute_in_block_template_content( $template_content )
}
/**
- * Injects the active theme's stylesheet as a `theme` attribute
- * into a given template part block.
+ * Returns a function that...
*
* @since 6.4.0
* @access private
*
- * @param array $block a parsed block.
- * @return array Updated block.
+ * @param WP_Block_Template $block_template a block template.
+ * @return callable A function that returns a block.
*/
-function _inject_theme_attribute_in_template_part_block( $block ) {
- if (
- 'core/template-part' === $block['blockName'] &&
- ! isset( $block['attrs']['theme'] )
- ) {
- $block['attrs']['theme'] = get_stylesheet();
+function _parsed_block_visitor( $block_template ) {
+ return function( $block ) use ( $block_template ) {
+ if (
+ 'core/template-part' === $block['blockName'] &&
+ ! isset( $block['attrs']['theme'] )
+ ) {
+ $block['attrs']['theme'] = get_stylesheet();
+ }
+
+ $original_block = $block;
+ $block = insert_hooked_blocks( $block );
+ /**
+ * This filter allows modifiying the auto-inserting behavior...
+ */
+ $block = apply_filters( 'auto_insert_blocks', $block, $original_block, $block_template );
+ return $block;
}
- return $block;
}
/**
@@ -609,7 +617,8 @@ function _build_block_template_result_from_file( $template_file, $template_type
}
$blocks = parse_blocks( $template_content );
- $template->content = serialize_blocks( $blocks, '_inject_theme_attribute_in_template_part_block' );
+ $visitor = _parsed_block_visitor( $template );
+ $template->content = serialize_blocks( $blocks, $visitor );
return $template;
}This should allow us to preserve separation of concerns (e.g. serialize_blocks doesn't know about auto-inserted blocks or templates; insert_block_hooks doesn't know about templates).
|
Note the current state of the PR: We're adding an optional argument to |
|
@gziolo See my inline comments for an explanation how I envision a filter to limit auto-insertion to certain template types to work 😊 |
|
Spoke too soon: This PR currently breaks the frontend 😕
Which means that we actually also need better unit test coverage that |
Fixed in 7fda6f5. (We should still improve test coverage.) |
|
@gziolo I think this code is almost working. I believe the reason why I don't see the Like button inserted as the Comment Template block's last child in the Twenty Twenty Three theme is that we're not yet inserting hooked blocks into block patterns (see). Other than that, note that sibling insertion isn't working yet (see diff). |
There was a problem hiding this comment.
Hey @ockham! I know this PR is still in draft. I'm just leaving some nitpick feedback early in case I miss a chance to review this one later, and to avoid additional instances of these being introduced later in the PR's development. 🙂
Feel free to ping me at any stage if I can help to review the implementation, tests, docs, etc. 🙂
ockham
force-pushed
the
add/block-hooks
branch
from
7af49ce
to
600e55d
Compare
| $blocks = parse_blocks( $pattern['content'] ); | ||
| $visitor = _parsed_block_visitor( $pattern ); // TODO: Should we use different functions for template vs pattern? | ||
| $patterns[ $index ]['content'] = serialize_blocks( $blocks, $visitor ); | ||
| } |
There was a problem hiding this comment.
TODO: Reuse logic from get_registered.
| $pattern = $this->registered_patterns[ $pattern_name ]; | ||
| $blocks = parse_blocks( $pattern['content'] ); | ||
| $visitor = _parsed_block_visitor( $pattern ); // TODO: Should we use different functions for template vs pattern? | ||
| $pattern['content'] = serialize_blocks( $blocks, $visitor ); |
There was a problem hiding this comment.
TODO (unrelated to Block Hooks): We probably want some kind of filter here to allow modifying the pattern content -- otherwise, there's no way for plugins (and GB) to modify patterns.
Thank you very much @costdev! Good to address these early one, before I break the PR down into smaller ones 😊 |
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
Co-authored-by: Colin Stewart <79332690+costdev@users.noreply.github.com>
ockham
force-pushed
the
add/block-hooks
branch
from
cab8fd6
to
5cd2ded
Compare
| if ( | ||
| 'core/template-part' === $block['blockName'] && | ||
| ! isset( $block['attrs']['theme'] ) | ||
| ) { | ||
| $block['attrs']['theme'] = get_stylesheet(); | ||
| } |
There was a problem hiding this comment.
| if ( | |
| 'core/template-part' === $block['blockName'] && | |
| ! isset( $block['attrs']['theme'] ) | |
| ) { | |
| $block['attrs']['theme'] = get_stylesheet(); | |
| } | |
| $block = _inject_theme_attribute_in_template_part_block( $block ); |
| /** | ||
| * This filter allows modifiying the auto-inserting behavior... | ||
| */ | ||
| $block = apply_filters( 'insert_hooked_block', $block, $hooked_blocks[ $hooked_block_type ], $block_template ); |
There was a problem hiding this comment.
Might use separate filters for template and pattern insertion
| $block = apply_filters( 'insert_hooked_block', $block, $hooked_blocks[ $hooked_block_type ], $block_template ); | |
| $block = apply_filters( 'insert_hooked_block_into_template', $block, $hooked_blocks[ $hooked_block_type ], $block_template ); |
| return $this->registered_patterns[ $pattern_name ]; | ||
| $pattern = $this->registered_patterns[ $pattern_name ]; | ||
| $blocks = parse_blocks( $pattern['content'] ); | ||
| $visitor = _parsed_block_visitor( $pattern ); // TODO: Should we use different functions for template vs pattern? |
There was a problem hiding this comment.
Probably something like
| $visitor = _parsed_block_visitor( $pattern ); // TODO: Should we use different functions for template vs pattern? | |
| $visitor = _parsed_block_visitor_for_pattern( $pattern ); // TODO: Use nicer factory name! |
In that function, apply a insert_hooked_block_into_pattern filter (akin to https://github.com/WordPress/wordpress-develop/pull/5158/files#r1325968103).
| function get_hooked_blocks( $name ) { | ||
| $block_types = WP_Block_Type_Registry::get_instance()->get_all_registered(); | ||
| $hooked_blocks = array(); | ||
| foreach ( $block_types as $block_type ) { | ||
| foreach ( $block_type->block_hooks as $anchor_block_type => $relative_position ) { | ||
| if ( $anchor_block_type === $name ) { | ||
| $hooked_blocks[ $block_type->name ] = $relative_position; | ||
| } | ||
| } | ||
| } | ||
| return $hooked_blocks; | ||
| } |
There was a problem hiding this comment.
@gziolo and I discussed adding a cache for this to WP_Block_Type_Registry that is updated whenever a block is registered or unregistered, respectively.
(We might want to implement the cache in a separate class for better encapsulation and call the relevant class methods from WP_Block_Type_Registry's register() and unregister() methods.)
There was a problem hiding this comment.
First version is ready with #5239. Let's integrate it with the auto-insertion logic first, because it might impact how it's going to be optimized.
| $hooked_blocks = get_hooked_blocks( $block['blockName'] ); | ||
| foreach ( $hooked_blocks as $hooked_block_type => $relative_position ) { | ||
| $hooked_block = array( | ||
| 'blockName' => $hooked_block_type, | ||
| 'attrs' => array(), | ||
| 'innerHTML' => '', | ||
| 'innerContent' => array(), | ||
| 'innerBlocks' => array(), | ||
| ); | ||
| // Need to pass full current block, (possibly its parent), relative position, and hooked_block. | ||
| $block = insert_hooked_block( $hooked_block, $relative_position, $block ); |
There was a problem hiding this comment.
One way to implement sibling insertion could be by changing this code by treating the two cases separately (child vs sibling insertion) -- a bit more similar to what we're doing in GB:
$hooked_blocks = get_hooked_blocks( $block['blockName'] );
$sibling_hooked_blocks = array_filter( $hooked_blocks, ... ); // position is 'before' or 'after'
$child_hooked_blocks = array_filter( $hooked_blocks , ... ); // position is 'first_child' or 'last_child'
foreach ( $child_hooked_blocks as $hooked_block_type => $relative_position ) {
/* Same implementation as before */
}
foreach ( $sibling_hooked_blocks as $hooked_block_type => $relative_position ) {
$anchor_block_index = array_search( $hooked_block_type, array_column( $block['innerBlocks'], 'blockName' ), true );
if ( false !== $anchor_block_index && ( 'after' === $relative_position || 'before' === $relative_position ) )
{
/* See GB's gutenberg_insert_hooked_block */
}(Rough draft; we might want to move some of that logic into insert_hooked_block, if possible.)
There was a problem hiding this comment.
The crucial conceptual difference is that for child insertion, the anchor block is the one that needs to be modified (by inserting the first or last child into its inner blocks), whereas for sibling insertion, it isn't; instead, it is the anchor block's parent -- which is an additional piece of information that we need in that case.
There was a problem hiding this comment.
They are definitely different, the case for first_child and last_child is way simpler to integrate as it can be done on with the new callback param as that's mostly about applying modifications to the $block object in exactly the same way as in the Gutenberg plugin.
I still don't have a clear picture about the sibling insertion so whatever works, should be implemented initially. We can discuss further optimization later, but I can't tell based on the abstract code as it's a very complex use case. I'm still impressed how you managed to integrate it using only existing filters in the plugin.
In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type. Extracted from #5158 and covered with unit tests. Props ockham. Fixes #59383. git-svn-id: https://develop.svn.wordpress.org/trunk@56610 602fd350-edb4-49c9-b593-d223f7449a82
In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type. Extracted from WordPress/wordpress-develop#5158 and covered with unit tests. Props ockham. Fixes #59383. Built from https://develop.svn.wordpress.org/trunk@56610 git-svn-id: http://core.svn.wordpress.org/trunk@56122 1a063a9b-81f0-0310-95a4-ce76da25c4cd
In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type. Extracted from WordPress/wordpress-develop#5158 and covered with unit tests. Props ockham. Fixes #59383. Built from https://develop.svn.wordpress.org/trunk@56610 git-svn-id: https://core.svn.wordpress.org/trunk@56122 1a063a9b-81f0-0310-95a4-ce76da25c4cd
| if ( 'first_child' === $relative_position ) { | ||
| array_unshift( $anchor_block['innerBlocks'], $inserted_block ); | ||
| /* | ||
| * Since WP_Block::render() iterates over `inner_content` (rather than `inner_blocks`) | ||
| * when rendering blocks, we also need to prepend a value (`null`, to mark a block | ||
| * location) to that array. | ||
| */ | ||
| array_unshift( $anchor_block['innerContent'], null ); | ||
| } elseif ( 'last_child' === $relative_position ) { | ||
| array_push( $anchor_block['innerBlocks'], $inserted_block ); | ||
| // Since WP_Block::render() iterates over `inner_content` (rather than `inner_blocks`) | ||
| // when rendering blocks, we also need to prepend a value (`null`, to mark a block | ||
| // location) to that array. | ||
| array_push( $anchor_block['innerContent'], null ); | ||
| } |
There was a problem hiding this comment.
Need to update based on WordPress/gutenberg#54349!
In order to implement Block Hooks (see #59313), we added block_hooks field to the WP_Block_Type class, as well as to block registration related functions. In this follow-up, new helper function gets introduced that is going to compute the list of hooked blocks by other registered blocks for a given block type. Extracted from WordPress#5158 and covered with unit tests. Props ockham. Fixes #59383. git-svn-id: https://develop.svn.wordpress.org/trunk@56610 602fd350-edb4-49c9-b593-d223f7449a82
|
Superseded by #5261. |
Port the Block Hooks feature (formerly known as Auto-inserting Blocks) from Gutenberg to Core.
WIP. Might break this into separate PRs later.
Testing Instructions
block.jsonfield name set toblockHooksand will work with this PR out-of-the-box.Trac ticket: https://core.trac.wordpress.org/ticket/59313
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.