[native] introduce MessageReactionsModal
ClosedPublic
Actions

Authored by ginsu on Jan 23 2023, 10:01 AM.

Details

Reviewers

atul
tomek

Commits

rCOMM4f331467faf7: [native] introduce MessageReactionsModal

Summary

introduce MessageReactionsModal. MessageReactionsModal shows all the users who have reacted to a message, what their username is and what reaction they used. I decided to punt the implementation of the filter buttons at the top of the design since every message will be a like for the moment

Depends on D6348
Linear Task: ENG-2719

Test Plan

Please watch the demo video to see how MessageReactionsModal works

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Jan 23 2023, 10:01 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 23 2023, 10:01 AM

ginsu edited the summary of this revision. (Show Details)Jan 23 2023, 10:09 AM

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

ginsu added reviewers: atul, tomek.

ginsu attached a referenced file: F340105: Screen Recording 2023-01-23 at 1.08.58 PM.mov. (Show Details)

Harbormaster completed remote builds in B15550: Diff 21211.Jan 23 2023, 10:16 AM

ginsu requested review of this revision.Jan 23 2023, 10:16 AM

ginsu edited the summary of this revision. (Show Details)Jan 23 2023, 10:19 AM

ginsu added a parent revision: D6348: [web/lib] put logic for creating message reaction list in reaction utils.

ginsu added a child revision: D6351: [native] implement new InlineEngagement design.Jan 23 2023, 10:25 AM

Overall looks super clean!

Two suggestions inline, let's definitely do the the memoization one before landing.

native/chat/message-reactions-modal.react.js
62 ↗	(On Diff #21211)	Let's memoize this... otherwise the object (in this case an array) will get recreated every time
86–88 ↗	(On Diff #21211)	Instead of all of these (and `marginTop` on line 90) can we just do margin: 0, ?

This revision is now accepted and ready to land.Jan 24 2023, 11:20 AM

ginsu added inline comments.Jan 24 2023, 12:50 PM

native/chat/message-reactions-modal.react.js
86–88 ↗	(On Diff #21211)	Unfortunately, we do need to set each margin property individually since the modal component sets them individually as well, and margin: 0, won't override those properties in the modal component

atul added inline comments.Jan 24 2023, 12:51 PM

native/chat/message-reactions-modal.react.js
86–88 ↗	(On Diff #21211)	Ah gotcha... could you group `marginTop` with these threes and add a comment saying we need to list them out explicitly to override?