Titlebar project menu double click (#3784)

This addresses a bug where popover menus in the titlebar were rendered
only after a 2nd click. The first click was creating the View which the
second one then rendered.
In addition to this, `PopoverMenu::menu` function argument can now
return an `Option<View<T>>` instead of `View<T>` as the creation of the
menu can fail (as it might in case of git popover).

Release Notes:

- N/A

crates/collab_ui2/src/collab_titlebar_item.rs

@@ -18,7 +18,7 @@ use ui::{
 };
 use util::ResultExt;
 use vcs_menu::{build_branch_list, BranchList, OpenRecent as ToggleVcsMenu};
-use workspace::{notifications::NotifyResultExt, Workspace, WORKSPACE_DB};
+use workspace::{notifications::NotifyResultExt, Workspace};
 
 const MAX_PROJECT_NAME_LENGTH: usize = 40;
 const MAX_BRANCH_NAME_LENGTH: usize = 40;
@@ -52,8 +52,6 @@ pub struct CollabTitlebarItem {
     user_store: Model<UserStore>,
     client: Arc<Client>,
     workspace: WeakView<Workspace>,
-    branch_popover: Option<(View<BranchList>, Subscription)>,
-    project_popover: Option<(View<recent_projects::RecentProjects>, Subscription)>,
     _subscriptions: Vec<Subscription>,
 }
 
@@ -291,8 +289,6 @@ impl CollabTitlebarItem {
             project,
             user_store,
             client,
-            branch_popover: None,
-            project_popover: None,
             _subscriptions: subscriptions,
         }
     }
@@ -329,22 +325,15 @@ impl CollabTitlebarItem {
         };
 
         let name = util::truncate_and_trailoff(name, MAX_PROJECT_NAME_LENGTH);
+        let workspace = self.workspace.clone();
         popover_menu("project_name_trigger")
             .trigger(
                 Button::new("project_name_trigger", name)
                     .style(ButtonStyle::Subtle)
                     .label_size(LabelSize::Small)
-                    .tooltip(move |cx| Tooltip::text("Recent Projects", cx))
+                    .tooltip(move |cx| Tooltip::text("Recent Projects", cx)),
-                    .on_click(cx.listener(|this, _, cx| {
-                        this.toggle_project_menu(&ToggleProjectMenu, cx);
-                    })),
-            )
-            .when_some(
-                self.project_popover
-                    .as_ref()
-                    .map(|(project, _)| project.clone()),
-                |this, project| this.menu(move |_| project.clone()),
             )
+            .menu(move |cx| Some(Self::render_project_popover(workspace.clone(), cx)))
     }
 
     pub fn render_project_branch(&self, cx: &mut ViewContext<Self>) -> Option<impl Element> {
@@ -357,7 +346,7 @@ impl CollabTitlebarItem {
 
             names_and_branches.next().flatten()
         };
-
+        let workspace = self.workspace.upgrade()?;
         let branch_name = entry
             .as_ref()
             .and_then(RepositoryEntry::branch)
@@ -376,17 +365,9 @@ impl CollabTitlebarItem {
                                 "Local branches only",
                                 cx,
                             )
-                        })
+                        }),
-                        .on_click(cx.listener(|this, _, cx| {
-                            this.toggle_vcs_menu(&ToggleVcsMenu, cx);
-                        })),
                 )
-                .when_some(
+                .menu(move |cx| Self::render_vcs_popover(workspace.clone(), cx)),
-                    self.branch_popover
-                        .as_ref()
-                        .map(|(branch, _)| branch.clone()),
-                    |this, branch| this.menu(move |_| branch.clone()),
-                ),
         )
     }
 
@@ -462,55 +443,25 @@ impl CollabTitlebarItem {
             .log_err();
     }
 
-    pub fn toggle_vcs_menu(&mut self, _: &ToggleVcsMenu, cx: &mut ViewContext<Self>) {
+    pub fn render_vcs_popover(
-        if self.branch_popover.take().is_none() {
+        workspace: View<Workspace>,
-            if let Some(workspace) = self.workspace.upgrade() {
+        cx: &mut WindowContext<'_>,
-                let Some(view) = build_branch_list(workspace, cx).log_err() else {
+    ) -> Option<View<BranchList>> {
-                    return;
+        let view = build_branch_list(workspace, cx).log_err()?;
-                };
-                self.project_popover.take();
         let focus_handle = view.focus_handle(cx);
         cx.focus(&focus_handle);
-                let subscription = cx.subscribe(&view, |this, _, _, _| {
+        Some(view)
-                    this.branch_popover.take();
-                });
-                self.branch_popover = Some((view, subscription));
-            }
     }
 
-        cx.notify();
+    pub fn render_project_popover(
-    }
+        workspace: WeakView<Workspace>,
-
+        cx: &mut WindowContext<'_>,
-    pub fn toggle_project_menu(&mut self, _: &ToggleProjectMenu, cx: &mut ViewContext<Self>) {
+    ) -> View<RecentProjects> {
-        if self.project_popover.take().is_none() {
+        let view = RecentProjects::open_popover(workspace, cx);
-            let workspace = self.workspace.clone();
-            cx.spawn(|this, mut cx| async move {
-                let workspaces = WORKSPACE_DB
-                    .recent_workspaces_on_disk()
-                    .await
-                    .unwrap_or_default()
-                    .into_iter()
-                    .map(|(_, location)| location)
-                    .collect();
-
-                let workspace = workspace.clone();
-                this.update(&mut cx, move |this, cx| {
-                    let view = RecentProjects::open_popover(workspace, workspaces, cx);
 
         let focus_handle = view.focus_handle(cx);
         cx.focus(&focus_handle);
-                    this.branch_popover.take();
+        view
-                    let subscription = cx.subscribe(&view, |this, _, _, _| {
-                        this.project_popover.take();
-                    });
-                    this.project_popover = Some((view, subscription));
-                    cx.notify();
-                })
-                .log_err();
-            })
-            .detach();
-        }
-        cx.notify();
     }
 
     fn render_connection_status(
@@ -587,6 +538,7 @@ impl CollabTitlebarItem {
                             .action("Share Feedback", feedback::GiveFeedback.boxed_clone())
                             .action("Sign Out", client::SignOut.boxed_clone())
                     })
+                    .into()
                 })
                 .trigger(
                     ButtonLike::new("user-menu")
@@ -609,6 +561,7 @@ impl CollabTitlebarItem {
                             .separator()
                             .action("Share Feedback", feedback::GiveFeedback.boxed_clone())
                     })
+                    .into()
                 })
                 .trigger(
                     ButtonLike::new("user-menu")

crates/copilot_button2/src/copilot_button.rs

@@ -102,8 +102,10 @@ impl Render for CopilotButton {
         div().child(
             popover_menu("copilot")
                 .menu(move |cx| match status {
-                    Status::Authorized => this.update(cx, |this, cx| this.build_copilot_menu(cx)),
+                    Status::Authorized => {
-                    _ => this.update(cx, |this, cx| this.build_copilot_start_menu(cx)),
+                        Some(this.update(cx, |this, cx| this.build_copilot_menu(cx)))
+                    }
+                    _ => Some(this.update(cx, |this, cx| this.build_copilot_start_menu(cx))),
                 })
                 .anchor(AnchorCorner::BottomRight)
                 .trigger(

crates/language_tools2/src/lsp_log.rs

@@ -828,6 +828,7 @@ impl Render for LspLogToolbarItemView {
                     }
                     menu
                 })
+                .into()
             });
 
         h_stack().size_full().child(lsp_menu).child(

crates/language_tools2/src/syntax_tree_view.rs

@@ -467,6 +467,7 @@ impl SyntaxTreeToolbarItemView {
                         }
                         menu
                     })
+                    .into()
                 }),
         )
     }

crates/recent_projects2/src/recent_projects.rs

@@ -12,10 +12,7 @@ use picker::{Picker, PickerDelegate};
 use std::sync::Arc;
 use ui::{prelude::*, ListItem, ListItemSpacing};
 use util::paths::PathExt;
-use workspace::{
+use workspace::{ModalView, Workspace, WorkspaceLocation, WORKSPACE_DB};
-    notifications::simple_message_notification::MessageNotification, ModalView, Workspace,
-    WorkspaceLocation, WORKSPACE_DB,
-};
 
 pub use projects::OpenRecent;
 
@@ -35,6 +32,25 @@ impl RecentProjects {
     fn new(delegate: RecentProjectsDelegate, rem_width: f32, cx: &mut ViewContext<Self>) -> Self {
         let picker = cx.build_view(|cx| Picker::new(delegate, cx));
         let _subscription = cx.subscribe(&picker, |_, _, _, cx| cx.emit(DismissEvent));
+        // We do not want to block the UI on a potentially lenghty call to DB, so we're gonna swap
+        // out workspace locations once the future runs to completion.
+        cx.spawn(|this, mut cx| async move {
+            let workspaces = WORKSPACE_DB
+                .recent_workspaces_on_disk()
+                .await
+                .unwrap_or_default()
+                .into_iter()
+                .map(|(_, location)| location)
+                .collect();
+            this.update(&mut cx, move |this, cx| {
+                this.picker.update(cx, move |picker, cx| {
+                    picker.delegate.workspace_locations = workspaces;
+                    picker.update_matches(picker.query(cx), cx)
+                })
+            })
+            .ok()
+        })
+        .detach();
         Self {
             picker,
             rem_width,
@@ -61,50 +77,20 @@ impl RecentProjects {
 
     fn open(_: &mut Workspace, cx: &mut ViewContext<Workspace>) -> Option<Task<Result<()>>> {
         Some(cx.spawn(|workspace, mut cx| async move {
-            let workspace_locations: Vec<_> = cx
-                .background_executor()
-                .spawn(async {
-                    WORKSPACE_DB
-                        .recent_workspaces_on_disk()
-                        .await
-                        .unwrap_or_default()
-                        .into_iter()
-                        .map(|(_, location)| location)
-                        .collect()
-                })
-                .await;
-
             workspace.update(&mut cx, |workspace, cx| {
-                if !workspace_locations.is_empty() {
                 let weak_workspace = cx.view().downgrade();
                 workspace.toggle_modal(cx, |cx| {
-                        let delegate =
+                    let delegate = RecentProjectsDelegate::new(weak_workspace, true);
-                            RecentProjectsDelegate::new(weak_workspace, workspace_locations, true);
 
                     let modal = RecentProjects::new(delegate, 34., cx);
                     modal
                 });
-                } else {
-                    workspace.show_notification(0, cx, |cx| {
-                        cx.build_view(|_| MessageNotification::new("No recent projects to open."))
-                    })
-                }
             })?;
             Ok(())
         }))
     }
-    pub fn open_popover(
+    pub fn open_popover(workspace: WeakView<Workspace>, cx: &mut WindowContext<'_>) -> View<Self> {
-        workspace: WeakView<Workspace>,
+        cx.build_view(|cx| Self::new(RecentProjectsDelegate::new(workspace, false), 20., cx))
-        workspaces: Vec<WorkspaceLocation>,
-        cx: &mut WindowContext<'_>,
-    ) -> View<Self> {
-        cx.build_view(|cx| {
-            Self::new(
-                RecentProjectsDelegate::new(workspace, workspaces, false),
-                20.,
-                cx,
-            )
-        })
     }
 }
 
@@ -140,14 +126,10 @@ pub struct RecentProjectsDelegate {
 }
 
 impl RecentProjectsDelegate {
-    fn new(
+    fn new(workspace: WeakView<Workspace>, render_paths: bool) -> Self {
-        workspace: WeakView<Workspace>,
-        workspace_locations: Vec<WorkspaceLocation>,
-        render_paths: bool,
-    ) -> Self {
         Self {
             workspace,
-            workspace_locations,
+            workspace_locations: vec![],
             selected_match_index: 0,
             matches: Default::default(),
             render_paths,

crates/ui2/src/components/popover_menu.rs

@@ -18,19 +18,19 @@ pub struct PopoverMenu<M: ManagedView> {
         Box<
             dyn FnOnce(
                     Rc<RefCell<Option<View<M>>>>,
-                    Option<Rc<dyn Fn(&mut WindowContext) -> View<M> + 'static>>,
+                    Option<Rc<dyn Fn(&mut WindowContext) -> Option<View<M>> + 'static>>,
                 ) -> AnyElement
                 + 'static,
         >,
     >,
-    menu_builder: Option<Rc<dyn Fn(&mut WindowContext) -> View<M> + 'static>>,
+    menu_builder: Option<Rc<dyn Fn(&mut WindowContext) -> Option<View<M>> + 'static>>,
     anchor: AnchorCorner,
     attach: Option<AnchorCorner>,
     offset: Option<Point<Pixels>>,
 }
 
 impl<M: ManagedView> PopoverMenu<M> {
-    pub fn menu(mut self, f: impl Fn(&mut WindowContext) -> View<M> + 'static) -> Self {
+    pub fn menu(mut self, f: impl Fn(&mut WindowContext) -> Option<View<M>> + 'static) -> Self {
         self.menu_builder = Some(Rc::new(f));
         self
     }
@@ -42,7 +42,9 @@ impl<M: ManagedView> PopoverMenu<M> {
                 .when_some(builder, |el, builder| {
                     el.on_click({
                         move |_, cx| {
-                            let new_menu = (builder)(cx);
+                            let Some(new_menu) = (builder)(cx) else {
+                                return;
+                            };
                             let menu2 = menu.clone();
                             let previous_focus_handle = cx.focused();