-
Notifications
You must be signed in to change notification settings - Fork 32
Fix: [BUG] [v1.1.0] Auxiliary activity bar: border side frozen when position changes at runtime #39613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix: [BUG] [v1.1.0] Auxiliary activity bar: border side frozen when position changes at runtime #39613
Changes from all commits
92bb359
436d263
ad95e3c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -25,3 +25,54 @@ pub fn restore_terminal(terminal: &mut Terminal<CrosstermBackend<io::Stdout>>) - | |||||||||||||
| terminal.show_cursor()?; | ||||||||||||||
| Ok(()) | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub struct AuxiliaryActivityBar { | ||||||||||||||
| position: String, | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| impl AuxiliaryActivityBar { | ||||||||||||||
| pub fn new(position: String) -> Self { | ||||||||||||||
| Self { position } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub fn update_position(&mut self, position: String) { | ||||||||||||||
| self.position = position; | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub fn get_activity_bar_style(&self) -> Style { | ||||||||||||||
| let mut style = Style::default(); | ||||||||||||||
| if self.position == "right" { | ||||||||||||||
| style = style.fg(Color::White).bg(Color::Black).add_modifier(Modifier::BOLD); | ||||||||||||||
| } else if self.position == "left" { | ||||||||||||||
| style = style.fg(Color::White).bg(Color::Black).add_modifier(Modifier::BOLD); | ||||||||||||||
| } | ||||||||||||||
| style | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| pub fn draw_auxiliary_activity_bar( | ||||||||||||||
| f: &mut Frame<CrosstermBackend<io::Stdout>>, | ||||||||||||||
| activity_bar: &mut AuxiliaryActivityBar, | ||||||||||||||
| ) -> Result<()> { | ||||||||||||||
| let chunks = Layout::default() | ||||||||||||||
| .constraints([Constraint::Percentage(100)].as_ref()) | ||||||||||||||
| .split(f.size()); | ||||||||||||||
|
Comment on lines
+57
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use In Ratatui 0.29.0, 🔧 Proposed fix let chunks = Layout::default()
.constraints([Constraint::Percentage(100)].as_ref())
- .split(f.size());
+ .split(f.area());📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| let activity_bar_style = activity_bar.get_activity_bar_style(); | ||||||||||||||
| let activity_bar_text = if activity_bar.position == "right" { | ||||||||||||||
| "Right" | ||||||||||||||
| } else { | ||||||||||||||
| "Left" | ||||||||||||||
| }; | ||||||||||||||
| let border_style = if activity_bar.position == "right" { | ||||||||||||||
| Borders::RIGHT | ||||||||||||||
| } else { | ||||||||||||||
| Borders::LEFT | ||||||||||||||
| }; | ||||||||||||||
| f.render_widget( | ||||||||||||||
| Paragraph::new(activity_bar_text) | ||||||||||||||
| .style(activity_bar_style) | ||||||||||||||
| .block(Block::default().borders(border_style)), | ||||||||||||||
| chunks[0], | ||||||||||||||
| )?; | ||||||||||||||
| Ok(()) | ||||||||||||||
|
Comment on lines
+71
to
+77
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Compilation error:
🐛 Proposed fix f.render_widget(
Paragraph::new(activity_bar_text)
.style(activity_bar_style)
.block(Block::default().borders(border_style)),
chunks[0],
- )?;
+ );
Ok(())
}🤖 Prompt for AI Agents |
||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -65,7 +65,7 @@ fn stat_block<'a>(label: &'a str, value: u64, color: Color) -> Paragraph<'a> { | |||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| fn ui(frame: &mut Frame, stats: &StatsData, error: &Option<String>) { | ||||||||||||||||||||||||||||||||||
| fn ui(frame: &mut Frame, stats: &StatsData, error: &Option<String>, position: &str) { | ||||||||||||||||||||||||||||||||||
| let outer = Layout::default() | ||||||||||||||||||||||||||||||||||
| .direction(Direction::Vertical) | ||||||||||||||||||||||||||||||||||
| .constraints([ | ||||||||||||||||||||||||||||||||||
|
|
@@ -117,13 +117,28 @@ fn ui(frame: &mut Frame, stats: &StatsData, error: &Option<String>) { | |||||||||||||||||||||||||||||||||
| .style(Style::default().fg(Color::DarkGray)) | ||||||||||||||||||||||||||||||||||
| .block(Block::default().borders(Borders::ALL)); | ||||||||||||||||||||||||||||||||||
| frame.render_widget(help, outer[2]); | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Update the border based on the position | ||||||||||||||||||||||||||||||||||
| let border_style = Style::default().fg(Color::Cyan); | ||||||||||||||||||||||||||||||||||
| let borders = match position { | ||||||||||||||||||||||||||||||||||
| "left" => Borders::LEFT, | ||||||||||||||||||||||||||||||||||
| "right" => Borders::RIGHT, | ||||||||||||||||||||||||||||||||||
| _ => Borders::NONE, | ||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||
| frame.render_widget( | ||||||||||||||||||||||||||||||||||
| Block::default() | ||||||||||||||||||||||||||||||||||
| .borders(borders) | ||||||||||||||||||||||||||||||||||
| .border_style(border_style), | ||||||||||||||||||||||||||||||||||
| frame.area(), | ||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+120
to
134
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Border overlay renders on top of all content, potentially obscuring widgets. The border Consider rendering the border first (before other widgets) or rendering it in a dedicated side region rather than the full frame. 🔧 Suggested approach: Render border firstMove the border rendering to the beginning of fn ui(frame: &mut Frame, stats: &StatsData, error: &Option<String>, position: &str) {
+ // Render position-based border first (background layer)
+ let border_style = Style::default().fg(Color::Cyan);
+ let borders = match position {
+ "left" => Borders::LEFT,
+ "right" => Borders::RIGHT,
+ _ => Borders::NONE,
+ };
+ frame.render_widget(
+ Block::default()
+ .borders(borders)
+ .border_style(border_style),
+ frame.area(),
+ );
+
let outer = Layout::default()
.direction(Direction::Vertical)
// ... rest of function
-
- // Update the border based on the position
- let border_style = Style::default().fg(Color::Cyan);
- let borders = match position {
- "left" => Borders::LEFT,
- "right" => Borders::RIGHT,
- _ => Borders::NONE,
- };
- frame.render_widget(
- Block::default()
- .borders(borders)
- .border_style(border_style),
- frame.area(),
- );
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| pub async fn run(rpc_url: &str) -> Result<()> { | ||||||||||||||||||||||||||||||||||
| let mut terminal = super::setup_terminal()?; | ||||||||||||||||||||||||||||||||||
| let mut stats = StatsData::default(); | ||||||||||||||||||||||||||||||||||
| let mut error: Option<String> = None; | ||||||||||||||||||||||||||||||||||
| let mut last_fetch = Instant::now() - Duration::from_secs(10); | ||||||||||||||||||||||||||||||||||
| let mut position = "left"; // default position | ||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent default position across modules. This module defaults to 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| loop { | ||||||||||||||||||||||||||||||||||
| if last_fetch.elapsed() >= Duration::from_secs(5) { | ||||||||||||||||||||||||||||||||||
|
|
@@ -137,19 +152,25 @@ pub async fn run(rpc_url: &str) -> Result<()> { | |||||||||||||||||||||||||||||||||
| last_fetch = Instant::now(); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| terminal.draw(|f| ui(f, &stats, &error))?; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Check for position updates | ||||||||||||||||||||||||||||||||||
| if event::poll(Duration::from_millis(100))? { | ||||||||||||||||||||||||||||||||||
| if let Event::Key(key) = event::read()? { | ||||||||||||||||||||||||||||||||||
| if key.kind == KeyEventKind::Press | ||||||||||||||||||||||||||||||||||
| if key.kind == KeyEventKind::Press && matches!(key.code, KeyCode::Char('l')) { | ||||||||||||||||||||||||||||||||||
| position = "left"; | ||||||||||||||||||||||||||||||||||
| } else if key.kind == KeyEventKind::Press && matches!(key.code, KeyCode::Char('r')) { | ||||||||||||||||||||||||||||||||||
| position = "right"; | ||||||||||||||||||||||||||||||||||
| } else if key.kind == KeyEventKind::Press | ||||||||||||||||||||||||||||||||||
| && matches!(key.code, KeyCode::Char('q') | KeyCode::Esc) | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| terminal.draw(|f| ui(f, &stats, &error, &position))?; | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| super::restore_terminal(&mut terminal)?; | ||||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: Missing
restore_terminalcall leaves terminal in corrupted state.When the user exits the leaderboard (via
qorEsc), the function returns without restoring the terminal. This leaves raw mode enabled and the alternate screen active, corrupting the user's terminal session.The
stats.rsmodule correctly callssuper::restore_terminal(&mut terminal)?before returning (see line 174 in stats.rs). This file should follow the same pattern.🐛 Proposed fix to restore terminal before returning
} + super::restore_terminal(&mut terminal)?; Ok(()) }📝 Committable suggestion
🤖 Prompt for AI Agents