Skip to content
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

Soft keyboard input for iOS #3571

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

simlay
Copy link
Contributor

@simlay simlay commented Mar 5, 2024

  • Tested on all platforms changed
  • Added an entry to CHANGELOG.md if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality
  • Updated feature matrix, if new features were added or implemented

Related to #2260 and #1823.

@@ -100,6 +100,7 @@ pub trait WindowExtIOS {
///
/// The default is to not recognize gestures.
fn recognize_rotation_gesture(&self, should_recognize: bool);
fn set_keyboard_visible(&self, visible: bool);
Copy link
Member

Choose a reason for hiding this comment

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

We already have set_ime_allowed, so wire to it for now.

@simlay simlay force-pushed the add-soft-keyboard-input-to-ios branch from 324d220 to 78bac6b Compare March 5, 2024 18:14
Copy link
Member

@madsmtm madsmtm left a comment

Choose a reason for hiding this comment

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

Have added a few comments wrt. objc2 stuff.

Regarding the implementation, I'm unsure if this is the right approach? Adding a hidden view like this seems like it could get into a lot of trouble with event focus?

There's no other way to control the software keyboard?

(I suspect this will be easier to solve after something like #3506, but I'd be fine with this until then).

src/platform_impl/ios/uikit/text_field.rs Outdated Show resolved Hide resolved
src/platform_impl/ios/uikit/text_field.rs Outdated Show resolved Hide resolved
Comment on lines 48 to 53
// These are methods from UIResponder
#[method(becomeFirstResponder)]
pub fn focus(&self) -> bool;

#[method(resignFirstResponder)]
pub fn unfocus(&self) -> bool;
Copy link
Member

Choose a reason for hiding this comment

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

Should be moved to an extern_methods! on UIResponder.

src/platform_impl/ios/uikit/text_field.rs Outdated Show resolved Hide resolved
Comment on lines +55 to +57
fn window(&self) -> Option<Id<WinitUIWindow>> {
unsafe { msg_send_id![self, window] }
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd rather have this on UIView and return UIWindow, and then when the WinitUIWindow is needed, you'd use the method on UIView, but with an Id::cast to turn it into WinitUIWindow.

}

declare_class!(
pub(crate) struct WinitTextField;
Copy link
Member

Choose a reason for hiding this comment

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

I suspect you don't need to subclass UITextField, i.e. this class is unnecessary as long as you make sure to store the WinitTextFieldDelegate somewhere else (e.g. on WinitView, as done currently).

simlay and others added 4 commits March 15, 2024 00:15
Co-authored-by: Mads Marquart <mads@marquart.dk>
Co-authored-by: Mads Marquart <mads@marquart.dk>
Co-authored-by: Mads Marquart <mads@marquart.dk>
@simlay
Copy link
Contributor Author

simlay commented Apr 3, 2024

Regarding the implementation, I'm unsure if this is the right approach? Adding a hidden view like this seems like it could get into a lot of trouble with event focus?

After some research and thought, I don't think so but I'm not sure what the least-bad-way to do it is. I wrote about this in a chat but felt like it needed to on this issue.

I decided to look at how Flutter does it and how SwiftUI probably does it.

For Flutter: I created a simple flutter app with just a TextField, loaded it into my iOS simulator inspected the UI heirarchy in xcode, and it showed FlutterTextInputView which is defined in the depths of flutter/engine repo. It's unclear how one puts various UIViews (from the apple uikit framework) mixed in with skia rendering parts.

For SwiftUI, I created a simple view with a TextView and it's a TextEditorTextView in the UI hierarchy. This has some sub parts which are UIViews. From the relatively few results on a github search for TextEditorTextView, the one that fits is SwiftUI.TextEditorTextView : UITextView which say they're generated/reverse engineered with RuntimeBrowser. So, it seems that UITextView mixed in similar to flutter.

I'd look into how xamrin does text input but given how old it is, I'm pretty sure it's using a UITextView or the like as well.

@lucasmerlin
Copy link
Contributor

In my fork of winit I implemented iOS keyboard input by just implementing UIKeyInput for the WinitView. This works pretty well, but it doesn't support autocomplete / autocorrect, for that to work we would need to implement UiTextInput as well.

Here are the relevant changes:

lucasmerlin@0eec676#diff-6540857eb4ce8f18ff73ad5a24513873e5572acb1ca867b055a462bdf8ea9e23

If this seems like a better way, I could open a PR

@madsmtm
Copy link
Member

madsmtm commented May 16, 2024

@simlay: Yeah, why are we not just using UITextInput and the related protocols? It really seems like UITextView/UITextField is the wrong approach here, since they also display the text itself, which we in Winit want to leave as a task to the user.

@kchibisov
Copy link
Member

Isn't all the preedit, etc is shown as a part of the keyboard UI? If so, I don't see how it's different to Android where we want to do things like that.

If it shows in some bubble(which is likely not the case, because it's not desktop), then sure, but regular preedit/corrections is just an IME.

@simlay
Copy link
Contributor Author

simlay commented May 16, 2024

@simlay: Yeah, why are we not just using UITextInput and the related protocols? It really seems like UITextView/UITextField is the wrong approach here, since they also display the text itself, which we in Winit want to leave as a task to the user.

Oh that was just because I was thinking this was how other frameworks did it. I think someone on matrix pointed out that react-native, flutter et al implement the UITextInput protocols.

If this seems like a better way, I could open a PR

@lucasmerlin your changes seem more of the correct path forward. Like you said, the autocomplete part is tough. Does implementing the UITextInput protocols enable moving the cursor of the input?

@lucasmerlin
Copy link
Contributor

lucasmerlin commented May 16, 2024

@simlay

Does implementing the UITextInput protocols enable moving the cursor of the input?

No Idea, but looking at the docs, I think that's what the floating cursor apis might be for? I have no idea how this would be handled by winit though.

@lucasmerlin your changes seem more of the correct path forward.

Feel free to fork / modify / copy the code from my fork if you want to keep working on this. I think it might make sense to open a PR that implements UiKeyInput with basic text input support (which should be fairly straightforward based on my changes) and then open another one that implements UiTextInput afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants