fix(provider): keep picker selection visible (#2241)
fix(provider): keep picker selection visible
This commit is contained in:
@@ -21,7 +21,7 @@ use crossterm::event::{KeyCode, KeyEvent, KeyModifiers};
|
||||
use ratatui::{
|
||||
buffer::Buffer,
|
||||
layout::{Constraint, Direction, Layout, Rect},
|
||||
style::{Modifier, Style},
|
||||
style::{Color, Modifier, Style},
|
||||
text::{Line, Span},
|
||||
widgets::{Block, Borders, Clear, Paragraph, Widget},
|
||||
};
|
||||
@@ -65,13 +65,23 @@ impl ProviderPickerView {
|
||||
}
|
||||
|
||||
fn move_up(&mut self) {
|
||||
if self.selected_idx > 0 {
|
||||
if self.providers.is_empty() {
|
||||
return;
|
||||
}
|
||||
if self.selected_idx == 0 {
|
||||
self.selected_idx = self.providers.len() - 1;
|
||||
} else {
|
||||
self.selected_idx -= 1;
|
||||
}
|
||||
}
|
||||
|
||||
fn move_down(&mut self) {
|
||||
if self.selected_idx + 1 < self.providers.len() {
|
||||
if self.providers.is_empty() {
|
||||
return;
|
||||
}
|
||||
if self.selected_idx + 1 == self.providers.len() {
|
||||
self.selected_idx = 0;
|
||||
} else {
|
||||
self.selected_idx += 1;
|
||||
}
|
||||
}
|
||||
@@ -116,6 +126,28 @@ impl ProviderPickerView {
|
||||
}
|
||||
}
|
||||
|
||||
fn visible_start(&self, visible_rows: usize) -> usize {
|
||||
if visible_rows == 0 {
|
||||
return 0;
|
||||
}
|
||||
let max_start = self.providers.len().saturating_sub(visible_rows);
|
||||
self.selected_idx
|
||||
.saturating_add(1)
|
||||
.saturating_sub(visible_rows)
|
||||
.min(max_start)
|
||||
}
|
||||
|
||||
fn selected_row_style(fg: Color) -> Style {
|
||||
Style::default()
|
||||
.fg(fg)
|
||||
.bg(palette::SURFACE_ELEVATED)
|
||||
.add_modifier(Modifier::BOLD)
|
||||
}
|
||||
|
||||
fn selected_row_bg_style() -> Style {
|
||||
Style::default().bg(palette::SURFACE_ELEVATED)
|
||||
}
|
||||
|
||||
fn render_list(&self, area: Rect, buf: &mut Buffer) {
|
||||
let outer = Block::default()
|
||||
.title(Line::from(Span::styled(
|
||||
@@ -138,39 +170,64 @@ impl ProviderPickerView {
|
||||
let inner = outer.inner(area);
|
||||
outer.render(area, buf);
|
||||
|
||||
let mut lines: Vec<Line> = Vec::with_capacity(self.providers.len());
|
||||
for (idx, (provider, has_key)) in self.providers.iter().enumerate() {
|
||||
let visible_rows = usize::from(inner.height);
|
||||
let visible_start = self.visible_start(visible_rows);
|
||||
let mut lines: Vec<Line> = Vec::with_capacity(visible_rows);
|
||||
for (idx, (provider, has_key)) in self
|
||||
.providers
|
||||
.iter()
|
||||
.enumerate()
|
||||
.skip(visible_start)
|
||||
.take(visible_rows)
|
||||
{
|
||||
let is_selected = idx == self.selected_idx;
|
||||
let is_active = *provider == self.active_provider;
|
||||
let arrow = if is_selected { "▸" } else { " " };
|
||||
let active_dot = if is_active { " *" } else { " " };
|
||||
let label_style = if is_selected {
|
||||
let spacer_style = if is_selected {
|
||||
Self::selected_row_bg_style()
|
||||
} else {
|
||||
Style::default()
|
||||
.fg(palette::SELECTION_TEXT)
|
||||
.bg(palette::SELECTION_BG)
|
||||
.add_modifier(Modifier::BOLD)
|
||||
};
|
||||
let label_style = if is_selected {
|
||||
Self::selected_row_style(palette::TEXT_PRIMARY)
|
||||
} else {
|
||||
Style::default().fg(palette::TEXT_PRIMARY)
|
||||
};
|
||||
let hint_style = if is_selected {
|
||||
Style::default()
|
||||
.fg(palette::SELECTION_TEXT)
|
||||
.bg(palette::SELECTION_BG)
|
||||
let hint_fg = if *has_key {
|
||||
palette::TEXT_MUTED
|
||||
} else {
|
||||
palette::STATUS_WARNING
|
||||
};
|
||||
Self::selected_row_style(hint_fg)
|
||||
} else if *has_key {
|
||||
Style::default().fg(palette::TEXT_MUTED)
|
||||
} else {
|
||||
Style::default().fg(palette::STATUS_WARNING)
|
||||
};
|
||||
let hint = Self::provider_hint(*provider, *has_key);
|
||||
lines.push(Line::from(vec![
|
||||
Span::raw(" "),
|
||||
let mut line = Line::from(vec![
|
||||
Span::styled(" ", spacer_style),
|
||||
Span::styled(arrow, label_style),
|
||||
Span::raw(" "),
|
||||
Span::styled(" ", spacer_style),
|
||||
Span::styled(provider.display_name().to_string(), label_style),
|
||||
Span::styled(active_dot, label_style),
|
||||
Span::raw(" "),
|
||||
Span::styled(" ", spacer_style),
|
||||
Span::styled(hint, hint_style),
|
||||
]));
|
||||
]);
|
||||
if is_selected {
|
||||
line.style = Self::selected_row_bg_style();
|
||||
let target_width = usize::from(inner.width);
|
||||
let line_width = line.width();
|
||||
if line_width < target_width {
|
||||
line.spans.push(Span::styled(
|
||||
" ".repeat(target_width - line_width),
|
||||
Self::selected_row_bg_style(),
|
||||
));
|
||||
}
|
||||
}
|
||||
lines.push(line);
|
||||
}
|
||||
Paragraph::new(lines).render(inner, buf);
|
||||
}
|
||||
@@ -347,7 +404,7 @@ impl ModalView for ProviderPickerView {
|
||||
fn render(&self, area: Rect, buf: &mut Buffer) {
|
||||
let popup_width = 64.min(area.width.saturating_sub(4)).max(40);
|
||||
let popup_height = match self.stage {
|
||||
Stage::List => 12,
|
||||
Stage::List => (self.providers.len() as u16).saturating_add(2),
|
||||
Stage::KeyEntry => 10,
|
||||
}
|
||||
.min(area.height.saturating_sub(4))
|
||||
@@ -388,6 +445,16 @@ mod tests {
|
||||
panic!("provider {provider:?} not found in picker");
|
||||
}
|
||||
|
||||
fn render_text(picker: &ProviderPickerView, width: u16, height: u16) -> String {
|
||||
let area = Rect::new(0, 0, width, height);
|
||||
let mut buf = Buffer::empty(area);
|
||||
picker.render(area, &mut buf);
|
||||
(0..height)
|
||||
.map(|y| (0..width).map(|x| buf[(x, y)].symbol()).collect::<String>())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn picker_lists_all_providers() {
|
||||
let config = Config::default();
|
||||
@@ -440,6 +507,18 @@ mod tests {
|
||||
assert_eq!(picker.active_provider, ApiProvider::Openrouter);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn list_navigation_wraps_between_first_and_last_provider() {
|
||||
let config = Config::default();
|
||||
let mut picker = ProviderPickerView::new(ApiProvider::Deepseek, &config);
|
||||
|
||||
picker.handle_key(key(KeyCode::Up));
|
||||
assert_eq!(picker.selected_provider(), ApiProvider::Ollama);
|
||||
|
||||
picker.handle_key(key(KeyCode::Down));
|
||||
assert_eq!(picker.selected_provider(), ApiProvider::Deepseek);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn enter_with_no_key_transitions_to_key_entry_stage() {
|
||||
let config = Config::default();
|
||||
@@ -459,8 +538,7 @@ mod tests {
|
||||
..Config::default()
|
||||
};
|
||||
let mut picker = ProviderPickerView::new(ApiProvider::NvidiaNim, &config);
|
||||
// Move up twice to DeepSeek (index 0), which has a key from the config.
|
||||
picker.handle_key(key(KeyCode::Up));
|
||||
// Move up once to DeepSeek (index 0), which has a key from the config.
|
||||
picker.handle_key(key(KeyCode::Up));
|
||||
let action = picker.handle_key(key(KeyCode::Enter));
|
||||
match action {
|
||||
@@ -529,4 +607,59 @@ mod tests {
|
||||
}
|
||||
assert_eq!(picker.api_key_input, "abcdef");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn small_list_render_keeps_selected_provider_visible_after_down_navigation() {
|
||||
let config = Config::default();
|
||||
let mut picker = ProviderPickerView::new(ApiProvider::Deepseek, &config);
|
||||
move_to_provider(&mut picker, ApiProvider::Ollama);
|
||||
|
||||
let rendered = render_text(&picker, 80, 12);
|
||||
|
||||
assert!(rendered.contains("Ollama"));
|
||||
assert!(!rendered.contains("DeepSeek *"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn small_list_render_keeps_initial_active_provider_visible() {
|
||||
let config = Config::default();
|
||||
let picker = ProviderPickerView::new(ApiProvider::Ollama, &config);
|
||||
|
||||
let rendered = render_text(&picker, 80, 12);
|
||||
|
||||
assert!(rendered.contains("Ollama *"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tall_list_render_shows_all_providers_without_scrolling() {
|
||||
let config = Config::default();
|
||||
let picker = ProviderPickerView::new(ApiProvider::Deepseek, &config);
|
||||
|
||||
let rendered = render_text(&picker, 80, 20);
|
||||
|
||||
assert!(rendered.contains("DeepSeek *"));
|
||||
assert!(rendered.contains("Ollama"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selected_provider_row_uses_strong_highlight() {
|
||||
let config = Config::default();
|
||||
let picker = ProviderPickerView::new(ApiProvider::Deepseek, &config);
|
||||
let area = Rect::new(0, 0, 80, 20);
|
||||
let mut buf = Buffer::empty(area);
|
||||
|
||||
picker.render(area, &mut buf);
|
||||
|
||||
let highlighted_cells = area
|
||||
.positions()
|
||||
.filter(|position| {
|
||||
let cell = &buf[*position];
|
||||
cell.bg == palette::SURFACE_ELEVATED
|
||||
})
|
||||
.count();
|
||||
assert!(
|
||||
highlighted_cells >= 32,
|
||||
"selected provider row should use a visible continuous highlight"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user