[$1000] Error in JS console when pressing the "Login" button or the "Create a New Account" link in the footer of Magic Code page

[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. Open the app.
2. Navigate to the Magic Code page.
3. Scroll down and click on either login or create a new account link.

Expected Result:

This should cause no error in the console.

Actual Result:

An error is displayed in the console when the login button or the account creation link is clicked.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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: 1.3.16.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

2023-05-16.21-14-12.online-video-cutter.com.mp4

Recording.649.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a33b3f9d19c242e2
• Upwork Job ID: 1660556947910672384
• Last Price Increase: 2023-05-22

[melvin-bot]

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

[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

[ahmedGaber93]

Proposal

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

Error in console when pressing the "Login" button or the "Create a New Account" link in the footer of Magic Code page.

What is the root cause of that problem?

in Hoverable component There is no null check here for this.wrapperView which it may be null between re-render.
We use Hoverable in SignInPageLayout > surround the footer links.

App/src/components/Hoverable/index.js

Lines 100 to 105 in 2090c05

	onBlur={(el) => {
	if (this.wrapperView.contains(el.relatedTarget)) {
	return;
	}
	this.setIsHovered(false);
	}}

App/src/components/Hoverable/index.js

Lines 83 to 91 in 2090c05

	onBlur: (el) => {
	if (!this.wrapperView.contains(el.relatedTarget)) {
	this.setIsHovered(false);
	}
	if (_.isFunction(this.props.children.props.onBlur)) {
	this.props.children.props.onBlur(el);
	}
	},

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

add null check for this.wrapperView in onBlur statment.

onBlur={(el) => {
    if (!_.isNull(this.wrapperView) && this.wrapperView.contains(el.relatedTarget)) {
        return;
    }
    this.setIsHovered(false);
}}

What alternative solutions did you explore? (Optional)

[jliexpensify]

Commented here: https://expensify.slack.com/archives/C049HHMV9SM/p1684453691932129?thread_ts=1684258502.046179&cid=C049HHMV9SM

[jliexpensify]

Getting a second opinion from the team here - https://expensify.slack.com/archives/C01SKUP7QR0/p1684725923013839

UPDATE: Moving to external based off this post - https://expensify.slack.com/archives/C01SKUP7QR0/p1684424938812639

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[Harshdeepjoshi]

Proposal

(Originally posted on Slack with bug report)
The issue occurs due to the this.wrapperView being set to null by default.

App/src/components/Hoverable/index.js

Line 18 in 7ad2be0

this.wrapperView = null;

so when this check executes. https://github.com/Expensify/App/blob/7ad2be02a9e7c9dc19e8a6556d3c8efde5ba5721/src/components/Hoverable/index.js#LL101C17-L101C17
it gives error
Cannot read properties of null (reading 'contains') as the state might not be set yet and still be null

Solution

We can simply add a null check for this.wrapperView in onBlur statement. (here src/components/Hoverable/index.js) to ensure that this.wrapperView is not null while executing the condition

like this

if (this.wraperView != null && this.wrapperView.contains(el.relatedTarget)) {
return;
}

Result

2023-05-16.23-20-38.online-video-cutter.com.1.mp4

[VitaliiP-0205]

Proposal

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

Error in console when pressing the "Login" button or the "Create a New Account" link in the footer of Magic Code page.

What is the root cause of that problem?

The issue occurs due to the this.wrapperView being set to null by default.

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

This issue can solve by adding null check for this.wrapperView in onblur event.
line 101 in here: https://github.com/Expensify/App/blob/main/src/components/Hoverable/index.js
if (this.wraperView != null && this.wrapperView.contains(el.relatedTarget)) {
return;
}

What alternative solutions did you explore? (Optional)

[mollfpr]

@jliexpensify @dangrous I believe the compensation for console error is $500?

Proposal from @ahmedGaber93 looks good to me!

🎀 👀 🎀 C+ reviewed!

[dangrous]

Works for me! Assigning now..

[melvin-bot]

📣 @ahmedGaber93 You have been assigned to this job by @dangrous!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[Harshdeepjoshi]

@dangrous @mollfpr I first proposed similar solution on slack before everyone and I am reporter of this bug as well. I also linked the slack proposal on this issue as per the rules. Shouldn't I be selected for this job (like here) if my proposal is correct?

[mollfpr]

Note: whilst you may optionally propose a solution for that job on Slack, solutions are ultimately reviewed in GitHub. The onus is on you to propose the solution on GitHub, and/or ensure the issue creator will include a link to your proposal.

@Harshdeepjoshi Sorry, even though you posted the solution first in Slack, the reviewer wouldn't know that, and not possible for us to always check the Slack thread. I even didn't notice that you post a proposal in Slack, because you're not using the proposal template, and maybe that's the reason the issue creator didn't include your proposal.

The idea of leaving a proposal in your bug report is, the issue creator will post it right after the Github issue is created. So, the first proposal is still what we have in the GitHub issue, and following the proposal template is essential to understand your solution.

cc @dangrous @jliexpensify

[jliexpensify]

@mollfpr , in regards to this:

I believe the compensation for console error is $500?

Is there a link you can share where this was discussed? This is the first time I've come across a console error, cheers!

[mollfpr]

@jliexpensify I only have an example of it #17876 (comment)

But I'm not sure because this is not a DEV error 🤷‍♂️

[jliexpensify]

Cheers, getting some clarity internally here - https://expensify.slack.com/archives/C01SKUP7QR0/p1684817728719929?thread_ts=1684725923.013839&cid=C01SKUP7QR0

[ahmedGaber93]

@mollfpr PR is ready for review.

[jliexpensify]

@mollfpr - all good, we can keep these JS issues at $1000 for consistency's sake.

[mollfpr]

@dangrous @jliexpensify This issue is solved at #16052 that refactored the Hoverable component because delete this line https://github.com/Expensify/App/blob/7ad2be02a9e7c9dc19e8a6556d3c8efde5ba5721/src/components/Hoverable/index.js#LL101C17-L101C17 where the issue from. What's the action in this situation?

[dangrous]

Huh, good question. I will start a discussion!

[jliexpensify]

Good discussion @dangrous - it looks like we will pay this one.

It looks like the other PR is still ongoing - so maybe let's pay once it's actually deployed to prod and properly tested? @mollfpr and @ahmedGaber93 , can you test this and share an update? Thanks!

[ahmedGaber93]

@jliexpensify

Screen.Recording.2023-05-28.at.8.30.34.PM.mov

[jliexpensify]

Thanks for checking!

@mollfpr and @ahmedGaber93 - after discussing it with the team, @dangrous and I will pay 50% (i.e. $500), since this was resolved with the other PR. Sound good?

[mollfpr]

@jliexpensify sounds good to me!

[jliexpensify]

@ahmedGaber93 what's your Upworks profile? Cheers

[ahmedGaber93]

@jliexpensify sounds good to me, too.

[ahmedGaber93]

@jliexpensify https://www.upwork.com/freelancers/~012883934e2c4dc4e7

[jliexpensify]

Cheers, invites sent - will pay once accepted.

[jliexpensify]

Paid, and job closed - thanks everyone!