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 Jan 29, 2019

Platform implementation of the method channel API for adding and removing JavaScript channels.

#1116 adds the Dart side support, the current PR will land first.

flutter/flutter#24837

@amirh amirh requested review from mklim and removed request for mklim January 29, 2019 00:27
@amirh amirh changed the title WebView JavasScript channels Android implementation. [WIP] WebView JavasScript channels Android implementation. Jan 29, 2019
@amirh amirh changed the title [WIP] WebView JavasScript channels Android implementation. WebView JavasScript channels Android implementation. Jan 30, 2019
@amirh amirh requested a review from mklim January 30, 2019 20:24
Copy link
Contributor

@mklim mklim left a comment

Choose a reason for hiding this comment

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

LGTM

Same as the other PR, not sure if it's totally worth it for this but this is the type of thing that you could add unit tests for.


applySettings((Map<String, Object>) params.get("settings"));

if (params.containsKey("javascriptChannelNames")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, optional: Consider breaking out the "javascriptChannelNames" magic string into a private constant on this class or a named variable. It'll prevent typo problems with getting the value.

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

private final MethodChannel methodChannel;
private final String javaScriptChannelName;

JavaScriptChannel(MethodChannel methodChannel, String javaScriptChannelName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can you also add docs explaining these params? I think it's just tricky enough that it's worth saying what they do here.

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

@mklim mklim removed the WIP label Jan 30, 2019
@amirh amirh merged commit 7d494b4 into flutter:master Feb 4, 2019
@amirh amirh deleted the webview_js_android branch February 4, 2019 20:56
andreidiaconu pushed a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
Platform implementation of the method channel API for adding and removing JavaScript channels.

flutter#1116 adds the Dart side support, the current PR will land first.

flutter/flutter#24837
andreidiaconu added a commit to andreidiaconu/plugins that referenced this pull request Feb 17, 2019
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
Platform implementation of the method channel API for adding and removing JavaScript channels.

flutter#1116 adds the Dart side support, the current PR will land first.

flutter/flutter#24837
karantanwar pushed a commit to karantanwar/plugins that referenced this pull request Feb 19, 2019
Platform implementation of the method channel API for adding and removing JavaScript channels.

flutter#1116 adds the Dart side support, the current PR will land first.

flutter/flutter#24837
alien190 added a commit to alien190/plugins that referenced this pull request Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants