Clicking on disabled item doesn't set focus to containing window

[geertbleyen]

Version/Branch of Dear ImGui:

Version 1.90.9, Branch: docking

Back-ends:

imgui_impl_osx.mm + imgui_impl_win32.cpp

Compiler, OS:

Windows11, VS2022; MacOS 15.0, Xcode16

Full config/build information:

Dear ImGui 1.90.8 (19080)
--------------------------------
sizeof(size_t): 8, sizeof(ImDrawIdx): 2, sizeof(ImDrawVert): 20
define: __cplusplus=201402
define: __APPLE__
define: __GNUC__=4
define: __clang_version__=16.0.0 (clang-1600.0.26.3)
define: IMGUI_HAS_VIEWPORT
define: IMGUI_HAS_DOCK
--------------------------------
io.BackendPlatformName: imgui_impl_osx
io.BackendRendererName: imgui_impl_metal
io.ConfigFlags: 0x00000443
 NavEnableKeyboard
 NavEnableGamepad
 DockingEnable
 ViewportsEnable
io.ConfigViewportsNoDecoration
io.ConfigMacOSXBehaviors
io.ConfigInputTextCursorBlink
io.ConfigWindowsResizeFromEdges
io.ConfigMemoryCompactTimer = 60.0
io.BackendFlags: 0x0000140A
 HasMouseCursors
 PlatformHasViewports
 RendererHasVtxOffset
 RendererHasViewports
--------------------------------
io.Fonts: 1 fonts, Flags: 0x00000000, TexSize: 512,64
io.DisplaySize: 1200.00,720.00
io.DisplayFramebufferScale: 2.00,2.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:

We are creating a node editor, and these nodes have an editable title. However, by default, this is a disabled TextInput and the node can be dragged by click+drag on the title as well. However, doing so does not set window focus on the nodeeditor window. I understand and agree that clicking disabled items shouldn't be navigated to, or be activatable, but I'd still expect the containing window gaining focus when clicking on an item inside of it.
Is it intentional that this doesn't happen?

As a quick experiment I hacked following code into ImGui::ButtonBehavior

...
if (flatten_hovered_children)
    g.HoveredWindow = backup_hovered_window;

// new:  here we handle clicking on disabled items to set focus to containing window
if (g.HoveredIdDisabled && g.HoveredId == id)
    {
    int mouse_button_clicked = -1;
    int mouse_button_released = -1;
    const ImGuiID test_owner_id = (flags & ImGuiButtonFlags_NoTestKeyOwner) ? ImGuiKeyOwner_Any : id;
    for (int button = 0; button < 3; button++)
      {
        if (flags & (ImGuiButtonFlags_MouseButtonLeft << button)) // Handle ImGuiButtonFlags_MouseButtonRight and ImGuiButtonFlags_MouseButtonMiddle here.
          {
            if (IsMouseClicked(button, ImGuiInputFlags_None, test_owner_id) && mouse_button_clicked == -1) { mouse_button_clicked = button; }
            if (IsMouseReleased(button, test_owner_id) && mouse_button_released == -1) { mouse_button_released = button; }
          }
      }
    if (mouse_button_released == 0)
        FocusWindow(window);
    }

// Mouse handling
const ImGuiID test_owner_id = (flags & ImGuiButtonFlags_NoTestKeyOwner) ? ImGuiKeyOwner_Any : id;
if (hovered)
{
...

And this makes it 'work' for button-like item (checkbox etc...). I'm assume similar changes would be required to other widget categories (textinput, ...?)

However, it may be intentional that it doesn't work like that at the moment, and a different approach may be better?

Any advice is appreciated on this.

Screenshots/Video:

Screen.Recording.2024-10-15.at.15.48.37.mov

Minimal, Complete and Verifiable Example code:

No response

[ocornut]

Hello,

Thanks for reporting this. I consider it to a bug indeed.

ButtonBehavior() would likely need to be reworked to handle this case better. Right now ItemHoverable() is going to return false yet set g.HoveredId=id, g.HoveredIdIsDisabled=true. I guess we need to rework the "Mouse Handling" block to enter into the block in this situation and do some of the processing, namely the FocusWindow() calls.
EDIT Sorry I have overlooked that you went through a similar path already!

It's a little complex and fragile so I would need to spend a few hours double-checking all paths and implementing new tests into the test suite to avoid making a mistake.

I don't know when I would have the resources to look into this.

[geertbleyen]

Indeed, it looked quite complex when we were going through it :).
Like I said, I quickly hacked something together in the current constraints to isolate it specifically for button-like widgets, by copy-pasting some logic from the regular if(hovered) code-path.
I'm also not sure if some users of imgui rely on this exact behavior for their purposes and fixing it could cause behavioral breaks in their applications.
For now, we have created a hack on our end (application code) that more or less has the same logic as in my code sample (check if editor window is hovered, check if hoveredIdDisabled is set and check mousebutton release state).

[ocornut]

I don't believe people would rely on this exact behavior and I would generally like to change it to focus windows in that situation. It just needs a bit of careful energy and testing.

[ocornut]

I have pushed a fix 83ecc84 + super basic test ocornut/imgui_test_engine@4b349fb. It seems simpler than I thought, even though digging into that code I noticed a few inconsistency which I would like to investigate later (added FIXME notes).

[geertbleyen]

Awesome, thanks for the fix. Can it also be fixed for the other widget 'kinds' (sliders, drags, textinput)?

[ocornut]

My bad, not only this was incomplete (some widgets using ItemHoverable() directly without ButtonBehavior()) but it was too complex, and I don't know why I avoided looping other all widget types in my test. I guess I was tired yesterday evening.

I've reverted 83ecc84 and pushed a simpler and more consistent change 706438a and amended the test.

[geertbleyen]

Great work, highly appreciated!