Do not open comments sidebar for adding comments#72214
Do not open comments sidebar for adding comments#72214karthick-murugan wants to merge 9 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. |
|
Thanks, @karthick-murugan! Discussion on this enhancement is still ongoing. Please take a look at the original issue. |
|
Also, just noticed that this changes the behavior differently than requested in the issue. If you're unsure about the issue, please don't hesitate to ask for clarification of the requirements. |
d5e2d57 to
e2dab33
Compare
|
@t-hamano @Mamaduka @adamsilverstein Regarding this PR, I have tried few options mentioned below, but not able to achieve the exact flow needed.
Current issue: Question: Any guidance would be much appreciated! |
|
@karthick-murugan, try making the following change to trunk. It's not perfect, but this is part of what I expect. Detailsdiff --git a/packages/editor/src/components/collab-sidebar/index.js b/packages/editor/src/components/collab-sidebar/index.js
index 54fa35f1d1..db541b9fba 100644
--- a/packages/editor/src/components/collab-sidebar/index.js
+++ b/packages/editor/src/components/collab-sidebar/index.js
@@ -48,15 +48,14 @@ function CollabSidebarContent( {
role="list"
spacing="3"
ref={ commentSidebarRef }
+ justify="flex-start"
>
- { ! isFloating && (
- <AddComment
- onSubmit={ onCreate }
- showCommentBoard={ showCommentBoard }
- setShowCommentBoard={ setShowCommentBoard }
- commentSidebarRef={ commentSidebarRef }
- />
- ) }
+ <AddComment
+ onSubmit={ onCreate }
+ showCommentBoard={ showCommentBoard }
+ setShowCommentBoard={ setShowCommentBoard }
+ commentSidebarRef={ commentSidebarRef }
+ />
<Comments
threads={ comments }
onEditComment={ onEdit }
@@ -125,15 +124,11 @@ export default function CollabSidebar() {
}
async function openTheSidebar() {
- enableComplementaryArea( 'core', collabHistorySidebarName );
+ enableComplementaryArea( 'core', collabSidebarName );
const activeArea = await getActiveComplementaryArea( 'core' );
// Move focus to the target element after the sidebar has opened.
- if (
- [ collabHistorySidebarName, collabSidebarName ].includes(
- activeArea
- )
- ) {
+ if ( [ collabSidebarName, collabSidebarName ].includes( activeArea ) ) {
setShowCommentBoard( ! blockCommentId );
focusCommentThread(
blockCommentId,
@@ -169,7 +164,7 @@ export default function CollabSidebar() {
commentLastUpdated={ commentLastUpdated }
/>
</PluginSidebar>
- { isLargeViewport && unresolvedSortedThreads.length > 0 && (
+ { isLargeViewport && (
<PluginSidebar
isPinnable={ false }
header={ false }4abee06e620985670541421718a60451.mp4This means that clicking the comment button opens the floating sidebar instead of the history sidebar. This means that the archive sidebar will not open unless the user explicitly clicks the notes button in the header. @adamsilverstein To make this work perfectly, I think we need to improve the floating logic and calculate the position of the new comment form as well. What do you think? |
e2dab33 to
f4c312e
Compare
@t-hamano Updated. |
|
Yes, the positioning logic will need to be adjusted to make room for the new comment form. I can work on this after we merge the other PR that pushes up threads above the active thread. For this PR it's fine to leave the new comment form at the top. |
|
What happens when the user already has the "Notes" non-floating sidebar open and then clicks the "add note" menu item? Should the form be displayed in that sidebar? Closing it and rendering the form in canvas seems a bit wasteful. P.S. The failing e2e tests are related to the changes in this PR. |
|
Now that #72309 is merged. @karthick-murugan, do you mind rebasing on top of the latest trunk? |
* Time to Read: Don't use wp_word_count() function * Update PHP docblock Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: dmsnell <dmsnell@git.wordpress.org>
* Implement unique IDs for Interactivity API directives * test: Add comprehensive edge cases for unique IDs regex parsing * Improve unit tests * Get rid of contants and replace regexp with function * Move test and add invalid chars back * More refactoring * Rename * Destructure state to eliminate redundant store accesses * Simplify context merging and avoid repeated initializations * Reset data-wp-context * Add support for multiple default entries to context * Fix all warns in directives * Add context e2e tests * Add bind tests * Add failing class e2e test * Fix test * Add failing each e2e test * Fix test * Fix router and each * Minor fix to test * Add init e2e test * Add on e2e test * Add on-document e2e test * Add on-window e2e test * Add run e2e test * Add styles e2e test * Add text e2e test * Add watch e2e tests * Remove unique-id tests * Mini refactor parseDirectiveName * Remove await page.pause(); * Fix split limits * Use warn in warn functions * Fix directive comments and some warns * Add failing test * Fix test * Add changelog --------- Co-authored-by: samueljseay <samueljseay@git.wordpress.org> Co-authored-by: luisherranz <luisherranz@git.wordpress.org> Co-authored-by: DAreRodz <darerodz@git.wordpress.org> Co-authored-by: Sahil1617 <sahiljadhav1617@git.wordpress.org>
Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: juanfra <juanfra@git.wordpress.org>
9a4195f to
795e80a
Compare
795e80a to
4d751fa
Compare
|
It looks like after rebasing, the PR contains unrelated changes, while the actual fix code is missing. |
@Mamaduka - Yes there seems to be an issue in the rebase which has brought unrelated changes to this PR. I am trying to resolve, else I will create a new PR with latest trunk. Thank you. |
|
Closing this PR during unrelated file changes due to rebasing.Created a new PR #72431 |
What?
Closes part of #72157 ( Since we have comments in the canvas, don't open the block comments sidebar when adding one. The sidebar should be thought of as a historical archive of comments, one you open to see previous or resolved comments, not the primary interface for engaging with comments.)
Improves block comment UX by preventing sidebar from opening when adding comments.
Why?
The block comments sidebar opens when adding a new comment, but it should be thought of as a historical archive for existing comments, not the primary interface for engaging with comments.
How?
Changes made:
Modified
CollabSidebarContentinindex.js: Added conditional rendering to only show theCommentscomponent when not in "add comment" mode (!showCommentBoard). This ensures only the comment form is visible when adding comments, not existing comments.Updated
AddCommentcomponent inadd-comment.js: AddedsetShowCommentBoard(false)after successfully adding a comment to switch the canvas from "add comment" mode to "view comments" mode.Testing Instructions
Video
REC-20251009183126.mp4