From 58bed77ba75518c4b097a7e802fdd84f5d53a749 Mon Sep 17 00:00:00 2001 From: easong-openai Date: Sun, 27 Jul 2025 11:04:09 -0700 Subject: [PATCH] Remove tab focus switching (#1694) Previously pressing tab would switch TUI focus to the history scrollbox - no longer necessary. --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 11 --- codex-rs/tui/src/bottom_pane/mod.rs | 11 --- codex-rs/tui/src/chatwidget.rs | 45 ++--------- .../tui/src/conversation_history_widget.rs | 76 ------------------- 4 files changed, 5 insertions(+), 138 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index bdfb6a23..6a1bb526 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -127,10 +127,6 @@ impl ChatComposer<'_> { .on_entry_response(log_id, offset, entry, &mut self.textarea) } - pub fn set_input_focus(&mut self, has_focus: bool) { - self.update_border(has_focus); - } - pub fn handle_paste(&mut self, pasted: String) -> bool { let char_count = pasted.chars().count(); if char_count > LARGE_PASTE_CHAR_THRESHOLD { @@ -638,13 +634,6 @@ impl ChatComposer<'_> { .border_style(bs.border_style), ); } - - pub(crate) fn is_popup_visible(&self) -> bool { - match self.active_popup { - ActivePopup::Command(_) | ActivePopup::File(_) => true, - ActivePopup::None => false, - } - } } impl WidgetRef for &ChatComposer<'_> { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ebec534f..0ddb36f6 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -104,12 +104,6 @@ impl BottomPane<'_> { } } - /// Update the UI to reflect whether this `BottomPane` has input focus. - pub(crate) fn set_input_focus(&mut self, has_focus: bool) { - self.has_input_focus = has_focus; - self.composer.set_input_focus(has_focus); - } - pub(crate) fn show_ctrl_c_quit_hint(&mut self) { self.ctrl_c_quit_hint = true; self.composer @@ -203,11 +197,6 @@ impl BottomPane<'_> { self.app_event_tx.send(AppEvent::RequestRedraw) } - /// Returns true when a popup inside the composer is visible. - pub(crate) fn is_popup_visible(&self) -> bool { - self.active_view.is_none() && self.composer.is_popup_visible() - } - // --- History helpers --- pub(crate) fn set_history_metadata(&mut self, log_id: u64, entry_count: usize) { diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 67447073..01285c02 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -44,7 +44,6 @@ pub(crate) struct ChatWidget<'a> { codex_op_tx: UnboundedSender, conversation_history: ConversationHistoryWidget, bottom_pane: BottomPane<'a>, - input_focus: InputFocus, config: Config, initial_user_message: Option, token_usage: TokenUsage, @@ -55,12 +54,6 @@ pub(crate) struct ChatWidget<'a> { answer_buffer: String, } -#[derive(Clone, Copy, Eq, PartialEq)] -enum InputFocus { - HistoryPane, - BottomPane, -} - struct UserMessage { text: String, image_paths: Vec, @@ -133,7 +126,6 @@ impl ChatWidget<'_> { app_event_tx, has_input_focus: true, }), - input_focus: InputFocus::BottomPane, config, initial_user_message: create_initial_user_message( initial_prompt.unwrap_or_default(), @@ -147,44 +139,17 @@ impl ChatWidget<'_> { pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) { self.bottom_pane.clear_ctrl_c_quit_hint(); - // Special-case : normally toggles focus between history and bottom panes. - // However, when the slash-command popup is visible we forward the key - // to the bottom pane so it can handle auto-completion. - if matches!(key_event.code, crossterm::event::KeyCode::Tab) - && !self.bottom_pane.is_popup_visible() - { - self.input_focus = match self.input_focus { - InputFocus::HistoryPane => InputFocus::BottomPane, - InputFocus::BottomPane => InputFocus::HistoryPane, - }; - self.conversation_history - .set_input_focus(self.input_focus == InputFocus::HistoryPane); - self.bottom_pane - .set_input_focus(self.input_focus == InputFocus::BottomPane); - self.request_redraw(); - return; - } - match self.input_focus { - InputFocus::HistoryPane => { - let needs_redraw = self.conversation_history.handle_key_event(key_event); - if needs_redraw { - self.request_redraw(); - } + match self.bottom_pane.handle_key_event(key_event) { + InputResult::Submitted(text) => { + self.submit_user_message(text.into()); } - InputFocus::BottomPane => match self.bottom_pane.handle_key_event(key_event) { - InputResult::Submitted(text) => { - self.submit_user_message(text.into()); - } - InputResult::None => {} - }, + InputResult::None => {} } } pub(crate) fn handle_paste(&mut self, text: String) { - if matches!(self.input_focus, InputFocus::BottomPane) { - self.bottom_pane.handle_paste(text); - } + self.bottom_pane.handle_paste(text); } /// Emits the last entry's plain lines from conversation_history, if any. diff --git a/codex-rs/tui/src/conversation_history_widget.rs b/codex-rs/tui/src/conversation_history_widget.rs index d8035eff..dede0caf 100644 --- a/codex-rs/tui/src/conversation_history_widget.rs +++ b/codex-rs/tui/src/conversation_history_widget.rs @@ -5,8 +5,6 @@ use crate::history_cell::PatchEventType; use codex_core::config::Config; use codex_core::protocol::FileChange; use codex_core::protocol::SessionConfiguredEvent; -use crossterm::event::KeyCode; -use crossterm::event::KeyEvent; use ratatui::prelude::*; use ratatui::style::Style; use ratatui::widgets::*; @@ -47,33 +45,6 @@ impl ConversationHistoryWidget { } } - pub(crate) fn set_input_focus(&mut self, has_input_focus: bool) { - self.has_input_focus = has_input_focus; - } - - /// Returns true if it needs a redraw. - pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) -> bool { - match key_event.code { - KeyCode::Up | KeyCode::Char('k') => { - self.scroll_up(1); - true - } - KeyCode::Down | KeyCode::Char('j') => { - self.scroll_down(1); - true - } - KeyCode::PageUp | KeyCode::Char('b') => { - self.scroll_page_up(); - true - } - KeyCode::PageDown | KeyCode::Char(' ') => { - self.scroll_page_down(); - true - } - _ => false, - } - } - /// Negative delta scrolls up; positive delta scrolls down. pub(crate) fn scroll(&mut self, delta: i32) { match delta.cmp(&0) { @@ -122,53 +93,6 @@ impl ConversationHistoryWidget { } } - /// Scroll up by one full viewport height (Page Up). - fn scroll_page_up(&mut self) { - let viewport_height = self.last_viewport_height.get().max(1); - - // If we are currently in the "stick to bottom" mode, first convert the - // implicit scroll position (`usize::MAX`) into an explicit offset that - // represents the very bottom of the scroll region. This mirrors the - // logic from `scroll_up()`. - if self.scroll_position == usize::MAX { - self.scroll_position = self - .num_rendered_lines - .get() - .saturating_sub(viewport_height); - } - - // Move up by a full page. - self.scroll_position = self.scroll_position.saturating_sub(viewport_height); - } - - /// Scroll down by one full viewport height (Page Down). - fn scroll_page_down(&mut self) { - // Nothing to do if we're already stuck to the bottom. - if self.scroll_position == usize::MAX { - return; - } - - let viewport_height = self.last_viewport_height.get().max(1); - let num_lines = self.num_rendered_lines.get(); - - // Calculate the maximum explicit scroll offset that is still within - // range. This matches the logic in `scroll_down()` and the render - // method. - let max_scroll = num_lines.saturating_sub(viewport_height); - - // Attempt to move down by a full page. - let new_pos = self.scroll_position.saturating_add(viewport_height); - - if new_pos >= max_scroll { - // We have reached (or passed) the bottom – switch back to - // automatic stick‑to‑bottom mode so that subsequent output keeps - // the viewport pinned. - self.scroll_position = usize::MAX; - } else { - self.scroll_position = new_pos; - } - } - pub fn scroll_to_bottom(&mut self) { self.scroll_position = usize::MAX; }