Skip to content
This repository has been archived by the owner on Oct 10, 2024. It is now read-only.

Add Reaction content type #9

Merged
merged 3 commits into from
Jun 22, 2023
Merged

Add Reaction content type #9

merged 3 commits into from
Jun 22, 2023

Conversation

rygine
Copy link
Collaborator

@rygine rygine commented Jun 20, 2023

This PR adds a new Reaction content type, and upgrades @xmtp/xmtp-js for the existing type.

@rygine rygine requested a review from jhaaaa as a code owner June 20, 2023 22:20
@rygine rygine requested a review from a team June 20, 2023 22:20
@changeset-bot
Copy link

changeset-bot bot commented Jun 20, 2023 •
edited
Loading

⚠️ No Changeset found

Latest commit: c00f455

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@rygine
Copy link
Collaborator Author

rygine commented Jun 20, 2023

@jhaaaa please take a look at the new README. there are 2 spots that have ... where i wasn't sure what to add.

rygine
rygine commented Jun 20, 2023
View reviewed changes
packages/content-type-reaction/README.md

### Display the reaction

Generally, reactions should be interpreted as emoji. So, `smile` would translate to :smile: in UI clients. That being said, how you ultimately choose to render a reaction is up to you.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i'm wondering how much direction we should give here. since a reaction can be anything, it might be worth it to present the primary use case as emoji while pointing developers to some sort of standard for translating strings to emoji.

@nakajima @neekolas

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this seems fine to me, I'd rather be less prescriptive here and just be able to show good examples.

@rygine rygine merged commit 1c29cc9 into main Jun 22, 2023
This was referenced Jul 14, 2023
Closed
Closed
Closed
Closed
@rygine rygine deleted the rygine/add-reaction-type branch January 16, 2024 20:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@nakajima nakajima nakajima approved these changes

@jhaaaa jhaaaa Awaiting requested review from jhaaaa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rygine @nakajima