diff --git a/crates/tui/src/tui/provider_picker.rs b/crates/tui/src/tui/provider_picker.rs index b2ac79e6..a4cdfb6c 100644 --- a/crates/tui/src/tui/provider_picker.rs +++ b/crates/tui/src/tui/provider_picker.rs @@ -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 = 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 = 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::()) + .collect::>() + .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" + ); + } }