diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index d6d01045b4..0d5fe44064 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1650,6 +1650,16 @@ impl ChatComposer { fn sync_popups(&mut self) { let file_token = Self::current_at_token(&self.textarea); + let browsing_history = self + .history + .should_handle_navigation(self.textarea.text(), self.textarea.cursor()); + // When browsing input history (shell-style Up/Down recall), skip all popup + // synchronization so nothing steals focus from continued history navigation. + if browsing_history { + self.active_popup = ActivePopup::None; + return; + } + let skill_token = self.current_skill_token(); let allow_command_popup = file_token.is_none() && skill_token.is_none(); @@ -4109,6 +4119,59 @@ mod tests { assert_eq!(result, InputResult::None); } + #[test] + fn history_navigation_takes_priority_over_popups() { + use codex_protocol::protocol::SkillScope; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + use tokio::sync::mpsc::unbounded_channel; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer.set_skill_mentions(Some(vec![SkillMetadata { + name: "codex-cli-release-notes".to_string(), + description: "example".to_string(), + short_description: None, + path: PathBuf::from("skills/codex-cli-release-notes/SKILL.md"), + scope: SkillScope::Repo, + }])); + + // Seed local history; the newest entry triggers the skills popup. + composer.history.record_local_submission("older"); + composer + .history + .record_local_submission("$codex-cli-release-notes"); + + // First Up recalls "$...", but we should not open the skills popup while browsing history. + let (result, _redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(result, InputResult::None); + assert_eq!(composer.textarea.text(), "$codex-cli-release-notes"); + assert!( + matches!(composer.active_popup, ActivePopup::None), + "expected no skills popup while browsing history" + ); + + // Second Up should navigate history again (no popup should interfere). + let (result, _redraw) = + composer.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::NONE)); + assert_eq!(result, InputResult::None); + assert_eq!(composer.textarea.text(), "older"); + assert!( + matches!(composer.active_popup, ActivePopup::None), + "expected popup to be dismissed after history navigation" + ); + } + #[test] fn slash_popup_activated_for_bare_slash_and_valid_prefixes() { // use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; diff --git a/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap b/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap new file mode 100644 index 0000000000..a2c3be0cf3 --- /dev/null +++ b/codex-rs/tui2/src/snapshots/codex_tui2__transcript_copy_ui__tests__copy_pill_clears_background.snap @@ -0,0 +1,7 @@ +--- +source: tui2/src/transcript_copy_ui.rs +expression: rendered +--- +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +xxxxxxxxxx ⧉ copy ctrl + shift + c xxxxx diff --git a/codex-rs/tui2/src/transcript_copy_ui.rs b/codex-rs/tui2/src/transcript_copy_ui.rs index 6ec76a1de9..9bc9e8d664 100644 --- a/codex-rs/tui2/src/transcript_copy_ui.rs +++ b/codex-rs/tui2/src/transcript_copy_ui.rs @@ -287,6 +287,13 @@ impl TranscriptCopyUi { let icon_style = base_style.add_modifier(Modifier::BOLD).fg(Color::LightCyan); let bold_style = base_style.add_modifier(Modifier::BOLD); + // Clear background so underlying text doesn't bleed through the pill. + for position in pill_area.positions() { + let cell = &mut buf[position]; + cell.set_symbol(" "); + cell.set_style(base_style); + } + let mut spans: Vec> = vec![ Span::styled(" ", base_style), Span::styled("⧉", icon_style), @@ -305,6 +312,7 @@ impl TranscriptCopyUi { #[cfg(test)] mod tests { use super::*; + use insta::assert_snapshot; use ratatui::buffer::Buffer; fn buf_to_string(buf: &Buffer, area: Rect) -> String { @@ -359,4 +367,21 @@ mod tests { assert_eq!(rect.y, 1); assert!(ui.hit_test(rect.x, rect.y)); } + + #[test] + fn copy_pill_clears_background() { + let area = Rect::new(0, 0, 40, 3); + let mut buf = Buffer::empty(area); + for y in 0..area.height { + for x in 0..area.width { + buf[(x, y)].set_symbol("x"); + } + } + + let mut ui = TranscriptCopyUi::new_with_shortcut(CopySelectionShortcut::CtrlShiftC); + ui.render_copy_pill(area, &mut buf, (1, 2), (1, 6), 0, 3); + + let rendered = buf_to_string(&buf, area); + assert_snapshot!("copy_pill_clears_background", rendered); + } }