-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[google_sign_in] Fix deprecated API usage issue by upgrading CocoaPod to 5.0 #2127
Changes from 4 commits
710283b
9b7eac5
89187b5
46d23e3
f0bb3ce
52ed349
34abc18
2faa78b
bb5c3eb
e3a79a8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ | |
details:error.localizedDescription]; | ||
} | ||
|
||
@interface FLTGoogleSignInPlugin () <GIDSignInDelegate, GIDSignInUIDelegate> | ||
@interface FLTGoogleSignInPlugin () <GIDSignInDelegate> | ||
@end | ||
|
||
@implementation FLTGoogleSignInPlugin { | ||
|
@@ -40,16 +40,16 @@ + (void)registerWithRegistrar:(NSObject<FlutterPluginRegistrar> *)registrar { | |
FlutterMethodChannel *channel = | ||
[FlutterMethodChannel methodChannelWithName:@"plugins.flutter.io/google_sign_in" | ||
binaryMessenger:[registrar messenger]]; | ||
FLTGoogleSignInPlugin *instance = [[FLTGoogleSignInPlugin alloc] init]; | ||
FLTGoogleSignInPlugin *instance = [[FLTGoogleSignInPlugin alloc] initWithController:[registrar messenger]]; | ||
[registrar addApplicationDelegate:instance]; | ||
[registrar addMethodCallDelegate:instance channel:channel]; | ||
} | ||
|
||
- (instancetype)init { | ||
- (instancetype)initWithController:(id)controller { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This changes the designated initializer from GoogleSignInPlugin.h can indicate this: #import <Flutter/Flutter.h>
NS_ASSUME_NONNULL_BEGIN
@interface FLTGoogleSignInPlugin : NSObject <FlutterPlugin>
- (instancetype)initWithController:(id)controller NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
@end
NS_ASSUME_NONNULL_END
Also, it looks like this should be a |
||
self = [super init]; | ||
if (self) { | ||
[GIDSignIn sharedInstance].delegate = self; | ||
[GIDSignIn sharedInstance].uiDelegate = self; | ||
[GIDSignIn sharedInstance].presentingViewController = controller; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That header says: // The view controller used to present |SFSafariViewContoller| on iOS 9 and 10.
@property(nonatomic, weak) UIViewController *presentingViewController; Does that mean this is only applicable for iOS 9 and 10? |
||
|
||
// On the iOS simulator, we get "Broken pipe" errors after sign-in for some | ||
// unknown reason. We can avoid crashing the app by ignoring them. | ||
|
@@ -84,10 +84,10 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result | |
} | ||
} else if ([call.method isEqualToString:@"signInSilently"]) { | ||
if ([self setAccountRequest:result]) { | ||
[[GIDSignIn sharedInstance] signInSilently]; | ||
[[GIDSignIn sharedInstance] restorePreviousSignIn]; | ||
} | ||
} else if ([call.method isEqualToString:@"isSignedIn"]) { | ||
result(@([[GIDSignIn sharedInstance] hasAuthInKeychain])); | ||
result(@([[GIDSignIn sharedInstance] hasPreviousSignIn])); | ||
} else if ([call.method isEqualToString:@"signIn"]) { | ||
if ([self setAccountRequest:result]) { | ||
@try { | ||
|
@@ -136,9 +136,7 @@ - (BOOL)setAccountRequest:(FlutterResult)request { | |
- (BOOL)application:(UIApplication *)app openURL:(NSURL *)url options:(NSDictionary *)options { | ||
NSString *sourceApplication = options[UIApplicationOpenURLOptionsSourceApplicationKey]; | ||
id annotation = options[UIApplicationOpenURLOptionsAnnotationKey]; | ||
collinjackson marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return [[GIDSignIn sharedInstance] handleURL:url | ||
sourceApplication:sourceApplication | ||
annotation:annotation]; | ||
return [[GIDSignIn sharedInstance] handleURL:url]; | ||
} | ||
|
||
#pragma mark - <GIDSignInUIDelegate> protocol | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ description: Flutter plugin for Google Sign-In, a secure authentication system | |
for signing in with a Google account on Android and iOS. | ||
author: Flutter Team <flutter-dev@googlegroups.com> | ||
homepage: https://github.com/flutter/plugins/tree/master/packages/google_sign_in | ||
version: 4.0.9 | ||
version: 4.0.10 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we not update the version to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Generally we don't update the Dart and wrapped SDK versions in lockstep. In this case, we're not making a breaking change to the semantics of the Dart plugin, so I don't think a major version increment is needed. |
||
|
||
flutter: | ||
plugin: | ||
|
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.
This is going to be a problem for you. You are ignoring types and forcing a binary messenger to be a view controller? Once you fix that, you'll find that the binary messenger isn't really a view controller anymore.
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.
Should I do something like this?
Or should the app developer manually pass their
FlutterViewController
to the plugin? I'm not sure what the best approach is here.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 know if grabbing the rootViewController here is going to work for you since the rootViewController might not still be the same rootViewController when GIDSignIn actually wants to present something. Also, I'm not sure what it would do if the rootViewController was a UINavigationController and its topmost view is a FlutterViewController, would it display correctly? You might have to try that out.
You might want to move the setter to the handle method call, is it possible GIDSignIn will want to display something without a method having triggered it?
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.
Also, i don't know if sending the FlutterViewController will work since I don't think there is a mechanism to do that?
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 agree that there probably isn't a mechanism to that.
I'm fine with grabbing the
rootViewController
at the time that the sign-in happens if you think that's a reasonable approach. It seems like the sign-in method is the only time that this gets used.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've updated the approach as described, PTAL