From 48f263336bc7651e0b648d9aaacd7828e31b23f8 Mon Sep 17 00:00:00 2001 From: ocornut Date: Mon, 13 Dec 2021 19:46:01 +0100 Subject: [PATCH] Menus: fixed closing a menu inside a popup/modal. Fixed menu inside a popup/modal not inhibiting hovering of items in the popup/modal. (#3496, #4797) Fixed sub-menu items inside a popups from closing the popup (debatable). --- docs/CHANGELOG.txt | 3 +++ imgui.cpp | 2 +- imgui.h | 2 +- imgui_demo.cpp | 21 ++++++++++++++++++--- imgui_widgets.cpp | 42 +++++++++++++++++++++++++++++++++++------- 5 files changed, 58 insertions(+), 12 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index 639a49f01bb6..05fdaeee9a89 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -68,7 +68,10 @@ Other Changes: - Nav: with ImGuiConfigFlags_NavEnableSetMousePos enabled: Fixed absolute mouse position when using Home/End leads to scrolling. Fixed not setting mouse position when a failed move request (e.g. when already at edge) reactivates the navigation highlight. +- Menus: fixed closing a menu inside a popup/modal by clicking on the popup/modal. (#3496, #4797) - Menus: fixed closing a menu by clicking on its menu-bar item when inside a popup. (#3496, #4797) [@xndcn] +- Menus: fixed menu inside a popup/modal not inhibiting hovering of items in the popup/modal. (#3496, #4797) +- Menus: fixed sub-menu items inside a popups from closing the popup. - InputText, Nav: fixed repeated calls to SetKeyboardFocusHere() preventing to use InputText(). (#4682) - Inputtext, Nav: fixed using SetKeyboardFocusHere() on InputTextMultiline(). (#4761) - InputText: made double-click select word, triple-line select line. Word delimitation logic differs diff --git a/imgui.cpp b/imgui.cpp index 39917cdb194b..71f7a4772b74 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -8705,7 +8705,7 @@ void ImGui::CloseCurrentPopup() ImGuiWindow* parent_popup_window = g.OpenPopupStack[popup_idx - 1].Window; bool close_parent = false; if (popup_window && (popup_window->Flags & ImGuiWindowFlags_ChildMenu)) - if (parent_popup_window == NULL || !(parent_popup_window->Flags & ImGuiWindowFlags_Modal)) + if (parent_popup_window && !(parent_popup_window->Flags & ImGuiWindowFlags_MenuBar)) close_parent = true; if (!close_parent) break; diff --git a/imgui.h b/imgui.h index 733752950c00..3a33653f303c 100644 --- a/imgui.h +++ b/imgui.h @@ -64,7 +64,7 @@ Index of this file: // Version // (Integer encoded as XYYZZ for use in #if preprocessor conditionals. Work in progress versions typically starts at XYY99 then bounce up to XYY00, XYY01 etc. when release tagging happens) #define IMGUI_VERSION "1.86 WIP" -#define IMGUI_VERSION_NUM 18520 +#define IMGUI_VERSION_NUM 18521 #define IMGUI_CHECKVERSION() ImGui::DebugCheckVersionAndDataLayout(IMGUI_VERSION, sizeof(ImGuiIO), sizeof(ImGuiStyle), sizeof(ImVec2), sizeof(ImVec4), sizeof(ImDrawVert), sizeof(ImDrawIdx)) #define IMGUI_HAS_TABLE diff --git a/imgui_demo.cpp b/imgui_demo.cpp index 97963ae1b0c8..b517aa2e93ed 100644 --- a/imgui_demo.cpp +++ b/imgui_demo.cpp @@ -3345,11 +3345,26 @@ static void ShowDemoWindowPopups() } // Call the more complete ShowExampleMenuFile which we use in various places of this demo - if (ImGui::Button("File Menu..")) + if (ImGui::Button("With a menu..")) ImGui::OpenPopup("my_file_popup"); - if (ImGui::BeginPopup("my_file_popup")) + if (ImGui::BeginPopup("my_file_popup", ImGuiWindowFlags_MenuBar)) { - ShowExampleMenuFile(); + if (ImGui::BeginMenuBar()) + { + if (ImGui::BeginMenu("File")) + { + ShowExampleMenuFile(); + ImGui::EndMenu(); + } + if (ImGui::BeginMenu("Edit")) + { + ImGui::MenuItem("Dummy"); + ImGui::EndMenu(); + } + ImGui::EndMenuBar(); + } + ImGui::Text("Hello from popup!"); + ImGui::Button("This is a dummy button.."); ImGui::EndPopup(); } diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index 3cca28776623..62151301e077 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -6868,6 +6868,23 @@ void ImGui::EndMainMenuBar() End(); } +static bool IsRootOfOpenMenuSet() +{ + ImGuiContext& g = *GImGui; + ImGuiWindow* window = g.CurrentWindow; + 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. + // 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. + const ImGuiPopupData* upper_popup = &g.OpenPopupStack[g.BeginPopupStack.Size]; + return (upper_popup && /*upper_popup->OpenParentId == window->IDStack.back() &&*/ upper_popup->Window && (upper_popup->Window->Flags & ImGuiWindowFlags_ChildMenu)); +} + bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) { ImGuiWindow* window = GetCurrentWindow(); @@ -6880,8 +6897,9 @@ bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) bool menu_is_open = IsPopupOpen(id, ImGuiPopupFlags_None); // Sub-menus are ChildWindow so that mouse can be hovering across them (otherwise top-most popup menu would steal focus and not allow hovering on parent menu) + // The first menu in a hierarchy isn't so hovering doesn't get accross (otherwise e.g. resizing borders with ImGuiButtonFlags_FlattenChildren would react), but top-most BeginMenu() will bypass that limitation. ImGuiWindowFlags flags = ImGuiWindowFlags_ChildMenu | ImGuiWindowFlags_AlwaysAutoResize | ImGuiWindowFlags_NoMove | ImGuiWindowFlags_NoTitleBar | ImGuiWindowFlags_NoSavedSettings | ImGuiWindowFlags_NoNavFocus; - if (window->Flags & (ImGuiWindowFlags_Popup | ImGuiWindowFlags_ChildMenu)) + if (window->Flags & ImGuiWindowFlags_ChildMenu) flags |= ImGuiWindowFlags_ChildWindow; // If a menu with same the ID was already submitted, we will append to it, matching the behavior of Begin(). @@ -6900,11 +6918,12 @@ bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) g.MenusIdSubmittedThisFrame.push_back(id); ImVec2 label_size = CalcTextSize(label, NULL, true); - bool pressed; - bool menuset_is_open = (window->Flags & ImGuiWindowFlags_MenuBar) && (g.OpenPopupStack.Size > g.BeginPopupStack.Size && g.OpenPopupStack[g.BeginPopupStack.Size].OpenParentId == window->IDStack.back()); + + // Odd hack to allow hovering across menus of a same menu-set (otherwise we wouldn't be able to hover parent without always being a Child window) + const bool menuset_is_open = IsRootOfOpenMenuSet(); ImGuiWindow* backed_nav_window = g.NavWindow; if (menuset_is_open) - g.NavWindow = window; // Odd hack to allow hovering across menus of a same menu-set (otherwise we wouldn't be able to hover parent) + g.NavWindow = window; // The reference position stored in popup_pos will be used by Begin() to find a suitable position for the child menu, // However the final position is going to be different! It is chosen by FindBestWindowPosForPopup(). @@ -6914,6 +6933,7 @@ bool ImGui::BeginMenuEx(const char* label, const char* icon, bool enabled) if (!enabled) BeginDisabled(); const ImGuiMenuColumns* offsets = &window->DC.MenuColumns; + bool pressed; if (window->DC.LayoutType == ImGuiLayoutType_Horizontal) { // Menu inside an horizontal menu bar @@ -7073,13 +7093,19 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut ImVec2 pos = window->DC.CursorPos; ImVec2 label_size = CalcTextSize(label, NULL, true); + const bool menuset_is_open = IsRootOfOpenMenuSet(); + ImGuiWindow* backed_nav_window = g.NavWindow; + if (menuset_is_open) + g.NavWindow = window; + // We've been using the equivalent of ImGuiSelectableFlags_SetNavIdOnHover on all Selectable() since early Nav system days (commit 43ee5d73), // but I am unsure whether this should be kept at all. For now moved it to be an opt-in feature used by menus only. bool pressed; PushID(label); if (!enabled) BeginDisabled(); - const ImGuiSelectableFlags flags = ImGuiSelectableFlags_SelectOnRelease | ImGuiSelectableFlags_SetNavIdOnHover; + + const ImGuiSelectableFlags selectable_flags = ImGuiSelectableFlags_SelectOnRelease | ImGuiSelectableFlags_SetNavIdOnHover; const ImGuiMenuColumns* offsets = &window->DC.MenuColumns; if (window->DC.LayoutType == ImGuiLayoutType_Horizontal) { @@ -7089,7 +7115,7 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut window->DC.CursorPos.x += IM_FLOOR(style.ItemSpacing.x * 0.5f); ImVec2 text_pos(window->DC.CursorPos.x + offsets->OffsetLabel, window->DC.CursorPos.y + window->DC.CurrLineTextBaseOffset); PushStyleVar(ImGuiStyleVar_ItemSpacing, ImVec2(style.ItemSpacing.x * 2.0f, style.ItemSpacing.y)); - pressed = Selectable("", selected, flags, ImVec2(w, 0.0f)); + pressed = Selectable("", selected, selectable_flags, ImVec2(w, 0.0f)); PopStyleVar(); RenderText(text_pos, label); window->DC.CursorPos.x += IM_FLOOR(style.ItemSpacing.x * (-1.0f + 0.5f)); // -1 spacing to compensate the spacing added when Selectable() did a SameLine(). It would also work to call SameLine() ourselves after the PopStyleVar(). @@ -7104,7 +7130,7 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut float checkmark_w = IM_FLOOR(g.FontSize * 1.20f); float min_w = window->DC.MenuColumns.DeclColumns(icon_w, label_size.x, shortcut_w, checkmark_w); // Feedback for next frame float stretch_w = ImMax(0.0f, GetContentRegionAvail().x - min_w); - pressed = Selectable("", false, flags | ImGuiSelectableFlags_SpanAvailWidth, ImVec2(min_w, 0.0f)); + pressed = Selectable("", false, selectable_flags | ImGuiSelectableFlags_SpanAvailWidth, ImVec2(min_w, 0.0f)); RenderText(pos + ImVec2(offsets->OffsetLabel, 0.0f), label); if (icon_w > 0.0f) RenderText(pos + ImVec2(offsets->OffsetIcon, 0.0f), icon); @@ -7121,6 +7147,8 @@ bool ImGui::MenuItemEx(const char* label, const char* icon, const char* shortcut if (!enabled) EndDisabled(); PopID(); + if (menuset_is_open) + g.NavWindow = backed_nav_window; return pressed; }