Skip to content

Commit

Permalink
Menus: Separate menu sets by nav layer. (#3496, #4797) + Demo: Remove…
Browse files Browse the repository at this point in the history
… incorrect and useless suggestion to use PushID().

Fixes a common case where opening menu in one nav layer and hovering a menu in another nav layer would open that menu without a click.
  • Loading branch information
rokups authored and ocornut committed Jun 8, 2022
1 parent a35e876 commit 0b1bcfc
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 11 deletions.
2 changes: 2 additions & 0 deletions docs/CHANGELOG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ Other Changes:
- Menus: Adjusted BeginMenu() closing logic so hovering void or non-MenuItem() in parent window
always lead to menu closure. Fixes using items that are not MenuItem() or BeginItem() at the root
level of a popup with a child menu opened.
- Menus: Menus emitted from the main/scrolling layer are not part of the same menuset as menus emitted
from the menu-bar, avoiding accidental hovering from one to the other. (#3496, #4797) [@rokups]
- Stack Tool: Added option to copy item path to clipboard. (#4631)
- Settings: Fixed out-of-bounds read when .ini file on disk is empty. (#5351) [@quantum5]
- DrawList: Fixed PathArcTo() emitting terminating vertices too close to arc vertices. (#4993) [@thedmd]
Expand Down
1 change: 1 addition & 0 deletions imgui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6071,6 +6071,7 @@ bool ImGui::Begin(const char* name, bool* p_open, ImGuiWindowFlags flags)
{
ImGuiPopupData& popup_ref = g.OpenPopupStack[g.BeginPopupStack.Size];
popup_ref.Window = window;
popup_ref.ParentNavLayer = parent_window_in_stack->DC.NavLayerCurrent;
g.BeginPopupStack.push_back(popup_ref);
window->PopupId = popup_ref.PopupId;
}
Expand Down
7 changes: 0 additions & 7 deletions imgui_demo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3546,19 +3546,12 @@ static void ShowDemoWindowPopups()
ImGui::TextWrapped("Below we are testing adding menu items to a regular window. It's rather unusual but should work!");
ImGui::Separator();

// Note: As a quirk in this very specific example, we want to differentiate the parent of this menu from the
// parent of the various popup menus above. To do so we are encloding the items in a PushID()/PopID() block
// to make them two different menusets. If we don't, opening any popup above and hovering our menu here would
// open it. This is because once a menu is active, we allow to switch to a sibling menu by just hovering on it,
// which is the desired behavior for regular menus.
ImGui::PushID("foo");
ImGui::MenuItem("Menu item", "CTRL+M");
if (ImGui::BeginMenu("Menu inside a regular window"))
{
ShowExampleMenuFile();
ImGui::EndMenu();
}
ImGui::PopID();
ImGui::Separator();
ImGui::TreePop();
}
Expand Down
3 changes: 2 additions & 1 deletion imgui_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1038,12 +1038,13 @@ struct ImGuiPopupData
ImGuiID PopupId; // Set on OpenPopup()
ImGuiWindow* Window; // Resolved on BeginPopup() - may stay unresolved if user never calls OpenPopup()
ImGuiWindow* SourceWindow; // Set on OpenPopup() copy of NavWindow at the time of opening the popup
int ParentNavLayer; // Resolved on BeginPopup(). Actually a ImGuiNavLayer type (declared down below), initialized to -1 which is not part of an enum, but serves well-enough as "not any of layers" value
int OpenFrameCount; // Set on OpenPopup()
ImGuiID OpenParentId; // Set on OpenPopup(), we need this to differentiate multiple menu sets from each others (e.g. inside menu bar vs loose menu items)
ImVec2 OpenPopupPos; // Set on OpenPopup(), preferred popup position (typically == OpenMousePos when using mouse)
ImVec2 OpenMousePos; // Set on OpenPopup(), copy of mouse position at the time of opening popup

ImGuiPopupData() { memset(this, 0, sizeof(*this)); OpenFrameCount = -1; }
ImGuiPopupData() { memset(this, 0, sizeof(*this)); ParentNavLayer = OpenFrameCount = -1; }
};

enum ImGuiNextWindowDataFlags_
Expand Down
11 changes: 8 additions & 3 deletions imgui_widgets.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6939,14 +6939,19 @@ static bool IsRootOfOpenMenuSet()
if ((g.OpenPopupStack.Size <= g.BeginPopupStack.Size) || (window->Flags & ImGuiWindowFlags_ChildMenu))
return false;

// Initially we used 'OpenParentId' to differentiate multiple menu sets from each others (e.g. inside menu bar vs loose menu items) based on parent ID.
// Initially we used 'upper_popup->OpenParentId == window->IDStack.back()' to differentiate multiple menu sets from each others
// (e.g. inside menu bar vs loose menu items) based on parent ID.
// This would however prevent the use of e.g. PuhsID() user code submitting menus.
// Previously this worked between popup and a first child menu because the first child menu always had the _ChildWindow flag,
// making hovering on parent popup possible while first child menu was focused - but this was generally a bug with other side effects.
// Instead we don't treat Popup specifically (in order to consistently support menu features in them), maybe the first child menu of a Popup
// doesn't have the _ChildWindow flag, and we rely on this IsRootOfOpenMenuSet() check to allow hovering between root window/popup and first chilld menu.
// doesn't have the _ChildWindow flag, and we rely on this IsRootOfOpenMenuSet() check to allow hovering between root window/popup and first child menu.
// In the end, lack of ID check made it so we could no longer differentiate between separate menu sets. To compensate for that, we at least check parent window nav layer.
// This fixes the most common case of menu opening on hover when moving between window content and menu bar. Multiple different menu sets in same nav layer would still
// open on hover, but that should be a lesser problem, because if such menus are close in proximity in window content then it won't feel weird and if they are far apart
// it likely won't be a problem anyone runs into.
const ImGuiPopupData* upper_popup = &g.OpenPopupStack[g.BeginPopupStack.Size];
return (/*upper_popup->OpenParentId == window->IDStack.back() &&*/ upper_popup->Window && (upper_popup->Window->Flags & ImGuiWindowFlags_ChildMenu));
return (window->DC.NavLayerCurrent == upper_popup->ParentNavLayer && upper_popup->Window && (upper_popup->Window->Flags & ImGuiWindowFlags_ChildMenu));
}

bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled)
Expand Down

0 comments on commit 0b1bcfc

Please sign in to comment.