Fixes #1973. Avoid positioning Submenus off screen. (#1975)

* Fixes #1973. Avoid positioning Submenus off screen.

* Firstly avoids negative positions on the sub-menus and then avoids the bottom not exceeding the console height, as possible.

Terminal.Gui/Views/Menu.cs

@@ -384,17 +384,26 @@ namespace Terminal.Gui {
 		internal int current;
 		internal View previousSubFocused;
 
-		internal static Rect MakeFrame (int x, int y, MenuItem [] items)
+		internal static Rect MakeFrame (int x, int y, MenuItem [] items, Menu parent = null)
 		{
 			if (items == null || items.Length == 0) {
 				return new Rect ();
 			}
-			int maxW = items.Max (z => z?.Width) ?? 0;
-
-			return new Rect (x, y, maxW + 2, items.Length + 2);
+			int minX = x;
+			int minY = y;
+			int maxW = (items.Max (z => z?.Width) ?? 0) + 2;
+			int maxH = items.Length + 2;
+			if (parent != null && x + maxW > Driver.Cols) {
+				minX = Math.Max (parent.Frame.Right - parent.Frame.Width - maxW, 0);
+			}
+			if (y + maxH > Driver.Rows) {
+				minY = Math.Max (Driver.Rows - maxH, 0);
+			}
+			return new Rect (minX, minY, maxW, maxH);
 		}
 
-		public Menu (MenuBar host, int x, int y, MenuBarItem barItems) : base (MakeFrame (x, y, barItems.Children))
+		public Menu (MenuBar host, int x, int y, MenuBarItem barItems, Menu parent = null)
+			: base (MakeFrame (x, y, barItems.Children, parent))
 		{
 			this.barItems = barItems;
 			this.host = host;
@@ -1232,7 +1241,7 @@ namespace Terminal.Gui {
 				} else {
 					var last = openSubMenu.Count > 0 ? openSubMenu.Last () : openMenu;
 					if (!UseSubMenusSingleFrame) {
-						openCurrentMenu = new Menu (this, last.Frame.Left + last.Frame.Width, last.Frame.Top + 1 + last.current, subMenu);
+						openCurrentMenu = new Menu (this, last.Frame.Left + last.Frame.Width, last.Frame.Top + 1 + last.current, subMenu, last);
 					} else {
 						var first = openSubMenu.Count > 0 ? openSubMenu.First () : openMenu;
 						var mbi = new MenuItem [2 + subMenu.Children.Length];

UnitTests/ContextMenuTests.cs

@@ -423,7 +423,7 @@ namespace Terminal.Gui.Core {
 			cm.Show ();
 			Assert.Equal (new Point (0, 0), cm.Position);
 			Application.Begin (Application.Top);
-			((FakeDriver)Application.Driver).SetBufferSize (80, 4);
+			((FakeDriver)Application.Driver).SetBufferSize (80, 3);
 
 			var expected = @"
 ┌──────┐
@@ -432,7 +432,7 @@ namespace Terminal.Gui.Core {
 ";
 
 			var pos = GraphViewTests.AssertDriverContentsWithFrameAre (expected, output);
-			Assert.Equal (new Rect (0, 1, 8, 3), pos);
+			Assert.Equal (new Rect (0, 0, 8, 3), pos);
 
 			cm.Hide ();
 			Assert.Equal (new Point (0, 0), cm.Position);
@@ -648,7 +648,6 @@ namespace Terminal.Gui.Core {
 			Application.Begin (Application.Top);
 			((FakeDriver)Application.Driver).SetBufferSize (44, 17);
 
-
 			Assert.Equal (new Rect (9, 3, 20, 1), tf.Frame);
 			Assert.True (tf.HasFocus);
 
@@ -679,5 +678,215 @@ namespace Terminal.Gui.Core {
 			var pos = GraphViewTests.AssertDriverContentsWithFrameAre (expected, output);
 			Assert.Equal (new Rect (2, 0, 44, 17), pos);
 		}
+
+		[Fact, AutoInitShutdown]
+		public void Menus_And_SubMenus_Always_Try_To_Be_On_Screen ()
+		{
+			var cm = new ContextMenu (-1, -2,
+				new MenuBarItem (new MenuItem [] {
+					new MenuItem ("One", "", null),
+					new MenuItem ("Two", "", null),
+					new MenuItem ("Three", "", null),
+					new MenuBarItem ("Four", new MenuItem [] {
+						new MenuItem ("SubMenu1", "", null),
+						new MenuItem ("SubMenu2", "", null),
+						new MenuItem ("SubMenu3", "", null),
+						new MenuItem ("SubMenu4", "", null),
+						new MenuItem ("SubMenu5", "", null),
+						new MenuItem ("SubMenu6", "", null),
+						new MenuItem ("SubMenu7", "", null)
+					}),
+					new MenuItem ("Five", "", null),
+					new MenuItem ("Six", "", null)
+				})
+			);
+
+			Assert.Equal (new Point (-1, -2), cm.Position);
+
+			cm.Show ();
+			Assert.Equal (new Point (-1, -2), cm.Position);
+			var top = Application.Top;
+			Application.Begin (top);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+┌────────┐
+│ One    │
+│ Two    │
+│ Three  │
+│ Four  ►│
+│ Five   │
+│ Six    │
+└────────┘
+", output);
+
+			Assert.True (top.Subviews [0].MouseEvent (new MouseEvent {
+				X = 0,
+				Y = 4,
+				Flags = MouseFlags.ReportMousePosition,
+				View = top.Subviews [0]
+			}));
+			Application.Refresh ();
+			Assert.Equal (new Point (-1, -2), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+┌────────┐             
+│ One    │             
+│ Two    │             
+│ Three  │             
+│ Four  ►│┌───────────┐
+│ Five   ││ SubMenu1  │
+│ Six    ││ SubMenu2  │
+└────────┘│ SubMenu3  │
+          │ SubMenu4  │
+          │ SubMenu5  │
+          │ SubMenu6  │
+          │ SubMenu7  │
+          └───────────┘
+", output);
+
+			((FakeDriver)Application.Driver).SetBufferSize (40, 20);
+			cm.Position = new Point (41, -2);
+			cm.Show ();
+			Application.Refresh ();
+			Assert.Equal (new Point (41, -2), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                              ┌────────┐
+                              │ One    │
+                              │ Two    │
+                              │ Three  │
+                              │ Four  ►│
+                              │ Five   │
+                              │ Six    │
+                              └────────┘
+", output);
+
+			Assert.True (top.Subviews [0].MouseEvent (new MouseEvent {
+				X = 30,
+				Y = 4,
+				Flags = MouseFlags.ReportMousePosition,
+				View = top.Subviews [0]
+			}));
+			Application.Refresh ();
+			Assert.Equal (new Point (41, -2), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                              ┌────────┐
+                              │ One    │
+                              │ Two    │
+                              │ Three  │
+                 ┌───────────┐│ Four  ►│
+                 │ SubMenu1  ││ Five   │
+                 │ SubMenu2  ││ Six    │
+                 │ SubMenu3  │└────────┘
+                 │ SubMenu4  │          
+                 │ SubMenu5  │          
+                 │ SubMenu6  │          
+                 │ SubMenu7  │          
+                 └───────────┘          
+", output);
+
+			cm.Position = new Point (41, 9);
+			cm.Show ();
+			Application.Refresh ();
+			Assert.Equal (new Point (41, 9), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                              ┌────────┐
+                              │ One    │
+                              │ Two    │
+                              │ Three  │
+                              │ Four  ►│
+                              │ Five   │
+                              │ Six    │
+                              └────────┘
+", output);
+
+			Assert.True (top.Subviews [0].MouseEvent (new MouseEvent {
+				X = 30,
+				Y = 4,
+				Flags = MouseFlags.ReportMousePosition,
+				View = top.Subviews [0]
+			}));
+			Application.Refresh ();
+			Assert.Equal (new Point (41, 9), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                              ┌────────┐
+                 ┌───────────┐│ One    │
+                 │ SubMenu1  ││ Two    │
+                 │ SubMenu2  ││ Three  │
+                 │ SubMenu3  ││ Four  ►│
+                 │ SubMenu4  ││ Five   │
+                 │ SubMenu5  ││ Six    │
+                 │ SubMenu6  │└────────┘
+                 │ SubMenu7  │          
+                 └───────────┘          
+", output);
+
+			cm.Position = new Point (41, 22);
+			cm.Show ();
+			Application.Refresh ();
+			Assert.Equal (new Point (41, 22), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                              ┌────────┐
+                              │ One    │
+                              │ Two    │
+                              │ Three  │
+                              │ Four  ►│
+                              │ Five   │
+                              │ Six    │
+                              └────────┘
+", output);
+
+			Assert.True (top.Subviews [0].MouseEvent (new MouseEvent {
+				X = 30,
+				Y = 4,
+				Flags = MouseFlags.ReportMousePosition,
+				View = top.Subviews [0]
+			}));
+			Application.Refresh ();
+			Assert.Equal (new Point (41, 22), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+                 ┌───────────┐          
+                 │ SubMenu1  │┌────────┐
+                 │ SubMenu2  ││ One    │
+                 │ SubMenu3  ││ Two    │
+                 │ SubMenu4  ││ Three  │
+                 │ SubMenu5  ││ Four  ►│
+                 │ SubMenu6  ││ Five   │
+                 │ SubMenu7  ││ Six    │
+                 └───────────┘└────────┘
+", output);
+
+			((FakeDriver)Application.Driver).SetBufferSize (18, 8);
+			cm.Position = new Point (19, 10);
+			cm.Show ();
+			Application.Refresh ();
+			Assert.Equal (new Point (19, 10), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+        ┌────────┐
+        │ One    │
+        │ Two    │
+        │ Three  │
+        │ Four  ►│
+        │ Five   │
+        │ Six    │
+        └────────┘
+", output);
+
+			Assert.True (top.Subviews [0].MouseEvent (new MouseEvent {
+				X = 30,
+				Y = 4,
+				Flags = MouseFlags.ReportMousePosition,
+				View = top.Subviews [0]
+			}));
+			Application.Refresh ();
+			Assert.Equal (new Point (19, 10), cm.Position);
+			GraphViewTests.AssertDriverContentsWithFrameAre (@"
+┌───────────┐────┐
+│ SubMenu1  │    │
+│ SubMenu2  │    │
+│ SubMenu3  │ee  │
+│ SubMenu4  │r  ►│
+│ SubMenu5  │e   │
+│ SubMenu6  │    │
+│ SubMenu7  │────┘
+", output);
+		}
 	}
 }

UnitTests/MenuTests.cs

@@ -670,7 +670,7 @@ Edit
 
 			menu.CloseAllMenus ();
 			menu.Frame = new Rect (0, 0, menu.Frame.Width, menu.Frame.Height);
-			((FakeDriver)Application.Driver).SetBufferSize (7, 4);
+			((FakeDriver)Application.Driver).SetBufferSize (7, 3);
 			menu.OpenMenu ();
 			Application.Refresh ();
 
@@ -681,7 +681,7 @@ Edit
 ";
 
 			pos = GraphViewTests.AssertDriverContentsWithFrameAre (expected, output);
-			Assert.Equal (new Rect (0, 1, 7, 3), pos);
+			Assert.Equal (new Rect (0, 0, 7, 3), pos);
 		}
 
 		[Fact, AutoInitShutdown]