[webview_flutter_platform_interface] Adds support to return null in PlatformWebViewController.setOnJavaScriptTextInputDialog
#5901
Conversation
|
Whoops it looks like I forgot the android implementation was already submitted. |
| @@ -870,19 +870,6 @@ class MockAndroidWebViewController extends _i1.Mock | |||
| returnValue: _i9.Future<void>.value(), | |||
| returnValueForMissingStub: _i9.Future<void>.value(), | |||
| ) as _i9.Future<void>); | |||
|
|
|||
| @override | |||
| _i9.Future<void> setOnJavaScriptTextInputDialog( | |||
There was a problem hiding this comment.
#5796 has not landed yet, so I removed these generated overrides to have the pathified test pass.
| @@ -318,7 +318,7 @@ abstract class PlatformWebViewController extends PlatformInterface { | |||
| /// Sets a callback that notifies the host application that the web page | |||
| /// wants to display a JavaScript prompt() dialog. | |||
| Future<void> setOnJavaScriptTextInputDialog( | |||
| Future<String> Function(JavaScriptTextInputDialogRequest request) | |||
| Future<String?> Function(JavaScriptTextInputDialogRequest request) | |||
There was a problem hiding this comment.
This is a breaking change; we'll need to make a replacement method instead (e.g., setOnJavaScriptTextInputDialogCancelable).
We should also document what null means in its doc comment.
There was a problem hiding this comment.
Is there not a way to submit this without a breaking change? The Android and iOS implementation PRs have not landed yet.
There was a problem hiding this comment.
It's already breaking our own tests in other packages to try to change it, and there's no reason a third-party implementation of webview_flutter couldn't already be relying on it.
There was a problem hiding this comment.
Thats fair. I'll create a separate method for it. It's unfortunate that I didn't catch this earlier.
There was a problem hiding this comment.
Yes, the reason I set the process up to have the individual PRs only split out once the combined PR has been fully reviewed is to try to avoid this kind of thing, but it's inevitable that sometimes we just won't notice things until that final read-through.
|
closing since this isn't on my current list of todos. Added flutter/flutter#145799 to track interest. |
Based on the documentation for
promptand WebChromClient.onJsPrompt, this method should support returning a null value for the scenario that a user cancels the operation to enter text.Side note: It seems reasonable the
confirmdialog can just returnfalseif the user cancels the operation, so no changes to that one.Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.