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

[SIWA] Show Apple credentials request. #12336

Merged
merged 6 commits into from Aug 20, 2019

Conversation

@ScoutHarris
Copy link
Contributor

commented Aug 18, 2019

Fixes # n/a
WPAuth PR: wordpress-mobile/WordPressAuthenticator-iOS#115

This uses the WPAuth changes in the above PR to show the Apple ID credentials request.

To test:
Run the app in Xcode 11.

  • Log out if necessary.
  • Select Log In > Continue with Apple.
  • The 'Sign In with Apple' process is presented until valid credentials are entered or 'Cancel' is selected.
  • Since account creation is not yet implemented, for now the user's email and full name will be logged in the console. Ex:
Apple Authenticator credentials: Optional("useremail@gmail.com"), Optional(givenName: First familyName: Last )

siwa_flow

Run the app in Xcode 10. Verify the app runs and does not show the 'Continue with Apple' login button.

Update release notes:

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

This comment has been minimized.

Copy link
Contributor Author

commented Aug 18, 2019

@mattmiklic - just fyi - that darker background in the SIWA flow is Apple's doing. And I don't see that there's any way to change it.

Also, if you'd like a video of this just let me know and I can DM it to you. I didn't add one here as it clearly shows credentials.

@mattmiklic

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

Hey @ScoutHarris this is looking pretty good. Two notes:

  • The "Or log in by entering your site address" subheadline should be 15pt by default, but it looks like it's 11pt right now.
  • Re: the darker background in the SIWA flow, I'm wondering if we're displaying Apple's scrim on top of the scrim we add in the step where you choose a login option? I think that screen would work fine without a scrim, and then Apple could display their own for their flow without it becoming too dark.

@ScoutHarris ScoutHarris changed the base branch from feature/show_siwa_button to develop Aug 19, 2019

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

Hey @mattmiklic .

The "Or log in by entering your site address" subheadline should be 15pt by default, but it looks like it's 11pt right now.

Woops. Fixed.

Re: the darker background in the SIWA flow, I'm wondering if we're displaying Apple's scrim on top of the scrim we add in the step where you choose a login option? I think that screen would work fine without a scrim, and then Apple could display their own for their flow without it becoming too dark.

The login button view is dismissed, which includes our scrim. Then the ASAuthorizationController is displayed.

siwa_view

@mattmiklic

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

The login button view is dismissed, which includes our scrim. Then the ASAuthorizationController is displayed.

Ok got it, thanks! Let's roll with that then. Text link looks good now. 👍

@mattmiklic mattmiklic removed their request for review Aug 19, 2019

@ScoutHarris

This comment has been minimized.

Copy link
Contributor Author

commented Aug 19, 2019

Thanks @mattmiklic !

@astralbodies ready for a review when you have a mo! Thanks!

@ScoutHarris ScoutHarris requested a review from frosty Aug 20, 2019

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

left a comment

Looks good to me! 👍

I'm not sure if we knew this already, but it seems to me that we only get provided the user's email address on account creation, not sign in. Just something to bear in mind 🤔

@ScoutHarris ScoutHarris merged commit 7eb08e8 into develop Aug 20, 2019

6 checks passed

Hound No violations found. Woof!
Peril All green. Yay.
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/show_siwa_request branch Aug 20, 2019

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