From b741e480e203f037ca726bc2a76d99a8e8668e66 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Sat, 3 Oct 2026 07:32:25 +0000 Subject: [PATCH] Allow transcript selection and copying while bottom modals are open (#50564) ## Why The plan confirmation prompt allowed transcript scrolling but blocked selecting and copying the visible plan text. ## What changed Allow mouse selection and copying in the visible transcript while a bottom modal is open, preserving drags across redraws. Keep other keyboard input with the modal and suppress transcript link activation, disclosure toggles, and follow-control clicks during modal mouse handling. ## Testing Extend the plan prompt test to cover selection, `Ctrl+Insert` copying, selection highlighting, and popup bounds. Add coverage confirming that ordinary and Control-clicks do not open links in selection-only mouse handling. GitOrigin-RevId: 9e1fdfcb4bc18215e62a714f173bc8cb372b090a --- codex-rs/tui/src/app/owned_transcript.rs | 34 +++++---- .../src/app/owned_transcript_input_tests.rs | 71 +++++++++++++++++-- ...t__input_tests__plan_prompt_selection.snap | 14 ++++ codex-rs/tui/src/transcript_view/input.rs | 71 ++++++++++++++----- .../tui/src/transcript_view/input_tests.rs | 32 +++++++++ 5 files changed, 189 insertions(+), 33 deletions(-) create mode 100644 codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__input_tests__plan_prompt_selection.snap diff --git a/codex-rs/tui/src/app/owned_transcript.rs b/codex-rs/tui/src/app/owned_transcript.rs index fd46ecc012..3fcd0cd23c 100644 --- a/codex-rs/tui/src/app/owned_transcript.rs +++ b/codex-rs/tui/src/app/owned_transcript.rs @@ -316,17 +316,32 @@ impl App { Ok(bottom_area) } - /// Keep modal input ownership while allowing wheel scrolling over the visible transcript. + /// Keep modal input ownership while allowing selection and copying in the visible transcript. pub(super) fn handle_owned_transcript_event( &mut self, tui: &mut tui::Tui, app_server: &mut AppServerSession, event: &TuiEvent, ) -> Result { + let has_modal = self.chat_widget.has_active_modal(); + let modal_transcript_mouse = has_modal + && matches!(event, TuiEvent::Mouse(mouse) + if self.chat_widget.centered_dialog().is_none() + || matches!(mouse.kind, crossterm::event::MouseEventKind::ScrollUp + | crossterm::event::MouseEventKind::ScrollDown)); + let modal_transcript_draw = has_modal + && self.chat_widget.centered_dialog().is_none() + && matches!(event, TuiEvent::Draw); + let modal_transcript_event = modal_transcript_mouse + || modal_transcript_draw + || (has_modal + && matches!(event, TuiEvent::Key(key) + if crate::text_selection::is_copy_key(*key) + && self.transcript_view.owns_interaction_key(*key))); if !tui.is_owned_screen() || matches!(event, TuiEvent::FocusLost | TuiEvent::Resume) || self.overlay.is_some() - || !self.chat_widget.no_modal_or_popup_active() + || (!self.chat_widget.no_modal_or_popup_active() && !modal_transcript_event) { self.transcript_view.end_drag(); } @@ -403,17 +418,7 @@ impl App { } if !self.chat_widget.no_modal_or_popup_active() { self.chat_widget.end_composer_drag(); - let is_modal_scroll = self.chat_widget.has_active_modal() - && matches!( - event, - TuiEvent::Mouse(mouse) - if matches!( - mouse.kind, - crossterm::event::MouseEventKind::ScrollUp - | crossterm::event::MouseEventKind::ScrollDown - ) - ); - if !is_modal_scroll { + if !modal_transcript_event { return Ok(false); } } @@ -532,6 +537,9 @@ impl App { TuiEvent::Key(key) => self .transcript_view .handle_key(*key, &self.transcript_cells), + TuiEvent::Mouse(mouse) if modal_transcript_mouse => self + .transcript_view + .handle_selection_mouse(*mouse, &self.transcript_cells), TuiEvent::Mouse(mouse) => self .transcript_view .handle_mouse(*mouse, &self.transcript_cells), diff --git a/codex-rs/tui/src/app/owned_transcript_input_tests.rs b/codex-rs/tui/src/app/owned_transcript_input_tests.rs index 26dce3a7a1..ee1dbf71f0 100644 --- a/codex-rs/tui/src/app/owned_transcript_input_tests.rs +++ b/codex-rs/tui/src/app/owned_transcript_input_tests.rs @@ -325,8 +325,9 @@ async fn plan_menu_allows_transcript_wheel_scrolling_and_keeps_keyboard_ownershi } #[tokio::test] -async fn plan_menu_wheel_scrolling_respects_modal_and_completion_popup_bounds() -> Result<()> { +async fn plan_menu_allows_transcript_selection_and_copy_but_respects_popup_bounds() -> Result<()> { let (mut app, mut events, _operations) = make_test_app_with_channels().await; + app.local_settings.tui.copy_on_select = codex_config::types::CopyOnSelect::Never; let mut server = Box::pin(crate::start_embedded_app_server_for_picker(&app.config)).await?; let mut tui = crate::tui::test_support::make_test_tui()?; tui.set_owned_screen(/*owned*/ true)?; @@ -343,9 +344,6 @@ async fn plan_menu_wheel_scrolling_respects_modal_and_completion_popup_bounds() (MouseEventKind::ScrollUp, 1, bottom.y), (MouseEventKind::ScrollDown, 1, bottom.y), (MouseEventKind::ScrollUp, size.width, 1), - (MouseEventKind::Down(MouseButton::Left), 1, 1), - (MouseEventKind::Drag(MouseButton::Left), 6, 1), - (MouseEventKind::Up(MouseButton::Left), 6, 1), ] { app.handle_tui_event(&mut tui, &mut server, pointer_event(kind, column, row)) .await?; @@ -353,8 +351,73 @@ async fn plan_menu_wheel_scrolling_respects_modal_and_completion_popup_bounds() assert_eq!(screen(&tui), before); assert!(!app.transcript_view.has_active_interaction()); } + let (plan_row, plan_column) = before + .lines() + .enumerate() + .find_map(|(row, line)| { + line.find("Plan step") + .map(|column| (row as u16, column as u16)) + }) + .expect("visible plan step"); + for event in [ + pointer_event( + MouseEventKind::Down(MouseButton::Left), + plan_column, + plan_row, + ), + TuiEvent::Draw, + pointer_event( + MouseEventKind::Drag(MouseButton::Left), + plan_column + 4, + plan_row, + ), + TuiEvent::Draw, + pointer_event( + MouseEventKind::Up(MouseButton::Left), + plan_column + 4, + plan_row, + ), + ] { + app.handle_tui_event(&mut tui, &mut server, event).await?; + } + let selected = app + .transcript_view + .selected_text(&app.transcript_cells) + .expect("selected plan text"); + assert!("Plan step".contains(&selected), "selected {selected:?}"); + app.handle_tui_event( + &mut tui, + &mut server, + TuiEvent::Key(KeyEvent::new(KeyCode::Insert, KeyModifiers::CONTROL)), + ) + .await?; + assert!(tui.clipboard.is_busy()); + assert!(app.chat_widget.has_active_modal()); + app.render_owned_transcript(&mut tui, size)?; + let buffer = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal); + let selection = (0..size.width) + .map(|column| { + if buffer[(column, plan_row)] + .modifier + .contains(ratatui::style::Modifier::REVERSED) + { + '^' + } else { + '·' + } + }) + .collect::(); + insta::assert_snapshot!( + "plan_prompt_selection", + format!( + "{selection}\n\n{}", + render_bottom_popup(&app.chat_widget, /*width*/ 80), + ) + ); app.handle_tui_event(&mut tui, &mut server, TuiEvent::Key(KeyCode::Esc.into())) .await?; + app.transcript_view.end_selection(&app.transcript_cells); + app.transcript_view.jump_to_latest(); app.chat_widget.apply_external_edit("/m".to_string()); app.render_owned_transcript(&mut tui, size)?; let popup = screen(&tui); diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__input_tests__plan_prompt_selection.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__input_tests__plan_prompt_selection.snap new file mode 100644 index 0000000000..6b013626ff --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__owned_transcript__input_tests__plan_prompt_selection.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/app/owned_transcript_input_tests.rs +expression: "format!(\"{selection}\\n\\n{}\", render_bottom_popup(&app.chat_widget, 80),)" +--- +······^^^^······································································ + + Implement this plan? + + +› 1. Yes, implement this plan Switch to Default and start coding + 2. Yes, clear context and implement Fresh thread with this plan + 3. No, stay in Plan mode Continue planning with the model + + enter select · esc back diff --git a/codex-rs/tui/src/transcript_view/input.rs b/codex-rs/tui/src/transcript_view/input.rs index 389aa5998d..ced88a72b0 100644 --- a/codex-rs/tui/src/transcript_view/input.rs +++ b/codex-rs/tui/src/transcript_view/input.rs @@ -26,6 +26,12 @@ pub(crate) enum ViewAction { OpenLink(String), } +#[derive(Clone, Copy, PartialEq, Eq)] +enum MouseMode { + Interactive, + Selection, +} + /// Transcript jumps shared with input routing so legacy terminals preserve their Alt modifier. #[derive(Clone, Copy, PartialEq, Eq)] pub(crate) enum JumpTarget { @@ -200,10 +206,31 @@ impl TranscriptView { event: MouseEvent, cells: &[Arc], ) -> Option { - if let Some(action) = self.handle_follow_control_mouse(event) { + self.handle_mouse_with_mode(event, cells, MouseMode::Interactive) + } + + /// Handles selection and scrolling without activating transcript content. + pub(crate) fn handle_selection_mouse( + &mut self, + event: MouseEvent, + cells: &[Arc], + ) -> Option { + self.handle_mouse_with_mode(event, cells, MouseMode::Selection) + } + + fn handle_mouse_with_mode( + &mut self, + event: MouseEvent, + cells: &[Arc], + mode: MouseMode, + ) -> Option { + if mode == MouseMode::Interactive + && let Some(action) = self.handle_follow_control_mouse(event) + { return Some(action); } - if event.kind == MouseEventKind::Down(MouseButton::Left) + if mode == MouseMode::Interactive + && event.kind == MouseEventKind::Down(MouseButton::Left) && event .modifiers .intersects(KeyModifiers::CONTROL | KeyModifiers::SUPER) @@ -264,21 +291,28 @@ impl TranscriptView { .filter(|text| !text.is_empty()) .map(ViewAction::Copy); } - MouseEventKind::Down(MouseButton::Left) => return self.pointer_down(event, cells), + MouseEventKind::Down(MouseButton::Left) => { + return self.pointer_down(event, cells, mode); + } MouseEventKind::Drag(MouseButton::Left) if dragging => { self.extend_selection(event.column, event.row); } MouseEventKind::Up(MouseButton::Left) if dragging => { // Open only a stationary click. Dragging back to the origin is still selection, // and wheel scrolling clears the pointer even when it cannot move the viewport. - let link = self.selection.as_mut().and_then(|selection| { - (inside - && !selection.moved - && selection.pointer == Some(ScreenPosition::new(event.column, event.row)) - && event.modifiers.is_empty()) - .then(|| selection.pressed_link.take()) - .flatten() - }); + let link = if mode == MouseMode::Interactive { + self.selection.as_mut().and_then(|selection| { + (inside + && !selection.moved + && selection.pointer + == Some(ScreenPosition::new(event.column, event.row)) + && event.modifiers.is_empty()) + .then(|| selection.pressed_link.take()) + .flatten() + }) + } else { + None + }; let link = link.filter(|destination| { self.link_at(event.column, event.row).as_ref() == Some(destination) }); @@ -385,17 +419,22 @@ impl TranscriptView { &mut self, event: MouseEvent, cells: &[Arc], + mode: MouseMode, ) -> Option { - if event.modifiers.is_empty() && self.toggle_disclosure_at(cells, event.column, event.row) { + if mode == MouseMode::Interactive + && event.modifiers.is_empty() + && self.toggle_disclosure_at(cells, event.column, event.row) + { return Some(ViewAction::Changed); } self.disclosure.focused = None; let visible = self .visible .get(usize::from(event.row.checked_sub(self.area.y)?))?; - if event - .modifiers - .intersects(KeyModifiers::CONTROL | KeyModifiers::SUPER) + if mode == MouseMode::Interactive + && event + .modifiers + .intersects(KeyModifiers::CONTROL | KeyModifiers::SUPER) { return self .link_at(event.column, event.row) @@ -420,7 +459,7 @@ impl TranscriptView { { return None; } - let link = (clicks == 1 && event.modifiers.is_empty()) + let link = (mode == MouseMode::Interactive && clicks == 1 && event.modifiers.is_empty()) .then(|| self.link_at(event.column, event.row)) .flatten(); self.begin_selection(cells, event.column, event.row, clicks); diff --git a/codex-rs/tui/src/transcript_view/input_tests.rs b/codex-rs/tui/src/transcript_view/input_tests.rs index e996f0006c..829240298e 100644 --- a/codex-rs/tui/src/transcript_view/input_tests.rs +++ b/codex-rs/tui/src/transcript_view/input_tests.rs @@ -306,6 +306,38 @@ fn ordinary_click_opens_bare_and_markdown_links_on_release() { } } +#[test] +fn selection_mouse_does_not_open_links() { + for modifiers in [KeyModifiers::NONE, KeyModifiers::CONTROL] { + let (mut view, cells) = transcript("[docs](https://example.com/docs)", /*width*/ 60); + let down = MouseEvent { + modifiers, + ..mouse( + MouseEventKind::Down(MouseButton::Left), + /*column*/ 4, + /*row*/ 1, + ) + }; + assert!(matches!( + view.handle_selection_mouse(down, &cells), + Some(ViewAction::Changed) + )); + let area = view.area; + view.render(area, &mut Buffer::empty(area), &cells); + assert!(matches!( + view.handle_selection_mouse( + MouseEvent { + kind: MouseEventKind::Up(MouseButton::Left), + ..down + }, + &cells, + ), + Some(ViewAction::Changed) + )); + assert_eq!(view.selected_text(&cells), None); + } +} + #[test] fn dragging_a_link_selects_text_and_never_opens_it() { for return_to_origin in [false, true] {