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

SetKeyboardFocusHere focus broken in popups / with clipping #343

Closed
benvanik opened this issue Sep 20, 2015 · 3 comments
Closed

SetKeyboardFocusHere focus broken in popups / with clipping #343

benvanik opened this issue Sep 20, 2015 · 3 comments

Comments

@benvanik
Copy link
Contributor

This issue makes it difficult to auto-highlight/focus text inputs in popups that aren't at the top.

Repro in imgui_demo.cpp, change the first aquarium popup to:

            bool opened_popup = false;
            if (ImGui::Button("Select..")) {
              ImGui::OpenPopup("select");
              opened_popup = true;
            }
            ImGui::SameLine();
            ImGui::Text(selected_fish == -1 ? "<None>" : names[selected_fish]);
            if (ImGui::BeginPopup("select"))
            {
                ImGui::Separator();
                for (int i = 0; i < IM_ARRAYSIZE(names); i++)
                    if (ImGui::Selectable(names[i]))
                        selected_fish = i;
                if (opened_popup) {
                  ImGui::SetKeyboardFocusHere();
                }
                char input[32] = "Aquarium";
                ImGui::InputText("test", input, sizeof(input));
                ImGui::EndPopup();
            }

If you do this, the SetKeyboardFocusHere fails because the first time the BeginPopup runs the InputText ends up getting clipped in InputText>InputTextEx>ItemAdd>IsClippedEx. If the InputText (and SKFH) is at the top of the popup as the first item it works, but anywhere below other items and it fails.

A workaround is that on the next pass through BeginPopup it works (as the InputText is no longer clipped), however that makes the logic messier as one has to keep a counter to wait for the second pass to do the SKFH :(

@ocornut
Copy link
Owner

ocornut commented Sep 20, 2015

One right way to fix it would be to do the processing for FocusableItemRegister() prior to clipping. This would also allow to tab-past the visible widgets (would also require to alter scroll accordingly).

This ties to a larger totally missing feature, that is keyboard/controller navigation. Which would require every widget to be focusable so this probably needs to be re-engineered. If only for performances, but also because this whole SKFH is probably a bit short-sighted and we'd ideally need ways of selecting / triggering any widget given an ID.

There's also the ImGuiSetCond_Appearing condition used by functions such as which might tie into a potential better API.

I'm not really sure what to do short term, I'd prefer to tackle the larger set of focus related problems. I may go for a short-term fix of just making SetKeyboardFocusHere() set a counter to essentially make IsClippedEx() not clip, but that would only work with positive values of the 'offset=0' parameter (which is most of the case).

This is all quite smelly.

@benvanik
Copy link
Contributor Author

Excellent points! For now I'll keep my workaround in.

@ocornut ocornut added the popups label Aug 16, 2017
@ocornut ocornut changed the title SetKeyboardFocusHere focus broken in popups SetKeyboardFocusHere focus broken in popups / with clipping Sep 1, 2017
@ocornut ocornut added the focus label Jan 17, 2018
ocornut added a commit that referenced this issue Apr 30, 2021
…le it prior to clipping (not yet the case) (#343, #4079)

Now performed in ItemAdd(). It can't be trivially moved above clipping effectively because it would require us to scroll to be useful, meaning we'd be better off locking the bounding box a frame earlier. Still wip.
As-is this commit has no value for end-user, but it's a reengineering that moves us closer to the solution. + Internals: moved internal flags.
ocornut added a commit that referenced this issue Oct 6, 2021
…for clipped items. (#343, #4079, #2352, #432)

+ Removed references to counter used by previous implementation of SetKeyboardFocus functions (the TabStop ones will be removed after)
@ocornut
Copy link
Owner

ocornut commented Oct 6, 2021

Better late than never.... using SetKeyboardFocusHere() on non-visible item is finally fixed by 31d033c + 8f495e5.
It was particularly trickier than meet-the-eyes but satisfied with current version.

(I'm not done with the refactor as Tabbing currently doesn't scroll, I still have a few things I don't yet know how to solve for the tabbing but we are nearly there).

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

No branches or pull requests

2 participants