Add default phone number to the Phone Auth number flow. #334
Conversation
| } | ||
|
|
||
| - (void)signInWithDefaultValue:(nullable NSString *)defaultValue | ||
| presentingViewController:(nullable UIViewController *)presentingViewController |
FirebaseGoogleAuthUI/FUIGoogleAuth.m
Outdated
| } | ||
|
|
||
| - (void)signInWithDefaultValue:(nullable NSString *)defaultValue | ||
| presentingViewController:(nullable UIViewController *)presentingViewController |
| a raw phone number and country code. | ||
| @return object or nil if any of the required parameters is nil. | ||
| */ | ||
| - (instancetype)initWithNormalizedPhoneNumber:(NSString *)normalizedPhoneNumber; |
There was a problem hiding this comment.
These three initializers need to return nullable instancetype, not instancetype.
| @param countryCode (required) The country code information | ||
| @return object or nil if any of the required parameters is nil. | ||
| */ | ||
| - (instancetype)initWithRawPhoneNumber:(NSString *)rawPhoneNumber |
There was a problem hiding this comment.
nullable instancetype
| @param countryCode (required) The country code information | ||
| @return object or nil if any of the required parameters is nil. | ||
| */ | ||
| - (instancetype)initWithNormalizedPhoneNumber:(NSString *)normalizedPhoneNumber |
There was a problem hiding this comment.
nullable instancetype
|
|
||
| - (instancetype)init __unavailable; | ||
|
|
||
| - (BOOL)validate:(NSError **)error; |
There was a problem hiding this comment.
Please add docs comments to this method.
| NSParameterAssert(rawPhoneNumber); | ||
| NSParameterAssert(countryCode); | ||
| if (!normalizedPhoneNumber || !rawPhoneNumber || !countryCode){ | ||
| return nil; |
There was a problem hiding this comment.
this return statement should be unreachable given the assertions above.
There was a problem hiding this comment.
If this is the only point where we return nil and it's unreachable, maybe the method signature should be modified to return a nonnull type and this code should be removed.
| if (error) { | ||
| *error = [NSError errorWithDomain:FUIPhoneNumberValidationErrorDomain | ||
| code:FUIPhoneNumberValidationErrorMissingPlus | ||
| userInfo:nil]; |
There was a problem hiding this comment.
We should be adding userInfo dictionaries with description keys, so meaningful error messages appear when the errors are logged.
| if (error) { | ||
| *error = [NSError errorWithDomain:FUIPhoneNumberValidationErrorDomain | ||
| code:FUIPhoneNumberValidationErrorMissingDialCode | ||
| userInfo:nil]; |
| - (instancetype)initWithNibName:(nullable NSString *)nibNameOrNil | ||
| bundle:(nullable NSBundle *)nibBundleOrNil | ||
| authUI:(FUIAuth *)authUI { | ||
| return [self initWithNibName:NSStringFromClass([self class]) |
There was a problem hiding this comment.
Should this initializer ignore the nib and bundle parameters?
|
Morgan, thanks for all your comments! |
|
|
||
| @implementation FUIPhoneNumber | ||
|
|
||
| - (nullable instancetype)initWithNormalizedPhoneNumber:(NSString *)normalizedPhoneNumber { |
There was a problem hiding this comment.
This function takes a nonnull normalizedPhoneNumber, so maybe instead of returning nil we can assert and return a nonnull type as well.
| - (nullable instancetype)initWithNormalizedPhoneNumber:(NSString *)normalizedPhoneNumber | ||
| rawPhoneNumber:(NSString *)rawPhoneNumber | ||
| countryCode:(FUICountryCodeInfo *)countryCode { | ||
| if (!normalizedPhoneNumber || !rawPhoneNumber || !countryCode){ |
There was a problem hiding this comment.
Same as the other initializer, all of these parameters are expected to be nonnull.
| return self; | ||
| } | ||
|
|
||
| - (nullable instancetype)initWithRawPhoneNumber:(NSString *)rawPhoneNumber |
There was a problem hiding this comment.
If the other two initializers are no longer failable, this one can be marked nonnull as well.
|
@morganchen12 Done! |
Related issue: #306