diff --git a/Terminal.Gui/View/SuperViewChangedEventArgs.cs b/Terminal.Gui/View/SuperViewChangedEventArgs.cs index 4f25cf47a..33eb87908 100644 --- a/Terminal.Gui/View/SuperViewChangedEventArgs.cs +++ b/Terminal.Gui/View/SuperViewChangedEventArgs.cs @@ -1,28 +1,26 @@ namespace Terminal.Gui; /// -/// Args for events where the of a is changed (e.g. +/// EventArgs for events where the state of the of a is changing (e.g. /// / events). /// public class SuperViewChangedEventArgs : EventArgs { /// Creates a new instance of the class. - /// - /// - public SuperViewChangedEventArgs (View parent, View child) + /// + /// + public SuperViewChangedEventArgs (View superView, View subView) { - Parent = parent; - Child = child; + SuperView = superView; + SubView = subView; } - // TODO: Child is the wrong name. It should be View. - /// The view that is having it's changed - public View Child { get; } + /// The SubView that is either being added or removed from . + public View SubView { get; } - // TODO: Parent is the wrong name. It should be SuperView. /// - /// The parent. For this is the old parent (new parent now being null). For - /// it is the new parent to whom view now belongs. + /// The SuperView that is changing state. For this is the SuperView is being removed from. For + /// it is the SuperView is being added to. /// - public View Parent { get; } + public View SuperView { get; } } diff --git a/Terminal.Gui/View/ViewSubViews.cs b/Terminal.Gui/View/ViewSubViews.cs index 05d332e30..9fccc0635 100644 --- a/Terminal.Gui/View/ViewSubViews.cs +++ b/Terminal.Gui/View/ViewSubViews.cs @@ -182,7 +182,7 @@ public partial class View /// Event where is the subview being added. public virtual void OnAdded (SuperViewChangedEventArgs e) { - View view = e.Child; + View view = e.SubView; view.IsAdded = true; view.OnResizeNeeded (); view.Added?.Invoke (this, e); @@ -192,7 +192,7 @@ public partial class View /// Event args describing the subview being removed. public virtual void OnRemoved (SuperViewChangedEventArgs e) { - View view = e.Child; + View view = e.SubView; view.IsAdded = false; view.Removed?.Invoke (this, e); } diff --git a/Terminal.Gui/Views/Scroll.cs b/Terminal.Gui/Views/Scroll.cs index faf9bed24..e16978834 100644 --- a/Terminal.Gui/Views/Scroll.cs +++ b/Terminal.Gui/Views/Scroll.cs @@ -1,7 +1,7 @@ namespace Terminal.Gui; /// -/// Represents the "inside part" of a scroll bar, minus the arrows. +/// Provides a proportional control for scrolling through content. Used within a . /// public class Scroll : View { @@ -39,7 +39,7 @@ public class Scroll : View private Orientation _orientation; /// - /// Determines the Orientation of the scroll. + /// Gets or sets if the Scroll is oriented vertically or horizontally. /// public Orientation Orientation { @@ -53,7 +53,7 @@ public class Scroll : View private int _position; /// - /// The position, relative to , to set the scrollbar at. + /// Gets or sets the position of the start of the Scroll slider, relative to . /// public int Position { @@ -74,26 +74,39 @@ public class Scroll : View return; } - int oldPos = _position; - _position = value; - OnPositionChanged (oldPos); if (!_wasSliderMouse) { AdjustSlider (); } + + int oldPos = _position; + _position = value; + OnPositionChanged (oldPos); } } - /// This event is raised when the position on the scrollbar has changed. + /// Raised when the has changed. public event EventHandler> PositionChanged; - /// This event is raised when the position on the scrollbar is changing. + /// Raised when the is changing. Set to to prevent the position from being changed. public event EventHandler> PositionChanging; + /// Virtual method called when has changed. Fires . + protected virtual void OnPositionChanged (int oldPos) { PositionChanged?.Invoke (this, new (oldPos, Position)); } + + /// Virtual method called when is changing. Fires , which is cancelable. + protected virtual StateEventArgs OnPositionChanging (int oldPos, int newPos) + { + StateEventArgs args = new (oldPos, newPos); + PositionChanging?.Invoke (this, args); + + return args; + } + private int _size; /// - /// The size of content the scroll represents. + /// Gets or sets the size of the Scroll. This is the total size of the content that can be scrolled through. /// public int Size { @@ -107,35 +120,10 @@ public class Scroll : View } } - /// This event is raised when the size of the scroll has changed. + /// Raised when has changed. public event EventHandler> SizeChanged; - /// - protected override void Dispose (bool disposing) - { - Added -= Scroll_Added; - Initialized -= Scroll_Initialized; - DrawContent -= Scroll_DrawContent; - MouseEvent -= Scroll_MouseEvent; - _slider.DrawContent -= Scroll_DrawContent; - _slider.MouseEvent -= Slider_MouseEvent; - - base.Dispose (disposing); - } - - /// Virtual method to invoke the event handler. - protected virtual void OnPositionChanged (int oldPos) { PositionChanged?.Invoke (this, new (oldPos, Position)); } - - /// Virtual method to invoke the cancelable event handler. - protected virtual StateEventArgs OnPositionChanging (int oldPos, int newPos) - { - StateEventArgs args = new (oldPos, newPos); - PositionChanging?.Invoke (this, args); - - return args; - } - - /// Virtual method to invoke the event handler. + /// Virtual method called when has changed. Fires . protected void OnSizeChanged (int oldSize) { SizeChanged?.Invoke (this, new (oldSize, Size)); } private int GetPositionFromSliderLocation (int location) @@ -145,18 +133,20 @@ public class Scroll : View return 0; } - int barSize = Orientation == Orientation.Vertical ? ContentSize.Height : ContentSize.Width; + int scrollSize = Orientation == Orientation.Vertical ? ContentSize.Height : ContentSize.Width; // Ensure the Position is valid if the slider is at end - if ((Orientation == Orientation.Vertical && location + _slider.Frame.Height == barSize) - || (Orientation == Orientation.Horizontal && location + _slider.Frame.Width == barSize)) + // We use Frame here instead of ContentSize because even if the slider has a margin or border, Frame indicates the actual size + if ((Orientation == Orientation.Vertical && location + _slider.Frame.Height == scrollSize) + || (Orientation == Orientation.Horizontal && location + _slider.Frame.Width == scrollSize)) { - return Size - barSize; + return Size - scrollSize; } - return Math.Min (location * Size / barSize, Size - barSize); + return Math.Min (location * Size / scrollSize, Size - scrollSize); } + // QUESTION: This method is only called from one place. Should it be inlined? Or, should it be made internal and unit tests be provided? private (int Location, int Dimension) GetSliderLocationDimensionFromPosition () { if (ContentSize.Height == 0 || ContentSize.Width == 0) @@ -164,37 +154,38 @@ public class Scroll : View return new (0, 0); } - int barSize = Orientation == Orientation.Vertical ? ContentSize.Height : ContentSize.Width; + int scrollSize = Orientation == Orientation.Vertical ? ContentSize.Height : ContentSize.Width; int location; int dimension; if (Size > 0) { - dimension = Math.Min (Math.Max (barSize * barSize / Size, 1), barSize); + dimension = Math.Min (Math.Max (scrollSize * scrollSize / Size, 1), scrollSize); // Ensure the Position is valid - if (Position > 0 && Position + barSize > Size) + if (Position > 0 && Position + scrollSize > Size) { - Position = Size - barSize; + Position = Size - scrollSize; } - location = Math.Min (Position * barSize / Size, barSize - dimension); + location = Math.Min (Position * scrollSize / Size, scrollSize - dimension); - if (Position == Size - barSize && location + dimension < barSize) + if (Position == Size - scrollSize && location + dimension < scrollSize) { - location = barSize - dimension; + location = scrollSize - dimension; } } else { location = 0; - dimension = barSize; + dimension = scrollSize; } return new (location, dimension); } - private void Parent_LayoutComplete (object sender, LayoutEventArgs e) + // TODO: This is unnecessary. If Scroll.Width/Height is Dim.Auto, the Superview will get resized automatically. + private void SuperView_LayoutComplete (object sender, LayoutEventArgs e) { if (!_wasSliderMouse) { @@ -206,19 +197,23 @@ public class Scroll : View } } - private void Parent_MouseEnter (object sender, MouseEventEventArgs e) { OnMouseEnter (e.MouseEvent); } + private void SuperView_MouseEnter (object sender, MouseEventEventArgs e) { OnMouseEnter (e.MouseEvent); } - private void Parent_MouseLeave (object sender, MouseEventEventArgs e) { OnMouseLeave (e.MouseEvent); } + private void SuperView_MouseLeave (object sender, MouseEventEventArgs e) { OnMouseLeave (e.MouseEvent); } private void Scroll_Added (object sender, SuperViewChangedEventArgs e) { - View parent = e.Parent is Adornment adornment ? adornment.Parent : e.Parent; + View parent = e.SuperView is Adornment adornment ? adornment.Parent : e.SuperView; - parent.LayoutComplete += Parent_LayoutComplete; - parent.MouseEnter += Parent_MouseEnter; - parent.MouseLeave += Parent_MouseLeave; + parent.LayoutComplete += SuperView_LayoutComplete; + + // QUESTION: I really don't like this. It feels like a hack that a subview needs to track its parent's mouse events. + // QUESTION: Can we figure out a way to do this without tracking the parent's mouse events? + parent.MouseEnter += SuperView_MouseEnter; + parent.MouseLeave += SuperView_MouseLeave; } + // TODO: Just override GetNormalColor instead of having this method (make Slider a View sub-class that overrides GetNormalColor) private void Scroll_DrawContent (object sender, DrawEventArgs e) { SetColorSchemeWithSuperview (sender as View); } private void Scroll_Initialized (object sender, EventArgs e) @@ -226,6 +221,9 @@ public class Scroll : View AdjustSlider (); } + // TODO: I think you should create a new `internal` view named "ScrollSlider" with an `Orientation` property. It should inherit from View and override GetNormalColor and the mouse events + // that can be moved within it's Superview, constrained to move only horizontally or vertically depending on Orientation. + // This will really simplify a lot of this. private void Scroll_MouseEvent (object sender, MouseEventEventArgs e) { MouseEvent me = e.MouseEvent; @@ -258,13 +256,13 @@ public class Scroll : View private void Scroll_Removed (object sender, SuperViewChangedEventArgs e) { - if (e.Parent is { }) + if (e.SuperView is { }) { - View parent = e.Parent is Adornment adornment ? adornment.Parent : e.Parent; + View parent = e.SuperView is Adornment adornment ? adornment.Parent : e.SuperView; - parent.LayoutComplete -= Parent_LayoutComplete; - parent.MouseEnter -= Parent_MouseEnter; - parent.MouseLeave -= Parent_MouseLeave; + parent.LayoutComplete -= SuperView_LayoutComplete; + parent.MouseEnter -= SuperView_MouseEnter; + parent.MouseLeave -= SuperView_MouseLeave; } } @@ -290,6 +288,7 @@ public class Scroll : View { TextDirection = Orientation == Orientation.Vertical ? TextDirection.TopBottom_LeftRight : TextDirection.LeftRight_TopBottom; + // QUESTION: Should these Glyphs be configurable via CM? Text = string.Concat ( Enumerable.Repeat ( Glyphs.Stipple.ToString (), @@ -318,10 +317,12 @@ public class Scroll : View Orientation == Orientation.Vertical ? ContentSize.Width : slider.Dimension, Orientation == Orientation.Vertical ? slider.Dimension : ContentSize.Height )); - SetSliderText (); } + // TODO: Move this into "ScrollSlider" and override it there. Scroll can then subscribe to _slider.LayoutComplete and call AdjustSlider. + // QUESTION: I've been meaning to add a "View.FrameChanged" event (fired from LayoutComplete only if Frame has changed). Should we do that as part of this PR? + // QUESTION: Note I *did* add "View.ViewportChanged" in a previous PR. private void Slider_MouseEvent (object sender, MouseEventEventArgs e) { MouseEvent me = e.MouseEvent; @@ -384,4 +385,18 @@ public class Scroll : View e.Handled = true; } + + /// + protected override void Dispose (bool disposing) + { + Added -= Scroll_Added; + Initialized -= Scroll_Initialized; + DrawContent -= Scroll_DrawContent; + MouseEvent -= Scroll_MouseEvent; + _slider.DrawContent -= Scroll_DrawContent; + _slider.MouseEvent -= Slider_MouseEvent; + + base.Dispose (disposing); + } + } diff --git a/UICatalog/KeyBindingsDialog.cs b/UICatalog/KeyBindingsDialog.cs index 128bff3ed..4e2e760dc 100644 --- a/UICatalog/KeyBindingsDialog.cs +++ b/UICatalog/KeyBindingsDialog.cs @@ -208,7 +208,7 @@ internal class KeyBindingsDialog : Dialog // (and always was wrong). Parents don't get to be told when new views are added // to them - view.Added += (s, e) => RecordView (e.Child); + view.Added += (s, e) => RecordView (e.SubView); } } } diff --git a/UnitTests/View/SubviewTests.cs b/UnitTests/View/SubviewTests.cs index 6f9d84f54..8fd38e437 100644 --- a/UnitTests/View/SubviewTests.cs +++ b/UnitTests/View/SubviewTests.cs @@ -19,13 +19,13 @@ public class SubviewTests { Assert.Same (v.SuperView, e.Parent); Assert.Same (t, e.Parent); - Assert.Same (v, e.Child); + Assert.Same (v, e.SubView); }; v.Removed += (s, e) => { Assert.Same (t, e.Parent); - Assert.Same (v, e.Child); + Assert.Same (v, e.SubView); Assert.True (v.SuperView == null); };