Details

Reviewers

atul
tomek

Commits

rCOMM97994f80279d: [lib] modified stringForReactionList to handle more than 9 likes/reactions for…

Summary

modified stringForReactionList to handle more than 9 likes/reactions for the same reaction type. Made the decision to cap the number of likes at 9, and then everything after will display 9+. If we want to change this though to another number, please let me know This was suggested by @atul in D5953

Depends on D5953

Test Plan

ran yarn test reaction-utils.test.js with a new unit test that has 12 message likes and all tests passed

Screenshot 2022-12-21 at 11.27.08 AM.png (542×1 px, 441 KB)

Diff Detail

Repository

rCOMM Comm

Branch

eng-2243 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

ginsu created this revision.Dec 21 2022, 8:44 AM

Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptDec 21 2022, 8:44 AM

ginsu edited the summary of this revision. (Show Details)Dec 21 2022, 8:46 AM

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

ginsu added reviewers: atul, tomek.

ginsu added a parent revision: D5953: [lib] introduce unit tests for stringForReactionList reaction util function.

ginsu attached a referenced file: F298983: Screenshot 2022-12-21 at 11.27.08 AM.png. (Show Details)

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

ginsu mentioned this in D5953: [lib] introduce unit tests for stringForReactionList reaction util function.Dec 21 2022, 8:48 AM

Harbormaster completed remote builds in B14634: Diff 19957.Dec 21 2022, 8:56 AM

ginsu requested review of this revision.Dec 21 2022, 8:56 AM

tomek requested changes to this revision.Dec 21 2022, 8:59 AM

tomek added inline comments.

lib/shared/reaction-utils.js
18–21	What is a type of `numOfReacts`? We're comparing it to `9` but then assign a string - can we make sure that we don't mix the types?

This revision now requires changes to proceed.Dec 21 2022, 8:59 AM

address tomek's feedback

Harbormaster completed remote builds in B14639: Diff 19967.Dec 21 2022, 11:32 AM

Thanks for writing this test. Left suggestion to cut indentation and simplify things.

lib/shared/reaction-utils.js
17–24 ↗	(On Diff #19967)	We can simplify this a lot. How about something like: const { size: numberOfReacts } = reactionInfo.users; if (numberOfReacts <= 1) { continue; } reactionText.push(numberOfReacts > 9 ? '9+' : numberOfReacts.toString());