[android_intent] add flags parameter #1972

thanhdatvo · 2019-08-13T03:50:13Z

Description

we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag

Similar to react native Link.openUrl() : https://github.com/facebook/react-native/blob/3b6f6ca4d5fcee6f1bc6d6242e3e2ef136e4d546/ReactAndroid/src/main/java/com/facebook/react/modules/intent/IntentModule.java#L97

Related Issues

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.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
No, this is not a breaking change.

googlebot · 2019-08-13T03:50:16Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

thanhdatvo · 2019-08-13T03:54:03Z

@googlebot I signed it!

googlebot · 2019-08-13T04:13:16Z

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

ened · 2019-08-15T09:32:51Z

@datvo0110 please bump version & add change log entry.

I'm wondering if this should be done explicitly via an extra enum flag in the AndroidIntent class.
Can you think of other flags that may be of interest in the future?

I can think of those, for example:

Intent.FLAG_ACTIVITY_NEW_TASK
Intent.FLAG_ACTIVITY_NO_HISTORY
Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS
Intent.FLAG_GRANT_READ_URI_PERMISSION

thanhdatvo · 2019-08-16T04:03:55Z

@ened I have bumped version from 0.3.2 to 0.3.3 & add change log entry. Is this ok?

mklim

Thanks for the contribution!

I'm wondering if this should be done explicitly via an extra enum flag in the AndroidIntent class.
Can you think of other flags that may be of interest in the future?

I agree. I think the behavior in this PR makes sense in most cases, but not every app would necessarily want external activities to behave in this way. I filed flutter/flutter#38701 to track the feature in general.

I'm following the initial PR review policy. This PR isn't trivial to review so I'm labeling it with "backlog." We'll prioritize according to the issue's priority.

Relevant issues:
flutter/flutter#38701

ened · 2019-08-16T20:17:27Z

@datvo0110 let’s rewrite this a bit and add the flags argument instead of having app behavior done implicitly using the package name only. L

Are you up for changing adding and implementing the ‘flags’ parameter?

thanhdatvo · 2019-08-17T01:46:51Z

ok, let's me do that! I'll try to cover most case :D

…

On Sat, Aug 17, 2019 at 3:17 AM Sebastian Roth ***@***.***> wrote: @datvo0110 <https://github.com/datvo0110> let’s rewrite this a bit and add the flags argument instead of having app behavior done implicitly using the package name only. L Are you up for changing adding and implementing the ‘flags’ parameter? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1972?email_source=notifications&email_token=AIUXZCAISFB742I2XQYGEHDQE4DOXA5CNFSM4ILG2X4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4PTEYI#issuecomment-522138209>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AIUXZCDOWYLV75LNWUNMBGLQE4DOXANCNFSM4ILG2X4A> .

thanhdatvo · 2019-08-20T03:46:58Z

I added flags parameter as List<String> and prepare some popular flags as you recommended Intent.FLAG_ACTIVITY_NEW_TASK Intent.FLAG_ACTIVITY_NO_HISTORY Intent.FLAG_ACTIVITY_EXCLUDE_FROM_RECENTS Intent.FLAG_GRANT_READ_URI_PERMISSION I also added an example on how to use flags option. I'm waiting for your opinions :D @ened @mklim

…

On Sat, Aug 17, 2019 at 8:46 AM Thanh Dat Vo ***@***.***> wrote: ok, let's me do that! I'll try to cover most case :D On Sat, Aug 17, 2019 at 3:17 AM Sebastian Roth ***@***.***> wrote: > @datvo0110 <https://github.com/datvo0110> let’s rewrite this a bit and > add the flags argument instead of having app behavior done implicitly using > the package name only. L > > Are you up for changing adding and implementing the ‘flags’ parameter? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1972?email_source=notifications&email_token=AIUXZCAISFB742I2XQYGEHDQE4DOXA5CNFSM4ILG2X4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4PTEYI#issuecomment-522138209>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AIUXZCDOWYLV75LNWUNMBGLQE4DOXANCNFSM4ILG2X4A> > . >

ened

In general, nice.
There is some potential to optimise the code (calculate the flags integer in Dart and send that to the platform, instead of a list of strings which needs separate handling).
Also, please add unit and integration tests for this.

ened · 2019-08-20T13:21:01Z

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

    }
  }

+  private int convertFlag(String flag) {


This PR converts the flags from String to integer using this method.
A better method is to let the Dart side know about the flag names & values and simply push the flag through. You can create a new Flags.dart (or similar) and fill in the constants from https://developer.android.com/reference/android/content/Intent (All the constants from FLAG_).

Please follow the hex numbering (0x00400000) and keep the order as it is listed on the documentation page.

ened · 2019-08-20T13:21:22Z

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

+    }
+  }
+
+  private int convertFlags(ArrayList<String> flags) {


As said above, this logic should be done in the Dart side.

ened · 2019-08-20T13:22:13Z

packages/android_intent/example/ios/Runner.xcodeproj/project.pbxproj

 /* Begin PBXBuildFile section */
 		1498D2341E8E89220040F4C2 /* GeneratedPluginRegistrant.m in Sources */ = {isa = PBXBuildFile; fileRef = 1498D2331E8E89220040F4C2 /* GeneratedPluginRegistrant.m */; };
 		3B3967161E833CAA004F5970 /* AppFrameworkInfo.plist in Resources */ = {isa = PBXBuildFile; fileRef = 3B3967151E833CAA004F5970 /* AppFrameworkInfo.plist */; };
-		2D5378261FAA1A9400D5DBA9 /* flutter_assets in Resources */ = {isa = PBXBuildFile; fileRef = 2D5378251FAA1A9400D5DBA9 /* flutter_assets */; };


The changes in here relate to the iOS example. If possible, please remove them from the commit.

ened · 2019-08-20T13:22:53Z

packages/android_intent/example/lib/main.dart

+    final AndroidIntent intent = AndroidIntent(
+        action: 'action_view',
+        data: Uri.encodeFull('https://flutter.io'),
+        flags: flags);


please add a , to before the parentheses end

this now becomes simpler as you can do the bit operation in Dart

ened · 2019-08-20T13:24:26Z

packages/android_intent/example/lib/main.dart

+              RaisedButton(
+                child: const Text(
+                    'Tap here to open link in browser in a seperated window.'),
+                onPressed: _openLinkInDefaultBrowserInSeperatedWindow,


I like a close-to-reality example like this one, however I would name it more accurate ("Start Activity in New Task"). What other ways could we use in the example to demonstrate the functionality?

thanhdatvo · 2019-08-22T10:54:09Z

@ened I already
-- brought logic from java to dart, add flag list
-- added test case
-- updated example
sorry because I tried to remove packages/android_intent/example/ios/Runner.xcodeproj/project.pbxproj so I have to reset to the old commit, now it contained some unrelated file @@

thanhdatvo · 2019-08-22T11:24:42Z

@ened
sorry, the commit is like a mess now
i have already prepared a new clean commit #2000
let's switch to the other commit to continue

ened · 2019-08-22T11:58:08Z

@datvo0110 ok let's close this one and move to #2000.

thanhdatvo · 2019-08-28T07:53:28Z

Hi @ened, @mklim please help me review #2000

add logic code

a14628d

googlebot added the cla: no label Aug 13, 2019

format file

5fbd54e

googlebot added cla: yes and removed cla: no labels Aug 13, 2019

thanhdatvo changed the title ~~add logic code~~ android_intent: we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag Aug 13, 2019

thanhdatvo changed the title ~~android_intent: we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag~~ [android_intent] we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag Aug 13, 2019

thanhdatvo changed the title ~~[android_intent] we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag~~ [android_intent] if we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag Aug 13, 2019

pump version

d11c5c5

thanhdatvo requested a review from mklim as a code owner August 16, 2019 04:01

mklim reviewed Aug 16, 2019

View reviewed changes

mklim added the backlog label Aug 16, 2019

add flags options

d58344a

thanhdatvo changed the title ~~[android_intent] if we are launching to a different package we need to set the FLAG_ACTIVITY_NEW_TASK flag~~ [android_intent] add flags parameter Aug 20, 2019

ened suggested changes Aug 20, 2019

View reviewed changes

thanhdatvo requested review from amirh, bkonyi, bparrishMines, collinjackson, cyanglaz, iskakaushik and kroikie as code owners August 22, 2019 10:24

remove project.pbxproj

bd5a534

thanhdatvo force-pushed the feature/android_intent_open_app branch from 59a9413 to bd5a534 Compare August 22, 2019 10:42

thanhdatvo added 2 commits August 26, 2019 09:08

merge

07d8b42

update tests

2e6ec6a

thanhdatvo closed this Aug 28, 2019

[android_intent] add flags parameter #1972

[android_intent] add flags parameter #1972

Uh oh!

Conversation

thanhdatvo commented Aug 13, 2019

Description

Related Issues

Checklist

Breaking Change

Uh oh!

googlebot commented Aug 13, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

thanhdatvo commented Aug 13, 2019

Uh oh!

googlebot commented Aug 13, 2019

Uh oh!

ened commented Aug 15, 2019

Uh oh!

thanhdatvo commented Aug 16, 2019

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

ened commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thanhdatvo commented Aug 17, 2019 via email

Uh oh!

thanhdatvo commented Aug 20, 2019 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ened left a comment

Choose a reason for hiding this comment

Uh oh!

ened Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

ened Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

ened Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

ened Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

ened Aug 20, 2019

Choose a reason for hiding this comment

Uh oh!

thanhdatvo commented Aug 22, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thanhdatvo commented Aug 22, 2019

Uh oh!

ened commented Aug 22, 2019

Uh oh!

thanhdatvo commented Aug 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ened commented Aug 16, 2019 •

edited

Loading

thanhdatvo commented Aug 20, 2019 via email •

edited

Loading

thanhdatvo commented Aug 22, 2019 •

edited

Loading