Skip to content

list: restore a valid keyboard cursor after search - #6

Draft
grishy wants to merge 2 commits into
sg-review-basefrom
sg-list-search-selection
Draft

list: restore a valid keyboard cursor after search#6
grishy wants to merge 2 commits into
sg-review-basefrom
sg-list-search-selection

Conversation

@grishy

@grishy grishy commented Sep 5, 2026

Copy link
Copy Markdown
Owner

I made this change while developing my application and am now contributing it upstream.

This patch restores a valid keyboard cursor after list search results change, without changing a Select's committed value.

The current code chooses a cursor from the previous render's row cache before asynchronous results arrive. That can leave the cursor missing or pointing into an empty first section.

The list now checks the delegate's current item counts at search start and completion. An optional preferred_selected_index can restore a specific item; an absent or invalid preference falls back to the first row in the first non-empty section. Empty results clear the cursor. Stale scroll requests are discarded.

The searchable-list adapter finds the committed item by value, so filtering can move it to a different row. Existing delegates need no changes. This does not commit a selection or reset the query when a popup reopens.

How to Test

The focused tests cover asynchronous and synchronous results, invalid preferences, empty sections and results, and the deferred scroll target:

cargo test -p gpui-component --lib search_cursor_tests --locked

The cursor regression failed before the fix and passes with it. The Select integration test starts with Go committed:

Query Cursor row Committed value
Go 0 Go
Rust 0 Go
Empty 1 Go
Tests and code checks
cargo test -p gpui-base -p gpui-component --lib --locked
cargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets --locked -- -D warnings

771 base and 420 component tests passed. Changed Rust files also passed formatting checks, and the contribution diff passed the whitespace check.

Native checks in MTL_HUD_ENABLED=1 ./script/run-story-macos List confirmed that typing a query selects a real row after the empty first section, Down moves to the next row, and clearing restores the first row. The demo retains old results until PR #8; empty and asynchronous replacement are covered by the automated tests, not this demo.

Windows and Linux runtime behavior has not been tested.

Checklist

  • Read CONTRIBUTING.md.
  • Human-reviewed the patch, including AI-assisted changes.
  • Manually tested the patch as a human reviewer.
  • Windows and Linux runtime validation.

AI assistance

OpenAI’s gpt-6-astra helped isolate the change and prepared the patch, tests, and PR description. The reported checks were run by the agent.

Search can replace results after the previous render cached its row counts.
Validate the preferred cursor against current results at search start and
completion, falling back to the first nonempty section. Resolve the preferred
item by its committed value without committing the search cursor.
@grishy grishy changed the title list: restore valid selection after search completes list: restore a valid keyboard cursor after search Sep 5, 2026
grishy added a commit that referenced this pull request Sep 6, 2026
Validate the preferred cursor against current results after synchronous and asynchronous search. Clear stale deferred scrolling before scheduling the selected row. Keep committed Select values separate from the keyboard cursor.

Includes the reviewed fork PR #6 implementation, regression tests and API docs.
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.

1 participant