Fix: [BUG] [v1.1.0] Auxiliary sidebar resize handle: no hover highlight after resize if pointer re-entered while dragging#39607
Conversation
… highlight after resize if pointer re-entered while dragging Signed-off-by: willkhinz <hinzwilliam52@gmail.com>
… highlight after resize if pointer re-entered while dragging Signed-off-by: willkhinz <hinzwilliam52@gmail.com>
… highlight after resize if pointer re-entered while dragging Signed-off-by: willkhinz <hinzwilliam52@gmail.com>
📝 WalkthroughWalkthroughThe changes add mouse event capture and handling to the TUI system, enabling resize state tracking across leaderboard and stats components. Mouse events are captured during terminal operation, the resizing state is maintained, and visual feedback is displayed when resize interactions occur. Changes
Sequence Diagram(s)sequenceDiagram
participant Terminal
participant EventHandler as Event Handler<br/>(handle_events)
participant AppState as App State<br/>(is_resizing)
participant UIComponent as UI Component<br/>(render)
Terminal->>EventHandler: Mouse Down Event
EventHandler->>AppState: Set is_resizing = true
AppState->>UIComponent: Trigger redraw with is_resizing=true
UIComponent->>UIComponent: Render "Resizing..." indicator
Terminal->>EventHandler: Mouse Up Event
EventHandler->>AppState: Set is_resizing = false
AppState->>UIComponent: Trigger redraw with is_resizing=false
UIComponent->>UIComponent: Render normal UI (no indicator)
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
bins/bounty-cli/src/tui/stats.rs (1)
121-126: Resize indicator completely overwrites help text.The "Resizing..." paragraph is rendered to the same area (
outer[2]) as the help text rendered at line 119, completely replacing it. If this is intentional, consider combining the messages or using a different indicator (e.g., border color change) so users don't lose the quit instructions during resize.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bins/bounty-cli/src/tui/stats.rs` around lines 121 - 126, The resize indicator currently renders the "Resizing..." Paragraph (resize_text) into the same area as the help text (frame.render_widget(resize_text, outer[2])), which completely overwrites the quit/help instructions; change the behavior so the indicator doesn't replace the help text — either render the Paragraph into a different rectangle (not outer[2]) or draw it as a smaller overlay/centered popup, or instead modify the existing help widget's style (e.g., change border color) when is_resizing is true; update the code around is_resizing, resize_text, and the frame.render_widget call to implement one of these non-destructive approaches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bins/bounty-cli/src/tui/leaderboard.rs`:
- Around line 126-130: The conditional in the call to row_highlight_style always
returns Color::DarkGray (if app.is_resizing { Color::DarkGray } else {
Color::DarkGray }), so remove the redundant branch or provide a different color
for the non-resizing branch; update the row_highlight_style invocation that uses
Style::default() and app.is_resizing to either pass Color::DarkGray directly or
choose a distinct color (e.g., Color::Gray or Color::Black) for the else branch
so the resizing flag actually changes the highlight color.
- Around line 193-195: The function calls super::setup_terminal() (which enables
raw mode/alternate screen) but never calls super::restore_terminal(&mut
terminal) on exit; add a call to super::restore_terminal(&mut terminal)? just
before returning Ok(()) so the terminal is restored (match the pattern used in
stats.rs), ensuring terminal is cleaned up even when the event loop exits or on
early returns.
- Around line 183-191: The code currently calls event::read() twice which
consumes events and can block; reuse the already-read event instead of calling
event::read() again. Change the second branch to match on the previously
obtained event value (the Event variable from the earlier event::read() call)
and handle Event::Mouse by updating app.is_resizing and app.last_mouse_position
accordingly (references: Event::Mouse, mouse.kind, app.is_resizing,
app.last_mouse_position) so mouse events are processed from the same event
object and no extra event::read() is performed.
In `@bins/bounty-cli/src/tui/mod.rs`:
- Around line 30-48: The handle_events function uses non-existent MouseEventKind
variants (Pressed/Released), blocks indefinitely with event::read(), never uses
the terminal param, and doesn't match the non-blocking pattern used by stats.rs
and leaderboard.rs; fix by updating pattern matches to
MouseEventKind::Down(MouseButton) and MouseEventKind::Up(MouseButton) in
handle_events, replace event::read() with the non-blocking
event::poll(Duration::from_millis(100))? + event::read() only when poll returns
true, add a clear exit/break condition consistent with stats.rs/leaderboard.rs
(e.g., checking for a Quit key or returning on a specific event), and either use
or remove the terminal parameter in handle_events to avoid unused-parameter
warnings; ensure is_resizing is set on Down/Up events and integrate this
function into the same non-blocking event loop architecture used by stats.rs and
leaderboard.rs.
In `@bins/bounty-cli/src/tui/stats.rs`:
- Around line 157-158: The code uses MouseEventKind but doesn't import it;
update the imports to either add MouseEventKind to the existing import list (so
MouseEventKind is available unqualified) or qualify its usage as
event::MouseEventKind where Event::Mouse is handled (in the block containing
Event::Mouse(event) and checks like event.kind == MouseEventKind::Down) so the
symbol resolves; ensure the change is applied near the top imports that
currently bring in event, Event, KeyCode, and KeyEventKind.
- Around line 157-164: The code calls event::read() twice which consumes the
first event and can block; instead, read the event once into a single variable
and then match on that variable (avoid calling event::read() again). Modify the
logic around Event::Mouse and the earlier key-handling branch so you use the
same event value (the variable returned from the single event::read() call) to
check MouseEventKind::Down and ::Up, toggle is_resizing, and call
terminal.draw(|f| ui(f, &stats, &error, is_resizing)) when needed; update any
pattern matches using Event::Mouse to reference the existing event variable
rather than calling event::read() again.
---
Nitpick comments:
In `@bins/bounty-cli/src/tui/stats.rs`:
- Around line 121-126: The resize indicator currently renders the "Resizing..."
Paragraph (resize_text) into the same area as the help text
(frame.render_widget(resize_text, outer[2])), which completely overwrites the
quit/help instructions; change the behavior so the indicator doesn't replace the
help text — either render the Paragraph into a different rectangle (not
outer[2]) or draw it as a smaller overlay/centered popup, or instead modify the
existing help widget's style (e.g., change border color) when is_resizing is
true; update the code around is_resizing, resize_text, and the
frame.render_widget call to implement one of these non-destructive approaches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e00a941e-3416-443d-8330-294c0f33a538
📒 Files selected for processing (3)
bins/bounty-cli/src/tui/leaderboard.rsbins/bounty-cli/src/tui/mod.rsbins/bounty-cli/src/tui/stats.rs
| .row_highlight_style(Style::default().bg(if app.is_resizing { | ||
| Color::DarkGray | ||
| } else { | ||
| Color::DarkGray | ||
| })); |
There was a problem hiding this comment.
Conditional always returns the same value.
Both branches of the if app.is_resizing conditional return Color::DarkGray, making the conditional meaningless. This appears to be incomplete implementation or copy-paste error.
🔧 Suggested fix - either remove the conditional or differentiate colors
.row_highlight_style(Style::default().bg(if app.is_resizing {
- Color::DarkGray
+ Color::Gray // or another distinct color during resize
} else {
Color::DarkGray
}));Or simply:
- .row_highlight_style(Style::default().bg(if app.is_resizing {
- Color::DarkGray
- } else {
- Color::DarkGray
- }));
+ .row_highlight_style(Style::default().bg(Color::DarkGray));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .row_highlight_style(Style::default().bg(if app.is_resizing { | |
| Color::DarkGray | |
| } else { | |
| Color::DarkGray | |
| })); | |
| .row_highlight_style(Style::default().bg(if app.is_resizing { | |
| Color::Gray // or another distinct color during resize | |
| } else { | |
| Color::DarkGray | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/leaderboard.rs` around lines 126 - 130, The
conditional in the call to row_highlight_style always returns Color::DarkGray
(if app.is_resizing { Color::DarkGray } else { Color::DarkGray }), so remove the
redundant branch or provide a different color for the non-resizing branch;
update the row_highlight_style invocation that uses Style::default() and
app.is_resizing to either pass Color::DarkGray directly or choose a distinct
color (e.g., Color::Gray or Color::Black) for the else branch so the resizing
flag actually changes the highlight color.
| } else if let Event::Mouse(mouse) = event::read()? { | ||
| if mouse.kind == event::MouseEventKind::Down { | ||
| app.is_resizing = true; | ||
| } else if mouse.kind == event::MouseEventKind::Up { | ||
| app.is_resizing = false; | ||
| } else if mouse.kind == event::MouseEventKind::Moved { | ||
| app.last_mouse_position = (mouse.column, mouse.row); | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Double event::read() causes event consumption bug and potential blocking.
Line 168 calls event::read() which consumes the event. If that event is NOT a Key event, the else if at line 183 calls event::read() again, which blocks waiting for a completely different event. This means mouse events will be lost or cause the application to hang.
The pattern should match on the already-read event, not read again:
🐛 Proposed fix
if event::poll(Duration::from_millis(100))? {
- if let Event::Key(key) = event::read()? {
+ match event::read()? {
+ Event::Key(key) => {
if key.kind == KeyEventKind::Press {
match key.code {
KeyCode::Char('q') | KeyCode::Esc => break,
KeyCode::Up | KeyCode::Char('k') => {
app.scroll_offset = app.scroll_offset.saturating_sub(1);
}
KeyCode::Down | KeyCode::Char('j') => {
if app.scroll_offset + 1 < app.entries.len() {
app.scroll_offset += 1;
}
}
_ => {}
}
}
- } else if let Event::Mouse(mouse) = event::read()? {
+ }
+ Event::Mouse(mouse) => {
if mouse.kind == event::MouseEventKind::Down {
app.is_resizing = true;
} else if mouse.kind == event::MouseEventKind::Up {
app.is_resizing = false;
} else if mouse.kind == event::MouseEventKind::Moved {
app.last_mouse_position = (mouse.column, mouse.row);
}
+ }
+ _ => {}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/leaderboard.rs` around lines 183 - 191, The code
currently calls event::read() twice which consumes events and can block; reuse
the already-read event instead of calling event::read() again. Change the second
branch to match on the previously obtained event value (the Event variable from
the earlier event::read() call) and handle Event::Mouse by updating
app.is_resizing and app.last_mouse_position accordingly (references:
Event::Mouse, mouse.kind, app.is_resizing, app.last_mouse_position) so mouse
events are processed from the same event object and no extra event::read() is
performed.
| } | ||
|
|
||
| super::restore_terminal(&mut terminal)?; | ||
| Ok(()) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Critical: Missing restore_terminal() leaves terminal in corrupted state.
The function calls super::setup_terminal() at line 141, which enables raw mode, enters alternate screen, and enables mouse capture. However, when the loop exits, there's no call to super::restore_terminal(&mut terminal)? before returning. This will leave the user's terminal in raw mode with the alternate screen still active.
Compare with stats.rs which correctly calls super::restore_terminal(&mut terminal)? at line 168.
🐛 Proposed fix
}
}
+ super::restore_terminal(&mut terminal)?;
Ok(())
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } | |
| super::restore_terminal(&mut terminal)?; | |
| Ok(()) | |
| } | |
| } | |
| } | |
| super::restore_terminal(&mut terminal)?; | |
| Ok(()) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/leaderboard.rs` around lines 193 - 195, The function
calls super::setup_terminal() (which enables raw mode/alternate screen) but
never calls super::restore_terminal(&mut terminal) on exit; add a call to
super::restore_terminal(&mut terminal)? just before returning Ok(()) so the
terminal is restored (match the pattern used in stats.rs), ensuring terminal is
cleaned up even when the event loop exits or on early returns.
| pub fn handle_events(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>, is_resizing: &mut bool) -> Result<()> { | ||
| loop { | ||
| if let event::Event::Mouse(event) = event::read()? { | ||
| match event.kind { | ||
| event::MouseEventKind::Moved => { | ||
| // Handle mouse move event | ||
| } | ||
| event::MouseEventKind::Pressed(_) => { | ||
| *is_resizing = true; | ||
| // Handle mouse press event | ||
| } | ||
| event::MouseEventKind::Released(_) => { | ||
| *is_resizing = false; | ||
| // Handle mouse release event | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
crossterm 0.28 MouseEventKind enum variants
💡 Result:
The MouseEventKind enum in crossterm 0.28 has the following variants: pub enum MouseEventKind { Down(MouseButton), Up(MouseButton), Drag(MouseButton), Moved, ScrollDown, ScrollUp, ScrollLeft, ScrollRight, }
Citations:
- 1: https://docs.rs/crossterm/latest/crossterm/event/enum.MouseEventKind.html
- 2: https://docs.rs/crossterm/latest/src/crossterm/event.rs.html
- 3: https://github.com/crossterm-rs/crossterm/blob/master/CHANGELOG.md
- 4: https://crates.io/crates/crossterm/0.28.0
🏁 Script executed:
cat bins/bounty-cli/src/tui/mod.rsRepository: PlatformNetwork/bounty-challenge
Length of output: 1652
🏁 Script executed:
rg "handle_events" --type rsRepository: PlatformNetwork/bounty-challenge
Length of output: 102
🏁 Script executed:
rg "handle_events"Repository: PlatformNetwork/bounty-challenge
Length of output: 222
🏁 Script executed:
cat bins/bounty-cli/src/tui/stats.rs | head -200Repository: PlatformNetwork/bounty-challenge
Length of output: 5271
🏁 Script executed:
cat bins/bounty-cli/src/tui/leaderboard.rs | head -200Repository: PlatformNetwork/bounty-challenge
Length of output: 6365
Critical: MouseEventKind variants don't exist in crossterm 0.28 and will cause compilation failure.
This function has multiple critical issues:
-
Non-existent enum variants:
MouseEventKind::Pressed(_)andMouseEventKind::Released(_)do not exist in crossterm 0.28. The correct variants areMouseEventKind::Down(MouseButton)andMouseEventKind::Up(MouseButton). This will cause immediate compilation errors. -
Infinite blocking loop: The loop has no break condition and
event::read()blocks indefinitely, so the function never returns. -
Unused parameter:
terminalis never used. -
Dead code and architectural mismatch: This function is never called. Additionally, both
stats.rsandleaderboard.rsuse the non-blocking pattern withevent::poll(Duration::from_millis(100))?and proper exit conditions. This blocking approach is incompatible with that architecture.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/mod.rs` around lines 30 - 48, The handle_events
function uses non-existent MouseEventKind variants (Pressed/Released), blocks
indefinitely with event::read(), never uses the terminal param, and doesn't
match the non-blocking pattern used by stats.rs and leaderboard.rs; fix by
updating pattern matches to MouseEventKind::Down(MouseButton) and
MouseEventKind::Up(MouseButton) in handle_events, replace event::read() with the
non-blocking event::poll(Duration::from_millis(100))? + event::read() only when
poll returns true, add a clear exit/break condition consistent with
stats.rs/leaderboard.rs (e.g., checking for a Quit key or returning on a
specific event), and either use or remove the terminal parameter in
handle_events to avoid unused-parameter warnings; ensure is_resizing is set on
Down/Up events and integrate this function into the same non-blocking event loop
architecture used by stats.rs and leaderboard.rs.
| } else if let Event::Mouse(event) = event::read()? { | ||
| if event.kind == MouseEventKind::Down { |
There was a problem hiding this comment.
Missing import for MouseEventKind.
MouseEventKind is used directly at lines 158 and 160-161, but it's not imported. The current import at line 2 only brings in event (the module), Event, KeyCode, and KeyEventKind. Either qualify as event::MouseEventKind (which would be consistent with leaderboard.rs) or add it to the import.
🔧 Suggested fix - add to imports
-use crossterm::event::{self, Event, KeyCode, KeyEventKind};
+use crossterm::event::{self, Event, KeyCode, KeyEventKind, MouseEventKind};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } else if let Event::Mouse(event) = event::read()? { | |
| if event.kind == MouseEventKind::Down { | |
| use crossterm::event::{self, Event, KeyCode, KeyEventKind, MouseEventKind}; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/stats.rs` around lines 157 - 158, The code uses
MouseEventKind but doesn't import it; update the imports to either add
MouseEventKind to the existing import list (so MouseEventKind is available
unqualified) or qualify its usage as event::MouseEventKind where Event::Mouse is
handled (in the block containing Event::Mouse(event) and checks like event.kind
== MouseEventKind::Down) so the symbol resolves; ensure the change is applied
near the top imports that currently bring in event, Event, KeyCode, and
KeyEventKind.
| } else if let Event::Mouse(event) = event::read()? { | ||
| if event.kind == MouseEventKind::Down { | ||
| is_resizing = true; | ||
| } else if event.kind == MouseEventKind::Up { | ||
| is_resizing = false; | ||
| terminal.draw(|f| ui(f, &stats, &error, is_resizing))?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Critical: Double event::read() causes event consumption bug and potential blocking.
Same issue as in leaderboard.rs: line 151 calls event::read() and consumes the event. If it's not a Key event, the else if at line 157 calls event::read() again, blocking for a new event instead of handling the one already read.
🐛 Proposed fix
if event::poll(Duration::from_millis(100))? {
- if let Event::Key(key) = event::read()? {
+ match event::read()? {
+ Event::Key(key) => {
if key.kind == KeyEventKind::Press
&& matches!(key.code, KeyCode::Char('q') | KeyCode::Esc)
{
break;
}
- } else if let Event::Mouse(event) = event::read()? {
+ }
+ Event::Mouse(event) => {
if event.kind == MouseEventKind::Down {
is_resizing = true;
} else if event.kind == MouseEventKind::Up {
is_resizing = false;
terminal.draw(|f| ui(f, &stats, &error, is_resizing))?;
}
+ }
+ _ => {}
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@bins/bounty-cli/src/tui/stats.rs` around lines 157 - 164, The code calls
event::read() twice which consumes the first event and can block; instead, read
the event once into a single variable and then match on that variable (avoid
calling event::read() again). Modify the logic around Event::Mouse and the
earlier key-handling branch so you use the same event value (the variable
returned from the single event::read() call) to check MouseEventKind::Down and
::Up, toggle is_resizing, and call terminal.draw(|f| ui(f, &stats, &error,
is_resizing)) when needed; update any pattern matches using Event::Mouse to
reference the existing event variable rather than calling event::read() again.
1 participant
Description
Fix for the auxiliary sidebar resize handle not displaying a hover highlight when the pointer re-enters the drag area while resizing.
Related Issue
Fixes #https://github.com/PlatformNetwork/bounty-challenge
Type of Change
Checklist
Testing
To verify the fix, the following commands were run:
cargo test cargo clippyAdditionally, manual testing was performed to ensure the hover highlight appears when the pointer re-enters the drag area while resizing.
Screenshots (if applicable)
No screenshots are necessary for this change, as it is a functional fix rather than a visual update.
🔍 Analysis
The issue arises from the auxiliary sidebar resize handle not displaying a hover highlight when the pointer re-enters the drag area while resizing. This suggests a problem with the event handling or UI update logic.
🛠️ Implementation
To address this issue, the code has been updated to properly handle mouse events and UI updates during the resizing process. The changes include:
anyhow::Resultfor improved error handlingcrosstermandratatuifor enhanced terminal event handling and UI renderingLeaderboardEntryandAppstructs to manage leaderboard data and application state✅ Verification
To verify the fix, the following steps were taken:
Summary by CodeRabbit