Block Hooks: Toggle (re-)inserts hooked block in wrong position (if added by filter)

[Bernhard Reiter]

As of ​this change, Block Hook toggles are displayed for hooked blocks that were added by a filter (rather than during block registration), if the containing template or template part has had user modifications. (Displaying the toggle also for unmodified templates/parts is the subject of #59574, see especially https://core.trac.wordpress.org/ticket/59574#comment:13.)

There is still one more problem with these toggles: If the user disables the toggle to remove the hooked block, and then re-enables it to re-insert the latter, it's not possible for the client to infer where to insert it. Instead, it simply defaults to (re-)inserting it after the anchor block.

The reason is that once the hooked block is removed, no information about its position relative to its anchor block is available. The only available information is the ignoredHookedBlocks metadata attribute on the anchor block, which is an array of hooked block type names (e.g. ["core/loginout","mycommerce/shopping-cart"]). They do however not include the relative position of these hooked blocks. (This was considered in the original ticket #59646 for enabling Block Hooks in modified templates/parts, but ultimately dismissed.)

[Bernhard Reiter]

In order to make the toggle work reliably for hooked blocks injected via a filter, we'll have to provide the missing information (i.e. their relative positions) to the client. There are (at least) two ways that this could be done:

1. Include the relative position in the ignoredHookedBlocks metadata, i.e. change the format to something like

<!-- wp:navigation {"metadata":{"ignoredHookedBlocks":{{"core/loginout":"after,"mycommerce/shopping-cart":"after"}}} -->

or

<!-- wp:navigation {"metadata":{"ignoredHookedBlocks":{"after":["core/loginout","mycommerce/shopping-cart"]}}} -->

This would amount to a format change for the ignoredHookedBlocks metadata attribute, which will require some additional fallback/compatibility logic in case the attribute was set using the "old" format.

2. Add an endpoint (or a field) to list hooked blocks per anchor block in a given context

It's not possible to simply extend the hooked-blocks endpoint to this end, as the set of hooked blocks for a given anchor block might vary depending on context (e.g. a "Like Button" block could be inserted by a filter below the Post Content block only when encountered within the single template).

Instead, a new endpoint is required that allows the client to specify the context. This was discussed in #59574 and explored in this ​Gutenberg PR (which was also filed as a solution for a separate issue). Ultimately, it was abandoned as overly complex.

A variation of this solution would be to add a hooked-blocks field to a number of existing endpoints (e.g. the templates or navigation ones); this would be context-dependent by definition. The downside is that it'd negatively impact separation of concerns for those controllers (requiring adding the same kind of logic and overhead to determine hooked blocks to all affected controllers). It might also translate poorly to the required state tree infrastructure (i.e. selectors etc.) on the client side.

[Bernhard Reiter]

#60755 was marked as a duplicate.

[swissspidy]

@bernhard-reiter could you please clarify the intended milestone here? Thanks!

[Bernhard Reiter]

Tentatively targetting 6.6 :)

[gziolo]

Moving to WordPress 6.7.

[Bernhard Reiter]

One thing that occurred to me the other day is that if we go with option 1 (Include the relative position in the ignoredHookedBlocks metadata), we’d still face the problem that when the user activates the toggle to insert the block, it would be inserted in its default state, i.e. any attributes and/or inner blocks set via the hooked_block filter would be ignored 😕

I’m not sure there’s an easy fix for that.

[chaion07]

Thanks @bernhard-reiter for reporting this. As we move closer to the RC1 for 6.7 in a matter of days and with no progress in this cycle, we are updating the milestone following the feedback received from a recent bug-scrub session.

Cheers!

[audrasjb]

As per today's bugscrub and since no activity happened in the 7 previous months, let's move this ticket to Future Release. Feel free to re-milestone it when needed.