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

Conversation

@vkjon
Copy link

@vkjon vkjon commented Apr 17, 2019

This PR introduces a forwardKey method in FlutterTextInputDelegate that is intended to forward incoming key events.

We had a requirement to track delete key presses in text fields even when there was no text present. For now this PR only forwards the delete key.

Refer to flutter/flutter#29845 for corresponding changes on the framework.

@cbracken
Copy link
Member

/cc @stuartmorgan for thoughts since I assume key event handling is something we'll eventually want for desktop.

I'm not a fan of handling delete key as a special-case one-off. I think we'd want a clear design doc around this and documentation. e.g. behaviour relative to multi-stage IMEs such as Japanese/Chinese input etc.

@stuartmorgan-g
Copy link
Contributor

/cc @gspencergoog who has been driving key event handling.

This is somewhat distinct in that it sounds like this case requires receiving the event whether it's handled or not, rather than only passing unhandled events up the chain. Agreed that just forwarding this one event seems odd though. Perhaps there's a way to make it so that a raw key event listener could be used in conjunction with a text field instead?

@gspencergoog
Copy link
Contributor

From looking at the existing code, I don't actually see why you should need this. In AndroidKeyProcessor.java, the event always gets sent to sendKeyEvent right before it sends it over the keyevent channel anyhow.

I just tested the existing code with a RawKeyboardListener around a text field, and it correctly reports having the delete (and backspace, and arrow keys) pressed as down and up key events, with a soft keyboard visible, or not visible.

In any case, if this is necessary, the right fix would include a way to receive any key, not just the delete key.

@vkjon
Copy link
Author

vkjon commented May 1, 2019

Hmmm @gspencergoog I tried using RawKeyboardListener around the TextField widget but I wasn't able to log any keys being received. I was testing it on a real device, iPhone Xs.

Would you mind posting the test sample? I'll try on a new project as well.

Edit: Maybe the iOS equivalent of feature is missing from the engine.

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented May 1, 2019

@gspencergoog
Copy link
Contributor

Yeah, we don't have raw key event support on iOS yet. Once that is implemented, then the RawKeyboardListener will be the way to go. Sorry about that.

@vkjon
Copy link
Author

vkjon commented May 2, 2019

From what I am aware of, Apple doesn't expose Key level events in their public API.

Maybe a solution for iOS would be to forward keys from UIKeyInput (impl. by TextFields) to the platform's keyEvent channel. (Not sure if this is what @stuartmorgan meant)

Tagging: flutter/flutter#2601, flutter/flutter#31945

@cbracken
Copy link
Member

cbracken commented May 6, 2019

From what I am aware of, Apple doesn't expose Key level events in their public API.

@snowxd you're correct (again AFAIK).

Maybe a solution for iOS would be to forward keys from UIKeyInput (impl. by TextFields) to the platform's keyEvent channel.

It's not ideal, but as you say, UIKeyInput/UITextInput probably does provide enough info to roughly synthesise such events, albeit somewhat inefficiently. Most edits to the text are funnelled through a couple methods.

@stuartmorgan is this what you were suggesting?

@stuartmorgan-g
Copy link
Contributor

stuartmorgan-g commented May 15, 2019

Yes, unless that ends up being unworkable in practice. It just seems like having best-effort implementation of the existing general API would be a better solution than one-off modifications to text field behavior for specific key use cases.

@cbracken
Copy link
Member

@snowxd are you interested in updating this PR to take the approach suggested by @stuartmorgan? If not, I suggest we close this PR but file a bug documenting that approach so we don't lose track of it.

@vkjon
Copy link
Author

vkjon commented May 25, 2019

@cbracken I submitted #9010 which implements the suggested approach.

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.

5 participants