Add keyboard control over context menus

Max Brunsfeld and Conrad created

Co-authored-by: Conrad <conrad@zed.dev>

Change summary

crates/gpui2/src/window.rs                        |  12 
crates/project_panel2/src/project_panel.rs        |  18 +
crates/ui2/src/components/context_menu.rs         | 156 +++++++++++-----
crates/ui2/src/components/stories/context_menu.rs |   4 
crates/workspace2/src/dock.rs                     |   2 
5 files changed, 126 insertions(+), 66 deletions(-)

Detailed changes

crates/gpui2/src/window.rs 🔗

@@ -1482,13 +1482,15 @@ impl<'a> WindowContext<'a> {
         }
     }
 
-    pub fn constructor_for<V: Render, R>(
+    pub fn handler_for<V: Render>(
         &self,
         view: &View<V>,
-        f: impl Fn(&mut V, &mut ViewContext<V>) -> R + 'static,
-    ) -> impl Fn(&mut WindowContext) -> R + 'static {
-        let view = view.clone();
-        move |cx: &mut WindowContext| view.update(cx, |view, cx| f(view, cx))
+        f: impl Fn(&mut V, &mut ViewContext<V>) + 'static,
+    ) -> impl Fn(&mut WindowContext) {
+        let view = view.downgrade();
+        move |cx: &mut WindowContext| {
+            view.update(cx, |view, cx| f(view, cx)).ok();
+        }
     }
 
     //========== ELEMENT RELATED FUNCTIONS ===========

crates/project_panel2/src/project_panel.rs 🔗

@@ -9,10 +9,10 @@ use file_associations::FileAssociations;
 use anyhow::{anyhow, Result};
 use gpui::{
     actions, div, overlay, px, uniform_list, Action, AppContext, AssetSource, AsyncWindowContext,
-    ClipboardItem, Div, EventEmitter, FocusHandle, Focusable, FocusableView, InteractiveElement,
-    Model, MouseButton, MouseDownEvent, ParentElement, Pixels, Point, PromptLevel, Render,
-    Stateful, Styled, Subscription, Task, UniformListScrollHandle, View, ViewContext,
-    VisualContext as _, WeakView, WindowContext,
+    ClipboardItem, DismissEvent, Div, EventEmitter, FocusHandle, Focusable, FocusableView,
+    InteractiveElement, Model, MouseButton, MouseDownEvent, ParentElement, Pixels, Point,
+    PromptLevel, Render, Stateful, Styled, Subscription, Task, UniformListScrollHandle, View,
+    ViewContext, VisualContext as _, WeakView, WindowContext,
 };
 use menu::{Confirm, SelectNext, SelectPrev};
 use project::{
@@ -403,7 +403,7 @@ impl ProjectPanel {
                     if is_root {
                         menu = menu.entry(
                             "Remove from Project",
-                            cx.listener_for(&this, move |this, _, cx| {
+                            cx.handler_for(&this, move |this, cx| {
                                 this.project.update(cx, |project, cx| {
                                     project.remove_worktree(worktree_id, cx)
                                 });
@@ -448,9 +448,11 @@ impl ProjectPanel {
             });
 
             cx.focus_view(&context_menu);
-            let subscription = cx.on_blur(&context_menu.focus_handle(cx), |this, cx| {
-                this.context_menu.take();
-                cx.notify();
+            let subscription = cx.subscribe(&context_menu, |this, _, event, cx| match event {
+                DismissEvent::Dismiss => {
+                    this.context_menu.take();
+                    cx.notify();
+                }
             });
             self.context_menu = Some((context_menu, position, subscription));
         }

crates/ui2/src/components/context_menu.rs 🔗

@@ -2,10 +2,11 @@ use crate::{
     h_stack, prelude::*, v_stack, KeyBinding, Label, List, ListItem, ListSeparator, ListSubHeader,
 };
 use gpui::{
-    overlay, px, Action, AnchorCorner, AnyElement, AppContext, Bounds, ClickEvent, DismissEvent,
-    DispatchPhase, Div, EventEmitter, FocusHandle, FocusableView, IntoElement, LayoutId,
-    ManagedView, MouseButton, MouseDownEvent, Pixels, Point, Render, View, VisualContext,
+    overlay, px, Action, AnchorCorner, AnyElement, AppContext, Bounds, DismissEvent, DispatchPhase,
+    Div, EventEmitter, FocusHandle, FocusableView, IntoElement, LayoutId, ManagedView, MouseButton,
+    MouseDownEvent, Pixels, Point, Render, View, VisualContext,
 };
+use menu::{SelectFirst, SelectLast, SelectNext, SelectPrev};
 use std::{cell::RefCell, rc::Rc};
 
 pub enum ContextMenuItem {
@@ -13,7 +14,7 @@ pub enum ContextMenuItem {
     Header(SharedString),
     Entry {
         label: SharedString,
-        click_handler: Rc<dyn Fn(&ClickEvent, &mut WindowContext)>,
+        handler: Rc<dyn Fn(&mut WindowContext)>,
         key_binding: Option<KeyBinding>,
     },
 }
@@ -21,6 +22,7 @@ pub enum ContextMenuItem {
 pub struct ContextMenu {
     items: Vec<ContextMenuItem>,
     focus_handle: FocusHandle,
+    selected_index: Option<usize>,
 }
 
 impl FocusableView for ContextMenu {
@@ -42,6 +44,7 @@ impl ContextMenu {
                 Self {
                     items: Default::default(),
                     focus_handle: cx.focus_handle(),
+                    selected_index: None,
                 },
                 cx,
             )
@@ -61,11 +64,11 @@ impl ContextMenu {
     pub fn entry(
         mut self,
         label: impl Into<SharedString>,
-        on_click: impl Fn(&ClickEvent, &mut WindowContext) + 'static,
+        on_click: impl Fn(&mut WindowContext) + 'static,
     ) -> Self {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
-            click_handler: Rc::new(on_click),
+            handler: Rc::new(on_click),
             key_binding: None,
         });
         self
@@ -80,19 +83,72 @@ impl ContextMenu {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
             key_binding: KeyBinding::for_action(&*action, cx),
-            click_handler: Rc::new(move |_, cx| cx.dispatch_action(action.boxed_clone())),
+            handler: Rc::new(move |cx| cx.dispatch_action(action.boxed_clone())),
         });
         self
     }
 
     pub fn confirm(&mut self, _: &menu::Confirm, cx: &mut ViewContext<Self>) {
-        // todo!()
+        if let Some(ContextMenuItem::Entry { handler, .. }) =
+            self.selected_index.and_then(|ix| self.items.get(ix))
+        {
+            (handler)(cx)
+        }
         cx.emit(DismissEvent::Dismiss);
     }
 
     pub fn cancel(&mut self, _: &menu::Cancel, cx: &mut ViewContext<Self>) {
         cx.emit(DismissEvent::Dismiss);
     }
+
+    fn select_first(&mut self, _: &SelectFirst, cx: &mut ViewContext<Self>) {
+        self.selected_index = self.items.iter().position(|item| item.is_selectable());
+        cx.notify();
+    }
+
+    fn select_last(&mut self, _: &SelectLast, cx: &mut ViewContext<Self>) {
+        for (ix, item) in self.items.iter().enumerate().rev() {
+            if item.is_selectable() {
+                self.selected_index = Some(ix);
+                cx.notify();
+                break;
+            }
+        }
+    }
+
+    fn select_next(&mut self, _: &SelectNext, cx: &mut ViewContext<Self>) {
+        if let Some(ix) = self.selected_index {
+            for (ix, item) in self.items.iter().enumerate().skip(ix + 1) {
+                if item.is_selectable() {
+                    self.selected_index = Some(ix);
+                    cx.notify();
+                    break;
+                }
+            }
+        } else {
+            self.select_first(&Default::default(), cx);
+        }
+    }
+
+    pub fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext<Self>) {
+        if let Some(ix) = self.selected_index {
+            for (ix, item) in self.items.iter().enumerate().take(ix).rev() {
+                if item.is_selectable() {
+                    self.selected_index = Some(ix);
+                    cx.notify();
+                    break;
+                }
+            }
+        } else {
+            self.select_last(&Default::default(), cx);
+        }
+    }
+}
+
+impl ContextMenuItem {
+    fn is_selectable(&self) -> bool {
+        matches!(self, Self::Entry { .. })
+    }
 }
 
 impl Render for ContextMenu {
@@ -103,52 +159,52 @@ impl Render for ContextMenu {
             v_stack()
                 .min_w(px(200.))
                 .track_focus(&self.focus_handle)
-                .on_mouse_down_out(
-                    cx.listener(|this: &mut Self, _, cx| this.cancel(&Default::default(), cx)),
-                )
-                // .on_action(ContextMenu::select_first)
-                // .on_action(ContextMenu::select_last)
-                // .on_action(ContextMenu::select_next)
-                // .on_action(ContextMenu::select_prev)
+                .on_mouse_down_out(cx.listener(|this, _, cx| this.cancel(&Default::default(), cx)))
+                .key_context("menu")
+                .on_action(cx.listener(ContextMenu::select_first))
+                .on_action(cx.listener(ContextMenu::select_last))
+                .on_action(cx.listener(ContextMenu::select_next))
+                .on_action(cx.listener(ContextMenu::select_prev))
                 .on_action(cx.listener(ContextMenu::confirm))
                 .on_action(cx.listener(ContextMenu::cancel))
                 .flex_none()
-                // .bg(cx.theme().colors().elevated_surface_background)
-                // .border()
-                // .border_color(cx.theme().colors().border)
                 .child(
-                    List::new().children(self.items.iter().map(|item| match item {
-                        ContextMenuItem::Separator => ListSeparator::new().into_any_element(),
-                        ContextMenuItem::Header(header) => {
-                            ListSubHeader::new(header.clone()).into_any_element()
-                        }
-                        ContextMenuItem::Entry {
-                            label: entry,
-                            click_handler: callback,
-                            key_binding,
-                        } => {
-                            let callback = callback.clone();
-                            let dismiss = cx.listener(|_, _, cx| cx.emit(DismissEvent::Dismiss));
-
-                            ListItem::new(entry.clone())
-                                .child(
-                                    h_stack()
-                                        .w_full()
-                                        .justify_between()
-                                        .child(Label::new(entry.clone()))
-                                        .children(
-                                            key_binding
-                                                .clone()
-                                                .map(|binding| div().ml_1().child(binding)),
-                                        ),
-                                )
-                                .on_click(move |event, cx| {
-                                    callback(event, cx);
-                                    dismiss(event, cx)
-                                })
-                                .into_any_element()
-                        }
-                    })),
+                    List::new().children(self.items.iter().enumerate().map(
+                        |(ix, item)| match item {
+                            ContextMenuItem::Separator => ListSeparator::new().into_any_element(),
+                            ContextMenuItem::Header(header) => {
+                                ListSubHeader::new(header.clone()).into_any_element()
+                            }
+                            ContextMenuItem::Entry {
+                                label: entry,
+                                handler: callback,
+                                key_binding,
+                            } => {
+                                let callback = callback.clone();
+                                let dismiss =
+                                    cx.listener(|_, _, cx| cx.emit(DismissEvent::Dismiss));
+
+                                ListItem::new(entry.clone())
+                                    .child(
+                                        h_stack()
+                                            .w_full()
+                                            .justify_between()
+                                            .child(Label::new(entry.clone()))
+                                            .children(
+                                                key_binding
+                                                    .clone()
+                                                    .map(|binding| div().ml_1().child(binding)),
+                                            ),
+                                    )
+                                    .selected(Some(ix) == self.selected_index)
+                                    .on_click(move |event, cx| {
+                                        callback(cx);
+                                        dismiss(event, cx)
+                                    })
+                                    .into_any_element()
+                            }
+                        },
+                    )),
                 ),
         )
     }

crates/ui2/src/components/stories/context_menu.rs 🔗

@@ -10,11 +10,11 @@ fn build_menu(cx: &mut WindowContext, header: impl Into<SharedString>) -> View<C
     ContextMenu::build(cx, |menu, _| {
         menu.header(header)
             .separator()
-            .entry("Print current time", |_event, cx| {
+            .entry("Print current time", |cx| {
                 println!("dispatching PrintCurrentTime action");
                 cx.dispatch_action(PrintCurrentDate.boxed_clone())
             })
-            .entry("Print best foot", |_event, cx| {
+            .entry("Print best foot", |cx| {
                 cx.dispatch_action(PrintBestFood.boxed_clone())
             })
     })

crates/workspace2/src/dock.rs 🔗

@@ -717,7 +717,7 @@ impl Render for PanelButtons {
                                         && panel.position_is_valid(position, cx)
                                     {
                                         let panel = panel.clone();
-                                        menu = menu.entry(position.to_label(), move |_, cx| {
+                                        menu = menu.entry(position.to_label(), move |cx| {
                                             panel.set_position(position, cx);
                                         })
                                     }