-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[webview_flutter_platform_interface] Adds support to return null in PlatformWebViewController.setOnJavaScriptTextInputDialog
#5901
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f8731c6
1b5b9d6
545e3ff
8c8501d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change; we'll need to make a replacement method instead (e.g., We should also document what
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there not a way to submit this without a breaking change? The Android and iOS implementation PRs have not landed yet.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thats fair. I'll create a separate method for it. It's unfortunate that I didn't catch this earlier.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| onJavaScriptTextInputDialog) async { | ||
| throw UnimplementedError( | ||
| 'setOnJavaScriptTextInputDialog is not implemented on the current platform', | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#5796 has not landed yet, so I removed these generated overrides to have the
pathifiedtest pass.