-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[NT-735] Sign in with Apple #1165
Conversation
…feature-sign-in-with-apple
…feature-sign-in-with-apple
Generated by 🚫 Danger |
* Adding ProcessingViewPresenting * Adding tests for isLoading behavior * Revert code signing * Adding voice over support * Adding accessibility labels to the logos * Automatic signing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we're good here! Just some minor (optional) suggestions.
if let error = self.signInWithAppleResult?.error { | ||
return SignalProducer(error: error) | ||
} | ||
return SignalProducer(value: self.signInWithAppleResult?.value ?? SignInWithAppleEnvelope.template) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really think we should ever return defaults from these things 🤷♂️ but I know it's been done in a couple places.
@@ -0,0 +1,24 @@ | |||
import Foundation | |||
|
|||
public struct SignInWithAppleInput: GraphMutationInput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we conform SignInWithAppleInput
to Encodable
then we don't need the toInputDictionary()
function below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! Also noticed we were missing tests for this, so I added those too.
…feature-sign-in-with-apple # Conflicts: # Kickstarter-iOS/Views/Controllers/PledgeViewController.swift
📲 What
Contains the PRs with the Sign In With Apple work
#1131 - Continue with Apple UI
#1152 - (1/2) Sign in with Apple mutation
#1158 - (2/2) Sign in with Apple
#1163 - Sign in with Apple event
#1168 - Add Loader to Login/Signup Page
⏰ TODO
👋
![GIF](https://camo.githubusercontent.com/3a4b8a7538650912b08c59d45312e62c816b8191c5557320ed57fb9b064ca37c/68747470733a2f2f6d656469612e67697068792e636f6d2f6d656469612f51306765426e364e4667514f412f67697068792e676966)