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

Inline Commenting: Added new sidebar as extension of the canvas #67347

Merged
merged 6 commits into from
Dec 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions packages/editor/src/components/collab-sidebar/constants.js
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export const collabHistorySidebarName = 'edit-post/collab-history-sidebar';
export const collabSidebarName = 'edit-post/collab-sidebar';
143 changes: 98 additions & 45 deletions packages/editor/src/components/collab-sidebar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { useSelect, useDispatch, resolveSelect } from '@wordpress/data';
import {
useSelect,
useDispatch,
resolveSelect,
subscribe,
} from '@wordpress/data';
import { useState, useMemo } from '@wordpress/element';
import { comment as commentIcon } from '@wordpress/icons';
import { addFilter } from '@wordpress/hooks';
Expand All @@ -15,12 +20,13 @@ import { store as interfaceStore } from '@wordpress/interface';
* Internal dependencies
*/
import PluginSidebar from '../plugin-sidebar';
import { collabSidebarName } from './constants';
import { collabHistorySidebarName, collabSidebarName } from './constants';
import { Comments } from './comments';
import { AddComment } from './add-comment';
import { store as editorStore } from '../../store';
import AddCommentButton from './comment-button';
import AddCommentToolbarButton from './comment-button-toolbar';
import { useGlobalStylesContext } from '../global-styles-provider';

const isBlockCommentExperimentEnabled =
window?.__experimentalEnableBlockComment;
Expand All @@ -44,61 +50,28 @@ addFilter(
modifyBlockCommentAttributes
);

function CollabSidebarContent( { showCommentBoard, setShowCommentBoard } ) {
function CollabSidebarContent( {
showCommentBoard,
setShowCommentBoard,
styles,
comments,
} ) {
const { createNotice } = useDispatch( noticesStore );
const { saveEntityRecord, deleteEntityRecord } = useDispatch( coreStore );
const { getEntityRecord } = resolveSelect( coreStore );

const { postId, threads } = useSelect( ( select ) => {
const { postId } = useSelect( ( select ) => {
const { getCurrentPostId } = select( editorStore );
const _postId = getCurrentPostId();
const data = !! _postId
? select( coreStore ).getEntityRecords( 'root', 'comment', {
post: _postId,
type: 'block_comment',
status: 'any',
per_page: 100,
} )
: null;

return {
postId: _postId,
threads: data,
};
}, [] );

const { getSelectedBlockClientId } = useSelect( blockEditorStore );
const { updateBlockAttributes } = useDispatch( blockEditorStore );

// Process comments to build the tree structure
const resultComments = useMemo( () => {
// Create a compare to store the references to all objects by id
const compare = {};
const result = [];

const filteredComments = ( threads ?? [] ).filter(
( comment ) => comment.status !== 'trash'
);

// Initialize each object with an empty `reply` array
filteredComments.forEach( ( item ) => {
compare[ item.id ] = { ...item, reply: [] };
} );

// Iterate over the data to build the tree structure
filteredComments.forEach( ( item ) => {
if ( item.parent === 0 ) {
// If parent is 0, it's a root item, push it to the result array
result.push( compare[ item.id ] );
} else if ( compare[ item.parent ] ) {
// Otherwise, find its parent and push it to the parent's `reply` array
compare[ item.parent ].reply.push( compare[ item.id ] );
}
} );

return result;
}, [ threads ] );

// Function to save the comment.
const addNewComment = async ( comment, parentCommentId ) => {
const args = {
Expand Down Expand Up @@ -222,14 +195,14 @@ function CollabSidebarContent( { showCommentBoard, setShowCommentBoard } ) {
};

return (
<div className="editor-collab-sidebar-panel">
<div className="editor-collab-sidebar-panel" style={ styles }>
<AddComment
onSubmit={ addNewComment }
showCommentBoard={ showCommentBoard }
setShowCommentBoard={ setShowCommentBoard }
/>
<Comments
threads={ resultComments }
threads={ comments }
onEditComment={ onEditComment }
onAddReply={ addNewComment }
onCommentDelete={ onCommentDelete }
Expand All @@ -245,6 +218,7 @@ function CollabSidebarContent( { showCommentBoard, setShowCommentBoard } ) {
export default function CollabSidebar() {
const [ showCommentBoard, setShowCommentBoard ] = useState( false );
const { enableComplementaryArea } = useDispatch( interfaceStore );
const { getActiveComplementaryArea } = useSelect( interfaceStore );

const { postStatus } = useSelect( ( select ) => {
return {
Expand All @@ -270,6 +244,68 @@ export default function CollabSidebar() {
enableComplementaryArea( 'core', 'edit-post/collab-sidebar' );
};

const { threads } = useSelect( ( select ) => {
const { getCurrentPostId } = select( editorStore );
const _postId = getCurrentPostId();
const data = !! _postId
? select( coreStore ).getEntityRecords( 'root', 'comment', {
post: _postId,
type: 'block_comment',
status: 'any',
per_page: 100,
} )
: null;

return {
postId: _postId,
threads: data,
};
}, [] );
Comment on lines +247 to +263
Copy link
Member

Choose a reason for hiding this comment

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

Moving the selector in the parent component re-introduces the performance issue fixed in #66670. We should be careful and avoid introducing similar regressions in the future.

P.S. I'm working on a fix and will create a PR shortly.

Copy link
Member Author

@akasunil akasunil Dec 4, 2024

Choose a reason for hiding this comment

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

Sorry, I didn't notice this. We now have two sidebars with separate comment orders, thus I had to transfer selector to the parent component.


// Process comments to build the tree structure
const resultComments = useMemo( () => {
// Create a compare to store the references to all objects by id
const compare = {};
const result = [];

const filteredComments = ( threads ?? [] ).filter(
( comment ) => comment.status !== 'trash'
);

// Initialize each object with an empty `reply` array
filteredComments.forEach( ( item ) => {
compare[ item.id ] = { ...item, reply: [] };
} );

// Iterate over the data to build the tree structure
filteredComments.forEach( ( item ) => {
if ( item.parent === 0 ) {
// If parent is 0, it's a root item, push it to the result array
result.push( compare[ item.id ] );
} else if ( compare[ item.parent ] ) {
// Otherwise, find its parent and push it to the parent's `reply` array
compare[ item.parent ].reply.push( compare[ item.id ] );
}
} );

return result;
}, [ threads ] );

// Get the global styles to set the background color of the sidebar.
const { merged: GlobalStyles } = useGlobalStylesContext();
const backgroundColor = GlobalStyles?.styles?.color?.background;

if ( 0 < resultComments.length ) {
const unsubscribe = subscribe( () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this to an effect? We shouldn't subscribe on render.

Copy link
Member Author

Choose a reason for hiding this comment

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

useEffect won't work in this case, as the component doesn't re-render when another sidebar is toggled from outside the editor.

const activeSidebar = getActiveComplementaryArea( 'core' );

if ( ! activeSidebar ) {
enableComplementaryArea( 'core', collabSidebarName );
unsubscribe();
}
} );
Copy link
Member

Choose a reason for hiding this comment

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

We could probably create an option for PluginSidebar to use the sidebar as a fallback sidebar that is always open instead of this, but it's fine as a temporary solution.

}

// Check if the experimental flag is enabled.
if ( ! isBlockCommentExperimentEnabled || postStatus === 'publish' ) {
return null; // or maybe return some message indicating no threads are available.
Expand All @@ -283,14 +319,31 @@ export default function CollabSidebar() {
<>
<AddCommentComponent onClick={ openCollabBoard } />
<PluginSidebar
identifier={ collabSidebarName }
identifier={ collabHistorySidebarName }
// translators: Comments sidebar title
title={ __( 'Comments' ) }
icon={ commentIcon }
>
<CollabSidebarContent
comments={ resultComments }
showCommentBoard={ showCommentBoard }
setShowCommentBoard={ setShowCommentBoard }
/>
</PluginSidebar>
<PluginSidebar
isPinnable={ false }
header={ false }
identifier={ collabSidebarName }
className="editor-collab-sidebar"
headerClassName="editor-collab-sidebar__header"
>
<CollabSidebarContent
comments={ resultComments }
showCommentBoard={ showCommentBoard }
setShowCommentBoard={ setShowCommentBoard }
styles={ {
backgroundColor,
} }
/>
</PluginSidebar>
</>
Expand Down
13 changes: 13 additions & 0 deletions packages/editor/src/components/collab-sidebar/style.scss
Original file line number Diff line number Diff line change
@@ -1,5 +1,18 @@
.interface-interface-skeleton__sidebar:has(.editor-collab-sidebar) {
box-shadow: none;

.interface-complementary-area-header {
display: none;
}
}

.editor-collab-sidebar {
height: 100%;
}

.editor-collab-sidebar-panel {
padding: $grid-unit-20;
height: 100%;

&__thread {
position: relative;
Expand Down
Loading