mirrors/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-30 19:32:04 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix jitter in TUI apps/connectors picker (#10593)

Browse Source 
This PR fixes jitter in the TUI apps menu by making the description
column stable during rendering and height measurement.
Added a `stable_desc_col` option to
`SelectionViewParams`/`ListSelectionView`, introduced stable variants of
the shared row render/measure helpers in `selection_popup_common`, and
enabled the stable mode for the apps/connectors picker in `chatwidget`.
With these changes, only the apps/connectors picker uses this new
option, though it could be used elsewhere in the future.

Why: previously, the description column was computed from only currently
visible rows, so as you scrolled or filtered, the column could shift and
cause wrapping/height changes that looked jumpy. Computing it from all
rows in this popup keeps alignment and layout consistent as users scroll
through avaialble apps.



**Before:**

https://github.com/user-attachments/assets/3856cb72-5465-4b90-a993-65a2ffb09113





**After:**

https://github.com/user-attachments/assets/37b9d626-0b21-4c0f-8bb8-244c9ef971ff
This commit is contained in:
canvrno-oai
2026-02-04 13:51:31 -08:00
committed by GitHub
parent 4922b3e571
commit d589ee05b1
7 changed files with 577 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files

259
codex-rs/tui/src/bottom_pane/selection_popup_common.rs
View File

@@ -19,7 +19,11 @@ use crate::style::user_message_style;
use super::scroll_state::ScrollState;
/// A generic representation of a display row for selection popups.
/// Render-ready representation of one row in a selection popup.
///
/// This type contains presentation-focused fields that are intentionally more
/// concrete than source domain models. `match_indices` are character offsets
/// into `name`, and `wrap_indent` is interpreted in terminal cell columns.
#[derive(Default)]
pub(crate) struct GenericDisplayRow {
pub name: String,
@@ -31,6 +35,25 @@ pub(crate) struct GenericDisplayRow {
pub wrap_indent: Option<usize>, // optional indent for wrapped lines
}
/// Controls how selection rows choose the split between left/right name/description columns.
///
/// Callers should use the same mode for both measurement and rendering, or the
/// popup can reserve the wrong number of lines and clip content.
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
#[cfg_attr(not(test), allow(dead_code))]
pub(crate) enum ColumnWidthMode {
/// Derive column placement from only the visible viewport rows.
#[default]
AutoVisible,
/// Derive column placement from all rows so scrolling does not shift columns.
AutoAllRows,
/// Use a fixed two-column split: 30% left (name), 70% right (description).
Fixed,
}
const FIXED_LEFT_COLUMN_NUMERATOR: usize = 3;
const FIXED_LEFT_COLUMN_DENOMINATOR: usize = 10;
const MENU_SURFACE_INSET_V: u16 = 1;
const MENU_SURFACE_INSET_H: u16 = 2;
@@ -50,7 +73,8 @@ pub(crate) const fn menu_surface_padding_height() -> u16 {
/// Paint the shared menu background and return the inset content area.
///
/// This keeps the surface treatment consistent across selection-style overlays
/// (for example `/model`, approvals, and request-user-input).
/// (for example `/model`, approvals, and request-user-input). Callers should
/// render all inner content in the returned rect, not the original area.
pub(crate) fn render_menu_surface(area: Rect, buf: &mut Buffer) -> Rect {
if area.is_empty() {
return area;
@@ -61,6 +85,10 @@ pub(crate) fn render_menu_surface(area: Rect, buf: &mut Buffer) -> Rect {
menu_surface_inset(area)
}
/// Wrap a styled line while preserving span styles.
///
/// The function clamps `width` to at least one terminal cell so callers can use
/// it safely with narrow layouts.
pub(crate) fn wrap_styled_line<'a>(line: &'a Line<'a>, width: u16) -> Vec<Line<'a>> {
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;
@@ -143,34 +171,60 @@ fn truncate_line_with_ellipsis_if_overflow(line: Line<'static>, max_width: usize
Line::from(spans)
}
/// Compute a shared description-column start based on the widest visible name
/// plus two spaces of padding. Ensures at least one column is left for the
/// description.
/// Computes the shared start column used for descriptions in selection rows.
/// The column is derived from the widest row name plus two spaces of padding
/// while always leaving at least one terminal cell for description content.
/// [`ColumnWidthMode::AutoAllRows`] computes width across the full dataset so
/// the description column does not shift as the user scrolls.
fn compute_desc_col(
rows_all: &[GenericDisplayRow],
start_idx: usize,
visible_items: usize,
content_width: u16,
col_width_mode: ColumnWidthMode,
) -> usize {
let visible_range = start_idx..(start_idx + visible_items);
let max_name_width = rows_all
.iter()
.enumerate()
.filter(|(i, _)| visible_range.contains(i))
.map(|(_, r)| {
let mut spans: Vec<Span> = vec![r.name.clone().into()];
if r.disabled_reason.is_some() {
spans.push(" (disabled)".dim());
}
Line::from(spans).width()
})
.max()
.unwrap_or(0);
let mut desc_col = max_name_width.saturating_add(2);
if (desc_col as u16) >= content_width {
desc_col = content_width.saturating_sub(1) as usize;
if content_width <= 1 {
return 0;
}
let max_desc_col = content_width.saturating_sub(1) as usize;
match col_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 {
ColumnWidthMode::AutoVisible => rows_all
.iter()
.enumerate()
.skip(start_idx)
.take(visible_items)
.map(|(_, row)| {
let mut spans: Vec<Span> = vec![row.name.clone().into()];
if row.disabled_reason.is_some() {
spans.push(" (disabled)".dim());
}
Line::from(spans).width()
})
.max()
.unwrap_or(0),
ColumnWidthMode::AutoAllRows => rows_all
.iter()
.map(|row| {
let mut spans: Vec<Span> = vec![row.name.clone().into()];
if row.disabled_reason.is_some() {
spans.push(" (disabled)".dim());
}
Line::from(spans).width()
})
.max()
.unwrap_or(0),
ColumnWidthMode::Fixed => 0,
};
max_name_width.saturating_add(2).min(max_desc_col)
}
}
desc_col
}
/// Determine how many spaces to indent wrapped lines for a row.
@@ -268,13 +322,14 @@ fn build_full_line(row: &GenericDisplayRow, desc_col: usize) -> Line<'static> {
/// Render a list of rows using the provided ScrollState, with shared styling
/// and behavior for selection popups.
pub(crate) fn render_rows(
fn render_rows_inner(
area: Rect,
buf: &mut Buffer,
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
empty_message: &str,
col_width_mode: ColumnWidthMode,
) {
if rows_all.is_empty() {
if area.height > 0 {
@@ -301,7 +356,13 @@ pub(crate) fn render_rows(
}
}
let desc_col = compute_desc_col(rows_all, start_idx, visible_items, area.width);
let desc_col = compute_desc_col(
rows_all,
start_idx,
visible_items,
area.width,
col_width_mode,
);
// Render items, wrapping descriptions and aligning wrapped lines under the
// shared description column. Stop when we run out of vertical space.
@@ -358,7 +419,88 @@ pub(crate) fn render_rows(
}
}
/// Render a list of rows using the provided ScrollState, with shared styling
/// and behavior for selection popups.
/// Description alignment is computed from visible rows only, which allows the
/// layout to adapt tightly to the current viewport.
///
/// This function should be paired with [`measure_rows_height`] when reserving
/// space; pairing it with a different measurement mode can cause clipping.
pub(crate) fn render_rows(
area: Rect,
buf: &mut Buffer,
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
empty_message: &str,
) {
render_rows_inner(
area,
buf,
rows_all,
state,
max_results,
empty_message,
ColumnWidthMode::AutoVisible,
);
}
/// 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.
///
/// This function should be paired with
/// [`measure_rows_height_stable_col_widths`] so reserved and rendered heights
/// stay in sync.
pub(crate) fn render_rows_stable_col_widths(
area: Rect,
buf: &mut Buffer,
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
empty_message: &str,
) {
render_rows_inner(
area,
buf,
rows_all,
state,
max_results,
empty_message,
ColumnWidthMode::AutoAllRows,
);
}
/// Render a list of rows using the provided ScrollState and explicit
/// [`ColumnWidthMode`] behavior.
///
/// This is the low-level entry point for callers that need to thread a mode
/// through higher-level configuration.
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,
col_width_mode: ColumnWidthMode,
) {
render_rows_inner(
area,
buf,
rows_all,
state,
max_results,
empty_message,
col_width_mode,
);
}
/// 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.
pub(crate) fn render_rows_single_line(
area: Rect,
buf: &mut Buffer,
@@ -390,7 +532,13 @@ pub(crate) fn render_rows_single_line(
}
}
let desc_col = compute_desc_col(rows_all, start_idx, visible_items, area.width);
let desc_col = compute_desc_col(
rows_all,
start_idx,
visible_items,
area.width,
ColumnWidthMode::AutoVisible,
);
let mut cur_y = area.y;
for (i, row) in rows_all
@@ -433,11 +581,62 @@ pub(crate) fn render_rows_single_line(
/// items from `rows_all` given the current scroll/selection state and the
/// available `width`. Accounts for description wrapping and alignment so the
/// caller can allocate sufficient vertical space.
///
/// This function matches [`render_rows`] semantics (`AutoVisible` column
/// sizing). Mixing it with stable or fixed render modes can under- or
/// over-estimate required height.
pub(crate) fn measure_rows_height(
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
width: u16,
) -> u16 {
measure_rows_height_inner(
rows_all,
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,
)
}
/// Measure selection-row height using explicit [`ColumnWidthMode`] behavior.
///
/// This is the low-level companion to [`render_rows_with_col_width_mode`].
pub(crate) fn measure_rows_height_with_col_width_mode(
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
width: u16,
col_width_mode: ColumnWidthMode,
) -> u16 {
measure_rows_height_inner(rows_all, state, max_results, width, col_width_mode)
}
fn measure_rows_height_inner(
rows_all: &[GenericDisplayRow],
state: &ScrollState,
max_results: usize,
width: u16,
col_width_mode: ColumnWidthMode,
) -> u16 {
if rows_all.is_empty() {
return 1; // placeholder "no matches" line
@@ -458,7 +657,13 @@ pub(crate) fn measure_rows_height(
}
}
let desc_col = compute_desc_col(rows_all, start_idx, visible_items, content_width);
let desc_col = compute_desc_col(
rows_all,
start_idx,
visible_items,
content_width,
col_width_mode,
);
use crate::wrapping::RtOptions;
use crate::wrapping::word_wrap_line;