[HOLD for payment 2024-10-17] Web - Saved search - Tooltip reappears after it disappears

[lanitochka17]

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.41-1
Reproducible in staging?: Y
Reproducible in prodaction?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): [email protected]
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Log in with a new account
3. Go to Search
4. Click Filters
5. Add some filters
6. Click Save search
7. Note that tooltip appears on the saved search
8. Click 3-dot menu > Rename
9. Rename the search and save it

Expected Result:

The tooltip should not reappear after it disappears

Actual Result:

The tooltip reappears after it disappears

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

Add any screenshot/video evidence

Bug6617794_1727466826401.20240928_035109.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @VictoriaExpensify

[melvin-bot]

Triggered auto assignment to @blimpich (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ 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.

[lanitochka17]

We think that this bug might be related to #wave-control

[Krishna2323]

Edited by proposal-police: This proposal was edited at 2024-09-27 22:48:30 UTC.

Proposal


Offending PR: #49682

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

Web - Saved search - Tooltip reappears after it disappears

What is the root cause of that problem?

We aren't calling onHideTooltip?.(); when tooltip is closed when modal is opened.

App/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx

Lines 41 to 51 in 71db3f1

	// If the modal is open, hide the tooltip immediately and clear the timeout
	if (!shouldShow) {
	hideTooltipRef.current();
	return;
	}
	// Automatically hide tooltip after 5 seconds if shouldAutoDismiss is true
	const timerID = setTimeout(() => {
	hideTooltipRef.current?.();
	onHideTooltip?.();
	}, 5000);

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


• We should call onHideTooltip?.(); after hideTooltipRef.current();.

App/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx

Lines 41 to 45 in 71db3f1

	// If the modal is open, hide the tooltip immediately and clear the timeout
	if (!shouldShow) {
	hideTooltipRef.current();
	return;
	}

• We can also call onHideTooltip?.(); in the cleanup function below.
  

  App/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx

  Lines 24 to 33 in 71db3f1

  	useEffect(
  	() => () => {
  	if (!hideTooltipRef.current) {
  	return;
  	}
  	hideTooltipRef.current();
  	},
  	[],
  	);

What alternative solutions did you explore? (Optional)

• We can call onHideTooltip(); inside the useEffect below.
  

  App/src/components/Tooltip/GenericTooltip.tsx

  Lines 124 to 145 in 71db3f1

  	/**
  	* Hide the tooltip in an animation.
  	*/
  	const hideTooltip = useCallback(() => {
  	animation.current.stopAnimation();
  	if (TooltipSense.isActive() && !isTooltipSenseInitiator.current) {
  	animation.current.setValue(0);
  	} else {
  	// Hide the first tooltip which initiated the TooltipSense with animation
  	isTooltipSenseInitiator.current = false;
  	Animated.timing(animation.current, {
  	toValue: 0,
  	duration: 140,
  	useNativeDriver: false,
  	}).start();
  	}
  	TooltipSense.deactivate();
  	setIsVisible(false);
  	}, []);

Result

[blimpich]

@Krishna2323 since this is marked as a deploy blocker you need to identify what caused the regression in the first place. I don't consider the proposal complete without that.

[Krishna2323]

@blimpich sorry, I knew that but forgot to add that in the proposal.

Offending PR: #49682

[blimpich]

In order for me to mark this as external I would want (A) the original contributor and C+ to not be reachable (B) the original PR for some reason shouldn't be reverted because it'll cause more problems then it'll solve and (C) the problem is urgent enough that I don't want to wait for the original contributor to get around to it.

For this issue A is true but B and C are not. I think this problem is minor enough that we can demote it and make it follow up work for people who worked on the PR who caused the regression. Not worth reverting over a slightly buggy tooltip IMO.

cc: @luacmartins @daledah @dukenv0307

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (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.

[dukenv0307]

@daledah Pls raise the PR, I think we just need to clear the timeout (second one)

[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 9.0.47-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: tooltip reappears #49913

If no regressions arise, payment will be issued on 2024-10-17. 🎊

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

• @dukenv0307 requires payment (Needs manual offer from BZ)
• @daledah 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.
• [@VictoriaExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @dukenv0307 paid $(AMOUNT) via Upwork (LINK)
• ROLE: @daledah paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@VictoriaExpensify)

• 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//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[VictoriaExpensify]

Hey @daledah and @dukenv0307 - I've sent you both offers in Upwork, can you please accept these and I'll organise payment?

@dukenv0307 could you also please complete the checklist? Thank you!

[dukenv0307]

BugZero Checklist:

• The PR that introduced the bug has been identified. Link to the PR: fix: tooltip show over dialog #49682
• 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/49682/files#r1817161652
• 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:

1. Create a saved search
2. Verify the tooltip appears.
3. Click 3-dot menu -> edit
4. Verify the tooltip disappears

Do we 👍 or 👎

[VictoriaExpensify]

TestRail issue created - https://github.com/Expensify/Expensify/issues/437475

[VictoriaExpensify]

@dukenv0307 and @daledah - still waiting for you to accept offers 🙏 https://www.upwork.com/ab/applicants/1847068669603673881/pending

[dukenv0307]

@VictoriaExpensify This is the regression as discussed here so no payment is needed

[VictoriaExpensify]

Ah I missed that comment. Thanks @dukenv0307

[melvin-bot]

@luacmartins @VictoriaExpensify Be sure to fill out the Contact List!

[tgolen]

@dukenv0307 I see your answers to the BZ Checklist, but I think there was a PR that was identified as the culprit. Should that be mentioned in the Checklist?

[dukenv0307]

@tgolen I added the regression PR. But it's my regression so I think we should process payment and checklists in the main issue here: #49517. What do you think?

[tgolen]

I think it's OK to handle the checklist here and the payments in #49517.

[dukenv0307]

@tgolen I updated the checklists

[VictoriaExpensify]

So just to check, do I need to process payment here or not. Based on this comment, I think not?

[dukenv0307]

@VictoriaExpensify I think we decided to process in #49517 based on #49873 (comment)

[VictoriaExpensify]

Ok closing this out since it doesn't look like I need to do anything more here. Thanks @dukenv0307

[melvin-bot]

@luacmartins @VictoriaExpensify Be sure to fill out the Contact List!