From a7e166ef5c355c94c86f38399fdba13d80ca44bf Mon Sep 17 00:00:00 2001 From: Tig Date: Sun, 6 Oct 2024 18:50:53 -0600 Subject: [PATCH] Shortcut code cleanup --- Terminal.Gui/Views/Button.cs | 60 ++++---- Terminal.Gui/Views/Shortcut.cs | 247 ++++++++++++++----------------- UnitTests/Views/ShortcutTests.cs | 4 +- 3 files changed, 143 insertions(+), 168 deletions(-) diff --git a/Terminal.Gui/Views/Button.cs b/Terminal.Gui/Views/Button.cs index fcc9d55cd..394157da4 100644 --- a/Terminal.Gui/Views/Button.cs +++ b/Terminal.Gui/Views/Button.cs @@ -67,36 +67,7 @@ public class Button : View, IDesignable CanFocus = true; - // Override default behavior of View - AddCommand ( - Command.HotKey, - (ctx) => - { - bool cachedIsDefault = IsDefault; // Supports "Swap Default" in Buttons scenario where IsDefault changes - - if (RaiseSelected (ctx) is true) - { - return true; - } - bool? handled = RaiseAccepted (); - - if (handled == true) - { - return true; - } - - SetFocus (); - - // TODO: If `IsDefault` were a property on `View` *any* View could work this way. That's theoretical as - // TODO: no use-case has been identified for any View other than Button to act like this. - // If Accept was not handled... - if (cachedIsDefault && SuperView is { }) - { - return SuperView.InvokeCommand (Command.Accept); - } - - return false; - }); + AddCommand (Command.HotKey, HandleHotKeyCommand); KeyBindings.Remove (Key.Space); KeyBindings.Add (Key.Space, Command.HotKey); @@ -110,6 +81,35 @@ public class Button : View, IDesignable HighlightStyle = DefaultHighlightStyle; } + private bool? HandleHotKeyCommand (CommandContext ctx) + { + bool cachedIsDefault = IsDefault; // Supports "Swap Default" in Buttons scenario where IsDefault changes + + if (RaiseSelected (ctx) is true) + { + return true; + } + + bool? handled = RaiseAccepted (); + + if (handled == true) + { + return true; + } + + SetFocus (); + + // TODO: If `IsDefault` were a property on `View` *any* View could work this way. That's theoretical as + // TODO: no use-case has been identified for any View other than Button to act like this. + // If Accept was not handled... + if (cachedIsDefault && SuperView is { }) + { + return SuperView.InvokeCommand (Command.Accept); + } + + return false; + } + private bool _wantContinuousButtonPressed; /// diff --git a/Terminal.Gui/Views/Shortcut.cs b/Terminal.Gui/Views/Shortcut.cs index 935335e5a..2c4aab465 100644 --- a/Terminal.Gui/Views/Shortcut.cs +++ b/Terminal.Gui/Views/Shortcut.cs @@ -103,52 +103,7 @@ public class Shortcut : View, IOrientation, IDesignable _orientationHelper.OrientationChanging += (sender, e) => OrientationChanging?.Invoke (this, e); _orientationHelper.OrientationChanged += (sender, e) => OrientationChanged?.Invoke (this, e); - // Accept (Enter key) - - AddCommand (Command.Accept, DispatchAcceptCommand); - - // Hotkey - - AddCommand ( - Command.HotKey, - ctx => - { - if (ctx.Data != this) - { - ctx.Data = this; - CommandView.InvokeCommand (Command.Select, ctx); - } - - if (RaiseSelected (ctx) is true) - { - return true; - } - - // The default HotKey handler sets Focus - SetFocus (); - - return DispatchAcceptCommand (ctx); - }); - - // Select (Space key or click) - - AddCommand ( - Command.Select, - ctx => - { - if (ctx.Data != this) - { - ctx.Data = this; - CommandView.InvokeCommand (Command.Select, ctx); - } - - if (RaiseSelected (ctx) is true) - { - return true; - } - - // The default HotKey handler sets Focus - SetFocus (); - - return DispatchAcceptCommand (ctx); - }); + AddCommands (); TitleChanged += Shortcut_TitleChanged; // This needs to be set before CommandView is set @@ -208,12 +163,6 @@ public class Shortcut : View, IOrientation, IDesignable } } - private readonly View? _targetView; // If set, _command will be invoked - - private readonly Command _command; // Used when _targetView is set - - private readonly OrientationHelper _orientationHelper; - private AlignmentModes _alignmentModes = AlignmentModes.StartToEnd | AlignmentModes.IgnoreFirstOrLast; // This is used to calculate the minimum width of the Shortcut when the width is NOT Dim.Auto @@ -230,16 +179,6 @@ public class Shortcut : View, IOrientation, IDesignable return true; } - /// - public bool EnableForDesign () - { - Title = "_Shortcut"; - HelpText = "Shortcut help"; - Key = Key.F1; - - return true; - } - /// /// Gets or sets the for this . /// @@ -261,30 +200,6 @@ public class Shortcut : View, IOrientation, IDesignable } } - /// - protected override void Dispose (bool disposing) - { - if (disposing) - { - if (CommandView?.IsAdded == false) - { - CommandView.Dispose (); - } - - if (HelpView?.IsAdded == false) - { - HelpView.Dispose (); - } - - if (KeyView?.IsAdded == false) - { - KeyView.Dispose (); - } - } - - base.Dispose (disposing); - } - // When one of the subviews is "empty" we don't want to show it. So we // Use Add/Remove. We need to be careful to add them in the right order // so Pos.Align works correctly. @@ -392,8 +307,81 @@ public class Shortcut : View, IOrientation, IDesignable } } + + #region Accept/Select/HotKey Command Handling + + private readonly View? _targetView; // If set, _command will be invoked + + private readonly Command _command; // Used when _targetView is set + + private void AddCommands () + { + // Accept (Enter key) - + AddCommand (Command.Accept, DispatchCommand); + // Hotkey - + AddCommand (Command.HotKey, DispatchCommand); + // Select (Space key or click) - + AddCommand (Command.Select, DispatchCommand); + } + + private bool? DispatchCommand (CommandContext ctx) + { + if (ctx.Data != this) + { + // Invoke Select on the command view to cause it to change state if it wants to + // If this causes CommandView to raise Accept, we eat it + ctx.Data = this; + CommandView.InvokeCommand (Command.Select, ctx); + } + + if (RaiseSelected (ctx) is true) + { + return true; + } + + // The default HotKey handler sets Focus + SetFocus (); + + var cancel = false; + + cancel = RaiseAccepted () is true; + + if (cancel) + { + return true; + } + + if (Action is { }) + { + Action.Invoke (); + + // Assume if there's a subscriber to Action, it's handled. + cancel = true; + } + + if (_targetView is { }) + { + _targetView.InvokeCommand (_command); + } + + return cancel; + } + + /// + /// Gets or sets the action to be invoked when the shortcut key is pressed or the shortcut is clicked on with the + /// mouse. + /// + /// + /// Note, the event is fired first, and if cancelled, the event will not be invoked. + /// + public Action? Action { get; set; } + + #endregion Accept/Select/HotKey Command Handling + #region IOrientation members + private readonly OrientationHelper _orientationHelper; + /// /// Gets or sets the for this . The default is /// . @@ -746,56 +734,6 @@ public class Shortcut : View, IOrientation, IDesignable #endregion Key - #region Accept Handling - - /// - /// Called when the command is received. This - /// occurs - /// - if the user clicks anywhere on the shortcut with the mouse - /// - if the user presses Key - /// - if the user presses the HotKey specified by CommandView - /// - if HasFocus and the user presses Space or Enter (or any other key bound to Command.Accept). - /// - internal bool? DispatchAcceptCommand (CommandContext ctx) - { - // Invoke Select on the command view to cause it to change state if it wants to - // If this causes CommandView to raise Accept, we eat it - var cancel = false; - - cancel = RaiseAccepted () is true; - - if (cancel) - { - return true; - } - - if (Action is { }) - { - Action.Invoke (); - - // Assume if there's a subscriber to Action, it's handled. - cancel = true; - } - - if (_targetView is { }) - { - _targetView.InvokeCommand (_command); - } - - return cancel; - } - - /// - /// Gets or sets the action to be invoked when the shortcut key is pressed or the shortcut is clicked on with the - /// mouse. - /// - /// - /// Note, the event is fired first, and if cancelled, the event will not be invoked. - /// - public Action? Action { get; set; } - - #endregion Accept Handling - #region Focus /// @@ -853,4 +791,41 @@ public class Shortcut : View, IOrientation, IDesignable protected override void OnHasFocusChanged (bool newHasFocus, View? previousFocusedView, View? view) { SetColors (); } #endregion Focus + + /// + public bool EnableForDesign () + { + Title = "_Shortcut"; + HelpText = "Shortcut help"; + Key = Key.F1; + + return true; + } + + + /// + protected override void Dispose (bool disposing) + { + if (disposing) + { + TitleChanged -= Shortcut_TitleChanged; + + if (CommandView?.IsAdded == false) + { + CommandView.Dispose (); + } + + if (HelpView?.IsAdded == false) + { + HelpView.Dispose (); + } + + if (KeyView?.IsAdded == false) + { + KeyView.Dispose (); + } + } + + base.Dispose (disposing); + } } diff --git a/UnitTests/Views/ShortcutTests.cs b/UnitTests/Views/ShortcutTests.cs index e7645885c..5ac9c8b88 100644 --- a/UnitTests/Views/ShortcutTests.cs +++ b/UnitTests/Views/ShortcutTests.cs @@ -636,7 +636,7 @@ public class ShortcutTests [InlineData (true, KeyCode.A, 1, 1)] [InlineData (true, KeyCode.C, 1, 1)] [InlineData (true, KeyCode.C | KeyCode.AltMask, 1, 1)] - [InlineData (true, KeyCode.Enter, 1, 0)] + [InlineData (true, KeyCode.Enter, 1, 1)] [InlineData (true, KeyCode.Space, 1, 1)] [InlineData (true, KeyCode.F1, 0, 0)] [InlineData (false, KeyCode.A, 1, 1)] @@ -681,7 +681,7 @@ public class ShortcutTests [InlineData (true, KeyCode.A, 1, 1)] [InlineData (true, KeyCode.C, 1, 1)] [InlineData (true, KeyCode.C | KeyCode.AltMask, 1, 1)] - [InlineData (true, KeyCode.Enter, 1, 0)] + [InlineData (true, KeyCode.Enter, 1, 1)] [InlineData (true, KeyCode.Space, 1, 1)] [InlineData (true, KeyCode.F1, 0, 0)] [InlineData (false, KeyCode.A, 1, 1)]