diff --git a/Terminal.Gui/Views/ScrollBar/ScrollBar.cs b/Terminal.Gui/Views/ScrollBar/ScrollBar.cs index 29736f134..7a37b8d81 100644 --- a/Terminal.Gui/Views/ScrollBar/ScrollBar.cs +++ b/Terminal.Gui/Views/ScrollBar/ScrollBar.cs @@ -58,8 +58,8 @@ public class ScrollBar : View, IOrientation, IDesignable _orientationHelper = new (this); // Do not use object initializer! _orientationHelper.Orientation = Orientation.Vertical; - _orientationHelper.OrientationChanging += (sender, e) => OrientationChanging?.Invoke (this, e); - _orientationHelper.OrientationChanged += (sender, e) => OrientationChanged?.Invoke (this, e); + //_orientationHelper.OrientationChanging += (sender, e) => OrientationChanging?.Invoke (this, e); + //_orientationHelper.OrientationChanged += (sender, e) => OrientationChanged?.Invoke (this, e); // This sets the width/height etc... OnOrientationChanged (Orientation); @@ -115,9 +115,13 @@ public class ScrollBar : View, IOrientation, IDesignable } } - private int CalculateSliderSize () + /// + /// INTERNAL (for unit tests). Calclates the size of the slider based on the Orientation, ViewportDimension, the actual Viewport, and Size. + /// + /// + internal int CalculateSliderSize () { - if (Size == 0 || ViewportDimension == 0) + if (Size <= 0 || ViewportDimension <= 0) { return 1; } @@ -205,6 +209,8 @@ public class ScrollBar : View, IOrientation, IDesignable _slider.Orientation = newOrientation; PositionSubviews (); + + OrientationChanged?.Invoke (this, new (newOrientation)); } #endregion @@ -371,10 +377,26 @@ public class ScrollBar : View, IOrientation, IDesignable return direction == NavigationDirection.Forward ? (int)Math.Floor (newSliderPosition) : (int)Math.Ceiling (newSliderPosition); } - private int CalculateContentPosition (int sliderPosition) + /// + /// INTERNAL API (for unit tests) - Calculates the content position based on the slider position. + /// + /// + /// Clamps the sliderPosition, ensuring the returned content position is always less than + /// Size - VieportDimension. + /// + /// + /// + internal int CalculateContentPosition (int sliderPosition) { + // Clamp the slider + int clampedSliderPosition = _slider.ClampPosition (sliderPosition); int scrollBarSize = Orientation == Orientation.Vertical ? Viewport.Height : Viewport.Width; - return (int)Math.Round ((double)(sliderPosition) / (scrollBarSize - _slider.Size - _slider.ShrinkBy) * (Size - ViewportDimension)); + double pos = (double)(clampedSliderPosition) / + (scrollBarSize - _slider.Size - _slider.ShrinkBy) * (Size - ViewportDimension); + double rounded = Math.Round (pos); + + // Clamp between 0 and Size - SliderSize + return (int)Math.Clamp (pos, 0, Math.Max (0, Size - _slider.Size)); } diff --git a/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs b/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs index 16aab2ee2..e68fe6ce3 100644 --- a/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs +++ b/Terminal.Gui/Views/ScrollBar/ScrollSlider.cs @@ -39,6 +39,34 @@ public class ScrollSlider : View, IOrientation, IDesignable HighlightStyle = HighlightStyle.Hover; FrameChanged += OnFrameChanged; + + SubviewLayout += (sender, args) => + { + }; + + SubviewsLaidOut += (sender, args) => + { + if (Orientation == Orientation.Vertical) + { + if (SuperView?.Viewport.Height > 0) + { + ViewportDimension = SuperView!.Viewport.Height; + } + } + else + { + if (SuperView?.Viewport.Width > 0) + { + ViewportDimension = SuperView!.Viewport.Width; + } + } + + if (NeedsLayout) + { + Layout (); + } + + }; } #region IOrientation members @@ -74,13 +102,14 @@ public class ScrollSlider : View, IOrientation, IDesignable if (Orientation == Orientation.Vertical) { Width = Dim.Fill (); - Height = 1; + Size = 1; } else { - Width = 1; Height = Dim.Fill (); + Size = 1; } + SetNeedsLayout (); } #endregion @@ -88,8 +117,14 @@ public class ScrollSlider : View, IOrientation, IDesignable /// protected override bool OnClearingViewport () { - FillRect (Viewport, Glyphs.ContinuousMeterSegment); - + if (Orientation == Orientation.Vertical) + { + FillRect (Viewport with { Height = Size }, Glyphs.ContinuousMeterSegment); + } + else + { + FillRect (Viewport with { Width = Size }, Glyphs.ContinuousMeterSegment); + } return true; } @@ -109,12 +144,12 @@ public class ScrollSlider : View, IOrientation, IDesignable } } + private int _size; + /// - /// Gets or sets the size of the ScrollSlider. This is a helper that simply gets or sets the Width or Height depending - /// on the - /// . The size will be constrained such that the ScrollSlider will not go outside the Viewport - /// of - /// the . The size will never be less than 1. + /// Gets or sets the size of the ScrollSlider. This is a helper that gets or sets Width or Height depending + /// on . The size will be clamed between 1 and the dimension of + /// the 's Viewport. /// /// /// @@ -124,40 +159,35 @@ public class ScrollSlider : View, IOrientation, IDesignable /// public int Size { - get - { - if (Orientation == Orientation.Vertical) - { - return Viewport.Height - ShrinkBy / 2; - } - - return Viewport.Width - ShrinkBy / 2; - } + get => _size; set { + if (value == _size) + { + return; + } + + _size = Math.Clamp (value, 1, ViewportDimension); + + if (Orientation == Orientation.Vertical) { - Width = Dim.Fill (); - int viewport = Math.Max (1, SuperView?.Viewport.Height ?? 1); - Height = Math.Clamp (value, 1, viewport); + Height = _size; } else { - Height = Dim.Fill (); - int viewport = Math.Max (1, SuperView?.Viewport.Width ?? 1); - Width = Math.Clamp (value, 1, viewport); + Width = _size; } + SetNeedsLayout (); } } private int? _viewportDimension; /// - /// Gets the size of the viewport into the content being scrolled, bounded by . + /// Gets or sets the size of the viewport into the content being scrolled. If not explicitly set, will be the + /// greater of 1 and the dimension of the . /// - /// - /// This is the SuperView's Viewport demension. - /// public int ViewportDimension { get @@ -167,14 +197,35 @@ public class ScrollSlider : View, IOrientation, IDesignable return _viewportDimension.Value; } - return Orientation == Orientation.Vertical ? SuperView?.Viewport.Height ?? 0 : SuperView?.Viewport.Width ?? 0; + return Math.Max (1, Orientation == Orientation.Vertical ? SuperView?.Viewport.Height ?? 2048 : SuperView?.Viewport.Width ?? 2048); + } + set + { + if (value == _viewportDimension) + { + return; + } + _viewportDimension = int.Max (1, value); + + if (Size > _viewportDimension) + { + Size = _viewportDimension.Value; + } + + if (_position > _viewportDimension - _size) + { + Position = _position; + } + + SetNeedsLayout (); } - set => _viewportDimension = value; } private void OnFrameChanged (object? sender, EventArgs e) { - Position = (Orientation == Orientation.Vertical ? e.CurrentValue.Y : e.CurrentValue.X) - ShrinkBy / 2; + + //ViewportDimension = (Orientation == Orientation.Vertical ? e.CurrentValue.Height : e.CurrentValue.Width); + //Position = (Orientation == Orientation.Vertical ? e.CurrentValue.Y : e.CurrentValue.X) - ShrinkBy / 2; } private int _position; @@ -189,27 +240,23 @@ public class ScrollSlider : View, IOrientation, IDesignable get => _position; set { - if (_position == value) + int clampedPosition = ClampPosition (value); + if (_position == clampedPosition) { return; } - RaisePositionChangeEvents (ClampPosition (value)); - + RaisePositionChangeEvents (clampedPosition); SetNeedsLayout (); } } /// - /// Moves the scroll slider to the specified position. - /// The position will be constrained such that the ScrollSlider will not go outside the Viewport of - /// its . + /// Moves the scroll slider to the specified position. Does not clamp. /// /// - public void MoveToPosition (int position) + internal void MoveToPosition (int position) { - _position = ClampPosition (position); - if (Orientation == Orientation.Vertical) { Y = _position + ShrinkBy / 2; @@ -219,17 +266,20 @@ public class ScrollSlider : View, IOrientation, IDesignable X = _position + ShrinkBy / 2; } - Layout (); + //SetNeedsLayout (); + + // Layout (); } - private int ClampPosition (int newPosittion) + /// + /// INTERNAL API (for unit tests) - Clamps the position such that the right side of the slider + /// never goes past the edge of the Viewport. + /// + /// + /// + internal int ClampPosition (int newPosition) { - if (Orientation == Orientation.Vertical) - { - return Math.Clamp (newPosittion, 0, Math.Max (0, ViewportDimension - Viewport.Height)); - } - - return Math.Clamp (newPosittion, 0, Math.Max (0, ViewportDimension - Viewport.Width)); + return Math.Clamp (newPosition, 0, Math.Max (0, ViewportDimension - Size)); } private void RaisePositionChangeEvents (int newPosition) @@ -248,7 +298,9 @@ public class ScrollSlider : View, IOrientation, IDesignable } int distance = newPosition - _position; - MoveToPosition (newPosition); + _position = ClampPosition (newPosition); + + MoveToPosition (_position); OnPositionChanged (_position); PositionChanged?.Invoke (this, new (in _position)); @@ -387,11 +439,11 @@ public class ScrollSlider : View, IOrientation, IDesignable if (args.CurrentValue == Orientation.Vertical) { Width = Dim.Fill (); - Height = 5; + Size = 5; } else { - Width = 5; + Size = 5; Height = Dim.Fill (); } }; diff --git a/UnitTests/View/Orientation/OrientationHelperTests.cs b/UnitTests/View/Orientation/OrientationHelperTests.cs index cc6a1f240..d4b79f90f 100644 --- a/UnitTests/View/Orientation/OrientationHelperTests.cs +++ b/UnitTests/View/Orientation/OrientationHelperTests.cs @@ -10,18 +10,18 @@ public class OrientationHelperTests // Arrange Mock mockIOrientation = new Mock (); var orientationHelper = new OrientationHelper (mockIOrientation.Object); - var changingEventInvoked = false; - var changedEventInvoked = false; + var changingEventInvoked = 0; + var changedEventInvoked = 0; - orientationHelper.OrientationChanging += (sender, e) => { changingEventInvoked = true; }; - orientationHelper.OrientationChanged += (sender, e) => { changedEventInvoked = true; }; + orientationHelper.OrientationChanging += (sender, e) => { changingEventInvoked++; }; + orientationHelper.OrientationChanged += (sender, e) => { changedEventInvoked++; }; // Act orientationHelper.Orientation = Orientation.Vertical; // Assert - Assert.True (changingEventInvoked, "OrientationChanging event was not invoked."); - Assert.True (changedEventInvoked, "OrientationChanged event was not invoked."); + Assert.Equal (1, changingEventInvoked); + Assert.Equal(1, changedEventInvoked); } [Fact] @@ -29,15 +29,15 @@ public class OrientationHelperTests { // Arrange Mock mockIOrientation = new Mock (); - var onChangingOverrideCalled = false; - var onChangedOverrideCalled = false; + var onChangingOverrideCalled = 0; + var onChangedOverrideCalled = 0; mockIOrientation.Setup (x => x.OnOrientationChanging (It.IsAny (), It.IsAny ())) - .Callback (() => onChangingOverrideCalled = true) + .Callback (() => onChangingOverrideCalled++) .Returns (false); // Ensure it doesn't cancel the change mockIOrientation.Setup (x => x.OnOrientationChanged (It.IsAny ())) - .Callback (() => onChangedOverrideCalled = true); + .Callback (() => onChangedOverrideCalled++); var orientationHelper = new OrientationHelper (mockIOrientation.Object); @@ -45,8 +45,8 @@ public class OrientationHelperTests orientationHelper.Orientation = Orientation.Vertical; // Assert - Assert.True (onChangingOverrideCalled, "OnOrientationChanging override was not called."); - Assert.True (onChangedOverrideCalled, "OnOrientationChanged override was not called."); + Assert.Equal (1, onChangingOverrideCalled); + Assert.Equal (1, onChangedOverrideCalled); } [Fact] @@ -56,18 +56,18 @@ public class OrientationHelperTests Mock mockIOrientation = new Mock (); var orientationHelper = new OrientationHelper (mockIOrientation.Object); orientationHelper.Orientation = Orientation.Horizontal; // Set initial orientation - var changingEventInvoked = false; - var changedEventInvoked = false; + var changingEventInvoked = 0; + var changedEventInvoked = 0; - orientationHelper.OrientationChanging += (sender, e) => { changingEventInvoked = true; }; - orientationHelper.OrientationChanged += (sender, e) => { changedEventInvoked = true; }; + orientationHelper.OrientationChanging += (sender, e) => { changingEventInvoked++; }; + orientationHelper.OrientationChanged += (sender, e) => { changedEventInvoked++; }; // Act orientationHelper.Orientation = Orientation.Horizontal; // Set to the same value // Assert - Assert.False (changingEventInvoked, "OrientationChanging event was invoked."); - Assert.False (changedEventInvoked, "OrientationChanged event was invoked."); + Assert.Equal (0, changingEventInvoked); + Assert.Equal (0, changedEventInvoked); } [Fact] diff --git a/UnitTests/Views/ScrollBarTests.cs b/UnitTests/Views/ScrollBarTests.cs index eadb6780a..c456585d3 100644 --- a/UnitTests/Views/ScrollBarTests.cs +++ b/UnitTests/Views/ScrollBarTests.cs @@ -52,7 +52,9 @@ public class ScrollBarTests Assert.False (scrollBar.CanFocus); Assert.Equal (Orientation.Vertical, scrollBar.Orientation); Assert.Equal (0, scrollBar.Size); + Assert.Equal (0, scrollBar.ViewportDimension); Assert.Equal (0, scrollBar.GetSliderPosition ()); + Assert.Equal (0, scrollBar.ContentPosition); Assert.True (scrollBar.AutoHide); } @@ -95,9 +97,8 @@ public class ScrollBarTests var changingCount = 0; var changedCount = 0; var scrollBar = new ScrollBar { }; - scrollBar.Layout (); - scrollBar.Size = scrollBar.Viewport.Height * 2; - scrollBar.Layout (); + scrollBar.Size = 4; + scrollBar.Frame = new Rectangle (0, 0, 1, 4); // Needs to be at least 4 for slider to move scrollBar.ContentPositionChanging += (s, e) => { @@ -686,7 +687,6 @@ public class ScrollBarTests super.Add (scrollBar); scrollBar.Size = contentSize; - scrollBar.Layout (); scrollBar.ContentPosition = contentPosition; super.BeginInit (); @@ -794,52 +794,130 @@ public class ScrollBarTests } [Theory] - [InlineData (1, 10, 0, 0)] - [InlineData (1, 10, 5, 0)] - [InlineData (1, 10, 10, 1)] - [InlineData (1, 20, 0, 0)] - [InlineData (1, 20, 10, 0)] - [InlineData (1, 20, 20, 1)] - [InlineData (2, 10, 0, 0)] - [InlineData (2, 10, 5, 1)] - [InlineData (2, 10, 10, 2)] - [InlineData (2, 20, 0, 0)] - [InlineData (2, 20, 10, 1)] - [InlineData (2, 20, 20, 2)] - [InlineData (3, 10, 0, 0)] - [InlineData (3, 10, 5, 1)] - [InlineData (3, 10, 10, 3)] - [InlineData (3, 20, 0, 0)] - [InlineData (3, 20, 10, 1)] - [InlineData (3, 20, 20, 3)] - [InlineData (4, 10, 0, 0)] - [InlineData (4, 10, 5, 2)] - [InlineData (4, 10, 10, 4)] - [InlineData (4, 20, 0, 0)] - [InlineData (4, 20, 10, 2)] - [InlineData (4, 20, 20, 4)] - [InlineData (5, 10, 0, 0)] - [InlineData (5, 10, 5, 2)] - [InlineData (5, 10, 10, 5)] - [InlineData (5, 20, 0, 0)] - [InlineData (5, 20, 10, 2)] - [InlineData (5, 20, 20, 5)] - public void ScrollBar_CombinatorialTests (int width, int size, int contentPosition, int expectedSliderPosition) + [InlineData (-1, 10, 1)] + [InlineData (0, 10, 1)] + [InlineData (10, 15, 5)] + [InlineData (10, 5, 10)] + [InlineData (10, 3, 10)] + [InlineData (10, 2, 10)] + [InlineData (10, 1, 10)] + [InlineData (10, 0, 1)] + [InlineData (10, 10, 8)] + [InlineData (10, 20, 4)] + [InlineData (10, 100, 1)] + [InlineData (15, 10, 15)] + [InlineData (15, 0, 1)] + [InlineData (15, 1, 15)] + [InlineData (15, 2, 15)] + [InlineData (15, 3, 15)] + [InlineData (15, 5, 15)] + [InlineData (15, 14, 13)] + [InlineData (15, 15, 13)] + [InlineData (15, 16, 12)] + [InlineData (20, 10, 20)] + [InlineData (100, 10, 100)] + public void CalculateSliderSize_Width_Matches_ViewportDimension (int viewportDimension, int size, int expectedSliderSize) { // Arrange var scrollBar = new ScrollBar { - Width = width, + ViewportDimension = viewportDimension, Size = size, - ContentPosition = contentPosition + Orientation = Orientation.Horizontal // Assuming horizontal for simplicity }; + scrollBar.Width = viewportDimension; // Changing orientation changes Width + scrollBar.BeginInit (); + scrollBar.EndInit (); scrollBar.Layout (); // Act - var sliderPosition = scrollBar.GetSliderPosition (); + var sliderSize = scrollBar.CalculateSliderSize (); + // Assert - Assert.Equal (expectedSliderPosition, sliderPosition); + Assert.Equal (expectedSliderSize, sliderSize); } + // 012345678901 + // ◄█░░░░░░░░░► + [Theory] + // ◄█► + [InlineData (3, 3, -1, 0)] + [InlineData (3, 3, 0, 0)] + [InlineData (3, 3, 1, 0)] + [InlineData (3, 3, 2, 0)] + + // ◄██► + [InlineData (4, 2, 1, 0)] + [InlineData (4, 2, 2, 0)] + + // 0123 + // --- + // ◄█░► + [InlineData (4, 3, 0, 0)] + // ◄░█► + [InlineData (4, 3, 1, 1)] + // ◄░█► + [InlineData (4, 3, 2, 1)] + + + // 01234 + // ---- + // ◄█░► + [InlineData (4, 4, 0, 0)] + // ◄░█► + [InlineData (4, 4, 1, 1)] + // ◄░█► + [InlineData (4, 4, 2, 1)] + + // 012345 + // ◄███► + // ----- + [InlineData (5, 5, 3, 0)] + [InlineData (5, 5, 4, 0)] + + // 0123456 + // ◄██░► + // ------ + [InlineData (5, 6, 0, 0)] + [InlineData (5, 6, 1, 1)] + [InlineData (5, 6, 2, 1)] + + // 01234567890 + // ◄█░░░► + // ---------- + [InlineData (5, 10, -1, 0)] + [InlineData (5, 10, 0, 0)] + + // 01234567890 + // ◄░█░░► + // --^------- + [InlineData (5, 10, 1, 2)] + [InlineData (5, 10, 2, 3)] + [InlineData (5, 10, 3, 3)] + [InlineData (5, 10, 4, 3)] + [InlineData (5, 10, 5, 3)] + [InlineData (5, 10, 6, 3)] + [InlineData (5, 10, 7, 3)] + [InlineData (5, 10, 8, 3)] + [InlineData (5, 10, 9, 3)] + [InlineData (5, 10, 10, 3)] + public void CalculateContentPosition_ComprehensiveTests (int viewportDimension, int size, int sliderPosition, int expectedContentPosition) + { + // Arrange + var scrollBar = new ScrollBar + { + ViewportDimension = viewportDimension, + Size = size, + Orientation = Orientation.Horizontal // Assuming horizontal for simplicity + }; + scrollBar.Width = viewportDimension; // Changing orientation changes Width + scrollBar.Layout (); + + // Act + var contentPosition = scrollBar.CalculateContentPosition (sliderPosition); + + // Assert + Assert.Equal (expectedContentPosition, contentPosition); + } } diff --git a/UnitTests/Views/ScrollSliderTests.cs b/UnitTests/Views/ScrollSliderTests.cs index c344d6ce0..691f87554 100644 --- a/UnitTests/Views/ScrollSliderTests.cs +++ b/UnitTests/Views/ScrollSliderTests.cs @@ -87,6 +87,90 @@ public class ScrollSliderTests (ITestOutputHelper output) Assert.True (scrollSlider.Size <= 5); } + [Theory] + [CombinatorialData] + public void Size_Clamps_To_ViewportDimensions ([CombinatorialRange (10, 10, 1)] int dimension, [CombinatorialRange (-1, 12, 1)] int sliderSize, Orientation orientation) + { + + var scrollSlider = new ScrollSlider + { + Orientation = orientation, + ViewportDimension = dimension, + Size = sliderSize, + }; + scrollSlider.Layout (); + + Assert.True (scrollSlider.Size > 0); + + Assert.True (scrollSlider.Size <= dimension); + } + + [Theory] + [CombinatorialData] + public void ViewportDimensions_Clamps_0_To_Dimension ([CombinatorialRange (0, 10, 1)] int dimension, Orientation orientation) + { + var scrollSlider = new ScrollSlider + { + Orientation = orientation, + ViewportDimension = dimension, + }; + + Assert.InRange (scrollSlider.ViewportDimension, 1, 10); + + View super = new () + { + Id = "super", + Height = dimension, + Width = dimension, + }; + + scrollSlider = new ScrollSlider + { + Orientation = orientation, + }; + super.Add (scrollSlider); + super.Layout (); + + Assert.InRange (scrollSlider.ViewportDimension, 1, 10); + + scrollSlider.ViewportDimension = dimension; + + Assert.InRange (scrollSlider.ViewportDimension, 1, 10); + } + + [Theory] + [CombinatorialData] + public void ClampPosition_Clamps_To_Viewport_Minus_Size ([CombinatorialRange (10, 10, 1)] int dimension, [CombinatorialRange (1, 5, 1)] int sliderSize, [CombinatorialRange (-2, 6, 2)] int sliderPosition, Orientation orientation) + { + var scrollSlider = new ScrollSlider + { + Orientation = orientation, + ViewportDimension = dimension, + Size = sliderSize, + }; + + int clampedPosition = scrollSlider.ClampPosition (sliderPosition); + + Assert.InRange (clampedPosition, 0, dimension - sliderSize); + + View super = new () + { + Id = "super", + Height = dimension, + Width = dimension, + }; + scrollSlider = new ScrollSlider + { + Orientation = orientation, + Size = sliderSize, + }; + super.Add (scrollSlider); + super.Layout (); + + clampedPosition = scrollSlider.ClampPosition (sliderPosition); + Assert.InRange (clampedPosition, 0, dimension - sliderSize); + } + [Theory] [CombinatorialData] public void Position_Clamps_To_SuperView_Viewport ([CombinatorialRange (0, 5, 1)] int sliderSize, [CombinatorialRange (-2, 6, 2)] int sliderPosition, Orientation orientation) @@ -431,15 +515,12 @@ public class ScrollSliderTests (ITestOutputHelper output) var scrollSlider = new ScrollSlider { Orientation = orientation, + Size = sliderSize, + Position = position, }; + Assert.Equal (sliderSize, scrollSlider.Size); super.Add (scrollSlider); - - scrollSlider.Size = sliderSize; - scrollSlider.Layout (); - scrollSlider.Position = position; - - super.BeginInit (); - super.EndInit (); + super.Layout (); super.Draw ();