Fix popin for project panel by pre-resolving keybindings in the project panel (#4141)

Mikayla Maki created

Also, factors out the fluent building APIs from IntoElement into their
own traits.

Also makes the project panel context menu initialization fluent:

<img width="1328" alt="Screenshot 2024-01-18 at 3 33 45 PM"
src="https://github.com/zed-industries/zed/assets/2280405/3468b6f2-07f0-48cf-bec1-ac0379333209">

Release Notes:

- Fixed pop in when right clicking on the project panel.

Change summary

crates/collab_ui/src/face_pile.rs                      |   3 
crates/gpui/src/element.rs                             |  38 ---
crates/gpui/src/prelude.rs                             |   6 
crates/gpui/src/util.rs                                |  52 +++++
crates/language_selector/src/active_buffer_language.rs |   2 
crates/project_panel/src/project_panel.rs              | 112 +++++------
crates/ui/src/components/context_menu.rs               |  18 +
crates/ui/src/components/popover_menu.rs               |   6 
8 files changed, 133 insertions(+), 104 deletions(-)

Detailed changes

crates/collab_ui/src/face_pile.rs 🔗

@@ -1,5 +1,6 @@
-use gpui::{div, AnyElement, Div, IntoElement, ParentElement, Styled};
+use gpui::{div, AnyElement, Div, ParentElement, Styled};
 use smallvec::SmallVec;
+use ui::FluentBuilder;
 
 #[derive(Default)]
 pub struct FacePile {

crates/gpui/src/element.rs 🔗

@@ -1,6 +1,6 @@
 use crate::{
-    ArenaBox, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId, Pixels, Point, Size,
-    ViewContext, WindowContext, ELEMENT_ARENA,
+    util::FluentBuilder, ArenaBox, AvailableSpace, BorrowWindow, Bounds, ElementId, LayoutId,
+    Pixels, Point, Size, ViewContext, WindowContext, ELEMENT_ARENA,
 };
 use derive_more::{Deref, DerefMut};
 pub(crate) use smallvec::SmallVec;
@@ -77,40 +77,10 @@ pub trait IntoElement: Sized {
             })
         }
     }
-
-    /// Convert self to another type by calling the given closure. Useful in rendering code.
-    fn map<U>(self, f: impl FnOnce(Self) -> U) -> U
-    where
-        Self: Sized,
-        U: IntoElement,
-    {
-        f(self)
-    }
-
-    /// Conditionally chain onto self with the given closure. Useful in rendering code.
-    fn when(self, condition: bool, then: impl FnOnce(Self) -> Self) -> Self
-    where
-        Self: Sized,
-    {
-        self.map(|this| if condition { then(this) } else { this })
-    }
-
-    /// Conditionally chain onto self with the given closure if the given option is Some.
-    /// The contents of the option are provided to the closure.
-    fn when_some<T>(self, option: Option<T>, then: impl FnOnce(Self, T) -> Self) -> Self
-    where
-        Self: Sized,
-    {
-        self.map(|this| {
-            if let Some(value) = option {
-                then(this, value)
-            } else {
-                this
-            }
-        })
-    }
 }
 
+impl<T: IntoElement> FluentBuilder for T {}
+
 pub trait Render: 'static + Sized {
     fn render(&mut self, cx: &mut ViewContext<Self>) -> impl IntoElement;
 }

crates/gpui/src/prelude.rs 🔗

@@ -1,5 +1,5 @@
 pub use crate::{
-    BorrowAppContext, BorrowWindow, Context, Element, FocusableElement, InteractiveElement,
-    IntoElement, ParentElement, Refineable, Render, RenderOnce, StatefulInteractiveElement, Styled,
-    VisualContext,
+    util::FluentBuilder, BorrowAppContext, BorrowWindow, Context, Element, FocusableElement,
+    InteractiveElement, IntoElement, ParentElement, Refineable, Render, RenderOnce,
+    StatefulInteractiveElement, Styled, VisualContext,
 };

crates/gpui/src/util.rs 🔗

@@ -9,6 +9,58 @@ use smol::future::FutureExt;
 
 pub use util::*;
 
+/// A helper trait for building complex objects with imperative conditionals in a fluent style.
+pub trait FluentBuilder {
+    /// Imperatively modify self with the given closure.
+    fn map<U>(self, f: impl FnOnce(Self) -> U) -> U
+    where
+        Self: Sized,
+    {
+        f(self)
+    }
+
+    /// Conditionally modify self with the given closure.
+    fn when(self, condition: bool, then: impl FnOnce(Self) -> Self) -> Self
+    where
+        Self: Sized,
+    {
+        self.map(|this| if condition { then(this) } else { this })
+    }
+
+    /// Conditionally unwrap and modify self with the given closure, if the given option is Some.
+    fn when_some<T>(self, option: Option<T>, then: impl FnOnce(Self, T) -> Self) -> Self
+    where
+        Self: Sized,
+    {
+        self.map(|this| {
+            if let Some(value) = option {
+                then(this, value)
+            } else {
+                this
+            }
+        })
+    }
+
+    /// Conditionally modify self with one closure or another
+    fn when_else(
+        self,
+        condition: bool,
+        then: impl FnOnce(Self) -> Self,
+        otherwise: impl FnOnce(Self) -> Self,
+    ) -> Self
+    where
+        Self: Sized,
+    {
+        self.map(|this| {
+            if condition {
+                then(this)
+            } else {
+                otherwise(this)
+            }
+        })
+    }
+}
+
 #[cfg(any(test, feature = "test-support"))]
 pub async fn timeout<F, T>(timeout: Duration, f: F) -> Result<T, ()>
 where

crates/language_selector/src/active_buffer_language.rs 🔗

@@ -1,7 +1,7 @@
 use editor::Editor;
 use gpui::{div, IntoElement, ParentElement, Render, Subscription, View, ViewContext, WeakView};
 use std::sync::Arc;
-use ui::{Button, ButtonCommon, Clickable, LabelSize, Tooltip};
+use ui::{Button, ButtonCommon, Clickable, FluentBuilder, LabelSize, Tooltip};
 use workspace::{item::ItemHandle, StatusItemView, Workspace};
 
 use crate::LanguageSelector;

crates/project_panel/src/project_panel.rs 🔗

@@ -381,67 +381,57 @@ impl ProjectPanel {
             let is_local = project.is_local();
             let is_read_only = project.is_read_only();
 
-            let context_menu = ContextMenu::build(cx, |mut menu, cx| {
-                if is_read_only {
-                    menu = menu.action("Copy Relative Path", Box::new(CopyRelativePath));
-                    if is_dir {
-                        menu = menu.action("Search Inside", Box::new(NewSearchInDirectory))
-                    }
-
-                    return menu;
-                }
-
-                if is_local {
-                    menu = menu.action(
-                        "Add Folder to Project",
-                        Box::new(workspace::AddFolderToProject),
-                    );
-                    if is_root {
-                        menu = menu.entry(
-                            "Remove from Project",
-                            None,
-                            cx.handler_for(&this, move |this, cx| {
-                                this.project.update(cx, |project, cx| {
-                                    project.remove_worktree(worktree_id, cx)
-                                });
-                            }),
-                        );
-                    }
-                }
-
-                menu = menu
-                    .action("New File", Box::new(NewFile))
-                    .action("New Folder", Box::new(NewDirectory))
-                    .separator()
-                    .action("Cut", Box::new(Cut))
-                    .action("Copy", Box::new(Copy));
-
-                if let Some(clipboard_entry) = self.clipboard_entry {
-                    if clipboard_entry.worktree_id() == worktree_id {
-                        menu = menu.action("Paste", Box::new(Paste));
-                    }
-                }
-
-                menu = menu
-                    .separator()
-                    .action("Copy Path", Box::new(CopyPath))
-                    .action("Copy Relative Path", Box::new(CopyRelativePath))
-                    .separator()
-                    .action("Reveal in Finder", Box::new(RevealInFinder));
-
-                if is_dir {
-                    menu = menu
-                        .action("Open in Terminal", Box::new(OpenInTerminal))
-                        .action("Search Inside", Box::new(NewSearchInDirectory))
-                }
-
-                menu = menu.separator().action("Rename", Box::new(Rename));
-
-                if !is_root {
-                    menu = menu.action("Delete", Box::new(Delete));
-                }
-
-                menu
+            let context_menu = ContextMenu::build(cx, |menu, cx| {
+                menu.context(self.focus_handle.clone()).when_else(
+                    is_read_only,
+                    |menu| {
+                        menu.action("Copy Relative Path", Box::new(CopyRelativePath))
+                            .when(is_dir, |menu| {
+                                menu.action("Search Inside", Box::new(NewSearchInDirectory))
+                            })
+                    },
+                    |menu| {
+                        menu.when(is_local, |menu| {
+                            menu.action(
+                                "Add Folder to Project",
+                                Box::new(workspace::AddFolderToProject),
+                            )
+                            .when(is_root, |menu| {
+                                menu.entry(
+                                    "Remove from Project",
+                                    None,
+                                    cx.handler_for(&this, move |this, cx| {
+                                        this.project.update(cx, |project, cx| {
+                                            project.remove_worktree(worktree_id, cx)
+                                        });
+                                    }),
+                                )
+                            })
+                        })
+                        .action("New File", Box::new(NewFile))
+                        .action("New Folder", Box::new(NewDirectory))
+                        .separator()
+                        .action("Cut", Box::new(Cut))
+                        .action("Copy", Box::new(Copy))
+                        .when_some(self.clipboard_entry, |menu, entry| {
+                            menu.when(entry.worktree_id() == worktree_id, |menu| {
+                                menu.action("Paste", Box::new(Paste))
+                            })
+                        })
+                        .separator()
+                        .action("Copy Path", Box::new(CopyPath))
+                        .action("Copy Relative Path", Box::new(CopyRelativePath))
+                        .separator()
+                        .action("Reveal in Finder", Box::new(RevealInFinder))
+                        .when(is_dir, |menu| {
+                            menu.action("Open in Terminal", Box::new(OpenInTerminal))
+                                .action("Search Inside", Box::new(NewSearchInDirectory))
+                        })
+                        .separator()
+                        .action("Rename", Box::new(Rename))
+                        .when(!is_root, |menu| menu.action("Delete", Box::new(Delete)))
+                    },
+                )
             });
 
             cx.focus_view(&context_menu);

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

@@ -27,6 +27,7 @@ enum ContextMenuItem {
 pub struct ContextMenu {
     items: Vec<ContextMenuItem>,
     focus_handle: FocusHandle,
+    action_context: Option<FocusHandle>,
     selected_index: Option<usize>,
     delayed: bool,
     clicked: bool,
@@ -41,6 +42,8 @@ impl FocusableView for ContextMenu {
 
 impl EventEmitter<DismissEvent> for ContextMenu {}
 
+impl FluentBuilder for ContextMenu {}
+
 impl ContextMenu {
     pub fn build(
         cx: &mut WindowContext,
@@ -56,6 +59,7 @@ impl ContextMenu {
                 Self {
                     items: Default::default(),
                     focus_handle,
+                    action_context: None,
                     selected_index: None,
                     delayed: false,
                     clicked: false,
@@ -66,6 +70,11 @@ impl ContextMenu {
         })
     }
 
+    pub fn context(mut self, focus: FocusHandle) -> Self {
+        self.action_context = Some(focus);
+        self
+    }
+
     pub fn header(mut self, title: impl Into<SharedString>) -> Self {
         self.items.push(ContextMenuItem::Header(title.into()));
         self
@@ -305,7 +314,14 @@ impl Render for ContextMenu {
                                         .child(label_element)
                                         .debug_selector(|| format!("MENU_ITEM-{}", label))
                                         .children(action.as_ref().and_then(|action| {
-                                            KeyBinding::for_action(&**action, cx)
+                                            self.action_context
+                                                .as_ref()
+                                                .map(|focus| {
+                                                    KeyBinding::for_action_in(&**action, focus, cx)
+                                                })
+                                                .unwrap_or_else(|| {
+                                                    KeyBinding::for_action(&**action, cx)
+                                                })
                                                 .map(|binding| div().ml_1().child(binding))
                                         })),
                                 )

crates/ui/src/components/popover_menu.rs 🔗

@@ -1,9 +1,9 @@
 use std::{cell::RefCell, rc::Rc};
 
 use gpui::{
-    overlay, point, px, rems, AnchorCorner, AnyElement, Bounds, DismissEvent, DispatchPhase,
-    Element, ElementId, InteractiveBounds, IntoElement, LayoutId, ManagedView, MouseDownEvent,
-    ParentElement, Pixels, Point, View, VisualContext, WindowContext,
+    overlay, point, prelude::FluentBuilder, px, rems, AnchorCorner, AnyElement, Bounds,
+    DismissEvent, DispatchPhase, Element, ElementId, InteractiveBounds, IntoElement, LayoutId,
+    ManagedView, MouseDownEvent, ParentElement, Pixels, Point, View, VisualContext, WindowContext,
 };
 
 use crate::{Clickable, Selectable};