-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Selecting on a comment should scroll to the block #66873
base: trunk
Are you sure you want to change the base?
Changes from 13 commits
912ec5a
154c273
52341d1
aef8840
0c34d4e
dad8215
cd730bb
583632d
499cfd8
d71c1c6
07c85f6
a7693d4
8367a44
9d60b44
e5d379b
939f2f5
68568d9
13194c1
e9b0e4b
cc4c27a
de2c602
07453a5
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 |
---|---|---|
|
@@ -6,7 +6,7 @@ import clsx from 'clsx'; | |
/** | ||
* WordPress dependencies | ||
*/ | ||
import { useState, RawHTML } from '@wordpress/element'; | ||
import { useState, RawHTML, useEffect } from '@wordpress/element'; | ||
import { | ||
__experimentalHStack as HStack, | ||
__experimentalVStack as VStack, | ||
|
@@ -17,7 +17,7 @@ import { | |
} from '@wordpress/components'; | ||
import { Icon, check, published, moreVertical } from '@wordpress/icons'; | ||
import { __, _x } from '@wordpress/i18n'; | ||
import { useSelect } from '@wordpress/data'; | ||
import { useSelect, useDispatch } from '@wordpress/data'; | ||
import { store as blockEditorStore } from '@wordpress/block-editor'; | ||
|
||
/** | ||
|
@@ -35,7 +35,7 @@ import CommentForm from './comment-form'; | |
* @param {Function} props.onAddReply - The function to add a reply to a comment. | ||
* @param {Function} props.onCommentDelete - The function to delete a comment. | ||
* @param {Function} props.onCommentResolve - The function to mark a comment as resolved. | ||
* @return {React.ReactNode} The rendered Comments component. | ||
* @return {JSX.Element} The rendered Comments component. | ||
*/ | ||
export function Comments( { | ||
threads, | ||
|
@@ -46,6 +46,9 @@ export function Comments( { | |
} ) { | ||
const [ actionState, setActionState ] = useState( false ); | ||
const [ isConfirmDialogOpen, setIsConfirmDialogOpen ] = useState( false ); | ||
// eslint-disable-next-line no-unused-vars | ||
const [ activeClientId, setActiveClientId ] = useState( null ); | ||
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. Why is eslint disabled here? 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. @ellatrix - The above lines no longer needed and removed it. |
||
const [ blocksList, setBlocksList ] = useState( null ); | ||
|
||
const handleConfirmDelete = () => { | ||
onCommentDelete( actionState.id ); | ||
|
@@ -72,6 +75,58 @@ export function Comments( { | |
); | ||
}, [] ); | ||
|
||
const { selectedClientBlocks, selectedActiveClientId } = useSelect( | ||
( select ) => { | ||
const clientID = | ||
select( blockEditorStore ).getSelectedBlockClientId(); | ||
const selClientBlocks = select( blockEditorStore ).getBlocks(); | ||
ellatrix marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
const getBlockCommentId = | ||
select( blockEditorStore ).getBlock( clientID )?.attributes | ||
?.blockCommentId ?? false; | ||
|
||
return { | ||
selectedClientBlocks: selClientBlocks, | ||
selectedActiveClientId: getBlockCommentId || null, | ||
}; | ||
}, | ||
[] | ||
); | ||
|
||
useEffect( () => { | ||
setBlocksList( selectedClientBlocks ); | ||
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. Why the extra local state here? It's not necessary? 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 line 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.
Can you elaborate more about this or how to reproduce the issue? As far as I can see, there's no need to keep a copy of 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. |
||
|
||
if ( selectedActiveClientId ) { | ||
setActiveClientId( selectedActiveClientId ); | ||
} | ||
}, [ selectedClientBlocks, selectedActiveClientId ] ); | ||
|
||
const findBlockByCommentId = ( blocks, commentId ) => { | ||
for ( const block of blocks ) { | ||
if ( block.attributes.blockCommentId === commentId ) { | ||
return block; | ||
} | ||
if ( block.innerBlocks && block.innerBlocks.length > 0 ) { | ||
const foundBlock = findBlockByCommentId( | ||
block.innerBlocks, | ||
commentId | ||
); | ||
if ( foundBlock ) { | ||
return foundBlock; | ||
} | ||
} | ||
} | ||
return null; | ||
}; | ||
|
||
const { selectBlock } = useDispatch( blockEditorStore ); | ||
const handleThreadClick = ( thread ) => { | ||
const block = findBlockByCommentId( blocksList, thread.id ); | ||
if ( block ) { | ||
selectBlock( block.clientId ); // Use the action to select the block | ||
} | ||
}; | ||
|
||
const CommentBoard = ( { thread, parentThread } ) => { | ||
return ( | ||
<> | ||
|
@@ -202,6 +257,7 @@ export function Comments( { | |
) } | ||
id={ thread.id } | ||
spacing="3" | ||
onClick={ () => handleThreadClick( thread ) } | ||
> | ||
<CommentBoard thread={ thread } /> | ||
{ 0 < thread?.reply?.length && | ||
|
@@ -270,7 +326,7 @@ export function Comments( { | |
* @param {Function} props.onDelete - The function to delete the comment. | ||
* @param {Function} props.onReply - The function to reply to the comment. | ||
* @param {string} props.status - The status of the comment. | ||
* @return {React.ReactNode} The rendered comment header. | ||
* @return {JSX.Element} The rendered comment header. | ||
*/ | ||
function CommentHeader( { | ||
thread, | ||
|
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.
Why did this need to change? Just curious? What are we using elsewhere?
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.
@ellatrix - Actually it was updated automatically when running the prettier command. Now replaced it back.