From 5a11b88f52626af994713baed4f9c5da5885e3e4 Mon Sep 17 00:00:00 2001 From: BDisp Date: Tue, 15 Aug 2023 18:31:54 +0100 Subject: [PATCH] v2 Fixes #2810. Pressing Alt key on a Toplevel with only a MenuBar throws System.InvalidOperationException. --- Terminal.Gui/Application.cs | 3 +- Terminal.Gui/View/ViewSubViews.cs | 2 +- Terminal.Gui/Views/Toplevel.cs | 1 + Terminal.Gui/Views/Window.cs | 24 ------ UnitTests/View/NavigationTests.cs | 49 +++++++++++- UnitTests/Views/ToplevelTests.cs | 119 +++++++++++++++++++++++++++++- UnitTests/Views/WindowTests.cs | 17 +++++ 7 files changed, 182 insertions(+), 33 deletions(-) diff --git a/Terminal.Gui/Application.cs b/Terminal.Gui/Application.cs index b571894a83..3f4968ecf1 100644 --- a/Terminal.Gui/Application.cs +++ b/Terminal.Gui/Application.cs @@ -832,7 +832,8 @@ public static void End (RunState runState) OverlappedTop.OnAllChildClosed (); } else { SetCurrentOverlappedAsTop (); - Current.OnEnter (Current); + runState.Toplevel.OnLeave (Current); + Current.OnEnter (runState.Toplevel); } Refresh (); } diff --git a/Terminal.Gui/View/ViewSubViews.cs b/Terminal.Gui/View/ViewSubViews.cs index 1657e09ed0..a04d60b049 100644 --- a/Terminal.Gui/View/ViewSubViews.cs +++ b/Terminal.Gui/View/ViewSubViews.cs @@ -383,7 +383,7 @@ public override bool CanFocus { SuperView?.EnsureFocus (); if (SuperView != null && SuperView.Focused == null) { SuperView.FocusNext (); - if (SuperView.Focused == null) { + if (SuperView.Focused == null && Application.Current != null) { Application.Current.FocusNext (); } Application.BringOverlappedTopToFront (); diff --git a/Terminal.Gui/Views/Toplevel.cs b/Terminal.Gui/Views/Toplevel.cs index 159774984d..7799615239 100644 --- a/Terminal.Gui/Views/Toplevel.cs +++ b/Terminal.Gui/Views/Toplevel.cs @@ -553,6 +553,7 @@ void FocusNearestView (IEnumerable views, Direction direction) /// public override void Add (View view) { + CanFocus = true; AddMenuStatusBar (view); base.Add (view); } diff --git a/Terminal.Gui/Views/Window.cs b/Terminal.Gui/Views/Window.cs index 0358c12a81..8d4c3a2cd3 100644 --- a/Terminal.Gui/Views/Window.cs +++ b/Terminal.Gui/Views/Window.cs @@ -56,29 +56,5 @@ void SetInitialProperties () ColorScheme = Colors.Base; // TODO: make this a theme property BorderStyle = DefaultBorderStyle; } - - // TODO: Are these overrides really needed? - /// - public override void Add (View view) - { - base.Add (view); - if (view.CanFocus) { - CanFocus = true; - } - AddMenuStatusBar (view); - } - - /// - public override void Remove (View view) - { - if (view == null) { - return; - } - - SetNeedsDisplay (); - base.Remove (view); - RemoveMenuStatusBar (view); - - } } } diff --git a/UnitTests/View/NavigationTests.cs b/UnitTests/View/NavigationTests.cs index d7f7ba8098..f632fccd23 100644 --- a/UnitTests/View/NavigationTests.cs +++ b/UnitTests/View/NavigationTests.cs @@ -764,11 +764,25 @@ public void CanFocus_Sets_To_False_On_Single_View_Focus_View_On_Another_Toplevel Assert.True (view1.CanFocus); Assert.True (view1.HasFocus); Assert.True (view2.CanFocus); - Assert.False (view2.HasFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus + Assert.True (view2.CanFocus); + Assert.True (view2.HasFocus); + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.True (view1.HasFocus); + Assert.True (view2.CanFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus view1.CanFocus = false; Assert.False (view1.CanFocus); Assert.False (view1.HasFocus); + Assert.True (view2.CanFocus); + Assert.True (view2.HasFocus); Assert.Equal (win2, Application.Current.Focused); Assert.Equal (view2, Application.Current.MostFocused); } @@ -790,11 +804,26 @@ public void CanFocus_Sets_To_False_With_Two_Views_Focus_Another_View_On_The_Same Assert.True (view1.CanFocus); Assert.True (view1.HasFocus); Assert.True (view2.CanFocus); - Assert.False (view2.HasFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ()))); + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus + Assert.True (view2.CanFocus); + Assert.True (view2.HasFocus); + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.True (view1.HasFocus); + Assert.True (view2.CanFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus view1.CanFocus = false; Assert.False (view1.CanFocus); Assert.False (view1.HasFocus); + Assert.True (view2.CanFocus); + Assert.False (view2.HasFocus); Assert.Equal (win1, Application.Current.Focused); Assert.Equal (view12, Application.Current.MostFocused); } @@ -815,13 +844,27 @@ public void CanFocus_Sets_To_False_On_Toplevel_Focus_View_On_Another_Toplevel () Assert.True (view1.CanFocus); Assert.True (view1.HasFocus); Assert.True (view2.CanFocus); - Assert.False (view2.HasFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.False (view1.HasFocus); // Only one of the most focused toplevels view can have focus + Assert.True (view2.CanFocus); + Assert.True (view2.HasFocus); + + Assert.True (Application.Top.ProcessKey (new KeyEvent (Key.Tab | Key.CtrlMask, new KeyModifiers ()))); + Assert.True (view1.CanFocus); + Assert.True (view1.HasFocus); + Assert.True (view2.CanFocus); + Assert.False (view2.HasFocus); // Only one of the most focused toplevels view can have focus win1.CanFocus = false; Assert.False (view1.CanFocus); Assert.False (view1.HasFocus); Assert.False (win1.CanFocus); Assert.False (win1.HasFocus); + Assert.True (view2.CanFocus); + Assert.True (view2.HasFocus); Assert.Equal (win2, Application.Current.Focused); Assert.Equal (view2, Application.Current.MostFocused); } diff --git a/UnitTests/Views/ToplevelTests.cs b/UnitTests/Views/ToplevelTests.cs index b33de39750..9252fac358 100644 --- a/UnitTests/Views/ToplevelTests.cs +++ b/UnitTests/Views/ToplevelTests.cs @@ -962,10 +962,104 @@ public void OnEnter_OnLeave_Triggered_On_Application_Begin_End () Application.End (rs); Assert.True (isEnter); - Assert.False (isLeave); - } - - [Fact, AutoInitShutdown] + Assert.False(isLeave); // Leave event cannot be trigger because it v.Enter was performed and v is focused + Assert.True(v.HasFocus); + } + + [Fact, AutoInitShutdown] + public void OnEnter_OnLeave_Triggered_On_Application_Begin_End_With_More_Toplevels() + { + var iterations = 0; + var steps = new int[5]; + var isEnterTop = false; + var isLeaveTop = false; + var vt = new View(); + var top = Application.Top; + var diag = new Dialog(); + + vt.Enter += (s, e) => { + iterations++; + isEnterTop = true; + if (iterations == 1) + { + steps[0] = iterations; + Assert.Null(e.View); + } + else + { + steps[4] = iterations; + Assert.Equal(diag, e.View); + } + }; + vt.Leave += (s, e) => { + iterations++; + steps[1] = iterations; + isLeaveTop = true; + Assert.Equal(diag, e.View); + }; + top.Add(vt); + + Assert.False(vt.CanFocus); + var exception = Record.Exception(() => top.OnEnter(top)); + Assert.Null(exception); + exception = Record.Exception(() => top.OnLeave(top)); + Assert.Null(exception); + + vt.CanFocus = true; + Application.Begin(top); + + Assert.True(isEnterTop); + Assert.False(isLeaveTop); + + isEnterTop = false; + var isEnterDiag = false; + var isLeaveDiag = false; + var vd = new View(); + vd.Enter += (s, e) => { + iterations++; + steps[2] = iterations; + isEnterDiag = true; + Assert.Null(e.View); + }; + vd.Leave += (s, e) => { + iterations++; + steps[3] = iterations; + isLeaveDiag = true; + Assert.Equal(top, e.View); + }; + diag.Add(vd); + + Assert.False(vd.CanFocus); + exception = Record.Exception(() => diag.OnEnter(diag)); + Assert.Null(exception); + exception = Record.Exception(() => diag.OnLeave(diag)); + Assert.Null(exception); + + vd.CanFocus = true; + var rs = Application.Begin(diag); + + Assert.True(isEnterDiag); + Assert.False(isLeaveDiag); + Assert.False(isEnterTop); + Assert.True(isLeaveTop); + + isEnterDiag = false; + isLeaveTop = false; + Application.End(rs); + + Assert.False(isEnterDiag); + Assert.True(isLeaveDiag); + Assert.True(isEnterTop); + Assert.False(isLeaveTop); // Leave event cannot be trigger because it v.Enter was performed and v is focused + Assert.True(vt.HasFocus); + Assert.Equal(1, steps[0]); + Assert.Equal(2, steps[1]); + Assert.Equal(3, steps[2]); + Assert.Equal(4, steps[3]); + Assert.Equal(5, steps[^1]); + } + + [Fact, AutoInitShutdown] public void PositionCursor_SetCursorVisibility_To_Invisible_If_Focused_Is_Null () { var tf = new TextField ("test") { Width = 5 }; @@ -1494,5 +1588,22 @@ void Current_DrawContentComplete (object sender, DrawEventArgs e) Application.End (rs); } + + [Fact, AutoInitShutdown] + public void Activating_MenuBar_By_Alt_Key_Does_Not_Throw () + { + var menu = new MenuBar (new MenuBarItem [] { + new MenuBarItem ("Child", new MenuItem [] { + new MenuItem ("_Create Child", "", null) + }) + }); + var topChild = new Toplevel (); + topChild.Add (menu); + Application.Top.Add (topChild); + Application.Begin (Application.Top); + + var exception = Record.Exception (() => topChild.ProcessHotKey (new KeyEvent (Key.AltMask, new KeyModifiers { Alt = true }))); + Assert.Null (exception); + } } } \ No newline at end of file diff --git a/UnitTests/Views/WindowTests.cs b/UnitTests/Views/WindowTests.cs index 242e34a678..955be94ffd 100644 --- a/UnitTests/Views/WindowTests.cs +++ b/UnitTests/Views/WindowTests.cs @@ -193,5 +193,22 @@ public void OnCanFocusChanged_Only_Must_ContentView_Forces_SetFocus_After_IsInit Assert.False (win2.HasFocus); Assert.False (view2.HasFocus); } + + [Fact, AutoInitShutdown] + public void Activating_MenuBar_By_Alt_Key_Does_Not_Throw () + { + var menu = new MenuBar (new MenuBarItem [] { + new MenuBarItem ("Child", new MenuItem [] { + new MenuItem ("_Create Child", "", null) + }) + }); + var win = new Window (); + win.Add (menu); + Application.Top.Add (win); + Application.Begin (Application.Top); + + var exception = Record.Exception (() => win.ProcessHotKey (new KeyEvent (Key.AltMask, new KeyModifiers { Alt = true }))); + Assert.Null (exception); + } } }