Skip to content
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

Reactions #1

Merged
merged 11 commits into from Jul 14, 2023
Merged

Reactions #1

merged 11 commits into from Jul 14, 2023

Conversation

nakajima
Copy link
Contributor

Pulling in @rygine's work from xmtp/xmtp-js#402, I started adding reactions here. We can delete the copy/pasted content types once they land in the content types repo.

Note: There's still some race conditions here that we'll need to iron out.

Screen.Recording.2023-06-20.at.5.18.19.PM.mov

@rygine rygine marked this pull request as ready for review July 14, 2023 15:46
@rygine rygine requested a review from a team as a code owner July 14, 2023 15:46

type ReactionString = keyof typeof defaultReactions;
type ReactionEmoji = (typeof defaultReactions)[ReactionString];
type ReactionEntries = [ReactionString, ReactionEmoji][];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did some type-fu here so typescript is happier

}
)}

{!isFullyReacted ? (
Copy link
Contributor

@rygine rygine Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we've reacted with every available emoji, don't display anything else

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this stateful per person in the chat? Like only do I not see it if I've react with all of them but others in the chat would still see them?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, this is a per person, per message state.

</button>
);
})
.filter(Boolean)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this filters out emoji that have already been reacted with

) : (
<button
className="text-xs text-zinc-500"
onClick={() => setIsReacting((prev) => !prev)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor update here to prevent a possible rare edge case where the committed react state may not represent the actual state

Copy link
Contributor

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link
Contributor Author

@nakajima nakajima left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a tiny change to get rid of the icon for the initial reaction button to bring it more inline with the reply button:
Screenshot 2023-07-14 at 10 32 20 AM

src/hooks/useMessages.tsx Show resolved Hide resolved
src/views/MessageRepliesView.tsx Outdated Show resolved Hide resolved
Co-authored-by: Pat Nakajima <patnakajima@gmail.com>
@rygine rygine merged commit 22a8ff6 into main Jul 14, 2023
@rygine rygine deleted the reactions branch July 14, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants