-
Notifications
You must be signed in to change notification settings - Fork 37.4k
Skip creating reactions container when no reactions or handler exist #286711
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
Conversation
…is available Co-authored-by: alexr00 <[email protected]>
| if (reactions.length === 0 && !hasReactionHandler) { | ||
| return; | ||
| } |
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.
maybe we shall create the container even if there is not reaction, but if there is a reaction handler
This way, adding the first reaction would still be possible
| if (reactions.length === 0 && !hasReactionHandler) { | |
| return; | |
| } | |
| if (!hasReactionHandler) { | |
| return; | |
| } |
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.
The proposed change in this file already handles that case.
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.
my bad, but the comment does not match (it says "or there is")
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.
Pull request overview
This PR eliminates wasted vertical space (25px) in comment threads when no reactions exist and no reaction handler is configured. The fix adds an early return check in the createReactionsContainer method to skip creating the reactions DOM container entirely when it's not needed.
- Skip creating the reactions container when there are no reactions to display and no reaction handler is available
- Reuse the filtered reactions array that was previously computed inline
Comment threads reserved 25px of vertical space for reactions UI even when comments had no reactions and no reaction handler was configured.
Changes
The
createReactionsContainermethod now checks upfront whether the container is needed:Without the DOM element, the CSS
min-height: 25pxrule doesn't apply, eliminating wasted space when reactions aren't in use.Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.