RadioGroup

Terminal.Gui/View/View.Keyboard.cs

@@ -14,10 +14,15 @@ public partial class View // Keyboard APIs
         HotKeySpecifier = (Rune)'_';
         TitleTextFormatter.HotKeyChanged += TitleTextFormatter_HotKeyChanged;
 
-        // TODO: It's incorrect to think of Commands as being Keyboard things. The code below should be moved to View.cs
+        // By default, the HotKey command sets the focus
+        AddCommand (Command.Select, () =>
+                                    {
+                                        SetFocus ();
+                                        return OnSelect ();
+                                    });
 
         // By default, the HotKey command sets the focus
-        AddCommand (Command.HotKey, OnHotKey);
+        AddCommand (Command.HotKey, () => SetFocus ());
 
         // By default, the Accept command raises the Accept event
         AddCommand (Command.Accept, OnAccept);
@@ -30,22 +35,6 @@ public partial class View // Keyboard APIs
 
     #region HotKey Support
 
-    /// <summary>
-    ///     Called when the HotKey command (<see cref="Command.HotKey"/>) is invoked. Causes this view to be focused.
-    /// </summary>
-    /// <returns>If <see langword="true"/> the command was canceled.</returns>
-    private bool? OnHotKey ()
-    {
-        if (CanFocus)
-        {
-            SetFocus ();
-
-            return true;
-        }
-
-        return false;
-    }
-
     /// <summary>Invoked when the <see cref="HotKey"/> is changed.</summary>
     public event EventHandler<KeyChangedEventArgs>? HotKeyChanged;
 
@@ -616,7 +605,7 @@ public partial class View // Keyboard APIs
         // Now, process any key bindings in the subviews that are tagged to KeyBindingScope.HotKey.
         foreach (View subview in Subviews)
         {
-            if (subview.HasFocus)
+            if (subview == Focused)
             {
                 continue;
             }

Terminal.Gui/View/View.cs

@@ -108,6 +108,13 @@ namespace Terminal.Gui;
 
 public partial class View : Responder, ISupportInitializeNotification
 {
+    /// <summary>
+    ///     Cancelable event fired when the <see cref="Command.Select"/> command is invoked. Set
+    ///     <see cref="HandledEventArgs.Handled"/>
+    ///     to cancel the event.
+    /// </summary>
+    public event EventHandler<HandledEventArgs>? Select;
+
     /// <summary>
     ///     Cancelable event fired when the <see cref="Command.Accept"/> command is invoked. Set
     ///     <see cref="HandledEventArgs.Handled"/>
@@ -163,6 +170,23 @@ public partial class View : Responder, ISupportInitializeNotification
         return Accept is null ? null : args.Handled;
     }
 
+
+    /// <summary>
+    ///     Called when the <see cref="Command.Accept"/> command is invoked. Raises <see cref="Accept"/>
+    ///     event.
+    /// </summary>
+    /// <returns>
+    ///     If <see langword="true"/> the event was canceled. If <see langword="false"/> the event was raised but not canceled.
+    ///     If <see langword="null"/> no event was raised.
+    /// </returns>
+    protected bool? OnSelect ()
+    {
+        var args = new HandledEventArgs ();
+        Select?.Invoke (this, args);
+
+        return Select is null ? null : args.Handled;
+    }
+
     #region Constructors and Initialization
 
     /// <summary>
@@ -287,7 +311,7 @@ public partial class View : Responder, ISupportInitializeNotification
         Initialized?.Invoke (this, EventArgs.Empty);
     }
 
-#endregion Constructors and Initialization
+    #endregion Constructors and Initialization
 
     #region Visibility
 
@@ -354,7 +378,8 @@ public partial class View : Responder, ISupportInitializeNotification
 
     private bool _visible = true;
 
-    /// <summary>Gets or sets a value indicating whether this <see cref="Responder"/> and all its child controls are displayed.</summary>
+    // TODO: Remove virtual once Menu/MenuBar are removed. MenuBar is the only override.
+    /// <summary>Gets or sets a value indicating whether this <see cref="View"/> is visible.</summary>
     public virtual bool Visible
     {
         get => _visible;
@@ -365,6 +390,18 @@ public partial class View : Responder, ISupportInitializeNotification
                 return;
             }
 
+            if (OnVisibleChanging ())
+            {
+                return;
+            }
+
+            CancelEventArgs<bool> args = new (in _visible, ref value);
+            VisibleChanging?.Invoke (this, args);
+            if (args.Cancel)
+            {
+                return;
+            }
+
             _visible = value;
 
             if (!_visible)
@@ -382,14 +419,22 @@ public partial class View : Responder, ISupportInitializeNotification
             }
 
             OnVisibleChanged ();
+            VisibleChanged?.Invoke (this, EventArgs.Empty);
+
             SetNeedsDisplay ();
         }
     }
 
-    /// <summary>Method invoked when the <see cref="Visible"/> property from a view is changed.</summary>
-    public virtual void OnVisibleChanged () { VisibleChanged?.Invoke (this, EventArgs.Empty); }
+    /// <summary>Called when <see cref="Visible"/> is changing. Can be cancelled by returning <see langword="true"/>.</summary>
+    protected virtual bool OnVisibleChanging () { return false; }
+
+    /// <summary>Raised when the <see cref="Visible"/> value is being changed. Can be cancelled by setting Cancel to <see langword="true"/>.</summary>
+    public event EventHandler<CancelEventArgs<bool>>? VisibleChanging;
+
+    /// <summary>Called when <see cref="Visible"/> has changed.</summary>
+    protected virtual void OnVisibleChanged () { }
 
-    /// <summary>Event fired when the <see cref="Visible"/> value is being changed.</summary>
+    /// <summary>Raised when <see cref="Visible"/> has changed.</summary>
     public event EventHandler? VisibleChanged;
 
     // TODO: This API is a hack. We should make Visible propogate automatically, no? See https://github.com/gui-cs/Terminal.Gui/issues/3703
@@ -416,7 +461,7 @@ public partial class View : Responder, ISupportInitializeNotification
         return true;
     }
 
-#endregion Visibility
+    #endregion Visibility
 
     #region Title
 

Terminal.Gui/Views/Menu/Menu.cs

@@ -330,7 +330,7 @@ internal sealed class Menu : View
     }
 
     /// <inheritdoc/>
-    public override void OnVisibleChanged ()
+    protected override void OnVisibleChanged ()
     {
         base.OnVisibleChanged ();
 

Terminal.Gui/Views/RadioGroup.cs

@@ -75,31 +75,43 @@ public class RadioGroup : View, IDesignable, IOrientation
                         return true;
                     }
                    );
+        AddCommand (
+                    Command.Select,
+                    () =>
+                    {
+                        if (SelectedItem == _cursor)
+                        {
+                            if (!MoveDownRight ())
+                            {
+                                MoveHome ();
+                            }
+                        }
+
+                        SelectedItem = _cursor;
 
+                        return true;
+                    });
         AddCommand (
                     Command.Accept,
                     () =>
                     {
                         SelectedItem = _cursor;
 
-                        return OnAccept () is true or null;
+                        return OnAccept () is false;
                     }
                    );
-
         AddCommand (
                     Command.HotKey,
                     ctx =>
                     {
-                        SetFocus ();
-
                         if (ctx.KeyBinding?.Context is { } && (int)ctx.KeyBinding?.Context! < _radioLabels.Count)
                         {
                             SelectedItem = (int)ctx.KeyBinding?.Context!;
 
-                            return OnAccept () is true or null;
+                            return OnSelect () is true or null;
                         }
 
-                        return true;
+                        return !SetFocus ();
                     });
 
         _orientationHelper = new (this);
@@ -136,7 +148,8 @@ public class RadioGroup : View, IDesignable, IOrientation
 
         KeyBindings.Add (Key.Home, Command.Start);
         KeyBindings.Add (Key.End, Command.End);
-        KeyBindings.Add (Key.Space, Command.Accept);
+        KeyBindings.Add (Key.Enter, Command.Accept);
+        KeyBindings.Add (Key.Space, Command.Select);
     }
 
     private void RadioGroup_MouseClick (object sender, MouseEventEventArgs e)

UICatalog/Scenarios/MessageBoxes.cs

@@ -30,7 +30,7 @@ public class MessageBoxes : Scenario
         app.Add (frame);
 
         // TODO: Use Pos.Align her to demo aligning labels and fields
-        var label = new Label { X = 0, Y = 0, Width = 15, TextAlignment = Alignment.End, Text = "Width:" };
+        var label = new Label { X = 0, Y = 0, Width = 15, TextAlignment = Alignment.End, Text = "W_idth:" };
         frame.Add (label);
 
         var widthEdit = new TextField
@@ -50,7 +50,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Height:"
+            Text = "_Height:"
         };
         frame.Add (label);
 
@@ -90,7 +90,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Title:"
+            Text = "_Title:"
         };
         frame.Add (label);
 
@@ -100,7 +100,7 @@ public class MessageBoxes : Scenario
             Y = Pos.Top (label),
             Width = Dim.Fill (),
             Height = 1,
-            Text = "Title"
+            Text = "The title"
         };
         frame.Add (titleEdit);
 
@@ -112,7 +112,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Message:"
+            Text = "_Message:"
         };
         frame.Add (label);
 
@@ -134,7 +134,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Num Buttons:"
+            Text = "_Num Buttons:"
         };
         frame.Add (label);
 
@@ -156,7 +156,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Default Button:"
+            Text = "_Default Button:"
         };
         frame.Add (label);
 
@@ -178,7 +178,7 @@ public class MessageBoxes : Scenario
             Width = Dim.Width (label),
             Height = 1,
             TextAlignment = Alignment.End,
-            Text = "Style:"
+            Text = "St_yle:"
         };
         frame.Add (label);
 
@@ -188,11 +188,23 @@ public class MessageBoxes : Scenario
         };
         frame.Add (styleRadioGroup);
 
+        label = new ()
+        {
+            X = 0,
+            Y = Pos.Bottom (styleRadioGroup),
+
+            Width = Dim.Width (label),
+            Height = 1,
+            TextAlignment = Alignment.End,
+            Text = "Wra_p:"
+        };
         var ckbWrapMessage = new CheckBox
         {
-            X = Pos.Right (label) + 1, Y = Pos.Bottom (styleRadioGroup), Text = "_Wrap Message", CheckedState = CheckState.Checked
+            X = Pos.Right (label) + 1, Y = Pos.Bottom (styleRadioGroup),
+            CheckedState = CheckState.Checked,
+            Text = "_Wrap Message",
         };
-        frame.Add (ckbWrapMessage);
+        frame.Add (label, ckbWrapMessage);
 
         frame.ValidatePosDim = true;
 
@@ -216,7 +228,7 @@ public class MessageBoxes : Scenario
             X = Pos.Center (), Y = Pos.Bottom (frame) + 2, IsDefault = true, Text = "_Show MessageBox"
         };
 
-        showMessageBoxButton.Accept += (s, e) =>
+        app.Accept += (s, e) =>
                                        {
                                            try
                                            {
@@ -263,6 +275,8 @@ public class MessageBoxes : Scenario
                                            {
                                                buttonPressedLabel.Text = "Invalid Options";
                                            }
+
+                                           e.Handled = true;
                                        };
         app.Add (showMessageBoxButton);
 

docfx/docs/navigation.md

@@ -625,10 +625,13 @@ In v2_develop it's all kinds of confused. Here's what it SHOULD do:
 
 ### `HasFocus`
 
-* `Enter` - `Command.Accept` -> Raises `Accept` 
+* `Enter` - `Command.Accept` -> Advances state to selected RadioItem and Raises `Accept` 
 * `Space` - `Command.Select` -> Advances state
-* `Hotkey` - `Command.Hotkey` -> Advances state
-* `Click` - Advances state
+* `Title.Hotkey` - `Command.Hotkey` -> does nothing
+* `RadioItem.Hotkey` - `Command.Select` -> Advance State to RadioItem with hotkey.
+* `Click` - advances state to clicked RadioItem.
 * `Double Click` - Advances state to clicked RadioItem and then raises `Accept` (this is what Office does; it's pretty nice. Windows does nothing).
 
-An interesting tid-bit about the above is for `Checkbox` the right thing to do is for Hotkey to NOT set focus. Why? If the user is in a TextField and wants to change a setting via a CheckBox, they should be able to use the hotkey and NOT have to then re-focus back on the TextView. The `TextView` in `Text Input Controls` Scenario is a good example of this.
+Like `Checkbox` the right thing to do is for Hotkey to NOT set focus. Why? If the user is in a TextField and wants to change a setting via a RadioGroup, they should be able to use the hotkey and NOT have to then re-focus back on the TextView. The `TextView` in `Text Input Controls` Scenario is a good example of this.
+
+## `Slider` - Should operate just like RadioGroup