Skip to content

Patterns: Don't inject theme attr on frontend #5455

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.

Sign up for GitHub

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

Closed
ockham wants to merge 10 commits into from
Closed

Patterns: Don't inject theme attr on frontend #5455

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
909ec6b
Patterns: Don't inject theme attr on frontend
ockham Oct 10, 2023
8fb2f79
Remove _inject_theme_attribute_in_template_part_block() and all its u…
felixarntz Oct 11, 2023
9751e1c
Revert "Remove _inject_theme_attribute_in_template_part_block() and a…
felixarntz Oct 11, 2023
9315aa3
Only call _inject_theme_attribute_in_template_part_block() if a REST …
felixarntz Oct 11, 2023
5932fba
Only call _inject_theme_attribute_in_template_part_block() if a REST …
felixarntz Oct 11, 2023
5c27373
Avoid duplicate code.
felixarntz Oct 11, 2023
44d7eab
Remove tests which are no longer applicable.
felixarntz Oct 11, 2023
14f6306
Implement test coverage to ensure template part block works correctly…
felixarntz Oct 11, 2023
2af8248
Use `!== null` equality checks
ockham Oct 12, 2023
17ca37a
Use `!== null` equality checks once more
ockham Oct 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Only call _inject_theme_attribute_in_template_part_block() if a REST … 
…API request.
  • Loading branch information
@felixarntz @ockham
felixarntz authored and ockham committed Oct 12, 2023
commit 9315aa3e9eb26f5a504ad9deba6f9c40848c7420
4 changes: 3 additions & 1 deletion src/wp-includes/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,9 @@ function make_before_block_visitor( $hooked_blocks, $context ) {
* @return string The serialized markup for the given block, with the markup for any hooked blocks prepended to it.
*/
return function ( &$block, $parent_block = null, $prev = null ) use ( $hooked_blocks, $context ) {
_inject_theme_attribute_in_template_part_block( $block );
if ( defined( 'REST_REQUEST' ) && REST_REQUEST ) {
_inject_theme_attribute_in_template_part_block( $block );
}

$markup = '';

Expand Down
8 changes: 5 additions & 3 deletions src/wp-includes/class-wp-block-patterns-registry.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,14 +165,16 @@ public function unregister( $pattern_name ) {
private function prepare_content( $pattern, $hooked_blocks ) {
$content = $pattern['content'];

$before_block_visitor = '_inject_theme_attribute_in_template_part_block';
$before_block_visitor = ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ? '_inject_theme_attribute_in_template_part_block' : null;
Copy link
Contributor Author

@ockham ockham Oct 12, 2023

Choose a reason for hiding this comment

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

We might consider moving the code that injects the theme attr to the patterns controller instead. Probably okay to leave as-is for now, but might be a nice follow-up post-6.4.

(Noting that if we still insert hooked blocks here, we'd be running traverse_and_serialize_blocks twice, which is suboptimal; so we'd also need to move hooked blocks insertion to the controller so we can remove it here; which would then require us to make sure it's run on the frontend. We can probably achieve that through running it in the Pattern block's render_block. This means that we'd need to make quite a few changes, which we most likely don't want to do before 6.4.)

Copy link
Member

@gziolo gziolo Oct 12, 2023

Choose a reason for hiding this comment

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

Can you explain why traverse_and_serialize_blocks would get called twice?

Copy link
Contributor Author

@ockham ockham Oct 12, 2023

Choose a reason for hiding this comment

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

Can you explain why traverse_and_serialize_blocks would get called twice?

Assuming that we inject the theme attr in the patterns controller, but keep hooked blocks insertion here in the patterns registry (since it's needed both for the frontend and editor (i.e. REST API)) -- both of these call traverse_and_serialize_blocks 😅

Copy link
Member

@gziolo gziolo Oct 12, 2023

Choose a reason for hiding this comment

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

I see what you mean here - If we were to run the theme attribute injection in the REST API controller. Yes, good point that it would get called twice with different processing 👍🏻

$after_block_visitor = null;
if ( ! empty( $hooked_blocks ) || has_filter( 'hooked_block_types' ) ) {
$before_block_visitor = make_before_block_visitor( $hooked_blocks, $pattern );
$after_block_visitor = make_after_block_visitor( $hooked_blocks, $pattern );
}
$blocks = parse_blocks( $content );
$content = traverse_and_serialize_blocks( $blocks, $before_block_visitor, $after_block_visitor );
if ( ! is_null( $before_block_visitor ) || ! is_null( $after_block_visitor ) ) {
$blocks = parse_blocks( $content );
$content = traverse_and_serialize_blocks( $blocks, $before_block_visitor, $after_block_visitor );
}

return $content;
}
Expand Down