mirror of
https://github.com/openai/codex.git
synced 2026-04-29 08:56:38 +00:00
Ask user question UI footer improvements (#9949)
## Summary Polishes the `request_user_input` TUI overlay Question 1 (unanswered) <img width="853" height="167" alt="Screenshot 2026-01-27 at 1 30 09 PM" src="https://github.com/user-attachments/assets/3c305644-449e-4e8d-a47b-d689ebd8702c" /> Tab to add notes <img width="856" height="198" alt="Screenshot 2026-01-27 at 1 30 25 PM" src="https://github.com/user-attachments/assets/0d2801b0-df0c-49ae-85af-e6d56fc2c67c" /> Question 2 (unanswered) <img width="854" height="168" alt="Screenshot 2026-01-27 at 1 30 55 PM" src="https://github.com/user-attachments/assets/b3723062-51f9-49c9-a9ab-bb1b32964542" /> Ctrl+p or h to go back to q1 (answered) <img width="853" height="195" alt="Screenshot 2026-01-27 at 1 31 27 PM" src="https://github.com/user-attachments/assets/c602f183-1c25-4c51-8f9f-e565cb6bd637" /> Unanswered freeform <img width="856" height="126" alt="Screenshot 2026-01-27 at 1 31 42 PM" src="https://github.com/user-attachments/assets/7e3d9d8b-820b-4b9a-9ef2-4699eed484c5" /> ## Key changes - Footer tips wrap at tip boundaries (no truncation mid‑tip); footer height scales to wrapped tips. - Keep tooltip text as Esc: interrupt in all states. - Make the full Tab: add notes tip cyan/bold when applicable; hide notes UI by default. - Notes toggling/backspace: - Tab opens notes when an option is selected; Tab again clears notes and hides the notes UI. - Backspace in options clears the current selection. - Backspace in empty notes closes notes and returns to options. - Selection/answering behavior: - Option questions highlight a default option but are not answered until Enter. - Enter no longer auto‑selects when there’s no selection (prevents accidental answers). - Notes submission can commit the selected option when present. - Freeform questions require Enter with non‑empty text to mark answered; drafts are not submitted unless committed. - Unanswered cues: - Skipped option questions count as unanswered. - Unanswered question titles are highlighted for visibility. - Typing/navigation in options: - Typing no longer opens notes; notes are Tab‑only. - j/k move option selection; h/l switch questions (Ctrl+n/Ctrl+p still work). ## Tests - Added unit coverage for: - tip‑level wrapping - focus reset when switching questions with existing drafts - backspace clearing selection - backspace closing empty notes - typing in options does not open notes - freeform draft submission gating - h/l question navigation in options - Updated snapshots, including narrow footer wrap. ## Why These changes make the ask‑user‑question overlay: - safer (no silent auto‑selection or accidental freeform submission), - clearer (tips wrap cleanly and unanswered states stand out), - more ergonomic (Tab explicitly controls notes; backspace acts like undo/close). ## Codex author `codex fork 019bfc3c-2c42-7982-9119-fee8b9315c2f` --------- Co-authored-by: Ahmed Ibrahim <aibrahim@openai.com>
This commit is contained in:
committed by
GitHub
parent
3ae966edd8
commit
19d8f71a98
@@ -1,41 +1,58 @@
|
||||
use crossterm::event::KeyCode;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Rect;
|
||||
use ratatui::style::Stylize;
|
||||
use ratatui::text::Line;
|
||||
use ratatui::text::Span;
|
||||
use ratatui::widgets::Paragraph;
|
||||
use ratatui::widgets::Widget;
|
||||
use unicode_width::UnicodeWidthChar;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
use crate::bottom_pane::selection_popup_common::menu_surface_inset;
|
||||
use crate::bottom_pane::selection_popup_common::menu_surface_padding_height;
|
||||
use crate::bottom_pane::selection_popup_common::render_menu_surface;
|
||||
use crate::bottom_pane::selection_popup_common::render_rows;
|
||||
use crate::key_hint;
|
||||
use crate::render::renderable::Renderable;
|
||||
|
||||
use super::DESIRED_SPACERS_WHEN_NOTES_HIDDEN;
|
||||
use super::RequestUserInputOverlay;
|
||||
use super::TIP_SEPARATOR;
|
||||
|
||||
impl Renderable for RequestUserInputOverlay {
|
||||
fn desired_height(&self, width: u16) -> u16 {
|
||||
let outer = Rect::new(0, 0, width, u16::MAX);
|
||||
let inner = menu_surface_inset(outer);
|
||||
let inner_width = inner.width.max(1);
|
||||
let has_options = self.has_options();
|
||||
let question_height = self.wrapped_question_lines(inner_width).len();
|
||||
let options_height = self.options_required_height(inner_width) as usize;
|
||||
let notes_height = self.notes_input_height(inner_width) as usize;
|
||||
let footer_height = if self.unanswered_count() > 0 { 2 } else { 1 };
|
||||
let options_height = if has_options {
|
||||
self.options_preferred_height(inner_width) as usize
|
||||
} else {
|
||||
0
|
||||
};
|
||||
let notes_visible = !has_options || self.notes_ui_visible();
|
||||
let notes_height = if notes_visible {
|
||||
self.notes_input_height(inner_width) as usize
|
||||
} else {
|
||||
0
|
||||
};
|
||||
let spacer_rows = if has_options && !notes_visible {
|
||||
DESIRED_SPACERS_WHEN_NOTES_HIDDEN as usize
|
||||
} else {
|
||||
0
|
||||
};
|
||||
let footer_height = self.footer_required_height(inner_width) as usize;
|
||||
|
||||
// Tight minimum height: progress + header + question + (optional) titles/options
|
||||
// + notes composer + footer + menu padding.
|
||||
let mut height = question_height
|
||||
.saturating_add(options_height)
|
||||
.saturating_add(spacer_rows)
|
||||
.saturating_add(notes_height)
|
||||
.saturating_add(footer_height)
|
||||
.saturating_add(2); // progress + header
|
||||
if self.has_options() {
|
||||
height = height
|
||||
.saturating_add(1) // answer title
|
||||
.saturating_add(1); // notes title
|
||||
if has_options && notes_visible {
|
||||
height = height.saturating_add(1); // notes title
|
||||
}
|
||||
height = height.saturating_add(menu_surface_padding_height() as usize);
|
||||
height.max(8) as u16
|
||||
@@ -63,12 +80,19 @@ impl RequestUserInputOverlay {
|
||||
return;
|
||||
}
|
||||
let sections = self.layout_sections(content_area);
|
||||
let notes_visible = self.notes_ui_visible();
|
||||
let unanswered = self.unanswered_count();
|
||||
|
||||
// Progress header keeps the user oriented across multiple questions.
|
||||
let progress_line = if self.question_count() > 0 {
|
||||
let idx = self.current_index() + 1;
|
||||
let total = self.question_count();
|
||||
Line::from(format!("Question {idx}/{total}").dim())
|
||||
let base = format!("Question {idx}/{total}");
|
||||
if unanswered > 0 {
|
||||
Line::from(format!("{base} ({unanswered} unanswered)").dim())
|
||||
} else {
|
||||
Line::from(base.dim())
|
||||
}
|
||||
} else {
|
||||
Line::from("No questions".dim())
|
||||
};
|
||||
@@ -76,8 +100,13 @@ impl RequestUserInputOverlay {
|
||||
|
||||
// Question title and wrapped prompt text.
|
||||
let question_header = self.current_question().map(|q| q.header.clone());
|
||||
let answered = self.current_question_answered();
|
||||
let header_line = if let Some(header) = question_header {
|
||||
Line::from(header.bold())
|
||||
if answered {
|
||||
Line::from(header.bold())
|
||||
} else {
|
||||
Line::from(header.cyan().bold())
|
||||
}
|
||||
} else {
|
||||
Line::from("No questions".dim())
|
||||
};
|
||||
@@ -101,54 +130,45 @@ impl RequestUserInputOverlay {
|
||||
);
|
||||
}
|
||||
|
||||
if sections.answer_title_area.height > 0 {
|
||||
let answer_label = "Answer";
|
||||
let answer_title = if self.focus_is_options() || self.focus_is_notes_without_options() {
|
||||
answer_label.cyan().bold()
|
||||
} else {
|
||||
answer_label.dim()
|
||||
};
|
||||
Paragraph::new(Line::from(answer_title)).render(sections.answer_title_area, buf);
|
||||
}
|
||||
|
||||
// Build rows with selection markers for the shared selection renderer.
|
||||
let option_rows = self.option_rows();
|
||||
|
||||
if self.has_options() {
|
||||
let mut option_state = self
|
||||
let mut options_ui_state = self
|
||||
.current_answer()
|
||||
.map(|answer| answer.option_state)
|
||||
.map(|answer| answer.options_ui_state)
|
||||
.unwrap_or_default();
|
||||
if sections.options_area.height > 0 {
|
||||
// Ensure the selected option is visible in the scroll window.
|
||||
option_state
|
||||
options_ui_state
|
||||
.ensure_visible(option_rows.len(), sections.options_area.height as usize);
|
||||
render_rows(
|
||||
sections.options_area,
|
||||
buf,
|
||||
&option_rows,
|
||||
&option_state,
|
||||
&options_ui_state,
|
||||
option_rows.len().max(1),
|
||||
"No options",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
if sections.notes_title_area.height > 0 {
|
||||
if notes_visible && sections.notes_title_area.height > 0 {
|
||||
let notes_label = if self.has_options()
|
||||
&& self
|
||||
.current_answer()
|
||||
.is_some_and(|answer| answer.selected.is_some())
|
||||
.is_some_and(|answer| answer.committed_option_idx.is_some())
|
||||
{
|
||||
if let Some(label) = self.current_option_label() {
|
||||
format!("Notes for {label} (optional)")
|
||||
format!("Notes for {label}")
|
||||
} else {
|
||||
"Notes (optional)".to_string()
|
||||
"Notes".to_string()
|
||||
}
|
||||
} else {
|
||||
"Notes (optional)".to_string()
|
||||
"Notes".to_string()
|
||||
};
|
||||
let notes_title = if self.focus_is_notes() {
|
||||
let notes_active = self.focus_is_notes();
|
||||
let notes_title = if notes_active {
|
||||
notes_label.as_str().cyan().bold()
|
||||
} else {
|
||||
notes_label.as_str().dim()
|
||||
@@ -156,7 +176,7 @@ impl RequestUserInputOverlay {
|
||||
Paragraph::new(Line::from(notes_title)).render(sections.notes_title_area, buf);
|
||||
}
|
||||
|
||||
if sections.notes_area.height > 0 {
|
||||
if notes_visible && sections.notes_area.height > 0 {
|
||||
self.render_notes_input(sections.notes_area, buf);
|
||||
}
|
||||
|
||||
@@ -164,69 +184,55 @@ impl RequestUserInputOverlay {
|
||||
.notes_area
|
||||
.y
|
||||
.saturating_add(sections.notes_area.height);
|
||||
if sections.footer_lines == 2 {
|
||||
// Status line for unanswered count when any question is empty.
|
||||
let warning = format!(
|
||||
"Unanswered: {} | Will submit as skipped",
|
||||
self.unanswered_count()
|
||||
);
|
||||
Paragraph::new(Line::from(warning.dim())).render(
|
||||
Rect {
|
||||
x: content_area.x,
|
||||
y: footer_y,
|
||||
width: content_area.width,
|
||||
height: 1,
|
||||
},
|
||||
buf,
|
||||
);
|
||||
let footer_area = Rect {
|
||||
x: content_area.x,
|
||||
y: footer_y,
|
||||
width: content_area.width,
|
||||
height: sections.footer_lines,
|
||||
};
|
||||
if footer_area.height == 0 {
|
||||
return;
|
||||
}
|
||||
let hint_y = footer_y.saturating_add(sections.footer_lines.saturating_sub(1));
|
||||
// Footer hints (selection index + navigation keys).
|
||||
let mut hint_spans = Vec::new();
|
||||
if self.has_options() {
|
||||
let options_len = self.options_len();
|
||||
let option_index = self.selected_option_index().map_or(0, |idx| idx + 1);
|
||||
hint_spans.extend(vec![
|
||||
format!("Option {option_index} of {options_len}").into(),
|
||||
" | ".into(),
|
||||
]);
|
||||
}
|
||||
hint_spans.extend(vec![
|
||||
key_hint::plain(KeyCode::Up).into(),
|
||||
"/".into(),
|
||||
key_hint::plain(KeyCode::Down).into(),
|
||||
" scroll | ".into(),
|
||||
key_hint::plain(KeyCode::Enter).into(),
|
||||
" next question | ".into(),
|
||||
]);
|
||||
if self.question_count() > 1 {
|
||||
hint_spans.extend(vec![
|
||||
key_hint::plain(KeyCode::PageUp).into(),
|
||||
" prev | ".into(),
|
||||
key_hint::plain(KeyCode::PageDown).into(),
|
||||
" next | ".into(),
|
||||
]);
|
||||
}
|
||||
hint_spans.extend(vec![
|
||||
key_hint::plain(KeyCode::Esc).into(),
|
||||
" interrupt".into(),
|
||||
]);
|
||||
Paragraph::new(Line::from(hint_spans).dim()).render(
|
||||
Rect {
|
||||
x: content_area.x,
|
||||
y: hint_y,
|
||||
width: content_area.width,
|
||||
let tip_lines = self.footer_tip_lines(footer_area.width);
|
||||
for (row_idx, tips) in tip_lines
|
||||
.into_iter()
|
||||
.take(footer_area.height as usize)
|
||||
.enumerate()
|
||||
{
|
||||
let mut spans = Vec::new();
|
||||
for (tip_idx, tip) in tips.into_iter().enumerate() {
|
||||
if tip_idx > 0 {
|
||||
spans.push(TIP_SEPARATOR.into());
|
||||
}
|
||||
if tip.highlight {
|
||||
spans.push(tip.text.cyan().bold().not_dim());
|
||||
} else {
|
||||
spans.push(tip.text.into());
|
||||
}
|
||||
}
|
||||
let line = Line::from(spans).dim();
|
||||
let line = truncate_line_word_boundary_with_ellipsis(line, footer_area.width as usize);
|
||||
let row_area = Rect {
|
||||
x: footer_area.x,
|
||||
y: footer_area.y.saturating_add(row_idx as u16),
|
||||
width: footer_area.width,
|
||||
height: 1,
|
||||
},
|
||||
buf,
|
||||
);
|
||||
};
|
||||
Paragraph::new(line).render(row_area, buf);
|
||||
}
|
||||
}
|
||||
|
||||
/// Return the cursor position when editing notes, if visible.
|
||||
pub(super) fn cursor_pos_impl(&self, area: Rect) -> Option<(u16, u16)> {
|
||||
let has_options = self.has_options();
|
||||
let notes_visible = self.notes_ui_visible();
|
||||
|
||||
if !self.focus_is_notes() {
|
||||
return None;
|
||||
}
|
||||
if has_options && !notes_visible {
|
||||
return None;
|
||||
}
|
||||
let content_area = menu_surface_inset(area);
|
||||
if content_area.width == 0 || content_area.height == 0 {
|
||||
return None;
|
||||
@@ -246,16 +252,118 @@ impl RequestUserInputOverlay {
|
||||
}
|
||||
self.composer.render(area, buf);
|
||||
}
|
||||
|
||||
fn focus_is_options(&self) -> bool {
|
||||
matches!(self.focus, super::Focus::Options)
|
||||
}
|
||||
|
||||
fn focus_is_notes(&self) -> bool {
|
||||
matches!(self.focus, super::Focus::Notes)
|
||||
}
|
||||
|
||||
fn focus_is_notes_without_options(&self) -> bool {
|
||||
!self.has_options() && self.focus_is_notes()
|
||||
}
|
||||
}
|
||||
|
||||
fn line_width(line: &Line<'_>) -> usize {
|
||||
line.iter()
|
||||
.map(|span| UnicodeWidthStr::width(span.content.as_ref()))
|
||||
.sum()
|
||||
}
|
||||
|
||||
/// Truncate a styled line to `max_width`, preferring a word boundary, and append an ellipsis.
|
||||
///
|
||||
/// This walks spans character-by-character, tracking the last width-safe position and the last
|
||||
/// whitespace boundary within the available width (excluding the ellipsis width). If the line
|
||||
/// overflows, it truncates at the last word boundary when possible (falling back to the last
|
||||
/// fitting character), trims trailing whitespace, then appends an ellipsis styled to match the
|
||||
/// last visible span (or the line style if nothing was kept).
|
||||
fn truncate_line_word_boundary_with_ellipsis(
|
||||
line: Line<'static>,
|
||||
max_width: usize,
|
||||
) -> Line<'static> {
|
||||
if max_width == 0 {
|
||||
return Line::from(Vec::<Span<'static>>::new());
|
||||
}
|
||||
|
||||
if line_width(&line) <= max_width {
|
||||
return line;
|
||||
}
|
||||
|
||||
let ellipsis = "…";
|
||||
let ellipsis_width = UnicodeWidthStr::width(ellipsis);
|
||||
if ellipsis_width >= max_width {
|
||||
return Line::from(ellipsis);
|
||||
}
|
||||
let limit = max_width.saturating_sub(ellipsis_width);
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
struct BreakPoint {
|
||||
span_idx: usize,
|
||||
byte_end: usize,
|
||||
}
|
||||
|
||||
// Track display width as we scan, along with the best "cut here" positions.
|
||||
let mut used = 0usize;
|
||||
let mut last_fit: Option<BreakPoint> = None;
|
||||
let mut last_word_break: Option<BreakPoint> = None;
|
||||
let mut overflowed = false;
|
||||
|
||||
'outer: for (span_idx, span) in line.spans.iter().enumerate() {
|
||||
let text = span.content.as_ref();
|
||||
for (byte_idx, ch) in text.char_indices() {
|
||||
let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0);
|
||||
if used.saturating_add(ch_width) > limit {
|
||||
overflowed = true;
|
||||
break 'outer;
|
||||
}
|
||||
used = used.saturating_add(ch_width);
|
||||
let bp = BreakPoint {
|
||||
span_idx,
|
||||
byte_end: byte_idx + ch.len_utf8(),
|
||||
};
|
||||
last_fit = Some(bp);
|
||||
if ch.is_whitespace() {
|
||||
last_word_break = Some(bp);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// If we never overflowed, the original line already fits.
|
||||
if !overflowed {
|
||||
return line;
|
||||
}
|
||||
|
||||
// Prefer breaking on whitespace; otherwise fall back to the last fitting character.
|
||||
let chosen_break = last_word_break.or(last_fit);
|
||||
let Some(chosen_break) = chosen_break else {
|
||||
return Line::from(ellipsis);
|
||||
};
|
||||
|
||||
let line_style = line.style;
|
||||
let mut spans_out: Vec<Span<'static>> = Vec::new();
|
||||
for (idx, span) in line.spans.into_iter().enumerate() {
|
||||
if idx < chosen_break.span_idx {
|
||||
spans_out.push(span);
|
||||
continue;
|
||||
}
|
||||
if idx == chosen_break.span_idx {
|
||||
let text = span.content.into_owned();
|
||||
let truncated = text[..chosen_break.byte_end].to_string();
|
||||
if !truncated.is_empty() {
|
||||
spans_out.push(Span::styled(truncated, span.style));
|
||||
}
|
||||
}
|
||||
break;
|
||||
}
|
||||
|
||||
while let Some(last) = spans_out.last_mut() {
|
||||
let trimmed = last
|
||||
.content
|
||||
.trim_end_matches(char::is_whitespace)
|
||||
.to_string();
|
||||
if trimmed.is_empty() {
|
||||
spans_out.pop();
|
||||
} else {
|
||||
last.content = trimmed.into();
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
let ellipsis_style = spans_out
|
||||
.last()
|
||||
.map(|span| span.style)
|
||||
.unwrap_or(line_style);
|
||||
spans_out.push(Span::styled(ellipsis, ellipsis_style));
|
||||
|
||||
Line::from(spans_out).style(line_style)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user