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

[Hold for payment 5/7] Blank screen after creating an account #2497

Closed
kidroca opened this issue Apr 20, 2021 · 12 comments · Fixed by #2575 or #2649
Closed

[Hold for payment 5/7] Blank screen after creating an account #2497

kidroca opened this issue Apr 20, 2021 · 12 comments · Fixed by #2575 or #2649
Assignees
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kidroca
Copy link
Contributor

kidroca commented Apr 20, 2021

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Expected Result:

User proceeds to expensify.cash chat list

Actual Result:

User is redirected to a blank screen and a JS error can be seen in the console

Action Performed:

  1. Start from logged out state and proceed to create a new account
  2. While logged out, type and login with brand new email address so it can trigger sign up flow
  3. Get validation link from e-mail. On dev I'm copying the email link and appending it to localhost - e.g. http://localhost:8080/setpassword/10024706/EHOHMPGWM
  4. Type new password and click "set password"
  5. User is redirected to a blank screen and a JS error can be seen in the console

Workaround:

Refreshing the page manually would fix the issue and the chat list would load

Platform:

Where is this issue occurring?
The issue is confirmed in:

Web ✔️
Android ✔️
iOS
Desktop App
Mobile Web

The other environments are probably affected as well

Version Number: 1.0.21-0
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Bug5020701_video.mp4

Expensify/Expensify Issue URL:

View all open jobs on Upwork

@kidroca
Copy link
Contributor Author

kidroca commented Apr 20, 2021

@marcaaron This is a follow up on the problem discovered in #2346 (comment)

Proposal

As per my comment here #2346 (comment) this was traced to a call for Navigation while the navigation component have no navigators (stacks or other) mounted. This would result in the error printed on the console "Error: The 'navigation' object hasn't been initialized yet."

This issue occurs only during initialization.

While navigation is not yet ready an incoming call can be captured and scheduled for when the navigator components mount. This would be shortly after the call. As @marcaaron suggested here: #2346 (comment)

  1. Allow NavigationContainer to initialize with no report
  2. React to the NavigationContainer.onReady event and perform execute any captured navigation action

The navigate function here can be stubbed while Navigation is not yet ready:

https://github.com/Expensify/Expensify.cash/blob/dba06b5ced71edc7ae1663179ca808162635b0fd/src/libs/Navigation/Navigation.js#L100-L105

Navigation.js
let lastAttemptedRoute;

function navigate(route) {
 // unmodified
}

function navigateLater(route) {
  lastAttemptedRoute = route;
}

function enableNavigation() {
  if (lastAttemptedRoute) navigate(lastAttemptedRoute);
    exports.navigate = navigate;
}

const exports = {
  navigate: navigateLater,
  enableNavigation,
};

export default exports;

And then we call enableNavigation here:

https://github.com/Expensify/Expensify.cash/blob/dba06b5ced71edc7ae1663179ca808162635b0fd/src/libs/Navigation/NavigationRoot.js#L37-L45

NavigationRoot.js
<NavigationContainer
    onReady={enableNavigation} 
    ...
/>

A note on exports.navigate = navigate.

Changing the implementation of navigate during runtime saves us from:

  • dirtying up the actual navigate code.
  • introducing bugs there
  • make a needles check for the lifetime of the navigation

Alternative

A simpler alternative is to just prevent navigation while it's not yet ready.
It is suggested by react navigation as a simple solution to the problem during initialization: https://reactnavigation.org/docs/navigating-without-navigation-prop/#handling-initialization

@marcaaron
Copy link
Contributor

Proposed solution makes sense to me. Let's do it!
Going to assign you and hand this off to a team member.

@marcaaron marcaaron added the External Added to denote the issue can be worked on by a contributor label Apr 20, 2021
@MelvinBot
Copy link

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

@marcaaron
Copy link
Contributor

@arielgreen can we get some help creating a job here? No need to add Exported label as I've already triaged this one and @kidroca has a good solution. Thanks!

@kidroca
Copy link
Contributor Author

kidroca commented Apr 22, 2021

Bump, I haven't started working on this as I'm not yet hired on Upwork

@roryabraham roryabraham added the DeployBlockerCash This issue or pull request should block deployment label Apr 23, 2021
@roryabraham roryabraham added the Daily KSv2 label Apr 23, 2021
@roryabraham
Copy link
Contributor

Adding the DeployBlockerCash label to this, @marcaaron if you disagree please remove the label.

@arielgreen Making this a daily since it's preventing E.cash production deploy.

@arielgreen
Copy link
Contributor

Thanks @roryabraham!

@kidroca, I've just sent over an invite to the Upwork job.

@kidroca
Copy link
Contributor Author

kidroca commented Apr 23, 2021

@arielgreen
I've got nothing new in Upwork, are you sure you have the correct profile? This is it: https://www.upwork.com/o/profiles/users/~01afad6ac000e0322f/

@arielgreen
Copy link
Contributor

@kidroca yep, correct profile. Can you see this job?

@kidroca kidroca changed the title Blanks screen after creating an account Blank screen after creating an account Apr 26, 2021
@marcaaron marcaaron removed the DeployBlockerCash This issue or pull request should block deployment label Apr 29, 2021
@marcaaron
Copy link
Contributor

Not a blocker but left a comment as I think it's something we should follow up on before considering this issue "closed".

#2575 (comment)

@marcaaron marcaaron reopened this Apr 30, 2021
@arielgreen arielgreen changed the title Blank screen after creating an account [Hold for payment 5/7] Blank screen after creating an account May 3, 2021
@arielgreen
Copy link
Contributor

Reopening, will close upon payment 5/7.

@arielgreen arielgreen reopened this May 3, 2021
@arielgreen
Copy link
Contributor

Paid contract + bonus for proposing the issue. Thanks @kidroca.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
5 participants