From 96386755b6d8a92c33bdef336fc985cf3fb1285a Mon Sep 17 00:00:00 2001 From: Charley Cunningham Date: Wed, 28 Jan 2026 09:41:59 -0800 Subject: [PATCH] Refine request_user_input TUI interactions and option UX (#10025) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Overhaul the ask‑user‑questions TUI to support “Other/None” answers, better notes handling, improved option selection UX, and a safer submission flow with confirmation for unanswered questions. Multiple choice (number keys for quick selection, up/down or jk for cycling through options): Screenshot 2026-01-27 at 7 22 29 PM Tab to add notes: Screenshot 2026-01-27 at 7 22 45 PM Freeform (also note enter tooltip is highlighted on last question to indicate questions UI will be exited upon submission): Screenshot 2026-01-27 at 7 23 13 PM Confirmation dialogue (submitting with unanswered questions): Screenshot 2026-01-27 at 7 23 29 PM ## Key Changes - **Options UI refresh** - Render options as numbered entries; allow number keys to select & submit. - Remove “Option X/Y” header and allow the question UI height to expand naturally. - Keep spacing between question, options, and notes even when notes are visible. - Hide the title line and render the question prompt in cyan **only when uncommitted**. - **“Other / None of the above” support** - Wire `isOther` to add “None of the above”. - Add guidance text: “Optionally, add details in notes (tab).” - **Notes composer UX** - Remove “Notes” heading; place composer directly under the selected option. - Preserve pending paste placeholders across question navigation and after submission. - Ctrl+C clears notes **only when the notes composer has focus**. - Ctrl+C now triggers an immediate redraw so the clear is visible. - **Committed vs uncommitted state** - Introduce a unified `answer_committed` flag per question. - Editing notes (including adding text or pastes) marks the answer uncommitted. - Changing the option highlight (j/k, up/down) marks the answer uncommitted. - Clearing options (Backspace/Delete) also clears pending notes. - Question prompt turns cyan only when the answer is uncommitted. - **Submission safety & confirmation** - Only submit notes/freeform text once explicitly committed. - Last-question submit with unanswered questions shows a confirmation dialog. - Confirmation options: 1. **Proceed** (default) 2. **Go back** - Description reflects count: “Submit with N unanswered question(s).” - Esc/Backspace in confirmation returns to first unanswered question. - Ctrl+C in confirmation interrupts and exits the overlay. - **Footer hints** - Cyan highlight restored for “enter to submit answer” / “enter to submit all”. ## Codex author `codex fork 019c00ed-323a-7000-bdb5-9f9c5a635bd9` --- .../tui/src/bottom_pane/bottom_pane_view.rs | 6 + codex-rs/tui/src/bottom_pane/chat_composer.rs | 14 +- codex-rs/tui/src/bottom_pane/mod.rs | 66 ++ .../bottom_pane/request_user_input/layout.rs | 217 ++--- .../src/bottom_pane/request_user_input/mod.rs | 883 ++++++++++++++---- .../bottom_pane/request_user_input/render.rs | 270 ++++-- ...tests__request_user_input_footer_wrap.snap | 14 +- ...t__tests__request_user_input_freeform.snap | 4 +- ...uest_user_input_hidden_options_footer.snap | 13 + ...quest_user_input_multi_question_first.snap | 9 +- ...equest_user_input_multi_question_last.snap | 4 +- ...ut__tests__request_user_input_options.snap | 9 +- ...uest_user_input_options_notes_visible.snap | 14 +- ..._request_user_input_scrolling_options.snap | 14 +- ...ests__request_user_input_tight_height.snap | 8 +- ...st_user_input_unanswered_confirmation.snap | 15 + ...s__request_user_input_wrapped_options.snap | 11 +- 17 files changed, 1164 insertions(+), 407 deletions(-) create mode 100644 codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_hidden_options_footer.snap create mode 100644 codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_unanswered_confirmation.snap 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 d14344922..039a9ca05 100644 --- a/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs +++ b/codex-rs/tui/src/bottom_pane/bottom_pane_view.rs @@ -21,6 +21,12 @@ pub(crate) trait BottomPaneView: Renderable { CancellationEvent::NotHandled } + /// Return true if Esc should be routed through `handle_key_event` instead + /// of the `on_ctrl_c` cancellation path. + fn prefer_esc_to_handle_key_event(&self) -> bool { + false + } + /// Optional paste handler. Return true if the view modified its state and /// needs a redraw. fn handle_paste(&mut self, _pasted: String) -> bool { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index e59f82082..4c1228cb3 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -653,6 +653,18 @@ impl ChatComposer { text } + pub(crate) fn pending_pastes(&self) -> Vec<(String, String)> { + self.pending_pastes.clone() + } + + pub(crate) fn set_pending_pastes(&mut self, pending_pastes: Vec<(String, String)>) { + let text = self.textarea.text().to_string(); + self.pending_pastes = pending_pastes + .into_iter() + .filter(|(placeholder, _)| text.contains(placeholder)) + .collect(); + } + /// Override the footer hint items displayed beneath the composer. Passing /// `None` restores the default shortcut footer. pub(crate) fn set_footer_hint_override(&mut self, items: Option>) { @@ -1431,7 +1443,7 @@ impl ChatComposer { } /// Expand large-paste placeholders using element ranges and rebuild other element spans. - fn expand_pending_pastes( + pub(crate) fn expand_pending_pastes( text: &str, mut elements: Vec, pending_pastes: &[(String, String)], diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index e0a7319a4..701e0762c 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -269,7 +269,10 @@ impl BottomPane { let (ctrl_c_completed, view_complete, view_in_paste_burst) = { let last_index = self.view_stack.len() - 1; let view = &mut self.view_stack[last_index]; + let prefer_esc = + key_event.code == KeyCode::Esc && view.prefer_esc_to_handle_key_event(); let ctrl_c_completed = key_event.code == KeyCode::Esc + && !prefer_esc && matches!(view.on_ctrl_c(), CancellationEvent::Handled) && view.is_complete(); if ctrl_c_completed { @@ -338,6 +341,7 @@ impl BottomPane { self.on_active_view_complete(); } self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); + self.request_redraw(); } event } else if self.composer_is_empty() { @@ -346,6 +350,7 @@ impl BottomPane { self.view_stack.pop(); self.clear_composer_for_ctrl_c(); self.show_quit_shortcut_hint(key_hint::ctrl(KeyCode::Char('c'))); + self.request_redraw(); CancellationEvent::Handled } } @@ -815,7 +820,9 @@ mod tests { use insta::assert_snapshot; use ratatui::buffer::Buffer; use ratatui::layout::Rect; + use std::cell::Cell; use std::path::PathBuf; + use std::rc::Rc; use tokio::sync::mpsc::unbounded_channel; fn snapshot_buffer(buf: &Buffer) -> String { @@ -1242,4 +1249,63 @@ mod tests { "expected Esc to send Op::Interrupt while a task is running" ); } + + #[test] + fn esc_routes_to_handle_key_event_when_requested() { + #[derive(Default)] + struct EscRoutingView { + on_ctrl_c_calls: Rc>, + handle_calls: Rc>, + } + + impl Renderable for EscRoutingView { + fn render(&self, _area: Rect, _buf: &mut Buffer) {} + + fn desired_height(&self, _width: u16) -> u16 { + 0 + } + } + + impl BottomPaneView for EscRoutingView { + fn handle_key_event(&mut self, _key_event: KeyEvent) { + self.handle_calls + .set(self.handle_calls.get().saturating_add(1)); + } + + fn on_ctrl_c(&mut self) -> CancellationEvent { + self.on_ctrl_c_calls + .set(self.on_ctrl_c_calls.get().saturating_add(1)); + CancellationEvent::Handled + } + + fn prefer_esc_to_handle_key_event(&self) -> bool { + true + } + } + + let (tx_raw, _rx) = unbounded_channel::(); + let tx = AppEventSender::new(tx_raw); + let mut pane = BottomPane::new(BottomPaneParams { + app_event_tx: tx, + frame_requester: FrameRequester::test_dummy(), + has_input_focus: true, + enhanced_keys_supported: false, + placeholder_text: "Ask Codex to do anything".to_string(), + disable_paste_burst: false, + animations_enabled: true, + skills: Some(Vec::new()), + }); + + let on_ctrl_c_calls = Rc::new(Cell::new(0)); + let handle_calls = Rc::new(Cell::new(0)); + pane.push_view(Box::new(EscRoutingView { + on_ctrl_c_calls: Rc::clone(&on_ctrl_c_calls), + handle_calls: Rc::clone(&handle_calls), + })); + + pane.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + + assert_eq!(on_ctrl_c_calls.get(), 0); + assert_eq!(handle_calls.get(), 1); + } } diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/layout.rs b/codex-rs/tui/src/bottom_pane/request_user_input/layout.rs index 11856bd1d..27d53229b 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/layout.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/layout.rs @@ -1,16 +1,14 @@ use ratatui::layout::Rect; -use super::DESIRED_SPACERS_WHEN_NOTES_HIDDEN; +use super::DESIRED_SPACERS_BETWEEN_SECTIONS; use super::RequestUserInputOverlay; pub(super) struct LayoutSections { pub(super) progress_area: Rect, - pub(super) header_area: Rect, pub(super) question_area: Rect, // Wrapped question text lines to render in the question area. pub(super) question_lines: Vec, pub(super) options_area: Rect, - pub(super) notes_title_area: Rect, pub(super) notes_area: Rect, // Number of footer rows (status + hints). pub(super) footer_lines: u16, @@ -26,16 +24,7 @@ impl RequestUserInputOverlay { let mut question_lines = self.wrapped_question_lines(area.width); let question_height = question_lines.len() as u16; - let ( - question_height, - progress_height, - spacer_after_question, - options_height, - spacer_after_options, - notes_title_height, - notes_height, - footer_lines, - ) = if has_options { + let layout = if has_options { self.layout_with_options( OptionsLayoutArgs { available_height: area.height, @@ -57,98 +46,52 @@ impl RequestUserInputOverlay { ) }; - let (progress_area, header_area, question_area, options_area, notes_title_area, notes_area) = - self.build_layout_areas( - area, - LayoutHeights { - progress_height, - question_height, - spacer_after_question, - options_height, - spacer_after_options, - notes_title_height, - notes_height, - }, - ); + let (progress_area, question_area, options_area, notes_area) = + self.build_layout_areas(area, layout); LayoutSections { progress_area, - header_area, question_area, question_lines, options_area, - notes_title_area, notes_area, - footer_lines, + footer_lines: layout.footer_lines, } } /// Layout calculation when options are present. - /// - /// Handles both tight layout (when space is constrained) and normal layout - /// (when there's sufficient space for all elements). - /// - /// Returns: (question_height, progress_height, spacer_after_question, options_height, spacer_after_options, notes_title_height, notes_height, footer_lines) fn layout_with_options( &self, args: OptionsLayoutArgs, question_lines: &mut Vec, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { + ) -> LayoutPlan { let OptionsLayoutArgs { available_height, width, - question_height, + mut question_height, notes_pref_height, footer_pref, notes_visible, } = args; - let options_heights = OptionsHeights { - preferred: self.options_preferred_height(width), - full: self.options_required_height(width), - }; - let min_options_height = 1u16; - let required = 1u16 - .saturating_add(question_height) - .saturating_add(options_heights.preferred); - - if required > available_height { - self.layout_with_options_tight( + let min_options_height = available_height.min(1); + let max_question_height = available_height.saturating_sub(min_options_height); + if question_height > max_question_height { + question_height = max_question_height; + question_lines.truncate(question_height as usize); + } + self.layout_with_options_normal( + OptionsNormalArgs { available_height, question_height, - min_options_height, - question_lines, - ) - } else { - self.layout_with_options_normal( - OptionsNormalArgs { - available_height, - question_height, - notes_pref_height, - footer_pref, - notes_visible, - }, - options_heights, - ) - } - } - - /// Tight layout for options case: allocate header + question + options first - /// and drop everything else when space is constrained. - fn layout_with_options_tight( - &self, - available_height: u16, - question_height: u16, - min_options_height: u16, - question_lines: &mut Vec, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { - let max_question_height = - available_height.saturating_sub(1u16.saturating_add(min_options_height)); - let adjusted_question_height = question_height.min(max_question_height); - question_lines.truncate(adjusted_question_height as usize); - let options_height = - available_height.saturating_sub(1u16.saturating_add(adjusted_question_height)); - - (adjusted_question_height, 0, 0, options_height, 0, 0, 0, 0) + notes_pref_height, + footer_pref, + notes_visible, + }, + OptionsHeights { + preferred: self.options_preferred_height(width), + full: self.options_required_height(width), + }, + ) } /// Normal layout for options case: allocate footer + progress first, and @@ -157,7 +100,7 @@ impl RequestUserInputOverlay { &self, args: OptionsNormalArgs, options: OptionsHeights, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { + ) -> LayoutPlan { let OptionsNormalArgs { available_height, question_height, @@ -165,19 +108,23 @@ impl RequestUserInputOverlay { footer_pref, notes_visible, } = args; - let min_options_height = 1u16; - let mut options_height = options.preferred.max(min_options_height); - let used = 1u16 - .saturating_add(question_height) - .saturating_add(options_height); + let max_options_height = available_height.saturating_sub(question_height); + let min_options_height = max_options_height.min(1); + let mut options_height = options + .preferred + .min(max_options_height) + .max(min_options_height); + let used = question_height.saturating_add(options_height); let mut remaining = available_height.saturating_sub(used); // When notes are hidden, prefer to reserve room for progress, footer, // and spacers by shrinking the options window if needed. let desired_spacers = if notes_visible { - 0 + // Notes already separate options from the footer, so only keep a + // single spacer between the question and options. + 1 } else { - DESIRED_SPACERS_WHEN_NOTES_HIDDEN + DESIRED_SPACERS_BETWEEN_SECTIONS }; let required_extra = footer_pref .saturating_add(1) // progress line @@ -211,45 +158,41 @@ impl RequestUserInputOverlay { } let grow_by = remaining.min(options.full.saturating_sub(options_height)); options_height = options_height.saturating_add(grow_by); - return ( + return LayoutPlan { question_height, progress_height, spacer_after_question, options_height, spacer_after_options, - 0, - 0, + notes_height: 0, footer_lines, - ); + }; } let footer_lines = footer_pref.min(remaining); remaining = remaining.saturating_sub(footer_lines); - // Prefer notes next, then labels, with any leftover rows expanding notes. - let spacer_after_question = 0; + // Prefer spacers before notes, then notes. + let mut spacer_after_question = 0; + if remaining > 0 { + spacer_after_question = 1; + remaining = remaining.saturating_sub(1); + } let spacer_after_options = 0; let mut notes_height = notes_pref_height.min(remaining); remaining = remaining.saturating_sub(notes_height); - let mut notes_title_height = 0; - if remaining > 0 { - notes_title_height = 1; - remaining = remaining.saturating_sub(1); - } - notes_height = notes_height.saturating_add(remaining); - ( + LayoutPlan { question_height, progress_height, spacer_after_question, options_height, spacer_after_options, - notes_title_height, notes_height, footer_lines, - ) + } } /// Layout calculation when no options are present. @@ -257,7 +200,6 @@ impl RequestUserInputOverlay { /// Handles both tight layout (when space is constrained) and normal layout /// (when there's sufficient space for all elements). /// - /// Returns: (question_height, progress_height, spacer_after_question, options_height, spacer_after_options, notes_title_height, notes_height, footer_lines) fn layout_without_options( &self, available_height: u16, @@ -265,8 +207,8 @@ impl RequestUserInputOverlay { notes_pref_height: u16, footer_pref: u16, question_lines: &mut Vec, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { - let required = 1u16.saturating_add(question_height); + ) -> LayoutPlan { + let required = question_height; if required > available_height { self.layout_without_options_tight(available_height, question_height, question_lines) } else { @@ -285,12 +227,20 @@ impl RequestUserInputOverlay { available_height: u16, question_height: u16, question_lines: &mut Vec, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { - let max_question_height = available_height.saturating_sub(1); + ) -> LayoutPlan { + let max_question_height = available_height; let adjusted_question_height = question_height.min(max_question_height); question_lines.truncate(adjusted_question_height as usize); - (adjusted_question_height, 0, 0, 0, 0, 0, 0, 0) + LayoutPlan { + question_height: adjusted_question_height, + progress_height: 0, + spacer_after_question: 0, + options_height: 0, + spacer_after_options: 0, + notes_height: 0, + footer_lines: 0, + } } /// Normal layout for no-options case: allocate space for notes, footer, and progress. @@ -300,8 +250,8 @@ impl RequestUserInputOverlay { question_height: u16, notes_pref_height: u16, footer_pref: u16, - ) -> (u16, u16, u16, u16, u16, u16, u16, u16) { - let required = 1u16.saturating_add(question_height); + ) -> LayoutPlan { + let required = question_height; let mut remaining = available_height.saturating_sub(required); let mut notes_height = notes_pref_height.min(remaining); remaining = remaining.saturating_sub(notes_height); @@ -317,29 +267,26 @@ impl RequestUserInputOverlay { notes_height = notes_height.saturating_add(remaining); - ( + LayoutPlan { question_height, progress_height, - 0, - 0, - 0, - 0, + spacer_after_question: 0, + options_height: 0, + spacer_after_options: 0, notes_height, footer_lines, - ) + } } /// Build the final layout areas from computed heights. fn build_layout_areas( &self, area: Rect, - heights: LayoutHeights, + heights: LayoutPlan, ) -> ( Rect, // progress_area - Rect, // header_area Rect, // question_area Rect, // options_area - Rect, // notes_title_area Rect, // notes_area ) { let mut cursor_y = area.y; @@ -350,14 +297,6 @@ impl RequestUserInputOverlay { height: heights.progress_height, }; cursor_y = cursor_y.saturating_add(heights.progress_height); - let header_height = area.height.saturating_sub(heights.progress_height).min(1); - let header_area = Rect { - x: area.x, - y: cursor_y, - width: area.width, - height: header_height, - }; - cursor_y = cursor_y.saturating_add(header_height); let question_area = Rect { x: area.x, y: cursor_y, @@ -376,13 +315,6 @@ impl RequestUserInputOverlay { cursor_y = cursor_y.saturating_add(heights.options_height); cursor_y = cursor_y.saturating_add(heights.spacer_after_options); - let notes_title_area = Rect { - x: area.x, - y: cursor_y, - width: area.width, - height: heights.notes_title_height, - }; - cursor_y = cursor_y.saturating_add(heights.notes_title_height); let notes_area = Rect { x: area.x, y: cursor_y, @@ -390,26 +322,19 @@ impl RequestUserInputOverlay { height: heights.notes_height, }; - ( - progress_area, - header_area, - question_area, - options_area, - notes_title_area, - notes_area, - ) + (progress_area, question_area, options_area, notes_area) } } #[derive(Clone, Copy, Debug)] -struct LayoutHeights { +struct LayoutPlan { progress_height: u16, question_height: u16, spacer_after_question: u16, options_height: u16, spacer_after_options: u16, - notes_title_height: u16, notes_height: u16, + footer_lines: u16, } #[derive(Clone, Copy, Debug)] diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs index 3fe16c431..979499835 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/mod.rs @@ -42,8 +42,15 @@ const ANSWER_PLACEHOLDER: &str = "Type your answer (optional)"; const MIN_COMPOSER_HEIGHT: u16 = 3; const SELECT_OPTION_PLACEHOLDER: &str = "Select an option to add notes"; pub(super) const TIP_SEPARATOR: &str = " | "; -pub(super) const MAX_VISIBLE_OPTION_ROWS: usize = 4; -pub(super) const DESIRED_SPACERS_WHEN_NOTES_HIDDEN: u16 = 2; +pub(super) const DESIRED_SPACERS_BETWEEN_SECTIONS: u16 = 2; +const OTHER_OPTION_LABEL: &str = "None of the above"; +const OTHER_OPTION_DESCRIPTION: &str = "Optionally, add details in notes (tab)."; +const UNANSWERED_CONFIRM_TITLE: &str = "Submit with unanswered questions?"; +const UNANSWERED_CONFIRM_GO_BACK: &str = "Go back"; +const UNANSWERED_CONFIRM_GO_BACK_DESC: &str = "Return to the first unanswered question."; +const UNANSWERED_CONFIRM_SUBMIT: &str = "Proceed"; +const UNANSWERED_CONFIRM_SUBMIT_DESC_SINGULAR: &str = "question"; +const UNANSWERED_CONFIRM_SUBMIT_DESC_PLURAL: &str = "questions"; #[derive(Clone, Copy, Debug, PartialEq, Eq)] enum Focus { @@ -51,22 +58,39 @@ enum Focus { Notes, } -#[derive(Default, Clone)] +#[derive(Default, Clone, PartialEq)] struct ComposerDraft { text: String, text_elements: Vec, local_image_paths: Vec, + pending_pastes: Vec<(String, String)>, +} + +impl ComposerDraft { + fn text_with_pending(&self) -> String { + if self.pending_pastes.is_empty() { + return self.text.clone(); + } + debug_assert!( + !self.text_elements.is_empty(), + "pending pastes should always have matching text elements" + ); + let (expanded, _) = ChatComposer::expand_pending_pastes( + &self.text, + self.text_elements.clone(), + &self.pending_pastes, + ); + expanded + } } struct AnswerState { - // Committed selection for the question (may be None when unanswered). - committed_option_idx: Option, // Scrollable cursor state for option navigation/highlight. - options_ui_state: ScrollState, + options_state: ScrollState, // Per-question notes draft. draft: ComposerDraft, - // Whether a freeform answer has been explicitly submitted. - freeform_committed: bool, + // Whether the answer for this question has been explicitly submitted. + answer_committed: bool, // Whether the notes UI has been explicitly opened for this question. notes_visible: bool, } @@ -106,6 +130,8 @@ pub(crate) struct RequestUserInputOverlay { current_idx: usize, focus: Focus, done: bool, + pending_submission_draft: Option, + confirm_unanswered: Option, } impl RequestUserInputOverlay { @@ -137,6 +163,8 @@ impl RequestUserInputOverlay { current_idx: 0, focus: Focus::Options, done: false, + pending_submission_draft: None, + confirm_unanswered: None, }; overlay.reset_for_request(); overlay.ensure_focus_available(); @@ -176,28 +204,28 @@ impl RequestUserInputOverlay { fn options_len(&self) -> usize { self.current_question() - .and_then(|question| question.options.as_ref()) - .map(std::vec::Vec::len) + .map(Self::options_len_for_question) .unwrap_or(0) } + fn option_index_for_digit(&self, ch: char) -> Option { + if !self.has_options() { + return None; + } + let digit = ch.to_digit(10)?; + if digit == 0 { + return None; + } + let idx = (digit - 1) as usize; + (idx < self.options_len()).then_some(idx) + } + fn selected_option_index(&self) -> Option { if !self.has_options() { return None; } - self.current_answer().and_then(|answer| { - answer - .committed_option_idx - .or(answer.options_ui_state.selected_idx) - }) - } - - fn current_option_label(&self) -> Option<&str> { - let idx = self.selected_option_index()?; - self.current_question() - .and_then(|question| question.options.as_ref()) - .and_then(|options| options.get(idx)) - .map(|option| option.label.as_str()) + self.current_answer() + .and_then(|answer| answer.options_state.selected_idx) } fn notes_has_content(&self, idx: usize) -> bool { @@ -232,26 +260,46 @@ impl RequestUserInputOverlay { matches!(self.focus, Focus::Notes) } + fn confirm_unanswered_active(&self) -> bool { + self.confirm_unanswered.is_some() + } + pub(super) fn option_rows(&self) -> Vec { self.current_question() - .and_then(|question| question.options.as_ref()) - .map(|options| { - options + .and_then(|question| question.options.as_ref().map(|options| (question, options))) + .map(|(question, options)| { + let selected_idx = self + .current_answer() + .and_then(|answer| answer.options_state.selected_idx); + let mut rows = options .iter() .enumerate() .map(|(idx, opt)| { - let selected = self - .current_answer() - .and_then(|answer| answer.committed_option_idx) - .is_some_and(|sel| sel == idx); - let prefix = if selected { "(x)" } else { "( )" }; + let selected = selected_idx.is_some_and(|sel| sel == idx); + let prefix = if selected { '›' } else { ' ' }; + let label = opt.label.as_str(); + let number = idx + 1; GenericDisplayRow { - name: format!("{prefix} {}", opt.label), + name: format!("{prefix} {number}. {label}"), description: Some(opt.description.clone()), ..Default::default() } }) - .collect::>() + .collect::>(); + + if Self::other_option_enabled_for_question(question) { + let idx = options.len(); + let selected = selected_idx.is_some_and(|sel| sel == idx); + let prefix = if selected { '›' } else { ' ' }; + let number = idx + 1; + rows.push(GenericDisplayRow { + name: format!("{prefix} {number}. {OTHER_OPTION_LABEL}"), + description: Some(OTHER_OPTION_DESCRIPTION.to_string()), + ..Default::default() + }); + } + + rows }) .unwrap_or_default() } @@ -268,7 +316,7 @@ impl RequestUserInputOverlay { let mut state = self .current_answer() - .map(|answer| answer.options_ui_state) + .map(|answer| answer.options_state) .unwrap_or_default(); if state.selected_idx.is_none() { state.selected_idx = Some(0); @@ -289,19 +337,18 @@ impl RequestUserInputOverlay { let mut state = self .current_answer() - .map(|answer| answer.options_ui_state) + .map(|answer| answer.options_state) .unwrap_or_default(); if state.selected_idx.is_none() { state.selected_idx = Some(0); } - let visible_items = rows.len().min(MAX_VISIBLE_OPTION_ROWS); - measure_rows_height(&rows, &state, visible_items, width.max(1)) + measure_rows_height(&rows, &state, rows.len(), width.max(1)) } fn capture_composer_draft(&self) -> ComposerDraft { ComposerDraft { - text: self.composer.current_text_with_pending(), + text: self.composer.current_text(), text_elements: self.composer.text_elements(), local_image_paths: self .composer @@ -309,6 +356,7 @@ impl RequestUserInputOverlay { .into_iter() .map(|img| img.path) .collect(), + pending_pastes: self.composer.pending_pastes(), } } @@ -316,6 +364,9 @@ impl RequestUserInputOverlay { let draft = self.capture_composer_draft(); let notes_empty = draft.text.trim().is_empty(); if let Some(answer) = self.current_answer_mut() { + if answer.answer_committed && answer.draft != draft { + answer.answer_committed = false; + } answer.draft = draft; if !notes_empty { answer.notes_visible = true; @@ -336,6 +387,7 @@ impl RequestUserInputOverlay { let draft = answer.draft.clone(); self.composer .set_text_content(draft.text, draft.text_elements, draft.local_image_paths); + self.composer.set_pending_pastes(draft.pending_pastes); self.composer.move_cursor_to_end(); } @@ -354,47 +406,72 @@ impl RequestUserInputOverlay { .set_placeholder_text(self.notes_placeholder().to_string()); } + fn clear_notes_draft(&mut self) { + if let Some(answer) = self.current_answer_mut() { + answer.draft = ComposerDraft::default(); + answer.answer_committed = false; + answer.notes_visible = true; + } + self.pending_submission_draft = None; + self.composer + .set_text_content(String::new(), Vec::new(), Vec::new()); + self.composer.move_cursor_to_end(); + self.sync_composer_placeholder(); + } + fn footer_tips(&self) -> Vec { let mut tips = Vec::new(); let notes_visible = self.notes_ui_visible(); if self.has_options() { - let options_len = self.options_len(); - if let Some(selected_idx) = self.selected_option_index() { - let option_index = selected_idx + 1; - tips.push(FooterTip::new(format!( - "Option {option_index} of {options_len}" - ))); - } else { - tips.push(FooterTip::new("No option selected")); - } - tips.push(FooterTip::new("\u{2191}/\u{2193} scroll")); if self.selected_option_index().is_some() && !notes_visible { - tips.push(FooterTip::highlighted("Tab: add notes")); + tips.push(FooterTip::highlighted("tab to add notes")); } if self.selected_option_index().is_some() && notes_visible && self.focus_is_notes() { - tips.push(FooterTip::new("Tab: clear notes")); + tips.push(FooterTip::new("tab to clear notes")); } } let question_count = self.question_count(); - let is_last_question = question_count > 0 && self.current_index() + 1 >= question_count; - let enter_tip = if question_count > 1 && is_last_question { - "Enter: submit all answers" + let is_last_question = self.current_index().saturating_add(1) >= question_count; + let enter_tip = if question_count == 1 { + FooterTip::highlighted("enter to submit answer") + } else if is_last_question { + FooterTip::highlighted("enter to submit all") } else { - "Enter: submit answer" + FooterTip::new("enter to submit answer") }; - tips.push(FooterTip::new(enter_tip)); + tips.push(enter_tip); if question_count > 1 { - tips.push(FooterTip::new("Ctrl+n next")); + if is_last_question { + tips.push(FooterTip::new("ctrl + n first question")); + } else { + tips.push(FooterTip::new("ctrl + n next question")); + } } - tips.push(FooterTip::new("Esc: interrupt")); + tips.push(FooterTip::new("esc to interrupt")); tips } pub(super) fn footer_tip_lines(&self, width: u16) -> Vec> { + self.wrap_footer_tips(width, self.footer_tips()) + } + + pub(super) fn footer_tip_lines_with_prefix( + &self, + width: u16, + prefix: Option, + ) -> Vec> { + let mut tips = Vec::new(); + if let Some(prefix) = prefix { + tips.push(prefix); + } + tips.extend(self.footer_tips()); + self.wrap_footer_tips(width, tips) + } + + fn wrap_footer_tips(&self, width: u16, tips: Vec) -> Vec> { let max_width = width.max(1) as usize; let separator_width = UnicodeWidthStr::width(TIP_SEPARATOR); - let tips = self.footer_tips(); if tips.is_empty() { return vec![Vec::new()]; } @@ -466,15 +543,14 @@ impl RequestUserInputOverlay { .options .as_ref() .is_some_and(|options| !options.is_empty()); - let mut options_ui_state = ScrollState::new(); + let mut options_state = ScrollState::new(); if has_options { - options_ui_state.selected_idx = Some(0); + options_state.selected_idx = Some(0); } AnswerState { - committed_option_idx: None, - options_ui_state, + options_state, draft: ComposerDraft::default(), - freeform_committed: false, + answer_committed: false, notes_visible: !has_options, } }) @@ -484,6 +560,47 @@ impl RequestUserInputOverlay { self.focus = Focus::Options; self.composer .set_text_content(String::new(), Vec::new(), Vec::new()); + self.confirm_unanswered = None; + self.pending_submission_draft = None; + } + + fn options_len_for_question( + question: &codex_protocol::request_user_input::RequestUserInputQuestion, + ) -> usize { + let options_len = question + .options + .as_ref() + .map(std::vec::Vec::len) + .unwrap_or(0); + if Self::other_option_enabled_for_question(question) { + options_len + 1 + } else { + options_len + } + } + + fn other_option_enabled_for_question( + question: &codex_protocol::request_user_input::RequestUserInputQuestion, + ) -> bool { + question.is_other + && question + .options + .as_ref() + .is_some_and(|options| !options.is_empty()) + } + + fn option_label_for_index( + question: &codex_protocol::request_user_input::RequestUserInputQuestion, + idx: usize, + ) -> Option { + let options = question.options.as_ref()?; + if idx < options.len() { + return options.get(idx).map(|opt| opt.label.clone()); + } + if idx == options.len() && Self::other_option_enabled_for_question(question) { + return Some(OTHER_OPTION_LABEL.to_string()); + } + None } /// Move to the next/previous question, wrapping in either direction. @@ -499,15 +616,25 @@ impl RequestUserInputOverlay { self.ensure_focus_available(); } + fn jump_to_question(&mut self, idx: usize) { + if idx >= self.question_count() { + return; + } + self.save_current_draft(); + self.current_idx = idx; + self.restore_current_draft(); + self.ensure_focus_available(); + } + /// Synchronize selection state to the currently focused option. - fn select_current_option(&mut self) { + fn select_current_option(&mut self, committed: bool) { if !self.has_options() { return; } let options_len = self.options_len(); let updated = if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.clamp_selection(options_len); - answer.committed_option_idx = answer.options_ui_state.selected_idx; + answer.options_state.clamp_selection(options_len); + answer.answer_committed = committed; true } else { false @@ -522,15 +649,16 @@ impl RequestUserInputOverlay { if !self.has_options() { return; } - self.save_current_draft(); - let notes_empty = self.composer.current_text_with_pending().trim().is_empty(); if let Some(answer) = self.current_answer_mut() { - answer.committed_option_idx = None; - answer.options_ui_state.reset(); - if notes_empty { - answer.notes_visible = false; - } + answer.options_state.reset(); + answer.draft = ComposerDraft::default(); + answer.answer_committed = false; + answer.notes_visible = false; } + self.pending_submission_draft = None; + self.composer + .set_text_content(String::new(), Vec::new(), Vec::new()); + self.composer.move_cursor_to_end(); self.sync_composer_placeholder(); } @@ -545,7 +673,12 @@ impl RequestUserInputOverlay { /// Advance to next question, or submit when on the last one. fn go_next_or_submit(&mut self) { if self.current_index() + 1 >= self.question_count() { - self.submit_answers(); + self.save_current_draft(); + if self.unanswered_count() > 0 { + self.open_unanswered_confirmation(); + } else { + self.submit_answers(); + } } else { self.move_question(true); } @@ -553,33 +686,28 @@ impl RequestUserInputOverlay { /// Build the response payload and dispatch it to the app. fn submit_answers(&mut self) { + self.confirm_unanswered = None; self.save_current_draft(); let mut answers = HashMap::new(); for (idx, question) in self.request.questions.iter().enumerate() { let answer_state = &self.answers[idx]; let options = question.options.as_ref(); // For option questions we may still produce no selection. - let selected_idx = if options.is_some_and(|opts| !opts.is_empty()) { - answer_state.committed_option_idx - } else { - None - }; + let selected_idx = + if options.is_some_and(|opts| !opts.is_empty()) && answer_state.answer_committed { + answer_state.options_state.selected_idx + } else { + None + }; // Notes are appended as extra answers. For freeform questions, only submit when // the user explicitly committed the draft. - let notes = if options.is_some_and(|opts| !opts.is_empty()) - || answer_state.freeform_committed - { - answer_state.draft.text.trim().to_string() + let notes = if answer_state.answer_committed { + answer_state.draft.text_with_pending().trim().to_string() } else { String::new() }; - let selected_label = selected_idx.and_then(|selected_idx| { - question - .options - .as_ref() - .and_then(|opts| opts.get(selected_idx)) - .map(|opt| opt.label.clone()) - }); + let selected_label = selected_idx + .and_then(|selected_idx| Self::option_label_for_index(question, selected_idx)); let mut answer_list = selected_label.into_iter().collect::>(); if !notes.is_empty() { answer_list.push(format!("user_note: {notes}")); @@ -606,6 +734,71 @@ impl RequestUserInputOverlay { } } + fn open_unanswered_confirmation(&mut self) { + let mut state = ScrollState::new(); + state.selected_idx = Some(0); + self.confirm_unanswered = Some(state); + } + + fn close_unanswered_confirmation(&mut self) { + self.confirm_unanswered = None; + } + + fn unanswered_question_count(&self) -> usize { + self.unanswered_count() + } + + fn unanswered_submit_description(&self) -> String { + let count = self.unanswered_question_count(); + let suffix = if count == 1 { + UNANSWERED_CONFIRM_SUBMIT_DESC_SINGULAR + } else { + UNANSWERED_CONFIRM_SUBMIT_DESC_PLURAL + }; + format!("Submit with {count} unanswered {suffix}.") + } + + fn first_unanswered_index(&self) -> Option { + let current_text = self.composer.current_text(); + self.request + .questions + .iter() + .enumerate() + .find(|(idx, _)| !self.is_question_answered(*idx, ¤t_text)) + .map(|(idx, _)| idx) + } + + fn unanswered_confirmation_rows(&self) -> Vec { + let selected = self + .confirm_unanswered + .as_ref() + .and_then(|state| state.selected_idx) + .unwrap_or(0); + let entries = [ + ( + UNANSWERED_CONFIRM_SUBMIT, + self.unanswered_submit_description(), + ), + ( + UNANSWERED_CONFIRM_GO_BACK, + UNANSWERED_CONFIRM_GO_BACK_DESC.to_string(), + ), + ]; + entries + .iter() + .enumerate() + .map(|(idx, (label, description))| { + let prefix = if idx == selected { '›' } else { ' ' }; + let number = idx + 1; + GenericDisplayRow { + name: format!("{prefix} {number}. {label}"), + description: Some(description.clone()), + ..Default::default() + } + }) + .collect() + } + fn is_question_answered(&self, idx: usize, _current_text: &str) -> bool { let Some(question) = self.request.questions.get(idx) else { return false; @@ -618,17 +811,12 @@ impl RequestUserInputOverlay { .as_ref() .is_some_and(|options| !options.is_empty()); if has_options { - answer.committed_option_idx.is_some() + answer.options_state.selected_idx.is_some() && answer.answer_committed } else { - answer.freeform_committed + answer.answer_committed } } - fn current_question_answered(&self) -> bool { - let current_text = self.composer.current_text(); - self.is_question_answered(self.current_index(), ¤t_text) - } - /// Count questions that would submit an empty answer list. fn unanswered_count(&self) -> usize { let current_text = self.composer.current_text(); @@ -660,6 +848,7 @@ impl RequestUserInputOverlay { text: text.clone(), text_elements: text_elements.clone(), local_image_paths: local_image_paths.clone(), + pending_pastes: Vec::new(), }; } self.composer @@ -668,6 +857,17 @@ impl RequestUserInputOverlay { self.composer.set_footer_hint_override(Some(Vec::new())); } + fn apply_submission_draft(&mut self, draft: ComposerDraft) { + if let Some(answer) = self.current_answer_mut() { + answer.draft = draft.clone(); + } + self.composer + .set_text_content(draft.text, draft.text_elements, draft.local_image_paths); + self.composer.set_pending_pastes(draft.pending_pastes); + self.composer.move_cursor_to_end(); + self.composer.set_footer_hint_override(Some(Vec::new())); + } + fn handle_composer_input_result(&mut self, result: InputResult) -> bool { match result { InputResult::Submitted { @@ -684,30 +884,87 @@ impl RequestUserInputOverlay { { let options_len = self.options_len(); if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.clamp_selection(options_len); - answer.committed_option_idx = answer.options_ui_state.selected_idx; + answer.options_state.clamp_selection(options_len); } } - if !self.has_options() - && let Some(answer) = self.current_answer_mut() - { - answer.freeform_committed = !text.trim().is_empty(); + if self.has_options() { + if let Some(answer) = self.current_answer_mut() { + answer.answer_committed = true; + } + } else if let Some(answer) = self.current_answer_mut() { + answer.answer_committed = !text.trim().is_empty(); + } + let draft_override = self.pending_submission_draft.take(); + if let Some(draft) = draft_override { + self.apply_submission_draft(draft); + } else { + self.apply_submission_to_draft(text, text_elements); } - self.apply_submission_to_draft(text, text_elements); self.go_next_or_submit(); true } _ => false, } } + + fn handle_confirm_unanswered_key_event(&mut self, key_event: KeyEvent) { + if key_event.kind == KeyEventKind::Release { + return; + } + let Some(state) = self.confirm_unanswered.as_mut() else { + return; + }; + + match key_event.code { + KeyCode::Esc | KeyCode::Backspace => { + self.close_unanswered_confirmation(); + if let Some(idx) = self.first_unanswered_index() { + self.jump_to_question(idx); + } + } + KeyCode::Up | KeyCode::Char('k') => { + state.move_up_wrap(2); + } + KeyCode::Down | KeyCode::Char('j') => { + state.move_down_wrap(2); + } + KeyCode::Enter => { + let selected = state.selected_idx.unwrap_or(0); + self.close_unanswered_confirmation(); + if selected == 0 { + self.submit_answers(); + } else if let Some(idx) = self.first_unanswered_index() { + self.jump_to_question(idx); + } + } + KeyCode::Char('1') | KeyCode::Char('2') => { + let idx = if matches!(key_event.code, KeyCode::Char('1')) { + 0 + } else { + 1 + }; + state.selected_idx = Some(idx); + } + _ => {} + } + } } impl BottomPaneView for RequestUserInputOverlay { + fn prefer_esc_to_handle_key_event(&self) -> bool { + true + } + fn handle_key_event(&mut self, key_event: KeyEvent) { if key_event.kind == KeyEventKind::Release { return; } + if self.confirm_unanswered_active() { + self.handle_confirm_unanswered_key_event(key_event); + return; + } + if matches!(key_event.code, KeyCode::Esc) { self.app_event_tx.send(AppEvent::CodexOp(Op::Interrupt)); self.done = true; @@ -720,11 +977,21 @@ impl BottomPaneView for RequestUserInputOverlay { code: KeyCode::Char('p'), modifiers: KeyModifiers::CONTROL, .. + } + | KeyEvent { + code: KeyCode::PageUp, + modifiers: KeyModifiers::NONE, + .. } => { self.move_question(false); return; } KeyEvent { + code: KeyCode::PageDown, + modifiers: KeyModifiers::NONE, + .. + } + | KeyEvent { code: KeyCode::Char('n'), modifiers: KeyModifiers::CONTROL, .. @@ -758,7 +1025,8 @@ impl BottomPaneView for RequestUserInputOverlay { match key_event.code { KeyCode::Up | KeyCode::Char('k') => { let moved = if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.move_up_wrap(options_len); + answer.options_state.move_up_wrap(options_len); + answer.answer_committed = false; true } else { false @@ -769,7 +1037,8 @@ impl BottomPaneView for RequestUserInputOverlay { } KeyCode::Down | KeyCode::Char('j') => { let moved = if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.move_down_wrap(options_len); + answer.options_state.move_down_wrap(options_len); + answer.answer_committed = false; true } else { false @@ -779,9 +1048,9 @@ impl BottomPaneView for RequestUserInputOverlay { } } KeyCode::Char(' ') => { - self.select_current_option(); + self.select_current_option(true); } - KeyCode::Backspace => { + KeyCode::Backspace | KeyCode::Delete => { self.clear_selection(); } KeyCode::Tab => { @@ -793,10 +1062,19 @@ impl BottomPaneView for RequestUserInputOverlay { KeyCode::Enter => { let has_selection = self.selected_option_index().is_some(); if has_selection { - self.select_current_option(); + self.select_current_option(true); } self.go_next_or_submit(); } + KeyCode::Char(ch) => { + if let Some(option_idx) = self.option_index_for_digit(ch) { + if let Some(answer) = self.current_answer_mut() { + answer.options_state.selected_idx = Some(option_idx); + } + self.select_current_option(true); + self.go_next_or_submit(); + } + } _ => {} } } @@ -805,6 +1083,7 @@ impl BottomPaneView for RequestUserInputOverlay { if self.has_options() && matches!(key_event.code, KeyCode::Tab) { if let Some(answer) = self.current_answer_mut() { answer.draft = ComposerDraft::default(); + answer.answer_committed = false; answer.notes_visible = false; } self.composer @@ -826,8 +1105,13 @@ impl BottomPaneView for RequestUserInputOverlay { } if matches!(key_event.code, KeyCode::Enter) { self.ensure_selected_for_notes(); + self.pending_submission_draft = Some(self.capture_composer_draft()); let (result, _) = self.composer.handle_key_event(key_event); if !self.handle_composer_input_result(result) { + self.pending_submission_draft = None; + if self.has_options() { + self.select_current_option(true); + } self.go_next_or_submit(); } return; @@ -837,7 +1121,8 @@ impl BottomPaneView for RequestUserInputOverlay { match key_event.code { KeyCode::Up => { let moved = if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.move_up_wrap(options_len); + answer.options_state.move_up_wrap(options_len); + answer.answer_committed = false; true } else { false @@ -848,7 +1133,8 @@ impl BottomPaneView for RequestUserInputOverlay { } KeyCode::Down => { let moved = if let Some(answer) = self.current_answer_mut() { - answer.options_ui_state.move_down_wrap(options_len); + answer.options_state.move_down_wrap(options_len); + answer.answer_committed = false; true } else { false @@ -862,13 +1148,40 @@ impl BottomPaneView for RequestUserInputOverlay { return; } self.ensure_selected_for_notes(); + if matches!( + key_event.code, + KeyCode::Char(_) | KeyCode::Backspace | KeyCode::Delete + ) && let Some(answer) = self.current_answer_mut() + { + answer.answer_committed = false; + } + let before = self.capture_composer_draft(); let (result, _) = self.composer.handle_key_event(key_event); - self.handle_composer_input_result(result); + let submitted = self.handle_composer_input_result(result); + if !submitted { + let after = self.capture_composer_draft(); + if before != after + && let Some(answer) = self.current_answer_mut() + { + answer.answer_committed = false; + } + } } } } fn on_ctrl_c(&mut self) -> CancellationEvent { + if self.confirm_unanswered_active() { + self.close_unanswered_confirmation(); + self.app_event_tx.send(AppEvent::CodexOp(Op::Interrupt)); + self.done = true; + return CancellationEvent::Handled; + } + if self.focus_is_notes() && !self.composer.current_text_with_pending().is_empty() { + self.clear_notes_draft(); + return CancellationEvent::Handled; + } + self.app_event_tx.send(AppEvent::CodexOp(Op::Interrupt)); self.done = true; CancellationEvent::Handled @@ -887,6 +1200,9 @@ impl BottomPaneView for RequestUserInputOverlay { self.focus = Focus::Notes; } self.ensure_selected_for_notes(); + if let Some(answer) = self.current_answer_mut() { + answer.answer_committed = false; + } self.composer.handle_paste(pasted) } @@ -952,6 +1268,29 @@ mod tests { } } + fn question_with_options_and_other(id: &str, header: &str) -> RequestUserInputQuestion { + RequestUserInputQuestion { + id: id.to_string(), + header: header.to_string(), + question: "Choose an option.".to_string(), + is_other: true, + options: Some(vec![ + RequestUserInputQuestionOption { + label: "Option 1".to_string(), + description: "First choice.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Option 2".to_string(), + description: "Second choice.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Option 3".to_string(), + description: "Third choice.".to_string(), + }, + ]), + } + } + fn question_with_wrapped_options(id: &str, header: &str) -> RequestUserInputQuestion { RequestUserInputQuestion { id: id.to_string(), @@ -1109,8 +1448,8 @@ mod tests { overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); assert_eq!(overlay.current_index(), 1); let first_answer = &overlay.answers[0]; - assert_eq!(first_answer.committed_option_idx, Some(0)); - assert_eq!(first_answer.options_ui_state.selected_idx, Some(0)); + assert!(first_answer.answer_committed); + assert_eq!(first_answer.options_state.selected_idx, Some(0)); assert!(rx.try_recv().is_err()); overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -1122,6 +1461,27 @@ mod tests { assert_eq!(answer.answers, vec!["Option 1".to_string()]); } + #[test] + fn number_keys_select_and_submit_options() { + let (tx, mut rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event("turn-1", vec![question_with_options("q1", "Pick one")]), + tx, + true, + false, + false, + ); + + overlay.handle_key_event(KeyEvent::from(KeyCode::Char('2'))); + + let event = rx.try_recv().expect("expected AppEvent"); + let AppEvent::CodexOp(Op::UserInputAnswer { response, .. }) = event else { + panic!("expected UserInputAnswer"); + }; + let answer = response.answers.get("q1").expect("answer missing"); + assert_eq!(answer.answers, vec!["Option 2".to_string()]); + } + #[test] fn vim_keys_move_option_selection() { let (tx, _rx) = test_sender(); @@ -1133,15 +1493,15 @@ mod tests { false, ); let answer = overlay.current_answer().expect("answer missing"); - assert_eq!(answer.options_ui_state.selected_idx, Some(0)); + assert_eq!(answer.options_state.selected_idx, Some(0)); overlay.handle_key_event(KeyEvent::from(KeyCode::Char('j'))); let answer = overlay.current_answer().expect("answer missing"); - assert_eq!(answer.options_ui_state.selected_idx, Some(1)); + assert_eq!(answer.options_state.selected_idx, Some(1)); overlay.handle_key_event(KeyEvent::from(KeyCode::Char('k'))); let answer = overlay.current_answer().expect("answer missing"); - assert_eq!(answer.options_ui_state.selected_idx, Some(0)); + assert_eq!(answer.options_state.selected_idx, Some(0)); } #[test] @@ -1205,8 +1565,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(1); - answer.committed_option_idx = Some(1); + answer.options_state.selected_idx = Some(1); assert_eq!(overlay.notes_ui_visible(), false); overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); @@ -1234,9 +1593,7 @@ mod tests { assert!(matches!(overlay.focus, Focus::Notes)); overlay.move_question(true); - let answer = overlay.current_answer().expect("answer missing"); assert!(matches!(overlay.focus, Focus::Options)); - assert_eq!(answer.committed_option_idx, None); assert_eq!(overlay.notes_ui_visible(), false); } @@ -1264,17 +1621,20 @@ mod tests { overlay.move_question(true); - let answer = overlay.current_answer().expect("answer missing"); assert!(matches!(overlay.focus, Focus::Options)); assert_eq!(overlay.notes_ui_visible(), false); - assert_eq!(answer.committed_option_idx, None); + overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert!(overlay.confirm_unanswered_active()); + overlay.handle_key_event(KeyEvent::from(KeyCode::Char('1'))); overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); let event = rx.try_recv().expect("expected AppEvent"); let AppEvent::CodexOp(Op::UserInputAnswer { response, .. }) = event else { panic!("expected UserInputAnswer"); }; + let answer = response.answers.get("q1").expect("answer missing"); + assert_eq!(answer.answers, Vec::::new()); let answer = response.answers.get("q2").expect("answer missing"); assert_eq!(answer.answers, vec!["Option 1".to_string()]); } @@ -1332,8 +1692,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); overlay.handle_key_event(KeyEvent::from(KeyCode::Esc)); @@ -1357,8 +1716,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); overlay.handle_key_event(KeyEvent::from(KeyCode::Char('a'))); @@ -1383,14 +1741,12 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(1); - answer.committed_option_idx = Some(1); + answer.options_state.selected_idx = Some(1); overlay.handle_key_event(KeyEvent::from(KeyCode::Backspace)); let answer = overlay.current_answer().expect("answer missing"); - assert_eq!(answer.committed_option_idx, None); - assert_eq!(answer.options_ui_state.selected_idx, None); + assert_eq!(answer.options_state.selected_idx, None); assert_eq!(overlay.notes_ui_visible(), false); assert!(rx.try_recv().is_err()); } @@ -1406,8 +1762,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); assert!(matches!(overlay.focus, Focus::Notes)); @@ -1418,7 +1773,7 @@ mod tests { let answer = overlay.current_answer().expect("answer missing"); assert!(matches!(overlay.focus, Focus::Options)); assert_eq!(overlay.notes_ui_visible(), false); - assert_eq!(answer.committed_option_idx, Some(0)); + assert_eq!(answer.options_state.selected_idx, Some(0)); assert!(rx.try_recv().is_err()); } @@ -1433,8 +1788,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); overlay @@ -1448,7 +1802,7 @@ mod tests { assert_eq!(overlay.notes_ui_visible(), false); assert_eq!(overlay.composer.current_text_with_pending(), ""); assert_eq!(answer.draft.text, ""); - assert_eq!(answer.committed_option_idx, Some(0)); + assert_eq!(answer.options_state.selected_idx, Some(0)); assert!(rx.try_recv().is_err()); } @@ -1477,7 +1831,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); + answer.options_state.selected_idx = Some(0); assert_eq!(overlay.unanswered_count(), 1); } @@ -1507,7 +1861,7 @@ mod tests { overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); - assert_eq!(overlay.answers[0].freeform_committed, true); + assert_eq!(overlay.answers[0].answer_committed, true); assert_eq!(overlay.unanswered_count(), 1); } @@ -1530,7 +1884,7 @@ mod tests { overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); - assert_eq!(overlay.answers[0].freeform_committed, false); + assert_eq!(overlay.answers[0].answer_committed, false); assert_eq!(overlay.unanswered_count(), 2); } @@ -1580,6 +1934,48 @@ mod tests { assert_eq!(answer.answers, Vec::::new()); } + #[test] + fn freeform_commit_resets_when_draft_changes() { + let (tx, mut rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![ + question_without_options("q1", "Notes"), + question_without_options("q2", "More"), + ], + ), + tx, + true, + false, + false, + ); + + overlay + .composer + .set_text_content("Committed".to_string(), Vec::new(), Vec::new()); + overlay.composer.move_cursor_to_end(); + overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_eq!(overlay.answers[0].answer_committed, true); + + overlay.move_question(false); + overlay + .composer + .set_text_content("Edited".to_string(), Vec::new(), Vec::new()); + overlay.composer.move_cursor_to_end(); + overlay.move_question(true); + assert_eq!(overlay.answers[0].answer_committed, false); + + overlay.submit_answers(); + + let event = rx.try_recv().expect("expected AppEvent"); + let AppEvent::CodexOp(Op::UserInputAnswer { response, .. }) = event else { + panic!("expected UserInputAnswer"); + }; + let answer = response.answers.get("q1").expect("answer missing"); + assert_eq!(answer.answers, Vec::::new()); + } + #[test] fn notes_are_captured_for_selected_option() { let (tx, mut rx) = test_sender(); @@ -1593,13 +1989,18 @@ mod tests { { let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(1); + answer.options_state.selected_idx = Some(1); } - overlay.select_current_option(); + overlay.select_current_option(false); overlay .composer .set_text_content("Notes for option 2".to_string(), Vec::new(), Vec::new()); overlay.composer.move_cursor_to_end(); + let draft = overlay.capture_composer_draft(); + if let Some(answer) = overlay.current_answer_mut() { + answer.draft = draft; + answer.answer_committed = true; + } overlay.submit_answers(); @@ -1645,7 +2046,61 @@ mod tests { assert_eq!(overlay.current_index(), 1); let answer = overlay.answers.first().expect("answer missing"); - assert_eq!(answer.committed_option_idx, Some(1)); + assert_eq!(answer.options_state.selected_idx, Some(1)); + assert!(answer.answer_committed); + } + + #[test] + fn is_other_adds_none_of_the_above_and_submits_it() { + let (tx, mut rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![question_with_options_and_other("q1", "Pick one")], + ), + tx, + true, + false, + false, + ); + + let rows = overlay.option_rows(); + let other_row = rows.last().expect("expected none-of-the-above row"); + assert_eq!(other_row.name, " 4. None of the above"); + assert_eq!( + other_row.description.as_deref(), + Some(OTHER_OPTION_DESCRIPTION) + ); + + let other_idx = overlay.options_len().saturating_sub(1); + { + let answer = overlay.current_answer_mut().expect("answer missing"); + answer.options_state.selected_idx = Some(other_idx); + } + overlay + .composer + .set_text_content("Custom answer".to_string(), Vec::new(), Vec::new()); + overlay.composer.move_cursor_to_end(); + let draft = overlay.capture_composer_draft(); + if let Some(answer) = overlay.current_answer_mut() { + answer.draft = draft; + answer.answer_committed = true; + } + + overlay.submit_answers(); + + let event = rx.try_recv().expect("expected AppEvent"); + let AppEvent::CodexOp(Op::UserInputAnswer { response, .. }) = event else { + panic!("expected UserInputAnswer"); + }; + let answer = response.answers.get("q1").expect("answer missing"); + assert_eq!( + answer.answers, + vec![ + OTHER_OPTION_LABEL.to_string(), + "user_note: Custom answer".to_string(), + ] + ); } #[test] @@ -1669,7 +2124,42 @@ mod tests { overlay.composer.handle_paste(large.clone()); overlay.move_question(true); - assert_eq!(overlay.answers[0].draft.text, large); + let draft = &overlay.answers[0].draft; + assert_eq!(draft.pending_pastes.len(), 1); + assert_eq!(draft.pending_pastes[0].1, large); + assert!(draft.text.contains(&draft.pending_pastes[0].0)); + assert_eq!(draft.text_with_pending(), large); + } + + #[test] + fn pending_paste_placeholder_survives_submission_and_back_navigation() { + let (tx, _rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![ + question_with_options("q1", "First"), + question_with_options("q2", "Second"), + ], + ), + tx, + true, + false, + false, + ); + + let large = "x".repeat(1_200); + overlay.focus = Focus::Notes; + overlay.ensure_selected_for_notes(); + overlay.composer.handle_paste(large.clone()); + + overlay.handle_key_event(KeyEvent::from(KeyCode::Enter)); + overlay.handle_key_event(KeyEvent::new(KeyCode::Char('p'), KeyModifiers::CONTROL)); + + let draft = &overlay.answers[0].draft; + assert_eq!(draft.pending_pastes.len(), 1); + assert!(draft.text.contains(&draft.pending_pastes[0].0)); + assert_eq!(draft.text_with_pending(), large); } #[test] @@ -1701,8 +2191,7 @@ mod tests { ); { let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); } overlay.handle_key_event(KeyEvent::from(KeyCode::Tab)); @@ -1747,9 +2236,8 @@ mod tests { let width = 48u16; let question_height = overlay.wrapped_question_lines(width).len() as u16; let options_height = overlay.options_required_height(width); - let extras = 1u16 // header - .saturating_add(1) // progress - .saturating_add(DESIRED_SPACERS_WHEN_NOTES_HIDDEN) + let extras = 1u16 // progress + .saturating_add(DESIRED_SPACERS_BETWEEN_SECTIONS) .saturating_add(overlay.footer_required_height(width)); let height = question_height .saturating_add(options_height) @@ -1783,7 +2271,7 @@ mod tests { let question_bottom = sections.question_area.y + sections.question_area.height; let options_bottom = sections.options_area.y + sections.options_area.height; let spacer_after_question = sections.options_area.y.saturating_sub(question_bottom); - let spacer_after_options = sections.notes_title_area.y.saturating_sub(options_bottom); + let spacer_after_options = sections.notes_area.y.saturating_sub(options_bottom); assert_eq!(spacer_after_question, 1); assert_eq!(spacer_after_options, 1); } @@ -1805,8 +2293,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); let width = 36u16; let lines = overlay.footer_tip_lines(width); @@ -1842,8 +2329,7 @@ mod tests { { let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(0); - answer.committed_option_idx = Some(0); + answer.options_state.selected_idx = Some(0); } let width = 110u16; @@ -1877,8 +2363,7 @@ mod tests { false, ); let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(1); - answer.committed_option_idx = Some(1); + answer.options_state.selected_idx = Some(1); let width = 52u16; let height = overlay.desired_height(width); @@ -1931,8 +2416,7 @@ mod tests { ); { let answer = overlay.current_answer_mut().expect("answer missing"); - answer.options_ui_state.selected_idx = Some(3); - answer.committed_option_idx = Some(3); + answer.options_state.selected_idx = Some(3); } let area = Rect::new(0, 0, 120, 12); insta::assert_snapshot!( @@ -1941,6 +2425,57 @@ mod tests { ); } + #[test] + fn request_user_input_hidden_options_footer_snapshot() { + let (tx, _rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![RequestUserInputQuestion { + id: "q1".to_string(), + header: "Next Step".to_string(), + question: "What would you like to do next?".to_string(), + is_other: false, + options: Some(vec![ + RequestUserInputQuestionOption { + label: "Discuss a code change (Recommended)".to_string(), + description: "Walk through a plan and edit code together.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Run tests".to_string(), + description: "Pick a crate and run its tests.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Review a diff".to_string(), + description: "Summarize or review current changes.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Refactor".to_string(), + description: "Tighten structure and remove dead code.".to_string(), + }, + RequestUserInputQuestionOption { + label: "Ship it".to_string(), + description: "Finalize and open a PR.".to_string(), + }, + ]), + }], + ), + tx, + true, + false, + false, + ); + { + let answer = overlay.current_answer_mut().expect("answer missing"); + answer.options_state.selected_idx = Some(3); + } + let area = Rect::new(0, 0, 80, 10); + insta::assert_snapshot!( + "request_user_input_hidden_options_footer", + render_snapshot(&overlay, area) + ); + } + #[test] fn request_user_input_freeform_snapshot() { let (tx, _rx) = test_sender(); @@ -2005,6 +2540,32 @@ mod tests { ); } + #[test] + fn request_user_input_unanswered_confirmation_snapshot() { + let (tx, _rx) = test_sender(); + let mut overlay = RequestUserInputOverlay::new( + request_event( + "turn-1", + vec![ + question_with_options("q1", "Area"), + question_without_options("q2", "Goal"), + ], + ), + tx, + true, + false, + false, + ); + + overlay.open_unanswered_confirmation(); + + let area = Rect::new(0, 0, 80, 12); + insta::assert_snapshot!( + "request_user_input_unanswered_confirmation", + render_snapshot(&overlay, area) + ); + } + #[test] fn options_scroll_while_editing_notes() { let (tx, _rx) = test_sender(); @@ -2015,7 +2576,7 @@ mod tests { false, false, ); - overlay.select_current_option(); + overlay.select_current_option(false); overlay.focus = Focus::Notes; overlay .composer @@ -2025,7 +2586,7 @@ mod tests { overlay.handle_key_event(KeyEvent::from(KeyCode::Down)); let answer = overlay.current_answer().expect("answer missing"); - assert_eq!(answer.committed_option_idx, Some(0)); - assert_eq!(answer.options_ui_state.selected_idx, Some(1)); + assert_eq!(answer.options_state.selected_idx, Some(1)); + assert!(!answer.answer_committed); } } diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/render.rs b/codex-rs/tui/src/bottom_pane/request_user_input/render.rs index 0ab53fb84..b7433f8a6 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/render.rs +++ b/codex-rs/tui/src/bottom_pane/request_user_input/render.rs @@ -5,21 +5,64 @@ use ratatui::text::Line; use ratatui::text::Span; use ratatui::widgets::Paragraph; use ratatui::widgets::Widget; +use std::borrow::Cow; use unicode_width::UnicodeWidthChar; use unicode_width::UnicodeWidthStr; +use crate::bottom_pane::popup_consts::standard_popup_hint_line; +use crate::bottom_pane::scroll_state::ScrollState; +use crate::bottom_pane::selection_popup_common::measure_rows_height; 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::bottom_pane::selection_popup_common::wrap_styled_line; use crate::render::renderable::Renderable; -use super::DESIRED_SPACERS_WHEN_NOTES_HIDDEN; +use super::DESIRED_SPACERS_BETWEEN_SECTIONS; use super::RequestUserInputOverlay; use super::TIP_SEPARATOR; +const MIN_OVERLAY_HEIGHT: usize = 8; +const PROGRESS_ROW_HEIGHT: usize = 1; +const SPACER_ROWS_WITH_NOTES: usize = 1; +const SPACER_ROWS_NO_OPTIONS: usize = 0; + +struct UnansweredConfirmationData { + title_line: Line<'static>, + subtitle_line: Line<'static>, + hint_line: Line<'static>, + rows: Vec, + state: ScrollState, +} + +struct UnansweredConfirmationLayout { + header_lines: Vec>, + hint_lines: Vec>, + rows: Vec, + state: ScrollState, +} + +fn line_to_owned(line: Line<'_>) -> Line<'static> { + Line { + style: line.style, + alignment: line.alignment, + spans: line + .spans + .into_iter() + .map(|span| Span { + style: span.style, + content: Cow::Owned(span.content.into_owned()), + }) + .collect(), + } +} + impl Renderable for RequestUserInputOverlay { fn desired_height(&self, width: u16) -> u16 { + if self.confirm_unanswered_active() { + return self.unanswered_confirmation_height(width); + } let outer = Rect::new(0, 0, width, u16::MAX); let inner = menu_surface_inset(outer); let inner_width = inner.width.max(1); @@ -36,26 +79,29 @@ impl Renderable for RequestUserInputOverlay { } else { 0 }; - let spacer_rows = if has_options && !notes_visible { - DESIRED_SPACERS_WHEN_NOTES_HIDDEN as usize + // When notes are visible, the composer already separates options from the footer. + // Without notes, we keep extra spacing so the footer hints don't crowd the options. + let spacer_rows = if has_options { + if notes_visible { + SPACER_ROWS_WITH_NOTES + } else { + DESIRED_SPACERS_BETWEEN_SECTIONS as usize + } } else { - 0 + SPACER_ROWS_NO_OPTIONS }; let footer_height = self.footer_required_height(inner_width) as usize; - // Tight minimum height: progress + header + question + (optional) titles/options + // Tight minimum height: progress + 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 has_options && notes_visible { - height = height.saturating_add(1); // notes title - } + .saturating_add(PROGRESS_ROW_HEIGHT); // progress height = height.saturating_add(menu_surface_padding_height() as usize); - height.max(8) as u16 + height.max(MIN_OVERLAY_HEIGHT) as u16 } fn render(&self, area: Rect, buf: &mut Buffer) { @@ -68,11 +114,145 @@ impl Renderable for RequestUserInputOverlay { } impl RequestUserInputOverlay { + fn unanswered_confirmation_data(&self) -> UnansweredConfirmationData { + let unanswered = self.unanswered_question_count(); + let subtitle = format!( + "{unanswered} unanswered question{}", + if unanswered == 1 { "" } else { "s" } + ); + UnansweredConfirmationData { + title_line: Line::from(super::UNANSWERED_CONFIRM_TITLE.bold()), + subtitle_line: Line::from(subtitle.dim()), + hint_line: standard_popup_hint_line(), + rows: self.unanswered_confirmation_rows(), + state: self.confirm_unanswered.unwrap_or_default(), + } + } + + fn unanswered_confirmation_layout(&self, width: u16) -> UnansweredConfirmationLayout { + let data = self.unanswered_confirmation_data(); + let content_width = width.max(1); + let mut header_lines = wrap_styled_line(&data.title_line, content_width); + let mut subtitle_lines = wrap_styled_line(&data.subtitle_line, content_width); + header_lines.append(&mut subtitle_lines); + let header_lines = header_lines.into_iter().map(line_to_owned).collect(); + let hint_lines = wrap_styled_line(&data.hint_line, content_width) + .into_iter() + .map(line_to_owned) + .collect(); + UnansweredConfirmationLayout { + header_lines, + hint_lines, + rows: data.rows, + state: data.state, + } + } + + fn unanswered_confirmation_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 layout = self.unanswered_confirmation_layout(inner_width); + let rows_height = measure_rows_height( + &layout.rows, + &layout.state, + layout.rows.len().max(1), + inner_width.max(1), + ); + let height = layout.header_lines.len() as u16 + + 1 + + rows_height + + 1 + + layout.hint_lines.len() as u16 + + menu_surface_padding_height(); + height.max(MIN_OVERLAY_HEIGHT as u16) + } + + fn render_unanswered_confirmation(&self, area: Rect, buf: &mut Buffer) { + let content_area = render_menu_surface(area, buf); + if content_area.width == 0 || content_area.height == 0 { + return; + } + let width = content_area.width.max(1); + let layout = self.unanswered_confirmation_layout(width); + + let mut cursor_y = content_area.y; + for line in layout.header_lines { + if cursor_y >= content_area.y + content_area.height { + return; + } + Paragraph::new(line).render( + Rect { + x: content_area.x, + y: cursor_y, + width: content_area.width, + height: 1, + }, + buf, + ); + cursor_y = cursor_y.saturating_add(1); + } + + if cursor_y < content_area.y + content_area.height { + cursor_y = cursor_y.saturating_add(1); + } + + let remaining = content_area + .height + .saturating_sub(cursor_y.saturating_sub(content_area.y)); + if remaining == 0 { + return; + } + + let hint_height = layout.hint_lines.len() as u16; + let spacer_before_hint = u16::from(remaining > hint_height); + let rows_height = remaining.saturating_sub(hint_height + spacer_before_hint); + + let rows_area = Rect { + x: content_area.x, + y: cursor_y, + width: content_area.width, + height: rows_height, + }; + render_rows( + rows_area, + buf, + &layout.rows, + &layout.state, + layout.rows.len().max(1), + "No choices", + ); + + cursor_y = cursor_y.saturating_add(rows_height); + if spacer_before_hint > 0 { + cursor_y = cursor_y.saturating_add(1); + } + for (offset, line) in layout.hint_lines.into_iter().enumerate() { + let y = cursor_y.saturating_add(offset as u16); + if y >= content_area.y + content_area.height { + break; + } + Paragraph::new(line).render( + Rect { + x: content_area.x, + y, + width: content_area.width, + height: 1, + }, + buf, + ); + } + } + /// Render the full request-user-input overlay. pub(super) fn render_ui(&self, area: Rect, buf: &mut Buffer) { if area.width == 0 || area.height == 0 { return; } + if self.confirm_unanswered_active() { + self.render_unanswered_confirmation(area, buf); + return; + } // Paint the same menu surface used by other bottom-pane overlays and // then render the overlay content inside its inset area. let content_area = render_menu_surface(area, buf); @@ -98,28 +278,22 @@ impl RequestUserInputOverlay { }; Paragraph::new(progress_line).render(sections.progress_area, buf); - // 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 { - if answered { - Line::from(header.bold()) - } else { - Line::from(header.cyan().bold()) - } - } else { - Line::from("No questions".dim()) - }; - Paragraph::new(header_line).render(sections.header_area, buf); - + // Question prompt text. let question_y = sections.question_area.y; + let answered = + self.is_question_answered(self.current_index(), &self.composer.current_text()); for (offset, line) in sections.question_lines.iter().enumerate() { if question_y.saturating_add(offset as u16) >= sections.question_area.y + sections.question_area.height { break; } - Paragraph::new(Line::from(line.clone())).render( + let question_line = if answered { + Line::from(line.clone()) + } else { + Line::from(line.clone()).cyan() + }; + Paragraph::new(question_line).render( Rect { x: sections.question_area.x, y: question_y.saturating_add(offset as u16), @@ -134,48 +308,25 @@ impl RequestUserInputOverlay { let option_rows = self.option_rows(); if self.has_options() { - let mut options_ui_state = self + let mut options_state = self .current_answer() - .map(|answer| answer.options_ui_state) + .map(|answer| answer.options_state) .unwrap_or_default(); if sections.options_area.height > 0 { // Ensure the selected option is visible in the scroll window. - options_ui_state + options_state .ensure_visible(option_rows.len(), sections.options_area.height as usize); render_rows( sections.options_area, buf, &option_rows, - &options_ui_state, + &options_state, option_rows.len().max(1), "No options", ); } } - if notes_visible && sections.notes_title_area.height > 0 { - let notes_label = if self.has_options() - && self - .current_answer() - .is_some_and(|answer| answer.committed_option_idx.is_some()) - { - if let Some(label) = self.current_option_label() { - format!("Notes for {label}") - } else { - "Notes".to_string() - } - } else { - "Notes".to_string() - }; - 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() - }; - Paragraph::new(Line::from(notes_title)).render(sections.notes_title_area, buf); - } - if notes_visible && sections.notes_area.height > 0 { self.render_notes_input(sections.notes_area, buf); } @@ -193,7 +344,17 @@ impl RequestUserInputOverlay { if footer_area.height == 0 { return; } - let tip_lines = self.footer_tip_lines(footer_area.width); + let options_hidden = self.has_options() + && sections.options_area.height > 0 + && self.options_required_height(content_area.width) > sections.options_area.height; + let option_tip = if options_hidden { + let selected = self.selected_option_index().unwrap_or(0).saturating_add(1); + let total = self.options_len(); + Some(super::FooterTip::new(format!("option {selected}/{total}"))) + } else { + None + }; + let tip_lines = self.footer_tip_lines_with_prefix(footer_area.width, option_tip); for (row_idx, tips) in tip_lines .into_iter() .take(footer_area.height as usize) @@ -224,6 +385,9 @@ impl RequestUserInputOverlay { /// Return the cursor position when editing notes, if visible. pub(super) fn cursor_pos_impl(&self, area: Rect) -> Option<(u16, u16)> { + if self.confirm_unanswered_active() { + return None; + } let has_options = self.has_options(); let notes_visible = self.notes_ui_visible(); diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_footer_wrap.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_footer_wrap.snap index 02f91a7b4..3fd719464 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_footer_wrap.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_footer_wrap.snap @@ -3,14 +3,12 @@ source: tui/src/bottom_pane/request_user_input/mod.rs expression: "render_snapshot(&overlay, area)" --- - Question 1/2 (1 unanswered) - Pick one + Question 1/2 (2 unanswered) Choose an option. - ( ) Option 1 First choice. - (x) Option 2 Second choice. - ( ) Option 3 Third choice. + 1. Option 1 First choice. + › 2. Option 2 Second choice. + 3. Option 3 Third choice. - Option 2 of 3 | ↑/↓ scroll | Tab: add notes - Enter: submit answer | Ctrl+n next - Esc: interrupt + tab to add notes | enter to submit answer + ctrl + n next question | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_freeform.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_freeform.snap index 3e947c015..3ae7b9d62 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_freeform.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_freeform.snap @@ -4,10 +4,10 @@ expression: "render_snapshot(&overlay, area)" --- Question 1/1 (1 unanswered) - Goal Share details. › Type your answer (optional) - Enter: submit answer | Esc: interrupt + + enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_hidden_options_footer.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_hidden_options_footer.snap new file mode 100644 index 000000000..d643647f7 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_hidden_options_footer.snap @@ -0,0 +1,13 @@ +--- +source: tui/src/bottom_pane/request_user_input/mod.rs +expression: "render_snapshot(&overlay, area)" +--- + + Question 1/1 (1 unanswered) + What would you like to do next? + + 2. Run tests Pick a crate and run its tests. + 3. Review a diff Summarize or review current changes. + › 4. Refactor Tighten structure and remove dead code. + + option 4/5 | tab to add notes | enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_first.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_first.snap index cdae6785d..536e34dbb 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_first.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_first.snap @@ -4,11 +4,10 @@ expression: "render_snapshot(&overlay, area)" --- Question 1/2 (2 unanswered) - Area Choose an option. - ( ) Option 1 First choice. - ( ) Option 2 Second choice. - ( ) Option 3 Third choice. + › 1. Option 1 First choice. + 2. Option 2 Second choice. + 3. Option 3 Third choice. - Option 1 of 3 | ↑/↓ scroll | Tab: add notes | Enter: submit answer | Ctrl+n next | Esc: interrupt + tab to add notes | enter to submit answer | ctrl + n next question | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_last.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_last.snap index d007b6b43..95507c335 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_last.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_multi_question_last.snap @@ -4,7 +4,6 @@ expression: "render_snapshot(&overlay, area)" --- Question 2/2 (2 unanswered) - Goal Share details. › Type your answer (optional) @@ -12,4 +11,5 @@ expression: "render_snapshot(&overlay, area)" - Enter: submit all answers | Ctrl+n next | Esc: interrupt + + enter to submit all | ctrl + n first question | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options.snap index 67eb6dc71..c93576246 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options.snap @@ -4,11 +4,10 @@ expression: "render_snapshot(&overlay, area)" --- Question 1/1 (1 unanswered) - Area Choose an option. - ( ) Option 1 First choice. - ( ) Option 2 Second choice. - ( ) Option 3 Third choice. + › 1. Option 1 First choice. + 2. Option 2 Second choice. + 3. Option 3 Third choice. - Option 1 of 3 | ↑/↓ scroll | Tab: add notes | Enter: submit answer | Esc: interrupt + tab to add notes | enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options_notes_visible.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options_notes_visible.snap index 24799ed63..d9d219b62 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options_notes_visible.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_options_notes_visible.snap @@ -3,17 +3,17 @@ source: tui/src/bottom_pane/request_user_input/mod.rs expression: "render_snapshot(&overlay, area)" --- - Question 1/1 - Area + Question 1/1 (1 unanswered) Choose an option. - (x) Option 1 First choice. - ( ) Option 2 Second choice. - ( ) Option 3 Third choice. - Notes for Option 1 + + › 1. Option 1 First choice. + 2. Option 2 Second choice. + 3. Option 3 Third choice. › Add notes - Option 1 of 3 | ↑/↓ scroll | Tab: clear notes | Enter: submit answer | Esc: interrupt + + tab to clear notes | enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_scrolling_options.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_scrolling_options.snap index 95813336b..2e8d120e4 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_scrolling_options.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_scrolling_options.snap @@ -3,13 +3,13 @@ source: tui/src/bottom_pane/request_user_input/mod.rs expression: "render_snapshot(&overlay, area)" --- - Question 1/1 - Next Step + Question 1/1 (1 unanswered) What would you like to do next? - ( ) Discuss a code change (Recommended) Walk through a plan and edit code together. - ( ) Run tests Pick a crate and run its tests. - ( ) Review a diff Summarize or review current changes. - (x) Refactor Tighten structure and remove dead code. + 1. Discuss a code change (Recommended) Walk through a plan and edit code together. + 2. Run tests Pick a crate and run its tests. + 3. Review a diff Summarize or review current changes. + › 4. Refactor Tighten structure and remove dead code. + 5. Ship it Finalize and open a PR. - Option 4 of 5 | ↑/↓ scroll | Tab: add notes | Enter: submit answer | Esc: interrupt + tab to add notes | enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_tight_height.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_tight_height.snap index a963370be..c93576246 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_tight_height.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_tight_height.snap @@ -4,10 +4,10 @@ expression: "render_snapshot(&overlay, area)" --- Question 1/1 (1 unanswered) - Area Choose an option. - ( ) Option 1 First choice. - ( ) Option 2 Second choice. + › 1. Option 1 First choice. + 2. Option 2 Second choice. + 3. Option 3 Third choice. - Option 1 of 3 | ↑/↓ scroll | Tab: add notes | Enter: submit answer | Esc: interrupt + tab to add notes | enter to submit answer | esc to interrupt diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_unanswered_confirmation.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_unanswered_confirmation.snap new file mode 100644 index 000000000..dd689c726 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_unanswered_confirmation.snap @@ -0,0 +1,15 @@ +--- +source: tui/src/bottom_pane/request_user_input/mod.rs +expression: "render_snapshot(&overlay, area)" +--- + + Submit with unanswered questions? + 2 unanswered questions + + › 1. Proceed Submit with 2 unanswered questions. + 2. Go back Return to the first unanswered question. + + + + + Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_wrapped_options.snap b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_wrapped_options.snap index 0d1e72742..71d32c5ab 100644 --- a/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_wrapped_options.snap +++ b/codex-rs/tui/src/bottom_pane/request_user_input/snapshots/codex_tui__bottom_pane__request_user_input__tests__request_user_input_wrapped_options.snap @@ -3,12 +3,11 @@ source: tui/src/bottom_pane/request_user_input/mod.rs expression: "render_snapshot(&overlay, area)" --- - Question 1/1 - Next Step + Question 1/1 (1 unanswered) Choose the next step for this task. - (x) Discuss a code change Walk through a plan, then implement it together with careful checks. - ( ) Run targeted tests Pick the most relevant crate and validate the current behavior first. - ( ) Review the diff Summarize the changes and highlight the most important risks and gaps. + › 1. Discuss a code change Walk through a plan, then implement it together with careful checks. + 2. Run targeted tests Pick the most relevant crate and validate the current behavior first. + 3. Review the diff Summarize the changes and highlight the most important risks and gaps. - Option 1 of 3 | ↑/↓ scroll | Tab: add notes | Enter: submit answer | Esc: interrupt + tab to add notes | enter to submit answer | esc to interrupt