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 implemenation of the filter buttons at the top of the design since every message will be a like for the moment
Details
Details
- Reviewers
atul tomek ashoat - Commits
- rCOMM2d9f99f71578: [web/lib] introduce MessageReactionsModal
Please watch the demo to see how MessageReactionsModal works
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/modals/chat/message-reactions-modal.css | ||
|---|---|---|
| 8 ↗ | (On Diff #21054) | |
| web/modals/chat/message-reactions-modal.react.js | ||
| 19 ↗ | (On Diff #21054) | This name is misleading. The value is not only a collection of users because it also contains a reaction. Also, a user can appear multiple times in it. |
| 22–24 ↗ | (On Diff #21054) | Maybe it is better to iterate over entries so that we don't need to use an invariant? |
| 26 ↗ | (On Diff #21054) | It isn't a good idea to just map the values without sorting. It seems like a new reaction might result in all the users being reshuffled. Can we avoid this? |
| 29–36 ↗ | (On Diff #21054) | |
| web/modals/chat/message-reactions-modal.react.js | ||
|---|---|---|
| 26 ↗ | (On Diff #21054) | This is a good point. I wrote a small sorting algorithm to address your comment, but having a user who reacted to the same message with 2 or more reactions and how we display that in this modal is probably something we want to rethink in the UI design |
| web/modals/chat/message-reactions-modal.react.js | ||
|---|---|---|
| 25–32 ↗ | (On Diff #21101) | |
| web/modals/chat/message-reactions-modal.react.js | ||
|---|---|---|
| 26 ↗ | (On Diff #21054) | Yeah, definitely. I'd prefer having the entries first sorted by reaction and then by user. Even if we decide to keep the current approach, we still should sort by the notification as the second step. Also, I think it would be more intuitive for users if we were sorting by usernames (stringForUserExplicit?) instead of the ids. |
| web/modals/chat/message-reactions-modal.react.js | ||
|---|---|---|
| 32 ↗ | (On Diff #21205) | This seems pretty complicated, can we simplify with Lodash _sortBy? We could pass it two predicates, one for number of reactions and one for username |
| web/modals/chat/message-reactions-modal.react.js | ||
|---|---|---|
| 42 ↗ | (On Diff #21227) | To reduce indentation and improve readability, I would assign this function to a variable named eg. sortByNumReactions |