From bda2cde68eed30c5c1992709c909ad220d226ba4 Mon Sep 17 00:00:00 2001 From: omar Date: Sun, 28 Apr 2019 21:52:12 +0200 Subject: [PATCH] Popups: Closing a popup restores the focused/nav window in place at the time of the popup opening, instead of restoring the window that was in the window stack at the time of the OpenPopup call. (#2517) Among other things, this allows opening a popup while no window are focused, and pressing Escape to clear the focus again. --- docs/CHANGELOG.txt | 4 ++++ docs/TODO.txt | 1 + imgui.cpp | 44 +++++++++++++++++++++++++++----------------- imgui_internal.h | 6 +++--- imgui_widgets.cpp | 5 +++-- 5 files changed, 38 insertions(+), 22 deletions(-) diff --git a/docs/CHANGELOG.txt b/docs/CHANGELOG.txt index c33d9204e..e6d387b77 100644 --- a/docs/CHANGELOG.txt +++ b/docs/CHANGELOG.txt @@ -59,6 +59,10 @@ Other Changes: highlight from parent menu items earlier than necessary while approaching the child menu. - Window: Close button is horizontally aligned with style.FramePadding.x. - Window: Fixed contents region being off by WindowBorderSize amount on the right when scrollbar is active. +- Popups: Closing a popup restores the focused/nav window in place at the time of the popup opening, + instead of restoring the window that was in the window stack at the time of the OpenPopup call. (#2517) + Among other things, this allows opening a popup while no window are focused, and pressing Escape to + clear the focus again. - GetMouseDragDelta(): also returns the delta on the mouse button released frame. (#2419) - GetMouseDragDelta(): verify that mouse positions are valid otherwise returns zero. - Inputs: Also add support for horizontal scroll with Shift+Mouse Wheel. (#2424, #1463) [@LucaRood] diff --git a/docs/TODO.txt b/docs/TODO.txt index 9e64b1630..f900e7e13 100644 --- a/docs/TODO.txt +++ b/docs/TODO.txt @@ -291,6 +291,7 @@ It's mostly a bunch of personal notes, probably incomplete. Feel free to query i - nav/menus: allow pressing Menu to leave a sub-menu. - nav/menus: a way to access the main menu bar with Alt? (currently needs CTRL+TAB) - nav/menus: when using the main menu bar, even though we restore focus after, the underlying window loses its title bar highlight during menu manipulation. could we prevent it? + - nav/menus: main menu bar currently cannot restore a NULL focus. Could save NavWindow at the time of being focused, similarly to what popup do? - nav: simulate right-click or context activation? (SHIFT+F10) - nav: tabs should go through most/all widgets (in submission order?). - nav: when CTRL-Tab/windowing is active, the HoveredWindow detection doesn't take account of the window display re-ordering. diff --git a/imgui.cpp b/imgui.cpp index 61d5a2cfd..e465fdf10 100644 --- a/imgui.cpp +++ b/imgui.cpp @@ -3603,7 +3603,7 @@ void ImGui::NewFrame() // Closing the focused window restore focus to the first active root window in descending z-order if (g.NavWindow && !g.NavWindow->WasActive) - FocusTopMostWindowIgnoringOne(NULL); + FocusTopMostWindowUnderOne(NULL, NULL); // No window should be open at the beginning of the frame. // But in order to allow the user to call NewFrame() multiple times without calling Render(), we are doing an explicit clear. @@ -5726,10 +5726,18 @@ void ImGui::FocusWindow(ImGuiWindow* window) BringWindowToDisplayFront(window); } -void ImGui::FocusTopMostWindowIgnoringOne(ImGuiWindow* ignore_window) +void ImGui::FocusTopMostWindowUnderOne(ImGuiWindow* under_this_window, ImGuiWindow* ignore_window) { ImGuiContext& g = *GImGui; - for (int i = g.WindowsFocusOrder.Size - 1; i >= 0; i--) + + int start_idx = g.WindowsFocusOrder.Size - 1; + if (under_this_window != NULL) + { + int under_this_window_idx = FindWindowFocusIndex(under_this_window); + if (under_this_window_idx != -1) + start_idx = under_this_window_idx - 1; + } + for (int i = start_idx; i >= 0; i--) { // We may later decide to test for different NoXXXInputs based on the active navigation input (mouse vs nav) but that may feel more confusing to the user. ImGuiWindow* window = g.WindowsFocusOrder[i]; @@ -5741,6 +5749,7 @@ void ImGui::FocusTopMostWindowIgnoringOne(ImGuiWindow* ignore_window) return; } } + FocusWindow(NULL); } void ImGui::SetNextItemWidth(float item_width) @@ -6949,7 +6958,7 @@ void ImGui::OpenPopupEx(ImGuiID id) ImGuiPopupData popup_ref; // Tagged as new ref as Window will be set back to NULL if we write this into OpenPopupStack. popup_ref.PopupId = id; popup_ref.Window = NULL; - popup_ref.ParentWindow = parent_window; + popup_ref.SourceWindow = g.NavWindow; popup_ref.OpenFrameCount = g.FrameCount; popup_ref.OpenParentId = parent_window->IDStack.back(); popup_ref.OpenPopupPos = NavCalcPreferredRefPos(); @@ -7035,24 +7044,25 @@ void ImGui::ClosePopupsOverWindow(ImGuiWindow* ref_window) void ImGui::ClosePopupToLevel(int remaining, bool apply_focus_to_window_under) { - IM_ASSERT(remaining >= 0); ImGuiContext& g = *GImGui; - ImGuiWindow* focus_window = (remaining > 0) ? g.OpenPopupStack[remaining-1].Window : g.OpenPopupStack[0].ParentWindow; + IM_ASSERT(remaining >= 0 && remaining < g.OpenPopupStack.Size); + ImGuiWindow* focus_window = g.OpenPopupStack[remaining].SourceWindow; + ImGuiWindow* popup_window = g.OpenPopupStack[remaining].Window; g.OpenPopupStack.resize(remaining); - // FIXME: This code is faulty and we may want to eventually to replace or remove the 'apply_focus_to_window_under=true' path completely. - // Instead of using g.OpenPopupStack[remaining-1].Window etc. we should find the highest root window that is behind the popups we are closing. - // The current code will set focus to the parent of the popup window which is incorrect. - // It rarely manifested until now because UpdateMouseMovingWindowNewFrame() would call FocusWindow() again on the clicked window, - // leading to a chain of focusing A (clicked window) then B (parent window of the popup) then A again. - // However if the clicked window has the _NoMove flag set we would be left with B focused. - // For now, we have disabled this path when called from ClosePopupsOverWindow() because the users of ClosePopupsOverWindow() don't need to alter focus anyway, - // but we should inspect and fix this properly. if (apply_focus_to_window_under) { - if (g.NavLayer == 0) - focus_window = NavRestoreLastChildNavWindow(focus_window); - FocusWindow(focus_window); + if (focus_window && !focus_window->WasActive && popup_window) + { + // Fallback + FocusTopMostWindowUnderOne(popup_window, NULL); + } + else + { + if (g.NavLayer == 0 && focus_window) + focus_window = NavRestoreLastChildNavWindow(focus_window); + FocusWindow(focus_window); + } } } diff --git a/imgui_internal.h b/imgui_internal.h index 075d18aac..415dfaea5 100644 --- a/imgui_internal.h +++ b/imgui_internal.h @@ -652,13 +652,13 @@ struct ImGuiPopupData { ImGuiID PopupId; // Set on OpenPopup() ImGuiWindow* Window; // Resolved on BeginPopup() - may stay unresolved if user never calls OpenPopup() - ImGuiWindow* ParentWindow; // Set on OpenPopup() + ImGuiWindow* SourceWindow; // Set on OpenPopup() copy of NavWindow at the time of opening the popup 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() { PopupId = 0; Window = ParentWindow = NULL; OpenFrameCount = -1; OpenParentId = 0; } + ImGuiPopupData() { PopupId = 0; Window = SourceWindow = NULL; OpenFrameCount = -1; OpenParentId = 0; } }; struct ImGuiColumnData @@ -1392,7 +1392,7 @@ namespace ImGui IMGUI_API ImGuiWindow* FindWindowByID(ImGuiID id); IMGUI_API ImGuiWindow* FindWindowByName(const char* name); IMGUI_API void FocusWindow(ImGuiWindow* window); - IMGUI_API void FocusTopMostWindowIgnoringOne(ImGuiWindow* ignore_window); + IMGUI_API void FocusTopMostWindowUnderOne(ImGuiWindow* under_this_window, ImGuiWindow* ignore_window); IMGUI_API void BringWindowToFocusFront(ImGuiWindow* window); IMGUI_API void BringWindowToDisplayFront(ImGuiWindow* window); IMGUI_API void BringWindowToDisplayBack(ImGuiWindow* window); diff --git a/imgui_widgets.cpp b/imgui_widgets.cpp index c707e0ee6..f3e9ecd02 100644 --- a/imgui_widgets.cpp +++ b/imgui_widgets.cpp @@ -5818,9 +5818,10 @@ void ImGui::EndMainMenuBar() EndMenuBar(); // When the user has left the menu layer (typically: closed menus through activation of an item), we restore focus to the previous window + // FIXME: With this strategy we won't be able to restore a NULL focus. ImGuiContext& g = *GImGui; if (g.CurrentWindow == g.NavWindow && g.NavLayer == 0) - FocusTopMostWindowIgnoringOne(g.NavWindow); + FocusTopMostWindowUnderOne(g.NavWindow, NULL); End(); } @@ -5951,7 +5952,7 @@ bool ImGui::BeginMenu(const char* label, bool enabled) { // Implement http://bjk5.com/post/44698559168/breaking-down-amazons-mega-dropdown to avoid using timers, so menus feels more reactive. bool moving_within_opened_triangle = false; - if (g.HoveredWindow == window && g.OpenPopupStack.Size > g.BeginPopupStack.Size && g.OpenPopupStack[g.BeginPopupStack.Size].ParentWindow == window && !(window->Flags & ImGuiWindowFlags_MenuBar)) + if (g.HoveredWindow == window && g.OpenPopupStack.Size > g.BeginPopupStack.Size && g.OpenPopupStack[g.BeginPopupStack.Size].SourceWindow == window && !(window->Flags & ImGuiWindowFlags_MenuBar)) { if (ImGuiWindow* next_window = g.OpenPopupStack[g.BeginPopupStack.Size].Window) {