Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Allow quick_actions to register more than once #783

Merged
merged 6 commits into from
Nov 6, 2018
Merged

Allow quick_actions to register more than once #783

merged 6 commits into from
Nov 6, 2018

Conversation

Cretezy
Copy link
Contributor

@Cretezy Cretezy commented Sep 15, 2018

@Hixie
Copy link
Contributor

Hixie commented Oct 30, 2018

cc @mehmetf can you take a look to see if this patch makes sense?

Copy link
Contributor

@mehmetf mehmetf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This plugin is unique in the sense that no other plugin expects Android system to build an activity to interact with it (UrlLauncher comes close but does not expect to post anything to the channel).

I believe this is safe in the current embedder since the system is supposed to construct a new activity in the UI thread and we override the _channel value when the application is initialized. However, if we allow this plugin to be registered anywhere else, there's a risk that the activity will be constructed before the channel's value is set.

@mehmetf
Copy link
Contributor

mehmetf commented Nov 4, 2018

@Cretezy please take a look at my comments. We can merge this in after you fix the minor issues I pointed out.

@Cretezy
Copy link
Contributor Author

Cretezy commented Nov 6, 2018

@mehmetf I applied your suggestions. Let me know if everything is okay!

Copy link
Contributor

@mehmetf mehmetf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just fix the failing tests and you are good to go.

@mehmetf
Copy link
Contributor

mehmetf commented Nov 6, 2018

  • build-ipas failure is unrelated to this. It does not touch any iOS files.
  • The analysis failure is due to old git client. It has been fixed already (and this PR does not touch it).

Please also update changelog and pubspec.yaml. This should be a minor version bump.

@Cretezy
Copy link
Contributor Author

Cretezy commented Nov 6, 2018

@mehmetf should be good to go! Also, the failing build before was for formatting, not the ipas/git

@mehmetf mehmetf merged commit 7128f74 into flutter:master Nov 6, 2018
@VadymPinchuk
Copy link

@mehmetf Hey guys, As I can see - currently a lot of plugins are outdated - changes are merged to plugins but new version are not published. Please fix that )
Interested in plugins: quick_actions and device_info

@mehmetf
Copy link
Contributor

mehmetf commented Nov 15, 2018

Done! Sorry about that.

@VadymPinchuk
Copy link

@mehmetf thank you a lot ;)

andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
5 participants