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

[webview_flutter] Add loadRequest functionality to app facing package. #4573

Merged
merged 23 commits into from Dec 7, 2021

Conversation

BeMacized
Copy link
Member

@BeMacized BeMacized commented Dec 3, 2021

Adds the loadRequest method to the WebViewController.

As this PR contains and depends on changes from #4563, containing the native implementation for Android, this PR will remain in draft mode until those changes have been merged and published.

Likewise, this should not be merged before #4558, as the proposed workaround for setting headers with a post request on Android is based on this functionality being available.

Related issue:

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]. (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], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@google-cla google-cla bot added the cla: yes label Dec 3, 2021
@github-actions github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android labels Dec 3, 2021
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

Small typo to fix when you update this once Android finishes postsubmit tests and in published.

@stuartmorgan
Copy link
Contributor

Test retries finally survived flaky tests; Android is published!

@BeMacized BeMacized marked this pull request as ready for review December 7, 2021 17:14
@flutter-dashboard flutter-dashboard bot changed the base branch from master to main December 7, 2021 17:15
@flutter-dashboard
Copy link

This pull request was opened against a branch other than master. Since Flutter pull requests should not normally be opened against branches other than master, I have changed the base to master. If this was intended, you may modify the base back to master. See the Release Process for information about how other branches get updated.

Reviewers: Use caution before merging pull requests to branches other than master, unless this is an intentional hotfix/cherrypick.

@stuartmorgan
Copy link
Contributor

Oh no, the rogue redirect bot runs when changing a PR from draft to non-draft 😱

@stuartmorgan stuartmorgan changed the base branch from main to master December 7, 2021 17:48
@stuartmorgan
Copy link
Contributor

Wait, there is UI to fix this after all. Whew. Not sure why I couldn't find it before.

@BeMacized
Copy link
Member Author

@stuartmorgan Pretty sure I've seen this happening on multiple PRs today. Is it alright to merge when the bot has changed it to main, or should it be fixed beforehand?

@stuartmorgan
Copy link
Contributor

Pretty sure I've seen this happening on multiple PRs today.

It's happening to every new PR today; there's an infra ticket open to find out what's going on.

Is it alright to merge when the bot has changed it to main, or should it be fixed beforehand?

Once the PR has been manually switched back to targeting master it's fine to land. Landing against main will basically destroy the PR because we're not using main yet (and there's a master->main clone task running that, based on experience this morning, appears to destroy any deviation on main).

Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM

@BeMacized BeMacized merged commit c691451 into flutter:master Dec 7, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 7, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
flutter#4573)

* Add android implementations for loadRequest.

* Update changelog and pubspec.

* Fix comment.

* Fix comment.

* Add tests

* Add back removed license headers

* Fix analysis error

* Add support for loadRequest to app facing package

* Add test

* Comment pending dependency in pubspec

* Remove workaround for supporting custom headers when making post requests on Android.

* Document android workaround in dart doc

* Enforce uri scheme

* Update loadRequest dartdoc

* Document android workaround in readme

* Processed PR feedback.

* Updated dependency, version and changelog.
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
flutter#4573)

* Add android implementations for loadRequest.

* Update changelog and pubspec.

* Fix comment.

* Fix comment.

* Add tests

* Add back removed license headers

* Fix analysis error

* Add support for loadRequest to app facing package

* Add test

* Comment pending dependency in pubspec

* Remove workaround for supporting custom headers when making post requests on Android.

* Document android workaround in dart doc

* Enforce uri scheme

* Update loadRequest dartdoc

* Document android workaround in readme

* Processed PR feedback.

* Updated dependency, version and changelog.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes p: webview_flutter Edits files for a webview_flutter plugin
Projects
None yet
2 participants