Comment Template Block: Set commentId context via filter
#50279
Conversation
In the Post Template block's render callback, instead of creating a new `WP_Block` instance with `postId` and `postType` context set, use the `render_block_context` filter to set that context and render the existing `WP_Block` instance. This approach arguably follows our established patterns with regard to handling block context better. Notably, with the previous approach, we were only setting block context for the Post Template block itself. In this PR, we extend it to apply to all child blocks, including ones that are dynamically inserted, e.g. via the `render_block` filter. This is relevant for Auto-inserting blocks (see #50103). This follows the precedent of the Comment Template block, see #50279.
|
Nice discovery @ockham 💯 Do you think we should apply a similar fix to the Post Template block? It has a very similar logic in place: gutenberg/packages/block-library/src/post-template/index.php Lines 90 to 98 in 4409ba4 |
|
I explored using an alternative approach: diff --git a/packages/block-library/src/comment-template/index.php b/packages/block-library/src/comment-template/index.php
index 6a8698f60f..4bf8f793b3 100644
--- a/packages/block-library/src/comment-template/index.php
+++ b/packages/block-library/src/comment-template/index.php
@@ -25,14 +25,15 @@ function block_core_comment_template_render_comments( $comments, $block ) {
$content = '';
foreach ( $comments as $comment ) {
- $comment_id = $comment->comment_ID;
- $filter_block_context = static function( $context ) use ( $comment_id ) {
- $context['commentId'] = $comment_id;
- return $context;
- };
- add_filter( 'render_block_context', $filter_block_context );
- $block_content = $block->render( array( 'dynamic' => false ) );
- remove_filter( 'render_block_context', $filter_block_context );
+ $block_content = ( new WP_Block(
+ $block->parsed_block,
+ array_merge(
+ $block->context,
+ array(
+ 'commentId' => $comment->comment_ID,
+ ),
+ )
+ ) )->render( array( 'dynamic' => false ) );
$children = $comment->get_children();
The intriguing discovery is that no place in the code explicitly exposes
|
I'm afraid this doesn't fix the issue I was originally seeing in 🙁 #50103. Try applying your patch to the branch from #50103 (and don't forget to rebuild!) Compare the two following screenshots:
With the patch, the auto-inserted Avatar block below the comment once again shows the "incognito person" instead of Wapuu (as it should) 😕 The reason is that the patch only changes the context that is passed to the
Yeah -- that's maybe something I should've mentioned in the PR description. The reason is that
Can you give a concrete example? I'm aware of this issue and the problem with the Post Template block in #50131, but I have a potential solution for the former and a theory that the latter is related to another (somewhat hackish) thing we're doing when rendering that block (and we're not doing anything like that to the Comments Template block). So I'm somewhat optimistic that we'll be able to eschew those problems... 🤞 |
|
As a next step, I'd like to explore if this change would fix this behavior (i.e. restore the behavior prior to this PR). Furthermore, I should probably add test coverage to ensure context is passed correctly to child blocks (as demonstrated e.g. here) to prevent future changes from breaking that behavior. |
Draft PR, currently only containing automated testing (which isn't behaving as expected yet 😕 ): #50883 |
In the Post Template block's render callback, instead of creating a new `WP_Block` instance with `postId` and `postType` context set, use the `render_block_context` filter to set that context and render the existing `WP_Block` instance. This approach arguably follows our established patterns with regard to handling block context better. Notably, with the previous approach, we were only setting block context for the Post Template block itself. In this PR, we extend it to apply to all child blocks, including ones that are dynamically inserted, e.g. via the `render_block` filter. This is relevant for Auto-inserting blocks (see #50103). This follows the precedent of the Comment Template block, see #50279.
Test works now; I've amended the PR to also include the fix for that issue and have opened it for review. |
* Post Template Block: Set block context via filter In the Post Template block's render callback, use the `render_block_context` filter to set `postId` and `postType` context, rather than passing that context directly to the `WP_Block` constructor. This approach arguably follows our established patterns with regard to handling block context better. Notably, with the previous approach, we were only setting block context for the Post Template block itself. In this PR, we extend it to apply to all child blocks, including ones that are dynamically inserted, e.g. via the `render_block` filter. This is relevant for Auto-inserting blocks (see #50103). This follows the precedent of the Comment Template block, see #50279. Furthermore, add some test coverage to guard against duplicated block-supports class names, which was an issue in a previous iteration of this PR.
* Post Template Block: Set block context via filter In the Post Template block's render callback, use the `render_block_context` filter to set `postId` and `postType` context, rather than passing that context directly to the `WP_Block` constructor. This approach arguably follows our established patterns with regard to handling block context better. Notably, with the previous approach, we were only setting block context for the Post Template block itself. In this PR, we extend it to apply to all child blocks, including ones that are dynamically inserted, e.g. via the `render_block` filter. This is relevant for Auto-inserting blocks (see WordPress#50103). This follows the precedent of the Comment Template block, see WordPress#50279. Furthermore, add some test coverage to guard against duplicated block-supports class names, which was an issue in a previous iteration of this PR.
What?
In the Comment Template block's render callback, instead of creating a new
WP_Blockinstance withcommentIdcontext set, use therender_block_contextfilter to set that context and render the existingWP_Blockinstance.Why?
This approach arguably follows our established patterns with regard to handling block context better. Notably, with the previous approach, we were only setting block context for the Comment Template block itself. In this PR, we extend it to apply to all child blocks, including ones that are dynamically inserted, e.g. via the
render_blockfilter. This is relevant for Auto-inserting blocks (see #50103).How?
See "What".
Testing Instructions
Verify that the Comments block (and children) are still working as expected on the frontend:
trunk, create a number of comments below a given post, nested at different levels. View that page on the frontend, and take note (and a screenshot) of what the comments look like.Additionally, you can copy the relevant unit tests file from
wordpress-develop, and run it against this branch in GB:Testing Instructions for Keyboard
N/A.
Screenshots or screencast
N/A.