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

Block Hooks: Use apply_block_hooks_to_content in Patterns and Templates #7220

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Aug 21, 2024

In the Patterns registry, use apply_block_hooks_to_content instead of the WP_Block_Patterns_Registry class's private get_content method. (The latter is removed at part of this PR.)

In a similar vein, use apply_block_hooks_to_content in the _build_block_template_result_from_file and _build_block_template_result_from_post functions, respectively.

For that to work, apply_block_hooks_to_content is amended to inject the theme attribute into Template Part blocks, even if no hooked blocks are present.

This kind of centralization is required as a preparation for https://core.trac.wordpress.org/ticket/61902.

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


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@ockham ockham self-assigned this Aug 21, 2024
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@ockham ockham changed the title Patterns: Use apply_block_hooks_to_content Block Hooks: Use apply_block_hooks_to_content in Patterns and Templates Aug 21, 2024
@ockham ockham force-pushed the update/use-apply-block-hooks-to-content-in-patterns branch from f41d44a to b901657 Compare September 17, 2024 07:11
@ockham ockham force-pushed the update/use-apply-block-hooks-to-content-in-patterns branch from b901657 to 1604b38 Compare September 26, 2024 08:29
@ockham ockham marked this pull request as ready for review September 26, 2024 10:48
Copy link

github-actions bot commented Sep 26, 2024

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 props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props bernhard-reiter, jonsurrell.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@ockham
Copy link
Contributor Author

ockham commented Sep 26, 2024

I have opened this for review, as it's going to be a prerequisite for #7296 (to which I'll push some commits shortly that will implement the required change inside of apply_block_hooks_to_content, as that's the most suitable location for it).

I haven't yet added dedicated unit test coverage for apply_block_hooks_to_content in this PR, but note that it's covered indirectly by unit tests for the functions where it is called from, e.g.:

  • _build_block_template_result_from_file: covered in buildBlockTemplateResultFromFile.php
  • _build_block_template_result_from_post: covered in buildBlockTemplateResultFromPost.php
  • inject_ignored_hooked_blocks_metadata_attributes: covered in injectIgnoredHookedBlocksMetadataAttributes.php
  • update_ignored_hooked_blocks_postmeta: covered in updateIgnoredHookedBlocksPostMeta.php
  • WP_Block_Patterns_Registry::get_registered: covered in wpBlockPatternsRegistry.php
  • WP_Block_Patterns_Registry::get_all_registered: covered in wpBlockPatternsRegistry.php

Copy link
Member

@sirreal sirreal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes make sense, it seems to be mostly shifting some code around so that apply_glock_hooks_to_content is used more consistently.

I'm not especially well versed in templates or block hooks, so if there's additional testing I can do I'd be happy to.

@ockham
Copy link
Contributor Author

ockham commented Sep 27, 2024

These changes make sense, it seems to be mostly shifting some code around so that apply_glock_hooks_to_content is used more consistently.

Thank you very much for reviewing and approving!

I'm not especially well versed in templates or block hooks, so if there's additional testing I can do I'd be happy to.

I just did a bunch of smoke-testing and I'm confident enough to merge this, but thank you for offering! FWIW, I typically use my own Like Button block plugin to test (which has code to use Block Hooks to automatically insert it after the Post Content block), or something like the following to inject the Login/out block into the Navigation menu. I then check if hooked blocks are inserted both on the frontend and in the editor, and move them around in the editor to see if those changes are respected on the frontend.

function register_logout_block_as_navigation_last_child( $hooked_blocks, $position, $anchor_block, $context ) {
	if ( $anchor_block === 'core/navigation' && $position === 'last_child' ) {
		$hooked_blocks[] = 'core/loginout';
	}

	return $hooked_blocks;
}
add_filter( 'hooked_block_types', 'register_logout_block_as_navigation_last_child', 10, 4 );

@ockham
Copy link
Contributor Author

ockham commented Sep 27, 2024

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

@ockham ockham closed this Sep 27, 2024
@ockham ockham deleted the update/use-apply-block-hooks-to-content-in-patterns branch September 27, 2024 09:19
@ockham
Copy link
Contributor Author

ockham commented Sep 27, 2024

There's one more instance where we should call apply_block_hooks_to_content (rather than invoke more low-level Block Hooks related functions): The Navigation block in Gutenberg. I've filed a PR: WordPress/gutenberg#65703.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants