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

[expo-apple-auth] Add “Sign up with Apple” option #7471

Merged
merged 3 commits into from Apr 10, 2020

Conversation

IjzerenHein
Copy link
Contributor

Why

Add support for "Sign up with Apple" to expo-apple-authentication.
This option was added by Apple in iOS 13.2, but wasn't available to our users yet.

How

Added this option to the expo-apple-authentication package. As well as the NCL and to the docs.

Renders an empty view when used on iOS 13.1 or lower.

Test Plan

New option has been added to the NCL test app:

image
image
image

Because this option is only available on iOS 13.2, running this on a lower iOS version will render an empty view (screenshot is from iOS 13.1):

image

Other

Fixes #7454

PR was created together with @jkhales during a pair programming session.

@ExpoBot
Copy link

ExpoBot commented Mar 25, 2020

Warnings
⚠️

You modified sdk/apple-authentication.md in the unversioned directory. Please consider copying the changes to the latest released version if applicable.

Generated by 🚫 dangerJS against b04346e

@brentvatne
Copy link
Member

brentvatne commented Mar 25, 2020

we should wait until after sdk 37 is released before merging this because we don't want to pull the docs changes in here from unversioned to 37

@IjzerenHein
Copy link
Contributor Author

The iOS build is currently failing because the new SignUp constants are not recognized on CI.
It gives this error:

/Users/distiller/project/packages/expo-apple-authentication/ios/EXAppleAuthentication/EXAppleAuthenticationButtonViewManagers.m
:
83
:
1
:
 
error
: 
use of undeclared identifier 'ASAuthorizationAppleIDButtonTypeSignUp'; did you mean 'ASAuthorizationAppleIDButtonTypeSignIn'?

EX_REGISTER_APPLE_AUTHENTICATION_VIEW_MANAGER(SignUp, White, ExpoAppleAuthenticationButtonSignUpWhite, 13.2)

^

/Users/distiller/project/packages/expo-apple-authentication/ios/EXAppleAuthentication/EXAppleAuthenticationButtonViewManagers.m
:
22
:
136
:
 
note
: 
expanded from macro 'EX_REGISTER_APPLE_AUTHENTICATION_VIEW_MANAGER'

  - (UIView *)view { if (@available(iOS ios_version, *)) { return [[EXAppleAuthenticationButton alloc] initWithAuthorizationButtonType:ASAuthorizationAppleIDButtonType ## type authorizationButtonStyle:ASAuthorizationAppleIDButtonStyle ## style]; } else { return nil; } } \

                                                                                                                                       ^

<scratch space>
:
79
:
1
:
 
note
: 
expanded from here

ASAuthorizationAppleIDButtonTypeSignUp

^

In module '
AuthenticationServices
' imported from 
/Users/distiller/project/packages/expo-apple-authentication/ios/EXAppleAuthentication/EXAppleAuthenticationButton.h
:
5
:
/Applications/Xcode-11.GM.Seed.2.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator13.0.sdk/System/Library/Frameworks/AuthenticationServices.framework/Headers/ASAuthorizationAppleIDButton.h

I think we need to upgrade XCode on the build server, What do you think @sjchmiela ?

@esamelson
Copy link
Contributor

@nicknovitski would you be able to help out with upgrading Xcode in CI, when you get a chance? for this PR we need at least Xcode 11.2.

@tsapeta
Copy link
Member

tsapeta commented Apr 9, 2020

@IjzerenHein I've upgraded Xcode on the CI to 11.4.0 (#7316), it's not merged yet but should be soon 😉

EDIT: it landed!

Copy link
Member

@tsapeta tsapeta left a comment

Choose a reason for hiding this comment

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

Approving, but please add changelog (packages/expo-apple-authentication/CHANGELOG.md) entry as well 🙏

@darrylyoung
Copy link
Contributor

Thanks for making this happen, guys.

It wasn't that long ago when I reported this so I really appreciate the quick turnaround. 🙏

@IjzerenHein IjzerenHein merged commit ff0e4f5 into master Apr 10, 2020
@IjzerenHein IjzerenHein deleted the @hein/apple-auth-signup branch April 10, 2020 15:52
@darrylyoung
Copy link
Contributor

Hi, @IjzerenHein. Should this be part of the recently-released 2.1.1? I just updated and I see that on master the new option type is there but I'm not seeing it locally. If I dig into the node_modules, I can see that AppleAuthentication.types.d.ts still looks like this:

export declare enum AppleAuthenticationButtonType {
    SIGN_IN = 0,
    CONTINUE = 1
}

Am I missing something? Thank you.

@IjzerenHein
Copy link
Contributor Author

Hi Darryl. No this has not been released yet.

@darrylyoung
Copy link
Contributor

Hi Darryl. No this has not been released yet.

Got it. Thanks anyway. Have a good week. ✌️

@darrylyoung
Copy link
Contributor

Hi again, @IjzerenHein. Will this be released at some point soon? Thanks!

@IjzerenHein
Copy link
Contributor Author

Yep this will be released in SDK 38 which is just about to enter QA and testing. Not sure what the exact release date will be, but should be within a couple weeks max

@darrylyoung
Copy link
Contributor

Great. Thank you, @IjzerenHein. Much appreciated.

@raRaRa
Copy link

raRaRa commented Sep 2, 2020

Hey all. Is there a good way to test the configuration for Sign in with Apple through the Expo app on iOS? I tried calling the https://appleid.apple.com/auth/token endpoint to get access & refresh token, but I keep getting invalid_client error. What would be the best way to test my config? Should I use TestFlight?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AppleAuthentication – where is the “Sign up with Apple” option?
7 participants