Make WordPress Core

Opened 5 months ago

Last modified 97 minutes ago

#60756 new defect (bug)

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

Reported by: bernhard-reiter's profile Bernhard Reiter Owned by:
Milestone: 6.7 Priority: normal
Severity: normal Version:
Component: Editor Keywords:
Focuses: Cc:

Description

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.)

Change History (6)

#1 @Bernhard Reiter
5 months ago

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.

#2 @Bernhard Reiter
5 months ago

#60755 was marked as a duplicate.

#3 @swissspidy
5 months ago

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

#4 @Bernhard Reiter
4 months ago

  • Milestone changed from Awaiting Review to 6.6

Tentatively targetting 6.6 :)

#5 @gziolo
7 weeks ago

  • Component changed from General to Editor
  • Milestone changed from 6.6 to 6.7

Moving to WordPress 6.7.

#6 @Bernhard Reiter
97 minutes ago

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.

Note: See TracTickets for help on using tickets.