godot
godot copied to clipboard
godotengine
Fix `ColorPicker` virtual keyboard popup on mobile
This PR fixes auto popup of virtual keyboard when colorPickerButton is pressed on mobile devices. Fixes #88235
It looks good but I don't know if we should return from TextEdit and LineEdit show_virtual_keyboard instead, to avoid showing the virtual keyboard when a physical keyboard is detected, at least we can do it for android and ios only but what if the users still wants to use the virtual keyboard ? There's no option to prefer virtual keyboard over physical one.
@WhalesState Isn't this the current behaviour, virtual keyboard does not appear if physical keyboard is connected.
@WhalesState Isn't this the current behaviour, virtual keyboard does not appear if physical keyboard is connected.
There was no way to check for physical keyboards before, it was recently merged so it always show the virtual keyboard on android and ios by default.
That's why i have applied a hack for this, since focusing any LineEdit or TextEdit will still show the virtual keyboard even if a hardware keyboard is connected, but I don't know if there are any opened issues or discussions about this.
There was no way to check for physical keyboards before, it was recently merged so it always show the virtual keyboard on android and ios by default.
No, virtual keyboard doesn't appear if physical kb is connected even before that PR . This was used internally just wasn't exposed.
There's an option on android Physical keyboard settings Show on-screen keyboard while a physical keyboard is being used, which is true by default.
There's an option on android
Physical keyboardsettingsShow on-screen keyboardwhile a physical keyboard is being used, which is true by default.
I just checked in two different devices and it was turned off in Motorola and turned on in Xiaomi, both are default. So, yeah the only way to fix this will be to not call show_virtual_keyboard() in lineEdit and textEdit.
But that is out of scope of this PR.
The linked issue was closed by another PR. Is this still relevant?
Yes, this fix is simpler and IMO better. I think the only difference is that, fix in that PR hides virtual keyboard but still keep html_color box focused, But I think completely removing the focus would be better.
@KoBeWi It would be great if you could have a look at changes 🙂
I've approved this PR since it was submitted after my own fix and my PR was approved but not merged yet. However, I do have some questions about the approach taken here.
I'm not sure why this fix was submitted as a separate PR instead of being discussed in my original PR, where the issue was already being tracked and addressed.
Regarding the fix itself, I agree that it's simpler. However, I do have some concerns about its accuracy. Specifically, every popup should focus on at least one control.
Also to avoid potential regressions on other platforms when has_hardware_keyboard() is supported, I made sure to apply the changes only to Android and iOS.
I'm not familiar enough with the underlying mechanics to provide a definitive opinion on which fix is better. I'd be happy to let more experienced team members weigh in and provide guidance on the best approach.
I'm not sure why this fix was submitted as a separate PR instead of being discussed in my original PR, where the issue was already being tracked and addressed.
That PR was already addressing multiple issues, so I thought it would be better to open a new one. ( I accept that was an error an my part)
Regarding the fix itself, I agree that it's simpler. However, I do have some concerns about its accuracy. Specifically, every popup should focus on at least one control.
If hardware keyboard is not connected, this fix won't even create a popup or automatically try to open virtual keyboard (when colorpicker button is pressed). It will only focus the html_color field if hardware keyboard is connected OR specifically clicked on the html_color field.
Also to avoid potential regressions on other platforms when has_hardware_keyboard() is supported, I made sure to apply the changes only to Android and iOS.
This fix won't affect any other platform because has_hardware_keyboard() will always return true on any other platform. And if this func gets supported on other platforms then it should work there too.
I hope all your questions are answered :)
