[HOLD for payment 2023-03-17] [$1000] Report item stays hovered after we edited the message or cancelled the edit message

[kavimuru]

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. Choose edit option for any message
2. Try to edit and save
3. Now hover on other rows and observe the edited message row

Expected Result:

After edit/cancel edit the message item should not be hovered

Actual Result:

After edit/cancel edit the message item keeps hovered

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• MacOS / Chrome / Safari

Version Number: 1.2.50-4
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Screen.Recording.2023-01-03.at.12.45.09.AM.mov

Recording.1221.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Pujan92
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1672687285656309

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01bc65e21d9f4a414d
• Upwork Job ID: 1613204426013020160
• Last Price Increase: 2023-01-11

[luacmartins]

I'm not able to reproduce this

[amyevans]

I also could not repro in Safari 16.1 (I did not test older versions).

Edit: nor Chrome.

[isabelastisser]

I'm also unable to reproduce it:

2023-01-09_11-24-06.mp4

[isabelastisser]

I'm able to reproduce it now.

testing123.mp4

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[s77rt]

Proposal

diff --git a/src/components/Hoverable/index.js b/src/components/Hoverable/index.js
index ef4bc2bf53..841696f9ef 100644
--- a/src/components/Hoverable/index.js
+++ b/src/components/Hoverable/index.js
@@ -107,6 +107,15 @@ class Hoverable extends Component {
                         onMouseLeave(el);
                     }
                 },
+                onBlur: (el) => {
+                    this.setIsHovered(false);
+
+                    // Call the original onBlur, if any
+                    const {onBlur} = this.props.children;
+                    if (_.isFunction(onBlur)) {
+                        onBlur(el);
+                    }
+                },
             });
         }
         return (
@@ -115,6 +124,7 @@ class Hoverable extends Component {
                 ref={el => this.wrapperView = el}
                 onMouseEnter={() => this.setIsHovered(true)}
                 onMouseLeave={() => this.setIsHovered(false)}
+                onBlur={() => this.setIsHovered(false)}
             >
                 { // If this.props.children is a function, call it to provide the hover state to the children.
                     _.isFunction(this.props.children)

Details

This is similar to another issue where the hovered item stays hovered due to the missing event onMouseLeave.
We can compensate this missing event by relaying on the onBlur event.

Note: The resetHoverStateOnOutsideClick can be removed since onBlur seems to do it's job, this it out of the scope of this issue, just worth noting. @roryabraham

[roryabraham]

Note: The resetHoverStateOnOutsideClick can be removed since onBlur seems to do it's job, this it out of the scope of this issue, just worth noting

I'd be a 👍🏼 to removing resetHoverStateOnOutsideClick, feels like a hack.

[s77rt]

@roryabraham I have only tested with closing the modal (as described in the comments) is there any other scenarios that we should test before removing that code?

[roryabraham]

I can't think of any others, no

[isabelastisser]

Not overdue.

[s77rt]

Thanks @getusha I didn't catch that while testing
The fix is coming up...

[s77rt]

The PR fixing the regression is ready

[ctkochan22]

We are good here?

[isabelastisser]

Update: All contributors were paid in Upwork now.

Reported by @Pujan92
Fixed by @s77rt
C+ review @mananjadhav

[gadhiyamanan]

The PR fixing the regression is ready

I think this is eligible for reporting bonus as i found the regression
Cc: @isabelastisser @mananjadhav @ctkochan22

[gadhiyamanan]

Bump @isabelastisser @ctkochan22 @mananjadhav ^

[mananjadhav]

@isabelastisser @gadhiyamanan is eligible for the reporting payout here.

[ctkochan22]

I'm not 100% sure how this works if its a regression, but probably that is correct. @isabelastisser can we ask in slack?

[isabelastisser]

Hi @gadhiyamanan , I invited you to the job in Upwork now. Please accept it and I will process the payment. Thanks!

[gadhiyamanan]

@isabelastisser applied

[isabelastisser]

The payment was processed in Upwork, we are all set!

[gadhiyamanan]

@isabelastisser please close the upwork contract

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

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

• Fixed regression on Hoverable blur event #14507

If no regressions arise, payment will be issued on 2023-03-17. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

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:

• [@mananjadhav / @s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@mananjadhav / @s77rt] 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:
• [@mananjadhav / @s77rt] 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:
• [@isabelastisser] Determine if we should create a regression test for the bug.
• [@mananjadhav / @s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to the appropriate location to ensure the same bug will not reach production again.
• [@isabelastisser] Review the proposed regression test steps and location.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[MelvinBot]

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.

[s77rt]

• The PR that introduced the bug has been identified: I think this existed from first implementation cc @mananjadhav
• The offending PR has been commented on: ^
• A discussion in #expensify-bugs has been started: cc @mananjadhav

---

Regression Test Proposal

1. Open context menu for any message item
2. Choose edit comment
3. Click cancel or save changes
4. Verify that the message item is not hovered
5. Open an attachment (modal) in chat
6. Close the modal
7. Verify that the message item of that attachment is not hovered

[aimane-chnaif]

(For BugZero Checklist)
PR which fixes this issue caused regression - #15300
It came from deprecation of resetHoverStateOnOutsideClick which is done unnecessarily to this issue but just for optimization. (#14080 (comment))

I understand this was removed as hack but in the future, when remove hack, we should make sure to find out and test all possible issues hidden by this hack.