Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@mindgraffiti
Copy link
Contributor

@mindgraffiti mindgraffiti commented Apr 6, 2020

Fixes #233

This PR fixes the black screen bug.

PR to test on WPiOS: wordpress-mobile/WordPress-iOS#13827

Summary

After the Jetpack install flow, the user is asked to sign in again. The LoginEmailViewController is presented, and there text and a link below the email field that says, "Don't have an account? Sign up"

Tapping the "Sign up" link pushes the 3 button view instead of presenting the 3 button view. Pushing the nav creates the black screen (seen in Figure 2 below).

Steps to Reproduce

  1. Have a self-hosted site that doesn't have Jetpack connected. (This also works if you go to the site, Jetpack > and scroll to the bottom to find the "Disconnect" button and click it.)
  2. Log out of the WPiOS app if you are logged in.
  3. Log into the app through the self-hosted flow: Login > enter site address text link button
  4. After completing the login flow, go to the Notifications tab
  5. Follow the Jetpack install flow. Once it is successful, it will ask you to set up Jetpack.
  6. The "Set up" Jetpack button should navigate you to the email login screen. (See Figure 1 below)
  7. Tap the "Sign up" text link button.
    Expected: the 3 button view appears as an overlay to the email login screen. (See Figure 3)
    Actual: the 3 button view is pushed and the background is black.

Figure 1

Figure 2

Figure 3

@mindgraffiti mindgraffiti added the bug Something isn't working label Apr 6, 2020
@mindgraffiti mindgraffiti added this to the 1.12.0 milestone Apr 6, 2020
@mindgraffiti mindgraffiti self-assigned this Apr 6, 2020
Comment on lines +566 to +569
func showWPComLogin(loginFields: LoginFields) {
self.loginFields = loginFields
performSegue(withIdentifier: .showWPComLogin, sender: self)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should say // no-op? Because in this context: coming from Jetpack Remote Install > Sign up > Continue with Apple, it would never call the showWPComLogin() method...right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do believe showWPComLogin is needed. It's nothing to do with Jetpack at this point, it's all about the SIWA flow.

It pertains to this logic in AppleAuthenticator:

guard !existingNonSocialAccount else {

    if existing2faAccount {
        self?.show2FA()
        return
    }

    self?.updateLoginEmail(wpcomUsername)
    self?.logInInstead()
    return
}

Basically, what happens here is if an Apple ID is used where the email has already been used on a WP account, the flow will re-direct to WP login.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ohhh. Nice!

Copy link
Contributor

@ScoutHarris ScoutHarris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mindgraffiti
Copy link
Contributor Author

Thanks @ScoutHarris !

@mindgraffiti mindgraffiti merged commit 65d31b4 into master Apr 6, 2020
@mindgraffiti mindgraffiti deleted the fix/233-black-screen branch April 6, 2020 21:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants