From ac2f769062eb52de93b4fb5bec91dc17c43308c7 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Thu, 16 Apr 2026 12:35:34 -0700 Subject: [PATCH 1/3] Add generic tabbed selection-list support to the bottom pane, single-line row rendering, and shared name-column sizing. Wire the current /plugins menu into the new single-line and column-width behavior, but leave tabbed menu adoption for future use. --- .../tui/src/bottom_pane/bottom_pane_view.rs | 5 + .../src/bottom_pane/list_selection_view.rs | 417 +++++++++++++++--- codex-rs/tui/src/bottom_pane/mod.rs | 10 + .../src/bottom_pane/selection_popup_common.rs | 124 +++--- .../tui/src/bottom_pane/selection_tabs.rs | 103 +++++ codex-rs/tui/src/chatwidget/plugins.rs | 11 + 6 files changed, 529 insertions(+), 141 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/selection_tabs.rs diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 3c0ab2c74eb7..207c4386f6ea 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -49,6 +49,11 @@ pub(crate) trait BottomPaneView: Renderable { None } + /// Active tab id for tabbed list-based views. + fn active_tab_id(&self) -> Option<&str> { + None + } + /// Handle Ctrl-C while this view is active. fn on_ctrl_c(&mut self) -> CancellationEvent { CancellationEvent::NotHandled 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 c95565455d80..e11bb9539091 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -24,14 +24,15 @@ use super::bottom_pane_view::BottomPaneView; use super::bottom_pane_view::ViewCompletion; use super::popup_consts::MAX_POPUP_ROWS; use super::scroll_state::ScrollState; +use super::selection_popup_common::ColumnWidthConfig; pub(crate) use super::selection_popup_common::ColumnWidthMode; use super::selection_popup_common::GenericDisplayRow; -use super::selection_popup_common::measure_rows_height; -use super::selection_popup_common::measure_rows_height_stable_col_widths; use super::selection_popup_common::measure_rows_height_with_col_width_mode; -use super::selection_popup_common::render_rows; -use super::selection_popup_common::render_rows_stable_col_widths; +use super::selection_popup_common::render_rows_single_line_with_col_width_mode; use super::selection_popup_common::render_rows_with_col_width_mode; +use super::selection_tabs::SelectionTab; +use super::selection_tabs::render_tab_bar; +use super::selection_tabs::tab_bar_height; use unicode_width::UnicodeWidthStr; /// Minimum list width (in content columns) required before the side-by-side @@ -91,6 +92,13 @@ pub(crate) fn side_by_side_layout_widths( (list_width >= MIN_LIST_WIDTH_FOR_SIDE).then_some((list_width, side_width)) } +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) enum SelectionRowDisplay { + #[default] + Wrapped, + SingleLine, +} + /// One selectable item in the generic selection list. pub(crate) type SelectionAction = Box; @@ -137,6 +145,7 @@ pub(crate) struct SelectionItem { /// `AutoVisible` (default) measures only rows visible in the viewport /// `AutoAllRows` measures all rows to ensure stable column widths as the user scrolls /// `Fixed` used a fixed 30/70 split between columns +/// `row_display` controls whether rows can wrap or stay single-line with ellipsis truncation pub(crate) struct SelectionViewParams { pub view_id: Option<&'static str>, pub title: Option, @@ -144,9 +153,14 @@ pub(crate) struct SelectionViewParams { pub footer_note: Option>, pub footer_hint: Option>, pub items: Vec, + pub tabs: Vec, + pub initial_tab_id: Option, pub is_searchable: bool, pub search_placeholder: Option, pub col_width_mode: ColumnWidthMode, + pub row_display: SelectionRowDisplay, + /// Rendered left-column width to use for auto-sized rows. + pub name_column_width: Option, pub header: Box, pub initial_selected_idx: Option, @@ -186,9 +200,13 @@ impl Default for SelectionViewParams { footer_note: None, footer_hint: None, items: Vec::new(), + tabs: Vec::new(), + initial_tab_id: None, is_searchable: false, search_placeholder: None, col_width_mode: ColumnWidthMode::AutoVisible, + row_display: SelectionRowDisplay::Wrapped, + name_column_width: None, header: Box::new(()), initial_selected_idx: None, side_content: Box::new(()), @@ -212,6 +230,8 @@ pub(crate) struct ListSelectionView { footer_note: Option>, footer_hint: Option>, items: Vec, + tabs: Vec, + active_tab_idx: Option, state: ScrollState, completion: Option, dismiss_after_child_accept: bool, @@ -220,6 +240,8 @@ pub(crate) struct ListSelectionView { search_query: String, search_placeholder: Option, col_width_mode: ColumnWidthMode, + row_display: SelectionRowDisplay, + name_column_width: Option, filtered_indices: Vec, last_selected_actual_idx: Option, header: Box, @@ -256,11 +278,25 @@ impl ListSelectionView { Box::new(subtitle), ])); } + let active_tab_idx = params.initial_tab_id.as_ref().and_then(|initial_tab_id| { + params + .tabs + .iter() + .position(|tab| tab.id.as_str() == initial_tab_id.as_str()) + }); + let active_tab_idx = if params.tabs.is_empty() { + None + } else { + Some(active_tab_idx.unwrap_or(0)) + }; + let has_initial_selected_idx = params.initial_selected_idx.is_some(); let mut s = Self { view_id: params.view_id, footer_note: params.footer_note, footer_hint: params.footer_hint, items: params.items, + tabs: params.tabs, + active_tab_idx, state: ScrollState::new(), completion: None, dismiss_after_child_accept: false, @@ -273,6 +309,8 @@ impl ListSelectionView { None }, col_width_mode: params.col_width_mode, + row_display: params.row_display, + name_column_width: params.name_column_width, filtered_indices: Vec::new(), last_selected_actual_idx: None, header, @@ -286,6 +324,9 @@ impl ListSelectionView { on_cancel: params.on_cancel, }; s.apply_filter(); + if s.tabs_enabled() && !has_initial_selected_idx { + s.select_first_enabled_row(); + } s } @@ -293,6 +334,30 @@ impl ListSelectionView { self.filtered_indices.len() } + fn tabs_enabled(&self) -> bool { + self.active_tab_idx.is_some() + } + + fn active_items(&self) -> &[SelectionItem] { + self.active_tab_idx + .and_then(|idx| self.tabs.get(idx)) + .map(|tab| tab.items.as_slice()) + .unwrap_or(self.items.as_slice()) + } + + fn active_header(&self) -> &dyn Renderable { + self.active_tab_idx + .and_then(|idx| self.tabs.get(idx)) + .map(|tab| tab.header.as_ref()) + .unwrap_or(self.header.as_ref()) + } + + fn active_tab_id(&self) -> Option<&str> { + self.active_tab_idx + .and_then(|idx| self.tabs.get(idx)) + .map(|tab| tab.id.as_str()) + } + fn max_visible_rows(len: usize) -> usize { MAX_POPUP_ROWS.min(len.max(1)) } @@ -308,7 +373,7 @@ impl ListSelectionView { .selected_actual_idx() .or_else(|| { (!self.is_searchable) - .then(|| self.items.iter().position(|item| item.is_current)) + .then(|| self.active_items().iter().position(|item| item.is_current)) .flatten() }) .or_else(|| self.initial_selected_idx.take()); @@ -316,7 +381,7 @@ impl ListSelectionView { if self.is_searchable && !self.search_query.is_empty() { let query_lower = self.search_query.to_lowercase(); self.filtered_indices = self - .items + .active_items() .iter() .positions(|item| { item.search_value @@ -325,7 +390,7 @@ impl ListSelectionView { }) .collect(); } else { - self.filtered_indices = (0..self.items.len()).collect(); + self.filtered_indices = (0..self.active_items().len()).collect(); } let len = self.filtered_indices.len(); @@ -363,7 +428,7 @@ impl ListSelectionView { .iter() .enumerate() .filter_map(|(visible_idx, actual_idx)| { - self.items.get(*actual_idx).map(|item| { + self.active_items().get(*actual_idx).map(|item| { let is_selected = self.state.selected_idx == Some(visible_idx); let prefix = if is_selected { '›' } else { ' ' }; let name = item.name.as_str(); @@ -411,6 +476,42 @@ impl ListSelectionView { .collect() } + fn switch_tab(&mut self, step: isize) { + let Some(active_idx) = self.active_tab_idx else { + return; + }; + let len = self.tabs.len(); + if len == 0 { + return; + } + + let next_idx = if step.is_negative() { + active_idx.checked_sub(1).unwrap_or(len - 1) + } else { + (active_idx + 1) % len + }; + self.active_tab_idx = Some(next_idx); + self.search_query.clear(); + self.state.reset(); + self.apply_filter(); + self.select_first_enabled_row(); + self.fire_selection_changed(); + } + + fn select_first_enabled_row(&mut self) { + let selected_visible_idx = self + .filtered_indices + .iter() + .position(|actual_idx| { + self.active_items() + .get(*actual_idx) + .is_some_and(|item| item.disabled_reason.is_none() && !item.is_disabled) + }) + .or_else(|| (!self.filtered_indices.is_empty()).then_some(0)); + self.state.selected_idx = selected_visible_idx; + self.state.scroll_top = 0; + } + fn move_up(&mut self) { let before = self.selected_actual_idx(); let len = self.visible_len(); @@ -444,20 +545,21 @@ impl ListSelectionView { } fn accept(&mut self) { - let selected_item = self + let selected_actual_idx = self .state .selected_idx - .and_then(|idx| self.filtered_indices.get(idx)) - .and_then(|actual_idx| self.items.get(*actual_idx)); - if let Some(item) = selected_item - && item.disabled_reason.is_none() - && !item.is_disabled - { - if let Some(idx) = self.state.selected_idx - && let Some(actual_idx) = self.filtered_indices.get(idx) - { - self.last_selected_actual_idx = Some(*actual_idx); - } + .and_then(|idx| self.filtered_indices.get(idx).copied()); + let selected_is_enabled = selected_actual_idx + .and_then(|actual_idx| self.active_items().get(actual_idx)) + .is_some_and(|item| item.disabled_reason.is_none() && !item.is_disabled); + if selected_is_enabled { + self.last_selected_actual_idx = selected_actual_idx; + let Some(actual_idx) = selected_actual_idx else { + return; + }; + let Some(item) = self.active_items().get(actual_idx) else { + return; + }; for act in &item.actions { act(&self.app_event_tx); } @@ -466,7 +568,7 @@ impl ListSelectionView { } else if item.dismiss_parent_on_child_accept { self.dismiss_after_child_accept = true; } - } else if selected_item.is_none() { + } else if selected_actual_idx.is_none() { if let Some(cb) = &self.on_cancel { cb(&self.app_event_tx); } @@ -551,7 +653,7 @@ impl ListSelectionView { if let Some(idx) = self.state.selected_idx && let Some(actual_idx) = self.filtered_indices.get(idx) && self - .items + .active_items() .get(*actual_idx) .is_some_and(|item| item.disabled_reason.is_some() || item.is_disabled) { @@ -568,7 +670,7 @@ impl ListSelectionView { if let Some(idx) = self.state.selected_idx && let Some(actual_idx) = self.filtered_indices.get(idx) && self - .items + .active_items() .get(*actual_idx) .is_some_and(|item| item.disabled_reason.is_some() || item.is_disabled) { @@ -599,6 +701,14 @@ impl BottomPaneView for ListSelectionView { modifiers: KeyModifiers::NONE, .. } /* ^P */ => self.move_up(), + KeyEvent { + code: KeyCode::Left, + .. + } if self.tabs_enabled() => self.switch_tab(/*step*/ -1), + KeyEvent { + code: KeyCode::Right, + .. + } if self.tabs_enabled() => self.switch_tab(/*step*/ 1), KeyEvent { code: KeyCode::Char('k'), modifiers: KeyModifiers::NONE, @@ -658,9 +768,9 @@ impl BottomPaneView for ListSelectionView { .to_digit(10) .map(|d| d as usize) .and_then(|d| d.checked_sub(1)) - && idx < self.items.len() + && idx < self.active_items().len() && self - .items + .active_items() .get(idx) .is_some_and(|item| item.disabled_reason.is_none() && !item.is_disabled) { @@ -701,6 +811,10 @@ impl BottomPaneView for ListSelectionView { self.selected_actual_idx() } + fn active_tab_id(&self) -> Option<&str> { + ListSelectionView::active_tab_id(self) + } + fn on_ctrl_c(&mut self) -> CancellationEvent { if let Some(cb) = &self.on_cancel { cb(&self.app_event_tx); @@ -725,29 +839,22 @@ impl Renderable for ListSelectionView { // Measure wrapped height for up to MAX_POPUP_ROWS items. let rows = self.build_rows(); - let rows_height = match self.col_width_mode { - ColumnWidthMode::AutoVisible => measure_rows_height( - &rows, - &self.state, - MAX_POPUP_ROWS, - effective_rows_width.saturating_add(1), - ), - ColumnWidthMode::AutoAllRows => measure_rows_height_stable_col_widths( + let column_width = ColumnWidthConfig::new(self.col_width_mode, self.name_column_width); + let rows_height = match self.row_display { + SelectionRowDisplay::Wrapped => measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, effective_rows_width.saturating_add(1), + column_width, ), - ColumnWidthMode::Fixed => measure_rows_height_with_col_width_mode( - &rows, - &self.state, - MAX_POPUP_ROWS, - effective_rows_width.saturating_add(1), - ColumnWidthMode::Fixed, - ), + SelectionRowDisplay::SingleLine => rows.len().clamp(1, MAX_POPUP_ROWS) as u16, }; - let mut height = self.header.desired_height(inner_width); + let header = self.active_header(); + let tab_height = tab_bar_height(&self.tabs, self.active_tab_idx.unwrap_or(0), inner_width); + let mut height = header.desired_height(inner_width); + height = height.saturating_add(tab_height + u16::from(tab_height > 0)); height = height.saturating_add(rows_height + 3); if self.is_searchable { height = height.saturating_add(1); @@ -806,28 +913,20 @@ impl Renderable for ListSelectionView { full_rows_width }; - let header_height = self.header.desired_height(inner_width); + let header = self.active_header(); + let header_height = header.desired_height(inner_width); + let tab_height = tab_bar_height(&self.tabs, self.active_tab_idx.unwrap_or(0), inner_width); let rows = self.build_rows(); - let rows_height = match self.col_width_mode { - ColumnWidthMode::AutoVisible => measure_rows_height( - &rows, - &self.state, - MAX_POPUP_ROWS, - effective_rows_width.saturating_add(1), - ), - ColumnWidthMode::AutoAllRows => measure_rows_height_stable_col_widths( - &rows, - &self.state, - MAX_POPUP_ROWS, - effective_rows_width.saturating_add(1), - ), - ColumnWidthMode::Fixed => measure_rows_height_with_col_width_mode( + let column_width = ColumnWidthConfig::new(self.col_width_mode, self.name_column_width); + let rows_height = match self.row_display { + SelectionRowDisplay::Wrapped => measure_rows_height_with_col_width_mode( &rows, &self.state, MAX_POPUP_ROWS, effective_rows_width.saturating_add(1), - ColumnWidthMode::Fixed, + column_width, ), + SelectionRowDisplay::SingleLine => rows.len().clamp(1, MAX_POPUP_ROWS) as u16, }; // Stacked (fallback) side content height — only used when not side-by-side. @@ -838,9 +937,20 @@ impl Renderable for ListSelectionView { }; let stacked_gap = if stacked_side_h > 0 { 1 } else { 0 }; - let [header_area, _, search_area, list_area, _, stacked_side_area] = Layout::vertical([ + let [ + header_area, + _, + tabs_area, + _, + search_area, + list_area, + _, + stacked_side_area, + ] = Layout::vertical([ Constraint::Max(header_height), Constraint::Max(1), + Constraint::Length(tab_height), + Constraint::Length(u16::from(tab_height > 0)), Constraint::Length(if self.is_searchable { 1 } else { 0 }), Constraint::Length(rows_height), Constraint::Length(stacked_gap), @@ -852,13 +962,18 @@ impl Renderable for ListSelectionView { if header_area.height < header_height { let [header_area, elision_area] = Layout::vertical([Constraint::Fill(1), Constraint::Length(1)]).areas(header_area); - self.header.render(header_area, buf); + header.render(header_area, buf); Paragraph::new(vec![ Line::from(format!("[… {header_height} lines] ctrl + a view all")).dim(), ]) .render(elision_area, buf); } else { - self.header.render(header_area, buf); + header.render(header_area, buf); + } + + // -- Tabs -- + if tab_height > 0 { + render_tab_bar(&self.tabs, self.active_tab_idx.unwrap_or(0), tabs_area, buf); } // -- Search bar -- @@ -883,31 +998,24 @@ impl Renderable for ListSelectionView { width: effective_rows_width.max(1), height: list_area.height, }; - match self.col_width_mode { - ColumnWidthMode::AutoVisible => render_rows( + match self.row_display { + SelectionRowDisplay::Wrapped => render_rows_with_col_width_mode( render_area, buf, &rows, &self.state, render_area.height as usize, "no matches", + column_width, ), - ColumnWidthMode::AutoAllRows => render_rows_stable_col_widths( + SelectionRowDisplay::SingleLine => render_rows_single_line_with_col_width_mode( render_area, buf, &rows, &self.state, render_area.height as usize, "no matches", - ), - ColumnWidthMode::Fixed => render_rows_with_col_width_mode( - render_area, - buf, - &rows, - &self.state, - render_area.height as usize, - "no matches", - ColumnWidthMode::Fixed, + column_width, ), }; } @@ -1320,6 +1428,171 @@ mod tests { ); } + #[test] + fn switching_tabs_changes_visible_items_and_clears_search() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = ListSelectionView::new( + SelectionViewParams { + tabs: vec![ + SelectionTab { + id: "alpha".to_string(), + label: "Alpha".to_string(), + header: Box::new(()), + items: vec![SelectionItem { + name: "Alpha Item".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + }, + SelectionTab { + id: "beta".to_string(), + label: "Beta".to_string(), + header: Box::new(()), + items: vec![SelectionItem { + name: "Beta Item".to_string(), + dismiss_on_select: true, + ..Default::default() + }], + }, + ], + initial_tab_id: Some("beta".to_string()), + is_searchable: true, + ..Default::default() + }, + tx, + ); + view.set_search_query("beta".to_string()); + + view.handle_key_event(KeyEvent::from(KeyCode::Left)); + + assert_eq!(view.active_tab_id(), Some("alpha")); + assert_eq!(view.search_query, ""); + let rendered = render_lines(&view); + assert!( + rendered.contains("Alpha Item") && !rendered.contains("Beta Item"), + "expected switched tab to render the alpha items, got:\n{rendered}" + ); + } + + #[test] + fn single_line_row_display_truncates_instead_of_wrapping() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let single_line_view = ListSelectionView::new( + SelectionViewParams { + title: Some("Debug".to_string()), + items: vec![SelectionItem { + name: "A very long plugin name".to_string(), + description: Some( + "A very long description that would normally wrap onto another line." + .to_string(), + ), + dismiss_on_select: true, + ..Default::default() + }], + row_display: SelectionRowDisplay::SingleLine, + ..Default::default() + }, + tx, + ); + let (wrapped_tx_raw, _wrapped_rx) = unbounded_channel::(); + let wrapped_tx = AppEventSender::new(wrapped_tx_raw); + let wrapped_view = ListSelectionView::new( + SelectionViewParams { + title: Some("Debug".to_string()), + items: vec![SelectionItem { + name: "A very long plugin name".to_string(), + description: Some( + "A very long description that would normally wrap onto another line." + .to_string(), + ), + dismiss_on_select: true, + ..Default::default() + }], + ..Default::default() + }, + wrapped_tx, + ); + + let rendered = render_lines_with_width(&single_line_view, /*width*/ 36); + assert!( + rendered.contains("…"), + "expected single-line rendering to truncate with an ellipsis, got:\n{rendered}" + ); + assert!( + single_line_view.desired_height(/*width*/ 36) + < wrapped_view.desired_height(/*width*/ 36), + "expected single-line rendering to reserve less height than wrapped rendering:\nsingle-line:\n{rendered}\n\nwrapped:\n{}", + render_lines_with_width(&wrapped_view, /*width*/ 36) + ); + } + + #[test] + fn name_column_width_override_moves_description_column_right() { + let auto_items = vec![ + SelectionItem { + name: "Short".to_string(), + description: Some("desc".to_string()), + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Longer".to_string(), + description: Some("desc".to_string()), + dismiss_on_select: true, + ..Default::default() + }, + ]; + let widened_items = vec![ + SelectionItem { + name: "Short".to_string(), + description: Some("desc".to_string()), + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Longer".to_string(), + description: Some("desc".to_string()), + dismiss_on_select: true, + ..Default::default() + }, + ]; + let (auto_tx_raw, _auto_rx) = unbounded_channel::(); + let auto_tx = AppEventSender::new(auto_tx_raw); + let auto_view = ListSelectionView::new( + SelectionViewParams { + items: auto_items, + row_display: SelectionRowDisplay::SingleLine, + col_width_mode: ColumnWidthMode::AutoVisible, + ..Default::default() + }, + auto_tx, + ); + let (widened_tx_raw, _widened_rx) = unbounded_channel::(); + let widened_tx = AppEventSender::new(widened_tx_raw); + let widened_view = ListSelectionView::new( + SelectionViewParams { + items: widened_items, + row_display: SelectionRowDisplay::SingleLine, + col_width_mode: ColumnWidthMode::AutoVisible, + name_column_width: Some(18), + ..Default::default() + }, + widened_tx, + ); + + let auto_rendered = render_lines_with_width(&auto_view, /*width*/ 48); + let widened_rendered = render_lines_with_width(&widened_view, /*width*/ 48); + let auto_col = description_col(&auto_rendered, "1. Short", "desc"); + let widened_col = description_col(&widened_rendered, "1. Short", "desc"); + + assert!( + widened_col > auto_col, + "expected name column override to push the description right:\nauto:\n{auto_rendered}\n\nwidened:\n{widened_rendered}" + ); + } + #[test] fn enter_with_no_matches_triggers_cancel_callback() { let (tx_raw, mut rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 2600a90cd9c2..12ec9e594c68 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -90,6 +90,7 @@ mod skills_toggle_view; mod slash_commands; pub(crate) use footer::CollaborationModeIndicator; pub(crate) use list_selection_view::ColumnWidthMode; +pub(crate) use list_selection_view::SelectionRowDisplay; pub(crate) use list_selection_view::SelectionViewParams; pub(crate) use list_selection_view::SideContentWidth; pub(crate) use list_selection_view::popup_content_width; @@ -115,6 +116,7 @@ mod pending_thread_approvals; pub(crate) mod popup_consts; mod scroll_state; mod selection_popup_common; +mod selection_tabs; mod textarea; mod unified_exec_footer; pub(crate) use feedback_view::FeedbackNoteView; @@ -852,6 +854,14 @@ impl BottomPane { .and_then(|view| view.selected_index()) } + #[cfg_attr(not(test), allow(dead_code))] + pub(crate) fn active_tab_id_for_active_view(&self, view_id: &'static str) -> Option<&str> { + self.view_stack + .last() + .filter(|view| view.view_id() == Some(view_id)) + .and_then(|view| view.active_tab_id()) + } + /// Update the pending-input preview shown above the composer. pub(crate) fn set_pending_input_preview( &mut self, 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 cff3a34f727d..3507cb31eca5 100644 --- a/codex-rs/tui/src/bottom_pane/selection_popup_common.rs +++ b/codex-rs/tui/src/bottom_pane/selection_popup_common.rs @@ -55,6 +55,22 @@ pub(crate) enum ColumnWidthMode { Fixed, } +/// Column-width behavior plus an optional shared left-column width override. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct ColumnWidthConfig { + pub mode: ColumnWidthMode, + pub name_column_width: Option, +} + +impl ColumnWidthConfig { + pub(crate) const fn new(mode: ColumnWidthMode, name_column_width: Option) -> Self { + Self { + mode, + name_column_width, + } + } +} + // Fixed split used by explicitly fixed column mode: 30% label, 70% // description. const FIXED_LEFT_COLUMN_NUMERATOR: usize = 3; @@ -126,7 +142,7 @@ fn compute_desc_col( start_idx: usize, visible_items: usize, content_width: u16, - col_width_mode: ColumnWidthMode, + column_width: ColumnWidthConfig, ) -> usize { if content_width <= 1 { return 0; @@ -141,12 +157,12 @@ fn compute_desc_col( / FIXED_LEFT_COLUMN_DENOMINATOR) .max(1), ); - match col_width_mode { + match column_width.mode { ColumnWidthMode::Fixed => ((content_width as usize * FIXED_LEFT_COLUMN_NUMERATOR) / FIXED_LEFT_COLUMN_DENOMINATOR) .clamp(1, max_desc_col), ColumnWidthMode::AutoVisible | ColumnWidthMode::AutoAllRows => { - let max_name_width = match col_width_mode { + let max_name_width = match column_width.mode { ColumnWidthMode::AutoVisible => rows_all .iter() .enumerate() @@ -177,7 +193,12 @@ fn compute_desc_col( ColumnWidthMode::Fixed => 0, }; - max_name_width.saturating_add(2).min(max_auto_desc_col) + column_width + .name_column_width + .map(|width| width.max(max_name_width)) + .unwrap_or(max_name_width) + .saturating_add(2) + .min(max_auto_desc_col) } } } @@ -373,7 +394,7 @@ fn adjust_start_for_wrapped_selection_visibility( desc_measure_items: usize, width: u16, viewport_height: u16, - col_width_mode: ColumnWidthMode, + column_width: ColumnWidthConfig, ) -> usize { let mut start_idx = compute_item_window_start(rows_all, state, max_items); let Some(sel) = state.selected_idx else { @@ -386,13 +407,8 @@ fn adjust_start_for_wrapped_selection_visibility( // If wrapped row heights push the selected item out of view, advance the // item window until the selected row is visible. while start_idx < sel { - let desc_col = compute_desc_col( - rows_all, - start_idx, - desc_measure_items, - width, - col_width_mode, - ); + let desc_col = + compute_desc_col(rows_all, start_idx, desc_measure_items, width, column_width); if is_selected_visible_in_wrapped_viewport( rows_all, start_idx, @@ -506,7 +522,7 @@ fn render_rows_inner( state: &ScrollState, max_results: usize, empty_message: &str, - col_width_mode: ColumnWidthMode, + column_width: ColumnWidthConfig, ) -> u16 { if rows_all.is_empty() { if area.height > 0 { @@ -531,7 +547,7 @@ fn render_rows_inner( desc_measure_items, area.width, area.height, - col_width_mode, + column_width, ); let desc_col = compute_desc_col( @@ -539,7 +555,7 @@ fn render_rows_inner( start_idx, desc_measure_items, area.width, - col_width_mode, + column_width, ); // Render items, wrapping descriptions and aligning wrapped lines under the @@ -603,26 +619,24 @@ pub(crate) fn render_rows( state, max_results, empty_message, - ColumnWidthMode::AutoVisible, + ColumnWidthConfig::default(), ) } -/// Render a list of rows using the provided ScrollState, with shared styling -/// and behavior for selection popups. -/// This mode keeps column placement stable while scrolling by sizing the -/// description column against the full dataset. +/// Render a list of rows using the provided ScrollState and explicit +/// [`ColumnWidthMode`] behavior. /// -/// This function should be paired with -/// [`measure_rows_height_stable_col_widths`] so reserved and rendered heights -/// stay in sync. +/// This is the low-level entry point for callers that need to thread a mode +/// through higher-level configuration. /// Returns the number of terminal lines actually rendered. -pub(crate) fn render_rows_stable_col_widths( +pub(crate) fn render_rows_with_col_width_mode( area: Rect, buf: &mut Buffer, rows_all: &[GenericDisplayRow], state: &ScrollState, max_results: usize, empty_message: &str, + column_width: ColumnWidthConfig, ) -> u16 { render_rows_inner( area, @@ -631,48 +645,44 @@ pub(crate) fn render_rows_stable_col_widths( state, max_results, empty_message, - ColumnWidthMode::AutoAllRows, + column_width, ) } -/// Render a list of rows using the provided ScrollState and explicit -/// [`ColumnWidthMode`] behavior. +/// Render rows as a single line each (no wrapping), truncating overflow with an ellipsis. /// -/// This is the low-level entry point for callers that need to thread a mode -/// through higher-level configuration. +/// This path always uses viewport-local width alignment and is best for dense +/// list UIs where multi-line descriptions would add too much vertical churn. /// Returns the number of terminal lines actually rendered. -pub(crate) fn render_rows_with_col_width_mode( +pub(crate) fn render_rows_single_line( area: Rect, buf: &mut Buffer, rows_all: &[GenericDisplayRow], state: &ScrollState, max_results: usize, empty_message: &str, - col_width_mode: ColumnWidthMode, ) -> u16 { - render_rows_inner( + render_rows_single_line_with_col_width_mode( area, buf, rows_all, state, max_results, empty_message, - col_width_mode, + ColumnWidthConfig::default(), ) } -/// Render rows as a single line each (no wrapping), truncating overflow with an ellipsis. -/// -/// This path always uses viewport-local width alignment and is best for dense -/// list UIs where multi-line descriptions would add too much vertical churn. -/// Returns the number of terminal lines actually rendered. -pub(crate) fn render_rows_single_line( +/// Render a list of rows as a single line each (no wrapping), truncating overflow with an +/// ellipsis while honoring the configured column width behavior. +pub(crate) fn render_rows_single_line_with_col_width_mode( area: Rect, buf: &mut Buffer, rows_all: &[GenericDisplayRow], state: &ScrollState, max_results: usize, empty_message: &str, + column_width: ColumnWidthConfig, ) -> u16 { if rows_all.is_empty() { if area.height > 0 { @@ -698,13 +708,7 @@ pub(crate) fn render_rows_single_line( } } - let desc_col = compute_desc_col( - rows_all, - start_idx, - visible_items, - area.width, - ColumnWidthMode::AutoVisible, - ); + let desc_col = compute_desc_col(rows_all, start_idx, visible_items, area.width, column_width); let mut cur_y = area.y; let mut rendered_lines: u16 = 0; @@ -766,25 +770,7 @@ pub(crate) fn measure_rows_height( state, max_results, width, - ColumnWidthMode::AutoVisible, - ) -} - -/// Measures selection-row height while using full-dataset column alignment. -/// This should be paired with [`render_rows_stable_col_widths`] so layout -/// reservation matches rendering behavior. -pub(crate) fn measure_rows_height_stable_col_widths( - rows_all: &[GenericDisplayRow], - state: &ScrollState, - max_results: usize, - width: u16, -) -> u16 { - measure_rows_height_inner( - rows_all, - state, - max_results, - width, - ColumnWidthMode::AutoAllRows, + ColumnWidthConfig::default(), ) } @@ -796,9 +782,9 @@ pub(crate) fn measure_rows_height_with_col_width_mode( state: &ScrollState, max_results: usize, width: u16, - col_width_mode: ColumnWidthMode, + column_width: ColumnWidthConfig, ) -> u16 { - measure_rows_height_inner(rows_all, state, max_results, width, col_width_mode) + measure_rows_height_inner(rows_all, state, max_results, width, column_width) } fn measure_rows_height_inner( @@ -806,7 +792,7 @@ fn measure_rows_height_inner( state: &ScrollState, max_results: usize, width: u16, - col_width_mode: ColumnWidthMode, + column_width: ColumnWidthConfig, ) -> u16 { if rows_all.is_empty() { return 1; // placeholder "no matches" line @@ -832,7 +818,7 @@ fn measure_rows_height_inner( start_idx, visible_items, content_width, - col_width_mode, + column_width, ); let mut total: u16 = 0; diff --git a/codex-rs/tui/src/bottom_pane/selection_tabs.rs b/codex-rs/tui/src/bottom_pane/selection_tabs.rs new file mode 100644 index 000000000000..8d5612b5aba2 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/selection_tabs.rs @@ -0,0 +1,103 @@ +use ratatui::buffer::Buffer; +use ratatui::layout::Rect; +use ratatui::style::Stylize; +use ratatui::text::Line; +use ratatui::text::Span; +use ratatui::widgets::Widget; + +use crate::render::renderable::Renderable; + +use super::SelectionItem; + +const TAB_GAP_WIDTH: usize = 2; + +pub(crate) struct SelectionTab { + pub(crate) id: String, + pub(crate) label: String, + pub(crate) header: Box, + pub(crate) items: Vec, +} + +pub(crate) fn tab_bar_height(tabs: &[SelectionTab], active_idx: usize, width: u16) -> u16 { + if tabs.is_empty() { + return 0; + } + tab_bar_lines(tabs, active_idx, width) + .len() + .try_into() + .unwrap_or(u16::MAX) +} + +pub(crate) fn render_tab_bar( + tabs: &[SelectionTab], + active_idx: usize, + area: Rect, + buf: &mut Buffer, +) { + for (offset, line) in tab_bar_lines(tabs, active_idx, area.width) + .into_iter() + .take(area.height as usize) + .enumerate() + { + line.render( + Rect { + x: area.x, + y: area.y.saturating_add(offset as u16), + width: area.width, + height: 1, + }, + buf, + ); + } +} + +fn tab_bar_lines(tabs: &[SelectionTab], active_idx: usize, width: u16) -> Vec> { + if tabs.is_empty() { + return Vec::new(); + } + + let max_width = width.max(1) as usize; + let mut lines = Vec::new(); + let mut current_spans: Vec> = Vec::new(); + let mut current_width = 0usize; + + for (idx, tab) in tabs.iter().enumerate() { + let unit = tab_unit(tab.label.as_str(), idx == active_idx); + let unit_width = Line::from(unit.clone()).width(); + let gap_width = if current_spans.is_empty() { + 0 + } else { + TAB_GAP_WIDTH + }; + + if !current_spans.is_empty() && current_width + gap_width + unit_width > max_width { + lines.push(Line::from(current_spans)); + current_spans = Vec::new(); + current_width = 0; + } + + if !current_spans.is_empty() { + current_spans.push(" ".into()); + current_width += TAB_GAP_WIDTH; + } + current_width += unit_width; + current_spans.extend(unit); + } + + if !current_spans.is_empty() { + lines.push(Line::from(current_spans)); + } + lines +} + +fn tab_unit(label: &str, active: bool) -> Vec> { + if active { + vec![ + "[".cyan().bold(), + label.to_string().cyan().bold(), + "]".cyan().bold(), + ] + } else { + vec![label.to_string().dim()] + } +} diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index 9b3f91b202ce..c1652fa61383 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -6,6 +6,7 @@ use super::ChatWidget; use crate::app_event::AppEvent; use crate::bottom_pane::ColumnWidthMode; use crate::bottom_pane::SelectionItem; +use crate::bottom_pane::SelectionRowDisplay; use crate::bottom_pane::SelectionViewParams; use crate::history_cell; use crate::onboarding::mark_url_hyperlink; @@ -31,8 +32,10 @@ use ratatui::text::Line; use ratatui::widgets::Paragraph; use ratatui::widgets::WidgetRef; use ratatui::widgets::Wrap; +use unicode_width::UnicodeWidthStr; const PLUGINS_SELECTION_VIEW_ID: &str = "plugins-selection"; +const PLUGIN_ROW_PREFIX_WIDTH: usize = 2; const LOADING_ANIMATION_DELAY: Duration = Duration::from_secs(1); const LOADING_ANIMATION_INTERVAL: Duration = Duration::from_millis(100); @@ -756,6 +759,12 @@ impl ChatWidget { .map(|(_, plugin, _)| plugin_status_label(plugin).chars().count()) .max() .unwrap_or(0); + let name_column_width = plugin_entries + .iter() + .map(|(_, _, display_name)| { + PLUGIN_ROW_PREFIX_WIDTH + UnicodeWidthStr::width(display_name.as_str()) + }) + .max(); let mut items: Vec = Vec::new(); for (marketplace, plugin, display_name) in plugin_entries { @@ -815,6 +824,8 @@ impl ChatWidget { is_searchable: true, search_placeholder: Some("Type to search plugins".to_string()), col_width_mode: ColumnWidthMode::AutoAllRows, + row_display: SelectionRowDisplay::SingleLine, + name_column_width, ..Default::default() } } From de2368643f4a7303f0b08c61abc021666de17017 Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Thu, 16 Apr 2026 12:50:37 -0700 Subject: [PATCH 2/3] tui: simplify disabled plugin labels and silence dead code Co-authored-by: Codex --- codex-rs/tui/src/bottom_pane/bottom_pane_view.rs | 1 + codex-rs/tui/src/bottom_pane/mod.rs | 2 +- codex-rs/tui/src/chatwidget/plugins.rs | 4 ++-- ..._chatwidget__tests__plugins_popup_curated_marketplace.snap | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs index 207c4386f6ea..bb168d3db188 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -50,6 +50,7 @@ pub(crate) trait BottomPaneView: Renderable { } /// Active tab id for tabbed list-based views. + #[allow(dead_code)] fn active_tab_id(&self) -> Option<&str> { None } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 12ec9e594c68..aee3b2bed605 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -854,7 +854,7 @@ impl BottomPane { .and_then(|view| view.selected_index()) } - #[cfg_attr(not(test), allow(dead_code))] + #[allow(dead_code)] pub(crate) fn active_tab_id_for_active_view(&self, view_id: &'static str) -> Option<&str> { self.view_stack .last() diff --git a/codex-rs/tui/src/chatwidget/plugins.rs b/codex-rs/tui/src/chatwidget/plugins.rs index c1652fa61383..9b8f53ba6c81 100644 --- a/codex-rs/tui/src/chatwidget/plugins.rs +++ b/codex-rs/tui/src/chatwidget/plugins.rs @@ -841,7 +841,7 @@ impl ChatWidget { if plugin.summary.enabled { "Installed" } else { - "Installed · Disabled" + "Disabled" } } else { match plugin.summary.install_policy { @@ -1014,7 +1014,7 @@ fn plugin_status_label(plugin: &PluginSummary) -> &'static str { if plugin.enabled { "Installed" } else { - "Installed · Disabled" + "Disabled" } } else { match plugin.install_policy { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap index 3c46da769c2a..ab8578251092 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_curated_marketplace.snap @@ -8,7 +8,7 @@ expression: popup Using cached marketplace data: remote sync timed out Type to search plugins -› Alpha Sync Installed · Disabled Press Enter to view plugin details. +› Alpha Sync Disabled Press Enter to view plugin details. Bravo Search Available · ChatGPT Marketplace · Search docs and tickets. Hidden Repo Plugin Available · Repo Marketplace · Should not be shown in /plugins. Starter Available by default · ChatGPT Marketplace · Included by default. From 0d1d5804ddede4e9e4774ce8bcaad3d47baf4cec Mon Sep 17 00:00:00 2001 From: canvrno-oai Date: Thu, 16 Apr 2026 13:04:33 -0700 Subject: [PATCH 3/3] snap update and preserve row selection --- .../src/bottom_pane/list_selection_view.rs | 65 ++++++++++++++++++- ..._tests__plugins_popup_search_filtered.snap | 2 +- 2 files changed, 64 insertions(+), 3 deletions(-) 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 e11bb9539091..eecf1bc580f2 100644 --- a/codex-rs/tui/src/bottom_pane/list_selection_view.rs +++ b/codex-rs/tui/src/bottom_pane/list_selection_view.rs @@ -324,7 +324,7 @@ impl ListSelectionView { on_cancel: params.on_cancel, }; s.apply_filter(); - if s.tabs_enabled() && !has_initial_selected_idx { + if s.tabs_enabled() && !has_initial_selected_idx && s.state.selected_idx.is_none() { s.select_first_enabled_row(); } s @@ -494,7 +494,9 @@ impl ListSelectionView { self.search_query.clear(); self.state.reset(); self.apply_filter(); - self.select_first_enabled_row(); + if self.state.selected_idx.is_none() { + self.select_first_enabled_row(); + } self.fire_selection_changed(); } @@ -1475,6 +1477,65 @@ mod tests { ); } + #[test] + fn tabbed_view_preserves_current_row_on_initial_selection_and_tab_switch() { + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut view = ListSelectionView::new( + SelectionViewParams { + tabs: vec![ + SelectionTab { + id: "alpha".to_string(), + label: "Alpha".to_string(), + header: Box::new(()), + items: vec![ + SelectionItem { + name: "Alpha First".to_string(), + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Alpha Current".to_string(), + is_current: true, + dismiss_on_select: true, + ..Default::default() + }, + ], + }, + SelectionTab { + id: "beta".to_string(), + label: "Beta".to_string(), + header: Box::new(()), + items: vec![ + SelectionItem { + name: "Beta First".to_string(), + dismiss_on_select: true, + ..Default::default() + }, + SelectionItem { + name: "Beta Current".to_string(), + is_current: true, + dismiss_on_select: true, + ..Default::default() + }, + ], + }, + ], + initial_tab_id: Some("beta".to_string()), + ..Default::default() + }, + tx, + ); + + assert_eq!(view.active_tab_id(), Some("beta")); + assert_eq!(view.selected_actual_idx(), Some(1)); + + view.handle_key_event(KeyEvent::from(KeyCode::Left)); + + assert_eq!(view.active_tab_id(), Some("alpha")); + assert_eq!(view.selected_actual_idx(), Some(1)); + } + #[test] fn single_line_row_display_truncates_instead_of_wrapping() { let (tx_raw, _rx) = unbounded_channel::(); diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_search_filtered.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_search_filtered.snap index d65308fb5ba4..3389ee5b8549 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_search_filtered.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__plugins_popup_search_filtered.snap @@ -7,6 +7,6 @@ expression: popup Installed 0 of 3 available plugins. sla -› Slack Available Press Enter to view plugin details. +› Slack Available Press Enter to view plugin details. Press esc to close.