Fixes #2787. MenuItem with CanExecute returning false is select when a MenuBar is opened.

Terminal.Gui/Views/Menu.cs

@@ -414,7 +414,7 @@ public Menu (MenuBar host, int x, int y, MenuBarItem barItems, Menu parent = nul
 
 				current = -1;
 				for (int i = 0; i < barItems.Children?.Length; i++) {
-					if (barItems.Children [i] != null) {
+					if (barItems.Children [i]?.IsEnabled () == true) {
 						current = i;
 						break;
 					}
@@ -572,17 +572,14 @@ public override void PositionCursor ()
 
 		public void Run (Action action)
 		{
-			if (action == null)
+			if (action == null || host == null)
 				return;
 
 			Application.UngrabMouse ();
 			host.CloseAllMenus ();
 			Application.Refresh ();
 
-			Application.MainLoop.AddIdle (() => {
-				action ();
-				return false;
-			});
+			host.Run (action);
 		}
 
 		public override bool OnLeave (View view)
@@ -946,6 +943,17 @@ public bool UseSubMenusSingleFrame {
 		/// </summary>
 		public Key Key { get; set; } = Key.F9;
 
+		///<inheritdoc/>
+		public override bool Visible {
+			get => base.Visible;
+			set {
+				base.Visible = value;
+				if (!value) {
+					CloseAllMenus ();
+				}
+			}
+		}
+
 		/// <summary>
 		/// Initializes a new instance of the <see cref="MenuBar"/>.
 		/// </summary>
@@ -1130,7 +1138,11 @@ void Selected (MenuItem item)
 			Application.UngrabMouse ();
 			CloseAllMenus ();
 			Application.Refresh ();
+			Run (action);
+		}
 
+		internal void Run (Action action)
+		{
 			Application.MainLoop.AddIdle (() => {
 				action ();
 				return false;
@@ -1641,6 +1653,44 @@ internal bool FindAndOpenMenuByHotkey (KeyEvent kb)
 					if (Char.ToUpperInvariant ((char)mi.Title [p + 1]) == c) {
 						ProcessMenu (i, mi);
 						return true;
+					} else if (mi.Children?.Length > 0) {
+						if (FindAndOpenChildrenMenuByHotkey (kb, mi.Children)) {
+							return true;
+						}
+					}
+				} else if (mi.Children?.Length > 0) {
+					if (FindAndOpenChildrenMenuByHotkey (kb, mi.Children)) {
+						return true;
+					}
+				}
+			}
+
+			return false;
+		}
+
+		bool FindAndOpenChildrenMenuByHotkey (KeyEvent kb, MenuItem [] children)
+		{
+			var c = ((uint)kb.Key & (uint)Key.CharMask);
+			for (int i = 0; i < children.Length; i++) {
+				var mi = children [i];
+				int p = mi.Title.IndexOf (MenuBar.HotKeySpecifier);
+				if (p != -1 && p + 1 < mi.Title.RuneCount) {
+					if (Char.ToUpperInvariant ((char)mi.Title [p + 1]) == c) {
+						if (mi.IsEnabled ()) {
+							var action = mi.Action;
+							if (action != null) {
+								Run (action);
+							}
+						}
+						return true;
+					} else if (mi is MenuBarItem menuBarItem && menuBarItem?.Children.Length > 0) {
+						if (FindAndOpenChildrenMenuByHotkey (kb, menuBarItem.Children)) {
+							return true;
+						}
+					}
+				} else if (mi is MenuBarItem menuBarItem && menuBarItem?.Children.Length > 0) {
+					if (FindAndOpenChildrenMenuByHotkey (kb, menuBarItem.Children)) {
+						return true;
 					}
 				}
 			}
@@ -1662,12 +1712,11 @@ internal bool FindAndOpenMenuByShortcut (KeyEvent kb, MenuItem [] children = nul
 					continue;
 				}
 				if ((!(mi is MenuBarItem mbiTopLevel) || mbiTopLevel.IsTopLevel) && mi.Shortcut != Key.Null && mi.Shortcut == (Key)key) {
-					var action = mi.Action;
-					if (action != null) {
-						Application.MainLoop.AddIdle (() => {
-							action ();
-							return false;
-						});
+					if (mi.IsEnabled ()) {
+						var action = mi.Action;
+						if (action != null) {
+							Run (action);
+						}
 					}
 					return true;
 				}
@@ -1707,10 +1756,11 @@ private void ProcessMenu (int i, MenuBarItem mi)
 		public override bool ProcessHotKey (KeyEvent kb)
 		{
 			if (kb.Key == Key) {
-				if (!IsMenuOpen)
+				if (Visible && !IsMenuOpen) {
 					OpenMenu ();
-				else
+				} else {
 					CloseAllMenus ();
+				}
 				return true;
 			}
 

UnitTests/Menus/MenuTests.cs

@@ -254,8 +254,9 @@ public void MenuOpened_On_Disabled_MenuItem ()
 			// open the menu
 			Assert.True (menu.ProcessHotKey (new KeyEvent (Key.F9, new KeyModifiers ())));
 			Assert.True (menu.IsMenuOpen);
-			Assert.Equal ("_New", miCurrent.Parent.Title);
-			Assert.Equal ("_New doc", miCurrent.Title);
+			// The _New doc isn't enabled because it can't execute and so can't be selected
+			Assert.Equal ("_File", miCurrent.Parent.Title);
+			Assert.Equal ("_New", miCurrent.Title);
 
 			Assert.True (mCurrent.ProcessKey (new KeyEvent (Key.CursorDown, new KeyModifiers ())));
 			Assert.True (menu.IsMenuOpen);
@@ -1746,5 +1747,67 @@ public void MenuBarItem_Children_Null_Does_Not_Throw ()
 			var exception = Record.Exception (() => menu.ProcessColdKey (new KeyEvent (Key.Space, new KeyModifiers ())));
 			Assert.Null (exception);
 		}
+
+		[Fact, AutoInitShutdown]
+		public void CanExecute_ProcessHotKey ()
+		{
+			Window win = null;
+			var menu = new MenuBar (new MenuBarItem [] {
+				new MenuBarItem ("File", new MenuItem [] {
+					new MenuItem ("_New", "", New, CanExecuteNew),
+					new MenuItem ("_Close", "", Close, CanExecuteClose)
+				}),
+			});
+			var top = Application.Top;
+			top.Add (menu);
+
+			bool CanExecuteNew () => win == null;
+
+			void New ()
+			{
+				win = new Window ();
+			}
+
+			bool CanExecuteClose () => win != null;
+
+			void Close ()
+			{
+				win = null;
+			}
+
+			Application.Begin (top);
+
+			Assert.Null (win);
+			Assert.True (CanExecuteNew ());
+			Assert.False (CanExecuteClose ());
+
+			Assert.True (top.ProcessHotKey (new KeyEvent (Key.N | Key.AltMask, new KeyModifiers () { Alt = true })));
+			Application.MainLoop.MainIteration ();
+			Assert.NotNull (win);
+			Assert.False (CanExecuteNew ());
+			Assert.True (CanExecuteClose ());
+		}
+
+		[Fact, AutoInitShutdown]
+		public void Visible_False_Key_Does_Not_Open_And_Close_All_Opened_Menus ()
+		{
+			var menu = new MenuBar (new MenuBarItem [] {
+				new MenuBarItem ("File", new MenuItem [] {
+					new MenuItem ("New", "", null)
+				})
+			});
+			Application.Top.Add (menu);
+			Application.Begin (Application.Top);
+
+			Assert.True (menu.Visible);
+			Assert.True (menu.ProcessHotKey (new KeyEvent (menu.Key, new KeyModifiers ())));
+			Assert.True (menu.IsMenuOpen);
+
+			menu.Visible = false;
+			Assert.False (menu.IsMenuOpen);
+
+			Assert.True (menu.ProcessHotKey (new KeyEvent (menu.Key, new KeyModifiers ())));
+			Assert.False (menu.IsMenuOpen);
+		}
 	}
 }