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

Log In: Display new login flow #12305

Merged
merged 6 commits into from Aug 14, 2019

Conversation

@ScoutHarris
Copy link
Contributor

commented Aug 13, 2019

Fixes #n/a
WPAuthenticator PR: wordpress-mobile/WordPressAuthenticator-iOS#111

This shows the new Log In flow added in the above WPAuth PR.

Previously, when 'Log In' was selected, the email entry view was immediately displayed. Now when 'Log In' is selected, a button view now appears with available options. These options then display the appropriate view.


Old Log In
old_flow

New Log In
new_flow


To test:

  • Run the app. Log out if necessary.
  • Select 'Log In'.
  • A button view should appear with log in options (as shown above).
    • Selecting 'Continue with Google' launches the existing Google auth flow.
    • Selecting log in by site address launches the existing self-hosted flow.
    • Select 'Continue with WordPress.com' shows a slightly modified email entry view.

WordPress view:

  • The instruction text has been modified.
  • 'Log in with Google' has been removed.
  • Only the log in by site address option is displayed.

login_wp

Each login flow still follows their respective paths, so logging in should not be affected.

Old flow:

  • To verify the old flow still works as expected:
    • In WordPressAuthenticationManager:initializeWordPressAuthenticator, change showNewLoginFlow to false.
    • Rerun the app. The old 'Log In' flow should display (as shown above).

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@ScoutHarris ScoutHarris added the Login label Aug 13, 2019

@ScoutHarris ScoutHarris added this to the 13.2 milestone Aug 13, 2019

@ScoutHarris ScoutHarris requested review from frosty, nheagy and mattmiklic Aug 13, 2019

@ScoutHarris ScoutHarris self-assigned this Aug 13, 2019

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 13, 2019

@mattmiklic - this is what I'm going to call "v1" of the log in redesign. Meaning, it's my plan to use this flow to implement Sign In with Apple. After SIWA is complete, I'll circle back around to the remaining changes from the design.

@ScoutHarris ScoutHarris requested a review from rachelmcr Aug 14, 2019

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

Hey @rachelmcr . I added you as a reviewer if you'd like to take a look at the UI test changes for the new flow. Thanks!

@rachelmcr
Copy link
Member

left a comment

The updates to the UI tests are 💯 ! I especially like that the two login tests that use the site address flow now test both paths to the site address screen.

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

Thanks @rachelmcr !

@mattmiklic

This comment has been minimized.

Copy link
Member

commented Aug 14, 2019

@mattmiklic - this is what I'm going to call "v1" of the log in redesign. Meaning, it's my plan to use this flow to implement Sign In with Apple. After SIWA is complete, I'll circle back around to the remaining changes from the design.

This looks great! Plan makes sense. When you circle back around to pick up the remaining stuff, could you see if you can match the margin below the email input field to the margin above it? I think there should be ~16pt of space between the input and the "Or log in by entering your site address" text.

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 14, 2019

Thanks @mattmiklic !

When you circle back around to pick up the remaining stuff, could you see if you can match the margin below the email input field to the margin above it? I think there should be ~16pt of space between the input and the "Or log in by entering your site address" text.

Ah, now that you mention it, it's obvious now. Yep, can do!

@frosty
frosty approved these changes Aug 14, 2019
Copy link
Contributor

left a comment

Looks great! :shipit: once the Authenticator changes are merged, pod is released, and Podfile updated :)

@ScoutHarris ScoutHarris merged commit ff68d6e into develop Aug 14, 2019

6 checks passed

Hound No violations found. Woof!
Peril All green. Well done.
Details
ci/circleci: Build UI Tests Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPad 6th generation) Your tests passed on CircleCI!
Details
ci/circleci: UI Tests (iPhone Xs) Your tests passed on CircleCI!
Details
ci/circleci: build_and_test Your tests passed on CircleCI!
Details

@ScoutHarris ScoutHarris deleted the feature/display_new_login_flow branch Aug 14, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.