Add support for Android explicit intents #332

alibitek · 2017-12-31T21:43:26Z

Allows to specify an explicit package name when sending an intent.
If there is no package installed in the system with that name or the package cannot handle the intent, it'll fallback to an implicit intent by removing the specified package from the intent.

mravn-google

LGTM

mravn-google · 2018-01-04T07:29:10Z

...droid_intent/android/src/main/java/io/flutter/plugins/androidintent/AndroidIntentPlugin.java

+      intent.setPackage((String) call.argument("package"));
+    }
+    // If we cannot resolve an explicit intent fallback to an implicit one
+    if (intent.getPackage() != null


Can we merge this conditional with the previous one like so:

if (call.argument("package") != null) { intent.setPackage((String) call.argument("package")); if (intent.resolveActivity(context.getPackageManager()) == null) { intent.setPackage(null); } }

Yes. Good catch, easier to reason about.

mravn-google · 2018-01-04T07:30:10Z

...droid_intent/android/src/main/java/io/flutter/plugins/androidintent/AndroidIntentPlugin.java

+    // If we cannot resolve an explicit intent fallback to an implicit one
+    if (intent.getPackage() != null
+        && intent.resolveActivity(context.getPackageManager()) == null) {
+      intent.setPackage(null);


Log that we're ignoring the package?

mravn-google · 2018-01-04T07:32:14Z

packages/android_intent/example/lib/main.dart

+                onPressed: _openLinkInGoogleChrome,
+              ),
+            ],
+          ));


Add trailing comma here to avoid extra indentation above.

mravn-google · 2018-01-04T07:38:38Z

packages/android_intent/lib/android_intent.dart

@@ -31,6 +32,7 @@ class AndroidIntent {
      this.category,


I believe we'd could help dartfmt produce something nicer with a trailing comma here:

const AndroidIntent({ @required this.action, this.category, this.data, this.arguments, this.package, Platform platform, })

Yes! The more I use Dart the more I'm becoming a trailing comma enthusiast :-)

mravn-google · 2018-01-04T07:38:54Z

packages/android_intent/example/lib/main.dart

+        package: 'com.android.chrome');
+    intent.launch();
+  }
+


Great examples, thanks!

I've restructured the example a bit to have the explicit intent test buttons grouped together in a separate view. It'll be easier for future tests to be added to the main view, instead of filling it up with explicit intent test buttons.

mravn-google · 2018-01-04T07:43:08Z

...droid_intent/android/src/main/java/io/flutter/plugins/androidintent/AndroidIntentPlugin.java

@@ -106,7 +108,10 @@ private boolean isStringKeyedMap(Object value) {

  @Override
  public void onMethodCall(MethodCall call, Result result) {
+    Context context =


Would this logic be a candidate for a Context activeContext() or similarly named method on Registrar?

Yes. Like so alibitek/engine@4f26a92 ?

I think it would be useful to have an activeContext() method in the Registrar itself, as the way to obtain a context is common among plugins.
It would reduce the need to repeat this code. What do you think?

In the meantime, I've added an activeContext method to the android_intent plugin.
Should I proceed with submitting the activeContext to the flutter/engine project? Any other changes there?
On iOS, the FlutterPluginRegistrar protocol doesn't have the same functionality, so no changes there.

@alibitek I think it would make sense to add it, if

Plugin authors often find themselves in a situation where they need a context and where it should be the current Activity, if one exists, but can live with the Application, if not; and

We can't just make the existing context() method return the active context---presumable because other callers need the returned context to be the Application.

If you think both of the above apply, then we'd much appreciate you moving forward with a flutter/engine PR.

Adding an activeContext method to the Registrar interface is a breaking change whose impact is limited since we provide the default implementation of that interface ourselves. Still, we need to announce it to flutter-dev.

Alternatively, we could create a utility class for such extensions:

public final class RegistrarExt implements Registrar { private final Registrar registrar; public RegistrarExt(Registrar registrar) { this.registrar = registrar; } public Context activeContext() { final Activity activity = registrar.activity(); return (activity == null) ? registrar.context() : activity; } // + delegate implementations of Registrar methods. }

This would make sense if we expect additional extension methods to accrue over time. A more modern alternative is to use default methods, but that is Java 8 syntax, and not supported prior to Android SDK N, AFAIK.

mit-mit · 2018-01-04T08:24:33Z

Thanks @alibitek! Can you take a quick look at the review feedback, please?

mravn-google · 2018-01-04T11:21:53Z

packages/android_intent/example/lib/main.dart

-      body = new Padding(
+    return new Scaffold(
+      appBar: new AppBar(
+        title: const Text('Test explict intents'),


explict -> explicit

mravn-google · 2018-01-04T11:22:01Z

packages/android_intent/example/lib/main.dart

@@ -104,16 +155,15 @@ class MyHomePage extends StatelessWidget {
                child: const Text('Tap here to open link in Google Chrome.'),
                onPressed: _openLinkInGoogleChrome,
              ),
+              new RaisedButton(
+                child: const Text(
+                    'Tap here to test explict intent fallback to implicit.'),


explict -> explicit

mit-mit · 2018-01-04T14:56:19Z

@mravn-google is this good to merge?

mravn-google · 2018-01-04T15:17:03Z

LGTM, @alibitek are you done?

alibitek · 2018-01-04T15:23:06Z

With this pull request, yes. I need some guidance regarding #332 (comment)

googlebot added the cla: yes label Dec 31, 2017

mit-mit requested a review from mravn-google January 2, 2018 20:39

Add support for Android explicit intents

892401e

alibitek force-pushed the android_intent_package branch from d0c024d to 892401e Compare January 3, 2018 11:58

mravn-google approved these changes Jan 4, 2018

View reviewed changes

Address review

1c86cb9

mravn-google approved these changes Jan 4, 2018

View reviewed changes

Fix typo

d66a6dc

alibitek force-pushed the android_intent_package branch from eadd522 to d66a6dc Compare January 4, 2018 13:13

Get active context

811770d

mravn-google merged commit e23ed1c into flutter:master Jan 4, 2018

Add support for Android explicit intents #332

Add support for Android explicit intents #332

Uh oh!

Conversation

alibitek commented Dec 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mravn-google left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alibitek Jan 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mravn-google Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mit-mit commented Jan 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mit-mit commented Jan 4, 2018

Uh oh!

mravn-google commented Jan 4, 2018

Uh oh!

alibitek commented Jan 4, 2018

Uh oh!

Uh oh!

alibitek commented Dec 31, 2017 •

edited

Loading

alibitek Jan 4, 2018 •

edited

Loading

mravn-google Jan 8, 2018 •

edited

Loading