-
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
Refactor Latest Comments block to use function component #23557
Refactor Latest Comments block to use function component #23557
Conversation
<ToggleControl | ||
label={ __( 'Display avatar' ) } | ||
checked={ displayAvatar } | ||
onChange={ () => |
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.
I believe you could keep the generic toggle function, since all three toggle on change like:
const toggleAttribute = ( propName ) => () =>
setAttributes( { [ propName ]: ! attributes[ propName ] } );
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.
I'm not convinced it would make much of a difference, performance-wise.
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.
It's more about reusability and not performance.
displayAvatar, | ||
displayDate, | ||
displayExcerpt, | ||
} = attributes; |
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.
You can destructure attributes
in the above destructuring.
{ attributes, attributes: { commentsToshow, ....... } setAttributes }
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.
Actually, it would be { attributes: { commentsToShow, (etc) }, setAttributes }
. No need to duplicate attributes
; actually, I think that would cause a linting error.
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.
This looks good to merge. It just needs a rebase to get tests passing.
Description
Related to #22890.
Types of changes
Refactor LatestComments to use function component.
Checklist: