Details

Reviewers

atul
tomek
rohan

Commits

rCOMM50cad2810848: [web] introduce reactions to inline sidebar

Summary

introduce reactions to inline sidebar. Right now, going with a temporary solution of just showing the number of likes right now, but in a subsequent diff will be extending this to show more information about the reaction message

Depends on D5812

Linear Task: ENG-2409

Test Plan

Please see the screenshot to see what "message likes" look like:

Screenshot 2022-12-05 at 9.03.09 PM.png (1×3 px, 835 KB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Dec 5 2022, 5:42 PM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 5 2022, 5:42 PM

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

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

ginsu added reviewers: atul, tomek, rohan.

Harbormaster completed remote builds in B13981: Diff 19154.Dec 5 2022, 5:54 PM

ginsu requested review of this revision.Dec 5 2022, 5:54 PM

add space between reaction and number

ginsu edited the test plan for this revision. (Show Details)Dec 5 2022, 6:03 PM

ginsu attached a referenced file: F272466: Screenshot 2022-12-05 at 9.03.09 PM.png. (Show Details)

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

ginsu added a parent revision: D5812: [native] introduced reactions to chat item height measurer.

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

Harbormaster completed remote builds in B13982: Diff 19155.Dec 5 2022, 6:14 PM

ashoat added inline comments.Dec 6 2022, 6:39 AM

web/chat/inline-sidebar.react.js
19 ↗	(On Diff #19155)	At some point we should rename this to `InlineEngagement` to better reflect that it includes reactions

tomek requested changes to this revision.Dec 6 2022, 7:05 AM

tomek added inline comments.

web/chat/inline-sidebar.react.js
33–39 ↗	(On Diff #19155)	The same issues like in D5813. Also we might consider extracting common logic to lib, but it doesn't seem to be mandatory now.

This revision now requires changes to proceed.Dec 6 2022, 7:05 AM

addressed feedback

Harbormaster completed remote builds in B14100: Diff 19300.Dec 12 2022, 8:50 PM

tomek accepted this revision.Dec 13 2022, 4:42 AM

tomek added inline comments.

web/chat/inline-sidebar.react.js
17 ↗	(On Diff #19300)	`$ReadOnlyMap`
39–47 ↗	(On Diff #19300)	Just like in D5813 we will need to take care about sorting and spaces between reactions
49–51 ↗	(On Diff #19300)	What's the reason behind having one div in another. Can we merge these? Or maybe, each reaction could be its own div, just like it was in the original version.