Imgui: Popup menu right click behavior

[ghost]

Hi

I try to open a context menu, the first time I "right click" it open, so it is ok.

The next time I right click at another position, the current menu stay open and the menu does not re-open at the new position. How to fix that ? It should be the default behaviour (Just try with a right click on your browser by example).

    if (ImGui::BeginPopupContextWindow())
    {
        if (ImGui::MenuItem("New camera"))
        {
        }

        ImGui::EndPopup();
    }

[thennequin]

Until better solution

#include "imgui_internal.h"
if (ImGui::GetIO().MouseClicked[1])
{
    ImGuiState& g = *GImGui;
    if (g.OpenedPopupStack.size() > 0)
        g.OpenedPopupStack.pop_back();

    ImGui::OpenPopup("test");
}

[ghost]

Thanks a lot ;-)

[ocornut]

I'll keep this open as I would like to fix that in the core library eventually!

[ocornut]

I have a little problem here, I can make subsequent calls to OpenPopup() recreate the popup but that would break cases of people calling OpenPopup every frame.. Of course that later pattern already had issues and wasn't recommended so breaking it more severely may be desirable? I'm just not sure of the side effects in real codebases.

Plan B is to add an additional parameter to OpenPopup bool reopen_existing = false and the BeginPopupContext functions would enable this option.

Looking into this.

[ghost]

My 2 cents is that you current version logic is good, so an additional parameter for "special cases" like popup menu looks to be the less destructive and more flexible one. If needed, later you can just switch the default value of this flag !

[ocornut]

_EDIT_
There's actually another problem, functions like IsItemHovered() which are used in e.g. BeginPopupContextItem always return false when hovering windows behind the popup, aka most certainly the item that opened the popup in the first place.

So while BeginPopupContextItem() could query the Hovered information in another way it would introduce some inconsistency with the api - e.g. the underlying button wouldn't appear hovered but right-click still functions? I'll need to think about different use cases a little more.

[ocornut]

I made it work for BeginPopupContextWindow and BeginPopupContextVoid but not for BeginPopupContextItem, that third one would probably need an extra parameter. Not happy about adding an extra boolean at the end considering BeginPopupContextWindow has a boolean at the first parameter. Boolean parameters in API are truly a mistake :/

I think it's mostly useful for those two cases and a little less so for BeginPopupContextItem() but I can definitively see cases what that would also be useful.

[ocornut]

Added the following comment to BeginPopupContextItem()

// This is a helper to handle the most simple case of associating one named popup to one given widget.
// 1. If you have many possible popups (for different "instances" of a same widget, or for wholly different widgets), you may be better off handling 
//    this yourself so you can store data relative to the widget that opened the popup instead of choosing different popup identifiers.
// 2. If you want right-clicking on the same item to reopen the popup at new location, use the same code replacing IsItemHovered() with IsItemHoveredRect().
//    Because: hovering an item in a window below the popup won't normally trigger is hovering behavior/coloring. The pattern of ignoring the fact that 
//    the item isn't interactable (because it is blocked by the active popup) may useful in some situation when e.g. large canvas as one item, content of menu
//    driven by click position.
bool ImGui::BeginPopupContextItem(const char* str_id, int mouse_button)
{
    if (ImGui::IsItemHovered() && ImGui::IsMouseClicked(mouse_button))
        ImGui::OpenPopupEx(str_id, false);
    return ImGui::BeginPopup(str_id);
}

I think it is reasonable to say that the function is an optional helper and more intricate behaviour can be assembled by doing it manually. I added declared OpenPopupEx() in imgui_internal.h now because I don't want to take the decision right away of whether adding the additional boolean flag is a future proof solutions (boolean rarely are). I could either decide that OpenPopupEx() become public API (as is, or with modifications) either add a new entry point aside from BeginPopupContextItem to specify the different behaviour. I think we could decide on that later and with the latest commit the situation is at least unblockable.

Let me know!

[ocornut]

// 2. If you want right-clicking on the same item to reopen the popup at new location, use the same code replacing IsItemHovered() with IsItemHoveredRect().

should be:

// 2. If you want right-clicking on the same item to reopen the popup at new location, use the same code replacing IsItemHovered() with IsItemHoveredRect()
//    and passing true to the OpenPopupEx().

Will turn that into a public thing later.

[ocornut]

FYI @ghost @thennequin the current version has this supported by BeginPopupContextWindow().

[ocornut]

Dear journal,

Interestingly, today I noticed that this old commit in 63e4677b8148d67336e9807334a4401a93bf9d82, effectively changing IsMouseClicked(1) to IsMouseReleased(1) in the BeginPopupContextxxx functions, as a side-effect made the ImGuiHoveredFlags_AllowWhenBlockedByPopup flag unnecessary in most situation when dealing with context menus, since right-mouse-down will close popups, and right-mouse-up will reopen a new one.

It's quite convenient as I was trying to solve a small puzzle when custom context menu handling in Tables, and the fact that ImGuiHoveredFlags_AllowWhenBlockedByPopup isn't actually required for context menus really simplified the problem for me.