From c25432ab72c1d7fb079aeb8e8233e4806239a0eb Mon Sep 17 00:00:00 2001 From: Gabriel Cohen Date: Sat, 28 Feb 2026 08:00:45 +0900 Subject: [PATCH] tui: strengthen resume picker row highlight Make the selected /resume row use the full selection treatment instead of only changing the marker. Checks run: - just fmt - cargo test -p codex-tui - cargo clippy -p codex-tui --tests Co-authored-by: Codex --- codex-rs/tui/src/resume_picker.rs | 126 +++++++++++++++--- ...me_picker__tests__resume_picker_table.snap | 57 +++++++- ...er__tests__resume_picker_thread_names.snap | 2 +- 3 files changed, 161 insertions(+), 24 deletions(-) diff --git a/codex-rs/tui/src/resume_picker.rs b/codex-rs/tui/src/resume_picker.rs index 6cb1387a4e0..a8b3b7bf994 100644 --- a/codex-rs/tui/src/resume_picker.rs +++ b/codex-rs/tui/src/resume_picker.rs @@ -6,6 +6,7 @@ use std::sync::Arc; use crate::diff_render::display_path_for; use crate::key_hint; +use crate::style::user_message_style; use crate::text_formatting::truncate_text; use crate::tui::FrameRequester; use crate::tui::Tui; @@ -29,6 +30,7 @@ use crossterm::event::KeyEventKind; use ratatui::layout::Constraint; use ratatui::layout::Layout; use ratatui::layout::Rect; +use ratatui::style::Style; use ratatui::style::Stylize as _; use ratatui::text::Line; use ratatui::text::Span; @@ -938,7 +940,7 @@ fn render_list( .enumerate() { let is_sel = start + idx == state.selected; - let marker = if is_sel { "> ".bold() } else { " ".into() }; + let marker = if is_sel { "› ".into() } else { " ".into() }; let marker_width = 2usize; let created_span = if visibility.show_created { Some(Span::from(format!("{created_label:]) { + let selected_style = Style::default().cyan().bold(); + for span in spans { + span.style = span.style.patch(selected_style); + } +} + +fn paint_selected_row_background(buf: &mut ratatui::buffer::Buffer, area: Rect) { + let selected_style = user_message_style(); + for y in area.y..area.y.saturating_add(area.height) { + for x in area.x..area.x.saturating_add(area.width) { + let cell = &mut buf[(x, y)]; + cell.set_style(cell.style().patch(selected_style)); + } + } +} + /// Pre-computed column widths and formatted labels for all visible rows. /// /// Widths are measured in Unicode display width (not byte length) so columns @@ -1528,8 +1554,7 @@ mod tests { #[test] fn resume_table_snapshot() { - use crate::custom_terminal::Terminal; - use crate::test_backend::VT100Backend; + use ratatui::buffer::Buffer; use ratatui::layout::Constraint; use ratatui::layout::Layout; @@ -1588,22 +1613,18 @@ mod tests { let width: u16 = 80; let height: u16 = 6; - let backend = VT100Backend::new(width, height); - let mut terminal = Terminal::with_options(backend).expect("terminal"); - terminal.set_viewport_area(Rect::new(0, 0, width, height)); - - { - let mut frame = terminal.get_frame(); - let area = frame.area(); - let segments = - Layout::vertical([Constraint::Length(1), Constraint::Min(1)]).split(area); - render_column_headers(&mut frame, segments[0], &metrics, state.sort_key); - render_list(&mut frame, segments[1], &state, &metrics); - } - terminal.flush().expect("flush"); + let area = Rect::new(0, 0, width, height); + let mut buf = Buffer::empty(area); + let segments = Layout::vertical([Constraint::Length(1), Constraint::Min(1)]).split(area); + let mut frame = crate::custom_terminal::Frame { + cursor_position: None, + viewport_area: area, + buffer: &mut buf, + }; + render_column_headers(&mut frame, segments[0], &metrics, state.sort_key); + render_list(&mut frame, segments[1], &state, &metrics); - let snapshot = terminal.backend().to_string(); - assert_snapshot!("resume_picker_table", snapshot); + assert_snapshot!("resume_picker_table", format!("{buf:?}")); } // TODO(jif) fix @@ -1876,6 +1897,75 @@ mod tests { assert_snapshot!("resume_picker_thread_names", snapshot); } + #[test] + fn selected_row_highlighting_styles_the_full_row() { + use ratatui::buffer::Buffer; + use ratatui::style::Color; + use ratatui::style::Modifier; + + let loader: PageLoader = Arc::new(|_| {}); + let mut state = PickerState::new( + PathBuf::from("/tmp"), + FrameRequester::test_dummy(), + loader, + String::from("openai"), + true, + None, + SessionPickerAction::Resume, + ); + + let now = Utc::now(); + state.filtered_rows = vec![ + Row { + path: PathBuf::from("/tmp/a.jsonl"), + preview: String::from("First row"), + thread_id: None, + thread_name: None, + created_at: Some(now - Duration::minutes(1)), + updated_at: Some(now - Duration::seconds(30)), + cwd: None, + git_branch: None, + }, + Row { + path: PathBuf::from("/tmp/b.jsonl"), + preview: String::from("Selected row"), + thread_id: None, + thread_name: None, + created_at: Some(now - Duration::minutes(2)), + updated_at: Some(now - Duration::minutes(1)), + cwd: None, + git_branch: None, + }, + ]; + state.selected = 1; + state.scroll_top = 0; + state.view_rows = Some(2); + + let metrics = calculate_column_metrics(&state.filtered_rows, state.show_all); + let area = Rect::new(0, 0, 80, 2); + let mut buf = Buffer::empty(area); + let mut frame = crate::custom_terminal::Frame { + cursor_position: None, + viewport_area: area, + buffer: &mut buf, + }; + render_list(&mut frame, area, &state, &metrics); + + let marker = &buf[(0, 1)]; + assert_eq!(marker.symbol(), "›"); + assert_eq!(marker.style().fg, Some(Color::Cyan)); + assert!(marker.style().add_modifier.contains(Modifier::BOLD)); + + let metadata = &buf[(2, 1)]; + assert_eq!(metadata.style().fg, Some(Color::Cyan)); + assert!(metadata.style().add_modifier.contains(Modifier::BOLD)); + + if let Some(selected_bg) = user_message_style().bg { + assert_eq!(buf[(79, 1)].style().bg, Some(selected_bg)); + assert_ne!(buf[(79, 0)].style().bg, Some(selected_bg)); + } + } + #[test] fn pageless_scrolling_deduplicates_and_keeps_order() { let loader: PageLoader = Arc::new(|_| {}); diff --git a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_table.snap b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_table.snap index 1505d6e7e39..096cf943f7d 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_table.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_table.snap @@ -1,8 +1,55 @@ --- source: tui/src/resume_picker.rs -expression: snapshot +assertion_line: 1627 +expression: "format!(\"{buf:?}\")" --- - Created at Updated at Branch CWD Conversation - 16 minutes ago 42 seconds ago - - Fix resume picker timestamps -> 1 hour ago 35 minutes ago - - Investigate lazy pagination cap - 2 hours ago 2 hours ago - - Explain the codebase +Buffer { + area: Rect { x: 0, y: 0, width: 80, height: 6 }, + content: [ + " Created at Updated at Branch CWD Conversation ", + " 16 minutes ago 42 seconds ago - - Fix resume picker timestamps ", + "› 1 hour ago 35 minutes ago - - Investigate lazy pagination cap ", + " 2 hours ago 2 hours ago - - Explain the codebase ", + " ", + " ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 16, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 18, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 32, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 34, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 40, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 42, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 45, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 47, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 59, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 16, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 18, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 32, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 34, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 40, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 42, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 45, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 2, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD | DIM, + x: 16, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 18, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD | DIM, + x: 32, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 34, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD | DIM, + x: 40, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 42, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD | DIM, + x: 45, y: 2, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 78, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 16, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 18, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 32, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 34, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 40, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 42, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 45, y: 3, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +} diff --git a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_thread_names.snap b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_thread_names.snap index d001fff0b9f..19155544bde 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_thread_names.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__resume_picker__tests__resume_picker_thread_names.snap @@ -3,5 +3,5 @@ source: tui/src/resume_picker.rs expression: snapshot --- Created at Updated at Branch CWD Conversation -> - 2 days ago - - Keep this for now +› - 2 days ago - - Keep this for now - 3 days ago - - Named thread