From d4848e558b067f689f51d7a28a305716663fa883 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Fri, 12 Sep 2025 15:31:24 -0400 Subject: [PATCH] Add spacing before composer footer hints (#3469) image --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 97 ++++++++++++++++--- ...mposer__tests__backspace_after_pastes.snap | 2 +- ...tom_pane__chat_composer__tests__empty.snap | 2 +- ...tom_pane__chat_composer__tests__large.snap | 2 +- ...chat_composer__tests__multiple_pastes.snap | 2 +- ...tom_pane__chat_composer__tests__small.snap | 2 +- ...chatwidget__tests__chat_small_idle_h2.snap | 2 +- ...exec_and_status_layout_vt100_snapshot.snap | 1 + ...atwidget__tests__status_widget_active.snap | 1 + 9 files changed, 92 insertions(+), 19 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 410701a6..065232d8 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -95,6 +95,10 @@ enum ActivePopup { File(FileSearchPopup), } +const FOOTER_HINT_HEIGHT: u16 = 1; +const FOOTER_SPACING_HEIGHT: u16 = 1; +const FOOTER_HEIGHT_WITH_HINT: u16 = FOOTER_HINT_HEIGHT + FOOTER_SPACING_HEIGHT; + impl ChatComposer { pub fn new( has_input_focus: bool, @@ -134,20 +138,20 @@ impl ChatComposer { pub fn desired_height(&self, width: u16) -> u16 { self.textarea.desired_height(width - 1) + match &self.active_popup { - ActivePopup::None => 1u16, + ActivePopup::None => FOOTER_HEIGHT_WITH_HINT, ActivePopup::Command(c) => c.calculate_required_height(), ActivePopup::File(c) => c.calculate_required_height(), } } pub fn cursor_pos(&self, area: Rect) -> Option<(u16, u16)> { - let popup_height = match &self.active_popup { - ActivePopup::Command(popup) => popup.calculate_required_height(), - ActivePopup::File(popup) => popup.calculate_required_height(), - ActivePopup::None => 1, + let popup_constraint = match &self.active_popup { + ActivePopup::Command(popup) => Constraint::Max(popup.calculate_required_height()), + ActivePopup::File(popup) => Constraint::Max(popup.calculate_required_height()), + ActivePopup::None => Constraint::Max(FOOTER_HEIGHT_WITH_HINT), }; let [textarea_rect, _] = - Layout::vertical([Constraint::Min(1), Constraint::Max(popup_height)]).areas(area); + Layout::vertical([Constraint::Min(1), popup_constraint]).areas(area); let mut textarea_rect = textarea_rect; textarea_rect.width = textarea_rect.width.saturating_sub(1); textarea_rect.x += 1; @@ -1223,13 +1227,16 @@ impl ChatComposer { impl WidgetRef for ChatComposer { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let popup_height = match &self.active_popup { - ActivePopup::Command(popup) => popup.calculate_required_height(), - ActivePopup::File(popup) => popup.calculate_required_height(), - ActivePopup::None => 1, + let (popup_constraint, hint_spacing) = match &self.active_popup { + ActivePopup::Command(popup) => (Constraint::Max(popup.calculate_required_height()), 0), + ActivePopup::File(popup) => (Constraint::Max(popup.calculate_required_height()), 0), + ActivePopup::None => ( + Constraint::Length(FOOTER_HEIGHT_WITH_HINT), + FOOTER_SPACING_HEIGHT, + ), }; let [textarea_rect, popup_rect] = - Layout::vertical([Constraint::Min(1), Constraint::Max(popup_height)]).areas(area); + Layout::vertical([Constraint::Min(1), popup_constraint]).areas(area); match &self.active_popup { ActivePopup::Command(popup) => { popup.render_ref(popup_rect, buf); @@ -1238,7 +1245,16 @@ impl WidgetRef for ChatComposer { popup.render_ref(popup_rect, buf); } ActivePopup::None => { - let bottom_line_rect = popup_rect; + let hint_rect = if hint_spacing > 0 { + let [_, hint_rect] = Layout::vertical([ + Constraint::Length(hint_spacing), + Constraint::Length(FOOTER_HINT_HEIGHT), + ]) + .areas(popup_rect); + hint_rect + } else { + popup_rect + }; let mut hint: Vec> = if self.ctrl_c_quit_hint { let ctrl_c_followup = if self.is_task_running { " to interrupt" @@ -1309,7 +1325,7 @@ impl WidgetRef for ChatComposer { Line::from(hint) .style(Style::default().dim()) - .render_ref(bottom_line_rect, buf); + .render_ref(hint_rect, buf); } } let border_style = if self.has_focus { @@ -1344,6 +1360,7 @@ mod tests { use super::*; use image::ImageBuffer; use image::Rgba; + use pretty_assertions::assert_eq; use std::path::PathBuf; use tempfile::tempdir; @@ -1356,6 +1373,60 @@ mod tests { use crate::bottom_pane::textarea::TextArea; use tokio::sync::mpsc::unbounded_channel; + #[test] + fn footer_hint_row_is_separated_from_composer() { + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + let area = Rect::new(0, 0, 40, 6); + let mut buf = Buffer::empty(area); + composer.render_ref(area, &mut buf); + + let row_to_string = |y: u16| { + let mut row = String::new(); + for x in 0..area.width { + row.push(buf[(x, y)].symbol().chars().next().unwrap_or(' ')); + } + row + }; + + let mut hint_row: Option<(u16, String)> = None; + for y in 0..area.height { + let row = row_to_string(y); + if row.contains(" send") { + hint_row = Some((y, row)); + break; + } + } + + let (hint_row_idx, hint_row_contents) = + hint_row.expect("expected footer hint row to be rendered"); + assert_eq!( + hint_row_idx, + area.height - 1, + "hint row should occupy the bottom line: {hint_row_contents:?}", + ); + + assert!( + hint_row_idx > 0, + "expected a spacing row above the footer hints", + ); + + let spacing_row = row_to_string(hint_row_idx - 1); + assert_eq!( + spacing_row.trim(), + "", + "expected blank spacing row above hints but saw: {spacing_row:?}", + ); + } + #[test] fn test_current_at_token_basic_cases() { let test_cases = vec![ diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap index 0c278fd7..eafcf945 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__backspace_after_pastes.snap @@ -10,5 +10,5 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -"▌ " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap index c6192485..8de047f1 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__empty.snap @@ -10,5 +10,5 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -"▌ " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap index 1660bb49..b9e96aed 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__large.snap @@ -10,5 +10,5 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -"▌ " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap index 21869a07..cb546595 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__multiple_pastes.snap @@ -10,5 +10,5 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -"▌ " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap index 1f257317..8843b4a5 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__small.snap @@ -10,5 +10,5 @@ expression: terminal.backend() "▌ " "▌ " "▌ " -"▌ " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap index 41f43bab..48e6154e 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chat_small_idle_h2.snap @@ -3,4 +3,4 @@ source: tui/src/chatwidget/tests.rs expression: terminal.backend() --- "▌ Ask Codex to do anything " -" ⏎ send ⌃J newline ⌃T transcript ⌃" +" " diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap index c6d5b002..df4fd4ca 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__chatwidget_exec_and_status_layout_vt100_snapshot.snap @@ -12,4 +12,5 @@ expression: visual Investigating rendering code (0s • Esc to interrupt) ▌Summarize recent commits + ⏎ send ⌃J newline ⌃T transcript ⌃C quit diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap index 088b43a7..8eb6e72a 100644 --- a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__status_widget_active.snap @@ -6,5 +6,6 @@ expression: terminal.backend() " Analyzing (0s • Esc to interrupt) " " " "▌ Ask Codex to do anything " +" " " ⏎ send ⌃J newline ⌃T transcript ⌃C quit " " "