From a63262620ef8d04ef168d4c3b1561ef9a3d1450a Mon Sep 17 00:00:00 2001 From: yishuiliunian Date: Sun, 5 Apr 2026 00:28:54 +0800 Subject: [PATCH 1/2] fix(tui): Up/Down keys navigate history instead of scrolling content area Up/Down were scrolling the content area instead of navigating input history because `content_overflows` was almost always true after any real conversation. Now Up/Down exclusively handle multiline cursor movement and history navigation; content scrolling is PageUp/PageDown only. Input-mode keys auto-reset scroll_offset to 0 (except PageUp/PageDown/Tab/Esc) so the user always sees the input area when interacting. Removed dead `content_overflows` field and render_progress return value. --- crates/loopal-tui/src/app/mod.rs | 4 - crates/loopal-tui/src/input/mod.rs | 46 ++---- crates/loopal-tui/src/input/navigation.rs | 2 + crates/loopal-tui/src/render.rs | 2 +- crates/loopal-tui/src/views/progress/mod.rs | 11 +- crates/loopal-tui/tests/suite.rs | 2 + .../tests/suite/focus_panel_keys_test.rs | 1 - .../tests/suite/input_scroll_edge_test.rs | 134 ++++++++++++++++++ .../tests/suite/input_scroll_test.rs | 94 +++++++----- 9 files changed, 206 insertions(+), 90 deletions(-) create mode 100644 crates/loopal-tui/tests/suite/input_scroll_edge_test.rs diff --git a/crates/loopal-tui/src/app/mod.rs b/crates/loopal-tui/src/app/mod.rs index a2e8b91a..8f342483 100644 --- a/crates/loopal-tui/src/app/mod.rs +++ b/crates/loopal-tui/src/app/mod.rs @@ -39,9 +39,6 @@ pub struct App { pub input_scroll: usize, /// Paste placeholder → original content map for large paste folding. pub paste_map: HashMap, - /// Whether the content area overflows the viewport (set by render pass). - /// Used by input handler to decide Up/Down = scroll vs history navigation. - pub content_overflows: bool, /// Whether the topology overlay is visible (toggled by /topology). pub show_topology: bool, /// Agent panel cursor — Tab cycles through agents. Purely TUI concept. @@ -91,7 +88,6 @@ impl App { last_esc_time: None, input_scroll: 0, paste_map: HashMap::new(), - content_overflows: false, show_topology: true, focused_agent: None, focused_bg_task: None, diff --git a/crates/loopal-tui/src/input/mod.rs b/crates/loopal-tui/src/input/mod.rs index 9d281a0d..6aa56b75 100644 --- a/crates/loopal-tui/src/input/mod.rs +++ b/crates/loopal-tui/src/input/mod.rs @@ -108,6 +108,13 @@ fn handle_panel_key(app: &mut App, key: &KeyEvent) -> InputAction { /// Keys in Input mode: typing, navigation, submit. fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { + // Auto-scroll to bottom on input interaction (except scroll/panel/escape keys) + if !matches!( + key.code, + KeyCode::PageUp | KeyCode::PageDown | KeyCode::Tab | KeyCode::Esc + ) { + app.scroll_offset = 0; + } match key.code { KeyCode::Enter if key.modifiers.contains(KeyModifiers::SHIFT) => { app.input.insert(app.input_cursor, '\n'); @@ -145,8 +152,8 @@ fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { multiline::line_end(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH); InputAction::None } - KeyCode::Up => handle_up_key(app), - KeyCode::Down => handle_down_key(app), + KeyCode::Up => handle_up(app), + KeyCode::Down => handle_down(app), KeyCode::Tab => InputAction::EnterPanel, KeyCode::Esc => handle_esc(app), KeyCode::PageUp => { @@ -160,38 +167,3 @@ fn handle_input_mode_key(app: &mut App, key: &KeyEvent) -> InputAction { _ => InputAction::None, } } - -/// Up key: multiline nav → scroll → history. -fn handle_up_key(app: &mut App) -> InputAction { - if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) - && let Some(pos) = multiline::cursor_up(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) - { - app.input_cursor = pos; - InputAction::None - } else if app.scroll_offset > 0 - || !app.session.lock().active_conversation().agent_idle - || app.content_overflows - { - app.scroll_offset = app.scroll_offset.saturating_add(1); - InputAction::None - } else { - handle_up(app) - } -} - -/// Down key: multiline nav → scroll → history. -fn handle_down_key(app: &mut App) -> InputAction { - if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) - && let Some(pos) = multiline::cursor_down(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) - { - app.input_cursor = pos; - InputAction::None - } else if app.scroll_offset > 0 { - app.scroll_offset = app.scroll_offset.saturating_sub(1); - InputAction::None - } else if app.content_overflows { - InputAction::None - } else { - handle_down(app) - } -} diff --git a/crates/loopal-tui/src/input/navigation.rs b/crates/loopal-tui/src/input/navigation.rs index a72dfb4b..694c3e38 100644 --- a/crates/loopal-tui/src/input/navigation.rs +++ b/crates/loopal-tui/src/input/navigation.rs @@ -31,6 +31,7 @@ pub(super) fn move_cursor_right(app: &mut App) { /// Up: multiline navigation first, then history browse. pub(super) fn handle_up(app: &mut App) -> InputAction { + app.scroll_offset = 0; if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) && let Some(new_cursor) = multiline::cursor_up(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) @@ -56,6 +57,7 @@ pub(super) fn handle_up(app: &mut App) -> InputAction { /// Down: multiline navigation first, then history browse. pub(super) fn handle_down(app: &mut App) -> InputAction { + app.scroll_offset = 0; if multiline::is_multiline(&app.input, DEFAULT_WRAP_WIDTH) && let Some(new_cursor) = multiline::cursor_down(&app.input, app.input_cursor, DEFAULT_WRAP_WIDTH) diff --git a/crates/loopal-tui/src/render.rs b/crates/loopal-tui/src/render.rs index 658bf0d8..a817a8dd 100644 --- a/crates/loopal-tui/src/render.rs +++ b/crates/loopal-tui/src/render.rs @@ -51,7 +51,7 @@ pub fn draw(f: &mut Frame, app: &mut App) { if breadcrumb_h > 0 { views::breadcrumb::render_breadcrumb(f, &state.active_view, layout.breadcrumb); } - app.content_overflows = views::progress::render_progress( + views::progress::render_progress( f, &state, app.scroll_offset, diff --git a/crates/loopal-tui/src/views/progress/mod.rs b/crates/loopal-tui/src/views/progress/mod.rs index d54d765b..cc587552 100644 --- a/crates/loopal-tui/src/views/progress/mod.rs +++ b/crates/loopal-tui/src/views/progress/mod.rs @@ -15,20 +15,16 @@ pub use line_cache::LineCache; pub use message_lines::{message_to_lines, streaming_to_lines}; /// Render the content area — no border, no title, content fills the area. -/// -/// Returns `true` when total content exceeds the visible viewport height. -/// The caller stores this flag so the input handler can decide whether -/// Up/Down should scroll content or navigate input history. pub fn render_progress( f: &mut Frame, state: &SessionState, scroll_offset: u16, line_cache: &mut LineCache, area: Rect, -) -> bool { +) { let visible_h = area.height as usize; if visible_h == 0 { - return false; + return; } let conv = state.active_conversation(); @@ -51,7 +47,6 @@ pub fn render_progress( let cached_tail = line_cache.tail(window_size); // Build the render lines: cached tail + thinking + streaming - let overflows = line_cache.total_lines() + streaming.len() + thinking_lines.len() > visible_h; let mut lines = Vec::with_capacity(cached_tail.len() + thinking_lines.len() + streaming.len()); lines.extend_from_slice(cached_tail); lines.extend(thinking_lines); @@ -65,6 +60,4 @@ pub fn render_progress( let paragraph = Paragraph::new(lines).scroll((scroll_row, 0)); f.render_widget(paragraph, area); - - overflows } diff --git a/crates/loopal-tui/tests/suite.rs b/crates/loopal-tui/tests/suite.rs index c84aa259..496e0cf8 100644 --- a/crates/loopal-tui/tests/suite.rs +++ b/crates/loopal-tui/tests/suite.rs @@ -34,6 +34,8 @@ mod focus_panel_keys_test; mod init_cmd_test; #[path = "suite/input_edge_test.rs"] mod input_edge_test; +#[path = "suite/input_scroll_edge_test.rs"] +mod input_scroll_edge_test; #[path = "suite/input_scroll_test.rs"] mod input_scroll_test; #[path = "suite/input_test.rs"] diff --git a/crates/loopal-tui/tests/suite/focus_panel_keys_test.rs b/crates/loopal-tui/tests/suite/focus_panel_keys_test.rs index 18b0cb67..f2fac2a2 100644 --- a/crates/loopal-tui/tests/suite/focus_panel_keys_test.rs +++ b/crates/loopal-tui/tests/suite/focus_panel_keys_test.rs @@ -76,7 +76,6 @@ fn up_in_input_mode_ignores_agent_panel() { spawn_agent(&app, "worker"); app.focused_agent = Some("worker".into()); app.focus_mode = FocusMode::Input; - app.content_overflows = true; let action = handle_key(&mut app, key(KeyCode::Up)); assert!(!matches!(action, InputAction::PanelUp)); } diff --git a/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs b/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs new file mode 100644 index 00000000..5e4e571d --- /dev/null +++ b/crates/loopal-tui/tests/suite/input_scroll_edge_test.rs @@ -0,0 +1,134 @@ +/// Edge-case tests for Up/Down key scroll reset, agent-busy behavior, +/// Ctrl+P/N scroll reset, and multiline boundary fall-through to history. +use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + +use loopal_protocol::ControlCommand; +use loopal_protocol::UserQuestionResponse; +use loopal_session::SessionController; +use loopal_tui::app::App; +use loopal_tui::input::handle_key; +use tokio::sync::mpsc; + +fn make_app() -> App { + let (control_tx, _) = mpsc::channel::(16); + let (perm_tx, _) = mpsc::channel::(16); + let (question_tx, _) = mpsc::channel::(16); + let session = SessionController::new( + "test-model".into(), + "act".into(), + control_tx, + perm_tx, + question_tx, + Default::default(), + std::sync::Arc::new(tokio::sync::watch::channel(0u64).0), + ); + App::new(session, std::env::temp_dir()) +} + +fn key(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) +} + +fn ctrl(c: char) -> KeyEvent { + KeyEvent::new(KeyCode::Char(c), KeyModifiers::CONTROL) +} + +// --- Auto-reset scroll: per-key verification --- + +#[test] +fn test_backspace_resets_scroll_offset() { + let mut app = make_app(); + app.input = "ab".into(); + app.input_cursor = 2; + app.scroll_offset = 4; + handle_key(&mut app, key(KeyCode::Backspace)); + assert_eq!(app.scroll_offset, 0, "Backspace should reset scroll"); + assert_eq!(app.input, "a"); +} + +#[test] +fn test_cursor_move_resets_scroll_offset() { + let mut app = make_app(); + app.input = "hello".into(); + app.input_cursor = 3; + app.scroll_offset = 6; + handle_key(&mut app, key(KeyCode::Left)); + assert_eq!(app.scroll_offset, 0, "Left should reset scroll"); + assert_eq!(app.input_cursor, 2); +} + +#[test] +fn test_tab_preserves_scroll_offset() { + let mut app = make_app(); + app.scroll_offset = 7; + handle_key(&mut app, key(KeyCode::Tab)); + assert_eq!(app.scroll_offset, 7, "Tab should not reset scroll"); +} + +#[test] +fn test_esc_preserves_scroll_offset() { + let mut app = make_app(); + app.scroll_offset = 3; + handle_key(&mut app, key(KeyCode::Esc)); + assert_eq!(app.scroll_offset, 3, "Esc should not reset scroll"); +} + +// --- Agent busy: Up/Down still navigates history --- + +#[test] +fn test_up_navigates_history_when_agent_busy() { + let mut app = make_app(); + app.session.lock().active_conversation_mut().agent_idle = false; + app.input_history.push("prev".into()); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!( + app.input, "prev", + "Up should navigate history even when agent busy" + ); +} + +// --- Ctrl+P/N resets scroll offset --- + +#[test] +fn test_ctrl_p_resets_scroll_offset() { + let mut app = make_app(); + app.scroll_offset = 10; + app.input_history.push("cmd".into()); + handle_key(&mut app, ctrl('p')); + assert_eq!(app.scroll_offset, 0, "Ctrl+P should reset scroll"); + assert_eq!(app.input, "cmd"); +} + +// --- Multiline boundary: fall through to history --- + +#[test] +fn test_up_at_first_line_falls_through_to_history() { + let mut app = make_app(); + app.input = "line1\nline2".into(); + app.input_cursor = 2; // middle of line1 (already at first visual line) + app.input_history.push("old command".into()); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!( + app.input, "old command", + "Up at first line should fall through to history" + ); +} + +#[test] +fn test_down_at_last_line_falls_through_to_history() { + let mut app = make_app(); + // First enter history via Up + app.input_history.push("first".into()); + app.input_history.push("second".into()); + handle_key(&mut app, key(KeyCode::Up)); // "second" + handle_key(&mut app, key(KeyCode::Up)); // "first" + // Now set multiline content and cursor at last line + app.input = "line1\nline2".into(); + app.input_cursor = app.input.len(); // end of line2 + handle_key(&mut app, key(KeyCode::Down)); + // cursor_down returns None (at last line), falls through to history forward + assert_eq!( + app.input, "second", + "Down at last line should fall through to history" + ); +} diff --git a/crates/loopal-tui/tests/suite/input_scroll_test.rs b/crates/loopal-tui/tests/suite/input_scroll_test.rs index bd431dd8..86d6f0c0 100644 --- a/crates/loopal-tui/tests/suite/input_scroll_test.rs +++ b/crates/loopal-tui/tests/suite/input_scroll_test.rs @@ -1,9 +1,11 @@ -/// Tests for Up/Down scroll routing, Ctrl+P/N history, and multiline priority. +/// Tests for Up/Down key routing, Ctrl+P/N history, and multiline priority. /// -/// Priority chain for Up/Down (via xterm alternate scroll = mouse wheel): +/// Priority chain for Up/Down in Input mode: /// 1. Multiline cursor navigation (Shift+Enter input) -/// 2. Content scroll -/// 3. History navigation (only when content fits) +/// 2. History navigation +/// +/// Content scrolling is exclusively handled by PageUp/PageDown. +/// All input-mode keys (except PageUp/PageDown/Tab/Esc) auto-reset scroll_offset to 0. /// /// Ctrl+P/N always navigate history regardless of scroll state. use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; @@ -50,59 +52,59 @@ fn test_page_up_down_scroll() { assert_eq!(app.scroll_offset, 0); } -// --- Up/Down content scroll --- +// --- Up/Down navigate history --- #[test] -fn test_up_scrolls_when_content_overflows() { +fn test_up_navigates_history_with_content() { let mut app = make_app(); - app.content_overflows = true; - handle_key(&mut app, key(KeyCode::Up)); - assert_eq!(app.scroll_offset, 1, "Up should scroll +1 when overflows"); + app.input_history.push("previous".into()); handle_key(&mut app, key(KeyCode::Up)); - assert_eq!(app.scroll_offset, 2, "repeated Up keeps incrementing"); + assert_eq!(app.input, "previous", "Up should browse history, not scroll"); + assert_eq!(app.scroll_offset, 0); } #[test] -fn test_down_scrolls_back_when_offset_positive() { +fn test_down_resets_scroll_and_navigates_history() { let mut app = make_app(); app.scroll_offset = 5; + app.input_history.push("first".into()); + app.input_history.push("second".into()); + handle_key(&mut app, key(KeyCode::Up)); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.input, "first"); handle_key(&mut app, key(KeyCode::Down)); - assert_eq!(app.scroll_offset, 4, "Down should scroll -1 when offset>0"); + assert_eq!(app.input, "second", "Down should navigate history forward"); + assert_eq!(app.scroll_offset, 0, "scroll_offset should be 0"); } #[test] -fn test_up_scrolls_when_content_overflows_and_idle() { +fn test_up_navigates_history_when_idle() { let mut app = make_app(); app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = true; app.input_history.push("older".into()); app.input_history.push("recent".into()); handle_key(&mut app, key(KeyCode::Up)); - assert_eq!(app.scroll_offset, 1, "Up should scroll even when idle"); - assert!(app.input.is_empty(), "input stays empty (no history)"); + assert_eq!(app.input, "recent", "Up should browse history"); + assert_eq!(app.scroll_offset, 0); } #[test] -fn test_down_absorbed_when_content_overflows_at_bottom() { +fn test_down_navigates_history_forward() { let mut app = make_app(); app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = true; - app.input_history.push("cmd".into()); + app.input_history.push("first".into()); + app.input_history.push("second".into()); + handle_key(&mut app, key(KeyCode::Up)); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.input, "first"); handle_key(&mut app, key(KeyCode::Down)); - assert_eq!(app.scroll_offset, 0, "scroll_offset stays 0"); - assert!( - app.input.is_empty(), - "Down at bottom should not trigger history" - ); + assert_eq!(app.input, "second", "Down should navigate history forward"); } -// --- Up/Down history (content fits) --- - #[test] fn test_up_navigates_history_when_content_fits() { let mut app = make_app(); app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = false; app.input_history.push("previous command".into()); let action = handle_key(&mut app, key(KeyCode::Up)); assert!(matches!(action, InputAction::None)); @@ -113,10 +115,9 @@ fn test_up_navigates_history_when_content_fits() { // --- Ctrl+P/N history navigation --- #[test] -fn test_ctrl_p_navigates_history_when_content_overflows() { +fn test_ctrl_p_navigates_history() { let mut app = make_app(); app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = true; app.input_history.push("first".into()); app.input_history.push("second".into()); handle_key(&mut app, ctrl('p')); @@ -137,15 +138,13 @@ fn test_ctrl_n_navigates_history_forward() { assert_eq!(app.input, "second", "Ctrl+N browses history forward"); } -// --- Multiline cursor priority over scroll --- +// --- Multiline cursor priority over history --- #[test] -fn test_up_multiline_cursor_beats_scroll() { +fn test_up_multiline_cursor_beats_history() { let mut app = make_app(); - app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = true; app.input = "line1\nline2".into(); - app.input_cursor = app.input.len(); // end of line2 + app.input_cursor = app.input.len(); handle_key(&mut app, key(KeyCode::Up)); assert_eq!(app.scroll_offset, 0, "should move cursor, not scroll"); assert!( @@ -156,12 +155,10 @@ fn test_up_multiline_cursor_beats_scroll() { } #[test] -fn test_down_multiline_cursor_beats_absorb() { +fn test_down_multiline_cursor_beats_history() { let mut app = make_app(); - app.session.lock().active_conversation_mut().agent_idle = true; - app.content_overflows = true; app.input = "line1\nline2".into(); - app.input_cursor = 0; // start of line1 + app.input_cursor = 0; handle_key(&mut app, key(KeyCode::Down)); assert_eq!(app.scroll_offset, 0, "should move cursor, not scroll"); assert!( @@ -170,3 +167,24 @@ fn test_down_multiline_cursor_beats_absorb() { app.input_cursor ); } + +// --- Auto-reset scroll on input interaction --- + +#[test] +fn test_typing_resets_scroll_offset() { + let mut app = make_app(); + app.scroll_offset = 5; + handle_key(&mut app, key(KeyCode::Char('a'))); + assert_eq!(app.scroll_offset, 0, "typing should reset scroll to bottom"); + assert_eq!(app.input, "a"); +} + +#[test] +fn test_up_resets_scroll_offset() { + let mut app = make_app(); + app.scroll_offset = 3; + app.input_history.push("cmd".into()); + handle_key(&mut app, key(KeyCode::Up)); + assert_eq!(app.scroll_offset, 0, "Up should reset scroll to bottom"); + assert_eq!(app.input, "cmd"); +} From 073fae400dd41668c67e49449235b7d35038deb0 Mon Sep 17 00:00:00 2001 From: yishuiliunian Date: Sun, 5 Apr 2026 00:37:53 +0800 Subject: [PATCH 2/2] fix: rustfmt long assert_eq line in input_scroll_test --- crates/loopal-tui/tests/suite/input_scroll_test.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/loopal-tui/tests/suite/input_scroll_test.rs b/crates/loopal-tui/tests/suite/input_scroll_test.rs index 86d6f0c0..a7eef8ca 100644 --- a/crates/loopal-tui/tests/suite/input_scroll_test.rs +++ b/crates/loopal-tui/tests/suite/input_scroll_test.rs @@ -59,7 +59,10 @@ fn test_up_navigates_history_with_content() { let mut app = make_app(); app.input_history.push("previous".into()); handle_key(&mut app, key(KeyCode::Up)); - assert_eq!(app.input, "previous", "Up should browse history, not scroll"); + assert_eq!( + app.input, "previous", + "Up should browse history, not scroll" + ); assert_eq!(app.scroll_offset, 0); }