Page MenuHomePhabricator

[web] fix reaction tooltip not showing the correct number of users
ClosedPublic

Authored by ginsu on Feb 9 2024, 8:24 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Nov 2, 3:33 AM
Unknown Object (File)
Tue, Oct 22, 1:06 PM
Unknown Object (File)
Tue, Oct 22, 9:02 AM
Unknown Object (File)
Tue, Oct 22, 8:37 AM
Unknown Object (File)
Tue, Oct 22, 8:37 AM
Unknown Object (File)
Tue, Oct 22, 8:36 AM
Unknown Object (File)
Sep 23 2024, 10:50 AM
Unknown Object (File)
Sep 6 2024, 10:38 PM
Subscribers

Details

Summary

When a message has more than 5 reactions, the reaction tooltip should only be showing the first 5 usernames then a "See more" text. However, this was not the case and we were showing all the usernames which resulted in the following visual bug.

Screenshot 2024-02-09 at 11.26.27 AM.png (458×306 px, 35 KB)

This diff fixes this issue

Linear task: https://linear.app/comm/issue/ENG-6328/show-more-in-reaction-tooltip-is-not-working-as-expected

Test Plan

Please see the screenshots below

before:

Screenshot 2024-02-09 at 11.28.50 AM.png (582×480 px, 44 KB)

after:

Screenshot 2024-02-09 at 11.28.05 AM.png (512×314 px, 27 KB)

confirmed tooltip still looks good with < 5 reactions

Screenshot 2024-02-09 at 11.27.50 AM.png (416×300 px, 16 KB)

Diff Detail

Repository
rCOMM Comm
Branch
eng-6328
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu edited the test plan for this revision. (Show Details)
ginsu added reviewers: atul, inka.
web/tooltips/tooltip-action-utils.js
460–465 ↗(On Diff #36939)

Rather than calculating showSeeMoreText and usernamesToShow separately in both calculateReactionTooltipSize and ReactionTooltip, we should calculate it once here then just pass the results down as props to both places

ginsu requested review of this revision.Feb 9 2024, 8:46 AM
This revision is now accepted and ready to land.Feb 9 2024, 12:56 PM