Details

Reviewers

atul
kamil

Commits

rCOMMe2a2434b7642: [web] introduce getTooltipScreenOverflowRightCorrection function

Summary

As I was doing some final tests for the ReactionTooltip, I noticed a scenario where the reaction tooltip for the most right reaction pill of a viewer message was overflowing off the screen if the tooltip was at its maxWidth. To address this I created a helper function which calculates the overFlow and shifts the tooltip x anchor point

Depends on D8859

Test Plan

Please see the screenshots below

Before:

Screenshot 2023-08-17 at 9.45.59 PM.png (2×3 px, 1 MB)

Screenshot 2023-08-17 at 9.45.51 PM.png (2×3 px, 1 MB)

After:

Screenshot 2023-08-17 at 10.52.48 PM.png (2×3 px, 1 MB)

Screenshot 2023-08-17 at 10.52.36 PM.png (2×3 px, 1 MB)

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

ginsu created this revision.Aug 17 2023, 6:54 PM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptAug 17 2023, 6:54 PM

ginsu edited the test plan for this revision. (Show Details)Aug 17 2023, 6:55 PM

ginsu added reviewers: atul, kamil.

ginsu attached a referenced file: F694678: Screenshot 2023-08-17 at 9.46.26 PM.png. (Show Details)

ginsu attached a referenced file: F694679: Screenshot 2023-08-17 at 9.49.18 PM.png. (Show Details)

ginsu attached a referenced file: F694676: Screenshot 2023-08-17 at 9.45.51 PM.png. (Show Details)

ginsu attached a referenced file: F694677: Screenshot 2023-08-17 at 9.45.59 PM.png. (Show Details)

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

ginsu added a parent revision: D8859: [web] introduce useReactionTooltip hook.

Harbormaster completed remote builds in B21876: Diff 30054.Aug 17 2023, 7:11 PM

ginsu requested review of this revision.Aug 17 2023, 7:12 PM

adjust padding

ginsu edited the test plan for this revision. (Show Details)Aug 17 2023, 7:53 PM

ginsu attached a referenced file: F694822: Screenshot 2023-08-17 at 10.52.36 PM.png. (Show Details)

ginsu attached a referenced file: F694823: Screenshot 2023-08-17 at 10.52.48 PM.png. (Show Details)

Harbormaster completed remote builds in B21879: Diff 30057.Aug 17 2023, 8:07 PM

Just one quick question, discovered it in D8859 but asking here.

Are you sure that overlapping thread list a bit is okay? (not sure what was original design and how it influence the UI)

web/utils/tooltip-utils.js
63–68 ↗	(On Diff #30057)	that's what's more readable for me, if you have the opposite opinion stick to your version 😅

This revision is now accepted and ready to land.Aug 24 2023, 2:39 PM

Are you sure that overlapping thread list a bit is okay?

Currently all our tooltips do this, so I thought it was okay to keep this consistent behavior.

Screenshot 2023-08-24 at 5.43.06 PM.png (2×3 px, 1 MB)

Screenshot 2023-08-24 at 5.43.13 PM.png (2×3 px, 1 MB)

Curious for Ted's take
cc @ted

ginsu attached a referenced file: F712730: Screenshot 2023-08-24 at 5.43.06 PM.png. (Show Details)Aug 24 2023, 2:45 PM

ginsu attached a referenced file: F712729: Screenshot 2023-08-24 at 5.43.13 PM.png. (Show Details)

In D8860#263537, @ginsu wrote:

Are you sure that overlapping thread list a bit is okay?

Currently all our tooltips do this, so I thought it was okay to keep this consistent behavior.

Curious for Ted's take
cc @ted

Thanks for the tag! Yea, I think it is okay if it overlaps. It's clear enough to me that the tooltip element is above the rest of the content.

address comments / rebase before landing

ginsu retitled this revision from [web] introduce getTooltipScreenOverflowXCorrection function to [web] introduce getTooltipScreenOverflowRightCorrection function.Aug 27 2023, 9:36 PM

rebase before landing

Harbormaster completed remote builds in B22147: Diff 30393.Aug 27 2023, 9:53 PM

Harbormaster completed remote builds in B22148: Diff 30394.Aug 27 2023, 9:58 PM

Closed by commit rCOMMe2a2434b7642: [web] introduce getTooltipScreenOverflowRightCorrection function (authored by ginsu). · Explain WhyAug 27 2023, 10:04 PM

This revision was automatically updated to reflect the committed changes.

ginsu added a commit: rCOMMe2a2434b7642: [web] introduce getTooltipScreenOverflowRightCorrection function.

thanks @ginsu and @ted for response!