Block Hooks: Allow hooked_block filter(s) to return null

[Bernhard Reiter]

The ​`hooked_block_types` filter allows extenders to conditionally insert a block, based on the anchor block's type.

However, it's currently not possible to base the criterion for insertion on the value of an attribute of the anchor block.

One fairly straight-forward way to implement this would be to allow the hooked_block (and hooked_block_{$hooked_block_type}) filter -- which has access to the entire parsed anchor block, i.e. including attributes -- to return null, which Block Hooks could then interpret as "do not render hooked block". (This would avoid changing the hooked_block_types filter's signature in an awkward way.)

[Bernhard Reiter]

One thing to consider when implementing this is that it somewhat muddles the invariant that only the hooked_block_types filter can change which hooked blocks are slated for insertion, whereas the hooked_block (and hooked_block_{$hooked_block_type}) filter cannot -- the latter have so far only been able to modify a given hooked block's attributes, inner blocks, etc.

This might be okay. We just need to be mindful of correctly setting ignoredHookedBlocks if hooked_block or hooked_block_{$hooked_block_type} returns null.

The ​`hooked_block_types` filter allows extenders to conditionally insert a block, based on the anchor block's type.

However, it's currently not possible to base the criterion for insertion on the value of an attribute of the anchor block.

One fairly straight-forward way to implement this would be to allow the hooked_block (and hooked_block_{$hooked_block_type}) filter -- which has access to the entire parsed anchor block, i.e. including attributes -- to return null, which Block Hooks could then interpret as "do not render hooked block". (This would avoid changing the hooked_block_types filter's signature in an awkward way.)

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

This would avoid changing the hooked_block_types filter's signature in an awkward way

You still need to change the signature to indicate the value can be null or an array

This would avoid changing the hooked_block_types filter's signature in an awkward way

You still need to change the signature to indicate the value can be null or an array

What I meant is I don't need to add a $parsed_anchor_block arg to ​`hooked_block_types` (when it already has $anchor_block_type).

It's true that I need to allow for hooked_block and hooked_block_{$hooked_block_type} to return null, but that's less of a problem -- they've only been introduced during the 6.5 cycle (unlike hooked_block_types, which has been around since 6.4).

[Bernhard Reiter]

Tentatively setting the milestone to 6.5.

This request has been brought up independently by two people now (@gziolo and ​https://github.com/joshuatf for who I couldn't find a WP.org profile), so I think it makes sense to add this. It's a fairly contained change to a filter that was introduced during the 6.5 cycle.

[Bernhard Reiter]

@swissspidy Think this can go into 6.5? If so, does it need to be set to "task (blessed)", and can I do so myself, or do I need someone else to sign off on this?

FYI @tjcafferkey -- if you'd like to give this a review 😄

[swissspidy]

Since these are new filters I'd prefer to have theme correct from the beginning, so yeah I'm fine with adding this. Are you able to finish this up before beta 2?

Please note that you still need to update the docblock to indicate that $parsed_hooked_block can be an array or null, as per my comment on the PR.

Also, it's @joshuatf on here as well.

@param array|null $parsed_hooked_block The parsed block array for the given hooked block type or null to remove the block.

@param array|null $parsed_hooked_block The parsed block array for the given hooked block type or null to remove the block.

@swissspidy Thank you, good point! Added in 556646426d. (I opted to write "to suppress the block" rather than "remove", as the latter would sound a bit like the block has already been inserted and is being removed. LMK if that's okay, or if you'd prefer "remove" after all.)

[Bernhard Reiter]

Replying to swissspidy:

Since these are new filters I'd prefer to have theme correct from the beginning, so yeah I'm fine with adding this. Are you able to finish this up before beta 2?

From my POV, it's pretty much ready. I've modified both hooked block insertion and setting ignoredHookedBlocks, and covered both with unit tests.

Please note that you still need to update the docblock to indicate that $parsed_hooked_block can be an array or null, as per my comment on the PR.

Thank you! I saw it there and pushed a commit to address.

Also, it's @joshuatf on here as well.

Great find! I should've tried that, rather than just Googling.

Rebased.

FYI @tjcafferkey -- if you'd like to give this a review 😄

I'm going to try and review this today if required 😄

@tjcafferkey If you'd like to, the sooner the better - beta 2 release party kicks off in around 2hrs.

[swissspidy]

Fixed in https://core.trac.wordpress.org/changeset/57668

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

[Bernhard Reiter]

Replying to swissspidy:

Fixed in https://core.trac.wordpress.org/changeset/57668

Thank you @swissspidy! (Silly me forgot the # before the ticket number in the commit msg.)

[Bernhard Reiter]

In 57676:

Block Hooks: Make test a bit easier to read.

Follow-up [57668].
Props gziolo.
See #60580.