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

Template Part Block: Use get_block_template() #55961

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

ockham
Copy link
Contributor

@ockham ockham commented Nov 8, 2023

WIP. Attempt to fix #55956.

What?

Replace template part lookup logic in the Template Part block with get_block_template.

Why?

The Template Part block has been duplicating functionality (by a low-level implementation of template part lookup from the database with a fallback to block theme provided files) that’s also present in various functions in Core’s block-template-utils.php file.

This has become a problem at least twice, when we needed Template Part blocks to apply template related filters that are only present in those higher-level methods from block-template-utils.php, leading to #52892 and #55811, where we’ve replaced some low-level logic with slightly higher-level methods.

This solved the problems at hand. However, there's still a risk that the template part lookup code will continue to diverge — e.g. when an enhancement or bugfix is introduced into the methods in block-template-utils.php, the Template Part block might not receive them.

How?

See What section 🙃

Testing Instructions

Verify that Template Part blocks still work as before, both on the frontend and in the editor.
Caveat: This PR doesn't currently work!

@ockham ockham self-assigned this Nov 8, 2023
* @param array $attributes The block attributes.
* @param string $template_part_file_path Absolute path to the not found template path.
*/
do_action( 'render_block_core_template_part_none', $template_part_id, $attributes, $template_part_file_path );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note that $template_part_file_path is no longer publicly available, as it's encapsulated in the implementation of get_block_template.

* @param string $template_part_file_path Absolute path to the template path.
* @param string $content The template part content.
*/
do_action( 'render_block_core_template_part_file', $template_part_id, $attributes, $template_part_file_path, $content );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -60,42 +53,17 @@ function render_block_core_template_part( $attributes ) {
*/
do_action( 'render_block_core_template_part_post', $template_part_id, $attributes, $template_part_post, $content );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

* @param string $template_part_file_path Absolute path to the not found template path.
*/
do_action( 'render_block_core_template_part_none', $template_part_id, $attributes, $template_part_file_path );
} elseif ( 'custom' === $block_template->source ) {
Copy link
Member

Choose a reason for hiding this comment

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

I think checking against wp_id might be more future-proof here.

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.

Template Part Block: Use get_block_template() for template part lookup
2 participants