From 4111b0ee9bbf088acf4c3ccc87698da64b9a088f Mon Sep 17 00:00:00 2001 From: BDisp Date: Tue, 18 Mar 2025 00:39:41 +0000 Subject: [PATCH 1/2] Fixes #3974. TabView steals keypresses from active ContextMenu --- Terminal.Gui/Views/TabView/TabView.cs | 94 ++++++++++++++++++++++- Tests/UnitTests/Views/ContextMenuTests.cs | 80 +++++++++++++++++++ Tests/UnitTests/Views/TabViewTests.cs | 8 +- 3 files changed, 176 insertions(+), 6 deletions(-) diff --git a/Terminal.Gui/Views/TabView/TabView.cs b/Terminal.Gui/Views/TabView/TabView.cs index 5dc14b8bf6..c127c7aed7 100644 --- a/Terminal.Gui/Views/TabView/TabView.cs +++ b/Terminal.Gui/Views/TabView/TabView.cs @@ -84,6 +84,94 @@ public TabView () } ); + AddCommand ( + Command.Up, + () => + { + if (_style.TabsOnBottom) + { + if (_tabsBar is { HasFocus: true } && _containerView.CanFocus) + { + _containerView.SetFocus (); + + return true; + } + } + else + { + if (_containerView is { HasFocus: true }) + { + var mostFocused = _containerView.MostFocused; + + if (mostFocused is { }) + { + for (int? i = mostFocused.SuperView?.InternalSubViews.IndexOf (mostFocused) - 1; i > -1; i--) + { + var view = mostFocused.SuperView?.InternalSubViews [(int)i]; + + if (view is { CanFocus: true, Enabled: true, Visible: true }) + { + view.SetFocus (); + + return true; + } + } + } + + SelectedTab?.SetFocus (); + + return true; + } + } + + return false; + } + ); + + AddCommand ( + Command.Down, + () => + { + if (_style.TabsOnBottom) + { + if (_containerView is { HasFocus: true }) + { + var mostFocused = _containerView.MostFocused; + + if (mostFocused is { }) + { + for (int? i = mostFocused.SuperView?.InternalSubViews.IndexOf (mostFocused) + 1; i < mostFocused.SuperView?.InternalSubViews.Count; i++) + { + var view = mostFocused.SuperView?.InternalSubViews [(int)i]; + + if (view is { CanFocus: true, Enabled: true, Visible: true }) + { + view.SetFocus (); + + return true; + } + } + } + + SelectedTab?.SetFocus (); + + return true; + } + } + else + { + if (_tabsBar is { HasFocus: true } && _containerView.CanFocus) + { + _containerView.SetFocus (); + + return true; + } + } + + return false; + } + ); + // Default keybindings for this view KeyBindings.Add (Key.CursorLeft, Command.Left); KeyBindings.Add (Key.CursorRight, Command.Right); @@ -91,6 +179,8 @@ public TabView () KeyBindings.Add (Key.End, Command.RightEnd); KeyBindings.Add (Key.PageDown, Command.PageDown); KeyBindings.Add (Key.PageUp, Command.PageUp); + KeyBindings.Add (Key.CursorUp, Command.Up); + KeyBindings.Add (Key.CursorDown, Command.Down); } /// @@ -155,7 +245,7 @@ public Tab? SelectedTab private bool TabCanSetFocus () { - return IsInitialized && SelectedTab is { } && (_selectedTabHasFocus || !_containerView.CanFocus); + return IsInitialized && SelectedTab is { } && (HasFocus || (bool)_containerView?.HasFocus) && (_selectedTabHasFocus || !_containerView.CanFocus); } private void ContainerViewCanFocus (object sender, EventArgs eventArgs) @@ -518,7 +608,7 @@ internal IEnumerable CalculateViewport (Rectangle bounds) { SelectedTab?.SetFocus (); } - else + else if (HasFocus) { SelectedTab?.View?.SetFocus (); } diff --git a/Tests/UnitTests/Views/ContextMenuTests.cs b/Tests/UnitTests/Views/ContextMenuTests.cs index fbb4d4a22a..4fc19d229d 100644 --- a/Tests/UnitTests/Views/ContextMenuTests.cs +++ b/Tests/UnitTests/Views/ContextMenuTests.cs @@ -2135,4 +2135,84 @@ public void Menu_Without_SubMenu_Is_Closed_When_Pressing_Key_Right_Or_Key_Left ( top.Dispose (); } + + [Fact] + [AutoInitShutdown] + public void Menu_Opened_In_SuperView_With_TabView_Has_Precedence_On_Key_Press () + { + var win = new Window + { + Title = "My Window", + X = 0, + Y = 0, + Width = Dim.Fill (), + Height = Dim.Fill () + }; + + // Tab View + var tabView = new TabView + { + X = 1, + Y = 1, + Width = Dim.Fill () - 2, + Height = Dim.Fill () - 2 + }; + tabView.AddTab (new () { DisplayText = "Tab 1" }, true); + tabView.AddTab (new () { DisplayText = "Tab 2" }, false); + win.Add (tabView); + + // Context Menu + var menuItems = new MenuBarItem ( + [ + new ("Item 1", "First item", () => MessageBox.Query ("Action", "Item 1 Clicked", "OK")), + new MenuBarItem ( + "Submenu", + new List + { + new [] + { + new MenuItem ( + "Sub Item 1", + "Submenu item", + () => { MessageBox.Query ("Action", "Sub Item 1 Clicked", "OK"); }) + } + }) + ]); + + var cm = new ContextMenu (); + + win.MouseClick += (s, e) => + { + if (e.Flags.HasFlag (MouseFlags.Button3Clicked)) // Right-click + { + cm.Position = e.Position; + cm.Show (menuItems); + } + }; + Application.Begin (win); + + cm.Show (menuItems); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorDown)); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorUp)); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorDown)); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorRight)); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorLeft)); + Assert.True (cm.MenuBar!.IsMenuOpen); + + Assert.True (Application.RaiseKeyDownEvent (Key.CursorLeft)); + Assert.False (cm.MenuBar!.IsMenuOpen); + Assert.True (tabView.HasFocus); + + win.Dispose (); + } } diff --git a/Tests/UnitTests/Views/TabViewTests.cs b/Tests/UnitTests/Views/TabViewTests.cs index 2860452945..b6fb467ccc 100644 --- a/Tests/UnitTests/Views/TabViewTests.cs +++ b/Tests/UnitTests/Views/TabViewTests.cs @@ -434,9 +434,8 @@ public void ProcessKey_Down_Up_Right_Left_Home_End_PageDown_PageUp_F6 () Assert.Equal (btnSubView, top.MostFocused); Assert.True (Application.RaiseKeyDownEvent (Key.CursorUp)); - // TabRow now has TabGroup which only F6 is allowed - Assert.NotEqual (tab2, top.MostFocused); - Assert.Equal (btn, top.MostFocused); + Assert.Equal (tab2, top.MostFocused); + Assert.NotEqual (btn, top.MostFocused); Assert.True (Application.RaiseKeyDownEvent (Key.CursorUp)); Assert.Equal (btnSubView, top.MostFocused); @@ -459,7 +458,8 @@ public void ProcessKey_Down_Up_Right_Left_Home_End_PageDown_PageUp_F6 () // Press the cursor up key to focus the selected tab which it's the only way to do that Assert.True (Application.RaiseKeyDownEvent (Key.CursorUp)); Assert.Equal (tab2, tv.SelectedTab); - Assert.Equal (btn, top.Focused); + Assert.False (tab2.View.HasFocus); + Assert.Equal (tv, top.Focused); Assert.True (Application.RaiseKeyDownEvent (Key.CursorUp)); Assert.Equal (tv, top.Focused); From 4ac13efd13b46bc8f1499e8a270d50351fbded47 Mon Sep 17 00:00:00 2001 From: BDisp Date: Tue, 18 Mar 2025 10:24:51 +0000 Subject: [PATCH 2/2] Using SubViews and letting the top-level handle the key if the TabView doesn't handle it --- Terminal.Gui/Views/TabView/TabView.cs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Terminal.Gui/Views/TabView/TabView.cs b/Terminal.Gui/Views/TabView/TabView.cs index c127c7aed7..98ce20b214 100644 --- a/Terminal.Gui/Views/TabView/TabView.cs +++ b/Terminal.Gui/Views/TabView/TabView.cs @@ -105,15 +105,14 @@ public TabView () if (mostFocused is { }) { - for (int? i = mostFocused.SuperView?.InternalSubViews.IndexOf (mostFocused) - 1; i > -1; i--) + for (int? i = mostFocused.SuperView?.SubViews.IndexOf (mostFocused) - 1; i > -1; i--) { - var view = mostFocused.SuperView?.InternalSubViews [(int)i]; + var view = mostFocused.SuperView?.SubViews.ElementAt ((int)i); if (view is { CanFocus: true, Enabled: true, Visible: true }) { - view.SetFocus (); - - return true; + // Let toplevel handle it + return false; } } } @@ -140,15 +139,14 @@ public TabView () if (mostFocused is { }) { - for (int? i = mostFocused.SuperView?.InternalSubViews.IndexOf (mostFocused) + 1; i < mostFocused.SuperView?.InternalSubViews.Count; i++) + for (int? i = mostFocused.SuperView?.SubViews.IndexOf (mostFocused) + 1; i < mostFocused.SuperView?.SubViews.Count; i++) { - var view = mostFocused.SuperView?.InternalSubViews [(int)i]; + var view = mostFocused.SuperView?.SubViews.ElementAt ((int)i); if (view is { CanFocus: true, Enabled: true, Visible: true }) { - view.SetFocus (); - - return true; + // Let toplevel handle it + return false; } } }