search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.45k stars 2.8k forks source link

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

Open lanitochka17 opened 1 week ago

lanitochka17 commented 1 week ago

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?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/a205ff48-0b16-4b01-a1d3-37ffa83fc9e8

View all open jobs on GitHub

melvin-bot[bot] commented 1 week ago

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

github-actions[bot] commented 1 week ago

:wave: 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 commented 1 week ago

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

Krishna2323 commented 1 week ago

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

Proposal


Offending PR: https://github.com/Expensify/App/pull/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. https://github.com/Expensify/App/blob/71db3f19691069182447b6b1b9fff62ec93be86c/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx#L41-L51

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


https://github.com/Expensify/App/blob/71db3f19691069182447b6b1b9fff62ec93be86c/src/components/Tooltip/EducationalTooltip/BaseEducationalTooltip.tsx#L41-L45

What alternative solutions did you explore? (Optional)

Result

blimpich commented 1 week ago

@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 commented 1 week ago

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

Offending PR: https://github.com/Expensify/App/pull/49682

blimpich commented 1 week ago

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[bot] commented 1 week ago

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 commented 1 week ago

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

daledah commented 1 week ago

@dukenv0307 Here's the PR

dukenv0307 commented 1 week ago

@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 commented 1 week ago

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.