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

[webview_flutter] Android implementation of loadFile and loadHtmlString methods #4544

Merged
merged 11 commits into from
Nov 30, 2021
Merged

[webview_flutter] Android implementation of loadFile and loadHtmlString methods #4544

merged 11 commits into from
Nov 30, 2021

Conversation

mvanbeusekom
Copy link
Contributor

Adds implementations of the loadFile and loadHtmlString method channel calls for the webview_flutter_android package.

The loadFile and loadHtmlString methods have been added to the platform interface by PR #4446 and this PR will add the Android implementation.

Related to issues:

NOTE: as per offline discussion with @stuartmorgan the loading of Flutter assets (assets registered within the pubspec.yaml) should be handled via the loadFile method and retrieve the path to the physical asset location the path_provider should be used (or something similar). At the moment the path_provider doesn't support returning the path to the physical location of Flutter assets there is however an issue (flutter/flutter#34026) to track this functionality.

If you had to change anything in the [flutter/tests] repo, include a link to the migration guide as per the [breaking change policy].

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter]. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
  • All existing and new tests are passing.

@mvanbeusekom
Added loadData and loadDataWithBaseUrl to API.
d124eeb 
This commit adds the `loadData` and `loadDataWithBaseUrl` methods to the
pigeon communication interface. Next step would be to include the
implementation details on the Android (JAVA) side.
@mvanbeusekom
Added loadData and loadDataWithBaseUrl JAVA implementation.
e8c529f 
Adds the native Android implementation for the `loadData` and
`loadDataWithBaseUrl` API methods.
@mvanbeusekom
Merge remote-tracking branch 'upstream/master' into webview_load_file…
2977a6e 
…_android
@mvanbeusekom
Adds Dart implementation of loadData and loadDataWithBaseUrl.
801139e 
Adds the Dart implementation of Android's WebView `loadData` and
`loadDataWithBaseUrl` methods.

Also fixed support for null parameters in the native Android
implementation.
@mvanbeusekom
Adds example usage of loadFile and loadHtmlString methods.
6b16597
@google-cla google-cla bot added the cla: yes label Nov 25, 2021
@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android labels Nov 25, 2021
mvanbeusekom
mvanbeusekom commented Nov 25, 2021
View reviewed changes
packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart Outdated
/// The [mimeType] parameter specifies the format of the data. If WebView
/// can't handle the specified MIME type, it will download the data. If
/// `null`, defaults to 'text/html'.
Future<void> loadData(String data, String? mimeType, String? encoding) {
Copy link
Contributor Author

@mvanbeusekom mvanbeusekom Nov 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I choose to make the mimeType and encoding parameters nullable positional parameters to match the original JAVA API as close as possible. Alternatively I could also make these optional named parameters which might make the Dart API a bit more user friendly. @bparrishMines, @stuartmorgan what would you prefer?

P.S. the same is true for the nullable parameters of the loadDataWithBaseUrl method.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think optional named parameters should work fine here. I agree it makes the code more Darty without changing the method.

bparrishMines
bparrishMines reviewed Nov 29, 2021
View reviewed changes
Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

This looks great! I left just a few comments

...tter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewHostApiImpl.java Show resolved Hide resolved
packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart Outdated
/// The [mimeType] parameter specifies the format of the data. If WebView
/// can't handle the specified MIME type, it will download the data. If
/// `null`, defaults to 'text/html'.
Future<void> loadData(String data, String? mimeType, String? encoding) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think optional named parameters should work fine here. I agree it makes the code more Darty without changing the method.

packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart Outdated Show resolved Hide resolved
packages/webview_flutter/webview_flutter_android/lib/src/android_webview.dart Outdated Show resolved Hide resolved
@mvanbeusekom
Copy link
Contributor Author

Hi @bparrishMines, thank you for the review. I have processed your feedback and would appreciate it if you could have another look.

@mvanbeusekom mvanbeusekom mentioned this pull request Nov 30, 2021
Merged
11 tasks
bparrishMines
bparrishMines approved these changes Nov 30, 2021
View reviewed changes
Copy link
Contributor

@bparrishMines bparrishMines left a comment

Choose a reason for hiding this comment

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

LGTM

@mvanbeusekom mvanbeusekom added the waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land. label Nov 30, 2021
@fluttergithubbot fluttergithubbot merged commit eca9088 into flutter:master Nov 30, 2021
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Dec 1, 2021
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 1, 2021
@engine-flutter-autoroll
eca9088 [webview_flutter] Android implementation of loadFile and `l…
cd61bbc 
…oadHtmlString` methods (flutter/plugins#4544)
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
@mvanbeusekom @amantoux
[webview_flutter] Android implementation of loadFile and `loadHtmlS…
423d8b8 
…tring` methods (flutter#4544)
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
@mvanbeusekom @KyleFin
[webview_flutter] Android implementation of loadFile and `loadHtmlS…
ac7ed7c 
…tring` methods (flutter#4544)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@bparrishMines bparrishMines bparrishMines approved these changes

@stuartmorgan stuartmorgan Awaiting requested review from stuartmorgan

Assignees
No one assigned
Labels
cla: yes p: webview_flutter Edits files for a webview_flutter plugin platform-android waiting for tree to go green (Use "autosubmit") This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
Milestone
No milestone
3 participants
@mvanbeusekom @bparrishMines @fluttergithubbot