-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[google_sign_in_web] Add a no-op Android implementation #2410
Conversation
public class GoogleSignInWebPlugin implements FlutterPlugin, MethodCallHandler { | ||
@Override | ||
public void onAttachedToEngine(FlutterPluginBinding flutterPluginBinding) { | ||
final MethodChannel channel = new MethodChannel(flutterPluginBinding.getFlutterEngine().getDartExecutor(), "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.
nit: delete the implementation
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.
Done.
import io.flutter.plugin.common.PluginRegistry.Registrar; | ||
|
||
/** GoogleSignInWebPlugin */ | ||
public class GoogleSignInWebPlugin implements FlutterPlugin, MethodCallHandler { |
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.
nit: this doesn't need to extend MethodCallHandler (delete onMethodCall)
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.
Done.
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.
LGTM
@mklim since you touched on google sigin package, can you please have a look at flutter/flutter#41296 It is quite unusable without it, and hope it is a quick change for you. There was a pr about to merge and hope that help to. thanks. |
Related Issues
flutter/flutter#46898
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]
). This will ensure a smooth and quick review process.///
).flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?