Skip to content

Fixes #5338. ListView.MoveUp/MoveDown cycles when TabStop == NoStop#5339

Merged
tig merged 3 commits into
developfrom
copilot/add-movement-cycling-tests
May 18, 2026
Merged

Fixes #5338. ListView.MoveUp/MoveDown cycles when TabStop == NoStop#5339
tig merged 3 commits into
developfrom
copilot/add-movement-cycling-tests

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

ListView.MoveDown/MoveUp now wrap around to the opposite end of the list when TabStop == TabBehavior.NoStop, and return false at boundaries when TabStop is anything else.

Proposed Changes/Todos

  • MoveDown (ListView.Movement.cs): added Viewport.Height > 0 guard on the scroll-viewport early-return branch (it was spuriously returning true when no frame was set); added TabStop == NoStop branch that selects item 0 instead of returning false
  • MoveUp (ListView.Movement.cs): added matching TabStop == NoStop branch that selects the last item instead of returning false
  • Added <remarks> XML doc to both MoveDown and MoveUp documenting the wrap-around behavior when TabStop == NoStop and the false-return behavior otherwise
  • Added 4 new tests from the issue spec (ListViewTests.cs)
  • Updated 4 existing Assert.True(lv.NewKeyDownEvent(Key.Space.WithShift)) assertions to Assert.False at list bottom — they previously passed only because the scroll-guard bug returned true; both Command.Activate (plain-view design, Default View KeyBindings blocking HotKeyBinding #4759) and Command.Down now correctly return false there, while marking side-effects are unaffected

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

Copilot AI changed the title [WIP] Add tests for ListView movement cycling with NoStop behavior Fixes #4891. ListView.MoveUp/MoveDown cycles when TabStop == NoStop May 18, 2026
Copilot AI requested a review from tig May 18, 2026 17:14
Copilot finished work on behalf of tig May 18, 2026 17:14
@tig tig changed the title Fixes #4891. ListView.MoveUp/MoveDown cycles when TabStop == NoStop Fixes #5338. ListView.MoveUp/MoveDown cycles when TabStop == NoStop May 18, 2026
@tig tig marked this pull request as ready for review May 18, 2026 17:26
@tig tig requested review from BDisp and Copilot May 18, 2026 17:26
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

Fixes a ListView movement bug and adds wrap-around behavior gated by TabStop. When TabStop == TabBehavior.NoStop, MoveDown at the bottom wraps to the first item and MoveUp at the top wraps to the last; otherwise the methods now correctly return false at the boundary. Also fixes a latent bug where MoveDown returned true from the "scroll viewport" branch when no frame had been set (Viewport.Height == 0).

Changes:

  • Add Viewport.Height > 0 guard to MoveDown's scroll-viewport branch so it no longer spuriously returns true with an unset frame.
  • Add TabStop == NoStop wrap-around branches in both MoveDown (→ item 0) and MoveUp (→ last item).
  • Add 4 new tests for the wrap/return-false behavior and update 4 existing assertions that previously masked the scroll-guard bug.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
Terminal.Gui/Views/ListView/ListView.Movement.cs Adds viewport-height guard and NoStop wrap-around branches in MoveDown/MoveUp.
Tests/UnitTestsParallelizable/Views/ListViewTests.cs Adds wrap/boundary tests; corrects 4 prior assertions that depended on the buggy true return.

Comment thread Terminal.Gui/Views/ListView/ListView.Movement.cs
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.

ListView.MoveUp/MoveDown should cycle if TabStop == TabBehavior.NoStop

3 participants