[webview_flutter] Add a backgroundColor option to the iOS webview #4570
Conversation
github-actions
bot
added
p: webview_flutter
There was a problem hiding this comment.
Looks great other than minor nits.
/cc @jmagman or @mvanbeusekom for secondary review.
| WebViewController controller, BuildContext context) async { | ||
| final String contentBase64 = | ||
| base64Encode(const Utf8Encoder().convert(kTransparentBackgroundPage)); | ||
| await controller.loadUrl('data:text/html;base64,$contentBase64'); |
There was a problem hiding this comment.
Nit: you can replace these two lines with the new controller.loadHtmlString.
| UIColor* backgroundColor = [UIColor colorWithRed:(backgroundColorInt >> 16 & 0xff) / 255.f | ||
| green:(backgroundColorInt >> 8 & 0xff) / 255.f | ||
| blue:(backgroundColorInt & 0xff) / 255.f | ||
| alpha:(backgroundColorInt >> 24 & 0xff) / 255.f]; |
There was a problem hiding this comment.
Nit: CGFloat is essentially always 64-bit these days, so these don't need to be floats: / 255.0
| alpha:(backgroundColorInt >> 24 & 0xff) / 255.f]; | ||
| _webView.opaque = NO; | ||
| _webView.backgroundColor = UIColor.clearColor; | ||
| _webView.scrollView.backgroundColor = backgroundColor; |
There was a problem hiding this comment.
Non-opaque views have worse performance, so it's almost certainly worth special-casing this:
int alpha = backgroundColorInt >> 24 & 0xff;
UIColor* backgroundColor = [... alpha:(alpha / 255.0)];
if (alpha == 255) {
_webView.backgroundColor = backgroundColor;
} else {
[these three lines]
}
There was a problem hiding this comment.
I tried to make the change, but if I don't set _webView.opaque = NO; for an opaque color too the WebView's background is white on my emulator.
There was a problem hiding this comment.
That's unfortunate; hopefully that means the drawing for WKWebView is doing something non-obvious then which will avoid the performance penalty.
| @@ -227,6 +228,13 @@ class WebView extends StatefulWidget { | |||
| /// The default policy is [AutoMediaPlaybackPolicy.require_user_action_for_all_media_types]. | |||
| final AutoMediaPlaybackPolicy initialMediaPlaybackPolicy; | |||
|
|
|||
| /// The background color of the webview. | |||
There was a problem hiding this comment.
nit: convert "webview" to breadcrum:
| /// The background color of the webview. | |
| /// The background color of the [WebView]. |
| /// When null the platform's webview default background color is used. | ||
| /// | ||
| /// By default `backgroundColor` is null. |
There was a problem hiding this comment.
nit: Could be combined into one paragraph:
| /// When null the platform's webview default background color is used. | |
| /// | |
| /// By default `backgroundColor` is null. | |
| /// When `null` the platform's webview default background color is used. By | |
| /// default [backgroundColor] is `null`. |
| @@ -70,6 +91,7 @@ class _WebViewExampleState extends State<_WebViewExample> { | |||
| @override | |||
| Widget build(BuildContext context) { | |||
| return Scaffold( | |||
| backgroundColor: Colors.green, | |||
There was a problem hiding this comment.
What about hard coding the color so if it changes in the framework it doesn't break this test in the plugin repo only on the master tests?
| backgroundColor: Colors.green, | |
| backgroundColor: Color(0xFF4CAF50), |
…utter#4570) This PR add an option to set the background color of the iOS webview. Part of flutter#3431 Part of flutter/flutter#29300
…utter#4570) This PR add an option to set the background color of the iOS webview. Part of flutter#3431 Part of flutter/flutter#29300
This PR add an option to set the background color of the iOS webview.
Original PR :
#3431
It fixes this issue :
flutter/flutter#29300
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.