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

Fix dropdowns showing popup keyboard on mobile even when search bar is initially hidden #6080

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

peppy
Copy link
Member

@peppy peppy commented Dec 18, 2023

No description provided.

@peppy peppy added area:UI platform:mobile next-release Pull requests which are almost there labels Dec 18, 2023
@bdach bdach self-requested a review December 18, 2023 13:26
@bdach
Copy link
Collaborator

bdach commented Dec 18, 2023

As is, this diff does nothing for me (testing on android).

This helps, but also feels tremendously jank:

diff --git a/osu.Framework/Graphics/UserInterface/DropdownSearchBar.cs b/osu.Framework/Graphics/UserInterface/DropdownSearchBar.cs
index 31fdf4bb1..3f7c009a6 100644
--- a/osu.Framework/Graphics/UserInterface/DropdownSearchBar.cs
+++ b/osu.Framework/Graphics/UserInterface/DropdownSearchBar.cs
@@ -74,8 +74,12 @@ public void ObtainFocus()
         {
             // On mobile platforms, let's not make the keyboard popup unless the dropdown is intentionally searchable.
             bool willShowOverlappingKeyboard = host?.OnScreenKeyboardOverlapsGameWindow == true;
+
             if (willShowOverlappingKeyboard && !AlwaysDisplayOnFocus)
+            {
+                textBoxInputManager.UseParentInput = false;
                 return;
+            }
 
             textBoxInputManager.ChangeFocus(textBox);
             obtainedFocus = true;

For whatever reason that I don't really wish to get into the nested input manager doesn't really care if its parent handled or didn't handle the input, and just proceeds to give focus to the textbox anyway, which causes text input to activate. So the "fix" is to just tell it to sit down and not break stuff.

Think it's okay to go with that?

cc @frenzibyte (i had several bad feelings about using nested input managers for anything that isn't a test, and the number of bugs that stem from introducing it tells me that i should have been more forceful in saying so)

@peppy
Copy link
Member Author

peppy commented Dec 18, 2023

I think we can go with that, but it should definitely have an inline disclaimer about how bad this is and that we probably want to find a better solution.

@bdach
Copy link
Collaborator

bdach commented Dec 18, 2023

Have applied with disclaimer.

@bdach bdach enabled auto-merge December 18, 2023 17:47
@peppy peppy disabled auto-merge December 18, 2023 17:48
@peppy peppy merged commit d267e0f into ppy:master Dec 18, 2023
11 of 13 checks passed
@peppy peppy deleted the dropdown-search-mobile-allowance branch January 17, 2024 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI next-release Pull requests which are almost there platform:mobile size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants