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

[webview_flutter] always make sure the contentInset or the sum of contentInset and adjustedContentInset is 0 on iOS. #2466

Merged
merged 8 commits into from
Jan 15, 2020

Conversation

cyanglaz
Copy link
Contributor

@cyanglaz cyanglaz commented Jan 13, 2020

Description

IOS sometimes updates the contentInset of the scroll view (for example to incorporate the keyboard) when the frame of the WebView changes.
The contentInset of the scrollview should be reverted to 0 after changing the view's frame so it doesn't interfere with Flutter.

This PR also fixes the XCTest test cases to follow XCTest's naming conventions.

Related Issues

Fixes flutter/flutter#45482

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See Contributor Guide).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the Flutter Style Guide.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the CLA.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@cyanglaz cyanglaz changed the title [webview_flutter] always insure contentInset or the sum between contentInset and adjustedContengInset is 0 on iOS. [webview_flutter] always make sure the contentInset or the sum of contentInset and adjustedContentInset is 0 on iOS. Jan 13, 2020
@cyanglaz cyanglaz marked this pull request as ready for review January 14, 2020 01:03
@cyanglaz cyanglaz requested a review from amirh as a code owner January 14, 2020 01:03
FLTWKWebView *webView = [[FLTWKWebView alloc] initWithFrame:CGRectMake(0, 0, 300, 400)];
webView.scrollView.contentInset = UIEdgeInsetsMake(0, 0, 300, 0);
XCTAssertFalse(UIEdgeInsetsEqualToEdgeInsets(webView.scrollView.contentInset, UIEdgeInsetsZero));
webView.frame = CGRectMake(0, 0, 300, 200);
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets also assert that the frame was updated, as we are overriding the setter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

webView.frame = CGRectMake(0, 0, 300, 200);
XCTAssertTrue(UIEdgeInsetsEqualToEdgeInsets(webView.scrollView.contentInset, UIEdgeInsetsZero));

if (@available(iOS 11, *)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the effect of such a condition on a XCTest, are we running once with iOS < 11 and once with iOS >= 11?

Copy link
Contributor Author

@cyanglaz cyanglaz Jan 15, 2020

Choose a reason for hiding this comment

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

our ci is currently running on the latest iOS (13), I added this condition in case we decided to run CI for a lower iOS version. In a lower version, since there is no adjustedContentInset, we don't need to test it.

@@ -21,4 +21,7 @@ NS_ASSUME_NONNULL_BEGIN
- (instancetype)initWithMessenger:(NSObject<FlutterBinaryMessenger>*)messenger;
@end

@interface FLTWKWebView : WKWebView
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: add class docs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@amirh amirh left a comment

Choose a reason for hiding this comment

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

LGTM

@cyanglaz cyanglaz merged commit af430fa into flutter:master Jan 15, 2020
@cyanglaz cyanglaz deleted the webview_content_inset branch January 15, 2020 19:26
FlutterSu pushed a commit to FlutterSu/flutter-plugins that referenced this pull request Nov 20, 2020
…tentInset and adjustedContentInset is 0 on iOS. (flutter#2466)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On iOS, when a text field is focused on a webpage, the webview scrolls into an empty white area
3 participants