Merge pull request #2844 from BDisp/v2_menu-can-execute-false-fix_2787

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

Terminal.Gui/Views/Menu.cs

@@ -468,7 +468,7 @@ namespace Terminal.Gui {
 
 				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;
 					}
@@ -706,17 +706,14 @@ namespace Terminal.Gui {
 
 		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)
@@ -1095,6 +1092,17 @@ namespace Terminal.Gui {
 		/// </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>
@@ -1289,6 +1297,11 @@ namespace Terminal.Gui {
 			CloseAllMenus ();
 			Application.Refresh ();
 
+			Run (action);
+		}
+
+		internal void Run (Action action)
+		{
 			Application.MainLoop.AddIdle (() => {
 				action ();
 				return false;
@@ -1811,6 +1824,44 @@ namespace Terminal.Gui {
 					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.ToString ());
+				if (p != -1 && p + 1 < mi.Title.GetRuneCount ()) {
+					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;
 					}
 				}
 			}
@@ -1878,10 +1929,11 @@ namespace Terminal.Gui {
 		public override bool ProcessHotKey (KeyEvent kb)
 		{
 			if (kb.Key == Key) {
-				if (!IsMenuOpen)
+				if (Visible && !IsMenuOpen) {
 					OpenMenu ();
-				else
+				} else {
 					CloseAllMenus ();
+				}
 				return true;
 			}
 

UnitTests/Views/MenuTests.cs

@@ -258,8 +258,9 @@ Edit
 			// 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);
@@ -2707,5 +2708,67 @@ wo
 			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.RunIteration ();
+			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);
+		}
 	}
 }