-
Notifications
You must be signed in to change notification settings - Fork 423
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
Allow TextBox
to specify type of text being input
#6408
base: master
Are you sure you want to change the base?
Changes from 4 commits
c0cd31c
d7ce29d
5d4c356
5a34404
06371f4
9b7683b
fb364b1
6a7a92a
4760939
a699201
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,16 +37,16 @@ private void handleTextEditing(string? text, int selectionStart, int selectionLe | |
TriggerImeComposition(text, selectionStart, selectionLength); | ||
} | ||
|
||
protected override void ActivateTextInput(bool allowIme) | ||
protected override void ActivateTextInput(TextInputProperties properties) | ||
{ | ||
window.TextInput += handleTextInput; | ||
window.TextEditing += handleTextEditing; | ||
window.StartTextInput(allowIme); | ||
window.StartTextInput(properties); | ||
} | ||
|
||
protected override void EnsureTextInputActivated(bool allowIme) | ||
protected override void EnsureTextInputActivated(TextInputProperties properties) | ||
{ | ||
window.StartTextInput(allowIme); | ||
window.StartTextInput(properties); | ||
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 doesn't appear to work correctly on android. When switching from a normal textbox to a password textbox immediately, IME remains available. Conversely, when switching from a password textbox to a normal textbox immediately, IME remains unavailable. Record_2024-11-08-11-57-01.mp4I wonder what this looks like on iOS. (I own an iOS device now, but no mac, so I still can't check myself.) Whether this is an SDL bug, or ours, is up for debate I guess. I can say that the naive solution, i.e. diff --git a/osu.Framework/Input/SDLWindowTextInput.cs b/osu.Framework/Input/SDLWindowTextInput.cs
index 79fa1d848..c67bf7f77 100644
--- a/osu.Framework/Input/SDLWindowTextInput.cs
+++ b/osu.Framework/Input/SDLWindowTextInput.cs
@@ -10,6 +10,8 @@ internal class SDLWindowTextInput : TextInputSource
{
private readonly ISDLWindow window;
+ private TextInputProperties? startedInputProperties;
+
public SDLWindowTextInput(ISDLWindow window)
{
this.window = window;
@@ -42,10 +44,14 @@ protected override void ActivateTextInput(TextInputProperties properties)
window.TextInput += handleTextInput;
window.TextEditing += handleTextEditing;
window.StartTextInput(properties);
+ startedInputProperties = properties;
}
protected override void EnsureTextInputActivated(TextInputProperties properties)
{
+ if (startedInputProperties != null && startedInputProperties != properties)
+ window.StopTextInput();
+
window.StartTextInput(properties);
}
@@ -54,6 +60,7 @@ protected override void DeactivateTextInput()
window.TextInput -= handleTextInput;
window.TextEditing -= handleTextEditing;
window.StopTextInput();
+ startedInputProperties = null;
}
public override void SetImeRectangle(RectangleF rectangle)
does not appear to help. 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. I'm seeing roughly the same issue on iOS, I will have to look into libsdl-org/SDL#11406 (comment) again first to tell whether this is an issue on SDL's side or ours. Not sure about Android though, someone else will have to investigate this (libsdl-org/SDL#11406 (comment) may be relevant). 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. After further investigation on the iOS issue, SDL doesn't handle updating text input properties appropriately on iOS, regardless of whether text input is active or not. I've fixed this in the PR linked above (libsdl-org/SDL@f4d81f2) for now. 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. ...so i guess at this point it's either up to me to fix android in sdl, or to accept it's broken over there?
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. I can try to take a stab at fixing this in SDL. On first glance, it doesn't seem to be an android-specific issue as SDL will ignore requests to show the keyboard if it's already active. 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. Created a PR, but I'm not expecting it to go in soon. |
||
} | ||
|
||
protected override void DeactivateTextInput() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
// Copyright (c) ppy Pty Ltd <[email protected]>. Licensed under the MIT Licence. | ||
// See the LICENCE file in the repository root for full licence text. | ||
|
||
namespace osu.Framework.Input | ||
{ | ||
/// <summary> | ||
/// Represents a number of properties to consider during a text input session. | ||
/// </summary> | ||
/// <param name="Type">The type of text being inputted.</param> | ||
bdach marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/// <param name="AllowIme"> | ||
/// <para> | ||
/// Whether IME should be allowed during this text input session. | ||
/// Useful for situations where IME input is not wanted, such as for passwords, numbers, or romanised text. | ||
/// </para> | ||
/// <para> | ||
/// Note that this is just a hint to the native implementation, some might respect this, | ||
/// while others will ignore and always have the IME (dis)allowed. | ||
/// </para> | ||
/// </param> | ||
public record struct TextInputProperties(TextInputType Type, bool AllowIme); | ||
} |
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.
What does this actually... suppress? Because in debug, when I run the game, and type into a password text box, key events are still logged. And that's because they're being logged in a completely different location, namely
osu-framework/osu.Framework/Input/ButtonEventManager.cs
Lines 135 to 136 in d4d9cca
So does this really do anything? Or did it just break at some point and nobody noticed?
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.
Seems like it did regress, since key input used to be logged in this location until it was moved to
KeyEventManager
. I'll correct it in this PR.