From 552f9ed0a828ca72ff1703a4a28066a991f24f42 Mon Sep 17 00:00:00 2001 From: BDisp Date: Sat, 31 Aug 2024 22:00:05 +0100 Subject: [PATCH] Fix some bug, clean code and more unit tests. --- Terminal.Gui/Views/Menu/Menu.cs | 28 ++--- Terminal.Gui/Views/Menu/MenuBar.cs | 138 ++++++++++++------------- Terminal.Gui/Views/Menu/MenuBarItem.cs | 32 +++--- Terminal.Gui/Views/Menu/MenuItem.cs | 44 ++++---- Terminal.sln.DotSettings | 1 + UICatalog/Scenarios/ContextMenus.cs | 4 +- UnitTests/Views/MenuBarTests.cs | 4 +- UnitTests/Views/MenuTests.cs | 64 ++++++++++++ 8 files changed, 187 insertions(+), 128 deletions(-) diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs index fe3af3b58..f596c114b 100644 --- a/Terminal.Gui/Views/Menu/Menu.cs +++ b/Terminal.Gui/Views/Menu/Menu.cs @@ -49,7 +49,7 @@ internal sealed class Menu : View } } - internal required MenuBarItem BarItems + internal required MenuBarItem? BarItems { get => _barItems!; init @@ -83,8 +83,8 @@ internal sealed class Menu : View { KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuItem); // Remove an existent ShortcutKey - menuItem._menuBar.KeyBindings.Remove (menuItem.ShortcutKey); - menuItem._menuBar.KeyBindings.Add (menuItem.ShortcutKey, keyBinding); + menuItem._menuBar.KeyBindings.Remove (menuItem.ShortcutKey!); + menuItem._menuBar.KeyBindings.Add (menuItem.ShortcutKey!, keyBinding); } } } @@ -213,9 +213,9 @@ internal sealed class Menu : View if (menuItem.HotKey != Key.Empty) { - KeyBindings.Remove (menuItem.HotKey); - KeyBindings.Add (menuItem.HotKey, keyBinding); - KeyBindings.Remove (menuItem.HotKey.WithAlt); + KeyBindings.Remove (menuItem.HotKey!); + KeyBindings.Add (menuItem.HotKey!, keyBinding); + KeyBindings.Remove (menuItem.HotKey!.WithAlt); KeyBindings.Add (menuItem.HotKey.WithAlt, keyBinding); } } @@ -233,8 +233,8 @@ internal sealed class Menu : View { if (menuItem.HotKey != Key.Empty) { - KeyBindings.Remove (menuItem.HotKey); - KeyBindings.Remove (menuItem.HotKey.WithAlt); + KeyBindings.Remove (menuItem.HotKey!); + KeyBindings.Remove (menuItem.HotKey!.WithAlt); } } } @@ -665,19 +665,19 @@ internal sealed class Menu : View { _currentChild++; - if (_currentChild >= _barItems.Children!.Length) + if (_currentChild >= _barItems?.Children?.Length) { _currentChild = 0; } - if (this != _host.OpenCurrentMenu && _barItems.Children [_currentChild]?.IsFromSubMenu == true && _host._selectedSub > -1) + if (this != _host.OpenCurrentMenu && _barItems?.Children? [_currentChild].IsFromSubMenu == true && _host._selectedSub > -1) { _host.PreviousMenu (true); _host.SelectEnabledItem (_barItems.Children, _currentChild, out _currentChild); _host.OpenCurrentMenu = this; } - MenuItem item = _barItems.Children [_currentChild]; + MenuItem? item = _barItems?.Children? [_currentChild]; if (item?.IsEnabled () != true) { @@ -689,7 +689,7 @@ internal sealed class Menu : View } if (_host is { UseSubMenusSingleFrame: false, UseKeysUpDownAsKeysLeftRight: true } - && _barItems.SubMenu (_barItems.Children [_currentChild]) != null! + && _barItems?.SubMenu (_barItems?.Children? [_currentChild]!) != null && !disabled && _host.IsMenuOpen) { @@ -706,7 +706,7 @@ internal sealed class Menu : View _host.OpenMenu (_host._selected); } } - while (_barItems.Children? [_currentChild] is null || disabled); + while (_barItems?.Children? [_currentChild] is null || disabled); SetNeedsDisplay (); SetParentSetNeedsDisplay (); @@ -913,7 +913,7 @@ internal sealed class Menu : View return true; } - MenuBarItem subMenu = _barItems.SubMenu (_barItems.Children [_currentChild]); + MenuBarItem? subMenu = _barItems.SubMenu (_barItems.Children [_currentChild]); // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract if (subMenu is { }) diff --git a/Terminal.Gui/Views/Menu/MenuBar.cs b/Terminal.Gui/Views/Menu/MenuBar.cs index 35cdfa326..1415c2d76 100644 --- a/Terminal.Gui/Views/Menu/MenuBar.cs +++ b/Terminal.Gui/Views/Menu/MenuBar.cs @@ -60,8 +60,8 @@ public class MenuBar : View, IDesignable private bool _initialCanFocus; private bool _isCleaning; private View? _lastFocused; - private Menu _ocm; - private View _previousFocused; + private Menu? _ocm; + private View? _previousFocused; private bool _reopen; private bool _useSubMenusSingleFrame; @@ -78,7 +78,9 @@ public class MenuBar : View, IDesignable //CanFocus = true; _selected = -1; _selectedSub = -1; + // ReSharper disable once VirtualMemberCallInConstructor ColorScheme = Colors.ColorSchemes ["Menu"]; + // ReSharper disable once VirtualMemberCallInConstructor WantMousePositionReports = true; IsMenuOpen = false; @@ -180,26 +182,26 @@ public class MenuBar : View, IDesignable { MenuBarItem menuBarItem = Menus [i]; - if (menuBarItem?.HotKey != Key.Empty) + if (menuBarItem.HotKey != Key.Empty) { - KeyBindings.Remove (menuBarItem!.HotKey); + KeyBindings.Remove (menuBarItem.HotKey!); KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.Focused, menuBarItem); - KeyBindings.Add (menuBarItem!.HotKey, keyBinding); - KeyBindings.Remove (menuBarItem.HotKey.WithAlt); + 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?.ShortcutKey != Key.Empty) + if (menuBarItem.ShortcutKey != Key.Empty) { // Technically this will never run because MenuBarItems don't have shortcuts // unless the IsTopLevel is true - KeyBindings.Remove (menuBarItem.ShortcutKey); + KeyBindings.Remove (menuBarItem.ShortcutKey!); KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, menuBarItem); - KeyBindings.Add (menuBarItem.ShortcutKey, keyBinding); + KeyBindings.Add (menuBarItem.ShortcutKey!, keyBinding); } - menuBarItem?.AddShortcutKeyBindings (this); + menuBarItem.AddShortcutKeyBindings (this); } } } @@ -257,9 +259,9 @@ public class MenuBar : View, IDesignable { if (_ocm != value) { - _ocm = value; + _ocm = value!; - if (_ocm is { } && _ocm._currentChild > -1) + if (_ocm is { _currentChild: > -1 }) { OnMenuOpened (); } @@ -271,16 +273,16 @@ public class MenuBar : View, IDesignable public bool CloseMenu (bool ignoreUseSubMenusSingleFrame = false) { return CloseMenu (false, false, ignoreUseSubMenusSingleFrame); } /// Raised when all the menu is closed. - public event EventHandler MenuAllClosed; + public event EventHandler? MenuAllClosed; /// Raised when a menu is closing passing . - public event EventHandler MenuClosing; + public event EventHandler? MenuClosing; /// Raised when a menu is opened. - public event EventHandler MenuOpened; + public event EventHandler? MenuOpened; /// Raised as a menu is opening. - public event EventHandler MenuOpening; + public event EventHandler? MenuOpening; /// public override void OnDrawContent (Rectangle viewport) @@ -344,25 +346,24 @@ public class MenuBar : View, IDesignable /// Virtual method that will invoke the event if it's defined. public virtual void OnMenuOpened () { - MenuItem mi = null; - MenuBarItem parent; + MenuItem? mi; + MenuBarItem? parent; - if (OpenCurrentMenu.BarItems.Children != null - && OpenCurrentMenu.BarItems!.Children.Length > 0 + if (OpenCurrentMenu?.BarItems?.Children is { Length: > 0 } && OpenCurrentMenu?._currentChild > -1) { parent = OpenCurrentMenu.BarItems; mi = parent.Children [OpenCurrentMenu._currentChild]; } - else if (OpenCurrentMenu!.BarItems.IsTopLevel) + else if (OpenCurrentMenu!.BarItems!.IsTopLevel) { parent = null; mi = OpenCurrentMenu.BarItems; } else { - parent = _openMenu.BarItems; - mi = parent.Children?.Length > 0 ? parent.Children [_openMenu._currentChild] : null; + parent = _openMenu?.BarItems; + mi = parent?.Children?.Length > 0 ? parent.Children [_openMenu!._currentChild] : null; } MenuOpened?.Invoke (this, new (parent, mi)); @@ -398,11 +399,11 @@ public class MenuBar : View, IDesignable OpenMenu (_selected); if (!SelectEnabledItem ( - OpenCurrentMenu!.BarItems.Children, - OpenCurrentMenu._currentChild, + OpenCurrentMenu?.BarItems?.Children, + OpenCurrentMenu!._currentChild, out OpenCurrentMenu._currentChild ) - && !CloseMenu (false)) + && !CloseMenu ()) { return; } @@ -448,14 +449,14 @@ public class MenuBar : View, IDesignable // Activates the menu, handles either first focus, or activating an entry when it was already active // For mouse events. - internal void Activate (int idx, int sIdx = -1, MenuBarItem subMenu = null) + internal void Activate (int idx, int sIdx = -1, MenuBarItem? subMenu = null!) { _selected = idx; _selectedSub = sIdx; if (_openMenu is null) { - _previousFocused = SuperView is null ? Application.Current?.Focused ?? null : SuperView.Focused; + _previousFocused = (SuperView is null ? Application.Current?.Focused ?? null : SuperView.Focused)!; } OpenMenu (idx, sIdx, subMenu); @@ -505,7 +506,7 @@ public class MenuBar : View, IDesignable return; } - if (!CloseMenu (false)) + if (!CloseMenu ()) { return; } @@ -530,8 +531,8 @@ public class MenuBar : View, IDesignable if (Application.Current is { }) { // Close others menu bar opened - View? cm = Application.Current!.Subviews.FirstOrDefault (v => v is Menu cm && cm.Host != this && cm.Host.IsMenuOpen); - ((Menu)cm!)?.Host.CleanUp (); + View? cm = Application.Current.Subviews.FirstOrDefault (v => v is Menu cm && cm.Host != this && cm.Host.IsMenuOpen); + (cm as Menu)?.Host.CleanUp (); } } @@ -546,7 +547,7 @@ public class MenuBar : View, IDesignable _isMenuClosing = true; _reopen = reopen; - MenuClosingEventArgs args = OnMenuClosing (mbi, reopen, isSubMenu); + MenuClosingEventArgs args = OnMenuClosing (mbi!, reopen, isSubMenu); if (args.Cancel) { @@ -571,7 +572,7 @@ public class MenuBar : View, IDesignable SetNeedsDisplay (); - if (_previousFocused is Menu && _openMenu is { } && _previousFocused.ToString () != OpenCurrentMenu.ToString ()) + if (_previousFocused is Menu && _openMenu is { } && _previousFocused.ToString () != OpenCurrentMenu!.ToString ()) { _previousFocused.SetFocus (); } @@ -625,7 +626,7 @@ public class MenuBar : View, IDesignable _selectedSub = -1; SetNeedsDisplay (); RemoveAllOpensSubMenus (); - OpenCurrentMenu!._previousSubFocused.SetFocus (); + OpenCurrentMenu!._previousSubFocused!.SetFocus (); _openSubMenu = null; IsMenuOpen = true; @@ -642,6 +643,7 @@ public class MenuBar : View, IDesignable /// The location offset. internal Point GetScreenOffset () { + // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract if (Driver is null) { return Point.Empty; @@ -656,7 +658,7 @@ public class MenuBar : View, IDesignable return Point.Empty; } - Point viewportOffset = sv?.GetViewportOffsetFromFrame () ?? Point.Empty; + Point viewportOffset = sv.GetViewportOffsetFromFrame (); return new ( superViewFrame.X - sv.Frame.X - viewportOffset.X, @@ -690,8 +692,8 @@ public class MenuBar : View, IDesignable OpenMenu (_selected); SelectEnabledItem ( - OpenCurrentMenu!.BarItems.Children, - OpenCurrentMenu._currentChild, + OpenCurrentMenu?.BarItems?.Children, + OpenCurrentMenu!._currentChild, out OpenCurrentMenu._currentChild ); @@ -706,9 +708,9 @@ public class MenuBar : View, IDesignable } else { - MenuBarItem? subMenu = OpenCurrentMenu!._currentChild > -1 && OpenCurrentMenu.BarItems.Children.Length > 0 + MenuBarItem? subMenu = OpenCurrentMenu!._currentChild > -1 && OpenCurrentMenu.BarItems?.Children!.Length > 0 ? OpenCurrentMenu.BarItems.SubMenu ( - OpenCurrentMenu.BarItems.Children [OpenCurrentMenu._currentChild] + OpenCurrentMenu.BarItems.Children? [OpenCurrentMenu._currentChild]! ) : null; @@ -723,8 +725,8 @@ public class MenuBar : View, IDesignable } else if (subMenu != null || (OpenCurrentMenu._currentChild > -1 - && !OpenCurrentMenu.BarItems - .Children [OpenCurrentMenu._currentChild] + && !OpenCurrentMenu.BarItems! + .Children! [OpenCurrentMenu._currentChild] .IsFromSubMenu)) { _selectedSub++; @@ -752,7 +754,7 @@ public class MenuBar : View, IDesignable } } - internal void OpenMenu (int index, int sIndex = -1, MenuBarItem subMenu = null) + internal void OpenMenu (int index, int sIndex = -1, MenuBarItem? subMenu = null!) { _isMenuOpening = true; MenuOpeningEventArgs newMenu = OnMenuOpening (Menus [index]); @@ -790,7 +792,7 @@ public class MenuBar : View, IDesignable } // This positions the submenu horizontally aligned with the first character of the - // text belonging to the menu + // text belonging to the menu for (var i = 0; i < index; i++) { pos += Menus [i].TitleLength + (Menus [i].Help.GetColumns () > 0 ? Menus [i].Help.GetColumns () + 2 : 0) + _leftPadding + _rightPadding; @@ -856,7 +858,7 @@ public class MenuBar : View, IDesignable OpenCurrentMenu = new () { Host = this, - X = last.Frame.Left + last.Frame.Width + locationOffset.X, + X = last!.Frame.Left + last.Frame.Width + locationOffset.X, Y = last.Frame.Top + locationOffset.Y + last._currentChild, BarItems = subMenu, Parent = last @@ -867,7 +869,7 @@ public class MenuBar : View, IDesignable Menu? first = _openSubMenu.Count > 0 ? _openSubMenu.First () : _openMenu; // 2 is for the parent and the separator - MenuItem? [] mbi = new MenuItem [2 + subMenu.Children.Length]; + MenuItem? [] mbi = new MenuItem [2 + subMenu.Children!.Length]; mbi [0] = new () { Title = subMenu.Title, Parent = subMenu }; mbi [1] = null; @@ -876,13 +878,13 @@ public class MenuBar : View, IDesignable mbi [j + 2] = subMenu.Children [j]; } - var newSubMenu = new MenuBarItem (mbi) { Parent = subMenu }; + var newSubMenu = new MenuBarItem (mbi!) { Parent = subMenu }; OpenCurrentMenu = new () { Host = this, X = first!.Frame.Left, Y = first.Frame.Top, BarItems = newSubMenu }; - last.Visible = false; + last!.Visible = false; Application.GrabMouse (OpenCurrentMenu); } @@ -902,7 +904,7 @@ public class MenuBar : View, IDesignable if (_selectedSub > -1 && SelectEnabledItem ( - OpenCurrentMenu!.BarItems.Children, + OpenCurrentMenu!.BarItems!.Children, OpenCurrentMenu._currentChild, out OpenCurrentMenu._currentChild )) @@ -939,8 +941,8 @@ public class MenuBar : View, IDesignable OpenMenu (_selected); if (!SelectEnabledItem ( - OpenCurrentMenu!.BarItems.Children, - OpenCurrentMenu._currentChild, + OpenCurrentMenu?.BarItems?.Children, + OpenCurrentMenu!._currentChild, out OpenCurrentMenu._currentChild, false )) @@ -977,7 +979,7 @@ public class MenuBar : View, IDesignable } } - internal bool Run (Action action) + internal bool Run (Action? action) { if (action is null) { @@ -1042,6 +1044,7 @@ public class MenuBar : View, IDesignable } } + // ReSharper disable once ConditionIsAlwaysTrueOrFalseAccordingToNullableAPIContract if (child is null || !child.IsEnabled ()) { if (forward) @@ -1068,7 +1071,7 @@ public class MenuBar : View, IDesignable /// Called when an item is selected; Runs the action. /// - internal bool SelectItem (MenuItem item) + internal bool SelectItem (MenuItem? item) { if (item?.Action is null) { @@ -1080,12 +1083,12 @@ public class MenuBar : View, IDesignable Application.Refresh (); _openedByAltKey = true; - return Run (item?.Action); + return Run (item.Action); } private void CloseMenuBar () { - if (!CloseMenu (false)) + if (!CloseMenu ()) { return; } @@ -1156,11 +1159,11 @@ public class MenuBar : View, IDesignable OpenMenu (i); if (!SelectEnabledItem ( - OpenCurrentMenu!.BarItems.Children, - OpenCurrentMenu._currentChild, + OpenCurrentMenu?.BarItems?.Children, + OpenCurrentMenu!._currentChild, out OpenCurrentMenu._currentChild ) - && !CloseMenu (false)) + && !CloseMenu ()) { return; } @@ -1189,7 +1192,7 @@ public class MenuBar : View, IDesignable _isMenuClosing = true; Menu? menu; - if (_openSubMenu.Count - 1 > 0) + if (_openSubMenu!.Count - 1 > 0) { menu = _openSubMenu [i - 1]; } @@ -1315,11 +1318,6 @@ public class MenuBar : View, IDesignable { MenuBarItem open = Menus [i]; - if (open is null) - { - continue; - } - if (open == OpenCurrentMenu!.BarItems && i == index) { CloseAllMenus (); @@ -1348,15 +1346,12 @@ public class MenuBar : View, IDesignable #region Mouse Handling - /// internal void LostFocus (View view) { - if (((!(view is MenuBar) && !(view is Menu))) && !_isCleaning && !_reopen) + if (view is not MenuBar && view is not Menu && !_isCleaning && !_reopen) { CleanUp (); } - - return; } /// @@ -1437,8 +1432,7 @@ public class MenuBar : View, IDesignable { if (!UseSubMenusSingleFrame || (UseSubMenusSingleFrame - && OpenCurrentMenu != null - && OpenCurrentMenu.BarItems.Parent != null + && OpenCurrentMenu is { BarItems.Parent: { } } && OpenCurrentMenu.BarItems.Parent.Parent != Menus [i])) { Activate (i); @@ -1467,7 +1461,7 @@ public class MenuBar : View, IDesignable internal bool _handled; internal bool _isContextMenuLoading; - private MenuBarItem [] _menus; + private MenuBarItem [] _menus = []; internal bool HandleGrabView (MouseEvent me, View current) { @@ -1614,7 +1608,7 @@ public class MenuBar : View, IDesignable { if (context is not Func actionFn) { - actionFn = (s) => true; + actionFn = (_) => true; } Menus = @@ -1647,7 +1641,9 @@ public class MenuBar : View, IDesignable null, KeyCode.CtrlMask | KeyCode.S ), +#pragma warning disable CS8625 // Cannot convert null literal to non-nullable reference type. null, +#pragma warning restore CS8625 // Cannot convert null literal to non-nullable reference type. // Don't use Application.Quit so we can disambiguate between quitting and closing the toplevel new ( diff --git a/Terminal.Gui/Views/Menu/MenuBarItem.cs b/Terminal.Gui/Views/Menu/MenuBarItem.cs index 3c9b0dab8..727e0df54 100644 --- a/Terminal.Gui/Views/Menu/MenuBarItem.cs +++ b/Terminal.Gui/Views/Menu/MenuBarItem.cs @@ -89,7 +89,7 @@ public class MenuBarItem : MenuItem /// Check if a is a . /// /// Returns a or null otherwise. - public MenuBarItem SubMenu (MenuItem menuItem) { return (menuItem as MenuBarItem)!; } + public MenuBarItem? SubMenu (MenuItem menuItem) { return menuItem as MenuBarItem; } internal void AddShortcutKeyBindings (MenuBar menuBar) { @@ -108,13 +108,14 @@ public class MenuBarItem : MenuItem if (menuItem.ShortcutKey != Key.Empty) { menuItem._menuBar = menuBar; - menuItem.UpdateShortcutKeyBinding (menuItem._menuBar, Key.Empty); + menuItem.UpdateShortcutKeyBinding (Key.Empty); } SubMenu (menuItem)?.AddShortcutKeyBindings (menuBar); } } + // ReSharper disable once ParameterOnlyUsedForPreconditionCheck.Local private void SetInitialProperties (string title, object? children, MenuItem? parent = null, bool isTopLevel = false) { if (!isTopLevel && children is null) @@ -125,7 +126,7 @@ public class MenuBarItem : MenuItem ); } - SetTitle (title ?? ""); + Title = title; if (parent is { }) { @@ -175,18 +176,17 @@ public class MenuBarItem : MenuItem } } - private void SetTitle (string title) - { - title ??= string.Empty; - Title = title; - } - /// /// Add a dynamically into the .Menus. /// + /// /// - public void AddMenuBarItem (MenuItem? menuItem = null) + public void AddMenuBarItem (MenuBar menuBar, MenuItem? menuItem = null) { + ArgumentNullException.ThrowIfNull (menuBar); + + _menuBar = menuBar; + if (menuItem is null) { MenuBarItem [] menus = _menuBar.Menus; @@ -208,12 +208,12 @@ public class MenuBarItem : MenuItem { if (Children is { }) { - foreach (MenuItem menuItem in Children) + foreach (MenuItem? menuItem in Children) { if (menuItem.ShortcutKey != Key.Empty) { // Remove an existent ShortcutKey - _menuBar?.KeyBindings.Remove (menuItem.ShortcutKey); + _menuBar?.KeyBindings.Remove (menuItem.ShortcutKey!); } } } @@ -221,19 +221,19 @@ public class MenuBarItem : MenuItem if (ShortcutKey != Key.Empty) { // Remove an existent ShortcutKey - _menuBar?.KeyBindings.Remove (ShortcutKey); + _menuBar?.KeyBindings.Remove (ShortcutKey!); } var index = _menuBar!.Menus.IndexOf (this); if (index > -1) { - if (_menuBar!.Menus [index].HotKey != Key.Empty) + if (_menuBar.Menus [index].HotKey != Key.Empty) { // Remove an existent HotKey - _menuBar?.KeyBindings.Remove (HotKey.WithAlt); + _menuBar.KeyBindings.Remove (HotKey!.WithAlt); } - _menuBar!.Menus [index] = null!; + _menuBar.Menus [index] = null!; } var i = 0; diff --git a/Terminal.Gui/Views/Menu/MenuItem.cs b/Terminal.Gui/Views/Menu/MenuItem.cs index 6ab6da4a2..0432823b9 100644 --- a/Terminal.Gui/Views/Menu/MenuItem.cs +++ b/Terminal.Gui/Views/Menu/MenuItem.cs @@ -21,8 +21,8 @@ public class MenuItem /// The of this menu item. /// The keystroke combination. public MenuItem ( - string title, - string help, + string? title, + string? help, Action? action, Func? canExecute = null, MenuItem? parent = null, @@ -50,7 +50,7 @@ public class MenuItem /// Gets or sets the action to be invoked when the menu item is triggered. /// Method to invoke. - public Action Action { get; set; } + public Action? Action { get; set; } /// /// Used only if is of type. If @@ -72,7 +72,7 @@ public class MenuItem /// returns the menu item will be enabled. Otherwise, it will be disabled. /// /// Function to determine if the action is can be executed or not. - public Func CanExecute { get; set; } + public Func? CanExecute { get; set; } /// /// Sets or gets whether the shows a check indicator or not. See @@ -188,7 +188,7 @@ public class MenuItem (Checked == true || CheckType.HasFlag (MenuItemCheckStyle.Checked) || CheckType.HasFlag (MenuItemCheckStyle.Radio) ? 2 : 0) - + // check glyph + space + + // check glyph + space (Help.GetColumns () > 0 ? 2 + Help.GetColumns () : 0) + // Two spaces before Help (ShortcutTag.GetColumns () > 0 @@ -219,12 +219,12 @@ public class MenuItem /// /// See also which enable global key-bindings to menu items. /// - public Key HotKey + public Key? HotKey { get => _hotKey; private set { - var oldKey = _hotKey ?? Key.Empty; + var oldKey = _hotKey; _hotKey = value ?? Key.Empty; UpdateHotKeyBinding (oldKey); } @@ -262,30 +262,30 @@ public class MenuItem /// text. See . /// /// - public Key ShortcutKey + public Key? ShortcutKey { get => _shortcutKey; set { - var oldKey = _shortcutKey ?? Key.Empty; + var oldKey = _shortcutKey; _shortcutKey = value ?? Key.Empty; - UpdateShortcutKeyBinding (_menuBar, oldKey); + UpdateShortcutKeyBinding (oldKey); } } /// Gets the text describing the keystroke combination defined by . - public string ShortcutTag => ShortcutKey != Key.Empty ? ShortcutKey.ToString () : string.Empty; + public string ShortcutTag => ShortcutKey != Key.Empty ? ShortcutKey!.ToString () : string.Empty; private void UpdateHotKeyBinding (Key oldKey) { - if (_menuBar is null || _menuBar?.IsInitialized == false) + if (_menuBar is null or { IsInitialized: false }) { return; } if (oldKey != Key.Empty) { - var index = _menuBar.Menus?.IndexOf (this); + var index = _menuBar.Menus.IndexOf (this); if (index > -1) { @@ -295,22 +295,20 @@ public class MenuItem if (HotKey != Key.Empty) { - var index = _menuBar.Menus?.IndexOf (this); + var index = _menuBar.Menus.IndexOf (this); if (index > -1) { - _menuBar.KeyBindings.Remove (HotKey.WithAlt); + _menuBar.KeyBindings.Remove (HotKey!.WithAlt); KeyBinding keyBinding = new ([Command.ToggleExpandCollapse], KeyBindingScope.HotKey, this); _menuBar.KeyBindings.Add (HotKey.WithAlt, keyBinding); } } } - internal void UpdateShortcutKeyBinding (MenuBar menuBar, Key oldKey) + internal void UpdateShortcutKeyBinding (Key oldKey) { - _menuBar ??= menuBar; - - if (_menuBar is null) + if (_menuBar is null or { IsInitialized: false }) { return; } @@ -324,8 +322,8 @@ public class MenuItem { KeyBinding keyBinding = new ([Command.Select], KeyBindingScope.HotKey, this); // Remove an existent ShortcutKey - _menuBar?.KeyBindings.Remove (ShortcutKey); - _menuBar?.KeyBindings.Add (ShortcutKey, keyBinding); + _menuBar.KeyBindings.Remove (ShortcutKey!); + _menuBar.KeyBindings.Add (ShortcutKey!, keyBinding); } } @@ -341,7 +339,7 @@ public class MenuItem MenuItem []? childrens = ((MenuBarItem)Parent).Children; var i = 0; - foreach (MenuItem c in childrens) + foreach (MenuItem c in childrens!) { if (c != this) { @@ -365,7 +363,7 @@ public class MenuItem if (ShortcutKey != Key.Empty) { // Remove an existent ShortcutKey - _menuBar?.KeyBindings.Remove (ShortcutKey); + _menuBar.KeyBindings.Remove (ShortcutKey!); } } } diff --git a/Terminal.sln.DotSettings b/Terminal.sln.DotSettings index 5c8380e3d..219c41cbe 100644 --- a/Terminal.sln.DotSettings +++ b/Terminal.sln.DotSettings @@ -400,6 +400,7 @@ True True True + True True True True diff --git a/UICatalog/Scenarios/ContextMenus.cs b/UICatalog/Scenarios/ContextMenus.cs index d9aeac1d8..477fa1198 100644 --- a/UICatalog/Scenarios/ContextMenus.cs +++ b/UICatalog/Scenarios/ContextMenus.cs @@ -182,7 +182,7 @@ public class ContextMenus : Scenario ) ), new MenuBarItem ( - "More options", + "M_ore options", new MenuItem [] { new ( @@ -220,7 +220,7 @@ public class ContextMenus : Scenario ), _miForceMinimumPosToZero = new ( - "ForceMinimumPosToZero", + "Fo_rceMinimumPosToZero", "", () => { diff --git a/UnitTests/Views/MenuBarTests.cs b/UnitTests/Views/MenuBarTests.cs index e069a652b..b9f69d65b 100644 --- a/UnitTests/Views/MenuBarTests.cs +++ b/UnitTests/Views/MenuBarTests.cs @@ -15,10 +15,10 @@ public class MenuBarTests (ITestOutputHelper output) Assert.Equal ("n", menuBarItem.HotKey); Assert.Equal ("i", menuItem.HotKey); Assert.Empty (menuBar.Menus); - menuBarItem.AddMenuBarItem (menuItem); + menuBarItem.AddMenuBarItem (menuBar, menuItem); menuBar.Menus = [menuBarItem]; Assert.Single (menuBar.Menus); - Assert.Single (menuBar.Menus [0].Children); + Assert.Single (menuBar.Menus [0].Children!); Assert.Contains (Key.N.WithAlt, menuBar.KeyBindings.Bindings); Assert.DoesNotContain (Key.I, menuBar.KeyBindings.Bindings); diff --git a/UnitTests/Views/MenuTests.cs b/UnitTests/Views/MenuTests.cs index 05cda3349..d00c4a375 100644 --- a/UnitTests/Views/MenuTests.cs +++ b/UnitTests/Views/MenuTests.cs @@ -43,4 +43,68 @@ public class MenuTests void Run () { } } + + [Fact] + public void MenuBarItem_SubMenu_Can_Return_Null () + { + var menuItem = new MenuItem (); + var menuBarItem = new MenuBarItem (); + Assert.Null (menuBarItem.SubMenu (menuItem)); + } + + [Fact] + public void MenuBarItem_Constructors_Defaults () + { + var menuBarItem = new MenuBarItem (); + Assert.Equal ("", menuBarItem.Title); + Assert.Equal ("", menuBarItem.Help); + Assert.Null (menuBarItem.Action); + Assert.Null (menuBarItem.CanExecute); + Assert.Null (menuBarItem.Parent); + Assert.Equal (Key.Empty, menuBarItem.ShortcutKey); + Assert.Equal ([], menuBarItem.Children); + Assert.False (menuBarItem.IsTopLevel); + + menuBarItem = new MenuBarItem (null!, null!, Run, () => true, new ()); + Assert.Equal ("", menuBarItem.Title); + Assert.Equal ("", menuBarItem.Help); + Assert.Equal (Run, menuBarItem.Action); + Assert.NotNull (menuBarItem.CanExecute); + Assert.NotNull (menuBarItem.Parent); + Assert.Equal (Key.Empty, menuBarItem.ShortcutKey); + Assert.Null (menuBarItem.Children); + Assert.False (menuBarItem.IsTopLevel); + + menuBarItem = new MenuBarItem (null!, Array.Empty (), new ()); + Assert.Equal ("", menuBarItem.Title); + Assert.Equal ("", menuBarItem.Help); + Assert.Null (menuBarItem.Action); + Assert.Null (menuBarItem.CanExecute); + Assert.NotNull (menuBarItem.Parent); + Assert.Equal (Key.Empty, menuBarItem.ShortcutKey); + Assert.Equal ([], menuBarItem.Children); + Assert.False (menuBarItem.IsTopLevel); + + menuBarItem = new MenuBarItem (null!, new List (), new ()); + Assert.Equal ("", menuBarItem.Title); + Assert.Equal ("", menuBarItem.Help); + Assert.Null (menuBarItem.Action); + Assert.Null (menuBarItem.CanExecute); + Assert.NotNull (menuBarItem.Parent); + Assert.Equal (Key.Empty, menuBarItem.ShortcutKey); + Assert.Equal ([], menuBarItem.Children); + Assert.False (menuBarItem.IsTopLevel); + + menuBarItem = new MenuBarItem ([]); + Assert.Equal ("", menuBarItem.Title); + Assert.Equal ("", menuBarItem.Help); + Assert.Null (menuBarItem.Action); + Assert.Null (menuBarItem.CanExecute); + Assert.Null (menuBarItem.Parent); + Assert.Equal (Key.Empty, menuBarItem.ShortcutKey); + Assert.Equal ([], menuBarItem.Children); + Assert.False (menuBarItem.IsTopLevel); + + void Run () { } + } }