Issue/6850 add google signup by theck13 · Pull Request #6999 · wordpress-mobile/WordPress-Android

theck13 · 2017-12-15T03:03:53Z

Fix

Add Google signup when the Sign Up With Google button is pressed on the prologue screen as described in #6850.

These changes also extract the code specific to Google login and signup from the LoginGoogleFragment into a new GoogleFragment file to avoid code duplication. Now, the code in the LoginGoogleFragment and SignupGoogleFragment files only relate to the Google login and signup flow, respectively.

Test

New Account

Clear app data.
Tap Sign Up button.
Tap Sign Up with Google button.
Choose Google account without existing WordPress.com account.
Notice nothing happens. The next step will be handling in a subsequent pull request.

Old Account / Social Disconnected / Two-Factor Disabled

Clear app data.
Tap Sign Up button.
Tap Sign Up with Google button.
Choose Google account with existing WordPress.com account.
Notice message explaining an account exists and continue with login.
Enter WordPress.com account password.
Tap Next button.
Notice Epilogue screen is shown.

Old Account / Social Connected / Two-Factor Disabled

Clear app data.
Tap Sign Up button.
Tap Sign Up with Google button.
Choose Google account with existing WordPress.com account.
Notice Epilogue screen is shown.

Old Account / Social Disconnected / Two-Factor Enabled

Clear app data.
Tap Sign Up button.
Tap Sign Up with Google button.
Choose Google account with existing WordPress.com account.
Notice message explaining an account exists and continue with login.
Enter WordPress.com account password.
Tap Next button.
Notice Two-Factor Authentication screen is shown.
Enter two-factor authentication code.
Tap Next button.
Notice Epilogue screen is shown.

Old Account / Social Connected / Two-Factor Enabled

Clear app data.
Tap Sign Up button.
Tap Sign Up with Google button.
Choose Google account with existing WordPress.com account.
Notice message explaining an account exists and continue with login.
Notice Two-Factor Authentication screen is shown.
Enter two-factor authentication code.
Tap Next button.
Notice Epilogue screen is shown.

Note

There are two copy changes. One is the new timeout error message "Google took too long to respond. You may need to wait until you have a better connection." which occurs when Google login or signup fails due to taking too long. The second is the addition of a message that appears when users are taken from the signup flow to the login flow. It is an ephemeral message based off an existing Google login error. See the screenshot below for illustration.

…th do login update parameter

…ccount

…gnup

nbradbury · 2017-12-15T22:06:39Z

WordPress/src/main/java/org/wordpress/android/ui/accounts/login/LoginGoogleFragment.java

                            // Attempted to connect, but the user is not signed in.
                            case GoogleSignInStatusCodes.SIGN_IN_REQUIRED:
-                                AppLog.e(T.NUX, "Google Sign-in Failed: user is not signed in.");
+                                AppLog.e(T.NUX, "Google Login Failed: user is not signed in.");


The docs are confusing, but if I'm reading correctly then when SIGN_IN_REQUIRED is returned we may be able to continue with sign in (?)

Yeah, the terminology is a little confusing. We've tried to differentiate by using login when referring to an existing account and signup when talking about creating a new account. The term sign-in used by the API and Google documentation refers to both login and signup depending on the situation, but the SIGN_IN_REQUIRED in this case means login.

nbradbury · 2017-12-15T22:36:31Z

WordPress/src/main/java/org/wordpress/android/ui/accounts/signup/SignupGoogleFragment.java

+                // WordPress account exists with input email address, and two-factor authentication is required.
+                case TWO_STEP_ENABLED:
+                    AnalyticsTracker.track(AnalyticsTracker.Stat.SIGNUP_SOCIAL_2FA_NEEDED);
+                    ToastUtils.showToast(getContext(), getString(R.string.signup_user_exists, mGoogleEmail),


Can we shorten the length of signup_user_exists? Even with a long toast, it's not easy to read the whole thing before it disappears, especially when you're not expecting it.

Also, the phrase "continue with login" is a bit cryptic and/or unfriendly. Might want to ask Editorial for better copy.

Yep, I totally agree. I meant to add the [Status] Needs Copy Review label for wordsmithing help from Editorial. I'll add that and [Status] Needs Design Review.

michelleweber · 2017-12-16T00:01:43Z

This message feels ambiguous -- continue with the current process? Or go back to the login screen?

theck13 · 2017-12-16T00:37:11Z

This message feels ambiguous -- continue with the current process? Or go back to the login screen?

I'm guessing you're referring to the "The Google account 'heckofatest@gmail.com' matches an existing account on WordPress.com. Continue with login." message when the user is taken from the signup flow to the login flow, right? The user is automatically taken to the login flow when this message is shown. We wanted to add something to inform the user they are no longer in the signup flow to minimize confusion. Do you know how we can express that better?

What do you think about the "Google took too long to respond. You may need to wait until you have a better connection." message shown when a timeout error occurs while trying to login or signup with Google? See the screenshot below for illustration.

nbradbury · 2017-12-17T17:47:32Z

@theck13 I'm finished reviewing this and apart from the wording it all looks good. We can wait to merge until after the wording is finalized, or I can merge now and you could change the wording in the next PR - just let me know which you prefer.

michelleweber · 2017-12-18T18:04:43Z

The "Google took too long" message is good. I'd maybe say "stronger internet connection" instead of just "better connection."

For the other, I'm thinking something like "There's already a WordPress.com account using [email address] -- log in normally with your WordPress.com username and password."

Also -- on the login screen behind, it'd be great to change that second sentence to "We'll only ask for this once."

theck13 · 2017-12-18T18:30:13Z

Thanks, @michelleweber!

The "Google took too long" message is good. I'd maybe say "stronger internet connection" instead of just "better connection."

I'll update the timeout message to "Google took too long to respond. You may need to wait until you have a stronger internet connection."

For the other, I'm thinking something like "There's already a WordPress.com account using [email address] -- log in normally with your WordPress.com username and password."

I can change the message to that, but this message is also shown when the user is taken to the two-factor authentication screen, which has only a verification code field rather than username and password fields. Should we have one message for both cases or unique messages for each case?

folletto · 2017-12-18T18:59:37Z

Ok so if I understand correctly there are two messages to review:

When the connection to Google auth server times out.
When the user trying to signup is moved instead to login.

Also:

Is a modal window with just close as action.
Can we make this a snackbar at the bottom?

Given the above, I'd suggest the following:

Timeout—

Modal with "Back" button ("Close" is ok too if you prefer that)
"Unable to connect to Google. Try again later."
The reason for this copy is that while it's likely for being a connection error, a timeout could also happen on Google side, or due to routing issues of some kind, so I would suggest to make the user wait a bit instead of suggesting it's a fault of the connection.

Signup tp Login—

Snackbar at the bottom, with no action. Disappears after the default amount of time (5 seconds?). Can we?
"Email already exists on WordPress.com. Proceeding with log in."

The above still pending copy review of course. ;)

michelleweber · 2017-12-18T20:37:53Z

I can change the message to that, but this message is also shown when the user is taken to the two-factor authentication screen, which has only a verification code field rather than username and password fields. Should we have one message for both cases or unique messages for each case?

Unique, I think. Any instruction/guidance we give should be as specific to what the user is currently seeing as possible, so I'd want the 2FA messaging to be about entering a 2FA code to continue login.

theck13 · 2017-12-18T21:01:39Z

@folletto:

Ok so if I understand correctly there are two messages to review:

When the connection to Google auth server times out.

When the user trying to signup is moved instead to login.

Yes, those are the two scenarios and both of your suggestions are possible as you described them.

@michelleweber:
How is the wording @folletto described above? One thing to note about the suggested design is the length of the messages. The timeout dialog is meant to contain a sentence or two summarizing the situation. The signup-to-login snackbar should be a very short text string containing a single line of text directly related to the operation performed.

theck13 · 2017-12-18T21:31:39Z

@folletto and @michelleweber:
When testing the "Email already exists on WordPress.com. Proceeding with login." on the Pixel and Pixel 2, the message is actually two lines. Would "Email already exists. Proceeding with login." work or something similar in length to keep the message to a single line?

folletto · 2017-12-19T13:31:34Z

the message is actually two lines

Yeh, I'd break on two lines on the ".":

Email already exists on WordPress.com.
Proceeding with log in.

I think given it's an automated message out of the part of the screen that the user is already looking at, having two lines raised a bit its visual profile. :)

theck13 · 2017-12-19T14:52:50Z

I was suggesting keeping the message to a single line to follow the design guidelines. Just to be clear, you are suggesting forcing the message to be two lines intentionally?

folletto · 2017-12-19T15:13:43Z

Yes, the guidelines include two lines support (80dp):

theck13 · 2017-12-19T15:30:17Z

Hurray for contradictory guidelines!

Snackbars should contain a single line of text directly related to the operation performed.

Multi-line snackbar height: 80dp

I'll use the "Email already exists on WordPress.com. Proceeding with login." and force the line break after the "WordPress.com." so that it always looks like this.

folletto · 2017-12-19T16:15:55Z

Hehe yeh. I think they updated in two steps and never revised the whole doc. :D

I'll use the "Email already exists on WordPress.com. Proceeding with login." and force the line break after the first period so that it always looks like this.

Cool!

michelleweber · 2017-12-19T20:00:33Z

These messages are good. My preference is always to go more natural-speech-y, but space doesn't always allow it!

… from signup to login

nbradbury · 2017-12-20T22:38:47Z

Looks good, let's !

theck13 added 23 commits December 13, 2017 09:21

Move Google API code to separate base fragment

0a3ddca

Update application log messages for Google login errors

8e08f9a

Add Google signup fragment

8e2233f

Add Google signup fragment on sign up with Google button tap

a859630

Refactor Google listener interface name

5c37558

Update Google listener interface with signup finished method

0511fb7

Add name, email, and photo handling to Google signup fragment

ec11153

Add social signup with FluxC

daec75c

Update login via social account method in login listener interface wi…

69f7739

…th do login update parameter

Update logged in and finish method with do login update parameter

0ec70ff

Add argument to do login update on epilogue screen

12d9c5c

Add social signup flow when account is created

c60e520

Add social login flow when signing up with an existing connected account

0b13e87

Add social login flow when signing up with an existing disconnected a…

1098a66

…ccount

Add 2FA flow when signing up with an existing 2FA-enabled account

c8dddbd

Add application logging for Google signup

b131ce4

Add analytics tracking for Google signup

cddab28

Update signup sheet dismiss listener to cancel listener

5f63c0d

Update analytics for signup bottom sheet

5c20094

Update signup sheet to dismiss rather than hide

e072edf

Add method to dismiss signup sheet in interface methods related to si…

317cb2b

…gnup

Add null checks to Google sign-in account usages

56951fa

Add killing signup fragment when changing flow from signup to login

3f2d030

theck13 added Signup [Type] Enhancement labels Dec 15, 2017

Merge branch 'feature/new-signup' into issue/6850-add-google-signup

d56b08b

aerych mentioned this pull request Dec 15, 2017

Signup: Master Issue #6838

Closed

10 tasks

Merge branch 'feature/new-signup' into issue/6850-add-google-signup

972d8ed

nbradbury self-assigned this Dec 15, 2017

nbradbury reviewed Dec 15, 2017

View reviewed changes

Add timeout error case to social login and signup

12ea33f

theck13 added [Status] Needs Copy Review [Status] Needs Design Review A designer needs to sign off on the implemented design. labels Dec 15, 2017

theck13 added 4 commits December 19, 2017 15:12

Update timeout error message for social login and signup

cd19ab7

Update message when switching from social signup to login

d025f42

Add method to login listener interface to show message when switching…

f29429f

… from signup to login

Merge branch 'feature/new-signup' into issue/6850-add-google-signup

94f2f68

nbradbury merged commit 1c08102 into feature/new-signup Dec 20, 2017

nbradbury deleted the issue/6850-add-google-signup branch December 20, 2017 22:38

theck13 removed [Status] Needs Copy Review [Status] Needs Design Review A designer needs to sign off on the implemented design. labels Dec 21, 2017

theck13 mentioned this pull request Aug 29, 2018

Issue/8225 fix google sign up #8252

Merged

rachelmcr mentioned this pull request Sep 6, 2018

Signup: Snackbar message cut off when Google email linked to existing WordPress.com account #8283

Closed

Conversation

theck13 commented Dec 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Test

Note

Uh oh!

nbradbury Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

theck13 Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

nbradbury Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

theck13 Dec 15, 2017

Choose a reason for hiding this comment

Uh oh!

michelleweber commented Dec 16, 2017

Uh oh!

theck13 commented Dec 16, 2017

Uh oh!

nbradbury commented Dec 17, 2017

Uh oh!

michelleweber commented Dec 18, 2017

Uh oh!

theck13 commented Dec 18, 2017

Uh oh!

folletto commented Dec 18, 2017

Uh oh!

michelleweber commented Dec 18, 2017

Uh oh!

theck13 commented Dec 18, 2017

Uh oh!

theck13 commented Dec 18, 2017

Uh oh!

folletto commented Dec 19, 2017

Uh oh!

theck13 commented Dec 19, 2017

Uh oh!

folletto commented Dec 19, 2017

Uh oh!

theck13 commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

folletto commented Dec 19, 2017

Uh oh!

michelleweber commented Dec 19, 2017

Uh oh!

nbradbury commented Dec 20, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

theck13 commented Dec 15, 2017 •

edited

Loading

theck13 commented Dec 19, 2017 •

edited

Loading