[HOLD for payment 2024-02-07] [$1000] Chat - Hover color is not displayed even on move of cursor on changing windows and back

[izarutskaya]

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. Open the app
2. Open any report
3. Hover on any message
4. Use CMD+TAB to change window and use it again to get back to app
5. Move cursor over the same message and observe that no hover color is displayed

Expected Result:

App should display hover color on message when we move cursor over that message

Actual Result:

App does not display hover color on message when we hover on the message, change window and again visit the app and move the cursor on that message

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: v1.3.79-3

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: Any additional supporting documentation

mac.chrome.desktop.background.hover.not.displayed.mov

20231009_014219.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696667037989909

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~017f7a0e22541d49f4
• Upwork Job ID: 1711324702431768576
• Last Price Increase: 2023-10-30
• Automatic offers:
• 0xmiroslav | Reviewer | 28117244
• Krishna2323 | Contributor | 28117245

[melvin-bot]

Triggered auto assignment to @dylanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~017f7a0e22541d49f4

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[chamysoftware]

Proposal

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

When switching to a different window, the hover effect is lost.
And the hover effect is not working untill we do mouse out and mouse enter again.

What is the root cause of that problem?

In the Hoverable component (src/components/Hoverable/index.js),
we're removing the hover effect when document is losing visibility.
But we're not setting the hover back when the document is gaining the visibility back.

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

We need to add a mouse move event handler in addition to the other handlers here:

App/src/components/Hoverable/index.js

Lines 157 to 170 in 3007017

	onMouseEnter: (el) => {
	this.setIsHovered(true);
	if (_.isFunction(child.props.onMouseEnter)) {
	child.props.onMouseEnter(el);
	}
	},
	onMouseLeave: (el) => {
	this.setIsHovered(false);
	if (_.isFunction(child.props.onMouseLeave)) {
	child.props.onMouseLeave(el);
	}
	},

On mouse move, we need to set is hovered to true.

We need to do this in a way that it doesn't trigger additional re-renders. I see we already have a class property called isHoveredRef and a method called setIsHovered on the component. We can use this to efficiently set the hovered state. We can call setIsHovered(true) function only when isHoveredRef is false.

What alternative solutions did you explore? (Optional)

NA

[melvin-bot]

📣 @Mostick! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[dylanexpensify]

@0xmiroslav mind reviewing proposals? 🙇‍♂️

[yh-0218]

Proposal

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

Hover color is not displayed even on move of cursor on changing windows and back

What is the root cause of that problem?

Because we don't have function to control Mouse Movement.

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

We can add onMouseMove event after onMouseLeave event.

onMouseMove: (el) => {
                if (this.isHoveredRef) {
                    return
                }
                this.setIsHovered(true);
                if (_.isFunction(child.props.onMouseMove)) {
                    child.props.onMouseMove(el);
                }
},

App/src/components/Hoverable/index.js

Lines 164 to 171 in 8d42ad3

	onMouseLeave: (el) => {
	this.setIsHovered(false);
	if (_.isFunction(child.props.onMouseLeave)) {
	child.props.onMouseLeave(el);
	}
	},
	onBlur: (el) => {

What alternative solutions did you explore? (Optional)

[Vadym-33]

Proposal

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

Hover color is not displayed even on move of cursor on changing windows and back

What is the root cause of that problem?

Because we don't have function to control Mouse Movement.

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

We need to add onMouseMove event to

App/src/components/Hoverable/index.js

Line 142 in 8d42ad3

return React.cloneElement(React.Children.only(child), {

onMouseMove: (el) => {
                if (_.isFunction(child.props.onMouseMove)) {
                    child.props.onMouseMove(el);
                }
                if (this.isHoveredRef) {
                    return
                }
                this.setIsHovered(true);
},

What alternative solutions did you explore? (Optional)

[dylanexpensify]

reviewing tomorrow AM!

[chamysoftware]

@dylanexpensify a gentle reminder to review the Proposals here :)

[0xmiros]

If possible, can we hover back even when mouse is not moved?

Screen.Recording.2023-10-12.at.2.33.47.PM.mov

[chamysoftware]

@0xmiroslav, just checked. My proposal above works even when the mouse is not moved:

Screen.Recording.2023-10-12.at.7.03.37.PM.mov

I'm not touching the mouse while recording the screen

[Vadym-33]

hello @0xmiroslav Do you check my proposal?

[dylanexpensify]

bump @0xmiroslav ^ 🙇‍♂️

[0xmiros]

still open for better solution

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[dylanexpensify]

Waiting for proposals

[Vadym-33]

Hello, @dylanexpensify How to think about my proposal

[yh-0218]

@dylanexpensify
If we want to change hover status when only back and move on that, then we can like this.
Need to update checkHover function

checkHover(e) {
        if (!this.wrapperView) {
            return;
        }

        if (this.wrapperView.contains(e.target)) {
            if (!this.state.isHovered) {this.setIsHovered(true);}
            return;
        }

        this.setIsHovered(false);
 }

App/src/components/Hoverable/index.js

Line 108 in 2ff8c49

checkHover(e) {

[mallenexpensify]

ah.. thanks, I think I was looking for 'merged' vs 'completed review', appreciate the headsup, if no additional work is needed on the PR, @0xmiroslav will be fully compensated. Letting @dylanexpensify finish this one up, Im unsubscribing, tag me if needed

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[Krishna2323]

Still reproducible.

[dylanexpensify]

@Krishna2323 can you add video to confirm?

[Krishna2323]

@dylanexpensify, the PR is deployed to staging 5 hours ago, here is the recording of production version.

Monosnap.screencast.2024-01-31.15-35-01.1.mp4

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

• fix: Chat - Hover color is not displayed even on move of cursor on window change and back. #34959

If no regressions arise, payment will be issued on 2024-02-07. 🎊

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

• @jjcoffee requires payment (Needs manual offer from BZ)
• @Krishna2323 requires payment automatic offer (Contributor)
• @dhanashree-sawant 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:

• [@jjcoffee] The PR that introduced the bug has been identified. Link to the PR:
• [@jjcoffee] 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:
• [@jjcoffee] 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:
• [@jjcoffee] Determine if we should create a regression test for this bug.
• [@jjcoffee] 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.
• [@dylanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[jjcoffee]

Unassigned as per this comment payment is due to @0xmiroslav here.

[dylanexpensify]

today!

[Krishna2323]

@mallenexpensify, is @0xmiroslav still active on this? If not the who will complete the checklist 🤔

[Julesssss]

@mallenexpensify, is @0xmiroslav still active on this? If not the who will complete the checklist 🤔

I just checked them off. THis wasn't a regression and is too minor for a regression test IMO

[aimane-chnaif]

I think this is side effect of #18675 (I reviewed this) so technically regression.
But agree not worth adding regression test.

[dylanexpensify]

Payment summary:

• Contributor+: @0xmiroslav $1000
• Contributor: @Krishna2323 $1000
• Reporter: @dhanashree-sawant $50

[dylanexpensify]

@0xmiroslav @dhanashree-sawant sent offer!

[dhanashree-sawant]

Thanks @dylanexpensify, I have accepted the offer.

[dylanexpensify]

done!