[webview_flutter]Allow specifying a navigation delegate(Android and Dart). #1236

amirh · 2019-02-19T00:23:00Z

This allows the app to prevent specific navigations(e.g prevent
navigating to specific URLs).

iOS implementation in #1323

...ew_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebViewClient.java

packages/webview_flutter/example/lib/main.dart

Hixie · 2019-02-20T22:49:43Z

packages/webview_flutter/example/lib/main.dart

            _toasterJavascriptChannel(context),
          ].toSet(),
+          navigationDelegate: (NavigationRequest request) {
+            if (request.url == 'https://www.youtube.com/') {


startsWith?

Hixie · 2019-02-20T22:50:57Z

packages/webview_flutter/lib/webview_flutter.dart


+/// Information about a navigation action that is about to be executed.
+class NavigationRequest {
+  NavigationRequest({this.url, this.isMainFrame});


constructor is missing docs

consider adding some sort of benchmark to check how many public members are missing docs in the repo

Made the constructor private, filed: flutter/flutter#28599

Hixie · 2019-02-20T22:53:51Z

packages/webview_flutter/lib/webview_flutter.dart

+class NavigationRequest {
+  NavigationRequest({this.url, this.isMainFrame});
+
+  /// The URL a navigation is requested to.


this doc doesn't pass our style guide ("url" and "navigation" and "request" are in the member and class name, so that leaves just "the a is ed to" which isn't meaningful).

Gave it another shot.

Hixie · 2019-02-20T22:54:31Z

packages/webview_flutter/lib/webview_flutter.dart

+
+  @override
+  String toString() {
+    return 'NavigationRequest(url: $url, isMainFrame: $isMainFrame)';


we usually use $runtimeType instead of hardcoding the type in the toString, it makes it work better when you subclass or rename the class or copy-paste the code

Hixie · 2019-02-20T22:56:57Z

general design SGTM.

Hixie · 2019-02-27T22:55:49Z

packages/webview_flutter/example/lib/main.dart


+const String kNavigationExamplePage = '''
+<!DOCTYPE html><html>
+<head><title>Navigation Delegate Example</title></head>thanks


stray "thanks"

This allows the app to prevent specific navigations(e.g prevent navigating to specific URLs). flutter/flutter#25329

amirh · 2019-03-07T22:10:57Z

@mklim / @bparrishMines can you take a look (Ian reviewed mainly the public API).

bparrishMines

LGTM

bparrishMines · 2019-03-07T23:49:21Z

...ew_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebViewClient.java

+
+    @Override
+    public void error(String errorCode, String s1, Object o) {
+      throw new IllegalStateException("navigation ");


nit: extra space after navigation

mklim

LGTM pending one nit.

mklim · 2019-03-08T23:48:23Z

packages/webview_flutter/example/android/gradle.properties

@@ -1 +1,2 @@
 org.gradle.jvmargs=-Xmx1536M
+android.useAndroixX=true


Suggested change

android.useAndroixX=true

android.useAndroidX=true

oops, thanks fixed.

This is the iOS implementation of the navigation delegate method channel. The Dart and Android implementations are in #1236 Splitting off the iOS to keep a reasonable change size. This PR will be merged first. flutter/flutter#25329

* commit '9017d6e7f867af278edd7e8e584d52524f37443f': (408 commits) [webview_flutter]Allow specifying a navigation delegate(Android and Dart). (flutter#1236) Allow specifying a navigation delegate (iOS implementation). (flutter#1323) Change build link in contributors site to cirrus (flutter#1327) [image_picker] Update versioning for flutter#1268 (flutter#1326) [image_picker] remove unnecessary camera permmision (flutter#1268) Exclude longPress from semantics (flutter#1324) [in_app_purchase] refactoring and tests (flutter#1322) [in_app_purchase] Adds Dart BillingClient APIs for loading purchases (flutter#1286) [connectivity] Update README.md (flutter#1201) [camera] Fixes #28350 (flutter#1261) [cloud_functions] Specify version for CocoaPod and handle null regions gracefully (flutter#1316) [in_app_purchase]retrieve receipt (flutter#1303) [firebase_analytics] Add resetAnalyticsData method (flutter#1311) trackCameraPosition is inferred from GoogleMap.onCameraMove (flutter#1314) [google_maps_flutter]ChangeNotifier is replaced with granular callbacks (flutter#1302) [video_player]Do not divide by zero (flutter#793) [firebase_dynamic_links] Version bump for firebase_dynamic_links PR flutter#1142 (flutter#1309) [firebase_dynamic_links] fix dynamic link crash when creating shortlink if warnings are null (flutter#1142) Fix typo in RewardedVideoAdd sample (flutter#927) Add my name to firebase_performance and firebase_dynamic_links owners (flutter#1300) ... # Conflicts: # packages/camera/android/src/main/java/io/flutter/plugins/camera/CameraPlugin.java # packages/camera/ios/Classes/CameraPlugin.m # packages/firebase_auth/ios/Classes/FirebaseAuthPlugin.m # packages/image_picker/android/build.gradle # packages/video_player/android/src/main/java/io/flutter/plugins/videoplayer/VideoPlayerPlugin.java

googlebot added the cla: yes label Feb 19, 2019

Hixie reviewed Feb 20, 2019

View reviewed changes

...ew_flutter/android/src/main/java/io/flutter/plugins/webviewflutter/FlutterWebViewClient.java Show resolved Hide resolved

Hixie reviewed Feb 20, 2019

View reviewed changes

packages/webview_flutter/example/lib/main.dart Show resolved Hide resolved

Hixie reviewed Feb 20, 2019

View reviewed changes

cyanglaz changed the title ~~[WIP] [WebView] Allow specifying a navigation delegate.~~ [webview_flutter][WIP] Allow specifying a navigation delegate. Feb 23, 2019

amirh force-pushed the navigation_delegate branch from 38f735a to e6df240 Compare February 27, 2019 19:45

Hixie reviewed Feb 27, 2019

View reviewed changes

amirh added 6 commits March 7, 2019 14:03

[WebView] Allow specifying a navigation delegate.

729bcbd

This allows the app to prevent specific navigations(e.g prevent navigating to specific URLs). flutter/flutter#25329

review comments follow-up

8ac15be

remove stray thanks

8e2a9e9

retain HTTP headers when possible when using a navigation delegate

46d8bf6

add more documentation for caveats on Android

8d9101c

format

a743f8d

amirh mentioned this pull request Mar 7, 2019

Allow specifying a navigation delegate (iOS implementation). #1323

Merged

amirh changed the title ~~[webview_flutter][WIP] Allow specifying a navigation delegate.~~ [webview_flutter]Allow specifying a navigation delegate(Android and Dart). Mar 7, 2019

amirh force-pushed the navigation_delegate branch from d878bfa to a743f8d Compare March 7, 2019 22:09

amirh requested review from bparrishMines and mklim March 7, 2019 22:10

bparrishMines approved these changes Mar 8, 2019

View reviewed changes

use WebViewClientCompat, rename isMainFrame to isForMainFrame

6df13d9

mklim approved these changes Mar 8, 2019

View reviewed changes

amirh added 3 commits March 8, 2019 16:06

typo fix, update documentation

d9d671e

bump version, update changelog

e3eec87

don't call super.shouldOverrideUrlLoading

530d024

amirh added 2 commits March 8, 2019 19:45

Merge branch 'master' into navigation_delegate

151b47d

fix nit

ad30926

amirh merged commit 9017d6e into flutter:master Mar 9, 2019

amirh deleted the navigation_delegate branch March 9, 2019 04:06

amirh mentioned this pull request Mar 9, 2019

[WebView] Allow the webview to take control when a URL is about to be loaded flutter/flutter#25329

Closed

amirh mentioned this pull request Mar 18, 2019

WebView crash: IllegalArgumentException: reasonPhrase can't be empty. flutter/flutter#29446

Closed

This was referenced Mar 29, 2019

[webview_flutter] fix link navigation for android webview #1228

Closed

[webview_flutter]fixed javascript redirect #1197 #1243

Closed

		@@ -1 +1,2 @@
		org.gradle.jvmargs=-Xmx1536M
		android.useAndroixX=true

[webview_flutter]Allow specifying a navigation delegate(Android and Dart). #1236

[webview_flutter]Allow specifying a navigation delegate(Android and Dart). #1236

Uh oh!

Conversation

amirh commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Feb 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amirh commented Mar 7, 2019

Uh oh!

bparrishMines left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mklim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

amirh commented Feb 19, 2019 •

edited

Loading