From 53468b97f6eded34f9e593ef6d9d9f0b2ddb303a Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 9 May 2026 16:10:56 -0300 Subject: [PATCH] fix(tui): improve light-mode selection contrast (#21950) ## Why On light terminal backgrounds, selected rows in several TUI pickers were rendered with the same bright cyan accent used on dark themes. Against the light menu surface, that made the current selection hard to distinguish at a glance.

Before

SCR-20260509-nmtz

After

SCR-20260509-nmox
## What changed - Added a shared background-aware accent style for active/selected TUI controls. - Use a darker cyan-family accent on light backgrounds while preserving the existing bright cyan accent on dark or unknown backgrounds. - Reused that accent across shared picker rows and the custom selection-like surfaces that had drifted separately: picker tabs, hooks browsing, external-agent migration choices, and /keymap affordances. - Added focused tests for the light/dark accent rule and rendered selected-row styling. ## How to Test 1. Start Codex in a terminal using a light background theme. 2. Type `/` to open the slash-command picker and move the selection through a few rows. 3. Confirm that the selected row is visibly colored with strong contrast instead of blending into the popup surface. 4. Open `/keymap` and confirm the active tab, selected rows, and picker hint accents use the same light-theme accent treatment. 5. In a dark terminal theme, repeat the slash-picker check and confirm the existing bright cyan selection styling is preserved. Targeted tests: - `cargo test -p codex-tui accent_style_uses_` - `cargo test -p codex-tui selected_rows_use_the_shared_accent_style` - `cargo test -p codex-tui selected_event_rows_use_the_shared_accent_style` Notes: - A full `cargo test -p codex-tui` run reached the end of the suite but hit an unrelated existing stack overflow in `tests::fork_last_filters_latest_session_by_cwd_unless_show_all`. --- .../tui/src/bottom_pane/hooks_browser_view.rs | 50 ++++++++++++++----- .../src/bottom_pane/selection_popup_common.rs | 33 ++++++++++-- .../tui/src/bottom_pane/selection_tabs.rs | 9 ++-- .../src/external_agent_config_migration.rs | 3 +- codex-rs/tui/src/keymap_setup/picker.rs | 20 +++++--- codex-rs/tui/src/style.rs | 40 +++++++++++++++ 6 files changed, 127 insertions(+), 28 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs index 7e1a41cf30..2ce5b33045 100644 --- a/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs +++ b/codex-rs/tui/src/bottom_pane/hooks_browser_view.rs @@ -10,6 +10,7 @@ use ratatui::buffer::Buffer; use ratatui::layout::Constraint; use ratatui::layout::Layout; use ratatui::layout::Rect; +use ratatui::style::Styled; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; @@ -29,6 +30,7 @@ use crate::key_hint; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::renderable::Renderable; use crate::status::format_directory_display; +use crate::style::accent_style; const EVENT_COLUMN_WIDTH: usize = 22; const COUNT_COLUMN_WIDTH: usize = 12; @@ -279,28 +281,23 @@ impl HooksBrowserView { lines.push(Line::from(header)); for (idx, row) in rows.into_iter().enumerate() { if self.state.selected_idx == Some(idx) { + let style = accent_style(); let mut row_line = vec![ Span::from(format!( "{: String { @@ -826,6 +824,14 @@ mod tests { .join("\n") } + fn render_buffer(view: &HooksBrowserView, width: u16) -> Buffer { + let height = view.desired_height(width); + let area = Rect::new(0, 0, width, height); + let mut buf = Buffer::empty(area); + view.render(area, &mut buf); + buf + } + #[allow(clippy::too_many_arguments)] fn hook( key: &str, @@ -908,6 +914,26 @@ mod tests { assert_snapshot!("hooks_browser_events", render_lines(&view, /*width*/ 112)); } + #[test] + fn selected_event_rows_use_the_shared_accent_style() { + let view = view(); + let buf = render_buffer(&view, /*width*/ 112); + let expected = accent_style(); + + let selected_cell = buf + .content + .iter() + .find(|cell| { + let style = cell.style(); + cell.symbol() == "P" + && style.fg == expected.fg + && style.add_modifier.contains(Modifier::BOLD) + }) + .expect("selected event row should use the shared accent style"); + + assert_eq!(selected_cell.style().fg, expected.fg); + } + #[test] fn renders_event_browser_with_review_column_when_needed() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs index 3507cb31ec..91d155b507 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -2,8 +2,6 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; // Note: Table-based layout previously used Constraint; the manual renderer // below no longer requires it. -use ratatui::style::Color; -use ratatui::style::Style; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; @@ -17,6 +15,7 @@ use crate::key_hint::KeyBinding; use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::Insets; use crate::render::RectExt as _; +use crate::style::accent_style; use crate::style::user_message_style; use super::scroll_state::ScrollState; @@ -318,7 +317,7 @@ fn apply_row_state_style(lines: &mut [Line<'static>], selected: bool, is_disable if selected { for line in lines.iter_mut() { line.spans.iter_mut().for_each(|span| { - span.style = Style::default().fg(Color::Cyan).bold(); + span.style = accent_style(); }); } } @@ -725,7 +724,7 @@ pub(crate) fn render_rows_single_line_with_col_width_mode( let mut full_line = build_full_line(row, desc_col); if Some(i) == state.selected_idx && !row.is_disabled { full_line.spans.iter_mut().for_each(|span| { - span.style = Style::default().fg(Color::Cyan).bold(); + span.style = accent_style(); }); } if row.is_disabled { @@ -839,6 +838,9 @@ fn measure_rows_height_inner( mod tests { use super::*; use pretty_assertions::assert_eq; + use ratatui::buffer::Buffer; + use ratatui::layout::Rect; + use ratatui::style::Modifier; #[test] fn one_cell_width_falls_back_without_panic_for_wrapped_two_column_rows() { @@ -852,4 +854,27 @@ mod tests { let two_col = wrap_two_column_row(&row, /*desc_col*/ 0, /*width*/ 1); assert_eq!(two_col.len(), 0); } + + #[test] + fn selected_rows_use_the_shared_accent_style() { + let rows = vec![GenericDisplayRow { + name: "selected".to_string(), + ..Default::default() + }]; + let state = ScrollState { + selected_idx: Some(0), + ..Default::default() + }; + let area = Rect::new(0, 0, 16, 1); + let mut buf = Buffer::empty(area); + + render_rows( + area, &mut buf, &rows, &state, /*max_results*/ 1, "no rows", + ); + + let style = buf[(0, 0)].style(); + let expected = accent_style(); + assert_eq!(style.fg, expected.fg); + assert!(style.add_modifier.contains(Modifier::BOLD)); + } } diff --git a/codex-rs/tui/src/bottom_pane/selection_tabs.rs b/codex-rs/tui/src/bottom_pane/selection_tabs.rs index 8d5612b5ab..a4c4c42a9d 100644 --- a/codex-rs/tui/src/bottom_pane/selection_tabs.rs +++ b/codex-rs/tui/src/bottom_pane/selection_tabs.rs @@ -1,11 +1,13 @@ use ratatui::buffer::Buffer; use ratatui::layout::Rect; +use ratatui::style::Styled; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; use ratatui::widgets::Widget; use crate::render::renderable::Renderable; +use crate::style::accent_style; use super::SelectionItem; @@ -92,10 +94,11 @@ fn tab_bar_lines(tabs: &[SelectionTab], active_idx: usize, width: u16) -> Vec
  • Vec> { if active { + let style = accent_style(); vec![ - "[".cyan().bold(), - label.to_string().cyan().bold(), - "]".cyan().bold(), + "[".set_style(style), + label.to_string().set_style(style), + "]".set_style(style), ] } else { vec![label.to_string().dim()] diff --git a/codex-rs/tui/src/external_agent_config_migration.rs b/codex-rs/tui/src/external_agent_config_migration.rs index fa6dcc6e43..1ce9d44751 100644 --- a/codex-rs/tui/src/external_agent_config_migration.rs +++ b/codex-rs/tui/src/external_agent_config_migration.rs @@ -4,6 +4,7 @@ use crate::line_truncation::truncate_line_with_ellipsis_if_overflow; use crate::render::Insets; use crate::render::RectExt as _; use crate::selection_list::selection_option_row_with_dim; +use crate::style::accent_style; use crate::tui::FrameRequester; use crate::tui::Tui; use crate::tui::TuiEvent; @@ -610,7 +611,7 @@ impl ExternalAgentConfigMigrationScreen { let mut line = entry.line.clone(); if selected { line.spans.iter_mut().for_each(|span| { - span.style = span.style.cyan().bold(); + span.style = span.style.patch(accent_style()); }); } else if entry.kind != RenderLineKind::Item && !line.spans.is_empty() { line.spans.iter_mut().for_each(|span| { diff --git a/codex-rs/tui/src/keymap_setup/picker.rs b/codex-rs/tui/src/keymap_setup/picker.rs index ed62d0c2e5..b6c89d9585 100644 --- a/codex-rs/tui/src/keymap_setup/picker.rs +++ b/codex-rs/tui/src/keymap_setup/picker.rs @@ -1,6 +1,7 @@ //! Shortcut picker construction for `/keymap`. use codex_config::types::TuiKeymap; +use ratatui::style::Styled; use ratatui::style::Stylize; use ratatui::text::Line; use ratatui::text::Span; @@ -15,6 +16,7 @@ use crate::bottom_pane::SelectionViewParams; use crate::keymap::RuntimeKeymap; use crate::render::renderable::ColumnRenderable; use crate::render::renderable::Renderable; +use crate::style::accent_style; use super::actions::KEYMAP_ACTIONS; use super::actions::KeymapActionFilter; @@ -415,7 +417,7 @@ fn keymap_selection_item(row: &KeymapActionRow) -> SelectionItem { fn keymap_row_prefix(row: &KeymapActionRow) -> Vec> { let indicator = if row.custom_binding { - "*".cyan() + "*".set_style(accent_style()) } else if row.is_unbound() { "-".dim() } else { @@ -450,25 +452,27 @@ fn action_count_line(count: usize) -> String { } fn keymap_picker_hint_line() -> Line<'static> { + let style = accent_style(); Line::from(vec![ - "left/right".cyan(), + "left/right".set_style(style), " group · ".dim(), - "enter".cyan(), + "enter".set_style(style), " edit shortcut · ".dim(), - "*".cyan(), + "*".set_style(style), " custom · ".dim(), - "-".cyan(), + "-".set_style(style), " unbound · ".dim(), - "esc".cyan(), + "esc".set_style(style), " close".dim(), ]) } fn keymap_debug_hint_line() -> Line<'static> { + let style = accent_style(); Line::from(vec![ - "enter".cyan(), + "enter".set_style(style), " start inspector · ".dim(), - "esc".cyan(), + "esc".set_style(style), " close".dim(), ]) } diff --git a/codex-rs/tui/src/style.rs b/codex-rs/tui/src/style.rs index fc8fd4c501..10c269174f 100644 --- a/codex-rs/tui/src/style.rs +++ b/codex-rs/tui/src/style.rs @@ -4,6 +4,9 @@ use crate::terminal_palette::best_color; use crate::terminal_palette::default_bg; use ratatui::style::Color; use ratatui::style::Style; +use ratatui::style::Stylize; + +const LIGHT_BG_ACCENT_RGB: (u8, u8, u8) = (0, 95, 135); pub fn user_message_style() -> Style { user_message_style_for(default_bg()) @@ -13,6 +16,11 @@ pub fn proposed_plan_style() -> Style { proposed_plan_style_for(default_bg()) } +/// Returns the shared accent style for active or selected TUI controls. +pub(crate) fn accent_style() -> Style { + accent_style_for(default_bg()) +} + /// Returns the style for a user-authored message using the provided terminal background. pub fn user_message_style_for(terminal_bg: Option<(u8, u8, u8)>) -> Style { match terminal_bg { @@ -28,6 +36,15 @@ pub fn proposed_plan_style_for(terminal_bg: Option<(u8, u8, u8)>) -> Style { } } +/// Returns the shared accent style for the provided terminal background. +pub(crate) fn accent_style_for(terminal_bg: Option<(u8, u8, u8)>) -> Style { + if terminal_bg.is_some_and(is_light) { + Style::default().fg(best_color(LIGHT_BG_ACCENT_RGB)).bold() + } else { + Style::default().fg(Color::Cyan).bold() + } +} + #[allow(clippy::disallowed_methods)] pub fn user_message_bg(terminal_bg: (u8, u8, u8)) -> Color { let (top, alpha) = if is_light(terminal_bg) { @@ -42,3 +59,26 @@ pub fn user_message_bg(terminal_bg: (u8, u8, u8)) -> Color { pub fn proposed_plan_bg(terminal_bg: (u8, u8, u8)) -> Color { user_message_bg(terminal_bg) } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use ratatui::style::Modifier; + + #[test] + fn accent_style_uses_darker_cyan_on_light_backgrounds() { + let style = accent_style_for(Some((255, 255, 255))); + + assert_eq!(style.fg, Some(best_color(LIGHT_BG_ACCENT_RGB))); + assert!(style.add_modifier.contains(Modifier::BOLD)); + } + + #[test] + fn accent_style_uses_cyan_on_dark_or_unknown_backgrounds() { + let expected = Style::default().fg(Color::Cyan).bold(); + + assert_eq!(accent_style_for(Some((0, 0, 0))), expected); + assert_eq!(accent_style_for(/*terminal_bg*/ None), expected); + } +}