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

Web - Saved search - Tooltip reappears after it disappears #49873

Open
2 of 6 tasks
lanitochka17 opened this issue Sep 27, 2024 · 12 comments
Open
2 of 6 tasks

Web - Saved search - Tooltip reappears after it disappears #49873

lanitochka17 opened this issue Sep 27, 2024 · 12 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Reviewing Has a PR in review Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Sep 27, 2024

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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): applausetester+kh010901@applause.expensifail.com
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

@lanitochka17 lanitochka17 added DeployBlockerCash This issue or pull request should block deployment DeployBlocker Indicates it should block deploying the API labels Sep 27, 2024
Copy link

melvin-bot bot commented Sep 27, 2024

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

Copy link
Contributor

👋 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
Copy link
Author

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

@Krishna2323
Copy link
Contributor

Krishna2323 commented Sep 27, 2024

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.

// 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();.

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

What alternative solutions did you explore? (Optional)

  • We can call onHideTooltip(); inside the useEffect below.
    /**
    * 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
Copy link
Contributor

blimpich commented Sep 27, 2024

@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
Copy link
Contributor

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

Offending PR: #49682

@blimpich
Copy link
Contributor

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

@blimpich blimpich removed DeployBlocker Indicates it should block deploying the API DeployBlockerCash This issue or pull request should block deployment labels Sep 27, 2024
@blimpich blimpich added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. and removed Hourly KSv2 labels Sep 27, 2024
Copy link

melvin-bot bot commented Sep 27, 2024

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.

@melvin-bot melvin-bot bot added the Overdue label Sep 30, 2024
@dukenv0307
Copy link
Contributor

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

@melvin-bot melvin-bot bot removed the Overdue label Sep 30, 2024
@daledah daledah mentioned this issue Sep 30, 2024
50 tasks
@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 30, 2024
@daledah
Copy link
Contributor

daledah commented Sep 30, 2024

@dukenv0307 Here's the PR

@dukenv0307
Copy link
Contributor

dukenv0307 commented Sep 30, 2024

@blimpich @luacmartins I want to confirm the expectation here

The tooltip should not reappear after it disappears

Should we dismiss the tooltip forever when users open the modal when the tooltip is open? If not so, we can see the tooltip re-appear when all modals are hide, then it'll be removed after 5s

@luacmartins
Copy link
Contributor

luacmartins commented Sep 30, 2024

Should we dismiss the tooltip forever when users open the modal when the tooltip is open

IMO we should do this. Left a comment on the PR.

@blimpich blimpich assigned daledah and unassigned blimpich Sep 30, 2024
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. Engineering Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

7 participants