Fixes #3652. Setting Menus causes unexpected Exception. (#3653)

* Moving ShortcutDelimiter from MenuBar to Key.

* Rename to ShortcutKey and change type to Key.

* Improving add and remove menu items dynamically.

* Code cleanup.

* Fix status bar shortcuts issues.

* Fix build error.

* Change HotKey type to Key.

* Change HotKey.setter to private.

* Fix warnings.

* Fix some bugs.

* Rename ShortcutDelimiter to Separator.

* Add Separator property into the Configuration Manager.

* Change XML doc for Separator.

* Replace KeyEvent with Key.

* Add unit test preventing the Key.Separator is never Null ('\0).

Terminal.Gui/Application/Application.Keyboard.cs

@@ -9,7 +9,6 @@ public static partial class Application // Keyboard handling
 
     /// <summary>Alternative key to navigate forwards through views. Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key NextTabKey
     {
         get => _nextTabKey;
@@ -27,7 +26,6 @@ public static partial class Application // Keyboard handling
 
     /// <summary>Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key PrevTabKey
     {
         get => _prevTabKey;
@@ -45,7 +43,6 @@ public static partial class Application // Keyboard handling
 
     /// <summary>Alternative key to navigate forwards through views. Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key NextTabGroupKey
     {
         get => _nextTabGroupKey;
@@ -63,7 +60,6 @@ public static partial class Application // Keyboard handling
 
     /// <summary>Alternative key to navigate backwards through views. Shift+Ctrl+Tab is the primary key.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key PrevTabGroupKey
     {
         get => _prevTabGroupKey;
@@ -81,7 +77,6 @@ public static partial class Application // Keyboard handling
 
     /// <summary>Gets or sets the key to quit the application.</summary>
     [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
-    [JsonConverter (typeof (KeyJsonConverter))]
     public static Key QuitKey
     {
         get => _quitKey;

Terminal.Gui/Input/Key.cs

@@ -1,5 +1,6 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
+using System.Text.Json.Serialization;
 
 namespace Terminal.Gui;
 
@@ -448,9 +449,9 @@ public class Key : EventArgs, IEquatable<Key>
 
     #region String conversion
 
-    /// <summary>Pretty prints the KeyEvent</summary>
+    /// <summary>Pretty prints the Key.</summary>
     /// <returns></returns>
-    public override string ToString () { return ToString (KeyCode, (Rune)'+'); }
+    public override string ToString () { return ToString (KeyCode, Separator); }
 
     private static string GetKeyString (KeyCode key)
     {
@@ -483,7 +484,7 @@ public class Key : EventArgs, IEquatable<Key>
     ///     The formatted string. If the key is a printable character, it will be returned as a string. Otherwise, the key
     ///     name will be returned.
     /// </returns>
-    public static string ToString (KeyCode key) { return ToString (key, (Rune)'+'); }
+    public static string ToString (KeyCode key) { return ToString (key, Separator); }
 
     /// <summary>Formats a <see cref="KeyCode"/> as a string.</summary>
     /// <param name="key">The key to format.</param>
@@ -584,7 +585,7 @@ public class Key : EventArgs, IEquatable<Key>
         key = null;
 
         // Split the string into parts
-        string [] parts = text.Split ('+', '-');
+        string [] parts = text.Split ('+', '-', (char)Separator.Value);
 
         if (parts.Length is 0 or > 4 || parts.Any (string.IsNullOrEmpty))
         {
@@ -971,4 +972,20 @@ public class Key : EventArgs, IEquatable<Key>
     public static Key F24 => new (KeyCode.F24);
 
     #endregion
+
+    private static Rune _separator = new ('+');
+
+    /// <summary>Gets or sets the separator character used when parsing and printing Keys. E.g. Ctrl+A. The default is '+'.</summary>
+    [SerializableConfigurationProperty (Scope = typeof (SettingsScope))]
+    public static Rune Separator
+    {
+        get => _separator;
+        set
+        {
+            if (_separator != value)
+            {
+                _separator = value == default (Rune) ? new ('+') : value;
+            }
+        }
+    }
 }

Terminal.Gui/Input/ShortcutHelper.cs

@@ -23,7 +23,7 @@ public class ShortcutHelper
     }
 
     /// <summary>The keystroke combination used in the <see cref="Shortcut"/> as string.</summary>
-    public virtual string ShortcutTag => Key.ToString (shortcut, MenuBar.ShortcutDelimiter);
+    public virtual string ShortcutTag => Key.ToString (shortcut, Key.Separator);
 
     /// <summary>Lookup for a <see cref="KeyCode"/> on range of keys.</summary>
     /// <param name="key">The source key.</param>
@@ -59,7 +59,7 @@ public class ShortcutHelper
         //var hasCtrl = false;
         if (delimiter == default (Rune))
         {
-            delimiter = MenuBar.ShortcutDelimiter;
+            delimiter = Key.Separator;
         }
 
         string [] keys = sCut.Split (delimiter.ToString ());

Terminal.Gui/Resources/config.json

@@ -22,6 +22,7 @@
   "Application.NextTabGroupKey": "F6",
   "Application.PrevTabGroupKey": "Shift+F6",
   "Application.QuitKey": "Esc",
+  "Key.Separator": "+",
 
   "Theme": "Default",
   "Themes": [

Terminal.Gui/Views/Menu/Menu.cs

@@ -120,7 +120,7 @@ internal sealed class Menu : View
                     }
                    );
 
-        AddKeyBindings (_barItems);
+        AddKeyBindingsHotKey (_barItems);
     }
 
     public Menu ()
@@ -179,7 +179,7 @@ internal sealed class Menu : View
         KeyBindings.Add (Key.Enter, Command.Accept);
     }
 
-    private void AddKeyBindings (MenuBarItem menuBarItem)
+    private void AddKeyBindingsHotKey (MenuBarItem menuBarItem)
     {
         if (menuBarItem is null || menuBarItem.Children is null)
         {
@@ -190,23 +190,30 @@ internal sealed class Menu : View
         {
             KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, menuItem);
 
-            if ((KeyCode)menuItem.HotKey.Value != KeyCode.Null)
+            if (menuItem.HotKey != Key.Empty)
             {
-                KeyBindings.Remove ((KeyCode)menuItem.HotKey.Value);
-                KeyBindings.Add ((KeyCode)menuItem.HotKey.Value, keyBinding);
-                KeyBindings.Remove ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask);
-                KeyBindings.Add ((KeyCode)menuItem.HotKey.Value | KeyCode.AltMask, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey);
+                KeyBindings.Add (menuItem.HotKey, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey.WithAlt);
+                KeyBindings.Add (menuItem.HotKey.WithAlt, keyBinding);
             }
+        }
+    }
+
+    private void RemoveKeyBindingsHotKey (MenuBarItem menuBarItem)
+    {
+        if (menuBarItem is null || menuBarItem.Children is null)
+        {
+            return;
+        }
 
-            if (menuItem.Shortcut != KeyCode.Null)
+        foreach (MenuItem menuItem in menuBarItem.Children.Where (m => m is { }))
+        {
+            if (menuItem.HotKey != Key.Empty)
             {
-                keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem);
-                KeyBindings.Remove (menuItem.Shortcut);
-                KeyBindings.Add (menuItem.Shortcut, keyBinding);
+                KeyBindings.Remove (menuItem.HotKey);
+                KeyBindings.Remove (menuItem.HotKey.WithAlt);
             }
-
-            MenuBarItem subMenu = menuBarItem.SubMenu (menuItem);
-            AddKeyBindings (subMenu);
         }
     }
 
@@ -910,6 +917,8 @@ internal sealed class Menu : View
 
     protected override void Dispose (bool disposing)
     {
+        RemoveKeyBindingsHotKey (_barItems);
+
         if (Application.Current is { })
         {
             Application.Current.DrawContentComplete -= Current_DrawContentComplete;

Terminal.Gui/Views/Menu/MenuBar.cs

@@ -66,6 +66,8 @@ public class MenuBar : View, IDesignable
     /// <summary>Initializes a new instance of the <see cref="MenuBar"/>.</summary>
     public MenuBar ()
     {
+        MenuItem._menuBar = this;
+
         TabStop = TabBehavior.NoStop;
         X = 0;
         Y = 0;
@@ -122,7 +124,7 @@ public class MenuBar : View, IDesignable
                         return true;
                     }
                    );
-        AddCommand (Command.ToggleExpandCollapse, ctx => Select ((int)ctx.KeyBinding?.Context!));
+        AddCommand (Command.ToggleExpandCollapse, ctx => Select (Menus.IndexOf (ctx.KeyBinding?.Context)));
         AddCommand (Command.Select, ctx => Run ((ctx.KeyBinding?.Context as MenuItem)?.Action));
 
         // Default key bindings for this view
@@ -172,19 +174,23 @@ public class MenuBar : View, IDesignable
             {
                 MenuBarItem menuBarItem = Menus [i];
 
-                if (menuBarItem?.HotKey != default (Rune))
+                if (menuBarItem?.HotKey != Key.Empty)
                 {
-                    KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.Focused, i);
-                    KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value, keyBinding);
-                    keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, i);
-                    KeyBindings.Add ((KeyCode)menuBarItem.HotKey.Value | KeyCode.AltMask, keyBinding);
+                    KeyBindings.Remove (menuBarItem!.HotKey);
+                    KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.Focused, menuBarItem);
+                    KeyBindings.Add (menuBarItem!.HotKey, keyBinding);
+                    KeyBindings.Remove (menuBarItem.HotKey.WithAlt);
+                    keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, menuBarItem);
+                    KeyBindings.Add (menuBarItem.HotKey.WithAlt, keyBinding);
                 }
 
-                if (menuBarItem?.Shortcut != KeyCode.Null)
+                if (menuBarItem?.ShortcutKey != Key.Empty)
                 {
                     // Technically this will never run because MenuBarItems don't have shortcuts
-                    KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, i);
-                    KeyBindings.Add (menuBarItem.Shortcut, keyBinding);
+                    // unless the IsTopLevel is true
+                    KeyBindings.Remove (menuBarItem.ShortcutKey);
+                    KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuBarItem);
+                    KeyBindings.Add (menuBarItem.ShortcutKey, keyBinding);
                 }
 
                 menuBarItem?.AddShortcutKeyBindings (this);
@@ -1255,21 +1261,6 @@ public class MenuBar : View, IDesignable
         }
     }
 
-    private static Rune _shortcutDelimiter = new ('+');
-
-    /// <summary>Sets or gets the shortcut delimiter separator. The default is "+".</summary>
-    public static Rune ShortcutDelimiter
-    {
-        get => _shortcutDelimiter;
-        set
-        {
-            if (_shortcutDelimiter != value)
-            {
-                _shortcutDelimiter = value == default (Rune) ? new ('+') : value;
-            }
-        }
-    }
-
     /// <summary>The specifier character for the hot keys.</summary>
     public new static Rune HotKeySpecifier => (Rune)'_';
 
@@ -1321,6 +1312,10 @@ public class MenuBar : View, IDesignable
         {
             OpenMenu ();
         }
+        else if (Menus [index].IsTopLevel)
+        {
+            Run (Menus [index].Action);
+        }
         else
         {
             Activate (index);
@@ -1766,4 +1761,12 @@ public class MenuBar : View, IDesignable
         ];
         return true;
     }
+
+    /// <inheritdoc />
+    protected override void Dispose (bool disposing)
+    {
+        MenuItem._menuBar = null;
+
+        base.Dispose (disposing);
+    }
 }

Terminal.Gui/Views/Menu/MenuBarItem.cs

@@ -2,7 +2,7 @@ namespace Terminal.Gui;
 
 /// <summary>
 ///     <see cref="MenuBarItem"/> is a menu item on  <see cref="MenuBar"/>. MenuBarItems do not support
-///     <see cref="MenuItem.Shortcut"/>.
+///     <see cref="MenuItem.ShortcutKey"/>.
 /// </summary>
 public class MenuBarItem : MenuItem
 {
@@ -100,11 +100,9 @@ public class MenuBarItem : MenuItem
         {
             // For MenuBar only add shortcuts for submenus
 
-            if (menuItem.Shortcut != KeyCode.Null)
+            if (menuItem.ShortcutKey != Key.Empty)
             {
-                KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem);
-                menuBar.KeyBindings.Remove (menuItem.Shortcut);
-                menuBar.KeyBindings.Add (menuItem.Shortcut, keyBinding);
+                menuItem.UpdateShortcutKeyBinding (Key.Empty);
             }
 
             SubMenu (menuItem)?.AddShortcutKeyBindings (menuBar);
@@ -176,4 +174,75 @@ public class MenuBarItem : MenuItem
         title ??= string.Empty;
         Title = title;
     }
+
+    /// <summary>
+    /// Add a <see cref="MenuBarItem"/> dynamically into the <see cref="MenuBar"/><c>.Menus</c>.
+    /// </summary>
+    /// <param name="menuItem"></param>
+    public void AddMenuBarItem (MenuItem menuItem = null)
+    {
+        if (menuItem is null)
+        {
+            MenuBarItem [] menus = _menuBar.Menus;
+            Array.Resize (ref menus, menus.Length + 1);
+            menus [^1] = this;
+            _menuBar.Menus = menus;
+        }
+        else
+        {
+            MenuItem [] childrens = Children ?? [];
+            Array.Resize (ref childrens, childrens.Length + 1);
+            childrens [^1] = menuItem;
+            Children = childrens;
+        }
+    }
+
+    /// <inheritdoc />
+    public override void RemoveMenuItem ()
+    {
+        if (Children is { })
+        {
+            foreach (MenuItem menuItem in Children)
+            {
+                if (menuItem.ShortcutKey != Key.Empty)
+                {
+                    // Remove an existent ShortcutKey
+                    _menuBar?.KeyBindings.Remove (menuItem.ShortcutKey);
+                }
+            }
+        }
+
+        if (ShortcutKey != Key.Empty)
+        {
+            // Remove an existent ShortcutKey
+            _menuBar?.KeyBindings.Remove (ShortcutKey);
+        }
+
+        var index = _menuBar!.Menus.IndexOf (this);
+        if (index > -1)
+        {
+            if (_menuBar!.Menus [index].HotKey != Key.Empty)
+            {
+                // Remove an existent HotKey
+                _menuBar?.KeyBindings.Remove (HotKey.WithAlt);
+            }
+
+            _menuBar!.Menus [index] = null;
+        }
+
+        var i = 0;
+
+        foreach (MenuBarItem m in _menuBar.Menus)
+        {
+            if (m != null)
+            {
+                _menuBar.Menus [i] = m;
+                i++;
+            }
+        }
+
+        MenuBarItem [] menus = _menuBar.Menus;
+        Array.Resize (ref menus, menus.Length - 1);
+        _menuBar.Menus = menus;
+    }
 }

Terminal.Gui/Views/Menu/MenuItem.cs

@@ -6,31 +6,25 @@ namespace Terminal.Gui;
 /// </summary>
 public class MenuItem
 {
-    private readonly ShortcutHelper _shortcutHelper;
-    private bool _allowNullChecked;
-    private MenuItemCheckStyle _checkType;
+    internal static MenuBar _menuBar;
 
-    private string _title;
-
-    // TODO: Update to use Key instead of KeyCode
     /// <summary>Initializes a new instance of <see cref="MenuItem"/></summary>
-    public MenuItem (KeyCode shortcut = KeyCode.Null) : this ("", "", null, null, null, shortcut) { }
+    public MenuItem (Key shortcutKey = null) : this ("", "", null, null, null, shortcutKey) { }
 
-    // TODO: Update to use Key instead of KeyCode
     /// <summary>Initializes a new instance of <see cref="MenuItem"/>.</summary>
     /// <param name="title">Title for the menu item.</param>
     /// <param name="help">Help text to display.</param>
     /// <param name="action">Action to invoke when the menu item is activated.</param>
     /// <param name="canExecute">Function to determine if the action can currently be executed.</param>
     /// <param name="parent">The <see cref="Parent"/> of this menu item.</param>
-    /// <param name="shortcut">The <see cref="Shortcut"/> keystroke combination.</param>
+    /// <param name="shortcutKey">The <see cref="ShortcutKey"/> keystroke combination.</param>
     public MenuItem (
         string title,
         string help,
         Action action,
         Func<bool> canExecute = null,
         MenuItem parent = null,
-        KeyCode shortcut = KeyCode.Null
+        Key shortcutKey = null
     )
     {
         Title = title ?? "";
@@ -38,14 +32,20 @@ public class MenuItem
         Action = action;
         CanExecute = canExecute;
         Parent = parent;
-        _shortcutHelper = new ();
 
-        if (shortcut != KeyCode.Null)
+        if (Parent is { } && Parent.ShortcutKey != Key.Empty)
         {
-            Shortcut = shortcut;
+            Parent.ShortcutKey = Key.Empty;
         }
+        // Setter will ensure Key.Empty if it's null
+        ShortcutKey = shortcutKey;
     }
 
+    private bool _allowNullChecked;
+    private MenuItemCheckStyle _checkType;
+
+    private string _title;
+
     /// <summary>Gets or sets the action to be invoked when the menu item is triggered.</summary>
     /// <value>Method to invoke.</value>
     public Action Action { get; set; }
@@ -104,6 +104,12 @@ public class MenuItem
     /// <value>The help text.</value>
     public string Help { get; set; }
 
+    /// <summary>
+    ///     Returns <see langword="true"/> if the menu item is enabled. This method is a wrapper around
+    ///     <see cref="CanExecute"/>.
+    /// </summary>
+    public bool IsEnabled () { return CanExecute?.Invoke () ?? true; }
+
     /// <summary>Gets the parent for this <see cref="MenuItem"/>.</summary>
     /// <value>The parent.</value>
     public MenuItem Parent { get; set; }
@@ -125,46 +131,6 @@ public class MenuItem
         }
     }
 
-    /// <summary>Gets if this <see cref="MenuItem"/> is from a sub-menu.</summary>
-    internal bool IsFromSubMenu => Parent != null;
-
-    internal int TitleLength => GetMenuBarItemLength (Title);
-
-    // 
-    // ┌─────────────────────────────┐
-    // │ Quit  Quit UI Catalog  Ctrl+Q │
-    // └─────────────────────────────┘
-    // ┌─────────────────┐
-    // │ ◌ TopLevel Alt+T │
-    // └─────────────────┘
-    // TODO: Replace the `2` literals with named constants 
-    internal int Width => 1
-                          + // space before Title
-                          TitleLength
-                          + 2
-                          + // space after Title - BUGBUG: This should be 1 
-                          (Checked == true || CheckType.HasFlag (MenuItemCheckStyle.Checked) || CheckType.HasFlag (MenuItemCheckStyle.Radio)
-                               ? 2
-                               : 0)
-                          + // check glyph + space 
-                          (Help.GetColumns () > 0 ? 2 + Help.GetColumns () : 0)
-                          + // Two spaces before Help
-                          (ShortcutTag.GetColumns () > 0
-                               ? 2 + ShortcutTag.GetColumns ()
-                               : 0); // Pad two spaces before shortcut tag (which are also aligned right)
-
-    /// <summary>Merely a debugging aid to see the interaction with main.</summary>
-    internal bool GetMenuBarItem () { return IsFromSubMenu; }
-
-    /// <summary>Merely a debugging aid to see the interaction with main.</summary>
-    internal MenuItem GetMenuItem () { return this; }
-
-    /// <summary>
-    ///     Returns <see langword="true"/> if the menu item is enabled. This method is a wrapper around
-    ///     <see cref="CanExecute"/>.
-    /// </summary>
-    public bool IsEnabled () { return CanExecute?.Invoke () ?? true; }
-
     /// <summary>
     ///     Toggle the <see cref="Checked"/> between three states if <see cref="AllowNullChecked"/> is
     ///     <see langword="true"/> or between two states if <see cref="AllowNullChecked"/> is <see langword="false"/>.
@@ -193,6 +159,40 @@ public class MenuItem
         }
     }
 
+    /// <summary>Merely a debugging aid to see the interaction with main.</summary>
+    internal bool GetMenuBarItem () { return IsFromSubMenu; }
+
+    /// <summary>Merely a debugging aid to see the interaction with main.</summary>
+    internal MenuItem GetMenuItem () { return this; }
+
+    /// <summary>Gets if this <see cref="MenuItem"/> is from a sub-menu.</summary>
+    internal bool IsFromSubMenu => Parent != null;
+
+    internal int TitleLength => GetMenuBarItemLength (Title);
+
+    // 
+    // ┌─────────────────────────────┐
+    // │ Quit  Quit UI Catalog  Ctrl+Q │
+    // └─────────────────────────────┘
+    // ┌─────────────────┐
+    // │ ◌ TopLevel Alt+T │
+    // └─────────────────┘
+    // TODO: Replace the `2` literals with named constants
+    internal int Width => 1
+                          + // space before Title
+                          TitleLength
+                          + 2
+                          + // space after Title - BUGBUG: This should be 1
+                          (Checked == true || CheckType.HasFlag (MenuItemCheckStyle.Checked) || CheckType.HasFlag (MenuItemCheckStyle.Radio)
+                               ? 2
+                               : 0)
+                          + // check glyph + space 
+                          (Help.GetColumns () > 0 ? 2 + Help.GetColumns () : 0)
+                          + // Two spaces before Help
+                          (ShortcutTag.GetColumns () > 0
+                               ? 2 + ShortcutTag.GetColumns ()
+                               : 0); // Pad two spaces before shortcut tag (which are also aligned right)
+
     private static int GetMenuBarItemLength (string title)
     {
         return title.EnumerateRunes ()
@@ -202,21 +202,32 @@ public class MenuItem
 
     #region Keyboard Handling
 
-    // TODO: Update to use Key instead of Rune
+    private Key _hotKey = Key.Empty;
+
     /// <summary>
     ///     The HotKey is used to activate a <see cref="MenuItem"/> with the keyboard. HotKeys are defined by prefixing the
     ///     <see cref="Title"/> of a MenuItem with an underscore ('_').
     ///     <para>
     ///         Pressing Alt-Hotkey for a <see cref="MenuBarItem"/> (menu items on the menu bar) works even if the menu is
-    ///         not active). Once a menu has focus and is active, pressing just the HotKey will activate the MenuItem.
+    ///         not active. Once a menu has focus and is active, pressing just the HotKey will activate the MenuItem.
     ///     </para>
     ///     <para>
     ///         For example for a MenuBar with a "_File" MenuBarItem that contains a "_New" MenuItem, Alt-F will open the
     ///         File menu. Pressing the N key will then activate the New MenuItem.
     ///     </para>
-    ///     <para>See also <see cref="Shortcut"/> which enable global key-bindings to menu items.</para>
+    ///     <para>See also <see cref="ShortcutKey"/> which enable global key-bindings to menu items.</para>
     /// </summary>
-    public Rune HotKey { get; set; }
+    public Key HotKey
+    {
+        get => _hotKey;
+        private set
+        {
+            var oldKey = _hotKey ?? Key.Empty;
+            _hotKey = value ?? Key.Empty;
+            UpdateHotKeyBinding (oldKey);
+        }
+    }
+
     private void GetHotKey ()
     {
         var nextIsHot = false;
@@ -227,47 +238,130 @@ public class MenuItem
             {
                 nextIsHot = true;
             }
-            else
+            else if (nextIsHot)
             {
-                if (nextIsHot)
-                {
-                    HotKey = (Rune)char.ToUpper (x);
+                    HotKey = char.ToLower (x);
 
-                    break;
-                }
-
-                nextIsHot = false;
-                HotKey = default (Rune);
+                    return;
             }
         }
+
+        HotKey = Key.Empty;
     }
 
-    // TODO: Update to use Key instead of KeyCode
+    private Key _shortcutKey = Key.Empty;
+
     /// <summary>
     ///     Shortcut defines a key binding to the MenuItem that will invoke the MenuItem's action globally for the
     ///     <see cref="View"/> that is the parent of the <see cref="MenuBar"/> or <see cref="ContextMenu"/> this
     ///     <see cref="MenuItem"/>.
     ///     <para>
-    ///         The <see cref="KeyCode"/> will be drawn on the MenuItem to the right of the <see cref="Title"/> and
+    ///         The <see cref="Key"/> will be drawn on the MenuItem to the right of the <see cref="Title"/> and
     ///         <see cref="Help"/> text. See <see cref="ShortcutTag"/>.
     ///     </para>
     /// </summary>
-    public KeyCode Shortcut
+    public Key ShortcutKey
     {
-        get => _shortcutHelper.Shortcut;
+        get => _shortcutKey;
         set
         {
-            if (_shortcutHelper.Shortcut != value && (ShortcutHelper.PostShortcutValidation (value) || value == KeyCode.Null))
+            var oldKey = _shortcutKey ?? Key.Empty;
+            _shortcutKey = value ?? Key.Empty;
+            UpdateShortcutKeyBinding (oldKey);
+        }
+    }
+
+    /// <summary>Gets the text describing the keystroke combination defined by <see cref="ShortcutKey"/>.</summary>
+    public string ShortcutTag => ShortcutKey != Key.Empty ? ShortcutKey.ToString () : string.Empty;
+
+    private void UpdateHotKeyBinding (Key oldKey)
+    {
+        if (_menuBar is null || _menuBar?.IsInitialized == false)
+        {
+            return;
+        }
+
+        if (oldKey != Key.Empty)
+        {
+            var index = _menuBar.Menus?.IndexOf (this);
+
+            if (index > -1)
+            {
+                _menuBar.KeyBindings.Remove (oldKey.WithAlt);
+            }
+        }
+
+        if (HotKey != Key.Empty)
+        {
+            var index = _menuBar.Menus?.IndexOf (this);
+
+            if (index > -1)
             {
-                _shortcutHelper.Shortcut = value;
+                _menuBar.KeyBindings.Remove (HotKey.WithAlt);
+                KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, this);
+                _menuBar.KeyBindings.Add (HotKey.WithAlt, keyBinding);
             }
         }
     }
 
-    /// <summary>Gets the text describing the keystroke combination defined by <see cref="Shortcut"/>.</summary>
-    public string ShortcutTag => _shortcutHelper.Shortcut == KeyCode.Null
-                                     ? string.Empty
-                                     : Key.ToString (_shortcutHelper.Shortcut, MenuBar.ShortcutDelimiter);
+    internal void UpdateShortcutKeyBinding (Key oldKey)
+    {
+        if (_menuBar is null)
+        {
+            return;
+        }
+
+        if (oldKey != Key.Empty)
+        {
+            _menuBar.KeyBindings.Remove (oldKey);
+        }
+
+        if (ShortcutKey != Key.Empty)
+        {
+            KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, this);
+            // Remove an existent ShortcutKey
+            _menuBar?.KeyBindings.Remove (ShortcutKey);
+            _menuBar?.KeyBindings.Add (ShortcutKey, keyBinding);
+        }
+    }
 
     #endregion Keyboard Handling
+
+    /// <summary>
+    /// Removes a <see cref="MenuItem"/> dynamically from the <see cref="Parent"/>.
+    /// </summary>
+    public virtual void RemoveMenuItem ()
+    {
+        if (Parent is { })
+        {
+            MenuItem [] childrens = ((MenuBarItem)Parent).Children;
+            var i = 0;
+
+            foreach (MenuItem c in childrens)
+            {
+                if (c != this)
+                {
+                    childrens [i] = c;
+                    i++;
+                }
+            }
+
+            Array.Resize (ref childrens, childrens.Length - 1);
+
+            if (childrens.Length == 0)
+            {
+                ((MenuBarItem)Parent).Children = null;
+            }
+            else
+            {
+                ((MenuBarItem)Parent).Children = childrens;
+            }
+        }
+
+        if (ShortcutKey != Key.Empty)
+        {
+            // Remove an existent ShortcutKey
+            _menuBar?.KeyBindings.Remove (ShortcutKey);
+        }
+    }
 }

UICatalog/Scenarios/ContextMenus.cs

@@ -179,7 +179,7 @@ public class ContextMenus : Scenario
                                                                        "This would open setup dialog",
                                                                        "Ok"
                                                                       ),
-                                                           shortcut: KeyCode.T
+                                                           shortcutKey: KeyCode.T
                                                                      | KeyCode
                                                                          .CtrlMask
                                                           ),

UICatalog/Scenarios/DynamicStatusBar.cs

@@ -15,7 +15,7 @@ public class DynamicStatusBar : Scenario
 {
     public override void Main ()
     {
-
+        Application.Init ();
         Application.Run<DynamicStatusBarSample> ().Dispose ();
         Application.Shutdown ();
     }
@@ -125,58 +125,9 @@ public class DynamicStatusBar : Scenario
 
             TextShortcut.KeyDown += (s, e) =>
                                     {
-                                        if (!ProcessKey (e))
-                                        {
-                                            return;
-                                        }
+                                        TextShortcut.Text = e.ToString ();
 
-                                        if (CheckShortcut (e.KeyCode, true))
-                                        {
-                                            e.Handled = true;
-                                        }
                                     };
-
-            bool ProcessKey (Key ev)
-            {
-                switch (ev.KeyCode)
-                {
-                    case KeyCode.CursorUp:
-                    case KeyCode.CursorDown:
-                    case KeyCode.Tab:
-                    case KeyCode.Tab | KeyCode.ShiftMask:
-                        return false;
-                }
-
-                return true;
-            }
-
-            bool CheckShortcut (KeyCode k, bool pre)
-            {
-                Shortcut m = _statusItem != null ? _statusItem : new Shortcut (k, "", null);
-
-                if (pre && !ShortcutHelper.PreShortcutValidation (k))
-                {
-                    TextShortcut.Text = "";
-
-                    return false;
-                }
-
-                if (!pre)
-                {
-                    return true;
-                }
-
-                TextShortcut.Text = k.ToString ();
-                return true;
-            }
-
-            TextShortcut.KeyUp += (s, e) =>
-                                  {
-                                      if (CheckShortcut (e.KeyCode, true))
-                                      {
-                                          e.Handled = true;
-                                      }
-                                  };
             Add (TextShortcut);
 
             var _btnShortcut = new Button
@@ -210,7 +161,7 @@ public class DynamicStatusBar : Scenario
                                   ? GetTargetAction (statusItem.Action)
                                   : string.Empty;
 
-            TextShortcut.Text = statusItem.CommandView.Text;
+            TextShortcut.Text = statusItem.Key;
         }
 
         public DynamicStatusItem EnterStatusItem ()
@@ -238,13 +189,6 @@ public class DynamicStatusBar : Scenario
                                   }
                                   else
                                   {
-                                      if (!string.IsNullOrEmpty (TextShortcut.Text))
-                                      {
-                                          TextTitle.Text = DynamicStatusBarSample.SetTitleText (
-                                                                                                TextTitle.Text,
-                                                                                                TextShortcut.Text
-                                                                                               );
-                                      }
 
                                       valid = true;
                                       Application.RequestStop ();
@@ -433,10 +377,6 @@ public class DynamicStatusBar : Scenario
                                   }
                                   else if (_currentEditStatusItem != null)
                                   {
-                                      _frmStatusBarDetails.TextTitle.Text = SetTitleText (
-                                                                                          _frmStatusBarDetails.TextTitle.Text,
-                                                                                          _frmStatusBarDetails.TextShortcut.Text
-                                                                                         );
 
                                       var statusItem = new DynamicStatusItem
                                       {
@@ -487,6 +427,7 @@ public class DynamicStatusBar : Scenario
                                       if (statusItem != null)
                                       {
                                           _statusBar.RemoveShortcut (_currentSelectedStatusBar);
+                                          statusItem.Dispose ();
                                           DataContext.Items.RemoveAt (_lstItems.SelectedItem);
 
                                           if (_lstItems.Source.Count > 0 && _lstItems.SelectedItem > _lstItems.Source.Count - 1)
@@ -526,6 +467,7 @@ public class DynamicStatusBar : Scenario
                                                }
 
                                                Remove (_statusBar);
+                                               _statusBar.Dispose ();
                                                _statusBar = null;
                                                DataContext.Items = [];
                                                _currentStatusItem = null;
@@ -588,7 +530,7 @@ public class DynamicStatusBar : Scenario
 
             Shortcut CreateNewStatusBar (DynamicStatusItem item)
             {
-                var newStatusItem = new Shortcut (Key.Empty, item.Title, null);
+                var newStatusItem = new Shortcut (item.Shortcut, item.Title, _frmStatusBarDetails.CreateAction (item));
 
                 return newStatusItem;
             }
@@ -599,8 +541,9 @@ public class DynamicStatusBar : Scenario
                 int index
             )
             {
-                _currentEditStatusItem = CreateNewStatusBar (statusItem);
-                //_statusBar.Items [index] = _currentEditStatusItem;
+                _statusBar.Subviews [index].Title = statusItem.Title;
+                ((Shortcut)_statusBar.Subviews [index]).Action = _frmStatusBarDetails.CreateAction (statusItem);
+                ((Shortcut)_statusBar.Subviews [index]).Key = statusItem.Shortcut;
 
                 if (DataContext.Items.Count == 0)
                 {
@@ -624,23 +567,9 @@ public class DynamicStatusBar : Scenario
 
         public DynamicStatusItemModel DataContext { get; set; }
 
-        public static string SetTitleText (string title, string shortcut)
-        {
-            string txt = title;
-            string [] split = title.Split ('~');
 
-            if (split.Length > 1)
-            {
-                txt = split [2].Trim ();
-            }
 
-            if (string.IsNullOrEmpty (shortcut) || shortcut == "Null")
-            {
-                return txt;
-            }
 
-            return $"~{shortcut}~ {txt}";
-        }
     }
 
     public class DynamicStatusItem
@@ -662,7 +591,7 @@ public class DynamicStatusBar : Scenario
 
         public Shortcut Shortcut { get; set; }
         public string Title { get; set; }
-        public override string ToString () { return $"{Title}, {Shortcut}"; }
+        public override string ToString () { return $"{Title}, {Shortcut.Key}"; }
     }
 
     public class DynamicStatusItemModel : INotifyPropertyChanged

UICatalog/Scenarios/VkeyPacketSimulator.cs

@@ -119,7 +119,7 @@ public class VkeyPacketSimulator : Scenario
                                                                                         "Keys",
                                                                                         $"'{Key.ToString (
                                                                                                           e.KeyCode,
-                                                                                                          MenuBar.ShortcutDelimiter
+                                                                                                          Key.Separator
                                                                                                          )}' pressed!",
                                                                                         "Ok"
                                                                                        )

UICatalog/UICatalog.cs

@@ -676,7 +676,7 @@ public class UICatalogApp
 
             ColorScheme = Colors.ColorSchemes [_topLevelColorScheme];
 
-            MenuBar!.Menus [0].Children [0].Shortcut = (KeyCode)Application.QuitKey;
+            MenuBar!.Menus [0].Children [0].ShortcutKey = Application.QuitKey;
 
             if (StatusBar is { })
             {
@@ -700,8 +700,8 @@ public class UICatalogApp
             {
                 var item = new MenuItem
                 {
-                    Title = $"_{theme.Key}",
-                    Shortcut = (KeyCode)new Key ((KeyCode)((uint)KeyCode.D1 + schemeCount++))
+                    Title = theme.Key == "Dark" ? $"{theme.Key.Substring (0, 3)}_{theme.Key.Substring (3, 1)}" : $"_{theme.Key}",
+                    ShortcutKey = new Key ((KeyCode)((uint)KeyCode.D1 + schemeCount++))
                         .WithCtrl
                 };
                 item.CheckType |= MenuItemCheckStyle.Checked;
@@ -735,6 +735,7 @@ public class UICatalogApp
                                    ColorScheme = Colors.ColorSchemes [_topLevelColorScheme];
                                    Application.Top!.SetNeedsDisplay ();
                                };
+                item.ShortcutKey = ((Key)sc.Key [0].ToString ().ToLower ()).WithCtrl;
                 schemeMenuItems.Add (item);
             }
 
@@ -796,7 +797,7 @@ public class UICatalogApp
             {
                 var item = new MenuItem
                 {
-                    Title = GetDiagnosticsTitle (diag), Shortcut = (KeyCode)new Key (index.ToString () [0]).WithAlt
+                    Title = GetDiagnosticsTitle (diag), ShortcutKey = new Key (index.ToString () [0]).WithAlt
                 };
                 index++;
                 item.CheckType |= MenuItemCheckStyle.Checked;
@@ -951,9 +952,8 @@ public class UICatalogApp
             List<MenuItem> menuItems = new ();
             MiIsMenuBorderDisabled = new () { Title = "Disable Menu _Border" };
 
-            MiIsMenuBorderDisabled.Shortcut =
-                (KeyCode)new Key (MiIsMenuBorderDisabled!.Title!.Substring (14, 1) [0]).WithAlt
-                                                                                       .WithCtrl.NoShift;
+            MiIsMenuBorderDisabled.ShortcutKey =
+                new Key (MiIsMenuBorderDisabled!.Title!.Substring (14, 1) [0]).WithAlt.WithCtrl.NoShift;
             MiIsMenuBorderDisabled.CheckType |= MenuItemCheckStyle.Checked;
 
             MiIsMenuBorderDisabled.Action += () =>
@@ -974,8 +974,8 @@ public class UICatalogApp
             List<MenuItem> menuItems = new ();
             MiIsMouseDisabled = new () { Title = "_Disable Mouse" };
 
-            MiIsMouseDisabled.Shortcut =
-                (KeyCode)new Key (MiIsMouseDisabled!.Title!.Substring (1, 1) [0]).WithAlt.WithCtrl.NoShift;
+            MiIsMouseDisabled.ShortcutKey =
+                new Key (MiIsMouseDisabled!.Title!.Substring (1, 1) [0]).WithAlt.WithCtrl.NoShift;
             MiIsMouseDisabled.CheckType |= MenuItemCheckStyle.Checked;
 
             MiIsMouseDisabled.Action += () =>
@@ -994,7 +994,7 @@ public class UICatalogApp
             List<MenuItem> menuItems = new ();
             MiUseSubMenusSingleFrame = new () { Title = "Enable _Sub-Menus Single Frame" };
 
-            MiUseSubMenusSingleFrame.Shortcut = KeyCode.CtrlMask
+            MiUseSubMenusSingleFrame.ShortcutKey = KeyCode.CtrlMask
                                                 | KeyCode.AltMask
                                                 | (KeyCode)MiUseSubMenusSingleFrame!.Title!.Substring (8, 1) [
                                                  0];
@@ -1017,7 +1017,7 @@ public class UICatalogApp
             MiForce16Colors = new ()
             {
                 Title = "Force _16 Colors",
-                Shortcut = (KeyCode)Key.F6,
+                ShortcutKey = Key.F6,
                 Checked = Application.Force16Colors,
                 CanExecute = () => Application.Driver?.SupportsTrueColor ?? false
             };

UnitTests/Configuration/SerializableConfigurationPropertyTests.cs

@@ -41,17 +41,11 @@ public class SerializableConfigurationPropertyTests
         }
 
         // Ensure no property has the generic JsonStringEnumConverter<>
-        foreach (var property in properties)
-        {
-            var jsonConverterAttributes = property.GetCustomAttributes (typeof (JsonConverterAttribute), false)
-                .Cast<JsonConverterAttribute> ();
-
-            foreach (var attribute in jsonConverterAttributes)
-            {
-                Assert.False (attribute.ConverterType!.IsGenericType &&
-                             attribute.ConverterType.GetGenericTypeDefinition () == typeof (JsonStringEnumConverter<>));
-            }
-        }
+        EnsureNoSpecifiedConverters (properties, new [] { typeof (JsonStringEnumConverter<>) });
+        // Ensure no property has the type RuneJsonConverter
+        EnsureNoSpecifiedConverters (properties, new [] { typeof (RuneJsonConverter) });
+        // Ensure no property has the type KeyJsonConverter
+        EnsureNoSpecifiedConverters (properties, new [] { typeof (KeyJsonConverter) });
 
         // Find all classes with the JsonConverter attribute of type ScopeJsonConverter<>
         var classesWithScopeJsonConverter = types.Where (t =>
@@ -66,7 +60,7 @@ public class SerializableConfigurationPropertyTests
         }
     }
 
-    private IEnumerable<Type> GetRegisteredTypes (Type contextType)
+    private static IEnumerable<Type> GetRegisteredTypes (Type contextType)
     {
         // Use reflection to find which types are registered in the JsonSerializerContext
         var registeredTypes = new List<Type> ();
@@ -83,4 +77,32 @@ public class SerializableConfigurationPropertyTests
 
         return registeredTypes.Distinct ();
     }
+
+    private static void EnsureNoSpecifiedConverters (List<PropertyInfo> properties, IEnumerable<Type> converterTypes)
+    {
+        // Ensure no property has any of the specified converter types
+        foreach (var property in properties)
+        {
+            var jsonConverterAttributes = property.GetCustomAttributes (typeof (JsonConverterAttribute), false)
+                                                  .Cast<JsonConverterAttribute> ();
+
+            foreach (var attribute in jsonConverterAttributes)
+            {
+                foreach (var converterType in converterTypes)
+                {
+                    if (attribute.ConverterType!.IsGenericType &&
+                        attribute.ConverterType.GetGenericTypeDefinition () == converterType)
+                    {
+                        Assert.Fail ($"Property '{property.Name}' should not use the converter '{converterType.Name}'.");
+                    }
+
+                    if (!attribute.ConverterType!.IsGenericType &&
+                        attribute.ConverterType == converterType)
+                    {
+                        Assert.Fail ($"Property '{property.Name}' should not use the converter '{converterType.Name}'.");
+                    }
+                }
+            }
+        }
+    }
 }

UnitTests/Input/KeyTests.cs

@@ -533,7 +533,7 @@ public class KeyTests
         var b = Key.A;
         Assert.True (a.Equals (b));
     }
-    
+
     [Fact]
     public void Equals_Handled_Changed_ShouldReturnTrue_WhenEqual ()
     {
@@ -552,4 +552,20 @@ public class KeyTests
         var b = Key.A;
         Assert.False (a.Equals (b));
     }
+
+    [Fact]
+    public void Set_Key_Separator_With_Rune_Default_Ensure_Using_The_Default_Plus ()
+    {
+        Key key = new (Key.A.WithCtrl);
+        Assert.Equal ((Rune)'+', Key.Separator);
+        Assert.Equal ("Ctrl+A", key.ToString ());
+
+        Key.Separator = new ('-');
+        Assert.Equal ((Rune)'-', Key.Separator);
+        Assert.Equal ("Ctrl-A", key.ToString ());
+
+        Key.Separator = new ();
+        Assert.Equal ((Rune)'+', Key.Separator);
+        Assert.Equal ("Ctrl+A", key.ToString ());
+    }
 }

UnitTests/Views/MenuBarTests.cs

@@ -1,4 +1,5 @@
-using Xunit.Abstractions;
+using System.Text;
+using Xunit.Abstractions;
 
 namespace Terminal.Gui.ViewsTests;
 
@@ -217,7 +218,7 @@ public class MenuBarTests (ITestOutputHelper output)
         Assert.Null (menuBarItem.Action);
         Assert.Null (menuBarItem.CanExecute);
         Assert.Null (menuBarItem.Parent);
-        Assert.Equal (KeyCode.Null, menuBarItem.Shortcut);
+        Assert.Equal (Key.Empty, menuBarItem.ShortcutKey);
     }
 
     [Fact]
@@ -1229,7 +1230,7 @@ wo
                 )]
     [InlineData ("Closed", "None", "About", KeyCode.F9, KeyCode.CursorRight, KeyCode.CursorRight, KeyCode.Enter)]
 
-    // Hotkeys
+    //// Hotkeys
     [InlineData ("_File", "_New", "", KeyCode.AltMask | KeyCode.F)]
     [InlineData ("Closed", "None", "", KeyCode.AltMask | KeyCode.ShiftMask | KeyCode.F)]
     [InlineData ("Closed", "None", "", KeyCode.AltMask | KeyCode.F, KeyCode.Esc)]
@@ -1245,9 +1246,10 @@ wo
     [InlineData ("_Edit", "_1st", "", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3)]
     [InlineData ("Closed", "None", "1", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3, KeyCode.D1)]
     [InlineData ("Closed", "None", "1", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3, KeyCode.Enter)]
-    [InlineData ("_Edit", "_3rd Level", "", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3, KeyCode.D4)]
+    [InlineData ("Closed", "None", "2", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3, KeyCode.D2)]
+    [InlineData ("_Edit", "_5th", "", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D3, KeyCode.D4)]
     [InlineData ("Closed", "None", "5", KeyCode.AltMask | KeyCode.E, KeyCode.F, KeyCode.D4, KeyCode.D5)]
-    [InlineData ("_About", "_About", "", KeyCode.AltMask | KeyCode.A)]
+    [InlineData ("Closed", "None", "About", KeyCode.AltMask | KeyCode.A)]
     public void KeyBindings_Navigation_Commands (
         string expectedBarTitle,
         string expectedItemTitle,
@@ -1283,7 +1285,7 @@ wo
         top.Add (menu);
         Application.Begin (top);
 
-        foreach (KeyCode key in keys)
+        foreach (Key key in keys)
         {
             top.NewKeyDownEvent (new (key));
             Application.MainLoop.RunIteration ();
@@ -3725,4 +3727,87 @@ Edit
         Assert.True (btnClicked);
         top.Dispose ();
     }
+
+    [Fact]
+    public void Update_ShortcutKey_KeyBindings_Old_ShortcutKey_Is_Removed ()
+    {
+        var menuBar = new MenuBar ()
+        {
+            Menus =
+            [
+                new MenuBarItem (
+                                 "_File",
+                                 new MenuItem []
+                                 {
+                                     new MenuItem ("New", "Create New", null, null, null, Key.A.WithCtrl)
+                                 }
+                                )
+            ]
+        };
+
+        Assert.Contains (Key.A.WithCtrl, menuBar.KeyBindings.Bindings);
+
+        menuBar.Menus [0].Children [0].ShortcutKey = Key.B.WithCtrl;
+
+        Assert.DoesNotContain (Key.A.WithCtrl, menuBar.KeyBindings.Bindings);
+        Assert.Contains (Key.B.WithCtrl, menuBar.KeyBindings.Bindings);
+    }
+
+    [Fact]
+    public void SetMenus_With_Same_HotKey_Does_Not_Throws ()
+    {
+        var mb = new MenuBar ();
+
+        var i1 = new MenuBarItem ("_heey", "fff", () => { }, () => true);
+
+        mb.Menus = new MenuBarItem [] { i1 };
+        mb.Menus = new MenuBarItem [] { i1 };
+
+        Assert.Equal (Key.H, mb.Menus [0].HotKey);
+    }
+
+    [Fact]
+    [AutoInitShutdown]
+    public void AddMenuBarItem_RemoveMenuItem_Dynamically ()
+    {
+        var menuBar = new MenuBar ();
+        var menuBarItem = new MenuBarItem { Title = "_New" };
+        var action = "";
+        var menuItem = new MenuItem { Title = "_Item", Action = () => action = "I", Parent = menuBarItem };
+        Assert.Equal ("n", menuBarItem.HotKey);
+        Assert.Equal ("i", menuItem.HotKey);
+        Assert.Empty (menuBar.Menus);
+        menuBarItem.AddMenuBarItem (menuItem);
+        menuBar.Menus = [menuBarItem];
+        Assert.Single (menuBar.Menus);
+        Assert.Single (menuBar.Menus [0].Children);
+        Assert.Contains (Key.N.WithAlt, menuBar.KeyBindings.Bindings);
+        Assert.DoesNotContain (Key.I, menuBar.KeyBindings.Bindings);
+
+        var top = new Toplevel ();
+        top.Add (menuBar);
+        Application.Begin (top);
+
+        top.NewKeyDownEvent (Key.N.WithAlt);
+        Application.MainLoop.RunIteration ();
+        Assert.True (menuBar.IsMenuOpen);
+        Assert.Equal ("", action);
+
+        top.NewKeyDownEvent (Key.I);
+        Application.MainLoop.RunIteration ();
+        Assert.False (menuBar.IsMenuOpen);
+        Assert.Equal ("I", action);
+
+        menuItem.RemoveMenuItem ();
+        Assert.Single (menuBar.Menus);
+        Assert.Equal (null, menuBar.Menus [0].Children);
+        Assert.Contains (Key.N.WithAlt, menuBar.KeyBindings.Bindings);
+        Assert.DoesNotContain (Key.I, menuBar.KeyBindings.Bindings);
+
+        menuBarItem.RemoveMenuItem ();
+        Assert.Empty (menuBar.Menus);
+        Assert.DoesNotContain (Key.N.WithAlt, menuBar.KeyBindings.Bindings);
+
+        top.Dispose ();
+    }
 }

UnitTests/Views/MenuTests.cs

@@ -31,7 +31,7 @@ public class MenuTests
         Assert.Null (menuItem.Action);
         Assert.Null (menuItem.CanExecute);
         Assert.Null (menuItem.Parent);
-        Assert.Equal (KeyCode.Null, menuItem.Shortcut);
+        Assert.Equal (Key.Empty, menuItem.ShortcutKey);
 
         menuItem = new MenuItem ("Test", "Help", Run, () => { return true; }, new MenuItem (), KeyCode.F1);
         Assert.Equal ("Test", menuItem.Title);
@@ -39,7 +39,7 @@ public class MenuTests
         Assert.Equal (Run, menuItem.Action);
         Assert.NotNull (menuItem.CanExecute);
         Assert.NotNull (menuItem.Parent);
-        Assert.Equal (KeyCode.F1, menuItem.Shortcut);
+        Assert.Equal (KeyCode.F1, menuItem.ShortcutKey);
 
         void Run () { }
     }