Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Epic: Block Hooks Features for WP 6.5 #54904

Closed
9 of 16 tasks
ockham opened this issue Sep 28, 2023 · 38 comments
Closed
9 of 16 tasks

Epic: Block Hooks Features for WP 6.5 #54904

ockham opened this issue Sep 28, 2023 · 38 comments
Labels
[Feature] Blocks Overall functionality of blocks [Type] Iteration Scoped iteration of an effort from a tracking issue or overview issue ideally for a major release.

Comments

@ockham
Copy link
Contributor

ockham commented Sep 28, 2023

Block Hooks have been formerly know as Auto-inserting Blocks.

Plans for WordPress 6.5

Other

  • Performance: Try refactoring get_hooked_blocks() to check impact wordpress-develop#5399
  • Block Hooks: Update the compatibility layer #58468 (Backport filter mechanism to Gutenberg.)
  • Add higher-level test coverage (integration and/or e2e).
  • Respect template locks.
    • I need to read up on them, but I imagine we shouldn't auto-insert into a locked template? 😬
  • Respect "multiple": false.
    • Blocks can stipulate they only be inserted once. Auto-insertion should probably respect that; @nerrad made a PoC PR during WCUS Contributor Day: Rough POC of respecting "supports multiple" in the auto-inserted block metadata #53925
    • Some prior discussion: "It could also be that autoInsert works upon the first encountered block of the specified type, so it doesn't get repeated if there are multiple navigation blocks in a page. Or maybe this is also a flag like useOnce."
    • This still doesn't give control over which navigation block that is (but will default to the first one encountered); we might want a mechanism to select that, too. See next item for a somewhat related problem.
    • Finally, note that the multiple field was originally introduced for a post editor context; it isn't really well-defined in a template/Site Editor, as @gziolo has observed.
  • Notify user that there’s a new auto-inserted block available (after activating a new plugin that includes that block).
    • In the editor, or even wp-admin? Needs design.
    • How it could be implemented: Make list of registered blocks before and after plugin activation, compare; see if newly added block.json has autoInsert field? (Doesn't cover blocks that are auto-inserted programmatically.)
@ockham ockham added [Feature] Blocks Overall functionality of blocks [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. labels Sep 28, 2023
@gziolo gziolo mentioned this issue Sep 28, 2023
69 tasks
@gziolo
Copy link
Member

gziolo commented Sep 28, 2023

One more item to consider would be to potentially iterate over the Plugins section UI in the sidebar panel based on the comments about different expectations from folks that were testing it so far: #53987 (comment), #53987 (comment) and #53987 (comment). I expect it will come back regardless after the call for testing when more people have a chance to play with the integration in the editor.

@ockham
Copy link
Contributor Author

ockham commented Oct 9, 2023

Update:

It was brought to @gziolo's and my attention that the performance of patterns is currently lacking in WP 6.4 (Beta), which is particularly apparent when using TT4. @dmsnell found that traverse_and_serialize_block(s) might be a likely culprit, so @gziolo and I looked into potential ways to improve performance. We ended up filing three candidate PRs:

All of them have since been benchmarked and/or profiled, and only one of them turned out to deliver actual improvements.

We'll thus land WordPress/wordpress-develop#5399 in time for Beta 3; there's also WordPress/wordpress-develop#5421 which might improve performance some more (but still needs verifying).

Other than than, @ndiego has done some preliminary testing of Block Hooks; we might try to make some further improvements (or at least file some issues/tickets) based on his results.

@fabiankaegy
Copy link
Member

Hey all 👋

I was just trying to create a demo of Block Hooks and it leaves me wondering whether I'm missing something 🤔

As far as I can tell from the documentation we can only define a block type where the additional elements should get injected. Is this correct?

I'm asking because I think that may be a pretty large limitation of the API currently. I had though this would work a bit more like CSS selectors where I could "query" for a block insertion point via a more complex selector like core/post-template core/post-author

The issue I am running into is that in a block based theme all the available blocks are used for so manny different things that it's almost impossible to make a blanked statement that a new block should be inserted after every author block for example. Because taking the example of the post author block it may be used very differently on a single post template compared to an archve template that renders query loops with the author of each post displayed within.

This is even more true when thinking about more generalized blocks such as the core/group etc.

Another thing I was hoping for is to target specific variations of a block. For example if one has a custom variation of a product query it would be really cool to say append a block to the post template in the product query variation of the query loop.

Thanks in advance for any response :) And I get that this is the first MVP version of this. I just want to make sure I'm not missing something :)

@fabiankaegy
Copy link
Member

Thinking about this more, I think one of the original goals of this was to make it possible for plugins like WooCommerce to automatically inject their "Mini Cart" block into the main navigation of the site.

However, if I get this right currently, that wouldn't be possible because all you could do is say add it to all navigation menus. But since most themes have way more than one menu that doesn't seem feasible 🤔

@colorful-tones
Copy link
Member

colorful-tones commented Oct 11, 2023

As far as I can tell from the documentation we can only define a block type where the additional elements should get injected. Is this correct?

@fabiankaegy I'm not sure I understand this statement, which makes me wonder if I'm missing a larger Block Hooks understanding/requirement for implementation altogether.

I'm currently trying to test an ACF Block by just dropping in the blockHooks and adding in any core block to reference. Is this an unexpected use case?

I'm using register_block_type() on init and the block.json is:

{
  "$schema": "https://schemas.wp.org/trunk/block.json",
  "apiVersion": 3,
  "name": "acf/album-meta",
  "version": "0.1.0",
  "title": "ACF Experiment",
  "category": "widgets",
  "icon": "coffee",
  "description": "Just a testing block.",
  "supports": {
    "html": false,
    "mode": false
  },
  "textdomain": "acf-experimental",
  "acf": {
    "renderTemplate": "render.php"
  },
  "usesContext": [
    "postType",
    "postId",
    "commentId"
  ],
  "blockHooks": {
    "core/comment-template": "lastChild"
  }
}

I'm running WP 6.4 Beta 3 through WordPress Beta Tester plugin on Local and I have Gutenberg plugin installed and activated. The Like Button plugin's block shows fine. However, my block does not display at all.

What am I doing wrong? 🤷

@fabiankaegy
Copy link
Member

@colorful-tones What I mean by that is that the API currently only supports this:

{
    "blockHooks": {
        "core/navigation": "lastChild"
    }
}

and not

{
    "blockHooks": {
        // core/template-part/header meaning the header variation of the template part block
        "core/template-part/header core/navigation": "lastChild"
    }
}

@colorful-tones
Copy link
Member

@fabiankaegy I see what you mean and that makes sense. I did not expect that level of explicitness for this first iteration of this new API, but I can certainly see how I would want it to get there quickly 😄

@colorful-tones
Copy link
Member

I've just tried npx @wordpress/create-block@latest copyright --variant dynamic and added the following to the block.json and this appeared to work fine. So, it is likely something to do with ACF's rendering. I'll try to coordinate with the ACF team on what might be happening. Thanks!

{
	"$schema": "https://schemas.wp.org/trunk/block.json",
	"apiVersion": 3,
	"name": "wpe/copyright",
	"version": "0.1.0",
	"title": "Copyright",
	"category": "widgets",
	"icon": "smiley",
	"description": "Example block scaffolded with Create Block tool.",
	"example": {},
	"supports": {
		"html": false
	},
	"textdomain": "copyright",
	"editorScript": "file:./index.js",
	"editorStyle": "file:./index.css",
	"style": "file:./style-index.css",
	"render": "file:./render.php",
	"viewScript": "file:./view.js",
	"blockHooks": {
		"core/comment-template": "lastChild"
	}
}

Screenshot 2023-10-11 at 10 51 07 AM

@lgladdy
Copy link

lgladdy commented Oct 11, 2023

ACF Blocks requires a block's save function to be called at least once for the block to work.

In the case of a ACF Block being added via a block hook, unless the template is edited and saved, that never happens - so we abort rendering because we can't figure out the block's type or data. If you make any changes to the template, save is fired, so it will work.

This issue is likely specific to ACF Blocks (although, there definitely could be other blocks that setup some data to the block comment on save which will also cause issues when hooked).

We'll work around this by inheriting more defaults from $this's WP_Block instance passed into the block renderer if they're not set in the block's attributes.

@ockham
Copy link
Contributor Author

ockham commented Oct 15, 2023

Update:

After landing WordPress/wordpress-develop#5399 (which sought to improve Block Hooks performance, see #54904 (comment)), it was unfortunately discovered that that change broke template parts. We managed to file and land a fix just in time for Beta 3; however, that fix also undid most of the original performance gains, as was found by @felixarntz 😕

After some discussion with Felix, we then filed a two-part fix seeking to avoid theme attribute injection on the frontend altogether, in order to improve performance at least a bit. We landed both changes on Thursday (the GB fix also necessitated a package sync) in order to have a bit more time to discover potential regressions and/or side effects before RC1.

@ockham
Copy link
Contributor Author

ockham commented Oct 15, 2023

@fabiankaegy @colorful-tones @lgladdy -- Apologies for not engaging in the conversation, I was a bit busy with the aforementioned issue last week. I won't be working tomorrow but hope to catch up with y'all on Tuesday!

@colorful-tones
Copy link
Member

@ockham I appreciate the update. No apologies necessary. You’re obviously busy and I hope you enjoy your day off. ❤️

@ockham
Copy link
Contributor Author

ockham commented Oct 18, 2023

Thinking about this more, I think one of the original goals of this was to make it possible for plugins like WooCommerce to automatically inject their "Mini Cart" block into the main navigation of the site.

However, if I get this right currently, that wouldn't be possible because all you could do is say add it to all navigation menus. But since most themes have way more than one menu that doesn't seem feasible 🤔

We discussed this a bit more elsewhere, but since that issue isn't (only) about Block Hooks, I wanted to bring back that conversation here.

FWIW, I succeeded having a Mini Cart block inserted after the Navigation block. The containing Header template part doesn't have any modifications; it seems like the Navigation block falls back to a list of select pages (although not a "complete" page list; possibly a menu created by WooCommerce during setup?)

mini-cart-after-navigation

@fabiankaegy
Copy link
Member

Thank you for sharing that! :)

How in the world did the navigation block get the ref ID added into the block markup whilst the template part is still not customized? :O

Looking at the actual template part file here: https://github.com/WordPress/wordpress-develop/blob/6528d9840b6800edfb8e75c4924936bb14b3f31b/src/wp-content/themes/twentytwentyfour/parts/header.html#L18

there is no Ref to be found. Since that is env specific :O

I am really surprised by those results. Really interesting & cool :) Thank you!

@pagelab
Copy link
Contributor

pagelab commented Oct 19, 2023

Hi, following this Make post, I did some testing with the PHP hooked_block_types filter, using footer as $context->area but it didn't work in a simple dynamic block plugin.

2023-10-19 19 29 40

It's working fine with header though.

@ockham
Copy link
Contributor Author

ockham commented Oct 30, 2023

I've added a new section called "Plans for WordPress 6.5" to the issue description (and might update the issue title accordingly), listing TODOs that I discussed with @mtias and @gziolo last week.

@ockham ockham changed the title Tracking issue: Block Hooks improvements Tracking issue: Block Hooks Features for WP 6.5 Oct 30, 2023
@ockham
Copy link
Contributor Author

ockham commented Oct 30, 2023

How in the world did the navigation block get the ref ID added into the block markup whilst the template part is still not customized? :O

Looking at the actual template part file here: https://github.com/WordPress/wordpress-develop/blob/6528d9840b6800edfb8e75c4924936bb14b3f31b/src/wp-content/themes/twentytwentyfour/parts/header.html#L18

there is no Ref to be found. Since that is env specific :O

Yeah, it is quite curious 🤔 My best guess is that the ref is actually inserted on the fly by the block's edit component:

/**
* This fallback displays (both in editor and on front)
* The fallback should not request a save (entity dirty state)
* nor to be undoable, hence why it is marked as non persistent
*/
__unstableMarkNextChangeAsNotPersistent();
setRef( navigationFallbackId );

The counterpart logic for the frontend seems to be somewhere around here:

// If:
// - the gutenberg plugin is active
// - `__unstableLocation` is defined
// - we have menu items at the defined location
// - we don't have a relationship to a `wp_navigation` Post (via `ref`).
// ...then create inner blocks from the classic menu assigned to that location.
if (
defined( 'IS_GUTENBERG_PLUGIN' ) && IS_GUTENBERG_PLUGIN &&
array_key_exists( '__unstableLocation', $attributes ) &&
! array_key_exists( 'ref', $attributes ) &&
! empty( block_core_navigation_get_menu_items_at_location( $attributes['__unstableLocation'] ) )
) {
$menu_items = block_core_navigation_get_menu_items_at_location( $attributes['__unstableLocation'] );
if ( empty( $menu_items ) ) {
return '';
}
$menu_items_by_parent_id = block_core_navigation_sort_menu_items_by_parent_id( $menu_items );
$parsed_blocks = block_core_navigation_parse_blocks_from_menu_items( $menu_items_by_parent_id[0], $menu_items_by_parent_id );
$inner_blocks = new WP_Block_List( $parsed_blocks, $attributes );
}


The Navigation block is a rather tricky beast. FWIW, it's on our TODO list for 6.5 (see updated issue description) to make it work with firstChild/lastChild hooked blocks.

@ockham
Copy link
Contributor Author

ockham commented Oct 30, 2023

Hi, following this Make post, I did some testing with the PHP hooked_block_types filter, using footer as $context->area but it didn't work in a simple dynamic block plugin.

Thank you for reporting, and apologies for the late reply, @pagelab! I'll try to reproduce this on my end.
In the meantime, and just to eliminate a common source of confusion: Can you confirm that your footer template part doesn't have any user modifications? (See the "Customized" property below.)

image

@pagelab
Copy link
Contributor

pagelab commented Oct 31, 2023

Can you confirm that your footer template part doesn't have any user modifications?

@ockham Yes, there were no modifications, as in your picture (Customized: No).

@ockham
Copy link
Contributor Author

ockham commented Nov 6, 2023

Thank you @pagelab! I haven't gotten around to investigating the issue just yet but will do so this week 😄

@ockham
Copy link
Contributor Author

ockham commented Nov 6, 2023

Update:

I got a basic version of hooked blocks insertion into modified templates/parts/patterns to work in WordPress/wordpress-develop#5523. I've started to spin off smaller PRs to land individually; among them #55811 and WordPress/wordpress-develop#5608, which are ready for review.

I plan to land those PRs this week and will continue to break out PRs, polish the code, and add test coverage.

@ockham
Copy link
Contributor Author

ockham commented Nov 9, 2023

@pagelab I finally managed to test hooked block insertion into a footer template part today (together with @SantosGuillamot 👋 ). It seemed to work for us 🤔

We had one thought: Is it possible that the Group block you're trying to have the dynamic year block inserted into isn't a "direct" child of the Footer template part, but is instead part of a pattern that's used in the Footer? That could explain why it's not working for you, as the filter's $context arg is only aware of the "closest" ancestor template, part, or pattern.

Another idea: Since you're trying to insert your block as the last_child of a Group block, can you confirm that that Group block already has at least one child block? (The way that child insertion is implemented doesn't work with "empty" parent blocks.)

@pagelab
Copy link
Contributor

pagelab commented Nov 22, 2023

Sorry for the delay, @ockham; I've missed the notification.

Is it possible that the Group block you're trying to have the dynamic year block inserted into isn't a "direct" child of the Footer template part but is instead part of a pattern that's used in the Footer?

That is exactly what happened. I'm testing with TT4, and indeed it uses a pattern to define the contents of its default footer template part (footer.html).

<!-- wp:pattern {"slug":"twentytwentyfour/footer"} /-->

The header.html file doesn't do this; it simply adds the code for all blocks directly in the file itself.

Can you confirm that the Group block already has at least one child block?

Yes, the patterns/footer.php file, included in footer.html, adds several child blocks.

@ockham
Copy link
Contributor Author

ockham commented Dec 4, 2023

Thank you for confirming, @pagelab (and apologies for my delay 😅 )

Somewhat relieved that it's at least expected behavior; although it indicates that it'd be desirable to maybe provide more context (not just the "immediate" surrounding one) 🤔

@ockham
Copy link
Contributor Author

ockham commented Dec 4, 2023

Update:

A bit less progress in the past couple of weeks as I had to tend to some other things.

I've filed WordPress/wordpress-develop#5712 to supersede WordPress/wordpress-develop#5523, as I wasn't quite happy with the function signature of insert_hooked_blocks.

I'm hoping to land that PR this week, albeit potentially only the parts that persist the information about hooked blocks in the anchor block's metadata attribute, but not yet the part that enables hooked blocks insertion for modified layouts. The reason is a UX quirk that @gziolo pointed out that that change would introduce, and that we should thus sort out before landing that final part.

I also replied to some feedback on the make/core post and filed one ticket based on that feedback.

@leewillis77
Copy link
Contributor

I've been looking at making some of our WordPress plugins (particularly WooCommerce extensions) work well in a block-based world, and the Woo devs suggested that dropping some of our feedback here might be helpful. Let me know if you need any more information!

  1. I'd echo the sentiment in Epic: Block Hooks Features for WP 6.5 #54904 (comment). For plugins to work reliably and effectively having a full chain of context would definitely be desirable so that we can do things like "insert block X after block Y whenever it appears inside block Z" even when Y isn't necessarily a direct descendent of Z. As well as being more robust, in many cases it would also simplify development.
  2. Working out the combination of anchor/context that should be used to insert at a given location is more complex than it feels like it should be as the registration covers all registered items, and there's no way (that I could work out) to see what anchor/contexts are triggered on a given page. Not sure what the solution is here, maybe it would be less of an issue if [1] was solved?
  3. Block hooks are only truly useful if they can be used to reliably insert blocks into other blocks. At the minute there are some situations where it can be used, and others where it can't. As an example, take the WooCommerce 'product collection' block. Block hooks can be used to register blocks into this if the user selects a template, but the hook doesn't take any effect for the default block insertion, which leads to a confusing developer experience, and an inconsistent user experience.

@annezazu annezazu added the [Type] Iteration Scoped iteration of an effort from a tracking issue or overview issue ideally for a major release. label Jan 4, 2024
@gziolo gziolo changed the title Tracking issue: Block Hooks Features for WP 6.5 Epic: Block Hooks Features for WP 6.5 Jan 4, 2024
@gziolo gziolo removed the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label Jan 4, 2024
@ockham
Copy link
Contributor Author

ockham commented Jan 8, 2024

Update:

I’ve spent most of last week exploring potential solutions to the sibling block alignment issue that was brought to my attention:

I started out with a proof of concept that would set a hooked block’s layout attribute — if the block supports it — to the same value of its sibling anchor block (i.e. in case of before or after insertion) .

Since there’s a risk that that might be too arbitrary or "overbearing", I also explored a correctly. WordPress/wordpress-develop#5835 that a block developer could use to write the code that's needed to set the layout attribute.

Finally, I also tried a variation of the latter approach that would solely extend the existing hooked_block_types filter.

I’ve tried to list pros and cons for the latter two approaches. I’ll continue exploring these options a bit more this week before deciding on how to proceed.

@ockham
Copy link
Contributor Author

ockham commented Jan 29, 2024

Update:

It was a pretty productive week for Block Hooks:

This is nicely reflected by a number of now-ticked checkboxes in the issue's TODO list 😄


As we're approaching the cut-off date for GB changes to be included in WP 6.5, my focus this week will be on finally unblocking WordPress/wordpress-develop#5726 (see).

@swissspidy suggested adding a non-dynamic version of the hooked_block_{$block_type} filter (i.e. just hooked_block), which I'll also look into.

Finally, @jsnajdr was kind enough to file a PR to introduce an allowedBlocks field in block.json, which would unblock #53989, a potential stretch goal for WP 6.5.

@ockham
Copy link
Contributor Author

ockham commented Feb 5, 2024

Update:

The last week saw a bit more of experimentation, with four new PRs opened, only half of which are probably worth pursuing in the short term.

I spent some time last week trying to update GB's Block Hooks compatibility layer for WP < 6.4, which I thought might be a requirement to also add a compat layer for WP < 6.5. While it looks like I got that to work, it also turned out to be a distraction ❌

I then also tried to make hooked blocks insertion into the Navigation block work when used on WP 6.4 (rather than trunk), as I wanted to highlight this new feature in the release notes for GB 17.6, and it seemed too strong a limitation if people had to run it against WP trunk. The approach I chose turned out to be a dead end ❌

I then finally started working on setting the ignoredHookedBlocks attribute upon insertion of a new block. This is shaping up nicely; one problem I found is while there we have a way of setting initialAttributes for blocks inserted from the inserter, that mechanism doesn't apply to a container block's inner blocks, so things get a bit less elegant there 🚧

While working on this, I discovered that we the client really needs a way to ask the server for a list of hooked blocks for a given anchor block in a given context (e.g. within a Single template, or a header area template part). I spent a bit of time thinking what a REST API endpoint to that effect would look like and then started working on one 🚧


GB 17.7 RC 1 will be published on Wednesday, which is the last GB version whose code will be included in WP 6.5 by default. My plan is to finish work on ignoredHookedBlocks attribute insertion into new blocks today, and on the hooked-blocks endpoint tomorrow (as the latter can be seen as a refinement). This should then allow us to finally unblock hooked blocks insertion into modified templates/parts/patterns.

@gziolo gziolo moved this to 🏗️ In Progress in WordPress 6.5 Editor Tasks Feb 7, 2024
@fabiankaegy
Copy link
Member

Now that we are past the Beta 1 cut off point I'm removing this tracking issue from the 6.5 Project board. From now on the board should only contain individual bugs that we want to still fix before 6.5 gets released.

@ockham
Copy link
Contributor Author

ockham commented Feb 19, 2024

Update:

Near the beginning of the past two-week period, @tjcafferkey found and subsequently fixed a bug that had hooked blocks inserted twice into the Navigation block when running the GB plugin with WP trunk.

The focus of the last two weeks was resolving the mismatch between editor and frontend for newly inserted anchor blocks in order to unblock injection of hooked blocks into modified templates (WordPress/wordpress-develop#5726).

To that end, @tjcafferkey and I continued work on the client-side PR to set the ignoredHookedBlocks attribute for a newly inserted block, and the hooked-blocks controller that would inform the client which blocks were hooked to a given anchor blocks (including those added via a filter).

We managed to merge the client-side PR, which was good enough to unblock hooked blocks injection into modified templates, allowing me to finally land WordPress/wordpress-develop#5726 🎉

However, it became apparent that we wouldn't be able to land the controller in time for WP 6.5's Feature Freeze. We were considering to let Core committers know about the issue and potentially seek "task (blessed)" status for it in order to continue work on it after Beta 1.

That was until last Monday when @gziolo suggested an alternative approach, which turned out much better in a number of ways (didn’t require creating a new controller, kept logic contained on the server side). I managed to get a basic PR up and running by Monday evening, and thanks to Tom’s help, review, and testing, I was able to land it just ahead of Beta 1 🙂

I also reverted the client-side PR from our previous approach that we’d already merged, since we’re not going to need that now.


There's a bit of follow-up work needed now that we've made some changes to how the ignoredHookedBlocks attribute is set, chiefly with regard to the Navigation block, for which we already have a WIP fix. There were a few other suggestions by folks that might be too late for 6.5, but I'll file tickets and/or PRs regardless (e.g. allowing returning null from hooked_block_{$block_type} filter, or adding a generic hooked_block version of that filter). Other than that, I'll stand by for bugfixes, and will start work on a Dev Note, time permitting.

@ockham
Copy link
Contributor Author

ockham commented Feb 26, 2024

Update:

We landed the fix for the Navigation block. I then filed tickets and PRs for the feature requests I'd gotten recently:

Both were fortunately accepted for 6.5 and have already been committed 🎉

Since WordPress/wordpress-develop#6136 changed the signature of the hooked_block_{$block_type} filter, I also released a new version of the Like Button block plugin that I use for testing Block Hooks.

Tom then kindly filed a fix for a bug that was reported two weeks ago.


With WP 6.5 Beta 3 scheduled for tomorrow, I'll prioritize reviewing and landing the bugfix. That aside, I have to start writing the Dev Note for Block Hooks in 6.5. I'll also stand by for other bugs that might crop up.

@ockham
Copy link
Contributor Author

ockham commented Mar 4, 2024

Update:

I filed and merged a fix to display the Block Hooks toggle based on the presence of the ignoredHookedBlocks metadata attribute. I then reviewed @tjcafferkey’s fix for the Block Hooks toggle when used to insert into the Nav block and decided to go with a simpler alternative (that I had originally advised against) after all. Finally, I filed another small PR to add a short description to the Plugins (i.e. Block Hooks) panel in the block inspector.

I also published the Dev Note for Block Hooks in WP 6.5.


I was alerted to two issues that seem to have been caused by our recent changes to Block Hooks and will need to investigate them: #59516 and https://core.trac.wordpress.org/ticket/60671.

@ockham
Copy link
Contributor Author

ockham commented Mar 11, 2024

Update:

Our major focus last week was on fixing two issues with garbled entities that were both caused by changes to Block Hooks (one of them affecting the Navigation block, the other one affecting template parts). We managed to fix them both in a similar way, by using a filter instead of an action in order to avoid an additional wp_update_post call (which turned out to be the problem): See #59561 and WordPress/wordpress-develop#6225 for the respective fixes.


The fixes are on track for inclusion in RC2, pending a package sync for #59561, and a backport from Core trunk to the 6.5 branch for WordPress/wordpress-develop#6225. I'll take care of the latter today.

There's a minor follow-up work in WordPress/wordpress-develop#6244 that I'll also backport to 6.5. Other than that, I'll keep my eyes open for any other potential bugs. I might also start working/helping with some Block Hooks related documentation.

@ockham
Copy link
Contributor Author

ockham commented Mar 18, 2024

Update:

Last week, I realized that as a side effect of using the rest_pre_insert_{$this->post_type} filter instead of the rest_after_insert_{$this->post_type} action, we were passing the wrong $context to Block Hooks filters. I filed a ticket, and we started working on a fix (which turned out to be a lot of work).

Additionally, it was reported that creating a new Navigation post object via the REST API didn't work; this was fixed by @tjcafferkey.

We've started filing a few issues to cover some of the changes we'd like to make to Block Hooks after 6.5, namely:

  • #60756: Toggle (re-)inserts hooked block in wrong position (if added by filter)
  • #60759: Harmonize ignoredHookedBlocks metadata injection logic
  • #60769: Consolidate approach to get the list of hooked blocks

With RC3 scheduled for tomorrow, we're hoping to land the fix for the wrong $context in time. Time permitting, we might continue to file tickets and issues for Block Hooks past 6.5.

@gziolo
Copy link
Member

gziolo commented Mar 18, 2024

We've started filing a few issues to cover some of the changes we'd like to make to Block Hooks after 6.5, namely

Do you plan to open a new epic for WP 6.6, or should we put these items in a new section under Block API overview issue?

@annezazu
Copy link
Contributor

annezazu commented Mar 21, 2024

Please create a new epic :D <3 and let's please close this one out when ready!

@ockham
Copy link
Contributor Author

ockham commented Mar 27, 2024

Closing in favor of #60252.

I've carried over the open issues and tickets from this issue and applied some preliminary gardening. I'll continue to do so over the next couple of days.


@leewillis77 Apologies for not replying to your comment, I must've somehow missed it. I'll reply shortly over on the new issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Iteration Scoped iteration of an effort from a tracking issue or overview issue ideally for a major release.
Projects
None yet
Development

No branches or pull requests

8 participants