Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
3612a84
added allowsLinkPreview to platform interface
b099l3 Mar 26, 2022
d13276f
Bump platform interface version to 1.8.2
b099l3 Mar 26, 2022
bb17b4d
added allowsLinkPreview to wkwebview
b099l3 Mar 26, 2022
e659cd5
bumped wkwebview to 2.7.2
b099l3 Mar 26, 2022
6b0ad2e
updated flutter android to use platform interface with allowsLinkPreview
b099l3 Mar 26, 2022
06ba8dd
updated flutter web to use platform interface with allowsLinkPreview
b099l3 Mar 26, 2022
4a78973
updated webview_flutter to use platform interface, wkwebview and andr…
b099l3 Mar 26, 2022
eec288f
bumped webview_flutter to 3.0.2
b099l3 Mar 26, 2022
78bec61
format changes
b099l3 Mar 26, 2022
ed14ad2
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Jul 31, 2022
baaf7d5
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 2, 2022
82be993
removed allowsLinkPreview setting in "Custom platform implementation …
b099l3 Sep 2, 2022
09ae8b6
Added tests for allowsLinkPreview
b099l3 Sep 2, 2022
9eadd8e
Changed docs comment to say not supported on all platforms
b099l3 Sep 2, 2022
023d1ff
added allowsLinkPreview to the new style and ran pigeon/gen commands
b099l3 Sep 5, 2022
6f7ef30
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 5, 2022
87382bc
formatter changes
b099l3 Sep 5, 2022
f21cac9
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 6, 2022
ebc1dec
some how the formatter doesnt like this
b099l3 Sep 6, 2022
9950af8
removed changes to android package that wont be needed
b099l3 Sep 6, 2022
920a253
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 6, 2022
b08bab3
changed to suggest docs comment - Whether pressing a link displays ...
b099l3 Sep 6, 2022
7d2f9ef
reverted changes on integration_test package
b099l3 Sep 6, 2022
9bfacc8
reverts on webview_flutter_web
b099l3 Sep 6, 2022
a413f89
changed to deps override
b099l3 Sep 6, 2022
e34c9c2
removed iOS specific default reason
b099l3 Sep 6, 2022
a69a34a
removed Dart integration test
b099l3 Sep 6, 2022
77fbc60
removed allowsLinkPreview on method channel
b099l3 Sep 6, 2022
a0ae928
updated versions of all packages that have changes
b099l3 Sep 6, 2022
8201ae4
revert of spacing from my formatter
b099l3 Sep 7, 2022
bb2d434
revert of spacing from my formatter
b099l3 Sep 7, 2022
1c5d385
revert of spacing from my formatter
b099l3 Sep 7, 2022
b8675f9
matching naming for native method
b099l3 Sep 7, 2022
8862792
Added native test for testSetAllowsLinkPreview in FWFWebViewHostApiTests
b099l3 Sep 7, 2022
f2ec62b
added dependancy overrides in examples
b099l3 Sep 7, 2022
b642bbe
reverted changes to pigeon 3.1.5
b099l3 Sep 7, 2022
1aaf428
wrong prop testing and dupe code removal
b099l3 Sep 10, 2022
f86918b
removed enableLinkPreview from webview controller
b099l3 Sep 10, 2022
d414528
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 10, 2022
9548784
removed allowsLinkPreview from websettings and method channel
b099l3 Sep 14, 2022
12f6da2
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 14, 2022
0b39035
added allowsLinkPreview back to WebSettings
b099l3 Sep 16, 2022
b0e2496
Merge branch 'main' into iain_smith/webview-wkwebview-link-preview
b099l3 Sep 16, 2022
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
1 change: 1 addition & 0 deletions packages/webview_flutter/webview_flutter/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Ignores unnecessary import warnings in preparation for [upcoming Flutter changes](https://github.com/flutter/flutter/pull/104231).
* Updates references to the obsolete master branch.
* Adds support for the `allowsLinkPreview` property on iOS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is changing production code it will need to update the version. (This applies for all the packages changed in this PR.)

Copy link
Author

Choose a reason for hiding this comment

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

I have run the update-release-info command with --version=minimal


## 3.0.4

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,32 @@ Future<void> main() async {
expect(currentUrl, primaryUrl);
});

testWidgets('launches with allowsLinkPreview on iOS',
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we can't actually test that the feature works in a Dart integration test, we should just fully cover the feature with unit tests at all layers (Dart and native) instead.

Copy link
Author

Choose a reason for hiding this comment

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

Removed Dart integration test

(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Completer<WebViewController>();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: SizedBox(
width: 400,
height: 300,
child: WebView(
key: GlobalKey(),
initialUrl: primaryUrl,
allowsLinkPreview: false,
onWebViewCreated: (WebViewController controller) {
controllerCompleter.complete(controller);
},
),
),
),
);
final WebViewController controller = await controllerCompleter.future;
final String? currentUrl = await controller.currentUrl();
expect(currentUrl, primaryUrl);
});

// TODO(bparrishMines): skipped due to https://github.com/flutter/flutter/issues/86757
testWidgets('target _blank opens in same window',
(WidgetTester tester) async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class WebView extends StatefulWidget {
this.onWebResourceError,
this.debuggingEnabled = false,
this.gestureNavigationEnabled = false,
this.allowsLinkPreview = true,
this.userAgent,
this.zoomEnabled = true,
this.initialMediaPlaybackPolicy =
Expand Down Expand Up @@ -260,6 +261,13 @@ class WebView extends StatefulWidget {
/// By default `gestureNavigationEnabled` is false.
final bool gestureNavigationEnabled;

/// A Boolean value that determines whether pressing a link displays a preview of the destination for the link.
Copy link
Contributor

Choose a reason for hiding this comment

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

This can just say "Whether pressing a link displays [...]", since the fact that it's a boolean value is obvious from the Dart code itself. (I know this was modeled on the comment for the field above, but that's not our usual style so it's better if we don't propagate it.)

Copy link
Author

Choose a reason for hiding this comment

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

changed to suggested docs comment, "Whether pressing a link displays ..."

///
/// This only works on iOS.
///
/// By default `allowsLinkPreview` is true, to match the default on iOS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is the cross-platform API, I'd prefer that we not include the "to match the default on iOS" part (since if we added support for other platforms in the future that had different platform defaults, it wouldn't be clear to a reader why we match iOS rather than the other platforms.)

Copy link
Author

Choose a reason for hiding this comment

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

Changed to "By default allowsLinkPreview is true."

final bool allowsLinkPreview;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this type be nullable, so on Platforms that doesn't support this feature, the value is null.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not a fan of nullable bools in most cases; they are annoying to use, and in practice you generally have to pick a default to treat null as eventually anyway.

(We shouldn't say this is iOS only by the way; we should instead say it's not supported on all platforms.)

Copy link
Author

Choose a reason for hiding this comment

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

Based on the comments, assuming I should keep it as a bool and not a nullable bool. If this is wrong, let me know, and I'll change it.

Changed doc comment to 'Not supported on all platforms.'


/// The value used for the HTTP User-Agent: request header.
///
/// When null the platform's webview default is used for the User-Agent header.
Expand Down Expand Up @@ -380,6 +388,7 @@ WebSettings _webSettingsFromWidget(WebView widget) {
hasProgressTracking: widget.onProgress != null,
debuggingEnabled: widget.debuggingEnabled,
gestureNavigationEnabled: widget.gestureNavigationEnabled,
allowsLinkPreview: widget.allowsLinkPreview,
allowsInlineMediaPlayback: widget.allowsInlineMediaPlayback,
userAgent: WebSetting<String?>.of(widget.userAgent),
zoomEnabled: widget.zoomEnabled,
Expand Down
15 changes: 12 additions & 3 deletions packages/webview_flutter/webview_flutter/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,18 @@ flutter:
dependencies:
flutter:
sdk: flutter
webview_flutter_android: ^2.8.0
webview_flutter_platform_interface: ^1.8.0
webview_flutter_wkwebview: ^2.7.0
# FOR TESTING ONLY. DO NOT MERGE.
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW if you use dependency_overrides to do the path-based deps, the analyzer won't fail in CI.

Copy link
Author

Choose a reason for hiding this comment

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

changed to use dependency_overrides

Copy link
Author

Choose a reason for hiding this comment

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

@stuartmorgan this seems to be causing problems where it is not finding the overridden version of these packages.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah right, you'll need to add the overrides in the examples as well. (Filed flutter/flutter#111091 for the tooling not handling that correctly.)

Copy link
Author

Choose a reason for hiding this comment

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

Nice that sorted it

webview_flutter_android:
path:
../webview_flutter_android
# FOR TESTING ONLY. DO NOT MERGE.
webview_flutter_platform_interface:
path:
../webview_flutter_platform_interface
# FOR TESTING ONLY. DO NOT MERGE.
webview_flutter_wkwebview:
path:
../webview_flutter_wkwebview

dev_dependencies:
build_runner: ^2.1.5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1139,6 +1139,7 @@ void main() {
const WebView(
initialUrl: 'https://youtube.com',
gestureNavigationEnabled: true,
allowsLinkPreview: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also test the default value is true?

Copy link
Author

@b099l3 b099l3 Sep 2, 2022

Choose a reason for hiding this comment

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

Removed the bool from this test 'creation' and created a new group for allowsLinkPreview with default, disable, and can change tests.

),
);

Expand All @@ -1155,6 +1156,7 @@ void main() {
debuggingEnabled: false,
userAgent: const WebSetting<String?>.of(null),
gestureNavigationEnabled: true,
allowsLinkPreview: false,
zoomEnabled: true,
),
)));
Expand Down Expand Up @@ -1337,6 +1339,7 @@ class MatchesWebSettings extends Matcher {
_webSettings!.debuggingEnabled == webSettings.debuggingEnabled &&
_webSettings!.gestureNavigationEnabled ==
webSettings.gestureNavigationEnabled &&
_webSettings!.allowsLinkPreview == webSettings.allowsLinkPreview &&
_webSettings!.userAgent == webSettings.userAgent &&
_webSettings!.zoomEnabled == webSettings.zoomEnabled;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1241,6 +1241,32 @@ Future<void> main() async {
expect(currentUrl, primaryUrl);
});

testWidgets('launches with allowsLinkPreview on iOS',
Copy link
Contributor

Choose a reason for hiding this comment

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

webview_flutter_android is an Android-only implementation package so it doesn't need any iOS tests, iOS parameters in its example code, etc. I believe for this PR you won't need to change this package at all.

Copy link
Author

Choose a reason for hiding this comment

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

I have removed all changes in webview_flutter_android

(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Completer<WebViewController>();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: SizedBox(
width: 400,
height: 300,
child: WebView(
key: GlobalKey(),
initialUrl: primaryUrl,
allowsLinkPreview: false,
onWebViewCreated: (WebViewController controller) {
controllerCompleter.complete(controller);
},
),
),
),
);
final WebViewController controller = await controllerCompleter.future;
final String? currentUrl = await controller.currentUrl();
expect(currentUrl, primaryUrl);
});

testWidgets('target _blank opens in same window',
(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ class WebView extends StatefulWidget {
this.onWebResourceError,
this.debuggingEnabled = false,
this.gestureNavigationEnabled = false,
this.allowsLinkPreview = true,
this.userAgent,
this.zoomEnabled = true,
this.initialMediaPlaybackPolicy =
Expand Down Expand Up @@ -216,6 +217,13 @@ class WebView extends StatefulWidget {
/// By default `gestureNavigationEnabled` is false.
final bool gestureNavigationEnabled;

/// A Boolean value that determines whether pressing a link displays a preview of the destination for the link.
///
/// This only works on iOS.
///
/// By default `allowsLinkPreview` is true, to match the default on iOS.
final bool allowsLinkPreview;

/// A Boolean value indicating whether the WebView should support zooming using its on-screen zoom controls and gestures.
///
/// By default 'zoomEnabled' is true
Expand Down Expand Up @@ -676,6 +684,7 @@ WebSettings _webSettingsFromWidget(WebView widget) {
hasProgressTracking: widget.onProgress != null,
debuggingEnabled: widget.debuggingEnabled,
gestureNavigationEnabled: widget.gestureNavigationEnabled,
allowsLinkPreview: widget.allowsLinkPreview,
allowsInlineMediaPlayback: widget.allowsInlineMediaPlayback,
userAgent: WebSetting<String?>.of(widget.userAgent),
zoomEnabled: widget.zoomEnabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ flutter:
dependencies:
flutter:
sdk: flutter
webview_flutter_platform_interface: ^1.8.0
# FOR TESTING ONLY. DO NOT MERGE.
webview_flutter_platform_interface:
path:
../webview_flutter_platform_interface

dev_dependencies:
build_runner: ^2.1.4
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## NEXT

* Ignores unnecessary import warnings in preparation for [upcoming Flutter changes](https://github.com/flutter/flutter/pull/106316).
* Adds support for the `allowsLinkPreview` property on iOS.

## 1.9.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,7 @@ class MethodChannelWebViewPlatform implements WebViewPlatformController {
_addIfNonNull('debuggingEnabled', settings.debuggingEnabled);
_addIfNonNull(
'gestureNavigationEnabled', settings.gestureNavigationEnabled);
_addIfNonNull('allowsLinkPreview', settings.allowsLinkPreview);
Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine to leave this, but it's not actually necessary to update the method channel implementation; we don't use it. It's only here for backward compatibility (which means it doesn't need to support new features).

Copy link
Author

Choose a reason for hiding this comment

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

Cool I removed it anyway to match convention

_addIfNonNull(
'allowsInlineMediaPlayback', settings.allowsInlineMediaPlayback);
_addSettingIfPresent('userAgent', settings.userAgent);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ class WebSettings {
this.hasProgressTracking,
this.debuggingEnabled,
this.gestureNavigationEnabled,
this.allowsLinkPreview,
this.allowsInlineMediaPlayback,
this.zoomEnabled,
required this.userAgent,
Expand Down Expand Up @@ -125,8 +126,13 @@ class WebSettings {
/// See also: [WebView.gestureNavigationEnabled]
final bool? gestureNavigationEnabled;

/// Determines whether pressing a link displays a preview of the destination for the link in iOS.
///
/// See also: [WebView.allowsLinkPreview]
final bool? allowsLinkPreview;

@override
String toString() {
return 'WebSettings(javascriptMode: $javascriptMode, hasNavigationDelegate: $hasNavigationDelegate, hasProgressTracking: $hasProgressTracking, debuggingEnabled: $debuggingEnabled, gestureNavigationEnabled: $gestureNavigationEnabled, userAgent: $userAgent, allowsInlineMediaPlayback: $allowsInlineMediaPlayback)';
return 'WebSettings(javascriptMode: $javascriptMode, hasNavigationDelegate: $hasNavigationDelegate, hasProgressTracking: $hasProgressTracking, debuggingEnabled: $debuggingEnabled, gestureNavigationEnabled: $gestureNavigationEnabled, allowsLinkPreview: $allowsLinkPreview, userAgent: $userAgent, allowsInlineMediaPlayback: $allowsInlineMediaPlayback)';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,7 @@ void main() {
hasProgressTracking: true,
debuggingEnabled: true,
gestureNavigationEnabled: true,
allowsLinkPreview: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe a unit test is failing because you removed this value from the Method Channel implementation in #5110 (comment). So you can remove the changes to this test.

Copy link
Author

Choose a reason for hiding this comment

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

Removed from the method channel test

allowsInlineMediaPlayback: true,
zoomEnabled: false,
);
Expand All @@ -457,6 +458,7 @@ void main() {
'hasProgressTracking': true,
'debuggingEnabled': true,
'gestureNavigationEnabled': true,
'allowsLinkPreview': false,
'allowsInlineMediaPlayback': true,
'zoomEnabled': false,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ WebSettings _webSettingsFromWidget(WebView widget) {
hasProgressTracking: false,
debuggingEnabled: false,
gestureNavigationEnabled: false,
allowsLinkPreview: true,
allowsInlineMediaPlayback: true,
userAgent: const WebSetting<String?>.of(''),
zoomEnabled: false,
Expand Down
5 changes: 4 additions & 1 deletion packages/webview_flutter/webview_flutter_web/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ dependencies:
sdk: flutter
flutter_web_plugins:
sdk: flutter
webview_flutter_platform_interface: ^1.8.0
# FOR TESTING ONLY. DO NOT MERGE.
webview_flutter_platform_interface:
path:
../webview_flutter_platform_interface

dev_dependencies:
build_runner: ^2.1.5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

* Fixes crash when an Objective-C object in `FWFInstanceManager` is released, but the dealloc
callback is no longer available.
* Adds support for the `allowsLinkPreview` property on iOS.

## 2.9.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,32 @@ Future<void> main() async {
});
});

testWidgets('launches with allowsLinkPreview on iOS',
Copy link
Contributor

Choose a reason for hiding this comment

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

As with Android, the changes in this package can be reverted.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted changes in integration_test

(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Completer<WebViewController>();
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: SizedBox(
width: 400,
height: 300,
child: WebView(
key: GlobalKey(),
initialUrl: primaryUrl,
allowsLinkPreview: false,
onWebViewCreated: (WebViewController controller) {
controllerCompleter.complete(controller);
},
),
),
),
);
final WebViewController controller = await controllerCompleter.future;
final String? currentUrl = await controller.currentUrl();
expect(currentUrl, primaryUrl);
});

testWidgets('launches with gestureNavigationEnabled on iOS',
(WidgetTester tester) async {
final Completer<WebViewController> controllerCompleter =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class WebView extends StatefulWidget {
this.onWebResourceError,
this.debuggingEnabled = false,
this.gestureNavigationEnabled = false,
this.allowsLinkPreview = true,
this.userAgent,
this.zoomEnabled = true,
this.initialMediaPlaybackPolicy =
Expand Down Expand Up @@ -206,6 +207,13 @@ class WebView extends StatefulWidget {
/// By default `gestureNavigationEnabled` is false.
final bool gestureNavigationEnabled;

/// A Boolean value that determines whether pressing a link displays a preview of the destination for the link.
///
/// This only works on iOS.
///
/// By default `allowsLinkPreview` is true, to match the default on iOS.
final bool allowsLinkPreview;

/// The value used for the HTTP User-Agent: request header.
///
/// When null the platform's webview default is used for the User-Agent header.
Expand Down Expand Up @@ -622,6 +630,7 @@ WebSettings _webSettingsFromWidget(WebView widget) {
hasProgressTracking: widget.onProgress != null,
debuggingEnabled: widget.debuggingEnabled,
gestureNavigationEnabled: widget.gestureNavigationEnabled,
allowsLinkPreview: widget.allowsLinkPreview,
allowsInlineMediaPlayback: widget.allowsInlineMediaPlayback,
userAgent: WebSetting<String?>.of(widget.userAgent),
zoomEnabled: widget.zoomEnabled,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ dependencies:
flutter:
sdk: flutter
path: ^1.8.0
webview_flutter_platform_interface: ^1.8.0
# FOR TESTING ONLY. DO NOT MERGE.
webview_flutter_platform_interface:
path:
../webview_flutter_platform_interface

dev_dependencies:
build_runner: ^2.1.5
Expand Down