[webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS #5110

b099l3 · 2022-03-26T00:51:35Z

This PR fixes this issue:

WebView: support attribute allowsLinkPreview for iOS flutter#69748
(duplicate) [webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS flutter#100783

Exposes the allowsLinkPreview boolean property on WKWebview. To enable or disable link previews.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…oid with allowsLinkPreview

bparrishMines · 2022-04-05T16:45:55Z

Hi @b099l3, thanks for the contribution! We're currently working on reworking the plugin interface and converting the platform implementation to mirror the native apis. We have a current halt on new features until these two issues are complete. We'll revisit this PR/issue thereafter.

stuartmorgan · 2022-07-28T20:18:51Z

The Dart conversion is complete, and this doesn't change anything problematic in the platform interface, so this is unblocked now. You'll just need to update the implementation for the new Dart version.

b099l3 · 2022-07-29T14:33:31Z

nice I'll take a look this weekend

cyanglaz · 2022-08-22T17:35:10Z

packages/webview_flutter/webview_flutter/test/webview_flutter_test.dart

@@ -1139,6 +1139,7 @@ void main() {
        const WebView(
          initialUrl: 'https://youtube.com',
          gestureNavigationEnabled: true,
+          allowsLinkPreview: false,


can we also test the default value is true?

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

cyanglaz · 2022-08-22T17:38:42Z

packages/webview_flutter/webview_flutter/lib/src/webview.dart

+  /// This only works on iOS.
+  ///
+  /// By default `allowsLinkPreview` is true, to match the default on iOS.
+  final bool allowsLinkPreview;


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

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.)

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.'

bparrishMines · 2022-08-26T20:52:07Z

@b099l3 Was this ready for another review? Are you still working on it?

b099l3 · 2022-09-02T14:22:56Z

Hi sorry still working on it, had to get my head back in the code, looking at it today

…> creation"

b099l3 · 2022-09-07T17:53:22Z

Cool think this is good for a review @bparrishMines when you have time

bparrishMines · 2022-09-07T13:22:00Z

packages/webview_flutter/webview_flutter/test/webview_flutter_test.dart

+          verify(mockWebViewPlatformController.updateSettings(captureAny))
+              .captured
+              .last as WebSettings;
+      expect(disabledSettings.debuggingEnabled, true);


Shouldn't this be testing allowsLinkPreview not debuggingEnabled

changed to allowsLinkPreview

bparrishMines · 2022-09-07T13:26:01Z

packages/webview_flutter/webview_flutter_wkwebview/lib/src/web_kit_webview_widget.dart

+    if (allowsInlineMediaPlayback != null) {
+      configuration.setAllowsInlineMediaPlayback(allowsInlineMediaPlayback);
+    }


These are a copy of the lines above.

deleted these lines

bparrishMines · 2022-09-07T13:29:56Z

...bview_flutter/webview_flutter_platform_interface/lib/v4/src/platform_webview_controller.dart

@@ -243,6 +243,12 @@ abstract class PlatformWebViewController extends PlatformInterface {
        'enableGestureNavigation is not implemented on the current platform');
  }

+  /// Whether to allow link previews on supported platforms.


So it looks like you're adding the ability to set the value in the PlatformWebViewController.enableLinkPreview and through WebView.allowsLinkPreview. It should be one or the other because they can contradict. I don't have a preference on which one, so you can decide.

removed PlatformWebViewController.enableLinkPreview in favour of WebView.allowsLinkPreview

b099l3 · 2022-09-10T12:17:47Z

@bparrishMines I'm getting some funky test failures but I don't think I have changed that code. I have made the changes from the review

bparrishMines · 2022-09-13T19:40:08Z

.../webview_flutter_platform_interface/test/src/method_channel/webview_method_channel_test.dart

@@ -435,6 +435,7 @@ void main() {
        hasProgressTracking: true,
        debuggingEnabled: true,
        gestureNavigationEnabled: true,
+        allowsLinkPreview: false,


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.

Removed from the method channel test

bparrishMines · 2022-09-13T19:54:27Z

packages/webview_flutter/webview_flutter/test/webview_flutter_test.dart

+      expect(params.webSettings!.allowsLinkPreview, false);
+    });
+
+    testWidgets('can be changed', (WidgetTester tester) async {


This test is failing because the allowsLinkPreview values needs to be updated at https://github.com/flutter/plugins/blob/main/packages/webview_flutter/webview_flutter/lib/src/webview.dart#L390.

I have removed allowsLinkPreview from WebSettings, I think that is correct?

No, you still want allowsLinkPreview in WebSettings so that updates can be made to it. I would also re-add the test that were removed for it. You just need to add the updates to allowsLinkPreview in the method mentioned above. e.g.

WebSettings _clearUnchangedWebSettings( WebSettings currentValue, WebSettings newValue) { assert(currentValue.javascriptMode != null); /* other values */ assert(newValue.allowsLinkPreview != null); JavascriptMode? javascriptMode; /* other values */ bool? allowsLinkPreview; if (currentValue.javascriptMode != newValue.javascriptMode) { javascriptMode = newValue.javascriptMode; } /* other values */ if (currentValue.allowsLinkPreview != newValue.allowsLinkPreview) { allowsLinkPreview = newValue.allowsLinkPreview; } return WebSettings( javascriptMode: javascriptMode, /* other values */ allowsLinkPreview: allowsLinkPreview, ); }

Sorry for the misunderstanding; that's my bad. I have added it back in and made the changes

bparrishMines · 2022-09-13T19:56:34Z

The ios-build_all_plugins CHANNEL:stable is failing due to an infra error. It should pass on a rerun.
ios-platform_tests CHANNEL:master is failing due to a flake. It should pass on a rerun as well.

bparrishMines

LGTM!

If this looks good to @stuartmorgan and @cyanglaz, you should be able to make a separate PR with just the interface changes and we can land that first (#3): https://github.com/flutter/flutter/wiki/Contributing-to-Plugins-and-Packages#changing-federated-plugins

stuartmorgan · 2023-01-11T19:37:10Z

This will need to be updated for the restructuring in webview_flutter v4; once that's done we can re-review. Apologies that this fell through the cracks on secondary review while the v4 changes were happening, but we can make sure to review faster once it's updated.

stuartmorgan · 2023-02-09T20:48:26Z

I'm going to go ahead and mark this as a draft since it needs to be reworked for v4, just to get it off of our review queue. Please mark it as ready for review once that's been done. Thanks!

stuartmorgan · 2023-02-22T15:57:12Z

We've just completed the migration of the plugin code to the flutter/packages repository, as described in https://flutter.dev/go/flutter-plugins-repo-migration, and this repository is now being archived. Unfortunately that means that all in-progress PRs here must be moved to flutter/packages.

Please see our instructions for an explanation of how to move your PR, and if you have any issues moving your PR please don't hesitate to reach out in the #hackers-ecosystem channel in Discord.

Our apologies that your PR was caught in this one-time transition. We're aware that it's disruptive in the short term, and appreciate your help in getting us to a better long-term state!

b099l3 added 9 commits March 26, 2022 00:03

added allowsLinkPreview to platform interface

3612a84

Bump platform interface version to 1.8.2

d13276f

added allowsLinkPreview to wkwebview

bb17b4d

bumped wkwebview to 2.7.2

e659cd5

updated flutter android to use platform interface with allowsLinkPreview

6b0ad2e

updated flutter web to use platform interface with allowsLinkPreview

06ba8dd

updated webview_flutter to use platform interface, wkwebview and andr…

4a78973

…oid with allowsLinkPreview

bumped webview_flutter to 3.0.2

eec288f

format changes

78bec61

b099l3 requested review from ditman and bparrishMines as code owners March 26, 2022 00:51

github-actions bot added p: webview_flutter Edits files for a webview_flutter plugin platform-android platform-ios platform-web labels Mar 26, 2022

Merge branch 'main' into iain_smith/webview-wkwebview-link-preview

ed14ad2

b099l3 requested a review from cyanglaz as a code owner July 31, 2022 21:32

cyanglaz reviewed Aug 22, 2022

View reviewed changes

b099l3 added 4 commits September 2, 2022 15:58

Merge branch 'main' into iain_smith/webview-wkwebview-link-preview

baaf7d5

removed allowsLinkPreview setting in "Custom platform implementation …

82be993

…> creation"

Added tests for allowsLinkPreview

09ae8b6

Changed docs comment to say not supported on all platforms

9eadd8e

b099l3 requested review from cyanglaz and removed request for ditman and bparrishMines September 2, 2022 16:24

b099l3 added 4 commits September 7, 2022 01:34

matching naming for native method

b8675f9

Added native test for testSetAllowsLinkPreview in FWFWebViewHostApiTests

8862792

added dependancy overrides in examples

f2ec62b

reverted changes to pigeon 3.1.5

b642bbe

bparrishMines reviewed Sep 9, 2022

View reviewed changes

b099l3 added 3 commits September 10, 2022 11:18

wrong prop testing and dupe code removal

1aaf428

removed enableLinkPreview from webview controller

f86918b

Merge branch 'main' into iain_smith/webview-wkwebview-link-preview

d414528

b099l3 requested review from bparrishMines and stuartmorgan and removed request for bparrishMines September 10, 2022 12:17

bparrishMines reviewed Sep 13, 2022

View reviewed changes

b099l3 added 4 commits September 14, 2022 01:13

removed allowsLinkPreview from websettings and method channel

9548784

Merge branch 'main' into iain_smith/webview-wkwebview-link-preview

12f6da2

added allowsLinkPreview back to WebSettings

0b39035

Merge branch 'main' into iain_smith/webview-wkwebview-link-preview

b0e2496

b099l3 requested review from bparrishMines and removed request for stuartmorgan September 17, 2022 12:19

bparrishMines reviewed Sep 21, 2022

View reviewed changes

stuartmorgan marked this pull request as draft February 9, 2023 20:48

stuartmorgan closed this Feb 22, 2023

camfrandsen mentioned this pull request Sep 28, 2023

[web_view]Expose the allowsLinkPreview property in WKWebView for iOS flutter/packages#5029

Draft

11 tasks

This was referenced Nov 1, 2023

[web_view]Expose the allowsLinkPreview property in WKWebView for iOS flutter/packages#5292

Closed

[web_view]Expose the allowsLinkPreview property in WKWebView for iOS flutter/packages#5293

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS #5110

[webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS #5110

b099l3 commented Mar 26, 2022 •

edited

Loading

bparrishMines commented Apr 5, 2022

stuartmorgan commented Jul 28, 2022

b099l3 commented Jul 29, 2022

cyanglaz Aug 22, 2022

b099l3 Sep 2, 2022 •

edited

Loading

cyanglaz Aug 22, 2022

stuartmorgan Aug 23, 2022

b099l3 Sep 2, 2022

bparrishMines commented Aug 26, 2022

b099l3 commented Sep 2, 2022 •

edited

Loading

b099l3 commented Sep 7, 2022

bparrishMines Sep 7, 2022

b099l3 Sep 10, 2022

bparrishMines Sep 7, 2022

b099l3 Sep 10, 2022

bparrishMines Sep 7, 2022

b099l3 Sep 10, 2022

b099l3 commented Sep 10, 2022

bparrishMines Sep 13, 2022

b099l3 Sep 14, 2022

bparrishMines Sep 13, 2022

b099l3 Sep 14, 2022

bparrishMines Sep 14, 2022

b099l3 Sep 16, 2022

bparrishMines commented Sep 13, 2022

bparrishMines left a comment

stuartmorgan commented Jan 11, 2023

stuartmorgan commented Feb 9, 2023

stuartmorgan commented Feb 22, 2023

[webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS #5110

[webview_flutter] Expose the allowsLinkPreview property in WKWebView for iOS #5110

Conversation

b099l3 commented Mar 26, 2022 • edited Loading

Pre-launch Checklist

bparrishMines commented Apr 5, 2022

stuartmorgan commented Jul 28, 2022

b099l3 commented Jul 29, 2022

Choose a reason for hiding this comment

b099l3 Sep 2, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines commented Aug 26, 2022

b099l3 commented Sep 2, 2022 • edited Loading

b099l3 commented Sep 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

b099l3 commented Sep 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bparrishMines commented Sep 13, 2022

bparrishMines left a comment

Choose a reason for hiding this comment

stuartmorgan commented Jan 11, 2023

stuartmorgan commented Feb 9, 2023

stuartmorgan commented Feb 22, 2023

b099l3 commented Mar 26, 2022 •

edited

Loading

b099l3 Sep 2, 2022 •

edited

Loading

b099l3 commented Sep 2, 2022 •

edited

Loading