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

[webview_flutter] Fix scroll bar position for Android non-hybrid WebViews #3765

Merged
blasten merged 10 commits into from
Apr 1, 2021
Merged

[webview_flutter] Fix scroll bar position for Android non-hybrid WebViews #3765

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
65d14b0
Remove explicit layoutDirection from webview_android.dart. Despite th…
math1man Mar 29, 2021
af2b9d1
Update CHANGELOG.md
math1man Mar 29, 2021
dfa48d2
Update pubspec.yaml
math1man Mar 29, 2021
793902f
Update webview_flutter_test.dart. Now that the WebView doesn't intern…
math1man Mar 30, 2021
32ceafa
Fix some formatting
math1man Mar 30, 2021
239671b
Fix some formatting
math1man Mar 30, 2021
51baf2d
Merge branch 'master' of https://github.com/flutter/plugins
math1man Mar 31, 2021
d90bcd0
Revert "Update webview_flutter_test.dart. Now that the WebView doesn'…
math1man Mar 31, 2021
7b1d8c9
Use Directionality.maybeOf(BuildContext) to specify the correct layou…
math1man Mar 31, 2021
bf60a06
Switch two lines to better preserve code history
math1man Mar 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/webview_flutter/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 2.0.3

* Fixes bug where scroll bars on the Android non-hybrid WebView are rendered on
the wrong side of the screen.

## 2.0.2

* Fixes bug where text fields are hidden behind the keyboard
Expand Down
5 changes: 1 addition & 4 deletions packages/webview_flutter/lib/src/webview_android.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ class AndroidWebView implements WebViewPlatform {
id, webViewPlatformCallbacksHandler));
},
gestureRecognizers: gestureRecognizers,
// WebView content is not affected by the Android view's layout direction,
// we explicitly set it here so that the widget doesn't require an ambient
// directionality.
layoutDirection: TextDirection.rtl,
Copy link

@blasten blasten Mar 29, 2021

Choose a reason for hiding this comment

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

have you tested it with older Android versions? https://github.com/flutter/plugins/pull/1618/files suggests that this is necessary for some version of Android.

Copy link
Contributor Author

@math1man math1man Mar 30, 2021

Choose a reason for hiding this comment

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

I'm not seeing anywhere in that PR that specifically talks about this choice, or any indication that it would affect older Android versions. I'll do my best to test on a couple versions of Android though.

Copy link
Contributor Author

@math1man math1man Mar 31, 2021

Choose a reason for hiding this comment

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

I've tested on M, O, and R, and haven't seen any negative side effects.

Copy link

@blasten blasten Mar 31, 2021

Choose a reason for hiding this comment

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

I wonder if Amir meant TextDirection.ltr.

I noted is that this is technically a small breaking change as now a parent widget must provide the layout direction. I don't know how common it's, but by looking at the changes to the unit test, I wonder if changing this value to TextDirection.ltr removes potential failures in other unit tests that we don't own.

Copy link
Contributor Author

@math1man math1man Mar 31, 2021

Choose a reason for hiding this comment

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

Unsurprisingly, that introduces the inverse problem that in an RTL locale, the scroll bars are on the wrong side.

Copy link

@blasten blasten Mar 31, 2021

Choose a reason for hiding this comment

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

We could also use https://api.flutter.dev/flutter/widgets/Directionality/maybeOf.html to detect the current direction, and if null we set LTR as the default. wdyt?

Copy link
Contributor Author

@math1man math1man Mar 31, 2021

Choose a reason for hiding this comment

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

Oooh that's a neat trick. I left the default as RTL to minimize side effects, but I can swap to LTR if you'd prefer.

layoutDirection: Directionality.maybeOf(context) ?? TextDirection.rtl,
creationParams:
MethodChannelWebViewPlatform.creationParamsToMap(creationParams),
creationParamsCodec: const StandardMessageCodec(),
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: webview_flutter
description: A Flutter plugin that provides a WebView widget on Android and iOS.
homepage: https://github.com/flutter/plugins/tree/master/packages/webview_flutter
version: 2.0.2
version: 2.0.3

environment:
sdk: ">=2.12.0-259.9.beta <3.0.0"
Expand Down