ocornut / imgui

Dear ImGui: Bloat-free Graphical User interface for C++ with minimal dependencies
MIT License
59.42k stars 10.12k forks source link

How do I allow use of the keyboard to change focus inside of a ListClipper? #7921

Open db48x opened 2 weeks ago

db48x commented 2 weeks ago

Version/Branch of Dear ImGui:

Version 1.89.6, Branch: master

Back-ends:

imgui_impl_sdl2.cpp

Compiler, OS:

gcc (GCC) 14.2.1, Linux

Full config/build information:

Dear ImGui 1.89.6 (18960)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201703
define: __linux__
define: __GNUC__=4
define: __clang_version__=18.1.6 (Fedora 18.1.6-3.fc40)
--------------------------------
io.BackendPlatformName: imgui_impl_sdl2
io.BackendRendererName: imgui_impl_sdlrenderer2
io.ConfigFlags: 0x00000003
 NavEnableKeyboard
 NavEnableGamepad
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000000E
 HasMouseCursors
 HasSetMousePos
 RendererHasVtxOffset
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,256
io.DisplaySize: 1920.00,1011.00
io.DisplayFramebufferScale: 1.00,1.00
--------------------------------
style.WindowPadding: 8.00,8.00
style.WindowBorderSize: 1.00
style.FramePadding: 4.00,3.00
style.FrameRounding: 0.00
style.FrameBorderSize: 0.00
style.ItemSpacing: 8.00,4.00
style.ItemInnerSpacing: 4.00,4.00

Details:

I have a big list of stuff, and I want the user to pick one. And I want them to be able to use the keyboard to do so.

The basic state that I am storing is a list of menu entries and the index of the selected item. When the user presses certain keys (arrow keys, pageup/pagedown, etc), I adjust the index accordingly. At first this was quite easy; I simply looped over the whole list adding a table row for each one and calling SetScrollHereY when I got to the selected row. This worked perfectly for keyboard inputs, but not for mouse scrolling.

And it doesn’t work if I use a ListClipper because once the selected row is outside the window bounds the clipper no longer iterates over that index and so I no longer get to call SetScrollHereY. I added a call to IncludeRangesByIndices after creating the clipper so that clipper would always iterate over the index of the selected entry. That works, but it still breaks mouse scrolling! So does calling SetNextWindowScroll before creating the child window.

I feel like it can’t possibly be this hard; perhaps I missed something? How can I get the current row of the table into view while not breaking anything else?

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

I don't yet have a completely isolated example, but I can work on it if it is necessary. In the mean time, here is most of the actual code I am running:

float entry_height = ImGui::GetTextLineHeightWithSpacing();
// ImGui::SetNextWindowScroll( { -1, parent.fselected * entry_height } );
if( ImGui::BeginChild( "scroll", parent.calculated_menu_size, false ) ) {
    if( ImGui::BeginTable( "menu items", 3, ImGuiTableFlags_SizingFixedFit ) ) {
        ImGui::TableSetupColumn( "hotkey", ImGuiTableColumnFlags_WidthFixed,
                                 parent.calculated_hotkey_width );
        ImGui::TableSetupColumn( "primary", ImGuiTableColumnFlags_WidthFixed,
                                 parent.calculated_label_width );
        ImGui::TableSetupColumn( "secondary", ImGuiTableColumnFlags_WidthFixed,
                                 parent.calculated_secondary_width );

        ImGuiListClipper clipper;
        clipper.Begin( parent.fentries.size(), entry_height );
        clipper.IncludeRangeByIndices( parent.fselected, parent.fselected+1);
        while( clipper.Step() )
        {
            for ( int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++ )
            {
                auto entry = parent.entries[parent.fentries[i]];
                bool is_selected = i == parent.fselected;
                ImGui::PushID( i );
                ImGui::TableNextRow( ImGuiTableRowFlags_None, entry_height );
                ImGui::TableSetColumnIndex( 0 );

                if( is_selected ) { ImGui::SetScrollHereY(); }
                ImGuiSelectableFlags flags = ImGuiSelectableFlags_SpanAllColumns | ImGuiSelectableFlags_AllowItemOverlap;
                if( !entry.enabled ) { flags |= ImGuiSelectableFlags_Disabled; }
                if( ImGui::Selectable( "##s", is_selected, flags ) )
                {
                    parent.fselected = i;
                    parent.selected = parent.hovered = parent.fentries[ parent.fselected ];
                    if( parent.callback != nullptr ) {
                        parent.callback->select( &parent );
                    }
                    is_selected = parent.clicked = true;
                }
                if( ImGui::IsItemHovered( ImGuiHoveredFlags_NoNavOverride ) )
                {
                    parent.hovered = parent.fentries[ i ];
                }
                ImGui::SameLine( 0, 0 );
                ImGui::PushStyleColor( ImGuiCol_Text, is_selected ? parent.hilight_color : parent.hotkey_color );
                ImGui::TextUnformatted( entry.hotkey.value().short_description().c_str() );
                ImGui::PopStyleColor();

                // … etc

                ImGui::PopID();
            }
        }
        ImGui::EndTable();
    }
}
ImGui::EndChild();
GamingMinds-DanielC commented 2 weeks ago

Mouse scrolling doesn't work because you are overwriting the scroll position every frame. Try updating the scroll position only as a response to changing the selection. Even better if you memorize the current scroll position and update the scroll position only if the newly selected item is not visible already.

db48x commented 2 weeks ago

After I asked the question I cooked and ate a meal, watched a movie, took a shower, and loaded the dishwasher. Finally I thought “What if there was a flag? Why can't I just make me a flag that’s true right after the user presses a key and then is false after?”

Well, I declared myself a bool need_to_scroll; and wrote the obvious thing:

if( is_selected && parent.need_to_scroll ) {
    ImGui::SetScrollHereY();
    parent.need_to_scroll = false;
}

In a different part of the loop I wrote need_to_scroll = scrollby( scroll_amount_from_action( ret_act ) ). And that worked. Perfectly. Ship it.

Except that just before I could commit I noticed one other thing. I have to display some context about the current menu item, but if the user hovers I want to show the context for the hovered item instead of the selected item. That part of the code wasn’t working quite right because if the user leaves their mouse there then one item is always hovered every time the menu is redrawn, and no context about the selected item is shown even if the user uses the arrow keys. I needed another flag that was true right after the mouse hovered over an item, but then went false afterwards. A few minutes of poking around in the ImGui code looking at any line with the word “hover” in it and I had this:

if( ImGui::Selectable( "##s", is_selected, flags ) ) {
    parent.selected = parent.hovered = parent.fentries[ i ];
    // …
}

bool mouse_moved = ImGui::GetCurrentContext()->HoveredId !=
                   ImGui::GetCurrentContext()->HoveredIdPreviousFrame;
if( ImGui::IsItemHovered( ImGuiHoveredFlags_NoNavOverride ) && mouse_moved ) {
    parent.hovered = parent.fentries[ i ];
}

Now hovered is the item the user has most recently looked at, either by hovering with the mouse or by selecting with the keyboard, while selected is what will be returned when the user hits enter. On a mouse click we set selected and then end up returning it.

Is there an ImGui function that gives me the equivalent of mouse_moved?

Finally, here is what I ended up with, in case any future questant has a similar problem:

ImGuiListClipper clipper;
clipper.Begin( parent.fentries.size(), entry_height );
clipper.IncludeRangeByIndices( parent.fselected, parent.fselected + 1 );
while( clipper.Step() ) {
    for( int i = clipper.DisplayStart; i < clipper.DisplayEnd; i++ ) {
        auto entry = parent.entries[parent.fentries[i]];
        bool is_selected = i == parent.fselected;
        ImGui::PushID( i );
        ImGui::TableNextRow( ImGuiTableRowFlags_None, entry_height );
        ImGui::TableSetColumnIndex( 0 );

        if( is_selected && parent.need_to_scroll ) {
            ImGui::SetScrollHereY();
            parent.need_to_scroll = false;
        }
        ImGuiSelectableFlags flags = ImGuiSelectableFlags_SpanAllColumns |
                                     ImGuiSelectableFlags_AllowItemOverlap;
        if( !entry.enabled ) {
            flags |= ImGuiSelectableFlags_Disabled;
        }
        if( ImGui::Selectable( "##s", is_selected, flags ) ) {
            parent.fselected = i;
            parent.selected = parent.hovered = parent.fentries[ parent.fselected ];
            parent.need_to_scroll = true;
            if( parent.callback != nullptr ) {
                parent.callback->select( &parent );
            }
            is_selected = parent.clicked = true;
        }
        bool mouse_moved = ImGui::GetCurrentContext()->HoveredId !=
                           ImGui::GetCurrentContext()->HoveredIdPreviousFrame;
        if( ImGui::IsItemHovered( ImGuiHoveredFlags_NoNavOverride ) && mouse_moved ) {
            parent.hovered = parent.fentries[ i ];
        }
        ImGui::SameLine( 0, 0 );
        ImGui::PushStyleColor( ImGuiCol_Text, is_selected ? parent.hilight_color : parent.hotkey_color );
        ImGui::TextUnformatted( entry.hotkey.value().short_description().c_str() );
        ImGui::PopStyleColor();

        // …

        ImGui::PopID();
    }
}

I should probably add some comments to it, now that I think about it.

Thank you @GamingMinds-DanielC, I bet if I had seen your answer first it would have done the trick!