-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Add Inline comment experimental flag #60622
Changes from 94 commits
10c9ed0
271895a
f2b7114
5e0890c
031e8d5
6b0c3b3
d2d6215
cbee86f
b36c2ab
b7b1ba4
6c0862d
77ee009
a8ea60e
ec302ff
f002607
fdc44a3
bb2c0bd
e6ac0b4
9447911
bf84cc4
57ed39a
b9668fd
6628689
1d81922
87fe4f9
139135d
20f2bd3
2df0bdb
515a592
08fd837
251ae5f
e747430
b7a5bd7
16d2186
877f090
1ce0191
1a677d8
d67d2b1
e0cb8b5
9004004
d23a0a6
cf605d5
f115c3b
f748ad9
59e96c4
18e5081
c9f4964
1ed6d54
84733c5
2d5d162
e68cdcc
8042e22
ceb188c
575361c
8c45982
a2345e8
2004c04
8ac8328
868c095
ab73559
d5df8cc
533c5d4
c0a5dcd
56e475b
9de8c69
4104869
b8b0292
6996e5f
0321430
3bf3c4c
c66fbcd
751d727
9a42d64
68d5e1c
1f5a2c2
6477102
66832a7
5c70d90
cc28c6a
951d8b4
b0ad9d5
a5e12e1
cf59158
6cfdb29
465b438
c9d50d8
fd6f327
15c85e0
c7be827
bd1a7ce
19dfab8
48ee928
8fa3f85
5f0ae80
69d1878
d6e01f3
668671b
ee6fe75
de2cd8f
8e6da19
90ba459
e2fd75a
aad19d0
6f03c48
2b057cc
59caf1c
efa41ac
4ee210d
38bea58
609cad6
8a54986
2fd78b0
f236a42
b3c879d
bc308e9
11cf1f1
22c189a
33cd398
211944c
a4abbec
5c99dcc
50e8101
ce7f7bc
b0c5960
f768b4a
92870c4
37fa922
4d6841a
71f7a39
6f73a0b
edab788
6e528a1
b129367
8d17f22
fd4c045
4d3eeb1
f06340a
6b8027a
06c8526
e1d4396
c78f981
da72b79
d7bca6e
68d203e
b296d2d
bddceeb
c5094a0
a23241a
9548565
06e840f
3883a11
334daf2
b028876
cd63f6a
9b12661
f287eff
16ebd4c
7aa5508
e55109f
f66d825
7ff9a79
d01c693
2caf2d1
6bda66d
64b1b31
31ea3b5
bdf982f
c5a53a2
c0aee07
1a251ba
452181d
898a924
11d6693
0326444
bf20345
c419ce4
667163e
25364d8
5a53245
f4dd385
83a5955
ec1a8b3
2a4a427
ffc7f51
0d36a58
ed6e2e6
9a6556f
ebc28ca
02bd950
f5be6ac
973f559
e3c49b7
ab1395c
f9f24d9
63f915f
fc43c1e
68c5272
3d14991
6b32fcf
9db107b
c92de93
97a6d6e
0136fae
13f53d8
4d0ca6e
c5b76e8
02aabd9
65e1eb4
e3bc354
c8ff644
8cd46c3
45f9553
27ab929
b9017a4
e3cb45b
c4d4b1d
345c227
7a6c085
aa2d47b
7ce91e9
4cd5835
00076ff
7bf2c78
514853c
4b59a86
1cfd88e
45f6ac9
42a4eb2
e42e647
8744c83
127228b
5781530
ca54345
2e212c1
4b8986b
b42cd97
30ddf61
e446297
1955469
22893f8
b89ce85
3f9d745
90b038b
b7c0c46
48c3351
7c00570
047e07f
a342e27
ae50d7a
2ca51c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -33,7 +33,7 @@ function wp_api_template_access_controller( $args, $post_type ) { | |||||
/** | ||||||
* Adds the post classes to the REST API response. | ||||||
* | ||||||
* @param array $post The response object data. | ||||||
* @param array $post The response object data. | ||||||
* | ||||||
* @return array | ||||||
*/ | ||||||
|
@@ -157,3 +157,72 @@ function gutenberg_register_wp_rest_themes_template_directory_uri_field() { | |||||
} | ||||||
} | ||||||
add_action( 'rest_api_init', 'gutenberg_register_wp_rest_themes_template_directory_uri_field' ); | ||||||
|
||||||
/** | ||||||
* Preload theme and global styles paths to avoid flash of variation styles in post editor. | ||||||
* | ||||||
* @param array $paths REST API paths to preload. | ||||||
* @param WP_Block_Editor_Context $context Current block editor context. | ||||||
* @return array Filtered preload paths. | ||||||
*/ | ||||||
function gutenberg_block_editor_preload_paths_6_6( $paths, $context ) { | ||||||
if ( 'core/edit-post' === $context->name ) { | ||||||
$paths[] = '/wp/v2/global-styles/themes/' . get_stylesheet(); | ||||||
$paths[] = '/wp/v2/themes?context=edit&status=active'; | ||||||
$paths[] = '/wp/v2/global-styles/' . WP_Theme_JSON_Resolver_Gutenberg::get_user_global_styles_post_id() . '?context=edit'; | ||||||
} | ||||||
return $paths; | ||||||
} | ||||||
add_filter( 'block_editor_rest_api_preload_paths', 'gutenberg_block_editor_preload_paths_6_6', 10, 2 ); | ||||||
|
||||||
/** | ||||||
* Updates comment metadata from a REST API request. | ||||||
* | ||||||
* This function is hooked to the `rest_prepare_comment` filter and is responsible for updating the comment metadata based on the data provided in the REST API request. | ||||||
* | ||||||
* It performs the following tasks: | ||||||
* - Updates the `block_comment` metadata for the comment based on the `meta` field in the request. | ||||||
* - Updates the `comment_type` and `comment_approved` fields for the comment based on the corresponding fields in the request. | ||||||
* - Retrieves the author's avatar URLs and adds them to the response data. | ||||||
* - Retrieves the `block_comment` metadata for the comment and adds it to the response data. | ||||||
* | ||||||
* @param WP_REST_Response $response The response object. | ||||||
* @param WP_Comment $comment The comment object. | ||||||
* @param WP_REST_Request $request The request object. | ||||||
* @return WP_REST_Response The updated response object. | ||||||
*/ | ||||||
if ( ! function_exists( 'update_comment_meta_from_rest_request' ) ) { | ||||||
function update_comment_meta_from_rest_request( $response, $comment, $request ) { | ||||||
|
||||||
if ( isset( $request['comment_type'] ) && ! empty( $request['comment_type'] ) ) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
$comment_data = array( | ||||||
'comment_ID' => $comment->comment_ID, | ||||||
'comment_type' => $request['comment_type'], | ||||||
'comment_approved' => isset( $request['comment_approved'] ) ? $request['comment_approved'] : 0, | ||||||
); | ||||||
|
||||||
wp_update_comment( $comment_data ); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updating a comment in the database in the middle of a REST request to return a comment feels incorrect. Can we find a better place / time to do it? What's the real reason to perform the update in the first place? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The primary reason for updating the comment in the middle of a REST request is to accommodate the storage of a custom comment_type. Currently, the REST API does not support a parameter for passing this information with the request. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds like we're solving the problem the wrong way. Should we extend the endpoint to support this parameter instead? |
||||||
} | ||||||
|
||||||
$author = get_user_by( 'login', $response->data['author_name'] ); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it make more sense to retrieve the comment author by email instead of by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We did not receive an email address from Gutenberg when fetching the current user's data using To resolve this, we initially fetched the user's data by username. However, we have modified the code to retrieve user avatar URLs using the user ID instead. |
||||||
if ( $author ) { | ||||||
$avatar_url = get_avatar_url( $author->ID ); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we now fetch the avatar URL using the user ID, we have removed the get_user_by function from the code. |
||||||
$response->data['author_avatar_urls'] = array( | ||||||
'default' => $avatar_url, | ||||||
'48' => add_query_arg( 's', 48, $avatar_url ), | ||||||
'96' => add_query_arg( 's', 96, $avatar_url ), | ||||||
); | ||||||
} | ||||||
|
||||||
$comment_id = $comment->comment_ID; | ||||||
$meta_key = 'block_comment'; | ||||||
$meta_value = get_comment_meta( $comment_id, $meta_key, true ); | ||||||
|
||||||
if ( ! empty( $meta_value ) ) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The thing with using
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||
$response->data['meta'] = $meta_value; | ||||||
} | ||||||
|
||||||
return $response; | ||||||
} | ||||||
} | ||||||
add_filter( 'rest_prepare_comment', 'update_comment_meta_from_rest_request', 10, 3 ); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,43 @@ | ||||||||||||||||||||||||
<?php | ||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Functions to support collaboration. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @package gutenberg | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
if ( ! function_exists( 'register_post_meta_for_collab_comment' ) ) { | ||||||||||||||||||||||||
/** | ||||||||||||||||||||||||
* Registers the collab meta field required for comments to work. | ||||||||||||||||||||||||
* | ||||||||||||||||||||||||
* @since // TODO: Add version number. | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
function register_post_meta_for_collab_comment() { | ||||||||||||||||||||||||
$post_types = get_post_types( array( 'show_in_rest' => true ) ); | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
foreach ( $post_types as $post_type ) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might be doing a bunch of unnecessary iterations here. You could do it with a more advanced
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The documentation definitely needs a bit of love, but with |
||||||||||||||||||||||||
// Only register the meta field if the post type supports the editor, custom fields, and revisions. | ||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||
post_type_supports( $post_type, 'editor' ) && | ||||||||||||||||||||||||
post_type_supports( $post_type, 'custom-fields' ) && | ||||||||||||||||||||||||
post_type_supports( $post_type, 'revisions' ) | ||||||||||||||||||||||||
) { | ||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This check won't be necessary if we expand the |
||||||||||||||||||||||||
register_post_meta( | ||||||||||||||||||||||||
$post_type, | ||||||||||||||||||||||||
'collab', | ||||||||||||||||||||||||
array( | ||||||||||||||||||||||||
'show_in_rest' => true, | ||||||||||||||||||||||||
'single' => true, | ||||||||||||||||||||||||
'type' => 'string', | ||||||||||||||||||||||||
'revisions_enabled' => true, | ||||||||||||||||||||||||
) | ||||||||||||||||||||||||
); | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
} | ||||||||||||||||||||||||
|
||||||||||||||||||||||||
/* | ||||||||||||||||||||||||
* Most post types are registered at priority 10, so use priority 20 here in | ||||||||||||||||||||||||
* order to catch them. | ||||||||||||||||||||||||
*/ | ||||||||||||||||||||||||
add_action( 'init', 'register_post_meta_for_collab_comment', 20 ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,15 @@ function gutenberg_enable_experiments() { | |
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-zoomed-out-patterns-tab', $gutenberg_experiments ) ) { | ||
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnableZoomedOutPatternsTab = true', 'before' ); | ||
} | ||
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-block-comment', $gutenberg_experiments ) ) { | ||
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalEnableBlockComment = true', 'before' ); | ||
} | ||
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-quick-edit-dataviews', $gutenberg_experiments ) ) { | ||
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalQuickEditDataViews = true', 'before' ); | ||
} | ||
if ( $gutenberg_experiments && array_key_exists( 'gutenberg-block-bindings-ui', $gutenberg_experiments ) ) { | ||
wp_add_inline_script( 'wp-block-editor', 'window.__experimentalBlockBindingsUI = true', 'before' ); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we add these by accident / through a bad rebase? |
||
} | ||
|
||
add_action( 'admin_init', 'gutenberg_enable_experiments' ); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -151,6 +151,42 @@ function gutenberg_initialize_experiments_settings() { | |
) | ||
); | ||
|
||
add_settings_field( | ||
'gutenberg-quick-edit-dataviews', | ||
__( 'Quick Edit in DataViews', 'gutenberg' ), | ||
'gutenberg_display_experiment_field', | ||
'gutenberg-experiments', | ||
'gutenberg_experiments_section', | ||
array( | ||
'label' => __( 'Allow access to a quick edit panel in the pages data views.', 'gutenberg' ), | ||
'id' => 'gutenberg-quick-edit-dataviews', | ||
) | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this added by accident through a bad rebase or something? |
||
|
||
add_settings_field( | ||
'gutenberg-block-comment', | ||
__( 'Comments', 'gutenberg' ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this title can be more self-explanatory - "Comments" doesn't say enough. Perhaps "Block Comments" or "Collaborative Block Comments"? |
||
'gutenberg_display_experiment_field', | ||
'gutenberg-experiments', | ||
'gutenberg_experiments_section', | ||
array( | ||
'label' => __( 'Enable multi-user commenting on blocks', 'gutenberg' ), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should say something about being internal or for collaborators. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We’ve made this update based on the feedback provided here: #60622 (comment). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm happy to defer to Matías if he has suggestions. Perhaps: "Enable collaborative commenting on blocks."? |
||
'id' => 'gutenberg-block-comment', | ||
) | ||
); | ||
|
||
add_settings_field( | ||
'gutenberg-block-bindings-ui', | ||
__( 'UI to create block bindings', 'gutenberg' ), | ||
'gutenberg_display_experiment_field', | ||
'gutenberg-experiments', | ||
'gutenberg_experiments_section', | ||
array( | ||
'label' => __( 'Add UI to create and update block bindings in block inspector controls.', 'gutenberg' ), | ||
'id' => 'gutenberg-block-bindings-ui', | ||
) | ||
); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this added by accident through a bad rebase or something? |
||
register_setting( | ||
'gutenberg-experiments', | ||
'gutenberg-experiments' | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -43,6 +43,7 @@ | |
"@wordpress/commands": "file:../commands", | ||
"@wordpress/components": "file:../components", | ||
"@wordpress/compose": "file:../compose", | ||
"@wordpress/core-data": "file:../core-data", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think adding |
||
"@wordpress/data": "file:../data", | ||
"@wordpress/date": "file:../date", | ||
"@wordpress/deprecated": "file:../deprecated", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,17 @@ import { pipe, useCopyToClipboard } from '@wordpress/compose'; | |
* Internal dependencies | ||
*/ | ||
import BlockActions from '../block-actions'; | ||
import BlockCommentMenuItem from '../collab/block-comment-menu-item'; | ||
import BlockHTMLConvertButton from './block-html-convert-button'; | ||
import __unstableBlockSettingsMenuFirstItem from './block-settings-menu-first-item'; | ||
import BlockSettingsMenuControls from '../block-settings-menu-controls'; | ||
import BlockParentSelectorMenuItem from './block-parent-selector-menu-item'; | ||
import { store as blockEditorStore } from '../../store'; | ||
import { unlock } from '../../lock-unlock'; | ||
|
||
const isBlockCommentExperimentEnabled = | ||
window?.__experimentalEnableBlockComment; | ||
|
||
const POPOVER_PROPS = { | ||
className: 'block-editor-block-settings-menu__popover', | ||
placement: 'bottom-start', | ||
|
@@ -64,6 +68,7 @@ export function BlockSettingsDropdown( { | |
selectedBlockClientIds, | ||
openedBlockSettingsMenu, | ||
isContentOnly, | ||
blockCommentID, | ||
} = useSelect( | ||
( select ) => { | ||
const { | ||
|
@@ -84,6 +89,11 @@ export function BlockSettingsDropdown( { | |
const parentBlockName = | ||
_firstParentClientId && getBlockName( _firstParentClientId ); | ||
|
||
const commentID = | ||
// eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
select( 'core/block-editor' ).getBlock( firstBlockClientId ) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's use the store definition instead:
This applies to the rest of the instances throughout the PR. |
||
?.attributes?.blockCommentId; | ||
|
||
return { | ||
firstParentClientId: _firstParentClientId, | ||
onlyBlock: 1 === getBlockCount( _firstParentClientId ), | ||
|
@@ -100,6 +110,7 @@ export function BlockSettingsDropdown( { | |
openedBlockSettingsMenu: getOpenedBlockSettingsMenu(), | ||
isContentOnly: | ||
getBlockEditingMode( firstBlockClientId ) === 'contentOnly', | ||
blockCommentID: commentID, | ||
}; | ||
}, | ||
[ firstBlockClientId ] | ||
|
@@ -278,6 +289,13 @@ export function BlockSettingsDropdown( { | |
</MenuItem> | ||
</> | ||
) } | ||
{ isBlockCommentExperimentEnabled && | ||
! blockCommentID && ( | ||
<BlockCommentMenuItem | ||
clientId={ clientIds } | ||
onClose={ onClose } | ||
/> | ||
) } | ||
</MenuGroup> | ||
{ canCopyStyles && ! isContentOnly && ( | ||
<MenuGroup> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { MenuItem } from '@wordpress/components'; | ||
import { collabComment } from '@wordpress/icons'; | ||
import { useSelect, useDispatch } from '@wordpress/data'; | ||
|
||
export default function BlockCommentMenuItem( { onClose } ) { | ||
// eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
const { openGeneralSidebar } = useDispatch( 'core/edit-post' ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This looks concerning beyond using a string literal for the store name. The block-editor package is expected to be WordPress-agnostic, so accessing the edit post store doesn't make sense. This likely hints that this component should live in another package, closer to the post editor. This applies to multiple instances in that PR that attempt to use the edit post store in the block editor package. |
||
|
||
// eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
const { updateBlockAttributes } = useDispatch( 'core/block-editor' ); | ||
|
||
const clientId = useSelect( ( select ) => { | ||
// eslint-disable-next-line @wordpress/data-no-store-string-literals | ||
const { getSelectedBlockClientId } = select( 'core/block-editor' ); | ||
return getSelectedBlockClientId(); | ||
}, [] ); | ||
|
||
const openCollabBoard = () => { | ||
onClose(); | ||
updateBlockAttributes( clientId, { | ||
showCommentBoard: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sort of state should not live in block attributes. |
||
} ); | ||
openGeneralSidebar( 'edit-post/collab-sidebar' ); | ||
}; | ||
|
||
return ( | ||
<MenuItem | ||
icon={ collabComment } | ||
onClick={ openCollabBoard } | ||
aria-haspopup="dialog" | ||
> | ||
{ __( 'Add Comment' ) } | ||
</MenuItem> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll need to bump the version, since 6.6 has been released already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have copied the code to the WordPress 6.7 version folder.