-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Block Hooks: Apply to Post Content (on frontend and in editor) #67272
Conversation
// The `the_content` filter does not provide the post that the content is coming from. | ||
// However, we can infer it by calling `get_post()`, which will return the current post | ||
// if no post ID is provided. | ||
return apply_block_hooks_to_content( $content, get_post(), 'insert_hooked_blocks' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As simple as that? apply_block_hooks_to_content
util is really powerful at this point. What risks do you see with this approach? What if there are synced patterns inside the rendered content? I assume they all are annotated with block hooks to ignore at this point, so double processing should be a non-issue if we ignore the additional CPU usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was also really happy when I saw how straight-forward this was. apply_block_hooks_to_content
was introduced by @tjcafferkey, and it really is a great high-level util to insert Block Hooks into a piece of block markup 😄
What risks do you see with this approach? What if there are synced patterns inside the rendered content? I assume they all are annotated with block hooks to ignore at this point, so double processing should be a non-issue if we ignore the additional CPU usage.
Yeah, intuitively I'd say this shouldn't duplicate any hooked blocks, but I do want to give it some good smoke testing to be sure.
I've filed a backport (WordPress/wordpress-develop#7889). I'm also experimenting with an alternative in WordPress/wordpress-develop#7898 (easier to do in Core than in GB), which will insert hooked blocks both on the frontend and in the editor. Maybe we can get that to work after all. |
5b5d0e8
to
9931b82
Compare
9931b82
to
e057209
Compare
Flaky tests detected in 705d3f6. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/12140570324
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
||
if ( 'wp_navigation' === $post->post_type ) { | ||
$wrapper_block_type = 'core/navigation'; | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that this else
clause can safely assume that the post_type
is either post
or page
based on the filters defined: rest_prepare_page
or rest_prepare_post
.
Everything works as advertised 👍🏻
In my testing, the Separator doesn't get prepended to the Heading block in the case when I add it to the post that previously has been saved with processed Heading blocks. What could I do differently? Screen.Recording.2024-12-11.at.14.45.20.movCould it be related to |
I still want to play with synced patterns and template parts when served from the database so they essentially behave like |
@@ -46,10 +46,33 @@ function render_block_core_post_content( $attributes, $content, $block ) { | |||
$content .= wp_link_pages( array( 'echo' => 0 ) ); | |||
} | |||
|
|||
$ignored_hooked_blocks = get_post_meta( $post_id, '_wp_ignored_hooked_blocks', true ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The trick for the first and last child of the Post Content block works like a charm!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything looks good to me. It works excellently for posts and pages.
FWIW, for the Core version of this change, I've actually been considering if we should apply Block Hooks directly from inside the Posts controller, rather than adding the filter to rest_prepare_page and rest_prepare_post, see WordPress/wordpress-develop#7898 (comment):
The only remaining question for Gutenberg implementation is whether we should explicitly limit the functionality to post
and page
post types for now and cover all custom post types in WP core by applying changes directly in the Posts controller? I would be in favor of that as otherwise, it might have unintended consequences for custom post types with the Gutenberg plugin installed.
Ah yes, good spot! Apologies, I think that this change in behavior is a side effect of some code I pushed after I wrote the testing instructions. It is, as you guessed correctly, code that was needed to make
Huh 🤔 I think you have a point there. It's actually not bad in terms of UX -- a bit more predictable maybe. I'm happy to just consider it a happy accident and land it as-is. (If needed, we can still tweak the behavior later, based on user feedback.) |
Yeah, that's a known issue. I filed a Trac ticket for this a while ago. The tl;dr is that the toggle uses the |
Great! 🎉 Thank you very much for reviewing 😊
Yeah, I agree 100% -- it's best if we limit the Gutenberg version to |
What?
Apply Block Hooks to post content -- i.e., insert hooked blocks into post content.
Why?
We've seen some demand for this. A typical example would be a plugin that provides blocks that can be used in posts and that would like to provide extensibility for those blocks.
How?
It works the same way as in templates:
ignoredHookedBlocks
metadata (into anchor blocks).ignoredHookedBlocks
metadata on individual anchor blocks persisted when saving.ignoredHookedBlocks
. OTOH, any hooked blocks that were present when saving the post have now simply become part of the markup.Testing Instructions, Part One
Start by installing the following plugin, but do not activate it yet:
Plugin code
Screenshots or screencast
Testing Instructions, Part Two
Edit: The behavior covered by the following testing instructions changed after I wrote them, as @gziolo noticed here. We ended up deciding that the new behavior -- where hooked blocks would not be inserted if the anchor block was added after the post was added might be preferable.
Now edit the post, and insert another heading at the bottom. Note that no separator is inserted at the client side "right away" -- the reason is that Block Hooks are almost exclusively handled on the server side. This is one minor UX inconsistency -- the same inconsistency is present in templates.Save the post, and reload it on the frontend. Note that a separator has now been inserted before the newly added heading!✨Finally, reload the post in the editor. Note that the separator is now also present there.Screenshots or screencast, Part Two