core/core-agent-ide
8
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix model picker wrapping (#6589)

Browse source 
Previously the popup measured rows using the full content width while
the renderer drew them with 2 columns of padding, so at certain widths
the layout allocated too little vertical space and hid the third option.
Now both desired_height and render call a shared helper that subtracts
the padding before measuring, so the height we reserve always matches
what we draw and the menu doesn't drops entries.


https://github.com/user-attachments/assets/59058fd9-1e34-4325-b5fe-fc888dfcb6bc
This commit is contained in:
Ahmed Ibrahim 2025-11-13 08:09:13 -08:00 • committed by GitHub
parent 2a417c47ac
commit ba74cee6f7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 212 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

193
codex-rs/tui/src/bottom_pane/list_selection_view.rs
View file

@ -250,6 +250,10 @@ impl ListSelectionView {
pub(crate) fn take_last_selected_index(&mut self) -> Option<usize> {
self.last_selected_actual_idx.take()
}
fn rows_width(total_width: u16) -> u16 {
total_width.saturating_sub(2)
}
}
impl BottomPaneView for ListSelectionView {
@ -327,8 +331,13 @@ impl Renderable for ListSelectionView {
// Measure wrapped height for up to MAX_POPUP_ROWS items at the given width.
// Build the same display rows used by the renderer so wrapping math matches.
let rows = self.build_rows();
let rows_height = measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, width);
let rows_width = Self::rows_width(width);
let rows_height = measure_rows_height(
&rows,
&self.state,
MAX_POPUP_ROWS,
rows_width.saturating_add(1),
);
// Subtract 4 for the padding on the left and right of the header.
let mut height = self.header.desired_height(width.saturating_sub(4));
@ -362,8 +371,13 @@ impl Renderable for ListSelectionView {
// Subtract 4 for the padding on the left and right of the header.
.desired_height(content_area.width.saturating_sub(4));
let rows = self.build_rows();
let rows_height =
measure_rows_height(&rows, &self.state, MAX_POPUP_ROWS, content_area.width);
let rows_width = Self::rows_width(content_area.width);
let rows_height = measure_rows_height(
&rows,
&self.state,
MAX_POPUP_ROWS,
rows_width.saturating_add(1),
);
let [header_area, _, search_area, list_area] = Layout::vertical([
Constraint::Max(header_height),
Constraint::Max(1),
@ -398,18 +412,18 @@ impl Renderable for ListSelectionView {
}
if list_area.height > 0 {
let list_area = Rect {
x: list_area.x - 2,
let render_area = Rect {
x: list_area.x.saturating_sub(2),
y: list_area.y,
width: list_area.width + 2,
width: rows_width.max(1),
height: list_area.height,
};
render_rows(
list_area,
render_area,
buf,
&rows,
&self.state,
list_area.height as usize,
render_area.height as usize,
"no matches",
);
}
@ -467,7 +481,10 @@ mod tests {
}
fn render_lines(view: &ListSelectionView) -> String {
let width = 48;
render_lines_with_width(view, 48)
}
fn render_lines_with_width(view: &ListSelectionView, width: u16) -> String {
let height = view.desired_height(width);
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
@ -535,4 +552,160 @@ mod tests {
"expected search query line to include rendered query, got {lines:?}"
);
}
#[test]
fn width_changes_do_not_hide_rows() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let items = vec![
SelectionItem {
name: "gpt-5.1-codex".to_string(),
description: Some(
"Optimized for Codex. Balance of reasoning quality and coding ability."
.to_string(),
),
is_current: true,
dismiss_on_select: true,
..Default::default()
},
SelectionItem {
name: "gpt-5.1-codex-mini".to_string(),
description: Some(
"Optimized for Codex. Cheaper, faster, but less capable.".to_string(),
),
dismiss_on_select: true,
..Default::default()
},
SelectionItem {
name: "gpt-4.1-codex".to_string(),
description: Some(
"Legacy model. Use when you need compatibility with older automations."
.to_string(),
),
dismiss_on_select: true,
..Default::default()
},
];
let view = ListSelectionView::new(
SelectionViewParams {
title: Some("Select Model and Effort".to_string()),
items,
..Default::default()
},
tx,
);
let mut missing: Vec<u16> = Vec::new();
for width in 60..=90 {
let rendered = render_lines_with_width(&view, width);
if !rendered.contains("3.") {
missing.push(width);
}
}
assert!(
missing.is_empty(),
"third option missing at widths {missing:?}"
);
}
#[test]
fn narrow_width_keeps_all_rows_visible() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let desc = "x".repeat(10);
let items: Vec<SelectionItem> = (1..=3)
.map(|idx| SelectionItem {
name: format!("Item {idx}"),
description: Some(desc.clone()),
dismiss_on_select: true,
..Default::default()
})
.collect();
let view = ListSelectionView::new(
SelectionViewParams {
title: Some("Debug".to_string()),
items,
..Default::default()
},
tx,
);
let rendered = render_lines_with_width(&view, 24);
assert!(
rendered.contains("3."),
"third option missing for width 24:\n{rendered}"
);
}
#[test]
fn snapshot_model_picker_width_80() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let items = vec![
SelectionItem {
name: "gpt-5.1-codex".to_string(),
description: Some(
"Optimized for Codex. Balance of reasoning quality and coding ability."
.to_string(),
),
is_current: true,
dismiss_on_select: true,
..Default::default()
},
SelectionItem {
name: "gpt-5.1-codex-mini".to_string(),
description: Some(
"Optimized for Codex. Cheaper, faster, but less capable.".to_string(),
),
dismiss_on_select: true,
..Default::default()
},
SelectionItem {
name: "gpt-4.1-codex".to_string(),
description: Some(
"Legacy model. Use when you need compatibility with older automations."
.to_string(),
),
dismiss_on_select: true,
..Default::default()
},
];
let view = ListSelectionView::new(
SelectionViewParams {
title: Some("Select Model and Effort".to_string()),
items,
..Default::default()
},
tx,
);
assert_snapshot!(
"list_selection_model_picker_width_80",
render_lines_with_width(&view, 80)
);
}
#[test]
fn snapshot_narrow_width_preserves_third_option() {
let (tx_raw, _rx) = unbounded_channel::<AppEvent>();
let tx = AppEventSender::new(tx_raw);
let desc = "x".repeat(10);
let items: Vec<SelectionItem> = (1..=3)
.map(|idx| SelectionItem {
name: format!("Item {idx}"),
description: Some(desc.clone()),
dismiss_on_select: true,
..Default::default()
})
.collect();
let view = ListSelectionView::new(
SelectionViewParams {
title: Some("Debug".to_string()),
items,
..Default::default()
},
tx,
);
assert_snapshot!(
"list_selection_narrow_width_preserves_rows",
render_lines_with_width(&view, 24)
);
}
}

13
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_model_picker_width_80.snap Normal file
View file

@ -0,0 +1,13 @@
---
source: tui/src/bottom_pane/list_selection_view.rs
expression: "render_lines_with_width(&view, 80)"
---
Select Model and Effort
› 1. gpt-5.1-codex (current) Optimized for Codex. Balance of reasoning
quality and coding ability.
2. gpt-5.1-codex-mini Optimized for Codex. Cheaper, faster, but less
capable.
3. gpt-4.1-codex Legacy model. Use when you need compatibility
with older automations.

16
codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__list_selection_view__tests__list_selection_narrow_width_preserves_rows.snap Normal file
View file

@ -0,0 +1,16 @@
---
source: tui/src/bottom_pane/list_selection_view.rs
expression: "render_lines_with_width(&view, 24)"
---
Debug
› 1. Item 1
xxxxxxxxx
x
2. Item 2
xxxxxxxxx
x
3. Item 3
xxxxxxxxx
x