Page MenuHomePhabricator

[native] introduced react option to the message tooltip
ClosedPublic

Authored by ginsu on Nov 30 2022, 4:56 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:15 PM
Unknown Object (File)
Fri, Nov 8, 4:51 PM
Unknown Object (File)
Wed, Nov 6, 7:07 PM
Unknown Object (File)
Wed, Nov 6, 7:07 PM
Unknown Object (File)
Tue, Nov 5, 7:21 AM
Subscribers

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

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

I think that allowing reactions to robotext makes sense:

  1. We allow making a sidebar from it, so allowing reactions seems consistent.
  2. 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?

atul accepted this revision.EditedDec 1 2022, 3:09 PM

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

added reacting to robotext messages

In D5783#172966, @atul wrote:

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

Just updated diff to handle robotext!