Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PAID] [$1000] Bug: Tooltip gets stuck on the page reported by @Puneet-here #12025

Closed
kavimuru opened this issue Oct 20, 2022 · 44 comments
Closed
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review

Comments

@kavimuru
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Decrease the screen size
  2. Hover over any contact at chat navigator
  3. When the tooltip appears open the chat
  4. Hover at back button until the tooltip shows up then press back

Expected Result:

The tooltip should go away

Actual Result:

The tooltip should doesn't go away

Workaround:

unknown

Platform:

Where is this issue occurring?

  • Web
  • Desktop App

Version Number: 1.2.18-2
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
https://user-images.githubusercontent.com/43996225/196830711-438ca9f7-1bbd-44df-80a2-3bf7e0b9ff01.mp4
https://user-images.githubusercontent.com/43996225/196830700-9a924829-e70f-4121-bd50-80e148d1fc91.mov

Expensify/Expensify Issue URL:
Issue reported by: @Puneet-here
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1666210802145239

View all open jobs on GitHub

@kavimuru kavimuru added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Oct 20, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2022

Triggered auto assignment to @laurenreidexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Oct 20, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2022

Triggered auto assignment to @chiragsalian (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@laurenreidexpensify
Copy link
Contributor

looks like a solid external @chiragsalian ? can you do the honours pls :)

@chiragsalian
Copy link
Contributor

chiragsalian commented Oct 20, 2022

Yup, i can reproduce and adding label.

@chiragsalian chiragsalian added the External Added to denote the issue can be worked on by a contributor label Oct 20, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2022

Current assignee @laurenreidexpensify is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 20, 2022
@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2022

Current assignee @chiragsalian is eligible for the External assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title Bug: Tooltip gets stuck on the page reported by @Puneet-here [$250] Bug: Tooltip gets stuck on the page reported by @Puneet-here Oct 20, 2022
@Ollyws
Copy link
Contributor

Ollyws commented Oct 21, 2022

Proposal
This seems to be happen because sometimes the setState callback which triggers the tooltip to hide is never called despite the state being updated. It seems to happen when combined with the navigation drawer opening/closing.

if (isHovered !== this.state.isHovered && !(isHovered && this.hoverDisabled)) {
this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut);
}

A solution to this is to remove the setState callback and call setState followed by the onHoverIn/Out callbacks.

        if (isHovered !== this.state.isHovered && !(isHovered && this.hoverDisabled)) {
-            this.setState({isHovered}, isHovered ? this.props.onHoverIn : this.props.onHoverOut);

+            this.setState({isHovered});

+            if (isHovered) {
+                this.props.onHoverIn();
+            } else {
+                this.props.onHoverOut();
+            }
        }

@melvin-bot
Copy link

melvin-bot bot commented Oct 21, 2022

Looks like something related to react-navigation may have been mentioned in this issue discussion.

As a reminder, please make sure that all proposals are not workarounds and that any and all attempt to fix the issue holistically have been made before proceeding with a solution. Proposals to change our DeprecatedCustomActions.js files should not be accepted.

Feel free to drop a note in #expensify-open-source with any questions.

@laurenreidexpensify
Copy link
Contributor

@rushatgabhane
Copy link
Member

@Ollyws thanks for your proposal!

sometimes the setState callback which triggers the tooltip to hide is never called despite the state being updated

I can't find any evidence that points to callback not being called. You could try console logging.

You're close but we need to fix the root cause because not using callbacks will lead to race conditions.

image

@melvin-bot melvin-bot bot added the Overdue label Oct 26, 2022
@laurenreidexpensify
Copy link
Contributor

not overdue, still looking for proposals

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 26, 2022
@laurenreidexpensify laurenreidexpensify changed the title [$250] Bug: Tooltip gets stuck on the page reported by @Puneet-here [$500] Bug: Tooltip gets stuck on the page reported by @Puneet-here Oct 31, 2022
@laurenreidexpensify
Copy link
Contributor

Price increased to $500

@melvin-bot melvin-bot bot removed the Overdue label Oct 31, 2022
@laurenreidexpensify laurenreidexpensify added Weekly KSv2 and removed Daily KSv2 labels Nov 1, 2022
@laurenreidexpensify
Copy link
Contributor

Offers out in Upwork for everyone with correct pricing

@getusha
Copy link
Contributor

getusha commented Nov 9, 2022

@laurenreidexpensify Thanks Accepted.

@melvin-bot

This comment was marked as outdated.

@JmillsExpensify
Copy link

@laurenreidexpensify Wanted to make sure you have visibility on the regression in the linked PR. Let's wait for that to play out until the BZ checklist is filled out.

@parasharrajat
Copy link
Member

parasharrajat commented Nov 11, 2022

PR of this issue caused a regression #12685.

@rushatgabhane
Copy link
Member

Fixed the regression before it got to staging
PR #12677

@laurenreidexpensify
Copy link
Contributor

laurenreidexpensify commented Nov 16, 2022

Updated 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:

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 17, 2022
@melvin-bot
Copy link

melvin-bot bot commented Nov 17, 2022

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

If no regressions arise, payment will be issued on 2022-11-24. 🎊

@melvin-bot melvin-bot bot changed the title [$1000] Bug: Tooltip gets stuck on the page reported by @Puneet-here [HOLD for payment 2022-11-24] [$1000] Bug: Tooltip gets stuck on the page reported by @Puneet-here Nov 17, 2022
@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Nov 23, 2022
@laurenreidexpensify
Copy link
Contributor

@chiragsalian @rushatgabhane can you link the PR you think caused the regression in the comment above? Thanks

@laurenreidexpensify
Copy link
Contributor

Regression test buddy check here https://expensify.slack.com/archives/C01SKUP7QR0/p1669220417075829

@chiragsalian
Copy link
Contributor

Updated the first two. Not sure what to do about the third. It was discussed on slack and not in expensify-bugs. So should we start a discussion in #expensify-bugs?

@laurenreidexpensify
Copy link
Contributor

@Puneet-here @rushatgabhane @getusha payment has been issued in Upwork 👍

@laurenreidexpensify laurenreidexpensify changed the title [HOLD for payment 2022-11-24] [$1000] Bug: Tooltip gets stuck on the page reported by @Puneet-here [PAID] [$1000] Bug: Tooltip gets stuck on the page reported by @Puneet-here Nov 24, 2022
@laurenreidexpensify laurenreidexpensify removed the Awaiting Payment Auto-added when associated PR is deployed to production label Nov 24, 2022
@getusha
Copy link
Contributor

getusha commented Nov 24, 2022

PR merged within 3 business days. so we're eligible for the 50% bonus? @laurenreidexpensify

@laurenreidexpensify
Copy link
Contributor

@getusha @rushatgabhane bonus sent 👍🏽

@situchan
Copy link
Contributor

It doesn't seem that #12572 fixed the issue completely. Issue reported again #13947

@getusha
Copy link
Contributor

getusha commented Jan 20, 2023

@situchan this approach was overwritten here #13146

@parasharrajat
Copy link
Member

parasharrajat commented Jan 20, 2023

I agree. It seems we forgot to make the solution generic. #13947. cc: @rushatgabhane

@getusha getusha mentioned this issue Mar 27, 2023
53 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

10 participants