Skip to content

TableView needs re-factoring before v2.0.0 can be released - BREAKING CHANGES #5064

Description

@tig

TableView Redesign Spec

POC branch: Issue-4963-filedialog (PR #5062) contains a proof-of-concept demonstrating several of the changes described here. This spec is the forward-looking design; the POC is exploratory and will be superseded by a clean implementation.

Problem Statement

TableView predates Terminal.Gui v2's command/event architecture. It suffers from:

  • A manual OnMouseEvent override instead of declarative MouseBindings
  • Custom event types (CellActivated, CellToggled, SelectedCellChanged) instead of the standard Command → CWP pattern (Accept/Activate/ValueChanged)
  • No IValue<T> implementation — consumers can't data-bind to the selection
  • Confusing terminology: "Activated" meant "accepted" (Enter/double-click), "Toggled" meant "extended into multi-select", and cursor position was conflated with selection state
  • A bloated OnKeyDown that mixed command-bound key dispatch with collection-navigator typing
  • ICommandContext not threaded through selection methods, blocking future context-aware behavior
  • -1 is used as a sentinel value for "no selection" in SelectedColumn/SelectedRow, which is error-prone and doesn't translate to the new Value model. Nullable types are more idiomatic and safer.

This spec defines the target design.


1. Lexicon & Taxonomy

Term Command Meaning Input Old Term(s)
Accept / OnAccepted Command.Accept User confirms/commits the current selection — "I'm done choosing." Enter, double-click CellActivated, OnCellActivated, CellActivatedEventArgs, CellActivationKey
Activate / OnActivated Command.Activate User navigates to or interacts with a cell — the cursor moves, Value updates. Arrow keys, mouse click (no direct equivalent — was implicit in OnMouseEvent and navigation code)
Extend / OnExtended Command.ToggleExtend User adds or removes a cell from the multi-selection set. Like Ctrl+click in Excel. Space, Ctrl+click CellToggled, OnCellToggled, IsToggled
ToggleExtend (ICommandContext?) Command.ToggleExtend Unified handler for extend operations. Inspects ICommandContext: if MouseBinding → extends the clicked cell; otherwise → extends the cursor cell. Space, Ctrl+click, Alt+click ToggleCurrentCellSelection, ExtendSelection
Cursor The single active cell for navigation. May or may not be in the selection set (e.g., Shift+Arrow extends a region while the cursor moves away from it). Accessed via Value.Cursor. SelectedColumn, SelectedRow, SelectedCellChanged, RaiseSelectedCellChanged, EnsureSelectedCellIsVisible
Value (IValue<TableSelection>) The complete selection state: cursor position + all extended regions. Enables data binding via ValueChanged/ValueChanging CWP pattern. Single source of truth — replaces all the separate column/row properties and their events. SelectedColumn, SelectedRow, SelectedCellChanged, Value (currently Point?)
TableSelection New composite type: Cursor (Point) + Regions (IReadOnlyList<TableSelectionRegion>). Replaces the raw Stack<TableSelection> for multi-select. MultiSelectedRegions (Stack<TableSelection>)
TableSelectionRegion A single contiguous rectangular selection region. Renamed from the current TableSelection class. TableSelection (current class)
DefaultKeyBindings Static dictionary declaring all TableView key→command mappings. Replaces hardcoded bindings and the CellActivationKey property. CellActivationKey, hardcoded KeyBindings.Add calls
EnsureCursorIsVisible Scrolls the viewport so the cursor cell is visible. Renamed for consistency with Cursor terminology. EnsureSelectedCellIsVisible

TableSelection Type Design

The current TableSelection class represents a single rectangular region (Origin, Rectangle, IsToggled). Disjoint multi-select uses Stack<TableSelection>. Neither works alone as IValue<T>:

  • TableSelection can't represent disjoint selections or the cursor
  • Stack<TableSelection> leaks implementation detail and has no cursor concept

Design: Rename the existing type and introduce a new composite:

// Rename current TableSelection → TableSelectionRegion
public class TableSelectionRegion
{
    public Point Origin { get; }
    public Rectangle Rectangle { get; }
    public bool IsExtended { get; }
}

// New composite type for IValue<T>
public class TableSelection
{
    public Point? Cursor { get; }                               // Active cell (navigation anchor); null = no cursor
    public IReadOnlyList<TableSelectionRegion> Regions { get; }  // All selected regions (may be disjoint)

    // Helpers
    public bool IsEmpty { get; }
    public bool Contains (int col, int row);
}

TableView implements IValue<TableSelection?>null means no selection (no table assigned, or selection explicitly cleared). This is a breaking change (pre-v2.0.0 release); all legacy APIs (SelectedColumn, SelectedRow, SelectedCellChanged, CellActivated, CellToggled, CellActivationKey, and their associated types) are removed, not deprecated.

Equality: TableSelection shall implement IEquatable<TableSelection> so ValueChanging/ValueChanged can detect actual changes.


2. Implement IValue<TableSelection?>

TableView shall implement IValue<TableSelection?> with full CWP pattern (ValueChanging/ValueChanged). null represents no selection.

Rationale: Every interactive view in v2 that holds a user-facing value implements IValue<T> so consumers can use ValueChanged/ValueChanging for data-binding and validation.

Design:

  • Value property of type TableSelection (see §1 for type design)
  • RaiseValueChanging (cancellable) and RaiseValueChanged follow the standard CWP workflow
  • OnValueChanged virtual method for subclass override
  • ValueChanging, ValueChanged, ValueChangedUntyped events
  • Selection-modifying operations update Value through the CWP pipeline
  • When Table is set to a new source, Value resets to a default (cursor at (0, 0), no regions)
  • When Table is set to null, Value resets to null (no selection)

POC note: The POC uses IValue<Point?> as a placeholder. The final implementation will use IValue<TableSelection>.


3. Replace OnMouseEvent with MouseBindings

The entire OnMouseEvent override shall be deleted. All mouse behavior shall use declarative MouseBindings.

Rationale: Manual OnMouseEvent overrides bypass the v2 command pipeline. MouseBindings enable:

  • Consistent input handling — keys and mouse go through the same Command dispatch
  • User-configurable bindings — consumers can remap mouse actions
  • Clean separation of "what input happened" from "what to do about it"

Binding map:

Mouse Action Command Behavior
WheeledUp/Down/Left/Right Command.Up/Down/Left/Right Scroll via standard navigation
LeftButtonClicked Command.Activate Move cursor/selection to clicked cell
LeftButtonClicked + Ctrl Command.ToggleExtend Extend/un-extend cell in multi-selection (Ctrl+click)
LeftButtonClicked + Alt Command.ToggleExtend Extend selection
LeftButtonDoubleClicked Command.Accept Accept/open the selected item

ToggleExtend (ICommandContext?) shall handle Command.ToggleExtend. It inspects the context: if ctx.Binding is a MouseBinding, it extracts the hit cell from MouseEvent.Position; otherwise it operates on the cursor cell.


4. Declare DefaultKeyBindings

DefaultKeyBindings shall be populated with all TableView-specific bindings.

Rationale: The current code has an empty DefaultKeyBindings dictionary with bindings hardcoded elsewhere. Using DefaultKeyBindings enables:

  • Compile-time visibility of what keys a view binds
  • User configuration via ConfigurationManager
  • Consistency with how other v2 views declare bindings

Target bindings:

Arrow keys   → Command.Up/Down/Left/Right  (navigation)
Ctrl+P/N     → Command.Up/Down             (Emacs)
Ctrl+V       → Command.PageDown            (Emacs)
Home         → Command.Start
End          → Command.End
PageUp/Down  → Command.PageUp/PageDown
Space        → Command.ToggleExtend
Enter        → Command.Accept

The legacy CellActivationKey property shall be removed — consumers should use KeyBindings.ReplaceCommands or DefaultKeyBindings instead.


5. Remove Legacy Selection API

The following are removed (breaking change, pre-v2.0.0):

  • SelectedColumn / SelectedRow properties
  • SelectedCellChanged / RaiseSelectedCellChanged event
  • CellActivated / OnCellActivated / CellActivatedEventArgs
  • CellToggled / OnCellToggled / CellToggledEventArgs
  • CellActivationKey property

Rationale:

  • The current properties conflate "where the user is navigating" with "what is selected"
  • Parallel notification paths (SelectedCellChanged + ValueChanged) are confusing and error-prone
  • All cursor state is accessible through Value?.Cursor; all selection state through Value?.Regions
  • This is a clean break before v2.0.0 ships — no need for deprecation shims

Migration:

  • SelectedColumn / SelectedRowValue?.Cursor?.X / Value?.Cursor?.Y
  • SelectedCellChangedValueChanged
  • CellActivatedAccepted
  • CellToggled → subscribe to ValueChanged and inspect Value?.Regions
  • CellActivationKeyDefaultKeyBindings / KeyBindings.ReplaceCommands
  • No cursor / no selection → Value is null (not -1 or sentinel values)

6. Move Collection Navigator to OnKeyDownNotHandled

OnKeyDown shall be simplified. The collection-navigator (type-to-search) logic shall move to OnKeyDownNotHandled.

Rationale: The current OnKeyDown does too much:

  1. Checks TableIsNullOrInvisible()
  2. Checks if the key is bound to a command (KeyBindings.TryGet)
  3. Filters out Accept/CellActivationKey
  4. Filters out Ctrl/Alt
  5. Checks CollectionNavigator.Matcher.IsCompatibleKey
  6. Calls CycleToNextTableEntryBeginningWith

Steps 2–5 all try to avoid intercepting keys that should go through the normal command pipeline. This is exactly what OnKeyDownNotHandled is for — it fires only after all key bindings and commands have had a chance to handle the key.

OnKeyDownNotHandled shall also include the standard Alt/Ctrl guard (consistent with the TextField/TextView fixes from issue #4963) so modified keys are never treated as type-to-search input.


7. Thread ICommandContext Through Selection Methods

All selection-modifying methods shall accept an ICommandContext? ctx parameter:

  • SetSelection
  • ChangeSelectionByOffset
  • ChangeSelectionToStartOfRow / ChangeSelectionToEndOfRow
  • ChangeSelectionToStartOfTable / ChangeSelectionToEndOfTable
  • PageUp / PageDown
  • Internal helpers

Rationale: All AddCommand handlers receive an ICommandContext carrying the originating binding (key, mouse, or programmatic). Threading it through enables:

  • Context-aware behavior (e.g., Shift+click extends selection, plain click replaces it)
  • ToggleExtend extracting mouse position from MouseBinding.MouseEvent
  • Tracing/debugging which input caused a selection change

8. Update Consumers (FileDialog, DatePicker, etc.)

All internal consumers of TableView's removed API shall be migrated. This is a breaking change — no compatibility shims.

FileDialog:

  • Subscribe to Accepted instead of CellActivated; read cursor from Value?.Cursor
  • Subscribe to ValueChanged instead of SelectedCellChanged
  • Remove dead #if MENU_V1 code

DatePicker:

  • Subscribe to Activated instead of CellActivated for date selection
  • Forward Accepted to RaiseAccepted for date confirmation

UICatalog TableEditor scenario:

  • Remove KeyBindings.ReplaceCommands(Key.Space, Command.Accept) — Space is Command.ToggleExtend by default
  • Update all event handler subscriptions to new API

9. Test Strategy

Tests shall be written/updated to cover:

  • Command mapping correctness: Command.Accept fires Accepted; Command.Activate fires Activated; Command.ToggleExtend extends/un-extends cell in multi-selection
  • MouseBindings: Each mouse action dispatches the expected Command; ToggleExtend extracts position from ICommandContext
  • IValue<TableSelection?>: Value is null when no selection; ValueChanging is cancellable; ValueChanged fires with correct old/new; Value reflects cursor + regions
  • Keyboard: DefaultKeyBindings map to correct commands; collection navigator only fires in OnKeyDownNotHandled; Alt/Ctrl keys are rejected
  • Consumer migration: FileDialog, DatePicker continue to work with new API
  • Edge cases: No Table assigned (Value is null); empty table; single-cell table; disjoint multi-select

10. Test Gap Analysis

Current state: 28 tests across 3 files. 17 of 22 registered commands have zero direct tests. Mouse handling, IValue, navigation, drawing, and cell mapping are essentially untested. Tests marked 🔴 must be written before further refactoring to establish a behavioral baseline; tests marked 🟡 should be added as part of the redesign work.

🔴 Pre-Refactor: Baseline Tests (write BEFORE making more changes)

These tests lock in current correct behavior so refactoring doesn't introduce silent regressions.

A. Arrow Key Cell Movement (0 tests exist)

  • Right arrow moves cursor from (0,0) to (1,0); wraps or stops at last column
  • Down arrow moves cursor from (0,0) to (0,1); stops at last row
  • Left arrow at (0,x) — boundary behavior
  • Up arrow at (x,0) — boundary behavior
  • Arrow keys update cursor position (currently SelectedColumn/SelectedRow; will become Value?.Cursor)
  • Arrow keys fire selection-changed notification with correct old/new values
  • Arrow keys scroll viewport when cursor reaches edge

B. Page/Home/End Navigation (0 tests exist)

  • PageDown moves cursor by visible-rows count; clamps at table end
  • PageUp moves cursor by visible-rows count; clamps at row 0
  • Home (Command.LeftStart) moves cursor to column 0, same row
  • End (Command.RightEnd) moves cursor to last column, same row
  • Ctrl+Home (Command.Start) moves cursor to (0,0)
  • Ctrl+End (Command.End) moves cursor to last cell
  • All navigation commands fire selection-changed notification

C. Mouse Click → Cell Selection (0 tests exist)

  • Left-click on a cell moves cursor to that cell
  • Left-click fires selection-changed notification
  • Left-click on header row does NOT change selection
  • Double-click fires Accepted
  • Wheel up/down scrolls viewport (changes RowOffset)
  • Wheel left/right scrolls viewport (changes ColumnOffset)

D. Multi-Select Baseline (partially tested — fill gaps)

  • ToggleCurrentCellSelection with MultiSelect = false returns null / is no-op
  • ToggleCurrentCellSelection adds cursor cell to MultiSelectedRegions
  • Toggling same cell twice removes it from MultiSelectedRegions
  • CellToggled.Cancel = true prevents the toggle
  • GetAllSelectedCells returns correct set after multiple toggles
  • IsSelected returns true for toggled cells, false for others
  • SelectAll marks all cells; GetAllSelectedCells returns full grid

E. ScreenToCell Mapping (0 tests exist)

  • ScreenToCell(x, y) returns correct Point for a known layout
  • Returns null for coordinates outside the table
  • Header click detection (out headerIfAny) returns correct column index
  • Correct mapping with non-zero ColumnOffset / RowOffset
  • Correct mapping with hidden columns or varying column widths

F. Edge Cases (mostly missing)

  • Table = nullValue is null; all navigation/selection operations are safe (no crash)
  • Empty table (0 rows) → Value is null, navigation is no-op
  • Single-row, single-column table → boundary checks
  • Setting SelectedColumn/SelectedRow beyond table bounds → clamped
  • ColumnOffset/RowOffset beyond bounds → clamped by EnsureValidScrollOffsets

G. Drawing Baseline (1 test exists)

  • Basic table renders expected characters for headers and data cells
  • FullRowSelect highlights entire row
  • Header visibility flags (ShowHorizontalHeaderOverline, ShowHorizontalHeaderUnderline)
  • NullSymbol renders for null cells
  • SeparatorSymbol renders between columns
  • ExpandLastColumn fills remaining width
  • Row/cell color getters apply expected attributes

🟡 Post-Refactor: New API Tests (write alongside redesign)

H. IValue<TableSelection?> (0 tests exist)

  • Value reflects cursor position after arrow key navigation
  • Value reflects cursor + regions after multi-select operations
  • Value is null when no Table is assigned
  • Value is null when selection is cleared
  • ValueChanging fires before cursor moves; setting Cancel = true prevents the move
  • ValueChanged fires after cursor moves with correct OldValue/NewValue
  • ValueChangedUntyped fires in sync with ValueChanged
  • Setting Value programmatically updates cursor and fires events
  • Setting Table = null resets Value to null
  • Setting Table = newSource resets Value to cursor (0,0), no regions

I. Command.ToggleExtend — Unified Handler (0 tests exist)

  • ToggleExtend via keyboard (Space) → extends/un-extends cursor cell
  • ToggleExtend via Ctrl+click → extends clicked cell (not cursor cell)
  • ToggleExtend via Alt+click → extends selection range to clicked cell
  • ToggleExtend with MultiSelect = false → no-op
  • ToggleExtend fires Extended / OnExtended event
  • ToggleExtend updates Value.Regions

J. Shift+Key Extend Selection (0 tests for any of 10 extend commands)

  • Shift+Right (Command.RightExtend) extends selection region by one column
  • Shift+Down (Command.DownExtend) extends selection region by one row
  • Shift+Home (Command.LeftStartExtend) extends to start of row
  • Shift+End (Command.RightEndExtend) extends to end of row
  • Ctrl+Shift+Home (Command.StartExtend) extends to (0,0)
  • Ctrl+Shift+End (Command.EndExtend) extends to last cell
  • Shift+PageUp/Down extends by page
  • All extend commands update Value.Regions
  • Extend then non-extend collapses selection to cursor

K. OnKeyDownNotHandled / Collection Navigator (2 tests exist — expand)

  • Typing a letter navigates to first matching cell
  • Multi-character search within timeout window
  • Alt+letter is NOT treated as search input (modifier guard)
  • Ctrl+letter is NOT treated as search input (modifier guard)
  • Search wraps from end to beginning
  • CollectionNavigator = null disables search
  • Search with FullRowSelect searches correct column

L. Consumer Integration

  • FileDialog: selecting a file row via Activated updates path display
  • FileDialog: double-click / Enter via Accepted opens file
  • DatePicker: clicking a date via Activated updates selected date
  • DatePicker: Accepted propagates up

Coverage Heat Map (Current → Target)

Area Current Target
Tab-out boundary 🟢 5 tests 🟢 Keep
Accept (Accepted) 🟢 3 tests 🟢 Keep (update to new API)
ToggleExtend 🟡 2 tests 🟢 + cancel path, multi-select off, un-extend
ValueChanged 🔴 0 🟢 Replaces SelectedCellChanged tests
DefaultKeyBindings config 🟢 4 tests 🟢 Keep
Arrow key cell movement 🔴 0 🟢 All 4 directions + viewport scroll
Page/Home/End navigation 🔴 0 🟢 All 6 commands
Shift+extend selection 🔴 0 🟢 All 10 extend commands
Mouse handling 🔴 0 🟢 Click, Ctrl+click, double-click, wheel
IValue<TableSelection?> 🔴 0 🟢 Value, ValueChanging, ValueChanged, null
ToggleExtend unified 🔴 0 🟢 Keyboard + mouse paths
Drawing/rendering 🔴 1 test 🟡 Basic rendering + style flags
ScreenToCell mapping 🔴 0 🟢 All overloads + edge cases
Collection navigator 🟡 2 tests 🟢 + modifier guard, wrap, disable
Edge cases (null/empty) 🔴 1 test 🟢 null, empty, single, bounds
Style/ColumnStyle 🔴 0 🟡 Basic property tests
Supporting source types 🔴 2 tests 🟡 All source types

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    Projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions