Nuked OnInvokingKeyBindings from RadioGroup

Terminal.Gui/View/ViewKeyboard.cs

@@ -129,9 +129,10 @@ public partial class View
     /// </remarks>
     /// <param name="prevHotKey">The HotKey <paramref name="hotKey"/> is replacing. Key bindings for this key will be removed.</param>
     /// <param name="hotKey">The new HotKey. If <see cref="Key.Empty"/> <paramref name="prevHotKey"/> bindings will be removed.</param>
+    /// <param name="context">Arbitrary context that can be associated with this key binding.</param>
     /// <returns><see langword="true"/> if the HotKey bindings were added.</returns>
     /// <exception cref="ArgumentException"></exception>
-    public virtual bool AddKeyBindingsForHotKey (Key prevHotKey, Key hotKey)
+    public virtual bool AddKeyBindingsForHotKey (Key prevHotKey, Key hotKey, [CanBeNull] object context = null)
     {
         if (_hotKey == hotKey)
         {
@@ -194,15 +195,16 @@ public partial class View
         // Add the new 
         if (newKey != Key.Empty)
         {
+            KeyBinding keyBinding = new ([Command.HotKey], KeyBindingScope.HotKey, context);
             // Add the base and Alt key
-            KeyBindings.Add (newKey, KeyBindingScope.HotKey, Command.HotKey);
-            KeyBindings.Add (newKey.WithAlt, KeyBindingScope.HotKey, Command.HotKey);
+            KeyBindings.Add (newKey, keyBinding);
+            KeyBindings.Add (newKey.WithAlt, keyBinding);
 
             // If the Key is A..Z, add ShiftMask and AltMask | ShiftMask
             if (newKey.IsKeyCodeAtoZ)
             {
-                KeyBindings.Add (newKey.WithShift, KeyBindingScope.HotKey, Command.HotKey);
-                KeyBindings.Add (newKey.WithShift.WithAlt, KeyBindingScope.HotKey, Command.HotKey);
+                KeyBindings.Add (newKey.WithShift, keyBinding);
+                KeyBindings.Add (newKey.WithShift.WithAlt, keyBinding);
             }
         }
 

Terminal.Gui/Views/RadioGroup.cs

@@ -65,16 +65,32 @@ public class RadioGroup : View
                     Command.Accept,
                     () =>
                     {
-                        SelectItem ();
+                        SelectedItem = _cursor;
+
                         return !OnAccept ();
                     }
                    );
 
+        AddCommand (
+                    Command.HotKey,
+                    ctx =>
+                    {
+                        SetFocus ();
+                        if (ctx.KeyBinding?.Context is { } && (int)ctx.KeyBinding?.Context! < _radioLabels.Count)
+                        {
+                            SelectedItem = (int)ctx.KeyBinding?.Context!;
+
+                            return !OnAccept();
+                        }
+
+                        return true;
+                    });
+
         SetupKeyBindings ();
 
         LayoutStarted += RadioGroup_LayoutStarted;
 
-        HighlightStyle = Gui.HighlightStyle.PressedOutside | Gui.HighlightStyle.Pressed;
+        HighlightStyle = HighlightStyle.PressedOutside | HighlightStyle.Pressed;
 
         MouseClick += RadioGroup_MouseClick;
     }
@@ -84,6 +100,7 @@ public class RadioGroup : View
     private void SetupKeyBindings ()
     {
         KeyBindings.Clear ();
+
         // Default keybindings for this view
         if (Orientation == Orientation.Vertical)
         {
@@ -95,6 +112,7 @@ public class RadioGroup : View
             KeyBindings.Add (Key.CursorLeft, Command.LineUp);
             KeyBindings.Add (Key.CursorRight, Command.LineDown);
         }
+
         KeyBindings.Add (Key.Home, Command.TopHome);
         KeyBindings.Add (Key.End, Command.BottomEnd);
         KeyBindings.Add (Key.Space, Command.Accept);
@@ -179,11 +197,13 @@ public class RadioGroup : View
             int prevCount = _radioLabels.Count;
             _radioLabels = value.ToList ();
 
-            foreach (string label in _radioLabels)
+            for (var index = 0; index < _radioLabels.Count; index++)
             {
+                string label = _radioLabels [index];
+
                 if (TextFormatter.FindHotKey (label, HotKeySpecifier, out _, out Key hotKey))
                 {
-                    AddKeyBindingsForHotKey (Key.Empty, hotKey);
+                    AddKeyBindingsForHotKey (Key.Empty, hotKey, index);
                 }
             }
 
@@ -192,7 +212,7 @@ public class RadioGroup : View
         }
     }
 
-    /// <inheritdoc />
+    /// <inheritdoc/>
     public override string Text
     {
         get
@@ -201,6 +221,7 @@ public class RadioGroup : View
             {
                 return string.Empty;
             }
+
             // Return labels as a CSV string
             return string.Join (",", _radioLabels);
         }
@@ -220,73 +241,51 @@ public class RadioGroup : View
     /// <summary>The currently selected item from the list of radio labels</summary>
     /// <value>The selected.</value>
     public int SelectedItem
-{
-    get => _selected;
-    set
     {
-        OnSelectedItemChanged (value, SelectedItem);
-        _cursor = Math.Max (_selected, 0);
-        SetNeedsDisplay ();
+        get => _selected;
+        set
+        {
+            OnSelectedItemChanged (value, SelectedItem);
+            _cursor = Math.Max (_selected, 0);
+            SetNeedsDisplay ();
+        }
     }
-}
 
-/// <inheritdoc/>
-public override void OnDrawContent (Rectangle viewport)
-{
-    base.OnDrawContent (viewport);
+    /// <inheritdoc/>
+    public override void OnDrawContent (Rectangle viewport)
+    {
+        base.OnDrawContent (viewport);
 
-    Driver.SetAttribute (GetNormalColor ());
+        Driver.SetAttribute (GetNormalColor ());
 
-    for (var i = 0; i < _radioLabels.Count; i++)
-    {
-        switch (Orientation)
+        for (var i = 0; i < _radioLabels.Count; i++)
         {
-            case Orientation.Vertical:
-                Move (0, i);
+            switch (Orientation)
+            {
+                case Orientation.Vertical:
+                    Move (0, i);
 
-                break;
-            case Orientation.Horizontal:
-                Move (_horizontal [i].pos, 0);
+                    break;
+                case Orientation.Horizontal:
+                    Move (_horizontal [i].pos, 0);
 
-                break;
-        }
-
-        string rl = _radioLabels [i];
-        Driver.SetAttribute (GetNormalColor ());
-        Driver.AddStr ($"{(i == _selected ? Glyphs.Selected : Glyphs.UnSelected)} ");
-        TextFormatter.FindHotKey (rl, HotKeySpecifier, out int hotPos, out Key hotKey);
+                    break;
+            }
 
-        if (hotPos != -1 && hotKey != Key.Empty)
-        {
-            Rune [] rlRunes = rl.ToRunes ();
+            string rl = _radioLabels [i];
+            Driver.SetAttribute (GetNormalColor ());
+            Driver.AddStr ($"{(i == _selected ? Glyphs.Selected : Glyphs.UnSelected)} ");
+            TextFormatter.FindHotKey (rl, HotKeySpecifier, out int hotPos, out Key hotKey);
 
-            for (var j = 0; j < rlRunes.Length; j++)
+            if (hotPos != -1 && hotKey != Key.Empty)
             {
-                Rune rune = rlRunes [j];
+                Rune [] rlRunes = rl.ToRunes ();
 
-                if (j == hotPos && i == _cursor)
-                {
-                    Application.Driver.SetAttribute (
-                                                     HasFocus
-                                                         ? ColorScheme.HotFocus
-                                                         : GetHotNormalColor ()
-                                                    );
-                }
-                else if (j == hotPos && i != _cursor)
+                for (var j = 0; j < rlRunes.Length; j++)
                 {
-                    Application.Driver.SetAttribute (GetHotNormalColor ());
-                }
-                else if (HasFocus && i == _cursor)
-                {
-                    Application.Driver.SetAttribute (ColorScheme.Focus);
-                }
+                    Rune rune = rlRunes [j];
 
-                if (rune == HotKeySpecifier && j + 1 < rlRunes.Length)
-                {
-                    j++;
-                    rune = rlRunes [j];
-
-                    if (i == _cursor)
+                    if (j == hotPos && i == _cursor)
                     {
                         Application.Driver.SetAttribute (
                                                          HasFocus
@@ -294,185 +293,177 @@ public override void OnDrawContent (Rectangle viewport)
                                                              : GetHotNormalColor ()
                                                         );
                     }
-                    else if (i != _cursor)
+                    else if (j == hotPos && i != _cursor)
                     {
                         Application.Driver.SetAttribute (GetHotNormalColor ());
                     }
-                }
+                    else if (HasFocus && i == _cursor)
+                    {
+                        Application.Driver.SetAttribute (ColorScheme.Focus);
+                    }
+
+                    if (rune == HotKeySpecifier && j + 1 < rlRunes.Length)
+                    {
+                        j++;
+                        rune = rlRunes [j];
+
+                        if (i == _cursor)
+                        {
+                            Application.Driver.SetAttribute (
+                                                             HasFocus
+                                                                 ? ColorScheme.HotFocus
+                                                                 : GetHotNormalColor ()
+                                                            );
+                        }
+                        else if (i != _cursor)
+                        {
+                            Application.Driver.SetAttribute (GetHotNormalColor ());
+                        }
+                    }
 
-                Application.Driver.AddRune (rune);
-                Driver.SetAttribute (GetNormalColor ());
+                    Application.Driver.AddRune (rune);
+                    Driver.SetAttribute (GetNormalColor ());
+                }
+            }
+            else
+            {
+                DrawHotString (rl, HasFocus && i == _cursor, ColorScheme);
             }
-        }
-        else
-        {
-            DrawHotString (rl, HasFocus && i == _cursor, ColorScheme);
         }
     }
-}
 
-/// <inheritdoc/>
-public override bool? OnInvokingKeyBindings (Key keyEvent)
-{
-    // TODO: Use CommandContext
-    // This is a bit of a hack. We want to handle the key bindings for the radio group but
-    // InvokeKeyBindings doesn't pass any context so we can't tell if the key binding is for
-    // the radio group or for one of the radio buttons. So before we call the base class
-    // we set SelectedItem appropriately.
-
-    Key key = keyEvent;
-
-    if (KeyBindings.TryGet (key, out _))
+    /// <summary>Called when the view orientation has changed. Invokes the <see cref="OrientationChanged"/> event.</summary>
+    /// <param name="newOrientation"></param>
+    /// <returns>True of the event was cancelled.</returns>
+    public virtual bool OnOrientationChanged (Orientation newOrientation)
     {
-        // Search RadioLabels 
-        for (var i = 0; i < _radioLabels.Count; i++)
+        var args = new OrientationEventArgs (newOrientation);
+        OrientationChanged?.Invoke (this, args);
+
+        if (!args.Cancel)
         {
-            if (TextFormatter.FindHotKey (
-                                          _radioLabels [i],
-                                          HotKeySpecifier,
-                                          out _,
-                                          out Key hotKey,
-                                          true
-                                         )
-                && key.NoAlt.NoCtrl.NoShift == hotKey)
-            {
-                SelectedItem = i;
-                break;
-            }
+            _orientation = newOrientation;
+            SetupKeyBindings ();
+            SetContentSize ();
         }
-    }
-
-    return base.OnInvokingKeyBindings (keyEvent);
-}
 
-/// <summary>Called when the view orientation has changed. Invokes the <see cref="OrientationChanged"/> event.</summary>
-/// <param name="newOrientation"></param>
-/// <returns>True of the event was cancelled.</returns>
-public virtual bool OnOrientationChanged (Orientation newOrientation)
-{
-    var args = new OrientationEventArgs (newOrientation);
-    OrientationChanged?.Invoke (this, args);
+        return args.Cancel;
+    }
 
-    if (!args.Cancel)
+    // TODO: This should be cancelable
+    /// <summary>Called whenever the current selected item changes. Invokes the <see cref="SelectedItemChanged"/> event.</summary>
+    /// <param name="selectedItem"></param>
+    /// <param name="previousSelectedItem"></param>
+    public virtual void OnSelectedItemChanged (int selectedItem, int previousSelectedItem)
     {
-        _orientation = newOrientation;
-        SetupKeyBindings ();
-        SetContentSize ();
+        _selected = selectedItem;
+        SelectedItemChanged?.Invoke (this, new (selectedItem, previousSelectedItem));
     }
 
-    return args.Cancel;
-}
+    /// <summary>
+    ///     Fired when the view orientation has changed. Can be cancelled by setting
+    ///     <see cref="OrientationEventArgs.Cancel"/> to true.
+    /// </summary>
+    public event EventHandler<OrientationEventArgs> OrientationChanged;
 
-// TODO: This should be cancelable
-/// <summary>Called whenever the current selected item changes. Invokes the <see cref="SelectedItemChanged"/> event.</summary>
-/// <param name="selectedItem"></param>
-/// <param name="previousSelectedItem"></param>
-public virtual void OnSelectedItemChanged (int selectedItem, int previousSelectedItem)
-{
-    _selected = selectedItem;
-    SelectedItemChanged?.Invoke (this, new SelectedItemChangedArgs (selectedItem, previousSelectedItem));
-}
+    /// <inheritdoc/>
+    public override Point? PositionCursor ()
+    {
+        var x = 0;
+        var y = 0;
 
-/// <summary>
-///     Fired when the view orientation has changed. Can be cancelled by setting
-///     <see cref="OrientationEventArgs.Cancel"/> to true.
-/// </summary>
-public event EventHandler<OrientationEventArgs> OrientationChanged;
+        switch (Orientation)
+        {
+            case Orientation.Vertical:
+                y = _cursor;
 
-/// <inheritdoc/>
-public override Point? PositionCursor ()
-{
-    int x = 0;
-    int y = 0;
-    switch (Orientation)
-    {
-        case Orientation.Vertical:
-            y = _cursor;
+                break;
+            case Orientation.Horizontal:
+                x = _horizontal [_cursor].pos;
 
-            break;
-        case Orientation.Horizontal:
-            x = _horizontal [_cursor].pos;
+                break;
 
-            break;
+            default:
+                return null;
+        }
 
-        default:
-            return null;
-    }
+        Move (x, y);
 
-    Move (x, y);
-    return null; // Don't show the cursor
-}
+        return null; // Don't show the cursor
+    }
 
-/// <summary>Allow to invoke the <see cref="SelectedItemChanged"/> after their creation.</summary>
-public void Refresh () { OnSelectedItemChanged (_selected, -1); }
+    /// <summary>Allow to invoke the <see cref="SelectedItemChanged"/> after their creation.</summary>
+    public void Refresh () { OnSelectedItemChanged (_selected, -1); }
 
-// TODO: This should use StateEventArgs<int> and should be cancelable.
-/// <summary>Invoked when the selected radio label has changed.</summary>
-public event EventHandler<SelectedItemChangedArgs> SelectedItemChanged;
+    // TODO: This should use StateEventArgs<int> and should be cancelable.
+    /// <summary>Invoked when the selected radio label has changed.</summary>
+    public event EventHandler<SelectedItemChangedArgs> SelectedItemChanged;
 
-private void MoveDownRight ()
-{
-    if (_cursor + 1 < _radioLabels.Count)
+    private void MoveDownRight ()
     {
-        _cursor++;
-        SetNeedsDisplay ();
-    }
-    else if (_cursor > 0)
-    {
-        _cursor = 0;
-        SetNeedsDisplay ();
+        if (_cursor + 1 < _radioLabels.Count)
+        {
+            _cursor++;
+            SetNeedsDisplay ();
+        }
+        else if (_cursor > 0)
+        {
+            _cursor = 0;
+            SetNeedsDisplay ();
+        }
     }
-}
 
-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 void MoveUpLeft ()
-{
-    if (_cursor > 0)
+    private void MoveUpLeft ()
     {
-        _cursor--;
-        SetNeedsDisplay ();
-    }
-    else if (_radioLabels.Count - 1 > 0)
-    {
-        _cursor = _radioLabels.Count - 1;
-        SetNeedsDisplay ();
+        if (_cursor > 0)
+        {
+            _cursor--;
+            SetNeedsDisplay ();
+        }
+        else if (_radioLabels.Count - 1 > 0)
+        {
+            _cursor = _radioLabels.Count - 1;
+            SetNeedsDisplay ();
+        }
     }
-}
 
-private void RadioGroup_LayoutStarted (object sender, EventArgs e) { SetContentSize (); }
-private void SelectItem () { SelectedItem = _cursor; }
+    private void RadioGroup_LayoutStarted (object sender, EventArgs e) { SetContentSize (); }
 
-private void SetContentSize ()
-{
-    switch (_orientation)
+    private void SetContentSize ()
     {
-        case Orientation.Vertical:
-            var width = 0;
+        switch (_orientation)
+        {
+            case Orientation.Vertical:
+                var width = 0;
 
-            foreach (string s in _radioLabels)
-            {
-                width = Math.Max (s.GetColumns () + 2, width);
-            }
+                foreach (string s in _radioLabels)
+                {
+                    width = Math.Max (s.GetColumns () + 2, width);
+                }
 
-            SetContentSize (new (width, _radioLabels.Count));
-            break;
+                SetContentSize (new (width, _radioLabels.Count));
 
-        case Orientation.Horizontal:
-            _horizontal = new List<(int pos, int length)> ();
-            var start = 0;
-            var length = 0;
+                break;
 
-            for (var i = 0; i < _radioLabels.Count; i++)
-            {
-                start += length;
+            case Orientation.Horizontal:
+                _horizontal = new ();
+                var start = 0;
+                var length = 0;
 
-                length = _radioLabels [i].GetColumns () + 2 + (i < _radioLabels.Count - 1 ? _horizontalSpace : 0);
-                _horizontal.Add ((start, length));
-            }
-            SetContentSize (new (_horizontal.Sum (item => item.length), 1));
-            break;
+                for (var i = 0; i < _radioLabels.Count; i++)
+                {
+                    start += length;
+
+                    length = _radioLabels [i].GetColumns () + 2 + (i < _radioLabels.Count - 1 ? _horizontalSpace : 0);
+                    _horizontal.Add ((start, length));
+                }
+
+                SetContentSize (new (_horizontal.Sum (item => item.length), 1));
+
+                break;
+        }
     }
 }
-}

UnitTests/Views/RadioGroupTests.cs

@@ -14,12 +14,12 @@ public class RadioGroupTests (ITestOutputHelper output)
         Assert.Equal (Rectangle.Empty, rg.Frame);
         Assert.Equal (0, rg.SelectedItem);
 
-        rg = new() { RadioLabels = new [] { "Test" } };
+        rg = new () { RadioLabels = new [] { "Test" } };
         Assert.True (rg.CanFocus);
         Assert.Single (rg.RadioLabels);
         Assert.Equal (0, rg.SelectedItem);
 
-        rg = new()
+        rg = new ()
         {
             X = 1,
             Y = 2,
@@ -32,7 +32,7 @@ public class RadioGroupTests (ITestOutputHelper output)
         Assert.Equal (new (1, 2, 20, 5), rg.Frame);
         Assert.Equal (0, rg.SelectedItem);
 
-        rg = new() { X = 1, Y = 2, RadioLabels = new [] { "Test" } };
+        rg = new () { X = 1, Y = 2, RadioLabels = new [] { "Test" } };
 
         var view = new View { Width = 30, Height = 40 };
         view.Add (rg);
@@ -130,6 +130,51 @@ public class RadioGroupTests (ITestOutputHelper output)
         Assert.Equal (1, rg.SelectedItem);
     }
 
+    [Fact]
+    public void HotKey_For_View_SetsFocus ()
+    {
+        var superView = new View
+        {
+            CanFocus = true
+        };
+        superView.Add (new View { CanFocus = true });
+
+        var group = new RadioGroup
+        {
+            Title = "Radio_Group",
+            RadioLabels = new [] { "_Left", "_Right", "Cen_tered", "_Justified" }
+        };
+        superView.Add (group);
+
+        Assert.False (group.HasFocus);
+        Assert.Equal (0, group.SelectedItem);
+
+        group.NewKeyDownEvent (Key.G.WithAlt);
+
+        Assert.Equal (0, group.SelectedItem);
+        Assert.True (group.HasFocus);
+    }
+
+    [Fact]
+    public void HotKey_For_Item_SetsFocus ()
+    {
+        var superView = new View
+        {
+            CanFocus = true
+        };
+        superView.Add (new View { CanFocus = true });
+        var group = new RadioGroup { RadioLabels = new [] { "_Left", "_Right", "Cen_tered", "_Justified" } };
+        superView.Add (group);
+
+        Assert.False (group.HasFocus);
+        Assert.Equal (0, group.SelectedItem);
+
+        group.NewKeyDownEvent (Key.R);
+
+        Assert.Equal (1, group.SelectedItem);
+        Assert.True (group.HasFocus);
+    }
+
     [Fact]
     public void HotKey_Command_Does_Not_Accept ()
     {
@@ -184,12 +229,8 @@ public class RadioGroupTests (ITestOutputHelper output)
 
         var expected = @$"
 ┌────────────────────────────┐
-│{
-    CM.Glyphs.Selected
-} Test                      │
-│{
-    CM.Glyphs.UnSelected
-} New Test 你               │
+│{CM.Glyphs.Selected} Test                      │
+│{CM.Glyphs.UnSelected} New Test 你               │
 │                            │
 └────────────────────────────┘
 ";
@@ -209,11 +250,7 @@ public class RadioGroupTests (ITestOutputHelper output)
 
         expected = @$"
 ┌────────────────────────────┐
-│{
-    CM.Glyphs.Selected
-} Test  {
-    CM.Glyphs.UnSelected
-} New Test 你       │
+│{CM.Glyphs.Selected} Test  {CM.Glyphs.UnSelected} New Test 你       │
 │                            │
 │                            │
 └────────────────────────────┘
@@ -234,11 +271,7 @@ public class RadioGroupTests (ITestOutputHelper output)
 
         expected = @$"
 ┌────────────────────────────┐
-│{
-    CM.Glyphs.Selected
-} Test    {
-    CM.Glyphs.UnSelected
-} New Test 你     │
+│{CM.Glyphs.Selected} Test    {CM.Glyphs.UnSelected} New Test 你     │
 │                            │
 │                            │
 └────────────────────────────┘