Inline Commenting: Re-order the comments in sidebar in which blocks are listed #66927
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akasunil
added
[Type] Enhancement
|
Size Change: +179 B (+0.01%) Total Size: 1.83 MB
ℹ️ View Unchanged
|
ellatrix
reviewed
Nov 18, 2024
| @@ -96,7 +97,20 @@ function CollabSidebarContent( { showCommentBoard, setShowCommentBoard } ) { | |||
| } | |||
| } ); | |||
|
|
|||
| return result; | |||
| const blockCommentIds = getBlocks() | |||
There was a problem hiding this comment.
Will this work for comments in posts with a template, such as Site Editor > Pages?
There was a problem hiding this comment.
I have updated PR, it should be working fine now on post/page with template.
|
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. |
…nt/inline-commenting-comments-order
|
Flaky tests detected in 5574c5e. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11910868493
|
|
Based on the new design of #66377, we will need to show these comments at the height of the blocks. And for the secondary sidebar, we'll want to keep the chronological order. Maybe the sidebar with this new order needs to be a duplicated sidebar? |
That is being worked on; we can position comments at block height, but we also need to sync the editor scroll with the sidebar. There's where I got stuck. Will handle that in a separate PR.
Yes, you are correct. This PR is for a sidebar containing comments in the order that the blocks appear. Another sidebar will have comments displayed chronologically, Where i think we include Resolved comments too. |
|
I have synced PR with latest code and updated PR description, Could use your review here @ellatrix |
ellatrix
reviewed
Dec 3, 2024
| id: postId, | ||
| } ); | ||
|
|
||
| const getCommentIdsFromBlocks = () => { |
There was a problem hiding this comment.
Also, can't this function be moved out of the component? It doesn't seem to depend on other variables.
ellatrix
reviewed
Dec 3, 2024
| @@ -262,8 +265,37 @@ export default function CollabSidebar() { | |||
| }; | |||
| }, [] ); | |||
|
|
|||
| const [ blocks ] = useEntityBlockEditor( 'postType', postType, { | |||
| id: postId, | |||
There was a problem hiding this comment.
Can there be a case where there is no current post ID?
There was a problem hiding this comment.
I think it does, Ill put condition there to avoid errors.
There was a problem hiding this comment.
After some finding, Undefined postId is already being handled under useEntityBlockEditor for cases like templates and patterns.
ellatrix
reviewed
Dec 3, 2024
| return result; | ||
| const blockCommentIds = getCommentIdsFromBlocks(); | ||
|
|
||
| const uniqueIds = [ ...new Set( blockCommentIds.values() ) ]; |
There was a problem hiding this comment.
Is the values call really needed her? Set accepts the array?
There was a problem hiding this comment.
Also, if you need unique IDs, maybe getCommentIdsFromBlocks can already check if it's been added before?
ellatrix
reviewed
Dec 3, 2024
| const uniqueIds = [ ...new Set( blockCommentIds.values() ) ]; | ||
|
|
||
| const threadIdMap = new Map( | ||
| result?.map( ( thread ) => [ thread.id, thread ] ) |
There was a problem hiding this comment.
If it's possible for result to not be an array, we should probably just return early?
ellatrix
approved these changes
Dec 3, 2024
ellatrix
reviewed
Dec 3, 2024
| // Process comments to build the tree structure | ||
| const resultComments = useMemo( () => { | ||
| const { resultComments, sortedThreads } = useMemo( () => { | ||
| // Create a compare to store the references to all objects by id | ||
| const compare = {}; | ||
| const result = []; |
There was a problem hiding this comment.
Isn't this always an array? Why guard with result??
ellatrix
reviewed
Dec 3, 2024
| ); | ||
|
|
||
| const sortedComments = blockCommentIds | ||
| .map( ( id ) => threadIdMap.get( id ) ) |
There was a problem hiding this comment.
Btw, I see above that a thread can be { ...item, reply: [] }, which will cause this component to constantly re-render.
There was a problem hiding this comment.
I don't follow, we are re-arranging comments here so reply comments will go under the parent comment.
There was a problem hiding this comment.
Sorry, I missed that this is within useMemo instead of useSelect
…nt/inline-commenting-comments-order
im3dabasia
pushed a commit
to im3dabasia/gutenberg
that referenced
this pull request
Dec 4, 2024
…re listed (WordPress#66927) Co-authored-by: akasunil <sunil25393@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
…re listed (#66927) Co-authored-by: akasunil <sunil25393@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Part of - #66377
What? & Why?
In inline commenting functionality, Comments are loaded randomly in comments sidebar. Comments should be listed in order in which blocks are listed in editor. It helps navigate to the specific block comment.
How?
This PR re-order the comments in which blocks are appear in editor only in sidebar that present in canvas. History sidebar will have comments in chronological order. See attached video for more info.
Testing Instructions
Gutenberg->ExperimentsScreenshots or screencast
Edit-Post-.Hello-World-.-.-gutenberg-.-WordPress.5.webm