-
Notifications
You must be signed in to change notification settings - Fork 626
feat(sdk): support latest SDK versions for Android and iOS #764
base: master
Are you sure you want to change the base?
feat(sdk): support latest SDK versions for Android and iOS #764
Conversation
@@ -26,164 +26,131 @@ - (void)handleOpenURLWithAppSourceAndAnnotation:(NSNotification*)notification | |||
|
|||
if ([possibleReversedClientId isEqualToString:self.getreversedClientId] && self.isSigningIn) { | |||
self.isSigningIn = NO; | |||
[[GIDSignIn sharedInstance] handleURL:url]; | |||
[GIDSignIn.sharedInstance handleURL:url]; |
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.
see all breaking changes in v6.0.0 - https://developers.google.com/identity/sign-in/ios/release
@@ -194,7 +212,7 @@ private synchronized void buildGoogleApiClient(JSONObject clientOptions) throws | |||
*/ | |||
private void signIn() { | |||
Intent signInIntent = Auth.GoogleSignInApi.getSignInIntent(this.mGoogleApiClient); | |||
cordova.getActivity().startActivityForResult(signInIntent, RC_GOOGLEPLUS); | |||
this.signInActivityLauncher.launch(signInIntent); |
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.
- (void) isAvailable:(CDVInvokedUrlCommand*)command; | ||
- (void) login:(CDVInvokedUrlCommand*)command; | ||
- (void) trySilentLogin:(CDVInvokedUrlCommand*)command; | ||
- (void) logout:(CDVInvokedUrlCommand*)command; | ||
- (void) disconnect:(CDVInvokedUrlCommand*)command; | ||
- (void) share_unused:(CDVInvokedUrlCommand*)command; |
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.
Compatibility breakage in iOS:
isAvailable
: even necessary?trySilentLogin
: open to backfills if community still requires itshare_unused
: seemed to be cruft
👍👍👍 |
I used this in a new cordova project (android only). I got it working by pulling the fork, merging the pr branch locally, and pointing cordova to my local dir. Somehow it actually just worked perfectly. Webstorm fussed about the bad git mappings, but I just ignored the warnings. Hope this helps someone else who is drowning in cordova auth hell like I was the past 2 weeks. |
I think this pull request should be reviewed before merging it. |
@marioshtika - I welcome any backfill for |
It's probably worth updating this PR as GoogleSignIn pod has now been updated to 7.0.0, it probably has breaking changes |
This repository is not being updated in years, so any merge would be a success. Let's hope @EddyVerbruggen hears our request for help 🤞 |
Any update on this @EddyVerbruggen ? |
This PR updates SDKs for Android and iOS to the latest versions and provides override support as suggested by @marioshtika in the event that a non-conflicting / minor patch version can be targeted as needed. Several aspects of the iOS and Android code needed to be updated.
This unbreaks integration with other plugins such as @havesource/cordova-plugin-push.
NOTE: The current inception of this PR breaks support for
trySilentLogin
as it was not a required feature to support our app. I'm open to upstream contributions if it's something the community still needs, but it may be worth just shelving if the plugin is otherwise broken for the majority of users due to conflicting SDK dependencies across projects.