-
Notifications
You must be signed in to change notification settings - Fork 100
Switch to federated google sign in plugin #525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
using a local copy from ditman
separate pr for this
b90fe38 to
2c03377
Compare
|
All the plumbing is fixed, now I have to sort out how to get this dependency :) |
digiter
left a comment
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.
The demo doesn't show avatar currently, is it expected?
The rest LGTM.
| } | ||
|
|
||
| Future<void> signOut() async { | ||
| await _googleSignIn.signOut(); |
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.
No action required, just want to mention that it's amusing to see "SignIn.signOut" :)
| url: git://github.com/ditman/plugins.git | ||
| ref: federated_google_sign_in_web | ||
| path: packages/google_sign_in/google_sign_in_web |
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 had no idea you could point to a repo+branch when using the git source. Nice!
app_flutter/lib/sign_in_button.dart
Outdated
| Widget build(BuildContext context) { | ||
| if (authService.isAuthenticated) { | ||
| return PopupMenuButton<String>( | ||
| child: Image.network(authService.user.photoUrl), |
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.
There's a GoogleUserCircleAvatar widget provided by the google_sign_in plugin that you may be able to use. It's used in the example app.
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.
Using GoogleUserCircleAvatar would throw 2 exceptions in the tests, but I could only catch the last one with WidgetTester.takeException() and it would fail because the first was not caught. So for now, I am just going to leave a TODO :)
@digiter that was to be expected at that time. I have redeployed with latest code from David's fork, and that seems to be working. I noticed an issue where it isn't working in incognito mode (works in release mode on my chrome, dev mode, but not incognito mode release mode), and need to follow up with David before submitting. |
|
|
||
| /// Whether or not the application has been signed in to. | ||
| bool get isAuthenticated => user != null; | ||
| Future<bool> get isAuthenticated => _googleSignIn.isSignedIn(); |
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.
If you don't want to change the getter signature you may be able to do:
bool get isAuthenticated => await _googleSignIn.isSignedIn();
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.
(You might need to change it to an async fn, so maybe you still need to change the signature after all, hmmm)
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.
The await expression can only be used in an async function.
Try marking the function body with either 'async' or 'async*'
The Flutter Google Sign In plugin should be getting official web support very soon. The open source fork the Flutter app was using did not get id tokens which required adding a new authentication method to the backend. This updates all the places in the Flutter app that reference the
GoogleSignInServiceto use the new methods (usingGoogleSignInAccountinstead of separate fields).Right now the web version is in the review stage, so this uses a forked version of it by the @ditman the PR owner.
Added ability to sign out by clicking the user avatar, and then clicking log out in the drop down.
Waiting on
flutter/plugins#2266and preferably flutter/plugins#2280Preview
Demo available at http://testchillers.flutter-dashboard.appspot.com/v2/