[WIP] Google sign in web #85
Conversation
|
(No danger on this being open, since it's a PR against my own repo) |
packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
Outdated
Show resolved
Hide resolved
packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
Outdated
Show resolved
Hide resolved
|
|
||
| import 'package:flutter/services.dart'; | ||
| import 'package:flutter_web_plugins/flutter_web_plugins.dart'; | ||
| import 'package:google_sign_in_web/src/gapi.dart'; |
There was a problem hiding this comment.
nit: prefer to use relative imports for same-package imports (i.e. import 'src/gapi.dart';)
There was a problem hiding this comment.
I'll change this, but I don't know enough dart to have been bitten by any issue with package vs relative imports. Any reason for this?
I did this because I think I remember reading somewhere in the dart docs something along the lines "always prefer import from package:blah, because it always works" or similar :/
packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
Outdated
Show resolved
Hide resolved
packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
Outdated
Show resolved
Hide resolved
| // Initialize the gapi | ||
| return _init(call.arguments); | ||
| case 'init': // void | ||
| return _init(call.arguments) |
There was a problem hiding this comment.
you should be able to just return null
There was a problem hiding this comment.
I think returning null breaks this logic, but that's what the java implementation is doing, ssssoooo null it is!
There was a problem hiding this comment.
Hmm I think you're right about breaking that null-check logic. Maybe return true or something? Just wary of the toString() call just to return a signal that the initialization was successful
There was a problem hiding this comment.
I like return true, I'll get that done. .toString rubs me the wrong way too.
However, the Java implementation returns a success-with-null; I guess null is being handled differently, OR the plugin itself makes sure not to call init multiple times.
packages/google_sign_in/google_sign_in_web/lib/google_sign_in_web.dart
Outdated
Show resolved
Hide resolved
|
Also consider migrating to the new "platform interface" way, so you can have a more type-safe and tree-shakeable API |
Yep, in fact I'm going to do that first, then rebase this branch with the platform interface-d plugin, and write the web version using that. |
|
Sounds good, but bear in mind that you have to submit the platform interface change in a separate PR so that this change can depend on it without a path dependency. |
Yes, that's the plan, submit the platform interface first in a separate place, then rebase this against the new master and move things around here. |
|
@hterkelsen I added the platform interface and other goodies here: I'll start prepping each package for individual release, with more relevant PRs shortly. |
|
(Moving these PRs to a more "final" set) |
Description
This WIP PR shows the code for a web implementation of the google_sign_in plugin, that uses the JS API behind the scenes.
Things that need to be done:
Note that this is based against this so it doesn't show a ton of uninteresting file moves.
This is just a RFC from the team, and not intended to be merged as is.