-
Notifications
You must be signed in to change notification settings - Fork 290
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
feat: Reactions UI Improvements #16433
Conversation
qualifiedIds={users} | ||
conversationUsers={conversationUsers} |
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.
Since we're passing all the conversation users anyway, would that make sense to filter out those that have not reacted at this point? And instead of qualifiedIds
pass reactionUsers
list of user entitties? 🤔
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #16433 +/- ##
==========================================
- Coverage 45.53% 45.49% -0.04%
==========================================
Files 735 736 +1
Lines 24021 24110 +89
Branches 5455 5487 +32
==========================================
+ Hits 10938 10970 +32
- Misses 11686 11734 +48
- Partials 1397 1406 +9 |
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.
Looks good 👌
@@ -63,32 +67,67 @@ export const EmojiPill: FC<EmojiPillProps> = ({ | |||
index, | |||
emojiListCount, | |||
hasUserReacted, | |||
reactedUsers, |
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.
reactedUsers, | |
reactingUsers, |
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.
@@ -48,9 +49,13 @@ const MessageReactionsList: FC<MessageReactionsListProps> = ({reactions, ...prop | |||
const emojiListCount = users.length; | |||
const hasUserReacted = users.some(user => matchQualifiedIds(selfUserId, user)); | |||
|
|||
const reactedUsers = users |
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.
const reactedUsers = users | |
const reactingUsers = users |
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.
transition-delay: @animation-timing-faster + 0.05; | ||
} | ||
} | ||
|
||
// MESSAGE - ACTIONS | ||
.message-body-actions { |
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.
is this class still used?
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.
Yes
<span key={text} onClick={onTooltipReactionCountClick} css={messageReactionButtonTooltipTextLink}> | ||
{text} | ||
</span> |
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.
could we make this a button?
https://wearezeta.atlassian.net/browse/WPB-5369
Description
Screenshots/Screencast (for UI changes)
Checklist
Important details for the reviewers
(Delete this section if unnecessary)