diff --git a/codex-rs/tui/src/bottom_pane/list_selection_view.rs b/codex-rs/tui/src/bottom_pane/list_selection_view.rs index ea49ff9083..ad40c1660f 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -42,6 +42,9 @@ const MIN_LIST_WIDTH_FOR_SIDE: u16 = 40; /// panel when side-by-side layout is active. const SIDE_CONTENT_GAP: u16 = 2; +/// Shared menu-surface horizontal inset (2 cells per side) used by selection popups. +const MENU_SURFACE_HORIZONTAL_INSET: u16 = 4; + /// Controls how the side content panel is sized relative to the popup width. /// /// When the computed side width falls below `side_content_min_width` or the @@ -61,17 +64,44 @@ impl Default for SideContentWidth { } } +/// Returns the popup content width after subtracting the shared menu-surface +/// horizontal inset (2 columns on each side). +pub(crate) fn popup_content_width(total_width: u16) -> u16 { + total_width.saturating_sub(MENU_SURFACE_HORIZONTAL_INSET) +} + +/// Returns side-by-side layout widths as `(list_width, side_width)` when the +/// layout can fit. Returns `None` when the side panel is disabled/too narrow or +/// when the remaining list width would become unusably small. +pub(crate) fn side_by_side_layout_widths( + content_width: u16, + side_content_width: SideContentWidth, + side_content_min_width: u16, +) -> Option<(u16, u16)> { + let side_width = match side_content_width { + SideContentWidth::Fixed(0) => return None, + SideContentWidth::Fixed(width) => width, + SideContentWidth::Half => content_width.saturating_sub(SIDE_CONTENT_GAP) / 2, + }; + if side_width < side_content_min_width { + return None; + } + let list_width = content_width.saturating_sub(SIDE_CONTENT_GAP + side_width); + (list_width >= MIN_LIST_WIDTH_FOR_SIDE).then_some((list_width, side_width)) +} + /// One selectable item in the generic selection list. pub(crate) type SelectionAction = Box; /// Callback invoked whenever the highlighted item changes (arrow keys, search /// filter, number-key jump). Receives the *actual* index into the unfiltered /// `items` list and the event sender. Used by the theme picker for live preview. -pub type OnSelectionChangedCallback = Option>; +pub(crate) type OnSelectionChangedCallback = + Option>; /// Callback invoked when the picker is dismissed without accepting (Esc or /// Ctrl+C). Used by the theme picker to restore the pre-open theme. -pub type OnCancelCallback = Option>; +pub(crate) type OnCancelCallback = Option>; /// One row in a [`ListSelectionView`] selection list. /// @@ -488,16 +518,12 @@ impl ListSelectionView { /// Returns `Some(side_width)` when the content area is wide enough for a /// side-by-side layout (list + gap + side panel), `None` otherwise. fn side_layout_width(&self, content_width: u16) -> Option { - let side_width = match self.side_content_width { - SideContentWidth::Fixed(0) => return None, - SideContentWidth::Fixed(width) => width, - SideContentWidth::Half => content_width.saturating_sub(SIDE_CONTENT_GAP) / 2, - }; - if side_width < self.side_content_min_width { - return None; - } - let list_width = content_width.saturating_sub(SIDE_CONTENT_GAP + side_width); - (list_width >= MIN_LIST_WIDTH_FOR_SIDE).then_some(side_width) + side_by_side_layout_widths( + content_width, + self.side_content_width, + self.side_content_min_width, + ) + .map(|(_, side_width)| side_width) } fn skip_disabled_down(&mut self) { @@ -652,7 +678,7 @@ impl BottomPaneView for ListSelectionView { impl Renderable for ListSelectionView { fn desired_height(&self, width: u16) -> u16 { // Inner content width after menu surface horizontal insets (2 per side). - let inner_width = width.saturating_sub(4); + let inner_width = popup_content_width(width); // When side-by-side is active, measure the list at the reduced width // that accounts for the gap and side panel. @@ -734,7 +760,7 @@ impl Renderable for ListSelectionView { // Paint the shared menu surface and then layout inside the returned inset. let content_area = render_menu_surface(outer_content_area, buf); - let inner_width = outer_content_area.width.saturating_sub(4); + let inner_width = popup_content_width(outer_content_area.width); let side_w = self.side_layout_width(inner_width); // When side-by-side is active, shrink the list to make room. diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 52f8520f6f..9c7b940f0e 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -79,6 +79,8 @@ pub(crate) use footer::CollaborationModeIndicator; pub(crate) use list_selection_view::ColumnWidthMode; pub(crate) use list_selection_view::SelectionViewParams; pub(crate) use list_selection_view::SideContentWidth; +pub(crate) use list_selection_view::popup_content_width; +pub(crate) use list_selection_view::side_by_side_layout_widths; mod feedback_view; pub(crate) use feedback_view::FeedbackAudience; pub(crate) use feedback_view::feedback_disabled_params; diff --git a/codex-rs/tui/src/render/highlight.rs b/codex-rs/tui/src/render/highlight.rs index 0b398fa2bc..b0898ffe0a 100644 --- a/codex-rs/tui/src/render/highlight.rs +++ b/codex-rs/tui/src/render/highlight.rs @@ -241,10 +241,12 @@ pub(crate) fn current_syntax_theme() -> Theme { } } -/// Return the kebab-case name of the currently active theme. -/// This accounts for the user override, custom .tmTheme files, and the -/// adaptive auto-detection fallback. -pub(crate) fn current_theme_name() -> String { +/// Return the configured kebab-case theme name when it resolves; otherwise +/// return the adaptive auto-detected default theme name. +/// +/// This intentionally reflects persisted configuration/default selection, not +/// transient runtime swaps applied via `set_syntax_theme`. +pub(crate) fn configured_theme_name() -> String { // Explicit user override? if let Some(Some(name)) = THEME_OVERRIDE.get() { if parse_theme_name(name).is_some() { diff --git a/codex-rs/tui/src/theme_picker.rs b/codex-rs/tui/src/theme_picker.rs index bfa5b8d569..9205b5fb31 100644 --- a/codex-rs/tui/src/theme_picker.rs +++ b/codex-rs/tui/src/theme_picker.rs @@ -26,6 +26,8 @@ use crate::bottom_pane::SelectionItem; use crate::bottom_pane::SelectionViewParams; use crate::bottom_pane::SideContentWidth; use crate::bottom_pane::popup_consts::standard_popup_hint_line; +use crate::bottom_pane::popup_content_width; +use crate::bottom_pane::side_by_side_layout_widths; use crate::diff_render::DiffLineType; use crate::diff_render::line_number_width; use crate::diff_render::push_wrapped_diff_line; @@ -134,15 +136,6 @@ const PREVIEW_FRAME_PADDING: u16 = 1; const PREVIEW_FALLBACK_SUBTITLE: &str = "Move up/down to live preview themes"; -/// Shared menu-surface horizontal inset (2 cells per side) used by selection popups. -const MENU_SURFACE_HORIZONTAL_INSET: u16 = 4; - -/// Horizontal gap between list and side panel when side-by-side layout is active. -const SIDE_CONTENT_GAP: u16 = 2; - -/// Minimum list width required for side-by-side mode in the selection popup. -const MIN_LIST_WIDTH_FOR_SIDE: u16 = 40; - /// Side-by-side preview: syntax-highlighted Rust diff snippet, vertically /// centered with a 2-column left inset. Fills the entire side panel height. struct ThemePreviewWideRenderable; @@ -261,12 +254,12 @@ impl Renderable for ThemePreviewNarrowRenderable { fn subtitle_available_width(terminal_width: Option) -> usize { let width = terminal_width.unwrap_or(80); - let content_width = width.saturating_sub(MENU_SURFACE_HORIZONTAL_INSET); - let side_width = content_width.saturating_sub(SIDE_CONTENT_GAP) / 2; - let list_width = content_width.saturating_sub(SIDE_CONTENT_GAP + side_width); - let side_by_side = - side_width >= WIDE_PREVIEW_MIN_WIDTH && list_width >= MIN_LIST_WIDTH_FOR_SIDE; - if side_by_side { + let content_width = popup_content_width(width); + if let Some((list_width, _side_width)) = side_by_side_layout_widths( + content_width, + SideContentWidth::Half, + WIDE_PREVIEW_MIN_WIDTH, + ) { list_width as usize } else { content_width as usize @@ -299,9 +292,9 @@ fn theme_picker_subtitle(codex_home: Option<&Path>, terminal_width: Option) /// /// `current_name` should be the value of `Config::tui_theme` (the persisted /// preference). When it names a theme that is currently available the picker -/// pre-selects it; otherwise the picker falls back to the active runtime theme -/// (from adaptive auto-detection) so opening the picker without a persisted -/// preference still highlights the correct entry. +/// pre-selects it; otherwise the picker falls back to the configured name (or +/// adaptive default) so opening the picker without a persisted preference still +/// highlights the most likely intended entry. pub(crate) fn build_theme_picker_params( current_name: Option<&str>, codex_home: Option<&Path>, @@ -314,14 +307,14 @@ pub(crate) fn build_theme_picker_params( let codex_home_owned = codex_home.map(Path::to_path_buf); // Resolve the effective theme name: honor explicit config only when it is - // currently available; otherwise fall back to the active runtime theme so - // opening `/theme` does not auto-preview an unrelated first entry. + // currently available; otherwise fall back to configured/default selection + // so opening `/theme` does not auto-preview an unrelated first entry. let effective_name = if let Some(name) = current_name && entries.iter().any(|entry| entry.name == name) { name.to_string() } else { - highlight::current_theme_name() + highlight::configured_theme_name() }; // Track the index of the current theme so we can pre-select it. @@ -611,8 +604,8 @@ mod tests { } #[test] - fn unavailable_configured_theme_falls_back_to_active_theme_selection() { - let active_theme = highlight::current_theme_name(); + fn unavailable_configured_theme_falls_back_to_configured_or_default_selection() { + let configured_or_default_theme = highlight::configured_theme_name(); let params = build_theme_picker_params(Some("not-a-real-theme"), None, Some(120)); let selected_idx = params .initial_selected_idx @@ -622,6 +615,6 @@ mod tests { .as_deref() .expect("expected search value to contain canonical theme name"); - assert_eq!(selected_name, active_theme); + assert_eq!(selected_name, configured_or_default_theme); } }