agent: Keyboard navigation improvements (#30274)

Cole Miller created 7 months ago

- Fix `ctrl-p` not working in the model selector
- Select first entry when opening the context picker

Release Notes:

- Fixed `menu::SelectPrevious` keybindings not working in the agent
panel's model selector.

Change summary

crates/agent/src/context_picker.rs       | 10 ++++++++++
crates/agent/src/context_strip.rs        | 25 +++++++++++++++++++------
crates/picker/src/picker.rs              | 23 +++++++++++++++++++++--
crates/ui/src/components/context_menu.rs |  2 +-
4 files changed, 51 insertions(+), 9 deletions(-)

Detailed changes

crates/agent/src/context_picker.rs

@@ -381,6 +381,16 @@ impl ContextPicker {
         cx.focus_self(window);
     }
 
+    pub fn select_first(&mut self, window: &mut Window, cx: &mut Context<Self>) {
+        match &self.mode {
+            ContextPickerState::Default(entity) => entity.update(cx, |entity, cx| {
+                entity.select_first(&Default::default(), window, cx)
+            }),
+            // Other variants already select their first entry on open automatically
+            _ => {}
+        }
+    }
+
     fn recent_menu_item(
         &self,
         context_picker: Entity<ContextPicker>,

crates/agent/src/context_strip.rs

@@ -420,12 +420,25 @@ impl Render for ContextStrip {
             })
             .child(
                 PopoverMenu::new("context-picker")
-                    .menu(move |window, cx| {
-                        context_picker.update(cx, |this, cx| {
-                            this.init(window, cx);
-                        });
-
-                        Some(context_picker.clone())
+                    .menu({
+                        let context_picker = context_picker.clone();
+                        move |window, cx| {
+                            context_picker.update(cx, |this, cx| {
+                                this.init(window, cx);
+                            });
+
+                            Some(context_picker.clone())
+                        }
+                    })
+                    .on_open({
+                        let context_picker = context_picker.downgrade();
+                        Rc::new(move |window, cx| {
+                            context_picker
+                                .update(cx, |context_picker, cx| {
+                                    context_picker.select_first(window, cx);
+                                })
+                                .ok();
+                        })
                     })
                     .trigger_with_tooltip(
                         IconButton::new("add-context", IconName::Plus)

crates/picker/src/picker.rs

@@ -1,5 +1,9 @@
 use anyhow::Result;
-use editor::{Editor, scroll::Autoscroll};
+use editor::{
+    Editor,
+    actions::{MoveDown, MoveUp},
+    scroll::Autoscroll,
+};
 use gpui::{
     AnyElement, App, ClickEvent, Context, DismissEvent, Entity, EventEmitter, FocusHandle,
     Focusable, Length, ListSizingBehavior, ListState, MouseButton, MouseUpEvent, Render,
@@ -451,6 +455,10 @@ impl<D: PickerDelegate> Picker<D> {
         }
     }
 
+    pub fn editor_move_up(&mut self, _: &MoveUp, window: &mut Window, cx: &mut Context<Self>) {
+        self.select_previous(&Default::default(), window, cx);
+    }
+
     fn select_previous(
         &mut self,
         _: &menu::SelectPrevious,
@@ -466,7 +474,16 @@ impl<D: PickerDelegate> Picker<D> {
         }
     }
 
-    fn select_first(&mut self, _: &menu::SelectFirst, window: &mut Window, cx: &mut Context<Self>) {
+    pub fn editor_move_down(&mut self, _: &MoveDown, window: &mut Window, cx: &mut Context<Self>) {
+        self.select_next(&Default::default(), window, cx);
+    }
+
+    pub fn select_first(
+        &mut self,
+        _: &menu::SelectFirst,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
         let count = self.delegate.match_count();
         if count > 0 {
             self.set_selected_index(0, Some(Direction::Down), true, window, cx);
@@ -857,6 +874,8 @@ impl<D: PickerDelegate> Render for Picker<D> {
             .when(self.is_modal, |this| this.elevation_3(cx))
             .on_action(cx.listener(Self::select_next))
             .on_action(cx.listener(Self::select_previous))
+            .on_action(cx.listener(Self::editor_move_down))
+            .on_action(cx.listener(Self::editor_move_up))
             .on_action(cx.listener(Self::select_first))
             .on_action(cx.listener(Self::select_last))
             .on_action(cx.listener(Self::cancel))

crates/ui/src/components/context_menu.rs

@@ -673,7 +673,7 @@ impl ContextMenu {
         self.selected_index = None;
     }
 
-    fn select_first(&mut self, _: &SelectFirst, window: &mut Window, cx: &mut Context<Self>) {
+    pub fn select_first(&mut self, _: &SelectFirst, window: &mut Window, cx: &mut Context<Self>) {
         if let Some(ix) = self.items.iter().position(|item| item.is_selectable()) {
             self.select_index(ix, window, cx);
         }