[native] toggle reaction action in onPressReact function
ClosedPublic
Actions

Authored by ginsu on Dec 12 2022, 8:43 PM.

Details

Reviewers

atul
tomek
rohan

Commits

rCOMM906003d474a7: [native] toggle reaction action in onPressReact function

Summary

toggle reaction action in onPressReact function. Hard setting reactionInput right now but will changing this when we expand from just sending likes to more reactions.

Also, right now the tooltip option does not change from '👍', but will be addressing this in a subsequent diff

Linear Task: ENG-2243
Depends on D5854

Test Plan

Please watch the demo video to see how the toggle works:

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Dec 12 2022, 8:43 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 12 2022, 8:43 PM

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

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

ginsu added reviewers: atul, tomek, rohan.

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

Harbormaster completed remote builds in B14101: Diff 19301.Dec 12 2022, 8:55 PM

ginsu requested review of this revision.Dec 12 2022, 8:55 PM

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

I think that we have to introduce some animations so that the UI doesn't jump when a reaction is added / removed, but that will be complicated so it should be a separate task which we can prioritize later.

native/chat/reaction-message-utils.js
45 ↗	(On Diff #19301)	Should we use `reactionInput` here?

addressed feedback

In D5854#175269, @tomek wrote:

I think that we have to introduce some animations so that the UI doesn't jump when a reaction is added / removed, but that will be complicated so it should be a separate task which we can prioritize later.

Totally agree, some of the animations are a bit clunky and need to be fine-tuned, but I think it will be better to address that in a separate task for the future

Harbormaster completed remote builds in B14131: Diff 19331.Dec 13 2022, 9:41 AM

ginsu added a child revision: D5870: [lib] introduced notificationTexts function to reaction message spec.Dec 14 2022, 1:16 PM

atul accepted this revision.Dec 15 2022, 9:24 AM

This revision is now accepted and ready to land.Dec 15 2022, 9:24 AM

You should never reference a task without linking it. @ginsu, please create and link the task you're describing before landing this diff

ginsu removed a child revision: D5870: [lib] introduced notificationTexts function to reaction message spec.Dec 15 2022, 1:39 PM

In D5854#175378, @ginsu wrote:

In D5854#175269, @tomek wrote:

I think that we have to introduce some animations so that the UI doesn't jump when a reaction is added / removed, but that will be complicated so it should be a separate task which we can prioritize later.

Totally agree, some of the animations are a bit clunky and need to be fine-tuned, but I think it will be better to address that in a separate task for the future