Page MenuHomePhabricator

[web] introduce calculateReactionTooltipSize function
ClosedPublic

Authored by ginsu on Aug 17 2023, 5:24 PM.
Tags
None
Referenced Files
F3348653: D8857.id30052.diff
Fri, Nov 22, 3:42 PM
F3348615: D8857.id30391.diff
Fri, Nov 22, 3:26 PM
Unknown Object (File)
Wed, Nov 13, 2:12 AM
Unknown Object (File)
Thu, Nov 7, 5:53 PM
Unknown Object (File)
Mon, Nov 4, 10:48 PM
Unknown Object (File)
Mon, Oct 28, 5:40 AM
Unknown Object (File)
Mon, Oct 28, 5:40 AM
Unknown Object (File)
Mon, Oct 28, 5:40 AM
Subscribers

Details

Summary

This diff introduces a function to calculate the size of the reaction tooltip. In order to build a tooltip, we need to first know the size, so this function will handles that for our new reaction tooltip.

Here is a screenshot of the reaction for context:

Screenshot 2023-08-17 at 9.22.20 PM.png (482×486 px, 45 KB)

Depends on D8858

Test Plan

logged out the values of width and height and confirmed that they matched the size of the tooltip that was being rendered

Screenshot 2023-08-17 at 9.18.17 PM.png (616×726 px, 84 KB)

Screenshot 2023-08-17 at 9.18.33 PM.png (170×3 px, 39 KB)

Screenshot 2023-08-17 at 9.18.52 PM.png (402×806 px, 56 KB)

Screenshot 2023-08-17 at 9.19.18 PM.png (152×3 px, 42 KB)

Screenshot 2023-08-17 at 9.17.18 PM.png (316×796 px, 39 KB)

Screenshot 2023-08-17 at 9.18.00 PM.png (148×2 px, 42 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu requested review of this revision.Aug 17 2023, 5:42 PM
web/utils/tooltip-utils.js
354 ↗(On Diff #30052)

Screenshot 2023-08-17 at 9.22.20 PM.png (482×486 px, 45 KB)

Hoping that having a visualization of the tooltip will help with reviewing my logic

366 ↗(On Diff #30052)

We want to make sure that even if all the usernames in the list are short ('a', 'z', 'c2' etc.) and 'See more' is wider than all the usernames we still get the correct width

370 ↗(On Diff #30052)

We wrap super long usernames

ginsu edited the test plan for this revision. (Show Details)
ginsu added inline comments.
web/utils/tooltip-utils.js
370 ↗(On Diff #30052)

eclipse not wrap...

kamil added inline comments.
web/utils/tooltip-utils.js
358–359 ↗(On Diff #30052)

const { paddingTop, paddingBottom, padding... } = reactionTooltipStyle;

I would destruct reactionTooltipStyle at the beginning to simply reduce the code lines width and remove those two lines - maybe this will improve readability

This revision is now accepted and ready to land.Aug 24 2023, 2:01 PM
ginsu edited the test plan for this revision. (Show Details)

address comments and rebase before landing