From 9813b614a0cbb114cb5c5258a4c7a0fa844eca21 Mon Sep 17 00:00:00 2001 From: BDisp Date: Fri, 16 Sep 2022 14:54:26 +0100 Subject: [PATCH 1/2] Fixes #1984. Added ClearOnVisibleFalse to flag if the view must be cleared or not. --- Terminal.Gui/Core/View.cs | 11 +++++-- UnitTests/ViewTests.cs | 60 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 2 deletions(-) diff --git a/Terminal.Gui/Core/View.cs b/Terminal.Gui/Core/View.cs index 9e082ee17b..dd7a52f4d7 100644 --- a/Terminal.Gui/Core/View.cs +++ b/Terminal.Gui/Core/View.cs @@ -1820,7 +1820,7 @@ public void ClearKeybinding (Key key) /// public void ClearKeybinding (params Command [] command) { - foreach (var kvp in KeyBindings.Where (kvp => kvp.Value.SequenceEqual (command)).ToArray()) { + foreach (var kvp in KeyBindings.Where (kvp => kvp.Value.SequenceEqual (command)).ToArray ()) { KeyBindings.Remove (kvp.Key); } } @@ -2556,6 +2556,11 @@ public override bool Enabled { } } + /// + /// Gets or sets whether a view is cleared if the property is . + /// + public bool ClearOnVisibleFalse { get; set; } = true; + /// > public override bool Visible { get => base.Visible; @@ -2566,7 +2571,9 @@ public override bool Visible { if (HasFocus) { SetHasFocus (false, this); } - Clear (); + if (ClearOnVisibleFalse) { + Clear (); + } } OnVisibleChanged (); SetNeedsDisplay (); diff --git a/UnitTests/ViewTests.cs b/UnitTests/ViewTests.cs index e0cfecce45..ef601b46b8 100644 --- a/UnitTests/ViewTests.cs +++ b/UnitTests/ViewTests.cs @@ -3830,6 +3830,66 @@ public void Visible_Clear_The_View_Output () │ │ │ │ └────────────────────────────┘ +", output); + } + + [Fact, AutoInitShutdown] + public void ClearOnVisibleFalse_Gets_Sets () + { + var text = "This is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test"; + var label = new Label (text); + Application.Top.Add (label); + + var sbv = new ScrollBarView (label, true, false) { + Size = 100, + ClearOnVisibleFalse = false + }; + Application.Begin (Application.Top); + + Assert.True (sbv.Visible); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes▲ +This is a tes┬ +This is a tes┴ +This is a tes░ +This is a tes░ +This is a tes▼ +", output); + + sbv.Visible = false; + Assert.False (sbv.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a test +This is a test +This is a test +This is a test +This is a test +This is a test +", output); + + sbv.Visible = true; + Assert.True (sbv.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes▲ +This is a tes┬ +This is a tes┴ +This is a tes░ +This is a tes░ +This is a tes▼ +", output); + + sbv.ClearOnVisibleFalse = true; + sbv.Visible = false; + Assert.False (sbv.Visible); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes +This is a tes +This is a tes +This is a tes +This is a tes +This is a tes ", output); } } From dfcccff80ef05a13f289a67e0c385206b387a174 Mon Sep 17 00:00:00 2001 From: BDisp Date: Fri, 16 Sep 2022 15:40:14 +0100 Subject: [PATCH 2/2] Fixes #2022. ScrollBarView doesn't show the down arrow on vertical if there isn't horizontal. --- Terminal.Gui/Views/ScrollBarView.cs | 58 +++++-- UnitTests/ScrollBarViewTests.cs | 225 +++++++++++++++++++++++++--- 2 files changed, 245 insertions(+), 38 deletions(-) diff --git a/Terminal.Gui/Views/ScrollBarView.cs b/Terminal.Gui/Views/ScrollBarView.cs index 25152cae05..f584a2ec7d 100644 --- a/Terminal.Gui/Views/ScrollBarView.cs +++ b/Terminal.Gui/Views/ScrollBarView.cs @@ -106,7 +106,7 @@ public ScrollBarView (View host, bool isVertical, bool showBothScrollIndicator = OtherScrollBarView.X = OtherScrollBarView.IsVertical ? Pos.Right (host) - 1 : Pos.Left (host); OtherScrollBarView.Y = OtherScrollBarView.IsVertical ? Pos.Top (host) : Pos.Bottom (host) - 1; OtherScrollBarView.Host.SuperView.Add (OtherScrollBarView); - OtherScrollBarView.showScrollIndicator = true; + OtherScrollBarView.ShowScrollIndicator = true; } ShowScrollIndicator = true; contentBottomRightCorner = new View (" ") { Visible = host.Visible }; @@ -116,6 +116,7 @@ public ScrollBarView (View host, bool isVertical, bool showBothScrollIndicator = contentBottomRightCorner.Width = 1; contentBottomRightCorner.Height = 1; contentBottomRightCorner.MouseClick += ContentBottomRightCorner_MouseClick; + ClearOnVisibleFalse = false; } private void Host_VisibleChanged () @@ -188,11 +189,9 @@ public bool IsVertical { public int Size { get => size; set { - if (hosted || (otherScrollBarView != null && otherScrollBarView.hosted)) { - size = value + 1; - } else { - size = value; - } + size = value; + SetRelativeLayout (Bounds); + ShowHideScrollBars (false); SetNeedsDisplay (); } } @@ -220,9 +219,6 @@ public int Position { position = Math.Max (position + max, 0); } var s = GetBarsize (vertical); - if (position + s == size && (hosted || (otherScrollBarView != null && otherScrollBarView.hosted))) { - position++; - } OnChangedPosition (); SetNeedsDisplay (); } @@ -327,11 +323,13 @@ public virtual void Refresh () ShowHideScrollBars (); } - void ShowHideScrollBars () + void ShowHideScrollBars (bool redraw = true) { if (!hosted || (hosted && !autoHideScrollBars)) { if (contentBottomRightCorner != null && contentBottomRightCorner.Visible) { contentBottomRightCorner.Visible = false; + } else if (otherScrollBarView != null && otherScrollBarView.contentBottomRightCorner != null && otherScrollBarView.contentBottomRightCorner.Visible) { + otherScrollBarView.contentBottomRightCorner.Visible = false; } return; } @@ -350,24 +348,34 @@ void ShowHideScrollBars () if (showBothScrollIndicator) { if (contentBottomRightCorner != null) { contentBottomRightCorner.Visible = true; + } else if (otherScrollBarView != null && otherScrollBarView.contentBottomRightCorner != null) { + otherScrollBarView.contentBottomRightCorner.Visible = true; } } else if (!showScrollIndicator) { if (contentBottomRightCorner != null) { contentBottomRightCorner.Visible = false; + } else if (otherScrollBarView != null && otherScrollBarView.contentBottomRightCorner != null) { + otherScrollBarView.contentBottomRightCorner.Visible = false; } if (Application.mouseGrabView != null && Application.mouseGrabView == this) { Application.UngrabMouse (); } - } else { + } else if (contentBottomRightCorner != null) { contentBottomRightCorner.Visible = false; + } else if (otherScrollBarView != null && otherScrollBarView.contentBottomRightCorner != null) { + otherScrollBarView.contentBottomRightCorner.Visible = false; } if (Host?.Visible == true && showScrollIndicator && !Visible) { Visible = true; } - if (Host?.Visible == true && otherScrollBarView != null && otherScrollBarView.showScrollIndicator - && !otherScrollBarView.Visible) { + if (Host?.Visible == true && otherScrollBarView?.showScrollIndicator == true && !otherScrollBarView.Visible) { otherScrollBarView.Visible = true; } + + if (!redraw) { + return; + } + if (showScrollIndicator) { Redraw (Bounds); } @@ -384,13 +392,22 @@ bool CheckBothScrollBars (ScrollBarView scrollBarView, bool pending = false) if (scrollBarView.showScrollIndicator) { scrollBarView.ShowScrollIndicator = false; } + if (scrollBarView.Visible) { + scrollBarView.Visible = false; + } } else if (barsize > 0 && barsize == scrollBarView.size && scrollBarView.OtherScrollBarView != null && pending) { if (scrollBarView.showScrollIndicator) { scrollBarView.ShowScrollIndicator = false; } + if (scrollBarView.Visible) { + scrollBarView.Visible = false; + } if (scrollBarView.OtherScrollBarView != null && scrollBarView.showBothScrollIndicator) { scrollBarView.OtherScrollBarView.ShowScrollIndicator = false; } + if (scrollBarView.OtherScrollBarView.Visible) { + scrollBarView.OtherScrollBarView.Visible = false; + } } else if (barsize > 0 && barsize == size && scrollBarView.OtherScrollBarView != null && !pending) { pending = true; } else { @@ -398,10 +415,16 @@ bool CheckBothScrollBars (ScrollBarView scrollBarView, bool pending = false) if (!scrollBarView.showBothScrollIndicator) { scrollBarView.OtherScrollBarView.ShowScrollIndicator = true; } + if (!scrollBarView.OtherScrollBarView.Visible) { + scrollBarView.OtherScrollBarView.Visible = true; + } } if (!scrollBarView.showScrollIndicator) { scrollBarView.ShowScrollIndicator = true; } + if (!scrollBarView.Visible) { + scrollBarView.Visible = true; + } } return pending; @@ -418,7 +441,7 @@ void SetWidthHeight () } else if (showScrollIndicator) { Width = vertical ? 1 : Dim.Width (Host) - 0; Height = vertical ? Dim.Height (Host) - 0 : 1; - } else if (otherScrollBarView != null && otherScrollBarView.showScrollIndicator) { + } else if (otherScrollBarView?.showScrollIndicator == true) { otherScrollBarView.Width = otherScrollBarView.vertical ? 1 : Dim.Width (Host) - 0; otherScrollBarView.Height = otherScrollBarView.vertical ? Dim.Height (Host) - 0 : 1; } @@ -432,7 +455,10 @@ void SetWidthHeight () /// public override void Redraw (Rect region) { - if (ColorScheme == null || Size == 0) { + if (ColorScheme == null || ((!showScrollIndicator || Size == 0) && AutoHideScrollBars && Visible)) { + if ((!showScrollIndicator || Size == 0) && AutoHideScrollBars && Visible) { + ShowHideScrollBars (false); + } return; } @@ -578,6 +604,8 @@ public override void Redraw (Rect region) if (contentBottomRightCorner != null && hosted && showBothScrollIndicator) { contentBottomRightCorner.Redraw (contentBottomRightCorner.Bounds); + } else if (otherScrollBarView != null && otherScrollBarView.contentBottomRightCorner != null && otherScrollBarView.hosted && otherScrollBarView.showBothScrollIndicator) { + otherScrollBarView.contentBottomRightCorner.Redraw (otherScrollBarView.contentBottomRightCorner.Bounds); } } diff --git a/UnitTests/ScrollBarViewTests.cs b/UnitTests/ScrollBarViewTests.cs index f4571602c1..12215d1540 100644 --- a/UnitTests/ScrollBarViewTests.cs +++ b/UnitTests/ScrollBarViewTests.cs @@ -192,9 +192,9 @@ public void Hosting_A_View_To_A_ScrollBarView () _hostView.Redraw (_hostView.Bounds); Assert.Equal (_scrollBar.Position, _hostView.Top); - Assert.Equal (_scrollBar.Size, _hostView.Lines + 1); + Assert.Equal (_scrollBar.Size, _hostView.Lines); Assert.Equal (_scrollBar.OtherScrollBarView.Position, _hostView.Left); - Assert.Equal (_scrollBar.OtherScrollBarView.Size, _hostView.Cols + 1); + Assert.Equal (_scrollBar.OtherScrollBarView.Size, _hostView.Cols); } [Fact] @@ -310,8 +310,8 @@ public void KeepContentAlwaysInViewport_True () Assert.Equal (25, _hostView.Bounds.Height); Assert.Equal (79, _scrollBar.OtherScrollBarView.Bounds.Width); Assert.Equal (24, _scrollBar.Bounds.Height); - Assert.Equal (31, _scrollBar.Size); - Assert.Equal (101, _scrollBar.OtherScrollBarView.Size); + Assert.Equal (30, _scrollBar.Size); + Assert.Equal (100, _scrollBar.OtherScrollBarView.Size); Assert.True (_scrollBar.ShowScrollIndicator); Assert.True (_scrollBar.OtherScrollBarView.ShowScrollIndicator); Assert.True (_scrollBar.Visible); @@ -320,8 +320,8 @@ public void KeepContentAlwaysInViewport_True () _scrollBar.Position = 50; Assert.Equal (_scrollBar.Position, _scrollBar.Size - _scrollBar.Bounds.Height); Assert.Equal (_scrollBar.Position, _hostView.Top); - Assert.Equal (7, _scrollBar.Position); - Assert.Equal (7, _hostView.Top); + Assert.Equal (6, _scrollBar.Position); + Assert.Equal (6, _hostView.Top); Assert.True (_scrollBar.ShowScrollIndicator); Assert.True (_scrollBar.OtherScrollBarView.ShowScrollIndicator); Assert.True (_scrollBar.Visible); @@ -330,8 +330,8 @@ public void KeepContentAlwaysInViewport_True () _scrollBar.OtherScrollBarView.Position = 150; Assert.Equal (_scrollBar.OtherScrollBarView.Position, _scrollBar.OtherScrollBarView.Size - _scrollBar.OtherScrollBarView.Bounds.Width); Assert.Equal (_scrollBar.OtherScrollBarView.Position, _hostView.Left); - Assert.Equal (22, _scrollBar.OtherScrollBarView.Position); - Assert.Equal (22, _hostView.Left); + Assert.Equal (21, _scrollBar.OtherScrollBarView.Position); + Assert.Equal (21, _hostView.Left); Assert.True (_scrollBar.ShowScrollIndicator); Assert.True (_scrollBar.OtherScrollBarView.ShowScrollIndicator); Assert.True (_scrollBar.Visible); @@ -350,14 +350,14 @@ public void KeepContentAlwaysInViewport_False () _scrollBar.Position = 50; Assert.Equal (_scrollBar.Position, _scrollBar.Size - 1); Assert.Equal (_scrollBar.Position, _hostView.Top); - Assert.Equal (30, _scrollBar.Position); - Assert.Equal (30, _hostView.Top); + Assert.Equal (29, _scrollBar.Position); + Assert.Equal (29, _hostView.Top); _scrollBar.OtherScrollBarView.Position = 150; Assert.Equal (_scrollBar.OtherScrollBarView.Position, _scrollBar.OtherScrollBarView.Size - 1); Assert.Equal (_scrollBar.OtherScrollBarView.Position, _hostView.Left); - Assert.Equal (100, _scrollBar.OtherScrollBarView.Position); - Assert.Equal (100, _hostView.Left); + Assert.Equal (99, _scrollBar.OtherScrollBarView.Position); + Assert.Equal (99, _hostView.Left); } [Fact] @@ -497,7 +497,7 @@ public void Constructor_ShowBothScrollIndicator_False_And_IsVertical_True_Refres }; listView.DrawContent += (e) => { - newScrollBarView.Size = listView.Source.Count - 1; + newScrollBarView.Size = listView.Source.Count; Assert.Equal (newScrollBarView.Size, listView.Source.Count); newScrollBarView.Position = listView.TopItem; Assert.Equal (newScrollBarView.Position, listView.TopItem); @@ -572,7 +572,7 @@ public void Constructor_ShowBothScrollIndicator_False_And_IsVertical_False_Refre }; listView.DrawContent += (e) => { - newScrollBarView.Size = listView.Maxlength - 1; + newScrollBarView.Size = listView.Maxlength; Assert.Equal (newScrollBarView.Size, listView.Maxlength); newScrollBarView.Position = listView.LeftItem; Assert.Equal (newScrollBarView.Position, listView.LeftItem); @@ -618,9 +618,9 @@ public void Internal_Tests () Assert.Equal (0, max); Assert.False (sbv.OtherScrollBarView.CanScroll (10, out max, sbv.OtherScrollBarView.IsVertical)); Assert.Equal (0, max); - // They are visible but are not drawn. - Assert.True (sbv.Visible); - Assert.True (sbv.OtherScrollBarView.Visible); + // They aren't visible so they aren't drawn. + Assert.False (sbv.Visible); + Assert.False (sbv.OtherScrollBarView.Visible); top.LayoutSubviews (); // Now the host bounds is not empty. Assert.True (sbv.CanScroll (10, out max, sbv.IsVertical)); @@ -628,17 +628,19 @@ public void Internal_Tests () Assert.True (sbv.OtherScrollBarView.CanScroll (10, out max, sbv.OtherScrollBarView.IsVertical)); Assert.Equal (10, max); Assert.True (sbv.CanScroll (50, out max, sbv.IsVertical)); - Assert.Equal (17, max); // 17+23=40 + Assert.Equal (40, sbv.Size); + Assert.Equal (15, max); // 15+25=40 Assert.True (sbv.OtherScrollBarView.CanScroll (150, out max, sbv.OtherScrollBarView.IsVertical)); - Assert.Equal (22, max); // 22+78=100 - Assert.True (sbv.Visible); - Assert.True (sbv.OtherScrollBarView.Visible); + Assert.Equal (100, sbv.OtherScrollBarView.Size); + Assert.Equal (20, max); // 20+80=100 + Assert.False (sbv.Visible); + Assert.False (sbv.OtherScrollBarView.Visible); sbv.KeepContentAlwaysInViewport = false; sbv.OtherScrollBarView.KeepContentAlwaysInViewport = false; Assert.True (sbv.CanScroll (50, out max, sbv.IsVertical)); - Assert.Equal (40, max); + Assert.Equal (39, max); Assert.True (sbv.OtherScrollBarView.CanScroll (150, out max, sbv.OtherScrollBarView.IsVertical)); - Assert.Equal (100, max); + Assert.Equal (99, max); Assert.True (sbv.Visible); Assert.True (sbv.OtherScrollBarView.Visible); } @@ -801,5 +803,182 @@ public void Hosting_ShowBothScrollIndicator_Invisible () pos = GraphViewTests.AssertDriverContentsWithFrameAre (expected, output); Assert.Equal (new Rect (0, 0, 10, 10), pos); } + + + [Fact, AutoInitShutdown] + public void ContentBottomRightCorner_Not_Redraw_If_Both_Size_Equal_To_Zero () + { + var text = "This is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test"; + var label = new Label (text); + Application.Top.Add (label); + + var sbv = new ScrollBarView (label, true, true) { + Size = 100, + }; + sbv.OtherScrollBarView.Size = 100; + Application.Begin (Application.Top); + + Assert.Equal (100, sbv.Size); + Assert.Equal (100, sbv.OtherScrollBarView.Size); + Assert.True (sbv.ShowScrollIndicator); + Assert.True (sbv.OtherScrollBarView.ShowScrollIndicator); + Assert.True (sbv.Visible); + Assert.True (sbv.OtherScrollBarView.Visible); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes▲ +This is a tes┬ +This is a tes┴ +This is a tes░ +This is a tes▼ +◄├─┤░░░░░░░░► +", output); + + sbv.Size = 0; + sbv.OtherScrollBarView.Size = 0; + Assert.Equal (0, sbv.Size); + Assert.Equal (0, sbv.OtherScrollBarView.Size); + Assert.False (sbv.ShowScrollIndicator); + Assert.False (sbv.OtherScrollBarView.ShowScrollIndicator); + Assert.False (sbv.Visible); + Assert.False (sbv.OtherScrollBarView.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a test +This is a test +This is a test +This is a test +This is a test +This is a test +", output); + + sbv.Size = 50; + sbv.OtherScrollBarView.Size = 50; + Assert.Equal (50, sbv.Size); + Assert.Equal (50, sbv.OtherScrollBarView.Size); + Assert.True (sbv.ShowScrollIndicator); + Assert.True (sbv.OtherScrollBarView.ShowScrollIndicator); + Assert.True (sbv.Visible); + Assert.True (sbv.OtherScrollBarView.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes▲ +This is a tes┬ +This is a tes┴ +This is a tes░ +This is a tes▼ +◄├──┤░░░░░░░► +", output); + + } + + [Fact, AutoInitShutdown] + public void ContentBottomRightCorner_Not_Redraw_If_One_Size_Equal_To_Zero () + { + var text = "This is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test"; + var label = new Label (text); + Application.Top.Add (label); + + var sbv = new ScrollBarView (label, true, false) { + Size = 100, + }; + Application.Begin (Application.Top); + + Assert.Equal (100, sbv.Size); + Assert.Null (sbv.OtherScrollBarView); + Assert.True (sbv.ShowScrollIndicator); + Assert.True (sbv.Visible); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a tes▲ +This is a tes┬ +This is a tes┴ +This is a tes░ +This is a tes░ +This is a tes▼ +", output); + + sbv.Size = 0; + Assert.Equal (0, sbv.Size); + Assert.False (sbv.ShowScrollIndicator); + Assert.False (sbv.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a test +This is a test +This is a test +This is a test +This is a test +This is a test +", output); + } + + [Fact, AutoInitShutdown] + public void ShowScrollIndicator_False_Must_Also_Set_Visible_To_False_To_Not_Respond_To_Events () + { + var clicked = false; + var text = "This is a test\nThis is a test\nThis is a test\nThis is a test\nThis is a test"; + var label = new Label (text) { Width = 14, Height = 5 }; + var btn = new Button (14, 0, "Click Me!"); + btn.Clicked += () => clicked = true; + Application.Top.Add (label, btn); + + var sbv = new ScrollBarView (label, true, false) { + Size = 5, + }; + Application.Begin (Application.Top); + + Assert.Equal (5, sbv.Size); + Assert.Null (sbv.OtherScrollBarView); + Assert.False (sbv.ShowScrollIndicator); + Assert.False (sbv.Visible); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a test[ Click Me! ] +This is a test +This is a test +This is a test +This is a test +", output); + + ReflectionTools.InvokePrivate ( + typeof (Application), + "ProcessMouseEvent", + new MouseEvent () { + X = 15, + Y = 0, + Flags = MouseFlags.Button1Clicked + }); + + Assert.Null (Application.mouseGrabView); + Assert.True (clicked); + + clicked = false; + + sbv.Visible = true; + Assert.Equal (5, sbv.Size); + Assert.False (sbv.ShowScrollIndicator); + Assert.True (sbv.Visible); + Application.Top.Redraw (Application.Top.Bounds); + GraphViewTests.AssertDriverContentsWithFrameAre (@" +This is a test[ Click Me! ] +This is a test +This is a test +This is a test +This is a test +", output); + + ReflectionTools.InvokePrivate ( + typeof (Application), + "ProcessMouseEvent", + new MouseEvent () { + X = 15, + Y = 0, + Flags = MouseFlags.Button1Clicked + }); + + Assert.Null (Application.mouseGrabView); + Assert.True (clicked); + Assert.Equal (5, sbv.Size); + Assert.False (sbv.ShowScrollIndicator); + Assert.False (sbv.Visible); + } } }