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

Conversation

@amirh
Copy link
Contributor

@amirh amirh commented May 29, 2019

This is a followup for #1618 and #1624, and moves all of the plugins.flutter.io/webview method channel calls behind the platform interface, which allows a third party package to provide a new platform implementation for all of these methods.

See the description for #1618 for more details.

The last remaining part is the plugins.flutter.io/cookie_manager channel which I'm leaving for a followup PR.

@amirh amirh requested a review from cyanglaz May 29, 2019 00:50
@amirh
Copy link
Contributor Author

amirh commented May 29, 2019

@nkoroste please take a look

Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

Overall approach looks good, left some comments.

}

@override
Future<void> goBack() async {
Copy link
Contributor

Choose a reason for hiding this comment

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

optional nit:

`Future goBack() =>_channel.invokeMethod("goBack");

Same for others.
Also, we prod don't need the async mark in the method signature if we keep this style.

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

Future<void> loadUrl(
String url, {
Map<String, String> headers,
}) async {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the async here?

Copy link
Contributor Author

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

Thanks, PTAL

}

@override
Future<void> goBack() async {
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

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM!

@amirh amirh merged commit 7b2d195 into flutter:master May 30, 2019
@amirh amirh deleted the multi_platform_webview_3 branch May 30, 2019 21:30
amirh added a commit that referenced this pull request May 31, 2019
This is a followup for #1618, #1624, and #1645, and moves the plugins.flutter.io/cookie_manager method channel behind the platform interface which allows a third party package to provide a new platform implementation the cooke manager.

See the description for #1618 for more details.

Following this PR all platform specific code can be replaced by an external package.
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.

3 participants