mirror of
https://github.com/openai/codex.git
synced 2026-10-20 00:18:49 +00:00
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
This commit is contained in:
@@ -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<bool> {
|
||||
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),
|
||||
|
||||
@@ -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::<String>();
|
||||
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);
|
||||
|
||||
+14
@@ -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
|
||||
@@ -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<dyn HistoryCell>],
|
||||
) -> Option<ViewAction> {
|
||||
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<dyn HistoryCell>],
|
||||
) -> Option<ViewAction> {
|
||||
self.handle_mouse_with_mode(event, cells, MouseMode::Selection)
|
||||
}
|
||||
|
||||
fn handle_mouse_with_mode(
|
||||
&mut self,
|
||||
event: MouseEvent,
|
||||
cells: &[Arc<dyn HistoryCell>],
|
||||
mode: MouseMode,
|
||||
) -> Option<ViewAction> {
|
||||
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<dyn HistoryCell>],
|
||||
mode: MouseMode,
|
||||
) -> Option<ViewAction> {
|
||||
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);
|
||||
|
||||
@@ -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] {
|
||||
|
||||
Reference in New Issue
Block a user