Skip to content

Commit

Permalink
Menus: fixed closing a menu inside a popup/modal. Fixed menu inside a…
Browse files Browse the repository at this point in the history
… popup/modal not inhibiting hovering of items in the popup/modal. (ocornut#3496, ocornut#4797)

Fixed sub-menu items inside a popups from closing the popup (debatable).
  • Loading branch information
ocornut committed Dec 14, 2021
1 parent a528398 commit 48f2633
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 12 deletions.
3 changes: 3 additions & 0 deletions docs/CHANGELOG.txt
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion imgui.cpp
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion imgui.h
Expand Up @@ -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

Expand Down
21 changes: 18 additions & 3 deletions imgui_demo.cpp
Expand Up @@ -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();
}

Expand Down
42 changes: 35 additions & 7 deletions imgui_widgets.cpp
Expand Up @@ -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();
Expand All @@ -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().
Expand All @@ -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().
Expand All @@ -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
Expand Down Expand Up @@ -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)
{
Expand All @@ -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().
Expand All @@ -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);
Expand All @@ -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;
}
Expand Down

0 comments on commit 48f2633

Please sign in to comment.