Beefed up unit tests

Terminal.Gui/View/Orientation/IOrientation.cs

@@ -37,100 +37,4 @@ public interface IOrientation
     /// <param name="newOrientation"></param>
     /// <returns></returns>
     public void OnOrientationChanged (Orientation oldOrientation, Orientation newOrientation) { return; }
-}
-
-
-/// <summary>
-///     Helper class for implementing <see cref="IOrientation"/>.
-/// </summary>
-public class OrientationHelper
-{
-    private Orientation _orientation = Orientation.Vertical;
-    private readonly IOrientation _owner;
-
-    /// <summary>
-    ///     Initializes a new instance of the <see cref="OrientationHelper"/> class.
-    /// </summary>
-    /// <param name="owner"></param>
-    public OrientationHelper (IOrientation owner)
-    {
-        _owner = owner;
-    }
-
-    /// <summary>
-    ///     Gets or sets the orientation of the View.
-    /// </summary>
-    public Orientation Orientation
-    {
-        get => _orientation;
-        set
-        {
-            if (_orientation == value)
-            {
-                return;
-            }
-
-            var args = new CancelEventArgs<Orientation> (in _orientation, ref value);
-            OrientationChanging?.Invoke (_owner, args);
-            if (args.Cancel)
-            {
-                return;
-            }
-
-            if (_owner?.OnOrientationChanging (value, _orientation) ?? false)
-            {
-                return;
-            }
-
-            Orientation old = _orientation;
-            if (_orientation != value)
-            {
-                _orientation = value;
-
-                if (_owner is { })
-                {
-                    _owner.Orientation = value;
-                }
-            }
-
-            args = new CancelEventArgs<Orientation> (in old, ref _orientation);
-            OrientationChanged?.Invoke (_owner, args);
-
-            _owner?.OnOrientationChanged (old, _orientation);
-        }
-    }
-
-    /// <summary>
-    /// 
-    /// </summary>
-    public event EventHandler<CancelEventArgs<Orientation>> OrientationChanging;
-
-    /// <summary>
-    /// 
-    /// </summary>
-    /// <param name="currentOrientation"></param>
-    /// <param name="newOrientation"></param>
-    /// <returns></returns>
-    protected bool OnOrientationChanging (Orientation currentOrientation, Orientation newOrientation)
-    {
-        return _owner?.OnOrientationChanging (currentOrientation, newOrientation) ?? false;
-    }
-
-    /// <summary>
-    /// 
-    /// </summary>
-    public event EventHandler<CancelEventArgs<Orientation>> OrientationChanged;
-
-    /// <summary>
-    /// 
-    /// </summary>
-    /// <param name="oldOrientation"></param>
-    /// <param name="newOrientation"></param>
-    /// <returns></returns>
-    protected void OnOrientationChanged (Orientation oldOrientation, Orientation newOrientation)
-    {
-        _owner?.OnOrientationChanged (oldOrientation, newOrientation);
-    }
-}
-
-
+}

Terminal.Gui/View/Orientation/OrientationHelper.cs

@@ -3,6 +3,14 @@
 /// <summary>
 ///     Helper class for implementing <see cref="IOrientation"/>.
 /// </summary>
+/// <remarks>
+///     <para>
+///     Implements the standard pattern for changing/changed events.
+///     </para>
+///     <para>
+///         Views that implement <see cref="IOrientation"/> should add a OrientationHelper property. See <see cref="RadioGroup"/> as an example.
+///     </para>
+/// </remarks>
 public class OrientationHelper
 {
     private Orientation _orientation = Orientation.Vertical;
@@ -11,11 +19,8 @@ public class OrientationHelper
     /// <summary>
     ///     Initializes a new instance of the <see cref="OrientationHelper"/> class.
     /// </summary>
-    /// <param name="owner"></param>
-    public OrientationHelper (IOrientation owner)
-    {
-        _owner = owner;
-    }
+    /// <param name="owner">Specifies the object that owns this helper instance.</param>
+    public OrientationHelper (IOrientation owner) { _owner = owner; }
 
     /// <summary>
     ///     Gets or sets the orientation of the View.
@@ -30,19 +35,25 @@ public class OrientationHelper
                 return;
             }
 
-            var args = new CancelEventArgs<Orientation> (in _orientation, ref value);
-            OrientationChanging?.Invoke (_owner, args);
-            if (args.Cancel)
+            // Best practice is to invoke the virtual method first.
+            // This allows derived classes to handle the event and potentially cancel it.
+            if (_owner?.OnOrientationChanging (value, _orientation) ?? false)
             {
                 return;
             }
 
-            if (_owner?.OnOrientationChanging (value, _orientation) ?? false)
+            // If the event is not canceled by the virtual method, raise the event to notify any external subscribers.
+            CancelEventArgs<Orientation> args = new (in _orientation, ref value);
+            OrientationChanging?.Invoke (_owner, args);
+
+            if (args.Cancel)
             {
                 return;
             }
 
+            // If the event is not canceled, update the value.
             Orientation old = _orientation;
+
             if (_orientation != value)
             {
                 _orientation = value;
@@ -53,42 +64,40 @@ public class OrientationHelper
                 }
             }
 
-            args = new CancelEventArgs<Orientation> (in old, ref _orientation);
-            OrientationChanged?.Invoke (_owner, args);
-
+            // Best practice is to invoke the virtual method first.
             _owner?.OnOrientationChanged (old, _orientation);
+
+            // Even though Changed is not cancelable, it is still a good practice to raise the event after.
+            args = new (in old, ref _orientation);
+            OrientationChanged?.Invoke (_owner, args);
         }
     }
 
     /// <summary>
-    /// 
+    ///     Raised when the orientation is changing. This is cancelable.
     /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Views that implement <see cref="IOrientation"/> should raise <see cref="IOrientation.OrientationChanging"/> after the orientation has changed
+    ///         (<code>_orientationHelper.OrientationChanging += (sender, e) => OrientationChanging?.Invoke (this, e);</code>).
+    ///     </para>
+    ///     <para>
+    ///         This event will be raised after the <see cref="IOrientation.OnOrientationChanging"/> method is called (assuming it was not canceled).
+    ///     </para>
+    /// </remarks>
     public event EventHandler<CancelEventArgs<Orientation>> OrientationChanging;
 
     /// <summary>
-    /// 
-    /// </summary>
-    /// <param name="currentOrientation"></param>
-    /// <param name="newOrientation"></param>
-    /// <returns></returns>
-    protected bool OnOrientationChanging (Orientation currentOrientation, Orientation newOrientation)
-    {
-        return _owner?.OnOrientationChanging (currentOrientation, newOrientation) ?? false;
-    }
-
-    /// <summary>
-    /// 
+    ///     Raised when the orientation has changed.
     /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         Views that implement <see cref="IOrientation"/> should raise <see cref="IOrientation.OrientationChanged"/> after the orientation has changed
+    ///         (<code>_orientationHelper.OrientationChanged += (sender, e) => OrientationChanged?.Invoke (this, e);</code>).
+    ///     </para>
+    ///     <para>
+    ///         This event will be raised after the <see cref="IOrientation.OnOrientationChanged"/> method is called.
+    ///     </para>
+    /// </remarks>
     public event EventHandler<CancelEventArgs<Orientation>> OrientationChanged;
-
-    /// <summary>
-    /// 
-    /// </summary>
-    /// <param name="oldOrientation"></param>
-    /// <param name="newOrientation"></param>
-    /// <returns></returns>
-    protected void OnOrientationChanged (Orientation oldOrientation, Orientation newOrientation)
-    {
-        _owner?.OnOrientationChanged (oldOrientation, newOrientation);
-    }
 }

Terminal.Gui/Views/RadioGroup.cs

@@ -44,6 +44,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                         {
                             return false;
                         }
+
                         MoveDownRight ();
 
                         return true;
@@ -58,6 +59,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                         {
                             return false;
                         }
+
                         MoveHome ();
 
                         return true;
@@ -72,6 +74,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                         {
                             return false;
                         }
+
                         MoveEnd ();
 
                         return true;
@@ -93,6 +96,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                     ctx =>
                     {
                         SetFocus ();
+
                         if (ctx.KeyBinding?.Context is { } && (int)ctx.KeyBinding?.Context! < _radioLabels.Count)
                         {
                             SelectedItem = (int)ctx.KeyBinding?.Context!;
@@ -103,13 +107,10 @@ public class RadioGroup : View, IDesignable, IOrientation
                         return true;
                     });
 
-        _orientationHelper = new OrientationHelper (this);
+        _orientationHelper = new (this);
         _orientationHelper.OrientationChanging += (sender, e) => OrientationChanging?.Invoke (this, e);
         _orientationHelper.OrientationChanged += (sender, e) => OrientationChanged?.Invoke (this, e);
 
-        //OrientationChanging += (sender, e) => OnOrientationChanging (e.CurrentValue, e.NewValue);
-        //OrientationChanged += (sender, e) => OnOrientationChanged (e.CurrentValue, e.NewValue);
-
         SetupKeyBindings ();
 
         LayoutStarted += RadioGroup_LayoutStarted;
@@ -331,16 +332,14 @@ public class RadioGroup : View, IDesignable, IOrientation
     }
 
     #region IOrientation
-    /// <inheritdoc />
+
+    /// <inheritdoc/>
     public event EventHandler<CancelEventArgs<Orientation>> OrientationChanging;
 
-    /// <inheritdoc />
-    public bool OnOrientationChanging (Orientation currentOrientation, Orientation newOrientation)
-    {
-        return false;
-    }
+    /// <inheritdoc/>
+    public bool OnOrientationChanging (Orientation currentOrientation, Orientation newOrientation) { return false; }
 
-    /// <inheritdoc />
+    /// <inheritdoc/>
     public event EventHandler<CancelEventArgs<Orientation>> OrientationChanged;
 
     /// <summary>Called when <see cref="Orientation"/> has changed.</summary>
@@ -351,6 +350,7 @@ public class RadioGroup : View, IDesignable, IOrientation
         SetupKeyBindings ();
         SetContentSize ();
     }
+
     #endregion IOrientation
 
     // TODO: This should be cancelable
@@ -363,6 +363,7 @@ public class RadioGroup : View, IDesignable, IOrientation
         {
             return;
         }
+
         _selected = selectedItem;
         SelectedItemChanged?.Invoke (this, new (selectedItem, previousSelectedItem));
     }
@@ -384,6 +385,7 @@ public class RadioGroup : View, IDesignable, IOrientation
                 {
                     x = _horizontal [_cursor].pos;
                 }
+
                 break;
 
             default:
@@ -470,34 +472,11 @@ public class RadioGroup : View, IDesignable, IOrientation
         }
     }
 
-    /// <inheritdoc />
+    /// <inheritdoc/>
     public bool EnableForDesign ()
     {
         RadioLabels = new [] { "Option _1", "Option _2", "Option _3" };
+
         return true;
     }
 }
-
-public class RadioGroupHorizontal : RadioGroup, IOrientation
-{
-    private bool _preventOrientationChange = false;
-    public RadioGroupHorizontal () : base ()
-    {
-        Orientation = Orientation.Horizontal;
-        _preventOrientationChange = true;
-
-        OrientationChanging += RadioGroupHorizontal_OrientationChanging;
-    }
-
-    private void RadioGroupHorizontal_OrientationChanging (object sender, CancelEventArgs<Orientation> e)
-    {
-        //e.Cancel = _preventOrientationChange;
-    }
-
-    /// <inheritdoc />
-    bool IOrientation.OnOrientationChanging (Orientation currrentOrientation, Orientation newOrientation)
-    {
-        return _preventOrientationChange;
-    }
-
-}

UnitTests/View/Orientation/OrientationTests.cs

@@ -25,14 +25,19 @@ public class OrientationTests
 
         public bool CancelOnOrientationChanging { get; set; }
 
+        public bool OnOrientationChangingCalled { get; private set; }
+        public bool OnOrientationChangedCalled { get; private set; }
+
         public bool OnOrientationChanging (Orientation currentOrientation, Orientation newOrientation)
         {
+            OnOrientationChangingCalled = true;
             // Custom logic before orientation changes
             return CancelOnOrientationChanging; // Return true to cancel the change
         }
 
         public void OnOrientationChanged (Orientation oldOrientation, Orientation newOrientation)
         {
+            OnOrientationChangedCalled = true;
             // Custom logic after orientation has changed
         }
     }
@@ -87,4 +92,45 @@ public class OrientationTests
         Assert.False (orientationChanged, "OrientationChanged event was invoked despite cancellation.");
         Assert.Equal (Orientation.Vertical, customView.Orientation); // Assuming Vertical is the default orientation
     }
+
+
+    [Fact]
+    public void OrientationChanging_VirtualMethodCalledBeforeEvent ()
+    {
+        // Arrange
+        var radioGroup = new CustomView ();
+        bool eventCalled = false;
+
+        radioGroup.OrientationChanging += (sender, e) =>
+                                          {
+                                              eventCalled = true;
+                                              Assert.True (radioGroup.OnOrientationChangingCalled, "OnOrientationChanging was not called before the event.");
+                                          };
+
+        // Act
+        radioGroup.Orientation = Orientation.Horizontal;
+
+        // Assert
+        Assert.True (eventCalled, "OrientationChanging event was not called.");
+    }
+
+    [Fact]
+    public void OrientationChanged_VirtualMethodCalledBeforeEvent ()
+    {
+        // Arrange
+        var radioGroup = new CustomView ();
+        bool eventCalled = false;
+
+        radioGroup.OrientationChanged += (sender, e) =>
+                                         {
+                                             eventCalled = true;
+                                             Assert.True (radioGroup.OnOrientationChangedCalled, "OnOrientationChanged was not called before the event.");
+                                         };
+
+        // Act
+        radioGroup.Orientation = Orientation.Horizontal;
+
+        // Assert
+        Assert.True (eventCalled, "OrientationChanged event was not called.");
+    }
 }