diff --git a/Terminal.Gui/Views/ListView/ListView.Drawing.cs b/Terminal.Gui/Views/ListView/ListView.Drawing.cs index 980619c9fc..c3cb2066da 100644 --- a/Terminal.Gui/Views/ListView/ListView.Drawing.cs +++ b/Terminal.Gui/Views/ListView/ListView.Drawing.cs @@ -11,66 +11,70 @@ protected override bool OnDrawingContent (DrawContext? context) } var current = Attribute.Default; - Move (0, 0); - Rectangle f = Viewport; int item = Viewport.Y; - bool focused = HasFocus; int col = ShowMarks ? 2 : 0; - int start = Viewport.X; + Move (0, 0); - for (var row = 0; row < f.Height; row++, item++) + for (var row = 0; row < Viewport.Height; row++, item++) { bool isSelected = item == SelectedItem; - bool isMarked = Source!.IsMarked (item); - bool hasFocus = focused; + bool isMarked = item < Source?.Count && Source?.IsMarked (item) is true; // Determine visual role based on the 4 combinations of ShowMarks and MarkMultiple VisualRole role; var applyHighlightStyle = false; - if (!ShowMarks && !MarkMultiple) - { - // Combination 1: Standard selection mode (no marking) - // Mark glyphs: None (MarkWidth = 0) - // Visual roles: SelectedItem uses Focus (focused) or Active (not focused) - role = isSelected ? hasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; - } - else if (!ShowMarks && MarkMultiple) + switch (ShowMarks) { - // Combination 2: Hidden marks with visual role indicators - // Mark glyphs: None (MarkWidth = 0) - marks exist internally - // Visual roles use Highlight for marked items; compose TextStyle when marked+selected+focused - if (isSelected && isMarked) - { - role = hasFocus ? VisualRole.Focus : VisualRole.Highlight; - applyHighlightStyle = hasFocus; // Apply Highlight's TextStyle to Focus - } - else if (isSelected) - { - role = hasFocus ? VisualRole.Focus : VisualRole.Normal; - } - else if (isMarked) - { - role = VisualRole.Highlight; - } - else + case false when !MarkMultiple: + // Combination 1: Standard selection mode (no marking) + // Mark glyphs: None (MarkWidth = 0) + // Visual roles: SelectedItem uses Focus (focused) or Active (not focused) + role = isSelected ? HasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; + + break; + + case false when MarkMultiple: { - role = VisualRole.Normal; + switch (isSelected) + { + // Combination 2: Hidden marks with visual role indicators + // Mark glyphs: None (MarkWidth = 0) - marks exist internally + // Visual roles use Highlight for marked items; compose TextStyle when marked+selected+focused + case true when isMarked: + role = HasFocus ? VisualRole.Focus : VisualRole.Highlight; + applyHighlightStyle = HasFocus; // Apply Highlight's TextStyle to Focus + + break; + + case true: role = HasFocus ? VisualRole.Focus : VisualRole.Normal; break; + + default: + { + role = isMarked ? VisualRole.Highlight : VisualRole.Normal; + + break; + } + } + + break; } - } - else if (ShowMarks && !MarkMultiple) - { - // Combination 3: Radio button style - // Mark glyphs: Radio-button style (◉ marked, ○ unmarked) - // Visual roles: Standard selection (mark glyphs provide visual indication) - role = isSelected ? hasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; - } - else // ShowMarks == true && MarkMultiple == true - { - // Combination 4: Checkbox style - // Mark glyphs: Checkbox style (☒ marked, ☐ unmarked) - // Visual roles: Standard selection (mark glyphs provide visual indication) - role = isSelected ? hasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; + + case true when !MarkMultiple: + // Combination 3: Radio button style + // Mark glyphs: Radio-button style (◉ marked, ○ unmarked) + // Visual roles: Standard selection (mark glyphs provide visual indication) + role = isSelected ? HasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; + + break; + + default: + // Combination 4: Checkbox style + // Mark glyphs: Checkbox style (☒ marked, ☐ unmarked) + // Visual roles: Standard selection (mark glyphs provide visual indication) + role = isSelected ? HasFocus ? VisualRole.Focus : VisualRole.Active : VisualRole.Normal; + + break; } Attribute newAttribute = GetAttributeForRole (role); @@ -92,7 +96,7 @@ protected override bool OnDrawingContent (DrawContext? context) if (Source is null || item >= Source.Count) { - for (var c = 0; c < f.Width; c++) + for (var c = 0; c < Viewport.Width; c++) { AddRune ((Rune)' '); } @@ -142,7 +146,7 @@ protected override bool OnDrawingContent (DrawContext? context) } int contentCol = col > 0 ? col : markWidth; - Source.Render (this, isSelected, item, contentCol, row, f.Width - contentCol, start); + Source.Render (this, isSelected, item, contentCol, row, Viewport.Width - contentCol, Viewport.X); } } @@ -159,5 +163,4 @@ protected override bool OnDrawingContent (DrawContext? context) /// This event is invoked when this is being drawn before rendering. public event EventHandler? RowRender; - } diff --git a/Tests/UnitTestsParallelizable/Views/ListViewTests.cs b/Tests/UnitTestsParallelizable/Views/ListViewTests.cs index 8debc4a000..549d7188e0 100644 --- a/Tests/UnitTestsParallelizable/Views/ListViewTests.cs +++ b/Tests/UnitTestsParallelizable/Views/ListViewTests.cs @@ -873,7 +873,7 @@ public void KeyBindings_Command () Assert.Equal (2, lv.SelectedItem); Assert.True (lv.NewKeyDownEvent (Key.PageUp)); Assert.Equal (0, lv.SelectedItem); - + // In standard selection mode (ShowMarks=false), Space doesn't mark Assert.False (lv.Source.IsMarked (lv.SelectedItem!.Value)); lv.NewKeyDownEvent (Key.Space); @@ -2670,4 +2670,413 @@ public void HorizontalScroll_With_Marks_Accounts_For_MarkWidth () } #endregion + + #region Out-of-range guard tests + + // Claude - Opus 4.6 + // Verifies that ListView.OnDrawingContent does not call IsMarked with out-of-range indices + // when the Viewport is taller than the number of items in the source. + [Fact] + public void Draw_DoesNotCall_IsMarked_OutOfRange () + { + ObservableCollection source = ["one", "two"]; + + // Track all indices passed to IsMarked + List isMarkedCalls = []; + Mock mockSource = new (MockBehavior.Strict); + mockSource.Setup (s => s.Count).Returns (source.Count); + + mockSource.Setup (s => s.IsMarked (It.IsAny ())) + .Returns ((int item) => + { + isMarkedCalls.Add (item); + Assert.True (item >= 0 && item < source.Count, $"IsMarked called with out-of-range index {item}"); + + return false; + }); + + mockSource.Setup (s => s.Render (It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny ())); + mockSource.Setup (s => s.RenderMark (It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny ())).Returns (false); + mockSource.Setup (s => s.MaxItemLength).Returns (5); + mockSource.Setup (s => s.ToList ()).Returns (source); + mockSource.Setup (s => s.SuspendCollectionChangedEvent).Returns (false); + mockSource.As ().Setup (d => d.Dispose ()); + + // Viewport height (10) is much larger than item count (2) + ListView lv = new () { Width = 20, Height = 10, Source = mockSource.Object }; + lv.Layout (); + lv.Draw (); + + // Verify IsMarked was only called with valid indices + Assert.All (isMarkedCalls, i => Assert.InRange (i, 0, source.Count - 1)); + } + + // Claude - Opus 4.6 + // Verifies that ListView.OnDrawingContent does not call Render with out-of-range indices + // when the Viewport is taller than the number of items in the source. + [Fact] + public void Draw_DoesNotCall_Render_OutOfRange () + { + ObservableCollection source = ["one", "two"]; + + List renderCalls = []; + Mock mockSource = new (MockBehavior.Strict); + mockSource.Setup (s => s.Count).Returns (source.Count); + mockSource.Setup (s => s.IsMarked (It.IsAny ())).Returns (false); + + mockSource.Setup (s => s.Render (It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny ())) + .Callback ((ListView _, bool _, int item, int _, int _, int _, int _) => + { + renderCalls.Add (item); + Assert.True (item >= 0 && item < source.Count, $"Render called with out-of-range index {item}"); + }); + + mockSource.Setup (s => s.RenderMark (It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny ())).Returns (false); + mockSource.Setup (s => s.MaxItemLength).Returns (5); + mockSource.Setup (s => s.ToList ()).Returns (source); + mockSource.Setup (s => s.SuspendCollectionChangedEvent).Returns (false); + mockSource.As ().Setup (d => d.Dispose ()); + + ListView lv = new () { Width = 20, Height = 10, Source = mockSource.Object }; + lv.Layout (); + lv.Draw (); + + // Verify Render was only called with valid indices + Assert.All (renderCalls, i => Assert.InRange (i, 0, source.Count - 1)); + } + + // Claude - Opus 4.6 + // Verifies that ListView.OnDrawingContent does not call SetMark with out-of-range indices + // when the Viewport is taller than the number of items in the source. + [Fact] + public void Draw_DoesNotCall_SetMark_OutOfRange () + { + ObservableCollection source = ["one", "two"]; + + List setMarkCalls = []; + Mock mockSource = new (MockBehavior.Strict); + mockSource.Setup (s => s.Count).Returns (source.Count); + mockSource.Setup (s => s.IsMarked (It.IsAny ())).Returns (false); + + mockSource.Setup (s => s.Render (It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny (), + It.IsAny ())); + mockSource.Setup (s => s.RenderMark (It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny (), It.IsAny ())).Returns (false); + + mockSource.Setup (s => s.SetMark (It.IsAny (), It.IsAny ())) + .Callback ((int item, bool _) => + { + setMarkCalls.Add (item); + Assert.True (item >= 0 && item < source.Count, $"SetMark called with out-of-range index {item}"); + }); + + mockSource.Setup (s => s.MaxItemLength).Returns (5); + mockSource.Setup (s => s.ToList ()).Returns (source); + mockSource.Setup (s => s.SuspendCollectionChangedEvent).Returns (false); + mockSource.As ().Setup (d => d.Dispose ()); + + ListView lv = new () { Width = 20, Height = 10, ShowMarks = true, Source = mockSource.Object }; + lv.Layout (); + lv.Draw (); + + // SetMark should not have been called at all during Draw, but if it was, only with valid indices + Assert.All (setMarkCalls, i => Assert.InRange (i, 0, source.Count - 1)); + } + + #endregion + + #region IValue Implementation + + // Claude - Opus 4.6 + [Fact] + public void ValueChanging_Event_Can_Cancel_Selection_Change () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + lv.ValueChanging += (_, args) => args.Handled = true; + + lv.SelectedItem = 1; + + Assert.Equal (0, lv.SelectedItem); + } + + // Claude - Opus 4.6 + [Fact] + public void ValueChanging_Event_Provides_CurrentValue_And_NewValue () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + int? capturedCurrent = null; + int? capturedNew = null; + + lv.ValueChanging += (_, args) => + { + capturedCurrent = args.CurrentValue; + capturedNew = args.NewValue; + }; + + lv.SelectedItem = 2; + + Assert.Equal (0, capturedCurrent); + Assert.Equal (2, capturedNew); + Assert.Equal (2, lv.SelectedItem); + } + + // Claude - Opus 4.6 + [Fact] + public void ValueChanged_Event_Fires_After_Selection_Change () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + int? oldValue = null; + int? newValue = null; + + lv.ValueChanged += (_, args) => + { + oldValue = args.OldValue; + newValue = args.NewValue; + }; + + lv.SelectedItem = 2; + + Assert.Equal (0, oldValue); + Assert.Equal (2, newValue); + } + + // Claude - Opus 4.6 + [Fact] + public void ValueChanged_DoesNotFire_When_ValueChanging_Cancels () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + var changedFired = false; + lv.ValueChanging += (_, args) => args.Handled = true; + lv.ValueChanged += (_, _) => changedFired = true; + + lv.SelectedItem = 1; + + Assert.False (changedFired); + Assert.Equal (0, lv.SelectedItem); + } + + // Claude - Opus 4.6 + [Fact] + public void ValueChangedUntyped_Event_Fires_After_Selection_Change () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + object? oldValue = null; + object? newValue = null; + + lv.ValueChangedUntyped += (_, args) => + { + oldValue = args.OldValue; + newValue = args.NewValue; + }; + + lv.SelectedItem = 1; + + Assert.Equal (0, oldValue); + Assert.Equal (1, newValue); + } + + // Claude - Opus 4.6 + [Fact] + public void Value_Property_Is_Alias_For_SelectedItem () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + + lv.Value = 2; + Assert.Equal (2, lv.SelectedItem); + Assert.Equal (2, lv.Value); + + lv.SelectedItem = 0; + Assert.Equal (0, lv.Value); + } + + // Claude - Opus 4.6 + [Fact] + public void IValue_GetValue_Returns_SelectedItem () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 1; + + object? value = ((IValue)lv).GetValue (); + + Assert.Equal (1, value); + } + + // Claude - Opus 4.6 + [Fact] + public void IValue_GetValue_Returns_Null_When_No_Selection () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + + object? value = ((IValue)lv).GetValue (); + + Assert.Null (value); + } + + #endregion IValue Implementation + + #region Ctrl+A / Ctrl+U SelectAll + + // Claude - Opus 4.6 + [Fact] + public void CtrlA_Marks_All_Items_When_MarkMultiple_True () + { + ObservableCollection source = ["one", "two", "three", "four"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source), MarkMultiple = true }; + lv.SelectedItem = 0; + + lv.NewKeyDownEvent (Key.A.WithCtrl); + + List marked = lv.GetAllMarkedItems ().ToList (); + Assert.Equal (4, marked.Count); + Assert.Equal ([0, 1, 2, 3], marked); + } + + // Claude - Opus 4.6 + [Fact] + public void CtrlU_Unmarks_All_Items_When_MarkMultiple_True () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source), MarkMultiple = true }; + lv.SelectedItem = 0; + + // Mark all first + lv.NewKeyDownEvent (Key.A.WithCtrl); + Assert.Equal (3, lv.GetAllMarkedItems ().Count ()); + + // Unmark all + lv.NewKeyDownEvent (Key.U.WithCtrl); + Assert.Empty (lv.GetAllMarkedItems ()); + } + + // Claude - Opus 4.6 + [Fact] + public void CtrlA_Does_Nothing_When_MarkMultiple_False () + { + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source), MarkMultiple = false }; + lv.SelectedItem = 0; + + lv.NewKeyDownEvent (Key.A.WithCtrl); + + Assert.Empty (lv.GetAllMarkedItems ()); + } + + #endregion Ctrl+A / Ctrl+U SelectAll + + #region RowRender RowAttribute Override + + // Claude - Opus 4.6 + [Fact] + public void RowRender_Event_RowAttribute_Applied_To_Specific_Row () + { + IApplication app = Application.Create (); + app.Init (DriverRegistry.Names.ANSI); + + ObservableCollection source = ["one", "two", "three"]; + ListView lv = new () { Width = 10, Height = 5, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + Attribute customAttr = new (Color.Red, Color.Blue); + List renderedRows = []; + + lv.RowRender += (_, args) => + { + renderedRows.Add (args.Row); + + if (args.Row == 1) + { + args.RowAttribute = customAttr; + } + }; + + Runnable top = new (); + top.Add (lv); + app.Begin (top); + app.LayoutAndDraw (); + + // Verify the event was called for each visible row + Assert.Contains (0, renderedRows); + Assert.Contains (1, renderedRows); + Assert.Contains (2, renderedRows); + + top.Dispose (); + app.Dispose (); + } + + // Claude - Opus 4.6 + [Fact] + public void RowRender_Event_Receives_Correct_Row_Indices () + { + ObservableCollection source = ["a", "b", "c", "d", "e"]; + ListView lv = new () { Width = 10, Height = 3, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + List receivedRows = []; + + lv.RowRender += (_, args) => receivedRows.Add (args.Row); + + lv.Layout (); + lv.Draw (); + + // With height=3, only 3 rows visible starting from viewport Y=0 + Assert.Equal ([0, 1, 2], receivedRows); + } + + // Claude - Opus 4.6 + [Fact] + public void RowRender_Event_Receives_Correct_Row_Indices_After_Scroll () + { + ObservableCollection source = ["a", "b", "c", "d", "e"]; + ListView lv = new () { Width = 10, Height = 3, Source = new ListWrapper (source) }; + lv.SelectedItem = 0; + + lv.Layout (); + + // Scroll down so viewport starts at row 2 + lv.Viewport = lv.Viewport with { Y = 2 }; + + List receivedRows = []; + lv.RowRender += (_, args) => receivedRows.Add (args.Row); + + lv.Draw (); + + // After scrolling, should render items 2, 3, 4 + Assert.Equal ([2, 3, 4], receivedRows); + } + + #endregion RowRender RowAttribute Override }