Skip to content

Fixes #4869. Add ListView<T> with typed Value, SelectedItem, Index#4870

Merged
tig merged 12 commits intogui-cs:v2_developfrom
YourRobotOverlord:feature/generic-listview
Mar 30, 2026
Merged

Fixes #4869. Add ListView<T> with typed Value, SelectedItem, Index#4870
tig merged 12 commits intogui-cs:v2_developfrom
YourRobotOverlord:feature/generic-listview

Conversation

@YourRobotOverlord
Copy link
Copy Markdown
Collaborator

Fixes

Proposed Changes

ListView<T>

New generic view extending ListView and implementing IValue<T>. The key difference from ListView: Value returns the actual selected object of type T rather than the selected index.

Member Type Notes
Value new T? Primary — the selected object
SelectedItem new T? Convenience alias for Value, mirrors ListView.SelectedItem
Index int? (get/set) Access/set the underlying int? index
AspectGetter Func<T,string>? Custom display-text converter
SetSource(ObservableCollection<T>?) method Typed source
ValueChanging event EventHandler<ValueChangingEventArgs<T?>> Typed, cancellable
ValueChanged event EventHandler<ValueChangedEventArgs<T?>> Typed

SelectedItem follows the existing doc convention: a convenience property that is an alias for Value. Index is the way to get/set the integer index when using ListView<T>.

Typed ValueChanging/ValueChanged events are produced via an event-bridge that subscribes to the base int? events and translates them to T?. Cancellation in the typed event correctly propagates back through intArgs.Handled.

ListWrapper<T>.AspectGetter

csharp public Func<T, string>? AspectGetter { get; set; }

When set, Render() and GetMaxLengthItem() use the delegate instead of ToString(). MaxItemLength is recalculated immediately when the property is set. Fully backward-compatible — null by default.

ListView<T>.AspectGetter forwards the delegate to the underlying ListWrapper<T>, whether set before or after SetSource().

UICatalog scenario

New GenericListView scenario demonstrating:

  • ListView<Country> backed by ObservableCollection<Country>
  • Detail panel populated from typed ValueChanged args (proves Value is the object, not an index)
  • Index label showing the int? alongside the typed value
  • Typed event log (country names, not ints)
  • ''Cancel next change'' checkbox exercising ValueChanging.Handled
  • AspectGetter = c => c.Name for clean display

Tests

28 new passing tests, 0 failures:

  • ListViewTTests.cs — 24 tests covering Value, SelectedItem, Index, ValueChanging/ValueChanged, cancellation, AspectGetter integration
  • ListWrapperTests.cs (new file) — 4 tests directly on ListWrapper<T>.AspectGetter (null fallback, delegate used, MaxItemLength updated, clearing recalculates)

Pull Request checklist

  • Named in the form Fixes #issue. Terse description.
  • Code follows Terminal.Gui style guidelines
  • Code follows Terminal.Gui library design guidelines
  • dotnet test run before commit — all 28 new tests pass, 0 failures
  • API documentation updated (/// XML docs on all public members)
  • Changes generate no new warnings
  • Spelling and grammar checked
  • Basic QA conducted (UICatalog scenario builds; GenericListView scenario included)

@YourRobotOverlord YourRobotOverlord requested a review from tig as a code owner March 29, 2026 00:13
YourRobotOverlord and others added 3 commits March 28, 2026 18:54
…perties

- ListView<T> extends ListView and implements IValue<T>
- Value (T?) is the primary property returning the selected object
- SelectedItem (new T?) is a convenience alias for Value, mirroring
  how ListView.SelectedItem aliases ListView.Value
- Index (int?) provides read/write access to the underlying int? index
- SetSource(ObservableCollection<T>?) wires the typed source
- Event bridge translates base int? ValueChanging/ValueChanged events
  to typed T? events; cancellation propagates back correctly
- 22 unit tests in UnitTestsParallelizable

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shows:
- ListView<T>.Value returning typed Country object (not int index)
- SelectedItem as a convenience alias for Value
- Index property for int? index access
- Typed ValueChanging<Country?>/ValueChanged<Country?> events
- Cancellation via ValueChanging.Handled

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ListWrapper<T>.AspectGetter (Func<T,string>?) lets callers provide a
custom display-text converter instead of relying on ToString().
Setting the property recalculates MaxItemLength immediately.

ListView<T>.AspectGetter exposes the same delegate and wires it to
the underlying ListWrapper<T> automatically — works whether set
before or after SetSource().

GenericListView UICatalog scenario updated to use:
  AspectGetter = c => c.Name

Tests added:
- ListWrapperTests.cs (4 new tests for ListWrapper<T> directly)
- ListViewTTests.cs (2 new integration tests via ListView<T>)
Total: 28 passing tests across both files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YourRobotOverlord YourRobotOverlord force-pushed the feature/generic-listview branch from 71150f2 to 70b5b93 Compare March 29, 2026 00:55
Replaces inheritdoc with explicit documentation clarifying that
GetValue() returns the typed T? object (not the int? index that
base ListView.GetValue() returns).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tig
Copy link
Copy Markdown
Collaborator

tig commented Mar 29, 2026

Do you think there would be any value in refactoring DropDownList to be based on this?

@YourRobotOverlord
Copy link
Copy Markdown
Collaborator Author

YourRobotOverlord commented Mar 29, 2026

Do you think there would be any value in refactoring DropDownList to be based on this?

Yes I do, although I'm not sure how it would behave in Editable mode. It would be pretty handy if it behaved like Prompt{T} for editing complex types.

@tig tig requested review from Copilot and removed request for tig March 29, 2026 14:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a strongly-typed ListView<T> API on top of the existing ListView/ListWrapper<T> infrastructure, enabling selection/value access as the selected object (not just an index) and enabling a simple AspectGetter for display text conversion.

Changes:

  • Introduces Terminal.Gui.Views.ListView<T> with typed Value/SelectedItem, Index, typed ValueChanging/ValueChanged, and AspectGetter.
  • Extends ListWrapper<T> with AspectGetter and integrates it into rendering and max-item-length calculation.
  • Adds a new UICatalog scenario plus new unit tests covering the generic view and wrapper behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
Tests/UnitTestsParallelizable/Views/ListWrapperTests.cs Adds tests for ListWrapper<T>.AspectGetter behavior and MaxItemLength recalculation.
Tests/UnitTestsParallelizable/Views/ListViewTTests.cs Adds tests for ListView<T> typed value semantics, typed events, cancellation, and AspectGetter forwarding.
Terminal.Gui/Views/ListView/ListWrapper.cs Implements AspectGetter and uses it for rendering and width calculation.
Terminal.Gui/Views/ListView/ListViewT.cs Adds the new generic ListView<T> type and event/value translation from index-based selection.
Examples/UICatalog/Scenarios/GenericListView.cs Adds a scenario demonstrating typed selection, typed events, cancellation, and AspectGetter.

Comment thread Examples/UICatalog/Scenarios/GenericListView.cs Outdated
Comment thread Examples/UICatalog/Scenarios/GenericListView.cs Outdated
Comment thread Examples/UICatalog/Scenarios/GenericListView.cs Outdated
Comment thread Examples/UICatalog/Scenarios/GenericListView.cs Outdated
Comment thread Terminal.Gui/Views/ListView/ListWrapper.cs Outdated
Comment thread Terminal.Gui/Views/ListView/ListWrapper.cs Outdated
Comment thread Terminal.Gui/Views/ListView/ListWrapper.cs Outdated
YourRobotOverlord and others added 4 commits March 29, 2026 12:08
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@YourRobotOverlord
Copy link
Copy Markdown
Collaborator Author

@tig @BDisp @tznind I have some questions about how to move forward with AspectGetter. Looking at it again, I'm not sure what I have is the best option. I'd like to get this right because I would like to take it a bit further and also add a ColorGetter property to align with what TreeView has.
I make heavy use of both colored list and trees and I think the feature might be useful to others as well.

Example
WindowsTerminal_CfoSFQkJOH

Problems
I've added the AspectGetter to the default ListWrapper implementation which has a couple implications:

  1. In order to supply a aspect getter, you can only use the default ListWrapper implementation or a subclass of it.
  2. ListWrapper was created for the base ListView and now contains code that does not apply to base ListView.

Options
Some options I can think of:

  1. Remove AspectGetter from ListView<T> altogether and let the developer handle it with their own IListDataSource implementation - ListWrapper.Render would revert to just using ToString.
  2. Remove AspectGetter from ListView<T> and only keep it in ListWrapper and IListDateSource (or a separate implementation). Doesn't get rid of Problem 1.
  3. Add AspectGetter and ColorGetter to base ListView and IListDataSource. Solves Problems 1 and 2, and aligns ListView with TreeView
  4. Other...

@tig
Copy link
Copy Markdown
Collaborator

tig commented Mar 29, 2026

I vote 1. AspectGetter is more elegant but given LV already has a way of doing custom rendering we can just leverage that.

An AspectGetter approach could be added later.

@BDisp
Copy link
Copy Markdown
Collaborator

BDisp commented Mar 29, 2026

I also vote for option 1 because ListWrapper already uses the generic ObservableCollection<T> where T can be any type that overrides the ToString method. Just look at the example in the DynamicStatusBar scenario where there's the DynamicStatusItemList class that's used as the source for ListWrapper. Furthermore, ListView has the OnRowRender method where it's possible to customize the attribute for each row. I think that even with just ListView it's possible to achieve the same or almost the same behavior.

@YourRobotOverlord YourRobotOverlord marked this pull request as draft March 29, 2026 20:34
YourRobotOverlord and others added 3 commits March 29, 2026 15:00
- Remove AspectGetter and ColorGetter from ListWrapper<T> and ListView<T>
- Delete IAspectGetter<T> and IColorGetter<T> interfaces (no longer needed)
- Fix GetMaxLengthItem(): use .GetColumns() instead of .Length for
  correct Unicode width measurement; rename pattern variable u -> s
- Update GenericListView scenario to demonstrate the same functionality
  using built-in mechanisms (per BDisp's suggestion):
  - Country.ToString() override for display text (replaces AspectGetter)
  - ListView.RowRender event for per-row coloring (replaces ColorGetter)
- Remove AspectGetter/ColorGetter tests from ListViewTTests.cs
- Remove ListWrapperTests.cs (all tests were AspectGetter/ColorGetter)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Render() and GetMaxLengthItem() cosmetic refactors (object? vs T typedItem,
switch statement vs switch expression) are reverted so ListWrapper<T> is
minimal-diff from upstream. The GetMaxLengthItem() bug fix is retained:
use .GetColumns() instead of .Length for correct Unicode width measurement,
and rename pattern variable u -> s (per PR review feedback).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Collaborator

@BDisp BDisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

Document that setting Value to an object not in the collection is a
no-op (selection unchanged), and explain why this differs from the
base ListView.SelectedItem setter which throws ArgumentException for
out-of-range indexes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@YourRobotOverlord YourRobotOverlord marked this pull request as ready for review March 29, 2026 22:04
@YourRobotOverlord
Copy link
Copy Markdown
Collaborator Author

Pretty sure the failing test is not from me:

failed ViewsTests.MenuBarTests.Escape_Closes_Open_PopoverMenu (1s 458ms)
  Assert.False() Failure
Expected: False
Actual:   True
  from D:\a\Terminal.Gui\Terminal.Gui\Tests\UnitTestsParallelizable\bin\Debug\net10.0\UnitTests.Parallelizable.dll (net10.0|x64)
  Assert.False() Failure
  Expected: False
  Actual:   True
    at ViewsTests.MenuBarTests.Escape_Closes_Open_PopoverMenu() in D:\a\Terminal.Gui\Terminal.Gui\Tests\UnitTestsParallelizable\Views\MenuBarTests.cs:997
    at System.RuntimeMethodHandle.InvokeMethod(ObjectHandleOnStack target, Void** arguments, ObjectHandleOnStack sig, BOOL isConstructor, ObjectHandleOnStack result)
    at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)

Copy link
Copy Markdown
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lovely work. Thank you!

@tig tig merged commit 956a292 into gui-cs:v2_develop Mar 30, 2026
10 of 11 checks passed
@YourRobotOverlord YourRobotOverlord changed the title Fixes #4869. Add ListView<T> with typed Value, SelectedItem, Index, and AspectGetter Fixes #4869. Add ListView<T> with typed Value, SelectedItem, Index Apr 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Generic ListView<T> with typed Value, SelectedItem

4 participants