From 66814464779ee531ac3be8c40bb6f5fe7d38fb38 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 1 Jun 2026 09:17:19 -0700 Subject: [PATCH] Reset slash popup selection when filter changes (#25492) ## Summary Fixes #25295. The slash-command popup reused its previous `ScrollState` when the composer filter token changed. After scrolling the full `/` command list, typing a narrower filter such as `/st` could clamp the stale selection into the filtered results and highlight the wrong command. This resets the popup selection and viewport only when the parsed filter token changes, so normal arrow navigation is preserved while new filters start at the first match. --- codex-rs/tui/src/bottom_pane/command_popup.rs | 37 +++++++++++++++++++ ...mmand_popup_filter_reset_after_scroll.snap | 25 +++++++++++++ 2 files changed, 62 insertions(+) create mode 100644 codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_filter_reset_after_scroll.snap diff --git a/codex-rs/tui/src/bottom_pane/command_popup.rs b/codex-rs/tui/src/bottom_pane/command_popup.rs index f6e6b4929..9d38d4033 100644 --- a/codex-rs/tui/src/bottom_pane/command_popup.rs +++ b/codex-rs/tui/src/bottom_pane/command_popup.rs @@ -98,6 +98,7 @@ impl CommandPopup { /// to narrow down the list of available commands. pub(crate) fn on_composer_text_change(&mut self, text: String) { let first_line = text.lines().next().unwrap_or(""); + let previous_filter = self.command_filter.clone(); if let Some(stripped) = first_line.strip_prefix('/') { // Extract the *first* token (sequence of non-whitespace @@ -116,6 +117,10 @@ impl CommandPopup { self.command_filter.clear(); } + if self.command_filter != previous_filter { + self.state.reset(); + } + // Reset or clamp selected index based on new filtered list. let matches_len = self.filtered_items().len(); self.state.clamp_selection(matches_len); @@ -407,6 +412,38 @@ mod tests { ); } + #[test] + fn changing_filter_resets_selection_after_scrolling() { + let mut popup = CommandPopup::new(CommandPopupFlags::default(), Vec::new()); + popup.on_composer_text_change("/".to_string()); + + for _ in 0..MAX_POPUP_ROWS { + popup.move_down(); + } + assert!(popup.state.scroll_top > 0); + + popup.on_composer_text_change("/st".to_string()); + + assert_eq!( + popup.selected_item(), + Some(CommandItem::Builtin(SlashCommand::Status)) + ); + assert_eq!(popup.state.scroll_top, 0); + let width = 72; + let area = Rect::new( + /*x*/ 0, + /*y*/ 0, + width, + popup.calculate_required_height(width), + ); + let mut buf = Buffer::empty(area); + popup.render_ref(area, &mut buf); + insta::assert_snapshot!( + "command_popup_filter_reset_after_scroll", + format!("{buf:?}") + ); + } + #[test] fn quit_hidden_in_empty_filter_but_shown_for_prefix() { let mut popup = CommandPopup::new(CommandPopupFlags::default(), Vec::new()); diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_filter_reset_after_scroll.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_filter_reset_after_scroll.snap new file mode 100644 index 000000000..424c13151 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__command_popup__tests__command_popup_filter_reset_after_scroll.snap @@ -0,0 +1,25 @@ +--- +source: tui/src/bottom_pane/command_popup.rs +expression: "format!(\"{buf:?}\")" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 72, height: 3 }, + content: [ + " /status show current session configuration and token usage ", + " /statusline configure which items appear in the status line ", + " /stop stop all background terminals ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 2, y: 0, fg: Cyan, bg: Reset, underline: Reset, modifier: BOLD, + x: 65, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 3, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 5, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 15, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 62, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 3, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 5, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 15, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 44, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +}