Adds email/password “email-already-in-use” flow when upgrading to a federated credential #410
Conversation
| [self.navigationController dismissViewControllerAnimated:YES completion:^{ | ||
| [self.authUI invokeResultCallbackWithAuthDataResult:authResult error:error]; | ||
| if (self->_onDismissCallback) { | ||
| self->_onDismissCallback(authResult, error); |
There was a problem hiding this comment.
It is usually a good idea to nil-out callback blocks after use to avoid circular references.
There was a problem hiding this comment.
I went with the property approach outlined in you other comment. Since the block will be reused I leave it up to the developer using a custom block to decide whether or not it should be set to nil.
| in this block if necessary. | ||
| @param callback The custom callback to execute during dismissal of the view controller. | ||
| */ | ||
| - (void)setOnDismissCallback:(FIRAuthDataResultCallback)callback; |
There was a problem hiding this comment.
I would just make it a property called completion, with initial value being the what we're doing by default. If you do that, make sure you only capture weak self in the block.
Alternatively you can use the delegate pattern but that might be too heavy-handed if you don't expect more methods in the delegate.
morganchen12
left a comment
morganchen12
left a comment
There was a problem hiding this comment.
Some minor notes, otherwise looks good.
|
|
||
| - (void)signInWithEmailHint:(NSString *)emailHint | ||
| presentingViewController:(UIViewController *)presentingViewController | ||
| presentingViewController:(FUIAuthBaseViewController *)presentingViewController |
There was a problem hiding this comment.
It seems like the only method being used on these controllers is pushViewController:, in which case UIViewController should be ok. If this is public API, we want to provide the highest-level types possible.
There was a problem hiding this comment.
signInWithEmailHint:presentingViewController:completion: isn't a public API, it's only used for a special case of automatic upgrade behind the scenes. The pushViewController: method being used here is a custom method which modifies the way the navigation bar is presented when the view controller is pushed.
FirebaseAuthUI/FUIAuth.m
Outdated
| NSError *_Nullable error, | ||
| FIRAuthResultCallback _Nullable result) { | ||
| if (error) { | ||
| completion(nil, error); |
There was a problem hiding this comment.
completion may be nil.
| /** @var _onDismissCallback | ||
| @brief The callback to be executed during view controller dismissal. | ||
| */ | ||
| FIRAuthDataResultCallback _onDismissCallback; |
There was a problem hiding this comment.
A better name might be onSuccessfulLoginCallback, since this block will not be called if the controller is dismissed by exiting the login flow.
…ederated credential (#410) * Adds email/password “email-already-in-use” flow * addresses commnets * Addresses comments
3 participants
Adds flow to sign into an existing email/password user (on a separate auth instance) before linking this user to the federated auth credential during a merge conflict while upgrading anonymous user.