[$250] Web - LHN - Blank tooltip displayed when created a workspace and hover over workspace selector

[IuliiaHerets]

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

---

Version Number: 9.0.30-4
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause Internal Team

Action Performed:

1. Open https://staging.new.expensify.com/
2. Tap on Workspace selector
3. Tap on Plus button to create a workspace
4. Tap on back arrow button twice in a row
5. Hover over the Workplace Selector
6. Reload page
7. Hover over the Workspace Selector
8. Repeat steps 2-5

Expected Result:

Tooltip "Choose a workspace" should be displayed when user created a workspace and hovers over the workspace selector

Actual Result:

Blank tooltip displayed when user created a workspace and hover over workspace selector. If user reloads the page, the workspace selector tooltip will be displayed properly.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6594725_1725577761937.Screen_Recording_2024-09-06_at_02.00.09.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021833240040535012696
• Upwork Job ID: 1833240040535012696
• Last Price Increase: 2024-09-23
• Automatic offers:
• jjcoffee | Reviewer | 104119257
• QichenZhu | Contributor | 104119259

Issue Owner

Current Issue Owner: @jjcoffee

[melvin-bot]

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[IuliiaHerets]

@anmurali FYI 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

[melvin-bot]

@anmurali Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

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

[melvin-bot]

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

[anmurali]

Waiting on proposals.

[QichenZhu]

Proposal

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

The tooltip size becomes small after displaying it, navigating two screens away, then returning and displaying the tooltip again.

What is the root cause of that problem?

The code below calculates the tooltip size. It works correctly the first time, but when called again, it calculates the size as 0 because the animation applied a scale(0) transformation.

App/src/components/Tooltip/BaseGenericTooltip/index.tsx

Lines 50 to 55 in 762bffc

	useLayoutEffect(() => {
	// Calculate the tooltip width and height before the browser repaints the screen to prevent flicker
	// because of the late update of the width and the height from onLayout.
	setContentMeasuredWidth(contentRef.current?.getBoundingClientRect().width);
	setWrapperMeasuredHeight(rootWrapper.current?.getBoundingClientRect().height);
	}, []);

The reason it is called again is that the component is frozen after navigating away for two screens and then resumed when returning.

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

A straightforward solution is to temporarily set the transformation to scale(1) before calculating the size.

const rootWrapperStyle = rootWrapper?.current?.style;
const isScaled = rootWrapperStyle?.transform === 'scale(0)';
if (isScaled) {
    rootWrapperStyle.transform = 'scale(1)';
}
setContentMeasuredWidth(contentRef.current?.getBoundingClientRect().width);
setWrapperMeasuredHeight(rootWrapper.current?.getBoundingClientRect().height);
if (isScaled) {
    rootWrapperStyle.transform = 'scale(0)';
}

What alternative solutions did you explore? (Optional)

We could calculate the size only once if we intend not to update it further.

if (!contentMeasuredWidth) {
    setContentMeasuredWidth(contentRef.current?.getBoundingClientRect().width);
}
if (!wrapperMeasuredHeight) {
    setWrapperMeasuredHeight(rootWrapper.current?.getBoundingClientRect().height);
}

[jjcoffee]

Thanks for the proposal @QichenZhu!

the animation applied a scale(0) transformation.

Could you clarify which animation you're talking about and where the scale(0) transform is being applied?

[QichenZhu]

@jjcoffee I’m referring to the animation of the tooltip scaling down as it disappears.

Screen.Recording.2024-09-11.at.11.13.04.PM.mov

scale(0) is applied to Animated.View.

App/src/components/Tooltip/BaseGenericTooltip/index.tsx

Lines 123 to 126 in 762bffc

	<Animated.View
	ref={viewRef(rootWrapper)}
	style={[rootWrapperStyle, animationStyle]}
	>

[jjcoffee]

@QichenZhu Thanks, I see what you mean now! For now I think the proposed solution sounds like a bit too much of a workaround, and I don't think the alternative solution necessarily works (unless you can argue for it's robustness).

[QichenZhu]

@jjcoffee Thank you for the review.

The alternative solution works if GenericTooltip instances correctly follow the usage described below.

App/src/components/Tooltip/GenericTooltip.tsx

Lines 179 to 181 in 12a0822

	// We pass a key, so whenever the content changes this component will completely remount with a fresh state.
	// This prevents flickering/moving while remaining performant.
	key={[text, ...renderTooltipContentKey, preferredLocale].join('-')}

In my understanding, this means that by design, a fresh new BaseGenericTooltip instance will be created whenever the content changes, so we only need to calculate the size once for each instance.

Details can be found here: #18189 (comment).

[jjcoffee]

@QichenZhu Hmm interesting, thanks for the clarification!

After a bit of further investigation, I'm happy to go with @QichenZhu's proposal. I agree with their RCA and don't think there's another way to solve this. We could probably go with either the main or alternative solution, but the main one might be a bit safer.

🎀👀🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @marcochavezf, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

@marcochavezf, @anmurali, @jjcoffee Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jjcoffee]

Not overdue - just waiting for @marcochavezf to assign.

[melvin-bot]

@marcochavezf @anmurali @jjcoffee this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@marcochavezf, @anmurali, @jjcoffee Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jjcoffee]

@marcochavezf Are you able to assign @QichenZhu here? Thanks!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @jjcoffee 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @QichenZhu 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[jjcoffee]

Deployed to production yesterday, so should be due for payment 2024-10-07. cc @anmurali

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[QichenZhu]

Deployed to production 7 days ago, and no regressions were reported. cc @anmurali

[jjcoffee]

• The PR that introduced the bug has been identified. Link to the PR: Migrate TooltipRenderedOnPageBody and fix tooltip flicker/moving when the content changes #18189 (though maybe more the freeze PR that brought it to light)
• 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: https://github.com/Expensify/App/pull/18189/files#r1791495783
• 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. Yes
• 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.

Regression Test Proposal
Desktop platforms only

1. Hover over the Workspace selector, and note the tooltip that displays.
2. Tap the Workspace selector and tap the + button to create a workspace.
3. Tap the back arrow button twice.
4. Hover over the Workspace selector again.
5. Verify that the tooltip Choose a workspace is displayed at the correct size.

Do we agree 👍 or 👎

[jjcoffee]

@anmurali Friendly bump for payment 🙇

[QichenZhu]

Gentle bump @anmurali

[anmurali]

Done