[native] introduced react option to the message tooltip
ClosedPublic
Actions

Authored by ginsu on Nov 30 2022, 4:56 PM.

Details

Reviewers

atul
tomek
rohan

Commits

rCOMM19ddb3a20e97: [native] introduced react option to the message tooltip

Summary

This diff won’t be landed until we are ready to flip the switch on message liking

introduced the react option to the text message, robotext, and the multimedia message tooltips. The react option will also only render if canCreateReactionFromMessage is true. canCreateReactionFromMessage was introduced in D5853

Depends on D5853 and D5782

Linear Task: ENG-2246

Test Plan

Please watch the demo below to see how the react option looks like:

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Nov 30 2022, 4:56 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptNov 30 2022, 4:56 PM

ginsu edited the summary of this revision. (Show Details)Nov 30 2022, 4:57 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added reviewers: atul, tomek, rohan.

ginsu added a parent revision: D5780: [native] introduce onPressReact to reaction message utils.

ginsu attached a referenced file: F267740: Screen Recording 2022-11-30 at 7.41.13 PM.mov. (Show Details)

Harbormaster completed remote builds in B13906: Diff 19058.Nov 30 2022, 5:08 PM

ginsu requested review of this revision.Nov 30 2022, 5:08 PM

ginsu edited the summary of this revision. (Show Details)Nov 30 2022, 9:59 PM

It seems like it will be really hard for users to discover this action.

I think that allowing reactions to robotext makes sense:

We allow making a sidebar from it, so allowing reactions seems consistent.
People might want to express e.g. that they're happy that someone joined a chat.

Agree that it should be possible to react to robotext messages

Personally wouldn't expect to be able to react to robotext messages based on what I've experienced in other messaging apps....

However, I think we should defer that if possible because we'd need to change things on the front-end to accommodate displaying reactions

Happy to defer for now, you're right that we don't exactly have designs for it. That said not sure how bad it would be to just use the existing designs?

That's fair... I guess like the "multiple reactions" thing it would be best to design things as flexibly as possible, and then later make the product decision about whether to toggle things on or not.

However, this diff looks good as-is so I don't want to block it.

I decided not to add it to the robotext message because I feel like users should not be able to react to those types of message, but if we want to change that it will be a very easy fix.

@ginsu can we put up a separate diff to handle robotext?

This revision is now accepted and ready to land.Dec 1 2022, 3:09 PM

ginsu edited the summary of this revision. (Show Details)Dec 5 2022, 12:11 PM

ginsu edited the summary of this revision. (Show Details)

ginsu added a parent revision: D5782: [lib] introduce sendReactionMessage success case to message reducers.

added reacting to robotext messages

ginsu edited the summary of this revision. (Show Details)Dec 12 2022, 1:39 PM

ginsu edited the test plan for this revision. (Show Details)

ginsu added a parent revision: D5853: [native] introduced useCanCreateReactionFromMessage function to reaction message utils.

ginsu attached a referenced file: F281449: Screen Recording 2022-12-12 at 4.38.13 PM.mov. (Show Details)

ginsu edited the summary of this revision. (Show Details)