From df5fdcf5bf61cd7cbb69aee3d5084690d51c5638 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Sat, 4 Jul 2026 02:03:36 -0300 Subject: [PATCH] feat(tui): add owned screen navigation --- codex-rs/tui/src/app/history_ui.rs | 5 +- codex-rs/tui/src/app/input.rs | 8 ++ codex-rs/tui/src/app/owned_screen.rs | 58 ++++++++++++++ codex-rs/tui/src/app/owned_screen_tests.rs | 79 +++++++++++++++++++ codex-rs/tui/src/app/resize_reflow.rs | 5 +- codex-rs/tui/src/conversation_viewport.rs | 5 ++ .../tui/src/conversation_viewport_tests.rs | 52 ++++++++++++ codex-rs/tui/src/pager_overlay.rs | 30 +++++-- codex-rs/tui/src/tui.rs | 5 ++ 9 files changed, 237 insertions(+), 10 deletions(-) diff --git a/codex-rs/tui/src/app/history_ui.rs b/codex-rs/tui/src/app/history_ui.rs index 4c49c577bb..ddd4375999 100644 --- a/codex-rs/tui/src/app/history_ui.rs +++ b/codex-rs/tui/src/app/history_ui.rs @@ -18,7 +18,9 @@ impl App { self.owned_screen_push_cell(cell.clone()); if self.has_owned_screen() { self.chat_widget.request_pending_usage_output_insertion(); - tui.frame_requester().schedule_frame(); + if !self.owned_screen_replay_in_progress() { + tui.frame_requester().schedule_frame(); + } return; } if self.initial_history_replay_buffer.as_ref().is_some() { @@ -190,6 +192,7 @@ impl App { self.overlay = None; self.transcript_cells.clear(); self.sync_owned_screen_cells(); + self.finish_owned_screen_replay(); self.deferred_history_lines.clear(); self.has_emitted_history_lines = false; self.transcript_reflow.clear(); diff --git a/codex-rs/tui/src/app/input.rs b/codex-rs/tui/src/app/input.rs index 8dc9a317b0..654a738257 100644 --- a/codex-rs/tui/src/app/input.rs +++ b/codex-rs/tui/src/app/input.rs @@ -211,6 +211,14 @@ impl App { return; } + if app_keymap_shortcuts_available && self.handle_owned_screen_navigation_key(tui, key_event) + { + if self.backtrack.primed { + self.reset_backtrack_state(); + } + return; + } + match key_event { _ if app_keymap_shortcuts_available && self.keymap.app.clear_terminal.is_pressed(key_event) => diff --git a/codex-rs/tui/src/app/owned_screen.rs b/codex-rs/tui/src/app/owned_screen.rs index da14c6b4bb..148908ebe8 100644 --- a/codex-rs/tui/src/app/owned_screen.rs +++ b/codex-rs/tui/src/app/owned_screen.rs @@ -4,6 +4,9 @@ //! bottom of every frame for the composer. Inline mode continues to use terminal scrollback. use crossterm::cursor::SetCursorStyle; +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyEventKind; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use ratatui::widgets::Clear; @@ -14,6 +17,8 @@ use crate::AltScreenBehavior; pub(super) struct OwnedScreen { viewport: ConversationViewport, + replay_in_progress: bool, + last_conversation_area: Rect, } struct RenderedOwnedScreen { @@ -29,6 +34,8 @@ impl OwnedScreen { chat_widget.history_render_mode(), keymap, ), + replay_in_progress: false, + last_conversation_area: Rect::default(), } } @@ -55,6 +62,7 @@ impl OwnedScreen { area.width, bottom_height, ); + self.last_conversation_area = conversation_area; self.viewport .set_render_mode(chat_widget.history_render_mode()); @@ -71,6 +79,19 @@ impl OwnedScreen { cursor_style: bottom_pane.cursor_style(bottom_area), } } + + fn handle_navigation_key(&mut self, key_event: KeyEvent) -> bool { + // Alternate-scroll wheel events are indistinguishable from physical arrow keys. Keep + // arrows, Home/End, and printable pager bindings available to the composer until the TUI + // has direct mouse events or an explicit viewport-focus mode. + if !matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) + || !matches!(key_event.code, KeyCode::PageUp | KeyCode::PageDown) + { + return false; + } + self.viewport + .handle_navigation_key(self.last_conversation_area, key_event) + } } impl App { @@ -95,6 +116,43 @@ impl App { } } + pub(super) fn begin_owned_screen_replay(&mut self) { + if let Some(screen) = &mut self.owned_screen { + screen.replay_in_progress = true; + } + } + + pub(super) fn finish_owned_screen_replay(&mut self) { + if let Some(screen) = &mut self.owned_screen { + screen.replay_in_progress = false; + } + } + + pub(super) fn owned_screen_replay_in_progress(&self) -> bool { + self.owned_screen + .as_ref() + .is_some_and(|screen| screen.replay_in_progress) + } + + pub(super) fn handle_owned_screen_navigation_key( + &mut self, + tui: &mut tui::Tui, + key_event: KeyEvent, + ) -> bool { + if !self.chat_widget.composer_is_empty() || !self.chat_widget.no_modal_or_popup_active() { + return false; + } + let handled = self + .owned_screen + .as_mut() + .is_some_and(|screen| screen.handle_navigation_key(key_event)); + if handled { + tui.frame_requester() + .schedule_frame_in(crate::tui::TARGET_FRAME_INTERVAL); + } + handled + } + pub(crate) fn sync_owned_screen_cells(&mut self) { if let Some(screen) = &mut self.owned_screen { screen.viewport.replace_cells(self.transcript_cells.clone()); diff --git a/codex-rs/tui/src/app/owned_screen_tests.rs b/codex-rs/tui/src/app/owned_screen_tests.rs index 0e5a5fbf76..56b99b84a3 100644 --- a/codex-rs/tui/src/app/owned_screen_tests.rs +++ b/codex-rs/tui/src/app/owned_screen_tests.rs @@ -1,7 +1,12 @@ +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyModifiers; use insta::assert_snapshot; use ratatui::Terminal; use ratatui::backend::TestBackend; use ratatui::text::Line; +use std::time::Duration; +use tokio::sync::broadcast::error::TryRecvError; use super::*; use crate::chatwidget::tests::make_chatwidget_manual_with_sender; @@ -68,3 +73,77 @@ async fn committed_cell_updates_viewport_without_queuing_terminal_history() { assert!(!app.has_emitted_history_lines); assert!(!tui.has_pending_history_lines()); } + +#[tokio::test] +async fn replay_retains_cells_while_draw_scheduling_is_deferred() { + let mut app = super::super::test_support::make_test_app().await; + app.owned_screen = App::owned_screen_for_behavior( + AltScreenBehavior::Owned, + &app.chat_widget, + app.keymap.pager.clone(), + ); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + let mut draw_rx = tui.subscribe_draws_for_test(); + + app.begin_initial_history_replay_buffer(); + app.insert_history_cell(&mut tui, Box::new(TestCell("first"))); + app.insert_history_cell(&mut tui, Box::new(TestCell("second"))); + + tokio::time::sleep(Duration::from_millis(/*millis*/ 50)).await; + assert!(matches!(draw_rx.try_recv(), Err(TryRecvError::Empty))); + + assert!(app.owned_screen_replay_in_progress()); + assert_eq!( + app.owned_screen + .as_ref() + .expect("owned screen") + .viewport + .committed_cell_count(), + 2 + ); + + app.finish_initial_history_replay_buffer(&mut tui); + + assert!(!app.owned_screen_replay_in_progress()); + tokio::time::timeout(Duration::from_secs(/*secs*/ 1), draw_rx.recv()) + .await + .expect("timed out waiting for replay completion draw") + .expect("draw channel closed"); +} + +#[tokio::test] +async fn navigation_does_not_steal_printable_or_draft_input() { + let mut app = super::super::test_support::make_test_app().await; + app.owned_screen = App::owned_screen_for_behavior( + AltScreenBehavior::Owned, + &app.chat_widget, + app.keymap.pager.clone(), + ); + let mut tui = crate::tui::test_support::make_test_tui().expect("create test TUI"); + + let cases = [ + (KeyCode::Char('k'), false), + (KeyCode::Up, false), + (KeyCode::Down, false), + (KeyCode::Home, false), + (KeyCode::End, false), + (KeyCode::PageUp, true), + (KeyCode::PageDown, true), + ]; + for (code, expected) in cases { + assert_eq!( + app.handle_owned_screen_navigation_key( + &mut tui, + KeyEvent::new(code, KeyModifiers::NONE), + ), + expected, + ); + } + + app.chat_widget + .set_composer_text("draft".to_string(), Vec::new(), Vec::new()); + assert!(!app.handle_owned_screen_navigation_key( + &mut tui, + KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE), + )); +} diff --git a/codex-rs/tui/src/app/resize_reflow.rs b/codex-rs/tui/src/app/resize_reflow.rs index 7c13528877..2525dcc509 100644 --- a/codex-rs/tui/src/app/resize_reflow.rs +++ b/codex-rs/tui/src/app/resize_reflow.rs @@ -119,6 +119,7 @@ impl App { /// overlay replay continues through the normal deferred-history path. pub(super) fn begin_initial_history_replay_buffer(&mut self) { if self.has_owned_screen() { + self.begin_owned_screen_replay(); return; } if self.overlay.is_none() { @@ -133,6 +134,7 @@ impl App { /// so only the rows the terminal would retain are formatted and inserted. pub(super) fn begin_thread_switch_history_replay_buffer(&mut self) { if self.has_owned_screen() { + self.begin_owned_screen_replay(); return; } if self.resize_reflow_max_rows().is_some() && self.overlay.is_none() { @@ -150,7 +152,8 @@ impl App { /// expensive than a later resize rebuild of the same transcript. pub(super) fn finish_initial_history_replay_buffer(&mut self, tui: &mut tui::Tui) { if self.has_owned_screen() { - self.initial_history_replay_buffer = None; + self.finish_owned_screen_replay(); + tui.frame_requester().schedule_frame(); return; } let Some(buffer) = self.initial_history_replay_buffer.take() else { diff --git a/codex-rs/tui/src/conversation_viewport.rs b/codex-rs/tui/src/conversation_viewport.rs index 7e83a6e7d1..40b2a9be81 100644 --- a/codex-rs/tui/src/conversation_viewport.rs +++ b/codex-rs/tui/src/conversation_viewport.rs @@ -9,6 +9,7 @@ use std::cell::Cell; use std::sync::Arc; use ratatui::buffer::Buffer; +use ratatui::crossterm::event::KeyEvent; use ratatui::layout::Rect; use ratatui::text::Text; use ratatui::widgets::Paragraph; @@ -61,6 +62,10 @@ impl ConversationViewport { self.content.render(area, buf); } + pub(crate) fn handle_navigation_key(&mut self, area: Rect, key_event: KeyEvent) -> bool { + self.content.handle_navigation_key(area, key_event) + } + pub(crate) fn push_cell(&mut self, cell: Arc) { let follow_bottom = self.content.is_following_bottom(); let had_prior_cells = !self.cells.is_empty(); diff --git a/codex-rs/tui/src/conversation_viewport_tests.rs b/codex-rs/tui/src/conversation_viewport_tests.rs index e9656beca5..2a0a23fe0c 100644 --- a/codex-rs/tui/src/conversation_viewport_tests.rs +++ b/codex-rs/tui/src/conversation_viewport_tests.rs @@ -1,3 +1,6 @@ +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyModifiers; use insta::assert_snapshot; use pretty_assertions::assert_eq; use ratatui::Terminal; @@ -222,6 +225,55 @@ fn narrower_resize_stays_pinned_to_the_latest_cell() { assert!(buffer_text(&buffer, narrow).contains("SENTINEL")); } +#[test] +fn page_navigation_leaves_and_restores_bottom_follow() { + let mut viewport = viewport(vec![cell("oldest"), cell("middle"), cell("LATEST")]); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 20, /*height*/ 2, + ); + let mut bottom = Buffer::empty(area); + viewport.render(area, &mut bottom); + assert!(buffer_text(&bottom, area).contains("LATEST")); + + assert!( + viewport.handle_navigation_key(area, KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE),) + ); + let mut scrolled = Buffer::empty(area); + viewport.render(area, &mut scrolled); + assert!(!viewport.is_following_bottom()); + assert!(buffer_text(&scrolled, area).contains("middle")); + + assert!( + viewport.handle_navigation_key(area, KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE),) + ); + let mut restored = Buffer::empty(area); + viewport.render(area, &mut restored); + assert!(viewport.is_following_bottom()); + let trim_rows = |buffer: &Buffer| { + buffer_text(buffer, area) + .lines() + .map(str::trim_end) + .collect::>() + .join("\n") + }; + assert_snapshot!(format!( + "bottom:\n{}\nscrolled:\n{}\nrestored:\n{}", + trim_rows(&bottom), + trim_rows(&scrolled), + trim_rows(&restored), + ), @r###" +bottom: + +LATEST +scrolled: + +middle +restored: + +LATEST +"###); +} + fn buffer_text(buffer: &Buffer, area: Rect) -> String { let mut rows = Vec::new(); for y in area.y..area.bottom() { diff --git a/codex-rs/tui/src/pager_overlay.rs b/codex-rs/tui/src/pager_overlay.rs index 9161bd10d3..9dbe512dbc 100644 --- a/codex-rs/tui/src/pager_overlay.rs +++ b/codex-rs/tui/src/pager_overlay.rs @@ -254,6 +254,14 @@ impl PagerView { } fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) -> Result<()> { + if self.apply_key_event(tui.terminal.viewport_area, key_event) { + tui.frame_requester() + .schedule_frame_in(crate::tui::TARGET_FRAME_INTERVAL); + } + Ok(()) + } + + fn apply_key_event(&mut self, viewport_area: Rect, key_event: KeyEvent) -> bool { match key_event { e if self.keymap.scroll_up.is_pressed(e) => { self.scroll_offset = self.scroll_offset.saturating_sub(1); @@ -262,20 +270,20 @@ impl PagerView { self.scroll_offset = self.scroll_offset.saturating_add(1); } e if self.keymap.page_up.is_pressed(e) => { - let page_height = self.page_height(tui.terminal.viewport_area); + let page_height = self.page_height(viewport_area); self.scroll_offset = self.scroll_offset.saturating_sub(page_height); } e if self.keymap.page_down.is_pressed(e) => { - let page_height = self.page_height(tui.terminal.viewport_area); + let page_height = self.page_height(viewport_area); self.scroll_offset = self.scroll_offset.saturating_add(page_height); } e if self.keymap.half_page_down.is_pressed(e) => { - let area = self.content_area(tui.terminal.viewport_area); + let area = self.content_area(viewport_area); let half_page = (area.height as usize).saturating_add(1) / 2; self.scroll_offset = self.scroll_offset.saturating_add(half_page); } e if self.keymap.half_page_up.is_pressed(e) => { - let area = self.content_area(tui.terminal.viewport_area); + let area = self.content_area(viewport_area); let half_page = (area.height as usize).saturating_add(1) / 2; self.scroll_offset = self.scroll_offset.saturating_sub(half_page); } @@ -286,12 +294,10 @@ impl PagerView { self.scroll_offset = usize::MAX; } _ => { - return Ok(()); + return false; } } - tui.frame_requester() - .schedule_frame_in(crate::tui::TARGET_FRAME_INTERVAL); - Ok(()) + true } /// Returns the height of one page in content rows. @@ -429,6 +435,14 @@ impl PagerContent { pub(crate) fn scroll_to_bottom(&mut self) { self.view.scroll_offset = usize::MAX; } + + pub(crate) fn handle_navigation_key( + &mut self, + viewport_area: Rect, + key_event: KeyEvent, + ) -> bool { + self.view.apply_key_event(viewport_area, key_event) + } } /// A renderable that caches its desired height. diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index 7b09afbfc6..08344820d0 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -618,6 +618,11 @@ impl Tui { self.frame_requester.clone() } + #[cfg(test)] + pub(crate) fn subscribe_draws_for_test(&self) -> broadcast::Receiver<()> { + self.draw_tx.subscribe() + } + pub fn enhanced_keys_supported(&self) -> bool { self.enhanced_keys_supported }