From d81bf050eed02bed5f7f9b02ac002f5627c836e7 Mon Sep 17 00:00:00 2001 From: BDisp Date: Mon, 27 Mar 2023 12:54:51 +0100 Subject: [PATCH] Fixes #2451. ListView SelectedItem should be -1 by default. --- Terminal.Gui/Views/ComboBox.cs | 12 +++--- Terminal.Gui/Views/ListView.cs | 26 ++++++------- UnitTests/Text/CollectionNavigatorTests.cs | 9 +++++ UnitTests/Views/ComboBoxTests.cs | 31 +++++++-------- UnitTests/Views/ListViewTests.cs | 44 +++++++++++++++------- 5 files changed, 74 insertions(+), 48 deletions(-) diff --git a/Terminal.Gui/Views/ComboBox.cs b/Terminal.Gui/Views/ComboBox.cs index 4bae179bf..05cd0ac7a 100644 --- a/Terminal.Gui/Views/ComboBox.cs +++ b/Terminal.Gui/Views/ComboBox.cs @@ -188,7 +188,7 @@ namespace Terminal.Gui { if (SuperView != null && SuperView.Subviews.Contains (this)) { SelectedItem = -1; search.Text = ""; - Search_Changed (this, new TextChangedEventArgs("")); + Search_Changed (this, new TextChangedEventArgs ("")); SetNeedsDisplay (); } } @@ -316,7 +316,7 @@ namespace Terminal.Gui { } }; - Added += (s, e) => { + Added += (s, e) => { // Determine if this view is hosted inside a dialog and is the only control for (View view = this.SuperView; view != null; view = view.SuperView) { @@ -328,7 +328,7 @@ namespace Terminal.Gui { SetNeedsLayout (); SetNeedsDisplay (); - Search_Changed (this, new TextChangedEventArgs( Text)); + Search_Changed (this, new TextChangedEventArgs (Text)); }; // Things this view knows how to do @@ -745,15 +745,17 @@ namespace Terminal.Gui { if (listview.Source.Count == 0 || (searchset?.Count ?? 0) == 0) { text = ""; HideList (); + isShow = false; return; } - SetValue (searchset [listview.SelectedItem]); + SetValue (listview.SelectedItem > -1 ? searchset [listview.SelectedItem] : text); search.CursorPosition = search.Text.ConsoleWidth; - Search_Changed (this, new TextChangedEventArgs(search.Text)); + Search_Changed (this, new TextChangedEventArgs (search.Text)); OnOpenSelectedItem (); Reset (keepSearchText: true); HideList (); + isShow = false; } private int GetSelectedItemFromSource (ustring value) diff --git a/Terminal.Gui/Views/ListView.cs b/Terminal.Gui/Views/ListView.cs index ce62a2cf0..b4b8e3e02 100644 --- a/Terminal.Gui/Views/ListView.cs +++ b/Terminal.Gui/Views/ListView.cs @@ -96,7 +96,7 @@ namespace Terminal.Gui { /// public class ListView : View { int top, left; - int selected; + int selected = -1; IListDataSource source; /// @@ -112,7 +112,7 @@ namespace Terminal.Gui { source = value; KeystrokeNavigator.Collection = source?.ToList ()?.Cast (); top = 0; - selected = 0; + selected = -1; lastSelectedItem = -1; SetNeedsDisplay (); } @@ -207,7 +207,7 @@ namespace Terminal.Gui { if (value < 0 || (source.Count > 0 && value >= source.Count)) throw new ArgumentException ("value"); - top = value; + top = Math.Max (value, 0); SetNeedsDisplay (); } } @@ -485,7 +485,7 @@ namespace Terminal.Gui { n = 0; if (n != selected) { selected = n; - top = selected; + top = Math.Max (selected, 0); OnSelectedChanged (); SetNeedsDisplay (); } @@ -506,7 +506,7 @@ namespace Terminal.Gui { if (n != selected) { selected = n; if (source.Count >= Frame.Height) - top = selected; + top = Math.Max (selected, 0); else top = 0; OnSelectedChanged (); @@ -540,9 +540,7 @@ namespace Terminal.Gui { if (selected >= top + Frame.Height) { top++; } else if (selected < top) { - top = selected; - } else if (selected < top) { - top = selected; + top = Math.Max (selected, 0); } OnSelectedChanged (); SetNeedsDisplay (); @@ -550,7 +548,7 @@ namespace Terminal.Gui { OnSelectedChanged (); SetNeedsDisplay (); } else if (selected >= top + Frame.Height) { - top = source.Count - Frame.Height; + top = Math.Max (source.Count - Frame.Height, 0); SetNeedsDisplay (); } @@ -581,14 +579,14 @@ namespace Terminal.Gui { selected = Source.Count - 1; } if (selected < top) { - top = selected; + top = Math.Max (selected, 0); } else if (selected > top + Frame.Height) { top = Math.Max (selected - Frame.Height + 1, 0); } OnSelectedChanged (); SetNeedsDisplay (); } else if (selected < top) { - top = selected; + top = Math.Max (selected, 0); SetNeedsDisplay (); } return true; @@ -604,7 +602,7 @@ namespace Terminal.Gui { if (source.Count > 0 && selected != source.Count - 1) { selected = source.Count - 1; if (top + selected > Frame.Height - 1) { - top = selected; + top = Math.Max (selected, 0); } OnSelectedChanged (); SetNeedsDisplay (); @@ -622,7 +620,7 @@ namespace Terminal.Gui { { if (selected != 0) { selected = 0; - top = selected; + top = Math.Max (selected, 0); OnSelectedChanged (); SetNeedsDisplay (); } @@ -750,7 +748,7 @@ namespace Terminal.Gui { { SuperView?.LayoutSubviews (); if (selected < top) { - top = selected; + top = Math.Max (selected, 0); } else if (Frame.Height > 0 && selected >= top + Frame.Height) { top = Math.Max (selected - Frame.Height + 1, 0); } diff --git a/UnitTests/Text/CollectionNavigatorTests.cs b/UnitTests/Text/CollectionNavigatorTests.cs index 118cdb275..8ae522ca8 100644 --- a/UnitTests/Text/CollectionNavigatorTests.cs +++ b/UnitTests/Text/CollectionNavigatorTests.cs @@ -35,12 +35,21 @@ namespace Terminal.Gui.TextTests { [Fact] public void Cycling () { + // cycling with 'b' var n = new CollectionNavigator (simpleStrings); Assert.Equal (2, n.GetNextMatchingItem (0, 'b')); Assert.Equal (3, n.GetNextMatchingItem (2, 'b')); // if 4 (candle) is selected it should loop back to bat Assert.Equal (2, n.GetNextMatchingItem (4, 'b')); + + // cycling with 'a' + n = new CollectionNavigator (simpleStrings); + Assert.Equal (0, n.GetNextMatchingItem (-1, 'a')); + Assert.Equal (1, n.GetNextMatchingItem (0, 'a')); + + // if 4 (candle) is selected it should loop back to appricot + Assert.Equal (0, n.GetNextMatchingItem (4, 'a')); } [Fact] diff --git a/UnitTests/Views/ComboBoxTests.cs b/UnitTests/Views/ComboBoxTests.cs index 74c2a33a7..9732803fc 100644 --- a/UnitTests/Views/ComboBoxTests.cs +++ b/UnitTests/Views/ComboBoxTests.cs @@ -88,13 +88,14 @@ namespace Terminal.Gui.ViewTests { Assert.Equal (-1, cb.SelectedItem); Assert.Equal (string.Empty, cb.Text); var opened = false; - cb.OpenSelectedItem += (s,_) => opened = true; + cb.OpenSelectedItem += (s, _) => opened = true; Assert.True (cb.ProcessKey (new KeyEvent (Key.Enter, new KeyModifiers ()))); Assert.False (opened); cb.Text = "Tw"; Assert.True (cb.ProcessKey (new KeyEvent (Key.Enter, new KeyModifiers ()))); Assert.True (opened); - Assert.Equal ("Two", cb.Text); + Assert.Equal ("", cb.Text); + Assert.False (cb.IsShow); cb.SetSource (null); Assert.False (cb.ProcessKey (new KeyEvent (Key.Enter, new KeyModifiers ()))); Assert.True (cb.ProcessKey (new KeyEvent (Key.F4, new KeyModifiers ()))); // with no source also expand empty @@ -253,11 +254,11 @@ Three Assert.True (cb.ProcessKey (new KeyEvent (Key.Enter, new KeyModifiers ()))); Assert.False (cb.IsShow); Assert.Equal (2, cb.Source.Count); - Assert.Equal (1, cb.SelectedItem); - Assert.Equal ("Two", cb.Text); + Assert.Equal (-1, cb.SelectedItem); + Assert.Equal ("", cb.Text); Assert.True (cb.ProcessKey (new KeyEvent (Key.Esc, new KeyModifiers ()))); Assert.False (cb.IsShow); - Assert.Equal (1, cb.SelectedItem); // retains last accept selected item + Assert.Equal (-1, cb.SelectedItem); // retains last accept selected item Assert.Equal ("", cb.Text); // clear text cb.SetSource (new List ()); Assert.Equal (0, cb.Source.Count); @@ -274,7 +275,7 @@ Three Width = 5 }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -369,7 +370,7 @@ Three HideDropdownListOnClick = true }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -431,7 +432,7 @@ Three HideDropdownListOnClick = true }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -551,7 +552,7 @@ Three ReadOnly = true }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -611,7 +612,7 @@ Three HideDropdownListOnClick = true }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -648,7 +649,7 @@ Three HideDropdownListOnClick = false }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -685,7 +686,7 @@ Three HideDropdownListOnClick = true }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -790,7 +791,7 @@ Three HideDropdownListOnClick = true, }; cb.SetSource (new List { "One", "Two", "Three" }); - cb.OpenSelectedItem += (s,e) => selected = e.Value.ToString (); + cb.OpenSelectedItem += (s, e) => selected = e.Value.ToString (); Application.Top.Add (cb); Application.Begin (Application.Top); @@ -912,8 +913,8 @@ Three ", output); }; var list = new List { "One", "Two", "Three" }; - cb.Expanded += (s,e) => cb.SetSource (list); - cb.Collapsed += (s,e) => cb.Source = null; + cb.Expanded += (s, e) => cb.SetSource (list); + cb.Collapsed += (s, e) => cb.Source = null; Application.Top.Add (cb); Application.Begin (Application.Top); diff --git a/UnitTests/Views/ListViewTests.cs b/UnitTests/Views/ListViewTests.cs index 82a648cc3..d034664f3 100644 --- a/UnitTests/Views/ListViewTests.cs +++ b/UnitTests/Views/ListViewTests.cs @@ -22,19 +22,24 @@ namespace Terminal.Gui.ViewTests { var lv = new ListView (); Assert.Null (lv.Source); Assert.True (lv.CanFocus); + Assert.Equal (-1, lv.SelectedItem); lv = new ListView (new List () { "One", "Two", "Three" }); Assert.NotNull (lv.Source); + Assert.Equal (-1, lv.SelectedItem); lv = new ListView (new NewListDataSource ()); Assert.NotNull (lv.Source); + Assert.Equal (-1, lv.SelectedItem); lv = new ListView (new Rect (0, 1, 10, 20), new List () { "One", "Two", "Three" }); Assert.NotNull (lv.Source); + Assert.Equal (-1, lv.SelectedItem); Assert.Equal (new Rect (0, 1, 10, 20), lv.Frame); lv = new ListView (new Rect (0, 1, 10, 20), new NewListDataSource ()); Assert.NotNull (lv.Source); + Assert.Equal (-1, lv.SelectedItem); Assert.Equal (new Rect (0, 1, 10, 20), lv.Frame); } @@ -46,8 +51,8 @@ namespace Terminal.Gui.ViewTests { Assert.NotNull (lv.Source); - // first item should be selected by default - Assert.Equal (0, lv.SelectedItem); + // first item should be deselected by default + Assert.Equal (-1, lv.SelectedItem); // nothing is ticked Assert.False (lv.Source.IsMarked (0)); @@ -61,6 +66,17 @@ namespace Terminal.Gui.ViewTests { // view should indicate that it has accepted and consumed the event Assert.True (lv.ProcessKey (ev)); + // first item should now be selected + Assert.Equal (0, lv.SelectedItem); + + // none of the items should be ticked + Assert.False (lv.Source.IsMarked (0)); + Assert.False (lv.Source.IsMarked (1)); + Assert.False (lv.Source.IsMarked (2)); + + // Press key combo again + Assert.True (lv.ProcessKey (ev)); + // second item should now be selected Assert.Equal (1, lv.SelectedItem); @@ -109,8 +125,8 @@ namespace Terminal.Gui.ViewTests { Assert.NotNull (lv.Source); - // first item should be selected by default - Assert.Equal (0, lv.SelectedItem); + // first item should be deselected by default + Assert.Equal (-1, lv.SelectedItem); // bind shift down to move down twice in control lv.AddKeyBinding (Key.CursorDown | Key.ShiftMask, Command.LineDown, Command.LineDown); @@ -119,8 +135,8 @@ namespace Terminal.Gui.ViewTests { Assert.True (lv.ProcessKey (ev), "The first time we move down 2 it should be possible"); - // After moving down twice from One we should be at 'Three' - Assert.Equal (2, lv.SelectedItem); + // After moving down twice from -1 we should be at 'Two' + Assert.Equal (1, lv.SelectedItem); // clear the items lv.SetSource (null); @@ -160,9 +176,9 @@ namespace Terminal.Gui.ViewTests { { List source = new List () { "One", "Two", "Three" }; ListView lv = new ListView (source) { Height = 2, AllowsMarking = true }; - Assert.Equal (0, lv.SelectedItem); + Assert.Equal (-1, lv.SelectedItem); Assert.True (lv.ProcessKey (new KeyEvent (Key.CursorDown, new KeyModifiers ()))); - Assert.Equal (1, lv.SelectedItem); + Assert.Equal (0, lv.SelectedItem); Assert.True (lv.ProcessKey (new KeyEvent (Key.CursorUp, new KeyModifiers ()))); Assert.Equal (0, lv.SelectedItem); Assert.True (lv.ProcessKey (new KeyEvent (Key.PageDown, new KeyModifiers ()))); @@ -175,7 +191,7 @@ namespace Terminal.Gui.ViewTests { Assert.True (lv.ProcessKey (new KeyEvent (Key.Space, new KeyModifiers ()))); Assert.True (lv.Source.IsMarked (lv.SelectedItem)); var opened = false; - lv.OpenSelectedItem += (s,_) => opened = true; + lv.OpenSelectedItem += (s, _) => opened = true; Assert.True (lv.ProcessKey (new KeyEvent (Key.Enter, new KeyModifiers ()))); Assert.True (opened); Assert.True (lv.ProcessKey (new KeyEvent (Key.End, new KeyModifiers ()))); @@ -191,7 +207,7 @@ namespace Terminal.Gui.ViewTests { var rendered = false; var source = new List () { "one", "two", "three" }; var lv = new ListView () { Width = Dim.Fill (), Height = Dim.Fill () }; - lv.RowRender += (s,_) => rendered = true; + lv.RowRender += (s, _) => rendered = true; Application.Top.Add (lv); Application.Begin (Application.Top); Assert.False (rendered); @@ -246,7 +262,7 @@ namespace Terminal.Gui.ViewTests { ((FakeDriver)Application.Driver).SetBufferSize (12, 12); Application.Refresh (); - Assert.Equal (0, lv.SelectedItem); + Assert.Equal (-1, lv.SelectedItem); TestHelpers.AssertDriverContentsWithFrameAre (@" ┌──────────┐ │Line0 │ @@ -263,7 +279,7 @@ namespace Terminal.Gui.ViewTests { Assert.True (lv.ScrollDown (10)); lv.Redraw (lv.Bounds); - Assert.Equal (0, lv.SelectedItem); + Assert.Equal (-1, lv.SelectedItem); TestHelpers.AssertDriverContentsWithFrameAre (@" ┌──────────┐ │Line10 │ @@ -280,9 +296,10 @@ namespace Terminal.Gui.ViewTests { Assert.True (lv.MoveDown ()); lv.Redraw (lv.Bounds); - Assert.Equal (1, lv.SelectedItem); + Assert.Equal (0, lv.SelectedItem); TestHelpers.AssertDriverContentsWithFrameAre (@" ┌──────────┐ +│Line0 │ │Line1 │ │Line2 │ │Line3 │ @@ -292,7 +309,6 @@ namespace Terminal.Gui.ViewTests { │Line7 │ │Line8 │ │Line9 │ -│Line10 │ └──────────┘", output); Assert.True (lv.MoveEnd ());