diff --git a/Terminal.Gui/View/View.Keyboard.cs b/Terminal.Gui/View/View.Keyboard.cs index ffc8906db..dda61eba4 100644 --- a/Terminal.Gui/View/View.Keyboard.cs +++ b/Terminal.Gui/View/View.Keyboard.cs @@ -11,6 +11,10 @@ public partial class View // Keyboard APIs private void SetupKeyboard () { KeyBindings = new (this); + KeyBindings.Add (Key.Space, Command.Select); + KeyBindings.Add (Key.Enter, Command.Accept); + + // Note, setting HotKey will bind HotKey to Command.HotKey HotKeySpecifier = (Rune)'_'; TitleTextFormatter.HotKeyChanged += TitleTextFormatter_HotKeyChanged; } diff --git a/Terminal.Gui/View/View.cs b/Terminal.Gui/View/View.cs index a6da7f221..4293d0d0f 100644 --- a/Terminal.Gui/View/View.cs +++ b/Terminal.Gui/View/View.cs @@ -136,20 +136,39 @@ public partial class View : Responder, ISupportInitializeNotification { SetupAdornments (); + AddCommand ( + Command.Select, + () => + { + SetFocus (); + + return RaiseSelectEvent (); + }); + + AddCommand ( + Command.HotKey, + () => + { + SetFocus (); + + return RaiseHotKeyCommandEvent (); + }); + + AddCommand ( + Command.Accept, + () => + { + SetFocus (); + + return RaiseAcceptEvent (); + }); + SetupKeyboard (); //SetupMouse (); SetupText (); - // By default, the Select command does nothing - AddCommand (Command.Select, RaiseSelectEvent); - - // By default, the HotKey command sets the focus - AddCommand (Command.HotKey, RaiseHotKeyEvent); - - // By default, the Accept command sets the focus - AddCommand (Command.Accept, RaiseAcceptEvent); } /// @@ -285,19 +304,9 @@ public partial class View : Responder, ISupportInitializeNotification /// Called when the command is received. Set to /// to stop processing. /// - /// - /// - /// The base implementation calls . - /// - /// /// /// to stop processing. - protected virtual bool OnAccept (HandledEventArgs args) - { - SetFocus (); - - return false; - } + protected virtual bool OnAccept (HandledEventArgs args) { return false; } /// /// Cancelable event raised when the command is invoked. Set @@ -335,19 +344,9 @@ public partial class View : Responder, ISupportInitializeNotification /// Called when the command is received. Set to /// to stop processing. /// - /// - /// - /// The base implementation calls . - /// - /// /// /// to stop processing. - protected virtual bool OnSelect (HandledEventArgs args) - { - SetFocus (); - - return false; - } + protected virtual bool OnSelect (HandledEventArgs args) { return false; } /// /// Cancelable event raised when the command is invoked. Set @@ -356,22 +355,21 @@ public partial class View : Responder, ISupportInitializeNotification /// public event EventHandler? Select; - /// /// Called when the command is invoked. Raises /// event. /// /// - /// If the event was canceled. If the event was raised but not canceled. + /// If the event was handled. If the event was raised but not handled. /// If no event was raised. /// - protected bool? RaiseHotKeyEvent () + protected bool? RaiseHotKeyCommandEvent () { HandledEventArgs args = new (); // Best practice is to invoke the virtual method first. // This allows derived classes to handle the event and potentially cancel it. - if (OnHotKey (args) || args.Handled) + if (OnHotKeyCommand (args) || args.Handled) { return true; } @@ -386,19 +384,9 @@ public partial class View : Responder, ISupportInitializeNotification /// Called when the command is received. Set to /// to stop processing. /// - /// - /// - /// The base implementation calls . - /// - /// /// /// to stop processing. - protected virtual bool OnHotKey (HandledEventArgs args) - { - SetFocus (); - - return false; - } + protected virtual bool OnHotKeyCommand (HandledEventArgs args) { return false; } /// /// Cancelable event raised when the command is invoked. Set diff --git a/Terminal.Gui/Views/Button.cs b/Terminal.Gui/Views/Button.cs index 432b330f5..aae96d1ed 100644 --- a/Terminal.Gui/Views/Button.cs +++ b/Terminal.Gui/Views/Button.cs @@ -89,7 +89,9 @@ public class Button : View, IDesignable return false; }); + KeyBindings.Remove (Key.Space); KeyBindings.Add (Key.Space, Command.HotKey); + KeyBindings.Remove (Key.Enter); KeyBindings.Add (Key.Enter, Command.HotKey); TitleChanged += Button_TitleChanged; diff --git a/Terminal.Gui/Views/CheckBox.cs b/Terminal.Gui/Views/CheckBox.cs index 351c6f92c..8e5297ef0 100644 --- a/Terminal.Gui/Views/CheckBox.cs +++ b/Terminal.Gui/Views/CheckBox.cs @@ -24,9 +24,6 @@ public class CheckBox : View AddCommand (Command.Accept, AdvanceCheckState); AddCommand (Command.HotKey, AdvanceCheckState); - // Default keybindings for this view - KeyBindings.Add (Key.Space, Command.Accept); - TitleChanged += Checkbox_TitleChanged; HighlightStyle = DefaultHighlightStyle; diff --git a/Terminal.Gui/Views/ComboBox.cs b/Terminal.Gui/Views/ComboBox.cs index 69c1b65d2..1b34c8211 100644 --- a/Terminal.Gui/Views/ComboBox.cs +++ b/Terminal.Gui/Views/ComboBox.cs @@ -90,7 +90,6 @@ public class ComboBox : View, IDesignable AddCommand (Command.UnixEmulation, () => UnixEmulation ()); // Default keybindings for this view - KeyBindings.Add (Key.Enter, Command.Accept); KeyBindings.Add (Key.F4, Command.Toggle); KeyBindings.Add (Key.CursorDown, Command.Down); KeyBindings.Add (Key.CursorUp, Command.Up); diff --git a/Terminal.Gui/Views/HexView.cs b/Terminal.Gui/Views/HexView.cs index a79a5cc32..062ae1cd9 100644 --- a/Terminal.Gui/Views/HexView.cs +++ b/Terminal.Gui/Views/HexView.cs @@ -80,7 +80,6 @@ public class HexView : View KeyBindings.Add (Key.CursorRight, Command.Right); KeyBindings.Add (Key.CursorDown, Command.Down); KeyBindings.Add (Key.CursorUp, Command.Up); - KeyBindings.Add (Key.Enter, Command.Accept); KeyBindings.Add (Key.V.WithAlt, Command.PageUp); KeyBindings.Add (Key.PageUp, Command.PageUp); diff --git a/Terminal.Gui/Views/Label.cs b/Terminal.Gui/Views/Label.cs index 344120eba..4d81d7181 100644 --- a/Terminal.Gui/Views/Label.cs +++ b/Terminal.Gui/Views/Label.cs @@ -19,9 +19,6 @@ public class Label : View // Things this view knows how to do AddCommand (Command.HotKey, FocusNext); - // Default key bindings for this view - KeyBindings.Add (Key.Space, Command.Accept); - TitleChanged += Label_TitleChanged; MouseClick += Label_MouseClick; } diff --git a/Terminal.Gui/Views/ListView.cs b/Terminal.Gui/Views/ListView.cs index 013848ff2..579c5e894 100644 --- a/Terminal.Gui/Views/ListView.cs +++ b/Terminal.Gui/Views/ListView.cs @@ -180,8 +180,8 @@ public class ListView : View, IDesignable KeyBindings.Add (Key.End, Command.End); - KeyBindings.Add (Key.Space, Command.Select); - + // BUGBUG: This should just be Command.Accept + KeyBindings.Remove (Key.Enter); KeyBindings.Add (Key.Enter, Command.Open); } @@ -476,9 +476,9 @@ public class ListView : View, IDesignable _selected = Viewport.Y + me.Position.Y; - if (!MarkUnmarkSelectedItem()) + if (MarkUnmarkSelectedItem()) { - return true; + // return true; } OnSelectedChanged (); diff --git a/Terminal.Gui/Views/Menu/Menu.cs b/Terminal.Gui/Views/Menu/Menu.cs index b5f8b6eab..533894ac4 100644 --- a/Terminal.Gui/Views/Menu/Menu.cs +++ b/Terminal.Gui/Views/Menu/Menu.cs @@ -195,7 +195,6 @@ internal sealed class Menu : View KeyBindings.Add (Key.CursorLeft, Command.Left); KeyBindings.Add (Key.CursorRight, Command.Right); KeyBindings.Add (Key.Esc, Command.Cancel); - KeyBindings.Add (Key.Enter, Command.Accept); } private void AddKeyBindingsHotKey (MenuBarItem? menuBarItem) diff --git a/Terminal.Gui/Views/Menu/MenuBar.cs b/Terminal.Gui/Views/Menu/MenuBar.cs index 34147eeb2..237bcce48 100644 --- a/Terminal.Gui/Views/Menu/MenuBar.cs +++ b/Terminal.Gui/Views/Menu/MenuBar.cs @@ -145,7 +145,6 @@ public class MenuBar : View, IDesignable KeyBindings.Add (Key.CursorRight, Command.Right); KeyBindings.Add (Key.Esc, Command.Cancel); KeyBindings.Add (Key.CursorDown, Command.Accept); - KeyBindings.Add (Key.Enter, Command.Accept); KeyBinding keyBinding = new ([Command.Toggle], KeyBindingScope.HotKey, -1); // -1 indicates Key was used KeyBindings.Add (Key, keyBinding); diff --git a/Terminal.Gui/Views/RadioGroup.cs b/Terminal.Gui/Views/RadioGroup.cs index 2e5d48bad..ab6829af6 100644 --- a/Terminal.Gui/Views/RadioGroup.cs +++ b/Terminal.Gui/Views/RadioGroup.cs @@ -3,13 +3,6 @@ /// Displays a group of labels each with a selected indicator. Only one of those can be selected at a given time. public class RadioGroup : View, IDesignable, IOrientation { - private int _cursor; - private List<(int pos, int length)> _horizontal; - private int _horizontalSpace = 2; - private List _radioLabels = []; - private int _selected; - private readonly OrientationHelper _orientationHelper; - /// /// Initializes a new instance of the class. /// @@ -42,6 +35,7 @@ public class RadioGroup : View, IDesignable, IOrientation { return false; } + return MoveDownRight (); } ); @@ -75,11 +69,12 @@ public class RadioGroup : View, IDesignable, IOrientation return true; } ); + AddCommand ( Command.Select, () => { - if (SelectedItem == _cursor) + if (SelectedItem == Cursor) { if (!MoveDownRight ()) { @@ -87,19 +82,21 @@ public class RadioGroup : View, IDesignable, IOrientation } } - SelectedItem = _cursor; + SelectedItem = Cursor; return true; }); + AddCommand ( Command.Accept, () => { - SelectedItem = _cursor; + SelectedItem = Cursor; return RaiseAcceptEvent () is false; } ); + AddCommand ( Command.HotKey, ctx => @@ -132,24 +129,26 @@ public class RadioGroup : View, IDesignable, IOrientation private void SetupKeyBindings () { - KeyBindings.Clear (); - // Default keybindings for this view if (Orientation == Orientation.Vertical) { + KeyBindings.Remove (Key.CursorUp); KeyBindings.Add (Key.CursorUp, Command.Up); + KeyBindings.Remove (Key.CursorDown); KeyBindings.Add (Key.CursorDown, Command.Down); } else { + KeyBindings.Remove (Key.CursorLeft); KeyBindings.Add (Key.CursorLeft, Command.Up); + KeyBindings.Remove (Key.CursorRight); KeyBindings.Add (Key.CursorRight, Command.Down); } + KeyBindings.Remove (Key.Home); KeyBindings.Add (Key.Home, Command.Start); + KeyBindings.Remove (Key.End); KeyBindings.Add (Key.End, Command.End); - KeyBindings.Add (Key.Enter, Command.Accept); - KeyBindings.Add (Key.Space, Command.Select); } private void RadioGroup_MouseClick (object sender, MouseEventEventArgs e) @@ -173,7 +172,7 @@ public class RadioGroup : View, IDesignable, IOrientation if (c > -1) { - _cursor = SelectedItem = c; + Cursor = SelectedItem = c; SetNeedsDisplay (); } } @@ -181,6 +180,9 @@ public class RadioGroup : View, IDesignable, IOrientation e.Handled = true; } + private List<(int pos, int length)> _horizontal; + private int _horizontalSpace = 2; + /// /// Gets or sets the horizontal space for this if the is /// @@ -199,6 +201,8 @@ public class RadioGroup : View, IDesignable, IOrientation } } + private List _radioLabels = []; + /// /// The radio labels to display. A key binding will be added for each radio enabling the user to select /// and/or focus the radio label using the keyboard. See for details on how HotKeys work. @@ -236,6 +240,8 @@ public class RadioGroup : View, IDesignable, IOrientation } } + private int _selected; + /// The currently selected item from the list of radio labels /// The selected. public int SelectedItem @@ -244,7 +250,7 @@ public class RadioGroup : View, IDesignable, IOrientation set { OnSelectedItemChanged (value, SelectedItem); - _cursor = Math.Max (_selected, 0); + Cursor = Math.Max (_selected, 0); SetNeedsDisplay (); } } @@ -283,19 +289,19 @@ public class RadioGroup : View, IDesignable, IOrientation { Rune rune = rlRunes [j]; - if (j == hotPos && i == _cursor) + if (j == hotPos && i == Cursor) { Application.Driver?.SetAttribute ( - HasFocus - ? ColorScheme.HotFocus - : GetHotNormalColor () - ); + HasFocus + ? ColorScheme.HotFocus + : GetHotNormalColor () + ); } - else if (j == hotPos && i != _cursor) + else if (j == hotPos && i != Cursor) { Application.Driver?.SetAttribute (GetHotNormalColor ()); } - else if (HasFocus && i == _cursor) + else if (HasFocus && i == Cursor) { Application.Driver?.SetAttribute (GetFocusColor ()); } @@ -305,15 +311,15 @@ public class RadioGroup : View, IDesignable, IOrientation j++; rune = rlRunes [j]; - if (i == _cursor) + if (i == Cursor) { Application.Driver?.SetAttribute ( - HasFocus - ? ColorScheme.HotFocus - : GetHotNormalColor () - ); + HasFocus + ? ColorScheme.HotFocus + : GetHotNormalColor () + ); } - else if (i != _cursor) + else if (i != Cursor) { Application.Driver?.SetAttribute (GetHotNormalColor ()); } @@ -325,11 +331,13 @@ public class RadioGroup : View, IDesignable, IOrientation } else { - DrawHotString (rl, HasFocus && i == _cursor); + DrawHotString (rl, HasFocus && i == Cursor); } } } + #region IOrientation + /// /// Gets or sets the for this . The default is /// . @@ -340,7 +348,7 @@ public class RadioGroup : View, IDesignable, IOrientation set => _orientationHelper.Orientation = value; } - #region IOrientation + private readonly OrientationHelper _orientationHelper; /// public event EventHandler> OrientationChanging; @@ -373,6 +381,17 @@ public class RadioGroup : View, IDesignable, IOrientation SelectedItemChanged?.Invoke (this, new (selectedItem, previousSelectedItem)); } + /// + /// Gets or sets the index for the cursor. The cursor may or may not be the selected + /// RadioItem. + /// + /// + /// + /// Maps to either the X or Y position within depending on . + /// + /// + public int Cursor { get; set; } + /// public override Point? PositionCursor () { @@ -382,13 +401,13 @@ public class RadioGroup : View, IDesignable, IOrientation switch (Orientation) { case Orientation.Vertical: - y = _cursor; + y = Cursor; break; case Orientation.Horizontal: if (_horizontal.Count > 0) { - x = _horizontal [_cursor].pos; + x = _horizontal [Cursor].pos; } break; @@ -411,9 +430,9 @@ public class RadioGroup : View, IDesignable, IOrientation private bool MoveDownRight () { - if (_cursor + 1 < _radioLabels.Count) + if (Cursor + 1 < _radioLabels.Count) { - _cursor++; + Cursor++; SetNeedsDisplay (); return true; @@ -423,18 +442,19 @@ public class RadioGroup : View, IDesignable, IOrientation return false; } - private void MoveEnd () { _cursor = Math.Max (_radioLabels.Count - 1, 0); } - private void MoveHome () { _cursor = 0; } + private void MoveEnd () { Cursor = Math.Max (_radioLabels.Count - 1, 0); } + private void MoveHome () { Cursor = 0; } private bool MoveUpLeft () { - if (_cursor > 0) + if (Cursor > 0) { - _cursor--; + Cursor--; SetNeedsDisplay (); return true; } + // Moving past should move focus to next view, not wrap return false; } diff --git a/Terminal.Gui/Views/Shortcut.cs b/Terminal.Gui/Views/Shortcut.cs index ea1ca6f91..9e396dd66 100644 --- a/Terminal.Gui/Views/Shortcut.cs +++ b/Terminal.Gui/Views/Shortcut.cs @@ -65,8 +65,6 @@ public class Shortcut : View, IOrientation, IDesignable AddCommand (Command.HotKey, ctx => OnAccept (ctx)); AddCommand (Command.Accept, ctx => OnAccept (ctx)); AddCommand (Command.Select, ctx => OnSelect (ctx)); - KeyBindings.Add (KeyCode.Enter, Command.Accept); - KeyBindings.Add (KeyCode.Space, Command.Select); TitleChanged += Shortcut_TitleChanged; // This needs to be set before CommandView is set diff --git a/Terminal.Gui/Views/TableView/TableView.cs b/Terminal.Gui/Views/TableView/TableView.cs index 5c3c58b9b..8b49c2f22 100644 --- a/Terminal.Gui/Views/TableView/TableView.cs +++ b/Terminal.Gui/Views/TableView/TableView.cs @@ -283,8 +283,8 @@ public class TableView : View KeyBindings.Add (Key.End.WithCtrl.WithShift, Command.EndExtend); KeyBindings.Add (Key.A.WithCtrl, Command.SelectAll); + KeyBindings.Remove (CellActivationKey); KeyBindings.Add (CellActivationKey, Command.Accept); - KeyBindings.Add (Key.Space, Command.Select); } // TODO: Update to use Key instead of KeyCode diff --git a/Terminal.Gui/Views/TextField.cs b/Terminal.Gui/Views/TextField.cs index b279a16e6..625878c22 100644 --- a/Terminal.Gui/Views/TextField.cs +++ b/Terminal.Gui/Views/TextField.cs @@ -409,7 +409,6 @@ public class TextField : View ContextMenu.KeyChanged += ContextMenu_KeyChanged; KeyBindings.Add (ContextMenu.Key, KeyBindingScope.HotKey, Command.Context); - KeyBindings.Add (Key.Enter, Command.Accept); } diff --git a/Terminal.Gui/Views/TextView.cs b/Terminal.Gui/Views/TextView.cs index 9ed8beb20..3ddb35c46 100644 --- a/Terminal.Gui/Views/TextView.cs +++ b/Terminal.Gui/Views/TextView.cs @@ -2275,7 +2275,9 @@ public class TextView : View return true; } ); - AddCommand (Command.NewLine, () => ProcessReturn ()); + + // BUGBUG: If AllowsReturn is false, Key.Enter should not be bound (so that Toplevel can cause Command.Accept). + AddCommand (Command.Accept, () => ProcessReturn ()); AddCommand ( Command.End, @@ -2479,8 +2481,6 @@ public class TextView : View KeyBindings.Add (Key.Delete.WithCtrl, Command.KillWordForwards); // kill-word-forwards KeyBindings.Add (Key.Backspace.WithCtrl, Command.KillWordBackwards); // kill-word-backwards - // BUGBUG: If AllowsReturn is false, Key.Enter should not be bound (so that Toplevel can cause Command.Accept). - KeyBindings.Add (Key.Enter, Command.NewLine); KeyBindings.Add (Key.End.WithCtrl, Command.End); KeyBindings.Add (Key.End.WithCtrl.WithShift, Command.EndExtend); KeyBindings.Add (Key.Home.WithCtrl, Command.Start); diff --git a/Terminal.Gui/Views/TreeView/TreeView.cs b/Terminal.Gui/Views/TreeView/TreeView.cs index 9115f4bf1..a9c2464ec 100644 --- a/Terminal.Gui/Views/TreeView/TreeView.cs +++ b/Terminal.Gui/Views/TreeView/TreeView.cs @@ -296,6 +296,8 @@ public class TreeView : View, ITreeView where T : class KeyBindings.Add (Key.Home, Command.Start); KeyBindings.Add (Key.End, Command.End); KeyBindings.Add (Key.A.WithCtrl, Command.SelectAll); + + KeyBindings.Remove (ObjectActivationKey); KeyBindings.Add (ObjectActivationKey, Command.Select); } @@ -1227,7 +1229,7 @@ public class TreeView : View, ITreeView where T : class { Move (0, idx - ScrollOffsetVertical); - return MultiSelect ? new (0, idx - ScrollOffsetVertical) : null ; + return MultiSelect ? new (0, idx - ScrollOffsetVertical) : null; } } return base.PositionCursor (); diff --git a/Terminal.Gui/Views/Window.cs b/Terminal.Gui/Views/Window.cs index 1cae3dbc4..51cebd300 100644 --- a/Terminal.Gui/Views/Window.cs +++ b/Terminal.Gui/Views/Window.cs @@ -31,8 +31,6 @@ public class Window : Toplevel ColorScheme = Colors.ColorSchemes ["Base"]; // TODO: make this a theme property BorderStyle = DefaultBorderStyle; ShadowStyle = DefaultShadow; - - KeyBindings.Add (Key.Enter, Command.Accept); } // TODO: enable this diff --git a/UICatalog/Scenarios/CharacterMap.cs b/UICatalog/Scenarios/CharacterMap.cs index 8c4a31133..6c4b7080c 100644 --- a/UICatalog/Scenarios/CharacterMap.cs +++ b/UICatalog/Scenarios/CharacterMap.cs @@ -449,7 +449,6 @@ internal class CharMap : View } ); - KeyBindings.Add (Key.Enter, Command.Accept); KeyBindings.Add (Key.CursorUp, Command.ScrollUp); KeyBindings.Add (Key.CursorDown, Command.ScrollDown); KeyBindings.Add (Key.CursorLeft, Command.ScrollLeft); diff --git a/UICatalog/Scenarios/Generic.cs b/UICatalog/Scenarios/Generic.cs index 97dab5372..37891e7a6 100644 --- a/UICatalog/Scenarios/Generic.cs +++ b/UICatalog/Scenarios/Generic.cs @@ -17,8 +17,8 @@ public sealed class MyScenario : Scenario Title = GetQuitKeyAndName (), }; - var button = new Button { X = Pos.Center (), Y = Pos.Center (), Text = "Press me!" }; - button.Accept += (s, e) => MessageBox.ErrorQuery ("Error", "You pressed the button!", "Ok"); + var button = new Button { X = Pos.Center (), Y = Pos.Center (), Text = "_Press me!" }; + button.Accept += (s, e) => MessageBox.ErrorQuery ("Error", "You pressed the button!", "_Ok"); appWindow.Add (button); // Run - Start the application. diff --git a/UnitTests/View/HotKeyTests.cs b/UnitTests/View/HotKeyTests.cs index 163b0b32d..328515ffc 100644 --- a/UnitTests/View/HotKeyTests.cs +++ b/UnitTests/View/HotKeyTests.cs @@ -341,4 +341,46 @@ public class HotKeyTests Assert.Equal ("", view.Title); Assert.Equal (KeyCode.Null, view.HotKey); } + + + [Fact] + public void HotKey_Raises_HotKeyCommand () + { + var hotKeyRaised = false; + var acceptRaised = false; + var selectRaised = false; + Application.Top = new Toplevel (); + var view = new View + { + CanFocus = true, + HotKeySpecifier = new Rune ('_'), + Title = "_Test" + }; + Application.Top.Add (view); + view.HotKeyCommand += (s, e) => hotKeyRaised = true; + view.Accept += (s, e) => acceptRaised = true; + view.Select += (s, e) => selectRaised = true; + + Assert.Equal (KeyCode.T, view.HotKey); + Assert.False (Application.OnKeyDown (Key.T)); // wasn't handled + Assert.True (hotKeyRaised); + Assert.False (acceptRaised); + Assert.False (selectRaised); + + hotKeyRaised = false; + Assert.False (Application.OnKeyDown (Key.T.WithAlt)); + Assert.True (hotKeyRaised); + Assert.False (acceptRaised); + Assert.False (selectRaised); + + hotKeyRaised = false; + view.HotKey = KeyCode.E; + Assert.False (Application.OnKeyDown (Key.E.WithAlt)); + Assert.True (hotKeyRaised); + Assert.False (acceptRaised); + Assert.False (selectRaised); + + Application.Top.Dispose (); + Application.ResetState (true); + } } diff --git a/UnitTests/View/ViewCommandTests.cs b/UnitTests/View/ViewCommandTests.cs new file mode 100644 index 000000000..92f5c2693 --- /dev/null +++ b/UnitTests/View/ViewCommandTests.cs @@ -0,0 +1,175 @@ +using System.ComponentModel; +using System.Text; +using Xunit.Abstractions; + +namespace Terminal.Gui.ViewTests; + +public class ViewCommandTests (ITestOutputHelper output) +{ + // OnAccept/Accept tests + [Fact] + public void Accept_Command_Raises () + { + var view = new ViewEventTester (); + Assert.False (view.HasFocus); + + Assert.False (view.InvokeCommand (Command.Accept)); // false means it was not handled + + Assert.Equal (1, view.OnAcceptCount); + + Assert.Equal (1, view.AcceptCount); + + Assert.True (view.HasFocus); + } + + [Fact] + public void Accept_Command_Handle_OnAccept_NoEvent () + { + var view = new ViewEventTester (); + Assert.False (view.HasFocus); + + view.HandleOnAccept = true; + Assert.True (view.InvokeCommand (Command.Accept)); + + Assert.Equal (1, view.OnAcceptCount); + + Assert.Equal (0, view.AcceptCount); + } + + + + + [Fact] + public void Accept_Handle_Event_OnAccept_Returns_True () + { + var view = new View (); + var acceptInvoked = false; + + view.Accept += ViewOnAccept; + + bool? ret = view.InvokeCommand (Command.Accept); + Assert.True (ret); + Assert.True (acceptInvoked); + + return; + + void ViewOnAccept (object sender, HandledEventArgs e) + { + acceptInvoked = true; + e.Handled = true; + } + } + + [Fact] + public void Accept_Command_Invokes_Accept_Event () + { + var view = new View (); + var accepted = false; + + view.Accept += ViewOnAccept; + + view.InvokeCommand (Command.Accept); + Assert.True (accepted); + + return; + + void ViewOnAccept (object sender, HandledEventArgs e) { accepted = true; } + } + + [Fact] + public void HotKey_Command_SetsFocus () + { + var view = new View (); + + view.CanFocus = true; + Assert.False (view.HasFocus); + view.InvokeCommand (Command.HotKey); + Assert.True (view.HasFocus); + } + + public class ViewEventTester : View + { + public ViewEventTester () + { + CanFocus = true; + + Accept += (s, a) => + { + a.Handled = HandleAccept; + AcceptCount++; + }; + + HotKeyCommand += (s, a) => + { + a.Handled = HandleHotKeyCommand; + HotKeyCommandCount++; + }; + + + Select += (s, a) => + { + a.Handled = HandleSelect; + SelectCount++; + }; + } + + public int OnAcceptCount { get; set; } + public int AcceptCount { get; set; } + public bool HandleOnAccept { get; set; } + + /// + protected override bool OnAccept (HandledEventArgs args) + { + OnAcceptCount++; + + if (!HandleOnAccept) + { + return base.OnAccept (args); + } + + return HandleOnAccept; + } + + public bool HandleAccept { get; set; } + + public int OnHotKeyCommandCount { get; set; } + public int HotKeyCommandCount { get; set; } + public bool HandleOnHotKeyCommand { get; set; } + + /// + protected override bool OnHotKeyCommand (HandledEventArgs args) + { + OnHotKeyCommandCount++; + if (!HandleOnHotKeyCommand) + { + return base.OnHotKeyCommand (args); + } + + + return HandleOnHotKeyCommand; + } + + public bool HandleHotKeyCommand { get; set; } + + + public int OnSelectCount { get; set; } + public int SelectCount { get; set; } + public bool HandleOnSelect { get; set; } + + /// + protected override bool OnSelect (HandledEventArgs args) + { + OnSelectCount++; + + if (!HandleOnSelect) + { + return base.OnSelect (args); + } + + return HandleOnSelect; + } + + public bool HandleSelect { get; set; } + + } +} diff --git a/UnitTests/View/ViewTests.cs b/UnitTests/View/ViewTests.cs index f79f2b17d..6f1392a46 100644 --- a/UnitTests/View/ViewTests.cs +++ b/UnitTests/View/ViewTests.cs @@ -160,7 +160,7 @@ public class ViewTests (ITestOutputHelper output) if (label) { Assert.False (v.CanFocus); - Assert.Equal (new (0, 0, text.Length, 1), v.Frame); + Assert.Equal (new (0, 0, text.Length, 1), v.Frame); } else { @@ -469,7 +469,7 @@ At 0,0 X = 0, // don't overcomplicate unit tests Y = 1, Height = Dim.Auto (DimAutoStyle.Text), - Width = Dim.Auto(DimAutoStyle.Text), + Width = Dim.Auto (DimAutoStyle.Text), Text = "Press me!" }; @@ -783,7 +783,7 @@ At 0,0 r.Dispose (); // Empty Rect - r = new() { Frame = Rectangle.Empty }; + r = new () { Frame = Rectangle.Empty }; Assert.NotNull (r); Assert.Equal ($"View(){r.Viewport}", r.ToString ()); Assert.False (r.CanFocus); @@ -807,7 +807,7 @@ At 0,0 r.Dispose (); // Rect with values - r = new() { Frame = new (1, 2, 3, 4) }; + r = new () { Frame = new (1, 2, 3, 4) }; Assert.NotNull (r); Assert.Equal ($"View(){r.Frame}", r.ToString ()); Assert.False (r.CanFocus); @@ -831,7 +831,7 @@ At 0,0 r.Dispose (); // Initializes a view with a vertical direction - r = new() + r = new () { Text = "Vertical View", TextDirection = TextDirection.TopBottom_LeftRight, @@ -870,11 +870,11 @@ At 0,0 { var r = new View (); - Assert.False (r.OnKeyDown (new() { KeyCode = KeyCode.Null })); + Assert.False (r.OnKeyDown (new () { KeyCode = KeyCode.Null })); //Assert.False (r.OnKeyDown (new KeyEventArgs () { Key = Key.Unknown })); - Assert.False (r.OnKeyUp (new() { KeyCode = KeyCode.Null })); - Assert.False (r.NewMouseEvent (new() { Flags = MouseFlags.AllEvents })); + Assert.False (r.OnKeyUp (new () { KeyCode = KeyCode.Null })); + Assert.False (r.NewMouseEvent (new () { Flags = MouseFlags.AllEvents })); r.Dispose (); @@ -960,7 +960,7 @@ At 0,0 view.Dispose (); // Object Initializer - view = new() { X = 1, Y = 2, Text = "" }; + view = new () { X = 1, Y = 2, Text = "" }; Assert.Equal (1, view.X); Assert.Equal (2, view.Y); Assert.Equal (0, view.Width); @@ -975,7 +975,7 @@ At 0,0 view.Y = 2; view.Width = 3; view.Height = 4; - super = new() { Frame = new (0, 0, 10, 10) }; + super = new () { Frame = new (0, 0, 10, 10) }; super.Add (view); super.BeginInit (); super.EndInit (); @@ -1153,69 +1153,4 @@ At 0,0 return true; } } - - // OnAccept/Accept tests - [Fact] - public void OnAccept_Fires_Accept () - { - var view = new View (); - var accepted = false; - - view.Accept += ViewOnAccept; - - view.InvokeCommand (Command.Accept); - Assert.True (accepted); - - return; - - void ViewOnAccept (object sender, HandledEventArgs e) { accepted = true; } - } - - [Fact] - public void Accept_Cancel_Event_OnAccept_Returns_True () - { - var view = new View (); - var acceptInvoked = false; - - view.Accept += ViewOnAccept; - - bool? ret = view.InvokeCommand (Command.Accept); - Assert.True (ret); - Assert.True (acceptInvoked); - - return; - - void ViewOnAccept (object sender, HandledEventArgs e) - { - acceptInvoked = true; - e.Handled = true; - } - } - - [Fact] - public void Accept_Command_Invokes_Accept_Event () - { - var view = new View (); - var accepted = false; - - view.Accept += ViewOnAccept; - - view.InvokeCommand (Command.Accept); - Assert.True (accepted); - - return; - - void ViewOnAccept (object sender, HandledEventArgs e) { accepted = true; } - } - - [Fact] - public void HotKey_Command_SetsFocus () - { - var view = new View (); - - view.CanFocus = true; - Assert.False (view.HasFocus); - view.InvokeCommand (Command.HotKey); - Assert.True (view.HasFocus); - } } diff --git a/UnitTests/Views/ButtonTests.cs b/UnitTests/Views/ButtonTests.cs index 681afe466..f97d945eb 100644 --- a/UnitTests/Views/ButtonTests.cs +++ b/UnitTests/Views/ButtonTests.cs @@ -252,16 +252,16 @@ public class ButtonTests (ITestOutputHelper output) btn.Accept += (s, e) => clicked = true; Assert.Equal (KeyCode.T, btn.HotKey); - Assert.True (btn.NewKeyDownEvent (Key.T)); + Assert.False (btn.NewKeyDownEvent (Key.T)); // Button processes, but does not handle Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (Key.T.WithAlt)); + Assert.False (btn.NewKeyDownEvent (Key.T.WithAlt)); // Button processes, but does not handle Assert.True (clicked); clicked = false; btn.HotKey = KeyCode.E; - Assert.True (btn.NewKeyDownEvent (Key.E.WithAlt)); + Assert.False (btn.NewKeyDownEvent (Key.E.WithAlt)); // Button processes, but does not handle Assert.True (clicked); } @@ -421,56 +421,56 @@ public class ButtonTests (ITestOutputHelper output) // Hot key. Both alone and with alt Assert.Equal (KeyCode.T, btn.HotKey); - Assert.True (btn.NewKeyDownEvent (Key.T)); + Assert.False (btn.NewKeyDownEvent (Key.T)); // Button processes, but does not handle Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (Key.T.WithAlt)); + Assert.False (btn.NewKeyDownEvent (Key.T.WithAlt)); Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (btn.HotKey)); + Assert.False (btn.NewKeyDownEvent (btn.HotKey)); Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (btn.HotKey)); + Assert.False (btn.NewKeyDownEvent (btn.HotKey)); Assert.True (clicked); clicked = false; // IsDefault = false // Space and Enter should work Assert.False (btn.IsDefault); - Assert.True (btn.NewKeyDownEvent (Key.Enter)); + Assert.False (btn.NewKeyDownEvent (Key.Enter)); Assert.True (clicked); clicked = false; // IsDefault = true // Space and Enter should work btn.IsDefault = true; - Assert.True (btn.NewKeyDownEvent (Key.Enter)); + Assert.False (btn.NewKeyDownEvent (Key.Enter)); Assert.True (clicked); clicked = false; // Toplevel does not handle Enter, so it should get passed on to button - Assert.True (Application.Top.NewKeyDownEvent (Key.Enter)); + Assert.False (Application.Top.NewKeyDownEvent (Key.Enter)); Assert.True (clicked); clicked = false; // Direct - Assert.True (btn.NewKeyDownEvent (Key.Enter)); + Assert.False (btn.NewKeyDownEvent (Key.Enter)); Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (Key.Space)); + Assert.False (btn.NewKeyDownEvent (Key.Space)); Assert.True (clicked); clicked = false; - Assert.True (btn.NewKeyDownEvent (new ((KeyCode)'T'))); + Assert.False (btn.NewKeyDownEvent (new ((KeyCode)'T'))); Assert.True (clicked); clicked = false; // Change hotkey: btn.Text = "Te_st"; - Assert.True (btn.NewKeyDownEvent (btn.HotKey)); + Assert.False (btn.NewKeyDownEvent (btn.HotKey)); Assert.True (clicked); clicked = false; diff --git a/UnitTests/Views/DatePickerTests.cs b/UnitTests/Views/DatePickerTests.cs index e6d26e1ad..fd78ac432 100644 --- a/UnitTests/Views/DatePickerTests.cs +++ b/UnitTests/Views/DatePickerTests.cs @@ -82,7 +82,7 @@ public class DatePickerTests Assert.Equal (datePicker.Subviews.First (v => v.Id == "_nextMonthButton"), datePicker.Focused); // Change month to December - Assert.True (Application.OnKeyDown (Key.Enter)); + Assert.False (Application.OnKeyDown (Key.Enter)); Assert.Equal (12, datePicker.Date.Month); // Next month button is disabled, so focus advanced to edit field @@ -111,7 +111,7 @@ public class DatePickerTests Assert.Equal (datePicker.Subviews.First (v => v.Id == "_previousMonthButton"), datePicker.Focused); // Change month to January - Assert.True (datePicker.NewKeyDownEvent (Key.Enter)); + Assert.False (datePicker.NewKeyDownEvent (Key.Enter)); Assert.Equal (1, datePicker.Date.Month); // Next prev button is disabled, so focus advanced to edit button diff --git a/UnitTests/Views/RadioGroupTests.cs b/UnitTests/Views/RadioGroupTests.cs index 26fe93071..87515b61b 100644 --- a/UnitTests/Views/RadioGroupTests.cs +++ b/UnitTests/Views/RadioGroupTests.cs @@ -55,7 +55,7 @@ public class RadioGroupTests (ITestOutputHelper output) rg.SetFocus (); Assert.Equal (-1, rg.SelectedItem); - Assert.True (Application.OnKeyDown (Key.Space)); + Application.OnKeyDown (Key.Space); Assert.Equal (0, rg.SelectedItem); Application.Top.Dispose (); @@ -84,23 +84,54 @@ public class RadioGroupTests (ITestOutputHelper output) Application.Top.Add (rg); rg.SetFocus (); Assert.Equal (Orientation.Vertical, rg.Orientation); + + // By default the first item is selected Assert.Equal (0, rg.SelectedItem); + + // Test up/down without Select Assert.False (Application.OnKeyDown (Key.CursorUp)); // Should not change (should focus prev view if there was one, which there isn't) Assert.Equal (0, rg.SelectedItem); + Assert.Equal (0, rg.Cursor); Assert.True (Application.OnKeyDown (Key.CursorDown)); + Assert.Equal (0, rg.SelectedItem); // Cursor changed, but selection didnt + Assert.Equal (1, rg.Cursor); + Assert.False (Application.OnKeyDown (Key.CursorDown)); // Should not change (should focus next view if there was one, which there isn't) + Assert.Equal (0, rg.SelectedItem); + Assert.Equal (1, rg.Cursor); + + // Now test Select (Space) when Cursor != SelectedItem Assert.True (Application.OnKeyDown (Key.Space)); Assert.Equal (1, rg.SelectedItem); - Assert.False (Application.OnKeyDown (Key.CursorDown)); // Should not change (should focus prev view if there was one, which there isn't) - Assert.True (Application.OnKeyDown (Key.Space)); - Assert.Equal (1, rg.SelectedItem); - Assert.True (Application.OnKeyDown (Key.Home)); + Assert.Equal (1, rg.Cursor); + + // Now test Select (Space) when Cursor == SelectedItem - Should cycle Assert.True (Application.OnKeyDown (Key.Space)); Assert.Equal (0, rg.SelectedItem); + Assert.Equal (0, rg.Cursor); + Assert.True (Application.OnKeyDown (Key.Space)); + Assert.Equal (1, rg.SelectedItem); + Assert.Equal (1, rg.Cursor); + Assert.True (Application.OnKeyDown (Key.Space)); + Assert.Equal (0, rg.SelectedItem); + Assert.Equal (0, rg.Cursor); + Assert.True (Application.OnKeyDown (Key.Space)); + Assert.Equal (1, rg.SelectedItem); + Assert.Equal (1, rg.Cursor); + + Assert.True (Application.OnKeyDown (Key.Home)); + Assert.Equal (1, rg.SelectedItem); + Assert.Equal (0, rg.Cursor); + Assert.True (Application.OnKeyDown (Key.Space)); + Assert.Equal (0, rg.SelectedItem); + Assert.Equal (0, rg.Cursor); + Assert.True (Application.OnKeyDown (Key.End)); + Assert.Equal (0, rg.SelectedItem); + Assert.Equal (1, rg.Cursor); Assert.True (Application.OnKeyDown (Key.Space)); Assert.Equal (1, rg.SelectedItem); - Assert.True (Application.OnKeyDown (Key.Space)); - Assert.Equal (1, rg.SelectedItem); + Assert.Equal (1, rg.Cursor); + Application.ResetState (ignoreDisposed: true); } @@ -184,7 +215,7 @@ public class RadioGroupTests (ITestOutputHelper output) } [Fact] - public void HotKey_For_Item_SetsFocus () + public void HotKey_For_Item_Does_Not_SetFocus () { var superView = new View { @@ -200,7 +231,7 @@ public class RadioGroupTests (ITestOutputHelper output) group.NewKeyDownEvent (Key.R); Assert.Equal (1, group.SelectedItem); - Assert.True (group.HasFocus); + Assert.False (group.HasFocus); } [Fact] diff --git a/docfx/docs/migratingfromv1.md b/docfx/docs/migratingfromv1.md index c89655988..3c27d141b 100644 --- a/docfx/docs/migratingfromv1.md +++ b/docfx/docs/migratingfromv1.md @@ -247,6 +247,7 @@ In v2, the API is (NOT YET IMPLEMENTED) simplified. A view simply reports the st See [navigation.md](navigation.md) for more details. See also [Keyboard](keyboard.md) where HotKey is covered more deeply... +* In v1, `View.CanFocus` was `true` by default. In v2, it is `false`. Any `View` subclass that wants to be focusable must set `CanFocus = true`. * In v1 it was not possible to remove focus from a view. `HasFocus` as a get-only property. In v2, `view.HasFocus` can be set as well. Setting to `true` is equivalent to calling `view.SetFocus`. Setting to `false` is equivalent to calling `view.SuperView.AdvanceFocus` (which might not actually cause `view` to stop having focus). * In v1, calling `super.Add (view)` where `view.CanFocus == true` caused all views up the hierarchy (all SuperViews) to get `CanFocus` set to `true` as well. In v2, developers need to explicitly set `CanFocus` for any view in the view-hierarchy where focus is desired. This simplifies the implementation and removes confusing automatic behavior. * In v1, if `view.CanFocus == true`, `Add` would automatically set `TabStop`. In v2, the automatic setting of `TabStop` in `Add` is retained because it is not overly complex to do so and is a nice convenience for developers to not have to set both `Tabstop` and `CanFocus`. Note v2 does NOT automatically change `CanFocus` if `TabStop` is changed. @@ -262,8 +263,9 @@ See also [Keyboard](keyboard.md) where HotKey is covered more deeply... ### How to Fix (Focus API) +* Set @Terminal.Gui.View.CanFocus to `true` for any View sub-class that wants to be focusable. * Use @Terminal.Gui.Application.Navigation.GetFocused to get the most focused view in the application. -* Use @Terminal.Gui.Application.Navigation.AdvanceFocus to cause focus to change. +* Use @Terminal.Gui.Application.Navigation.AdvanceFocus to cause focus to change. ### Keyboard Navigation diff --git a/docfx/docs/navigation.md b/docfx/docs/navigation.md index 7b211b300..56ab96de4 100644 --- a/docfx/docs/navigation.md +++ b/docfx/docs/navigation.md @@ -652,7 +652,7 @@ Like `Checkbox` the right thing to do is for Hotkey to NOT set focus. Why? If th ### `HasFocus` -* `Enter` - `Command.Accept` -> Advances state to selected RadioItem and Raises `Accept` +* `Enter` - `Command.Accept` -> Raises `Accept` * `Space` - `Command.Select` -> Advances state * `Title.Hotkey` - `Command.Hotkey` -> does nothing * `RadioItem.Hotkey` - `Command.Select` -> Advance State to RadioItem with hotkey.