mirror of
https://github.com/openai/codex.git
synced 2026-04-24 14:45:27 +00:00
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 <noreply@openai.com>
This commit is contained in:
@@ -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:<max_created_width$}")).dim())
|
||||
@@ -1023,9 +1025,16 @@ fn render_list(
|
||||
}
|
||||
spans.push(preview.into());
|
||||
|
||||
if is_sel {
|
||||
apply_selected_row_text_style(&mut spans);
|
||||
}
|
||||
|
||||
let line: Line = spans.into();
|
||||
let rect = Rect::new(area.x, y, area.width, 1);
|
||||
frame.render_widget_ref(line, rect);
|
||||
if is_sel {
|
||||
paint_selected_row_background(frame.buffer_mut(), rect);
|
||||
}
|
||||
y = y.saturating_add(1);
|
||||
}
|
||||
|
||||
@@ -1166,6 +1175,23 @@ fn render_column_headers(
|
||||
frame.render_widget_ref(Line::from(spans), area);
|
||||
}
|
||||
|
||||
fn apply_selected_row_text_style(spans: &mut [Span<'_>]) {
|
||||
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 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 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 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(|_| {});
|
||||
|
||||
@@ -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,
|
||||
]
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user