Page MenuHomePhabricator

[lib] modified stringForReactionList to handle more than 9 likes/reactions for the same reaction type
ClosedPublic

Authored by ginsu on Dec 21 2022, 8:44 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 5:16 PM
Unknown Object (File)
Fri, Nov 8, 5:15 PM
Unknown Object (File)
Fri, Nov 8, 5:15 PM
Unknown Object (File)
Fri, Nov 8, 5:15 PM
Unknown Object (File)
Fri, Nov 8, 5:15 PM
Unknown Object (File)
Fri, Nov 8, 4:51 PM
Unknown Object (File)
Thu, Nov 7, 5:54 AM
Unknown Object (File)
Wed, Nov 6, 6:30 AM
Subscribers

Details

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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, tomek.
ginsu edited the summary of this revision. (Show Details)
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 ↗(On Diff #19957)

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
atul requested changes to this revision.Dec 21 2022, 2:46 PM

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());
This revision now requires changes to proceed.Dec 21 2022, 2:46 PM
lib/shared/reaction-utils.js
17–24 ↗(On Diff #19967)

Nice yea this is a lot better, thanks for the suggestion

This revision is now accepted and ready to land.Dec 22 2022, 8:03 AM