[HOLD for payment 2024-04-05] [$250] Tooltips - Tooltips on Desktop overlap with App window "dots"

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Found when validating PR : #37421

Version Number: v1.4.52-0
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): [email protected]
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Access staging.new.expensify.com
2. Sign into any account
3. Hover over the tool tips that display in the top left corner from either the home page or the account setting Share code page

EXTRA: Also happens when user long presses on the chat to open the modal

Expected Result:

User expects the tooltips to be "underneath" as on prod and not overlap with the 3 dots

Actual Result:

The tooltips are slightly cut off with this new UI update

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6413082_1710388502434.Overlapping_on_new_UI_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01ec455ad18cff87b2
• Upwork Job ID: 1768271312655159296
• Last Price Increase: 2024-03-14

[melvin-bot]

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @lakchote (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[izarutskaya]

@lakchote I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think that this bug might be related to #vip-vsb
CC @quinthar

[izarutskaya]

Production

Bug6413082_1710388408018.On_Prod_the_tooltips_do_not_overlap_.mp4

[mountiny]

This is not a blocker due to its impact on users, I think think we make this external and for $250 as its low impact

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01ec455ad18cff87b2

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @aimane-chnaif (External)

[aimane-chnaif]

User expects the tooltips to be "underneath" as on prod and not overlap with the 3 dots

Is there any such case on production?
I have trouble finding any tooltip super closer to desktop window "dots" in production

[bernhardoj]

Proposal

Please re-state the problem that we are trying to solve in this issue.

The tooltip overlaps with the desktop window dots.

What is the root cause of that problem?

This happens after the ideal nav v2 where the TopBar is moved to the sidebar screen. Both the sidebar screen and CustomBottomTabNavigator are wrapped with ScreenWrapper,

App/src/pages/home/sidebar/SidebarScreen/BaseSidebarScreen.js

Lines 30 to 31 in 206d293

	return (
	<ScreenWrapper

App/src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/index.tsx

Lines 48 to 49 in 206d293

	return (
	<ScreenWrapper

and ScreenWrapper has a header gap component that has a height of 12 for desktop only.

App/src/components/ScreenWrapper.tsx

Line 250 in 206d293

<HeaderGap styles={headerGapStyles} />

Because there are 2 screen wrappers, the total height is 24 which means the top bar is pushed down 12 more than the previous version and this gives enough space for the tooltip logic to detect that the tooltip is 12 px (GUTTER_WIDTH) far from the window top edge.

App/src/styles/utils/generators/TooltipStyleUtils.ts

Lines 187 to 188 in 206d293

	shouldShowBelow =
	shouldForceRenderingBelow || yOffset - tooltipHeight < GUTTER_WIDTH || !!(tooltip && isOverlappingAtTop(tooltip, xOffset, yOffset, tooltipTargetWidth, tooltipTargetHeight));

yOffset - tooltipHeight < GUTTER_WIDTH

yOffset is the element wrapped with the tooltip y position, in our case the workspace switcher button.

There are 2 problems with the calculation above.

1. The tooltipHeight doesn't include the tooltip arrow yet
2. It doesn't include the case for the desktop header yet.

What changes do you think we should make in order to solve the problem?

For the double header gap, we can add a new props to ScreenWrapper to conditionally disable it. (or perhaps we should remove the screen wrapper from the custom bottom tab navigator?)

Fixing the above is actually enough, if we also want to fix the tooltip calculation, we can add the tooltip arrow height and consider the desktop header height.

// this is just a random number.
const desktopHeaderHeight = 24;
yOffset - tooltipHeight - POINTER_HEIGHT < GUTTER_WIDTH + desktopHeaderHeight

[melvin-bot]

Upwork job price has been updated to $250

[sakluger]

Is there any such case on production?
I have trouble finding any tooltip super closer to desktop window "dots" in production

There are two examples in the original issue description:

1. Hover over the Workspace selector at the top-left
2. Right-click on the highest chat in the LHN near the top-left of the chat row

[jhohlfeld]

User expects the tooltips to be "underneath" as on prod and not overlap with the 3 dots

Is there any such case on production? I have trouble finding any tooltip super closer to desktop window "dots" in production

To be honest, I can't reproduce the positioning issue for tooltips on production right now. However, for context menus, it still exists (potentially interfering with the electron window elements as the menu is reaching the top of the viewport).

[dukenv0307]

I think we can keep the ScreenWrapper

@bernhardoj Yes we should keep the ScreenWrapper

[dukenv0307]

@jhohlfeld I think we should understand the meaning of gutterWidth. Why it's name is gutterWidth not gutterHeight? Because we just need to horizontally adjust the tooltip/menu to avoid it being too close to the edge. That why we just do it in computeHorizontalShift, not computeVerticalShift

Also, gutterWidth is only used for this task - no side effects here.

Besides, if we change the gutterWidth to 30px, it can affect both tooltip and popover

Pls note that this bug just happens on desktop since we have header gap, so we need to keep other platforms operating as before, just fix the desktop bug

[dukenv0307]

We should go with @bernhardoj's solution.

BTW, I think we should handle popover bug that is mentioned by @jhohlfeld in other issue, since we have the different RCA and solution.

🎀👀🎀 C+ reviewed

[melvin-bot]

Current assignee @lakchote is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[lakchote]

Thanks everyone for your proposals and thanks @dukenv0307 for your review. I do agree with what you've said about the gutter width.

BTW, I think we should handle #38292 (comment) that is mentioned by @jhohlfeld in other issue, since we have the different RCA and solution.

I do agree that the issue @jhohlfeld reported should be handled in another issue. cc @sakluger

--

@bernhardoj's proposal LGTM 👍

[melvin-bot]

❌ There was an error making the offer to @dukenv0307 for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @bernhardoj for the Contributor role. The BZ member will need to manually hire the contributor.

[bernhardoj]

PR is ready

cc: @dukenv0307

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.57-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix tooltip overlaps with the desktop header #38806

If no regressions arise, payment will be issued on 2024-04-05. 🎊

For reference, here are some details about the assignees on this issue:

• @bernhardoj requires payment (Needs manual offer from BZ)
• @dukenv0307 requires payment (Needs manual offer from BZ)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@dukenv0307] The PR that introduced the bug has been identified. Link to the PR:
• [@dukenv0307] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@dukenv0307] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@dukenv0307] Determine if we should create a regression test for this bug.
• [@dukenv0307] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@sakluger] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: [Wave 8] Ideal nav  #33280

• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A

• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A

• Determine if we should create a regression test for this bug. No

• If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again. No

[melvin-bot]

Payment Summary

Upwork Job

• Contributor: @bernhardoj paid $250 via Upwork (https://www.upwork.com/nx/wm/offer/101781111/)
• C+: @dukenv0307 paid $250 via Upwork (https://www.upwork.com/nx/wm/offer/101781117)

BugZero Checklist (@sakluger)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1768271312655159296/hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[sakluger]

I sent offers via Upwork, I'll pay out once the offers are accepted.

[sakluger]

All paid out, thanks 🙇‍♂️