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

[webview_flutter] Add a backgroundColor option to the webview platform interface #4567

Merged

Conversation

e-adrien
Copy link
Contributor

@e-adrien e-adrien commented Dec 2, 2021

This PR add an option to set a transparent background for the webview.

Original PR (app-facing package, iOS, Android, platform interface): #3431

Part of flutter/flutter#29300

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@flutter-dashboard
Copy link

This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled.

@google-cla google-cla bot added the cla: yes label Dec 2, 2021
@github-actions github-actions bot added the p: webview_flutter Edits files for a webview_flutter plugin label Dec 2, 2021
@e-adrien e-adrien changed the title [webview_flutter] Add a backgroundColor option to the webview plateform interface [webview_flutter] Add a backgroundColor option to the webview platform interface Dec 2, 2021
@e-adrien e-adrien force-pushed the webview_flutter_platform_interface branch from b8d71c9 to e3d6af3 Compare December 2, 2021 15:33
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.

This needs a test of the method channel passthrough (should be trivial); otherwise looks good.

@@ -266,6 +266,7 @@ class MethodChannelWebViewPlatform implements WebViewPlatformController {
'userAgent': creationParams.userAgent,
'autoMediaPlaybackPolicy': creationParams.autoMediaPlaybackPolicy.index,
'usesHybridComposition': usesHybridComposition,
'backgroundColor': creationParams.backgroundColor?.value,
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs a corresponding test here

Copy link
Contributor Author

@e-adrien e-adrien Dec 2, 2021

Choose a reason for hiding this comment

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

I put the test in the group('Tests on plugin.flutter.io/webview_<channel_id> channel', () {}) but maybe it would have been better to have them in a dedicated group for changes in creationParamsToMap()

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!

@mvanbeusekom for secondary review.

Copy link
Contributor

@mvanbeusekom mvanbeusekom 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 a note, the version will conflict with my PR #4562 so let's make sure this lands first and I will update #4562.

@stuartmorgan stuartmorgan merged commit 509d3e2 into flutter:master Dec 2, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Dec 2, 2021
amantoux pushed a commit to amantoux/plugins that referenced this pull request Dec 11, 2021
KyleFin pushed a commit to KyleFin/plugins that referenced this pull request Dec 21, 2021
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
3 participants