[go: nahoru, domu]
Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_sign_in] Fix deprecated API usage issue by upgrading CocoaPod to 5.0 #2127

Merged
merged 10 commits into from
Oct 29, 2019
Merged

[google_sign_in] Fix deprecated API usage issue by upgrading CocoaPod to 5.0 #2127

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
710283b
Fix Deprecated API Usage issue
collinjackson Sep 30, 2019
9b7eac5
Update to 5.0 compatibility
collinjackson Sep 30, 2019
89187b5
reformat
collinjackson Sep 30, 2019
46d23e3
Update to use [registrar messenger]
collinjackson Oct 1, 2019
f0bb3ce
Merge remote-tracking branch 'origin/master' into google_sign_in
collinjackson Oct 22, 2019
52ed349
Implement code review feedback
collinjackson Oct 22, 2019
34abc18
Remove dead code
collinjackson Oct 22, 2019
2faa78b
Merge remote-tracking branch 'origin/master' into google_sign_in
collinjackson Oct 25, 2019
bb5c3eb
format
collinjackson Oct 25, 2019
e3a79a8
Merge remote-tracking branch 'origin/master' into google_sign_in
collinjackson Oct 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update to use [registrar messenger]
  • Loading branch information
@collinjackson
collinjackson committed Oct 1, 2019
commit 46d23e36f7dad7d98e0791418c73e3ad3d36a49c
9 changes: 3 additions & 6 deletions packages/google_sign_in/ios/Classes/GoogleSignInPlugin.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +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]];
Copy link
Member

Choose a reason for hiding this comment

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

initWithController:[registrar messenger]]

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.

Copy link
Contributor Author
@collinjackson collinjackson Oct 2, 2019 •
edited
Loading

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?

    UIViewController *controller = [UIApplication sharedApplication].keyWindow.rootViewController;	
    [GIDSignIn sharedInstance].presentingViewController = controller;

Or should the app developer manually pass their FlutterViewController to the plugin? I'm not sure what the best approach is here.

Copy link
Member

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?

Copy link
Member

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?

Copy link
Contributor Author
@collinjackson collinjackson Oct 2, 2019 •
edited
Loading

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.

Copy link
Contributor Author
@collinjackson collinjackson Oct 22, 2019 •
edited
Loading

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

[registrar addApplicationDelegate:instance];
[registrar addMethodCallDelegate:instance channel:channel];
}

- (instancetype)init {
- (instancetype)initWithController:(id)controller {
Copy link
Member
@jmagman jmagman Oct 1, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

This changes the designated initializer from -init to -initWithController:

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 UIViewController instead of an id (meaning a type check on [registar.messenger]).

self = [super init];
if (self) {
[GIDSignIn sharedInstance].delegate = self;
[GIDSignIn sharedInstance].presentingViewController = controller;
Copy link
Member

Choose a reason for hiding this comment

The 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.
Expand Down Expand Up @@ -88,10 +89,6 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result
} else if ([call.method isEqualToString:@"isSignedIn"]) {
result(@([[GIDSignIn sharedInstance] hasPreviousSignIn]));
} else if ([call.method isEqualToString:@"signIn"]) {
// TODO(jackson): It might be more appropriate to use controller of the FlutterView
UIViewController *controller = [UIApplication sharedApplication].keyWindow.rootViewController;
[GIDSignIn sharedInstance].presentingViewController = controller;

if ([self setAccountRequest:result]) {
@try {
[[GIDSignIn sharedInstance] signIn];
Expand Down