Block Comments: Add blockComment block support#71847
Block Comments: Add blockComment block support#71847karthick-murugan wants to merge 3 commits intoWordPress:trunkfrom
Conversation
|
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. |
BLOCK_COMMENT_SUPPORT.md
Outdated
There was a problem hiding this comment.
This seems to be a leftover from an AI.
lib/block-supports/block-comment.php
Outdated
| * @param array $args Array of arguments for registering a block type. | ||
| * @return array Modified arguments. | ||
| */ | ||
| function gutenberg_add_block_comment_global_attribute( $args ) { |
There was a problem hiding this comment.
The feature is still experimental, so this should be placed in lib/experimental/block-comments.php.
There was a problem hiding this comment.
This logic already exists in packages/editor/src/components/collab-sidebar/index.js. There is no need for duplication.
|
Thanks for the PR! I think there is a contradiction between the PR title and the implementation. I believe this PR just adds block attributes on the server-side, not block support. This is code generated by an AI and has not been tested, but if we really want to add block support, the implementation should look something like this: Details<?php
/**
* Block comment block support flag.
*
* @package gutenberg
*/
/**
* Registers the blockComment block attribute for block types that support it.
* This support is enabled by default and can be disabled by setting blockComment: false in block.json.
*
* @param WP_Block_Type $block_type Block Type.
*/
function gutenberg_register_block_comment_support( $block_type ) {
// Check if block has explicitly disabled block comment support
$has_block_comment_disabled = block_has_support( $block_type, array( 'blockComment' => false ), false );
// If explicitly disabled, don't register the attribute
if ( $has_block_comment_disabled ) {
return;
}
if ( ! $block_type->attributes ) {
$block_type->attributes = array();
}
if ( ! array_key_exists( 'blockComment', $block_type->attributes ) ) {
$block_type->attributes['blockComment'] = array(
'type' => 'string',
);
}
}
// Register the block support.
WP_Block_Supports::get_instance()->register(
'block-comment',
array(
'register_attribute' => 'gutenberg_register_block_comment_support',
)
); |
IMO, that's what's needed for this case, but I agree the title/description needs to be updated. P.S. There's no need to add |
|
Another idea: how about treating the blockCommentId as part of the metadata? That way we can avoid a core backport PR and hook. |
I like the idea; the value seems more suitable for |
I like the idea in principle, would this mean moving the id from attributes to metadata? I'm not familiar with how block metadata works so I'll have to dig deeper to review or test a PR. |
Yes. The block markup will be changed as follows: <!-- From -->
<!-- wp:paragraph { "metadata": { "name": "My Paragraph" }, "blockCommentId": 1 } -->
<p>Hello World</p>
<!-- /wp:paragraph -->
<!-- To -->
<!-- wp:paragraph { "metadata": { "name": "My Paragraph", "blockCommentId": 1 } } -->
<p>Hello World</p>
<!-- /wp:paragraph --> |
To advance this approach, I have updated the title of the issue to follow the new approach. #71784 |
Great, thanks for clarifying @t-hamano. My next question which I can answer by testing your PR is if existing comment Ids need to be migrated to the metadata location to keep existing comments intact. ps. Does the project have a description of what types of data should be stored in metadata vs. attributes? is metadata just a specific attribute or does it have special properties/APIs? |
The Block Commenting is still an experimental feature, so I don't think we should migrate existing comment IDs. Breaking changes should be acceptable for now. |
Makes sense, thanks! |
|
Let's try the new approach in a separate PR. @karthick-murugan, I'd like to close this PR, but thanks for your efforts! |
What?
Closes #71784
Adds
blockCommentIdas a global attribute to all blocks to resolve 400 errors when adding block comments to ServerSideRender blocks.Why?
When adding block comments to blocks that use
ServerSideRendercomponents, a 400 error occurs because theblockCommentIdattribute is not recognized server-side. The REST API validation rejects the request with:How?
This PR implements
blockCommentIdas a global attribute similar tometadataandlockattributes:PHP Implementation
register_block_type_argsto injectblockCommentIdas a global attribute for all blocksJavaScript Implementation
blocks.registerBlockTypeto ensureblockCommentIdis registered client-sidelockandmetadatahooksTesting Instructions
ServerSideRender(e.g., Latest Comments)Video
REC-20250923193256.mp4