Blocks: No toggle shown for hooked blocks added via filter

[Bernhard Reiter]

Per #59313, one core tenet of Block Hooks is to give the user the ultimate say in persisting or removing a hooked block. This is done by showing hooked blocks in the editor, and making them customizable by the user.

Additionally, in order to aid visibility, a panel is added to the anchor block's block inspector with a list of blocks hooked to it, and toggles to enable or disable each of them.

However, that toggle is currently only shown if the hooked block was added unconditionally (i.e. via the blockHooks field in block.json, or registerBlockType()). Conversely, the toggle is not shown if the hooked block was added via the hooked_block_types filter (#59424)).

This is due to the how the toggles are implemented: The editor evaluates the block types endpoint's blockHooks field for a given anchor block (which corresponds to the block.json/block registration setting), and if it finds a hooked block in the specified location, it will show the toggle as enabled (and as disabled otherwise, unless the block is present elsewhere in the template).

By its nature, that endpoint is "unconditional", i.e. in cannot provide the information which hooked blocks are present based on what template (or pattern) is being edited. A fix will thus require a different mechanism.

[Bernhard Reiter]

The best way I can think of to fix this would involve an entirely new endpoint (e.g. block-hooks or hooked-blocks) that receives pretty much the same information as the hooked_block_types filter.

I.e. something like

/hooked-blocks/core/navigation?position=after&part=header
/hooked-blocks/core/comment-template?position=last-child&pattern=tt3/hidden-comments

that should return the list of blocks hooked to that specific anchor block and in the position given by the eponymous query arg.

(Some creative license taken here to allow for clients to have a pragmatic way of specifying context, see the part or pattern query args.)

[gziolo]

One idea that comes to my mind would be to always mark hooked blocks with a special metadata attribute:

<!-- wp:tests/hooked-last-child {"metadata":{"hookedBlock":"after"}} /-->

This way, the block editor could learn about them even when they were injected using hooked_block_types filter. The UI in the sidebar's Plugin section would be mostly about removing and then adding back this block.

[Bernhard Reiter]

Noting that a fix for #59646 would also fix this issue.

[Bernhard Reiter]

Replying to Bernhard Reiter:

Noting that a fix for #59646 would also fix this issue.

I think I misspoke. Working on ​https://github.com/WordPress/gutenberg/pull/58553, I realized that we still need information on what blocks are hooked to a given anchor blocks when that anchor block is newly inserted (so set its metadata.ignoredHookedBlocks attribute). In this case, the problem still applies: We can infer them from the blockHooks field of the `block-types' endpoint, but that is non-contextual; so we won't be able to correctly set the attribute if any hooked blocks were added (or removed) by a filter.

[Bernhard Reiter]

Replying to Bernhard Reiter:

The best way I can think of to fix this would involve an entirely new endpoint (e.g. block-hooks or hooked-blocks) that receives pretty much the same information as the hooked_block_types filter.

I.e. something like

/hooked-blocks/core/navigation?position=after&part=header
/hooked-blocks/core/comment-template?position=last-child&pattern=tt3/hidden-comments

that should return the list of blocks hooked to that specific anchor block and in the position given by the eponymous query arg.

Per ​https://github.com/WordPress/gutenberg/pull/57754, wp_navigation post objects can also be context. So we need to add something like

/hooked-blocks/core/page-list?position=after&navigation=4

to the list.

Pragmatically, we'd probably like to get a list of all anchor blocks that have hooked blocks when used in a given context, so we might want to support a syntax like

/hooked-blocks/?position=after&part=header
/hooked-blocks/?position=last-child&pattern=tt3/hidden-comments

(Some creative license taken here to allow for clients to have a pragmatic way of specifying context, see the part or pattern query args.)

This is probably the trickiest part to figure out: How do we best allow clients to specify the $context as freely as possible?

[Bernhard Reiter]

We might want to reserve the entirety of the query string to specifying the context, and specify the anchor block and relative position solely through the path. For a first iteration, we might not even need the relative position, as client-side code will typically be interested in any and all hooked blocks for a given anchor block.

[Bernhard Reiter]

$context, as specified as a filter argument, can be fairly heterogeneous. It can be

• a WP_Block_Template object (for templates or template parts)
• an array (for patterns),
• or a WP_Post object with post_type set to wp_navigation (for navigation menus).

(The latter is being added by ​https://github.com/WordPress/gutenberg/pull/57754 and ​https://github.com/WordPress/wordpress-develop/pull/5946.)

How do we best map this variety of $contexts to a REST API endpoint query string syntax (where we can e.g. limit to "only template parts for the header area"), and keep it extensible?

We can think of these different "entities" at different levels. One is WordPress’ PHP classes and types (see above). But there’s also the DB level (where templates, template parts, and Navigation menus are all stored in wp_posts, with different post_types; it's a bit more subtle for patterns), and finally the REST API level, where there are different endpoints (/templates/, /block-patterns/patterns/, and /navigation/).

Maybe the latter would be a good fit to inspire the query syntax for the hooked blocks' context, as they're operating in the same realm -- the REST API.

[Bernhard Reiter]

Replying to Bernhard Reiter:

[...] the REST API level, where there are different endpoints (/templates/, /block-patterns/patterns/, and /navigation/).

Here's the documentation for each of the aforementioned endpoints:

• ​Templates
• ​Patterns
• ​Navigations

Note in particular the (query) "Arguments" sections to retrieve an individual item:

• ​Templates support wp_id, area, and post_type.
• ​Navigations support id (and password, if the item is password-protected).
• Patterns don't seem to support any arguments.

[Bernhard Reiter]

Thinking a bit more about this: We might not actually have to "construct" entities via query args to pass to the filter. Our use case is inside the editor, which has information readily available about what template or navigation menu it is editing; we just need to pass that template's or nav menu's ID to the endpoint, which can then look up the matching template or nav menu and pass it as context to the filter.

(It might get trickier if it's a newly created template or nav menu.)

[gziolo]

We can infer them from the blockHooks field of the `block-types' endpoint, but that is non-contextual; so we won't be able to correctly set the attribute if any hooked blocks were added (or removed) by a filter.

How much is that an issue? What are the use cases where that would fail? What are the consequences?

The technical solution explored is very complex and depends on several moving parts. We need to keep in mind that the editor is only an approximation of what we will see on the frontend. In particular, when using WP hook, there might be nuances that change depending on the currently browsed page.

[Bernhard Reiter]

Replying to gziolo:

We can infer them from the blockHooks field of the `block-types' endpoint, but that is non-contextual; so we won't be able to correctly set the attribute if any hooked blocks were added (or removed) by a filter.

How much is that an issue? What are the use cases where that would fail? What are the consequences?

It's less about the toggle not being shown but rather about the ​other problem with inserting a new anchor block into an already-modified template. To set the ignoredHookedBlocks attribute upon insertion, we also need to know which hooked blocks would be inserted; and if those are set contextually by the hooked_block_types filter (e.g. only when inside the "Single" template or "Header" area template part), the editor won't have any knowledge of them.

The technical solution explored is very complex and depends on several moving parts. We need to keep in mind that the editor is only an approximation of what we will see on the frontend. In particular, when using WP hook, there might be nuances that change depending on the currently browsed page.

IMO, if we'd like insertion of a new anchor block to work before enabling ​insertion into modified templates, then we should also address this issue. The good news is that I have a ​prototype that I think will solve this problem.

My plan is to get the ​PR that sets the `ignoredHookedBlocks` attribute on newly inserted anchor blocks (based on the non-contextual information from the block-types endpoint) ready today, and then work on the new ​`hooked-blocks` endpoint controller tomorrow, as it can be seen as a refinement. (I'll post an update to the ​Epic shortly.)

[gziolo]

My plan is to get the PR that sets the ignoredHookedBlocks attribute on newly inserted anchor blocks (based on the non-contextual information from the block-types endpoint) ready today, and then work on the new hooked-blocks endpoint controller tomorrow, as it can be seen as a refinement.

I see, so that would be an additional enhancement. That sounds like a good plan!

What are the reasons the new endpoint is going to work better than extending existing endpoints that server templates, template parts, patterns, and navigation to return a custom field that offers hooked blocks with the hooked_block_types filter applied? These endpoints already have the proper context set when executed from the editor. I don't know how difficult would it be to process the list of hooked blocks on the server, but we wouldn't need any additional network requests.

[Bernhard Reiter]

With ​this Gutenberg PR merged and included in WP 6.5 RC1, the toggle is now displayed for modified layouts.

In order to also display it for unmodified ones, we'll have to run ​injection of the `ignoredHookedBlocks` metadata attribute not only upon writing to the database, but also upon reading from it (i.e. in the respective Templates/Patterns/Navigation endpoints' get_item() methods, in addition to insert_hooked_blocks).

The block hooks toggle relies on the ignoredHookedBlocks when determining whether to show the toggle in the Site Editor UI. The current problem is that for uncustomized templates we don't add this meta data so to fix this problem we need to run the block hooks logic with the set_ignored_hooked_blocks_metadata callback when preparing the template for the response.

TODO:

• [ ] Unit test coverage
• [ ] Add filter to prepare_item_for_response() method
• [ ] Add filter callback to run block hooks logic and add metadata attributes

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

The block hooks UI toggle relies on the ignoredHookedBlocks when determining whether to show the toggle in the Site Editor UI. The current problem is that for uncustomized templates we don't add this meta data. Currently the visitor functions have a default callback of insert_hooked_blocks and we only add the meta data when we're writing to the database via an available filter in the template controller.

This PR creates a new default callback which both inserts the hooked blocks and adds the ignoredHookedBlocks meta data to the anchor block, and makes this the new default callback for the visitor functions. This means we will continue to add ignoredHookedBlocks meta when writing to the database meaning this operation happens twice. Although not idea this is necessary to cover the following scenarios;

• When the user adds an anchor block within the editor we still need to add the ignoredHookedBlocks meta to it to prevent anchor blocks hooking on to it unexpectedly, this means we keep parity between the frontend and editor.
• When a user writes template data to the database directly through the API (instead of the editor) we need to again ensure we're not inserting hooked blocks unexpectedly.

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

@gziolo since @ockham is AFK I was wondering how I should proceed with this?

[Bernhard Reiter]

In 58186:

Block Hooks API: Insert metadata at the same time as hooked blocks.

The Block Hooks UI relies on the ignoredHookedBlocks metadata when determining whether to show the toggle in the Site Editor. The problem is that for uncustomized templates we don't add this metadata.

Currently the visitor functions have a default callback of insert_hooked_blocks and we only add the metadata when we're writing to the database via an available filter in the template controller.

This changeset creates a new callback which both inserts the hooked blocks and adds the ignoredHookedBlocks metadata to the anchor block, and uses this new callback explicitly in the visitor functions that are run upon reading from the database.

We continue to set the ignoredHookedBlocks metadata when writing to the database, i.e. this operation happens twice. Although not ideal, this is necessary to cover the following scenarios:

• When the user adds an anchor block within the editor, we still need to add the ignoredHookedBlocks meta to it to prevent hooked blocks hooking on to it unexpectedly on the frontend. This is required to keep parity between the frontend and editor.
• When a user writes template data to the database directly through the API (instead of the editor), we need to again ensure we're not inserting hooked blocks unexpectedly.

It is worth noting that with this change, the first hooked block to insert relative to its anchor block will be accepted. Any additional blocks of the same type (e.g. a second core/loginout block) trying to hook onto the same anchor block will be ignored, irrespective of the position.

Props tomjcafferkey, bernhard-reiter, gziolo.
Fixes #59574.

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

[kebbet]

Please milestone this ticket in 6.6 @bernhard-reiter