Browse Source

v2 Fixes #2810. Pressing Alt key on a Toplevel with only a MenuBar throws System.InvalidOperationException.

BDisp 2 years ago
parent
commit
5a11b88f52

+ 2 - 1
Terminal.Gui/Application.cs

@@ -832,7 +832,8 @@ namespace Terminal.Gui {
 					OverlappedTop.OnAllChildClosed ();
 				} else {
 					SetCurrentOverlappedAsTop ();
-					Current.OnEnter (Current);
+					runState.Toplevel.OnLeave (Current);
+					Current.OnEnter (runState.Toplevel);
 				}
 				Refresh ();
 			}

+ 1 - 1
Terminal.Gui/View/ViewSubViews.cs

@@ -383,7 +383,7 @@ namespace Terminal.Gui {
 						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 ();

+ 1 - 0
Terminal.Gui/Views/Toplevel.cs

@@ -553,6 +553,7 @@ namespace Terminal.Gui {
 		///<inheritdoc/>
 		public override void Add (View view)
 		{
+			CanFocus = true;
 			AddMenuStatusBar (view);
 			base.Add (view);
 		}

+ 0 - 24
Terminal.Gui/Views/Window.cs

@@ -56,29 +56,5 @@ namespace Terminal.Gui {
 			ColorScheme = Colors.Base; // TODO: make this a theme property
 			BorderStyle = DefaultBorderStyle;
 		}
-
-		// TODO: Are these overrides really needed? 
-		/// <inheritdoc/>
-		public override void Add (View view)
-		{
-			base.Add (view);
-			if (view.CanFocus) {
-				CanFocus = true;
-			}
-			AddMenuStatusBar (view);
-		}
-
-		/// <inheritdoc/>
-		public override void Remove (View view)
-		{
-			if (view == null) {
-				return;
-			}
-
-			SetNeedsDisplay ();
-			base.Remove (view);
-			RemoveMenuStatusBar (view);
-
-		}
 	}
 }

+ 46 - 3
UnitTests/View/NavigationTests.cs

@@ -764,11 +764,25 @@ namespace Terminal.Gui.ViewTests {
 			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 @@ namespace Terminal.Gui.ViewTests {
 			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 @@ namespace Terminal.Gui.ViewTests {
 			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);
 		}

+ 115 - 4
UnitTests/Views/ToplevelTests.cs

@@ -962,10 +962,104 @@ namespace Terminal.Gui.ViewsTests {
 			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 @@ namespace Terminal.Gui.ViewsTests {
 
 			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);
+		}
 	}
 }

+ 17 - 0
UnitTests/Views/WindowTests.cs

@@ -193,5 +193,22 @@ namespace Terminal.Gui.ViewsTests {
 			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);
+		}
 	}
 }