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

InvisibleButton: add a way to optionally call RenderNavHighlight inside? #8057

Closed
achabense opened this issue Oct 14, 2024 · 3 comments
Closed
Labels
nav keyboard/gamepad navigation

Comments

@achabense
Copy link

Version/Branch of Dear ImGui:

Version 1.91.2

Back-ends:

imgui_impl_sdl2.cpp + imgui_impl_sdlrenderer2.cpp

Compiler, OS:

Windows 10 + MSVC 2022

Full config/build information:

No response

Details:

ImGui::InVisibleButton is useful for defining custom widgets. However, when ImGuiConfigFlags_NavEnableKeyboard is specified, the keyboard navigation will work on them in an invisible way.
Take the code below for example. If not calling RenderNavHighlight, the effect would be like this. It's hard to tell where the navigation goes.
Example
It will work well if calling RenderNavHighlight. However, as RenderNavHighlight is an internal function, I think it will be helpful to add a way to call RenderNavHighlight inside InvisibleButton. A new button flag may be fine for this change.
Example

Screenshots/Video:

No response

Minimal, Complete and Verifiable Example code:

// ImGui::GetIO().ConfigFlags |= ImGuiConfigFlags_NavEnableKeyboard;

static void example() {
    if (ImGui::Begin("Example", 0, ImGuiWindowFlags_NoSavedSettings)) {
        const int c = 5;
        static bool vals[c]{};
        for (int i = 0; i < c; ++i) {
            if (i != 0) {
                ImGui::SameLine();
            }
            ImGui::PushID(i);
            if (ImGui::InvisibleButton("In", {20, 20})) {
                vals[i] = !vals[i];
            }
            ImGui::PopID();

            // Custom rendering:
            const auto min = ImGui::GetItemRectMin();
            const auto max = ImGui::GetItemRectMax();
            // ImGui::RenderNavHighlight({min, max}, ImGui::GetItemID()); // "imgui_internal.h"
            ImGui::GetWindowDrawList()->AddRectFilled(min, max, vals[i] ? IM_COL32_WHITE : IM_COL32_BLACK);
            ImGui::GetWindowDrawList()->AddRect(min, max, IM_COL32_WHITE);
        }
    }
    ImGui::End();
}
@ocornut ocornut added the nav keyboard/gamepad navigation label Oct 14, 2024
@ocornut
Copy link
Owner

ocornut commented Oct 14, 2024

You are right. I'm surprised that this hasn't been brought before.

I believe the most correct approach would be to add this call in InvisibleButton().
The problem being that large invisible button calls are used in many place and in those cases the lack of nav highlight was accidentally desirable.

So we'd need make sure that the user know they can use e..g PushItemFlag(ImGuiItemFlags_NoNav, ...); Or we may add a public ImGuiButtonFlags_NoNav shortcut for this purpose too.

@ocornut
Copy link
Owner

ocornut commented Oct 14, 2024

A convenient default may to do the opposite:

  • Change InvisibleButton() to use _NoNav by default and so it is correct that it doesn't render highlight. (we need to bundle those two properties together to be consistent)
  • And you can opt-out aka re-enable navigation.
    It's a little bit inconsistent but no breakage and might be a better default. I will be polling a few large users.

ocornut added a commit that referenced this issue Oct 14, 2024
…oNav. (#8057)

Not fully honored in ItemHoverable/IsItemHovered, seems more destructive. This is mostly designed to avoid rectangle being rendered by large InvisibleButton() when ctrl+tabbing back to a window with a big one.
ocornut added a commit that referenced this issue Oct 16, 2024
@ocornut
Copy link
Owner

ocornut commented Oct 16, 2024

I have been agonizing a little too much over this...
It would be consistent to enable nav by default, but with the fix of rendering highlight this is going to create many more issues (e.g. ctrl+tabbing back to a window with a large invisible button).

I also polled some users, and as one in particular said "we have 595 calls to InvisibleButton() in our codebase" I though I would play it safe 😆

Pushed 67e5f35, disable nav by default + added ImGuiButtonFlags_EnableNav.

There is a possibility that someone would want to use InvisibleButton() with nav but no highlight (drawing a custom one). In which case we could decide to specifically add a ImGuiItemFlags_NoNavHighlight item flag. But I suspect most people are going to use lower-level ButtonBehavior() in this situation anyhow.

@ocornut ocornut closed this as completed Oct 16, 2024
ocornut added a commit that referenced this issue Oct 18, 2024
…(), ImGuiNavHighlightFlags to ImGuiNavRenderCursorFlags. (#1074, #2048, #7237, #8059, #1712, #7370, #787)

+ referenced in #8057, #3882, #3411, #2155, #3351, #4722, #1658, #4050.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nav keyboard/gamepad navigation
Projects
None yet
Development

No branches or pull requests

2 participants