This repository was archived by the owner on Feb 22, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[webview_flutter] Implement zoom enabled for ios and android #4417
Merged
fluttergithubbot
merged 15 commits into
flutter:master
from
NickalasB:implement_zoomEnabled_for_ios_and_android
Oct 27, 2021
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
5c72815
Updating android AUTHORS/CHANGELOG/pubspec.yaml
NickalasB 1172d73
Adding Java implementation for zoomEnabled
NickalasB 68283a0
Updating Android WebView with zoomEnabled param
NickalasB 3d5467c
Updating android pubspec version of platform_interface
NickalasB b0f92d6
Updating iOS pubspec/AUTHORS/CHANGELOG
NickalasB a342931
Adding iOS implementation for zoomEnabled
NickalasB b6b4c30
Updating iOS pubspec platform_interface version and WebView
NickalasB ef139b6
Removing extra line in Android pubspec.yaml
NickalasB 8b7bcb0
Merge branch 'master' into implement_zoomEnabled_for_ios_and_android
NickalasB 64db0db
Update platform_interface_version
NickalasB 74addca
Adding test coverage for iOS
NickalasB f2dd5ad
Updating formatting
NickalasB 6672b4f
Merge branch 'master' into implement_zoomEnabled_for_ios_and_android
NickalasB 024397a
Addressing PR feedback.
NickalasB 59e1b03
Merge branch 'master' into implement_zoomEnabled_for_ios_and_android
NickalasB File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,4 +65,5 @@ Anton Borries <[email protected]> | |
Alex Li <[email protected]> | ||
Rahul Raj <[email protected]> | ||
Maurits van Beusekom <[email protected]> | ||
Nick Bradshaw <[email protected]> | ||
|
4 changes: 4 additions & 0 deletions
4
packages/webview_flutter/webview_flutter_android/CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,7 @@ | ||
## 2.1.0 | ||
|
||
* Add `zoomEnabled` functionality. | ||
|
||
## 2.0.15 | ||
|
||
* Added Overrides in FlutterWebView.java | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,3 +65,5 @@ Anton Borries <[email protected]> | |
Alex Li <[email protected]> | ||
Rahul Raj <[email protected]> | ||
Maurits van Beusekom <[email protected]> | ||
Antonino Di Natale <[email protected]> | ||
Nick Bradshaw <[email protected]> |
4 changes: 4 additions & 0 deletions
4
packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would happen if
shouldEnableZoom
was changed without the page reloading? Shouldn't this also be called for the current webpage along with every call todidFinishNavigation
.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense but I'm not totally sure where you are suggesting I should also make this check? Inside
decidePolicyForNavigationAction
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This workaround actually makes me question if this feature should even be supported by iOS directly. I'm leaning towards that a platform shouldn't support a feature from the platform interface unless it is directly supported by the API or there is a reasonable workaround. And I wouldn't consider running java script as a reasonable workaround.
@stuartmorgan Do we have a policy for this already?
cc @mvanbeusekom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the WKWebView API doesn't provide a built-in way of toggling whether or not zoom is enabled. This workaround was intended to at least allow Flutter devs to toggle that on both Android and iOS. The original intention of this work was strictly to ENABLE zoom for Android as that functionality is currently missing. My background is in Android but in my research, I couldn't find a cleaner way of DISABLING zoom for iOS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One other proposal, although it would require reverting #4404, is to just enable zooming for Android, and not make it togglable in the Flutter
WebView
. That would basically only require code-changes towebview_flutter/webview_flutter_android/android/src/main/java/io/flutter/plugins/webviewflutter/WebViewBuilder.java
. Could just be me but it doesn't seem like there are many valid use-cases for disabling zoom... which is probably why theWKWebView
API doesn't provide a way to do it. Again, appreciate the team helping move this forward so we can get off our fork.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can see this either way. I spent years working on a project that attempted to make UIWebView usable in a browser context though, and basically everything had to be done with JS in that case, so my judgement on this may well be skewed.
In this case, my take was:
Given that my feeling is that it's reasonable to provide a best-effort implementation rather than nothing, as if someone is explicitly trying to turn on this behavior they presumably would want it to work on iOS too.
We don't have an explicit policy. There are obvious cases where we would say no, like use of private API on iOS, but this isn't doing anything like that, and using JS as a polyfill for OS web view limitations is something that definitely has a history of being relatively common on iOS, so I don't think this is a clear-cut case of "unreasonable".
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. Then I'm fine with the current implementation of the feature. I would just make it clear how the feature works on iOS in the followup PR for
webview_flutter
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you both!