fix(tui): add guarded mouse list interactions#281
Merged
peaktwilight merged 1 commit intomainfrom May 1, 2026
Merged
Conversation
📝 WalkthroughWalkthroughAdds mouse support to the TUI findings list with scroll-wheel navigation, click-to-select, and hover tracking. Simultaneously fixes scrolling behavior by persisting Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tui.rs`:
- Around line 412-418: The handler is calling drain_queued_scroll_events(kind)
which consumes and discards the first non-scroll event and collapses a burst of
wheel events into a single move, causing rapid scrolls to be limited and queued
clicks/keys to be lost; change the logic to not consume past the first
non-scroll event—either process every MouseEventKind::ScrollUp/ScrollDown
individually (calling move_selection per scroll event) or implement safe
coalescing by counting consecutive scroll events, buffering the first non-scroll
event returned by drain_queued_scroll_events (do not drop it) and replaying it
into the main event loop after applying the appropriate number of
move_selection(delta) calls; update the code paths around
drain_queued_scroll_events and the match handling in the main loop to ensure
non-scroll events are preserved and replayed rather than discarded.
- Around line 143-146: hover_index is stored as an absolute index into the
filtered list and must be cleared whenever the visible list or ListState
changes; update code paths that mutate scrolling, filtering, sorting or the list
state (e.g., the functions that handle wheel scrolling, filter/sort updates, and
any code that updates list_state or list_area) to set self.hover_index = None
after mutating the list or ListState (and after resizing the list_area) so the
hover background won’t stick to the wrong visible row; locate usages of
hover_index, ListState, selected and list_area and add hover_index invalidation
immediately after those mutations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
143
to
+146
| selected: usize, | ||
| list_state: ListState, | ||
| list_area: Rect, | ||
| hover_index: Option<usize>, |
There was a problem hiding this comment.
Invalidate hover_index when the visible list changes.
hover_index is cached as an absolute filtered-list index, so after wheel scrolling or any filter/sort change the row under the pointer can change without a MouseEventKind::Moved. The hover background can then stick to the wrong visible row until the mouse moves again.
Also applies to: 431-439, 1376-1387
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui.rs` around lines 143 - 146, hover_index is stored as an absolute
index into the filtered list and must be cleared whenever the visible list or
ListState changes; update code paths that mutate scrolling, filtering, sorting
or the list state (e.g., the functions that handle wheel scrolling, filter/sort
updates, and any code that updates list_state or list_area) to set
self.hover_index = None after mutating the list or ListState (and after resizing
the list_area) so the hover background won’t stick to the wrong visible row;
locate usages of hover_index, ListState, selected and list_area and add
hover_index invalidation immediately after those mutations.
Comment on lines
+412
to
+418
| kind @ (MouseEventKind::ScrollUp | MouseEventKind::ScrollDown) => { | ||
| let last_kind = drain_queued_scroll_events(kind); | ||
| match last_kind { | ||
| MouseEventKind::ScrollUp => self.move_selection(-1), | ||
| MouseEventKind::ScrollDown => self.move_selection(1), | ||
| _ => {} | ||
| } |
There was a problem hiding this comment.
Don't drain and collapse the event queue here.
drain_queued_scroll_events() consumes the first queued non-scroll event and drops it on _ => break, and it also compresses an arbitrary wheel burst into a single row move. That means rapid scrolling is capped to one step, and a click/key queued behind the wheel can disappear.
🛠️ Safe fallback
- kind @ (MouseEventKind::ScrollUp | MouseEventKind::ScrollDown) => {
- let last_kind = drain_queued_scroll_events(kind);
- match last_kind {
- MouseEventKind::ScrollUp => self.move_selection(-1),
- MouseEventKind::ScrollDown => self.move_selection(1),
- _ => {}
- }
- }
+ MouseEventKind::ScrollUp => self.move_selection(-1),
+ MouseEventKind::ScrollDown => self.move_selection(1),If you still want coalescing, buffer the first non-scroll event and replay it in the main loop instead of reading past it.
Also applies to: 2922-2934
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tui.rs` around lines 412 - 418, The handler is calling
drain_queued_scroll_events(kind) which consumes and discards the first
non-scroll event and collapses a burst of wheel events into a single move,
causing rapid scrolls to be limited and queued clicks/keys to be lost; change
the logic to not consume past the first non-scroll event—either process every
MouseEventKind::ScrollUp/ScrollDown individually (calling move_selection per
scroll event) or implement safe coalescing by counting consecutive scroll
events, buffering the first non-scroll event returned by
drain_queued_scroll_events (do not drop it) and replaying it into the main event
loop after applying the appropriate number of move_selection(delta) calls;
update the code paths around drain_queued_scroll_events and the match handling
in the main loop to ensure non-scroll events are preserved and replayed rather
than discarded.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ListStateso keyboard and mouse navigation do not pin selection to the viewport edge.Review notes
mainand intentionally excludes already-merged PQ/SARIF/manifest changes.Closes #269
Summary by CodeRabbit