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
F2148603: D11014.diff
Sun, Jun 30, 5:42 AM
Unknown Object (File)
Mon, Jun 24, 12:20 AM
Unknown Object (File)
Mon, Jun 24, 12:17 AM
Unknown Object (File)
Thu, Jun 20, 1:16 AM
Unknown Object (File)
Tue, Jun 11, 11:54 PM
Unknown Object (File)
Tue, Jun 11, 10:07 AM
Unknown Object (File)
Tue, Jun 11, 10:07 AM
Unknown Object (File)
Tue, Jun 11, 10:07 AM
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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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