Block Hooks: Enable for user-modified templates/parts/patterns

[Bernhard Reiter]

The biggest tradeoff we made in the implementation of Block Hooks was that we had to limit them to templates, template parts, and patterns that didn't have any user modifications (see #59313 for the reason). We would like to remove this limitation, so they’ll also work with user-modified templates, parts, and patterns.

The crucial problem we need to solve is to acknowledge if a user has opted to remove or persist a hooked block, so that the auto-insertion mechanism won't run again and inject an extraneous hooked block on the frontend when none is solicited.

[Bernhard Reiter]

Per discussion with @matveb, the most straight-forward mechanism to achieve this is probably an extra attribute on the anchor block (not on the hooked block'').

It seems that we might not even need to store specifically whether a block was persisted or dismissed; it might be enough to simply store the information that an anchor block has had a dependent hooked block inserted. (Think of this as somehow mimicking the current approach of unmodified vs user-modified templates.)

The syntax could be as follows:

<!-- wp:navigation {"blockHooks":{"after":["mycommerce/mini-cart"]}} -->

Here’s an example scenario:

• Let’s assume we start with a "vanilla" theme, i.e. no user modifications.
• We activate an eCommerce plugin that inserts a shopping cart block after the Navigation block. We want this to show immediately on the frontend.
• As the frontend is being rendered, we encounter a Navigation block. We check its blockHooks attribute to see if it contains the shopping cart block. It doesn't, so we proceed to inject the shopping cart block after the Navigation block.
• The user opens the Header template that contains the Navigation block (and shopping cart block) in the editor. The templates REST API controller, upon encountering the Navigation block, adds a blockHooks attribute that's set to {"after":["mycommerce/mini-cart"]}.
• The user modifies the header block; they either store it with the shopping cart block still in place, or remove it.
• The next time the frontend is rendered, we encounter the blockHooks attribute on the Navigation block. Since it now contains the shopping cart block, we refrain from automatically injecting the shopping cart block, thus preventing extraneous insertion.

• The user now activates a plugin that inserts a Login/out button block after the Navigation block (i.e. into the Header template that already has some user modifications).
• Since the Navigation block's blockHooks attribute doesn't contain the Login/out button, it is rendered on the frontend.
• If the user opens the Header template in the editor, the shopping cart block is added to the Navigation block's blockHooks attribute. This ensures that any further user modifications to the template will be reflected on the frontend; specifically, no extraneous instance of the Login/out block will be auto-inserted.

The biggest change that this will require is a global blockHooks attribute (or some other sort of global per-block metadata).

From a pair-programming session with @gziolo.

Description TBD.

TODO:

• [ ] Make sure hookedBlocks global attribute default is set correctly (to empty array).
• [ ] Make sure hooked block is correctly added to hookedBlocks attribute from "after" block visitor.
• [ ] Inject hooked blocks into DB-based templates.

Trac ticket: https://core.trac.wordpress.org/ticket/59646

[gziolo]

More notes from the aforementioned pairing session. We tried to figure out the necessary changes on the server to enable Block Hooks in the modified templates and content. We discussed a few ideas, and it all looks promising to use block attributes as the place to remember which blocks were hooked in the past. The next steps could look as follows:

• Finish refactoring the server implementation to always mark anchor blocks with the list of block types hooked to it at any given time in the history.
• Rethink the implementation for the Plugins panel in the block sidebar, to use the list of hooked blocks stored in the block attribute to present toggles for hooked blocks.
• Enable the same functionality for modified templates and content, taking into account that the list of hooked blocks stored for the anchor block needs to support additions (for unmodified templates and patterns, we always inject the attribute from scratch).
• The only identified challenge would be deciding what should happen when you insert a new block that is also an anchor block for other block types.

If I apply the following patch to this branch, I can see that the Mini Cart block is added to the Navigation block's blockHooks attribute in the preloaded routes:

src/wp-includes/blocks.php

@@ -772 +772 @@
                if ( ! isset( $block['attrs']['blockHooks'] ) || ! in_array( $hooked_block_type, $block['attrs']['blockHooks'] ) ) {
                        $markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
                }
-               if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
+               //if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
                        if ( ! isset( $block['attrs']['blockHooks'] ) ) {
                                $block['attrs']['blockHooks'] = array();
                        }
                        $block['attrs']['blockHooks'][] = $hooked_block_type;
-               }
+               //}
        }
        return $markup;
 }

The attribute still seems to be filtered out on the client side (in the editor) though -- I guess we need to make the global blockHooks attribute known to the editor.

I also can't see the attribute injected into the Comment Template block (for the Like Button) yet.

Needs more tweaking, I guess 😅

The attribute still seems to be filtered out on the client side (in the editor) though -- I guess we need to make the global blockHooks attribute known to the editor.

Right, that would be necessary here. Unless we reuse metadata attribute that is already being injected in the editor here:

​https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/hooks/metadata.js

In that case, we would need to make metadata the globally registered attribute on the server (which is probably needed anyway). The information would get stored as attributes.metadata.blockHooks.

So whatever works for you best 😀

[Bernhard Reiter]

In 56970:

Blocks: During traversal, allow post callback to modify block.

Both the $pre_callback and $post_callback functions that are given as arguments to traverse_and_serialize_block(s) receive a reference to the current block as their first argument. However, while any changes that the "pre" callback makes to the block are reflected by the serialized markup, the same wasn't true for the "post" callback: Any changes that it made were only applied after the block had already been serialized.

This commit changes the behavior such that $post_callback's changes to the current block are also reflected in the serialized markup.

See #59646.
Props gziolo.
Fixes #59669.

[Bernhard Reiter]

Replying to Bernhard Reiter:

The syntax could be as follows:

<!-- wp:navigation {"blockHooks":{"after":["mycommerce/mini-cart"]}} -->

As discussed with @matveb and @gziolo, we can probably flatten the syntax by removing the relative position, i.e. something like

<!-- wp:navigation {"blockHooks":["mycommerce/mini-cart"]} -->

We're still brainstorming to find a more descriptive / less confusing name for the attribute. We're also likely to put it inside of the metadata attribute (that's been introduced on the client side to allow e.g. naming of blocks IIRC).

[Bernhard Reiter]

Replying to Bernhard Reiter:

We're still brainstorming to find a more descriptive / less confusing name for the attribute. We're also likely to put it inside of the metadata attribute (that's been introduced on the client side to allow e.g. naming of blocks IIRC).

We'll go with ignoredHookedBlocks for a start.

[Bernhard Reiter]

Per discussion with @gziolo and @matveb, we'll indeed wrap the ignoredHookedBlocks property in the metadata attribute, i.e.

<!-- wp:navigation {"metadata":{"ignoredHookedBlocks":["mycommerce/mini-cart"]}} -->

I've added some more comments to the Trac issue; the _tl;dr_ is:

We'll go with ignoredHookedBlocks for a start.

Per discussion with @gziolo and @mtias, we'll indeed wrap the ignoredHookedBlocks property in the metadata attribute, i.e.

On the note of the global metadata attribute: It seems that per ​https://github.com/WordPress/gutenberg/pull/54426, it's no longer experimental 👍

Update: With the attribute renamed to ignoredHookedBlocks, and moved into metadata, the following patch

src/wp-includes/blocks.php

@@ -781 +781 @@
                ) {
                        $markup .= get_comment_delimited_block_content( $hooked_block_type, array(), '' );
                }
-               if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
+               //if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
                        if ( ! isset( $block['attrs']['metadata']['ignoredHookedBlocks'] ) ) {
                                $block['attrs']['metadata']['ignoredHookedBlocks'] = array();
                        }
                        $block['attrs']['metadata']['ignoredHookedBlocks'][] = $hooked_block_type;
-               }
+               //}
        }
        return $markup;
 }

now successfully adds the information to the block in the editor 🎉

Also working for lastChild insertion, although it seems like it's attaching the info to the wrong block 🤔

_tl;dr:_ In 6.5, we'll need arguments to two functions introduced during the 6.4 cycle to become references. Since that can be deemed a back-compat breaking change, I'm suggesting to make that change already.

---

The callbacks returned by make_before_block_visitor and make_after_block_visitor, respectively (which are passed as arguments to traverse_and_serialize_block(s)) currently accept three arguments, all of which are block arrays (i.e. with properties blockName, attrs, etc.):

• A _reference_ to the block they're currently visiting, &$block;
• the block's $parent_block; and
• the $previous block (for make_before_block_visitor), or the $next block (for make_after_block_visitor), respectively.

Those arguments are passed to the "block visitor" callbacks by traverse_and_serialize_block(s) during traversal. The block that the callback is currently visiting is passed _by reference_ to allow modifying it, which is e.g. used to inject the theme attribute into Template Part blocks.

One major limitation with Block Hooks is that they currently only work with templates, parts, and patterns that _don't have any user modifications_ (i.e. that come straight from the corresponding theme files, rather than the DB). For WP 6.5, we're planning to change that to also make Block Hooks work for templates, parts, and patterns that _do_ have user modifications: `#59646`.

The way we'll make this work is by storing an attribute on the "anchor" block. This is currently being developed in ​https://github.com/WordPress/wordpress-develop/pull/5523.

While working on that PR, I noticed that that means that we'll also need to allow the aforementioned callbacks to modify not only the currently visited $block, but also the $parent_block -- i.e. to pass the latter argument by reference, too. This is consistent with the requirement of adding an attribute to an anchor block, as it's not only the currently visited block that can serve as an anchor block (in the case of before or after sibling insertion), but also its parent (for first_child and last_child insertion).

Per ​discussion with @hellofromtonya, changing the $parent_block argument to become a reference can be considered a backwards-compatibility breaking change. I'm thus suggesting to make that change already as part of 6.4, which is the cycle where the relevant functions were first introduced. This should have no impact on existing code, since nothing currently relies on $parent_block remaining unmodified by the respective callback, nor is anything currently modifying that argument.

Trac ticket: https://core.trac.wordpress.org/ticket/59776

Better now:

(Also, no more patching needed -- the PR now Just Works™️)

Committed to Core trunk in `[57038], and backported to the 6.4 branch in [https://core.trac.wordpress.org/changeset/57039 [57039]`].

[Bernhard Reiter]

In 57043:

Blocks: During traversal, allow post callback to modify block.

Both the $pre_callback and $post_callback functions that are given as arguments to traverse_and_serialize_block(s) receive a reference to the current block as their first argument. However, while any changes that the "pre" callback makes to the block are reflected by the serialized markup, the same wasn't true for the "post" callback: Any changes that it made were only applied after the block had already been serialized.

This commit changes the behavior such that $post_callback's changes to the current block are also reflected in the serialized markup.

Reviewed by hellofromTonya.
Merges [56970] to the 6.4 branch.

See #59646.
Props gziolo.
Fixes #59669.

For use cases such as allowing the user to ​assign custom names to blocks or for making Block Hooks work with user-modified templates/parts/patterns (`#59646`), there's a requirement for a new global attribute (i.e. an attribute that can be added to all and any blocks) to hold that sort of metadata.

Spun off from ​https://github.com/WordPress/wordpress-develop/pull/5523.
Fixes ​https://github.com/WordPress/gutenberg/issues/55194.

Trac ticket: https://core.trac.wordpress.org/ticket/59797

Thanks all!

I also made sure to test this change with Gutenberg installed and activated, to make sure this change wouldn't commit with code from ​https://github.com/WordPress/gutenberg/pull/54426 ✅ (Fortunately, the latter has a number of isset guards to avoid that kind of collision.)

Committed to Core in https://core.trac.wordpress.org/changeset/57068.

I was very pleased to return to work and find this completed. Thank you so much 🙇

Some notes:

• ​https://github.com/WordPress/gutenberg/pull/55811 has been merged; we might want to do a package sync to pull in those changes, as we'll need them for hooked block insertion into modified templates. (I hope that a package sync won't have any adversary side effects, e.g. if other blocks rely on PHP code that's not in Core yet? Might be less risky since we're only at the beginning of the release cycle, and there's not too much stuff yet in GB's ​`lib/compat/wordpress-6.5`. Curious to hear your thoughts @gziolo)
• In a similar vein, we need to test the Block Hooks UI with this PR. We will need to update how it works, which has to be done in GB. Depending on the logic we'll have to implement there (to possibly accommodate both for the "old" way of hooked block insertion and the "new" way), this might raise the question if we should also implement insertion into modified templates in GB first 🤔 (We will probably be able to make an informed decision based on what those changes to the UI logic will need to look like.)

WIP. Alternative approach to #5523.

Trac ticket: https://core.trac.wordpress.org/ticket/59646

Alternative approach to ​https://github.com/WordPress/wordpress-develop/pull/5523. See ​https://github.com/WordPress/wordpress-develop/pull/5609#discussion_r1408393016 for the rationale.

The proposed alternative looks very appealing. I need to think more about it, but on the first look I would go with this approach.

It's less important, but the Ci job reports some coding style violations.

The concept introduced around ignoredHookedBlocks on the anchor block works great, and I'm unable to find any drawbacks of this approach on the server side. If you still favor the new helper proposed get_hooked_block_markup (most likely annotated as private), feel free to commit it together with related unit tests.

I'm a bit hesitant to open modified templates todat to fully integrate with Block Hooks because of a single edge discovered. Everything works like charm with the special ignoredHookedBlocks metadata value for existing and modified blocks. The only blocker is the case where a new block gets inserted in the editor that is also an anchor block. We discussed with Bernie possible solutions and it isn't that trivial. However, I'm leaning towards trying to avoid auto-inserting hooked blocks for the newly inserted anchor blocks and finding a way to annotate it with the list of hooked blocks to make it as close as possible to what happens on the server. The only limitation on the client is no ability to apply WP filter that can modify the list of hooked blocks for a certain target position.

Closing in favor or #5712.

I'm a bit hesitant to open modified templates todat to fully integrate with Block Hooks because of a single edge discovered. Everything works like charm with the special ignoredHookedBlocks metadata value for existing and modified blocks. The only blocker is the case where a new block gets inserted in the editor that is also an anchor block. We discussed with Bernie possible solutions and it isn't that trivial. However, I'm leaning towards trying to avoid auto-inserting hooked blocks for the newly inserted anchor blocks and finding a way to annotate it with the list of hooked blocks to make it as close as possible to what happens on the server. The only limitation on the client is no ability to apply WP filter that can modify the list of hooked blocks for a certain target position.

Here's a short screencast to illustrate the issue:

[Bernhard Reiter]

I've filed #60008 to have a dedicated ticket for introducing the ignoredBlockHooks metadata, without actually enabling the new technique for modified layouts. For the latter, we'll continue to use this ticket.

As stated in this comment,

I've filed `#60008` to have a dedicated ticket for introducing the ignoredBlockHooks metadata, without actually enabling the new technique for modified layouts. For the latter, we'll continue to use this ticket.

I'll link this PR to the new ticket, will update the title and description, and remove the changes to _build_block_template_result_from_post.

[Bernhard Reiter]

In 57157:

Block Hooks: Store ignored hooked blocks metadata in anchor block.

The biggest tradeoff that was made in the implementation of Block Hooks was that they were limited to layouts (i.e. templates, template parts, and patterns) that didn't have any user modifications (see #59313 for the reason). This changeset is a preparatory step to remove this limitation, so they’ll eventually also work with user-modified layouts.

The crucial problem to solve is how to acknowledge that a user has opted to remove or persist a hooked block, so that the auto-insertion mechanism won't run again and inject an extraneous hooked block on the frontend when none is solicited.

This is achieved by storing all known blocks hooked to a given anchor block in the metadata attribute on that anchor block; specifically in a field called ignoredHookedBlocks inside of the metadata. Hooked blocks are only rendered on the frontend if they're absent from that field; OTOH, they're injected into that field (via the REST API) when first loaded in the editor.

This simple logic guarantees that once a user modifies a given layout, those changes are respected on the frontend; yet if a plugin that includes a hooked block is activated after those modifications have taken place, the hooked block will be rendered on the frontend. This new technique supplants the one previously used (i.e. rendering hooked blocks on the frontend only if a layout doesn't have any modifications) in a rather direct way.

Note that this changeset only introduces the new metadata field and relevant logic; it does not yet enable hooked block insertion into modified layouts. That will be done in a subsequent step (see #59646).

Props gziolo.
Closes #60008.

Committed to Core in https://core.trac.wordpress.org/changeset/57157.

Using the new technique of storing information about whether or not a hooked block should be considered for injection or not, extend said injection to encompass _modified_ templates and parts.

Follow-up to ​https://github.com/WordPress/wordpress-develop/pull/5712.

Trac ticket: https://core.trac.wordpress.org/ticket/59646

[swissspidy]

@bernhard-reiter Can this one be closed?

This is one of the major changes planned for 6.5 and has been in the works for a while. It has been discussed quite extensively on the accompanying ticket, and most work has happened on preparatory changesets such as https://core.trac.wordpress.org/changeset/57157 and ​https://github.com/WordPress/gutenberg/pull/58553. Unit test coverage in this PR should ensure that it works properly.

I'll commit shortly 🙂

[Bernhard Reiter]

In 57594:

Block Hooks: Inject hooked blocks into modified templates and parts.

Using the new technique introduced in [57157] of using a metadata.ignoredHookedBlocks attribute in the anchor block to store information about whether or not a hooked block should be considered for injection, extend said injection to encompass modified templates and parts.

Fixes #59646.
Props gziolo, matveb.

Committed to Core in https://core.trac.wordpress.org/changeset/57594.