Fixed CheckBox

Terminal.Gui/Views/CheckBox.cs

@@ -107,26 +107,44 @@ public class CheckBox : View
     public CheckState CheckedState
     {
         get => _checkedState;
-        set
+        set => ChangeCheckedState (value);
+    }
+
+    /// <summary>
+    ///     INTERNAL Sets CheckedState.
+    /// </summary>
+    /// <param name="value"></param>
+    /// <returns><see langword="true"/> if state change was canceled, <see langword="false"/> if the state changed, and <see langword="null"/> if the state was not changed for some other reason.</returns>
+    private bool? ChangeCheckedState (CheckState value)
+    {
+        if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
         {
-            if (_checkedState == value || (value is CheckState.None && !AllowCheckStateNone))
-            {
-                return;
-            }
+            return null;
+        }
 
-            _checkedState = value;
-            UpdateTextFormatterText ();
-            OnResizeNeeded ();
+        if (RaiseSelectEvent () == true)
+        {
+            return true;
         }
+
+        CancelEventArgs<CheckState> e = new (in _checkedState, ref value);
+        CheckedStateChanging?.Invoke (this, e);
+        if (e.Cancel)
+        {
+            return e.Cancel;
+        }
+
+        _checkedState = value;
+        UpdateTextFormatterText ();
+        OnResizeNeeded ();
+
+        return false;
     }
 
     /// <summary>
     ///     Advances <see cref="CheckedState"/> to the next value. Invokes the cancelable <see cref="CheckedStateChanging"/> event.
     /// </summary>
     /// <remarks>
-    /// </remarks>
-    /// <returns>If <see langword="true"/> the <see cref="CheckedStateChanging"/> event was canceled.</returns>
-    /// <remarks>
     /// <para>
     ///     Cycles through the states <see cref="CheckState.None"/>, <see cref="CheckState.Checked"/>, and <see cref="CheckState.UnChecked"/>.
     /// </para>
@@ -134,6 +152,7 @@ public class CheckBox : View
     ///     If the <see cref="CheckedStateChanging"/> event is not canceled, the <see cref="CheckedState"/> will be updated and the <see cref="Command.Accept"/> event will be raised.
     /// </para>
     /// </remarks>
+    /// <returns><see langword="true"/> if state change was canceled, <see langword="false"/> if the state changed, and <see langword="null"/> if the state was not changed for some other reason.</returns>
     public bool? AdvanceCheckState ()
     {
         CheckState oldValue = CheckedState;
@@ -162,15 +181,9 @@ public class CheckBox : View
                 break;
         }
 
-        CheckedStateChanging?.Invoke (this, e);
-        if (e.Cancel)
-        {
-            return e.Cancel;
-        }
-
-        CheckedState = e.NewValue;
+        bool? cancelled = ChangeCheckedState (e.NewValue);
 
-        return RaiseSelectEvent ();
+        return !cancelled;
     }
 
     /// <summary>Raised when the <see cref="CheckBox"/> state is changing.</summary>

Terminal.Gui/Views/RadioGroup.cs

@@ -83,16 +83,16 @@ public class RadioGroup : View, IDesignable, IOrientation
                             }
                         }
 
-                        return SetSelectedItem (Cursor);
+                        return ChangeSelectedItem (Cursor);
                     });
 
         AddCommand (
                     Command.Accept,
                     () =>
                     {
-                        if (!SetSelectedItem (Cursor))
+                        if (ChangeSelectedItem (Cursor) == true)
                         {
-                            return false;
+                            return true;
                         }
 
                         return RaiseAcceptEvent () is false;
@@ -122,7 +122,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                             }
 
                             // If a RadioItem.HotKey is pressed we always set the selected item - never SetFocus
-                            if (SetSelectedItem (item.Value))
+                            if (ChangeSelectedItem (item.Value) == true)
                             {
                                 return true;
                             }
@@ -130,7 +130,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                             return false;
                         }
 
-                        if (SelectedItem == -1 && SetSelectedItem (0))
+                        if (SelectedItem == -1 && ChangeSelectedItem (0) == true)
                         {
                             return true;
                         }
@@ -276,24 +276,24 @@ public class RadioGroup : View, IDesignable, IOrientation
     public int SelectedItem
     {
         get => _selected;
-        set => SetSelectedItem (value);
+        set => ChangeSelectedItem (value);
     }
 
     /// <summary>
     ///     INTERNAL Sets the selected item.
     /// </summary>
     /// <param name="value"></param>
-    /// <returns>true if the selection changed.</returns>
-    private bool SetSelectedItem (int value)
+    /// <returns><see langword="true"/> if state change was canceled, <see langword="false"/> if the state changed, and <see langword="null"/> if the state was not changed for some other reason.</returns>
+    private bool? ChangeSelectedItem (int value)
     {
         if (_selected == value || value > _radioLabels.Count - 1)
         {
-            return false;
+            return null;
         }
 
         if (RaiseSelectEvent () == true)
         {
-            return false;
+            return true;
         }
 
         int savedSelected = _selected;
@@ -305,7 +305,7 @@ public class RadioGroup : View, IDesignable, IOrientation
 
         SetNeedsDisplay ();
 
-        return true;
+        return false;
     }
 
     /// <inheritdoc/>

UnitTests/Views/CheckBoxTests.cs

@@ -95,10 +95,12 @@ public class CheckBoxTests (ITestOutputHelper output)
 
     [Fact]
     [SetupFakeDriver]
-    public void AllowNoneChecked_Get_Set ()
+    public void AllowCheckStateNone_Get_Set ()
     {
         var checkBox = new CheckBox { Text = "Check this out 你" };
 
+        checkBox.HasFocus = true;
+        Assert.True (checkBox.HasFocus);
         Assert.Equal (CheckState.UnChecked, checkBox.CheckedState);
         Assert.True (checkBox.NewKeyDownEvent (Key.Space));
         Assert.Equal (CheckState.Checked, checkBox.CheckedState);
@@ -510,7 +512,7 @@ public class CheckBoxTests (ITestOutputHelper output)
     }
 
     [Fact]
-    public void HotKey_Command_Fires_Accept ()
+    public void HotKey_Command_Does_Not_Fire_Accept ()
     {
         var cb = new CheckBox ();
         var accepted = false;
@@ -518,34 +520,64 @@ public class CheckBoxTests (ITestOutputHelper output)
         cb.Accept += CheckBoxOnAccept;
         cb.InvokeCommand (Command.HotKey);
 
-        Assert.True (accepted);
+        Assert.False (accepted);
 
         return;
 
         void CheckBoxOnAccept (object sender, HandledEventArgs e) { accepted = true; }
     }
 
+
     [Theory]
     [InlineData (CheckState.Checked)]
     [InlineData (CheckState.UnChecked)]
     [InlineData (CheckState.None)]
-    public void Toggled_Cancel_Event_Prevents_Toggle (CheckState initialState)
+    public void Select_Handle_Event_Prevents_Change (CheckState initialState)
     {
         var ckb = new CheckBox { AllowCheckStateNone = true };
         var checkedInvoked = false;
 
-        ckb.CheckedStateChanging += CheckBoxToggle;
+        ckb.CheckedState = initialState;
+
+        ckb.Select += OnSelect;
+
+        Assert.Equal (initialState, ckb.CheckedState);
+        bool? ret = ckb.InvokeCommand (Command.Select);
+        Assert.False (ret);
+        Assert.True (checkedInvoked);
+        Assert.Equal (initialState, ckb.CheckedState);
+
+        return;
+
+        void OnSelect (object sender, HandledEventArgs e)
+        {
+            checkedInvoked = true;
+            e.Handled = true;
+        }
+    }
+
+    [Theory]
+    [InlineData (CheckState.Checked)]
+    [InlineData (CheckState.UnChecked)]
+    [InlineData (CheckState.None)]
+    public void CheckedStateChanging_Cancel_Event_Prevents_Change (CheckState initialState)
+    {
+        var ckb = new CheckBox { AllowCheckStateNone = true };
+        var checkedInvoked = false;
 
         ckb.CheckedState = initialState;
+
+        ckb.CheckedStateChanging += OnCheckedStateChanging;
+
         Assert.Equal (initialState, ckb.CheckedState);
         bool? ret = ckb.AdvanceCheckState ();
-        Assert.True (ret);
+        Assert.False (ret);
         Assert.True (checkedInvoked);
         Assert.Equal (initialState, ckb.CheckedState);
 
         return;
 
-        void CheckBoxToggle (object sender, CancelEventArgs e)
+        void OnCheckedStateChanging (object sender, CancelEventArgs e)
         {
             checkedInvoked = true;
             e.Cancel = true;