-
Notifications
You must be signed in to change notification settings - Fork 9.7k
[url_launcher] Split Activity and plugin files #1936
Conversation
mklim
left a comment
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.
Thanks for fixing this up!
Basic changes look good, just have a couple minor nits.
packages/url_launcher/CHANGELOG.md
Outdated
| @@ -1,3 +1,8 @@ | |||
| ## 5.1.1+1 | |||
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 should be 5.1.2 here and in pubspec.yaml. Pubspec changes its versioning scheme depending on if a package is above or below 1.0.0. For below it's 0.major.minor+patch, but for above it's major.minor.patch.
| /* Launches WebView activity */ | ||
| public class WebViewActivity extends Activity { | ||
|
|
||
| public static String ACTION_CLOSE = "close action"; |
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: Some Javadoc explaining what this constant is used for would be useful. https://google.github.io/styleguide/javaguide.html#s7.3-javadoc-where-required
I didn't realize spaces were supported here. Neat.
- add javadoc.
mklim
left a comment
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. Thanks again!
This PR : - Updates AGP and Gradle versions. - Splits WebViewActivity and the plugin class to improve separation of concerns. The separation of classes and files provides encapsulation for constants used in the WebViewActivity.
Description
This PR :
The separation of classes and files provides encapsulation for constants used in the WebViewActivity.
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?