From dc1ed3c39d27dc1689f2565665c2d436e38db8d9 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Fri, 22 Dec 2023 17:18:12 +0100 Subject: [PATCH] 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>` instead of `View` 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 | 99 +++++-------------- crates/copilot_button2/src/copilot_button.rs | 6 +- crates/language_tools2/src/lsp_log.rs | 1 + .../language_tools2/src/syntax_tree_view.rs | 1 + .../recent_projects2/src/recent_projects.rs | 78 ++++++--------- crates/ui2/src/components/popover_menu.rs | 10 +- 6 files changed, 68 insertions(+), 127 deletions(-) diff --git a/crates/collab_ui2/src/collab_titlebar_item.rs b/crates/collab_ui2/src/collab_titlebar_item.rs index e4a6ff089a17efd8ab8abd36e0e8ade954cfb30a..75c4cc5263895a499d6865dee7c005d99026f031 100644 --- a/crates/collab_ui2/src/collab_titlebar_item.rs +++ b/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, client: Arc, workspace: WeakView, - branch_popover: Option<(View, Subscription)>, - project_popover: Option<(View, Subscription)>, _subscriptions: Vec, } @@ -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)) - .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()), + .tooltip(move |cx| Tooltip::text("Recent Projects", cx)), ) + .menu(move |cx| Some(Self::render_project_popover(workspace.clone(), cx))) } pub fn render_project_branch(&self, cx: &mut ViewContext) -> Option { @@ -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( - self.branch_popover - .as_ref() - .map(|(branch, _)| branch.clone()), - |this, branch| this.menu(move |_| branch.clone()), - ), + .menu(move |cx| Self::render_vcs_popover(workspace.clone(), cx)), ) } @@ -462,55 +443,25 @@ impl CollabTitlebarItem { .log_err(); } - pub fn toggle_vcs_menu(&mut self, _: &ToggleVcsMenu, cx: &mut ViewContext) { - if self.branch_popover.take().is_none() { - if let Some(workspace) = self.workspace.upgrade() { - let Some(view) = build_branch_list(workspace, cx).log_err() else { - return; - }; - self.project_popover.take(); - let focus_handle = view.focus_handle(cx); - cx.focus(&focus_handle); - let subscription = cx.subscribe(&view, |this, _, _, _| { - this.branch_popover.take(); - }); - self.branch_popover = Some((view, subscription)); - } - } - - cx.notify(); + pub fn render_vcs_popover( + workspace: View, + cx: &mut WindowContext<'_>, + ) -> Option> { + let view = build_branch_list(workspace, cx).log_err()?; + let focus_handle = view.focus_handle(cx); + cx.focus(&focus_handle); + Some(view) } - pub fn toggle_project_menu(&mut self, _: &ToggleProjectMenu, cx: &mut ViewContext) { - if self.project_popover.take().is_none() { - 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(); - let subscription = cx.subscribe(&view, |this, _, _, _| { - this.project_popover.take(); - }); - this.project_popover = Some((view, subscription)); - cx.notify(); - }) - .log_err(); - }) - .detach(); - } - cx.notify(); + pub fn render_project_popover( + workspace: WeakView, + cx: &mut WindowContext<'_>, + ) -> View { + let view = RecentProjects::open_popover(workspace, cx); + + let focus_handle = view.focus_handle(cx); + cx.focus(&focus_handle); + view } 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") diff --git a/crates/copilot_button2/src/copilot_button.rs b/crates/copilot_button2/src/copilot_button.rs index 7cd2b09db80d3ef4627cbad23f253bc766c77c4b..d3a1c1e58c0e6e0113cc965017aecf0a3c3cdca9 100644 --- a/crates/copilot_button2/src/copilot_button.rs +++ b/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)), - _ => this.update(cx, |this, cx| this.build_copilot_start_menu(cx)), + Status::Authorized => { + 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( diff --git a/crates/language_tools2/src/lsp_log.rs b/crates/language_tools2/src/lsp_log.rs index 973cd9c355713a30b03b79bc28802991db81f4da..310726e4a525c0cbd77eb0ad7092e7f86fca7a27 100644 --- a/crates/language_tools2/src/lsp_log.rs +++ b/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( diff --git a/crates/language_tools2/src/syntax_tree_view.rs b/crates/language_tools2/src/syntax_tree_view.rs index c9a621f967155097a97fc9017ed6b17ccba1f7ec..e4ee4b0cfb6a7532670beb376262afeb6ca5be44 100644 --- a/crates/language_tools2/src/syntax_tree_view.rs +++ b/crates/language_tools2/src/syntax_tree_view.rs @@ -467,6 +467,7 @@ impl SyntaxTreeToolbarItemView { } menu }) + .into() }), ) } diff --git a/crates/recent_projects2/src/recent_projects.rs b/crates/recent_projects2/src/recent_projects.rs index 1a38a5c2004044085dbfaed394255e7dd87c1a19..2d6f4bf0e3160a214bc7a25a24a10d94129c694f 100644 --- a/crates/recent_projects2/src/recent_projects.rs +++ b/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::{ - notifications::simple_message_notification::MessageNotification, ModalView, Workspace, - WorkspaceLocation, WORKSPACE_DB, -}; +use workspace::{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 { 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) -> Option>> { 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 = - RecentProjectsDelegate::new(weak_workspace, workspace_locations, true); + let weak_workspace = cx.view().downgrade(); + workspace.toggle_modal(cx, |cx| { + let delegate = RecentProjectsDelegate::new(weak_workspace, 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.")) - }) - } + let modal = RecentProjects::new(delegate, 34., cx); + modal + }); })?; Ok(()) })) } - pub fn open_popover( - workspace: WeakView, - workspaces: Vec, - cx: &mut WindowContext<'_>, - ) -> View { - cx.build_view(|cx| { - Self::new( - RecentProjectsDelegate::new(workspace, workspaces, false), - 20., - cx, - ) - }) + pub fn open_popover(workspace: WeakView, cx: &mut WindowContext<'_>) -> View { + cx.build_view(|cx| Self::new(RecentProjectsDelegate::new(workspace, false), 20., cx)) } } @@ -140,14 +126,10 @@ pub struct RecentProjectsDelegate { } impl RecentProjectsDelegate { - fn new( - workspace: WeakView, - workspace_locations: Vec, - render_paths: bool, - ) -> Self { + fn new(workspace: WeakView, render_paths: bool) -> Self { Self { workspace, - workspace_locations, + workspace_locations: vec![], selected_match_index: 0, matches: Default::default(), render_paths, diff --git a/crates/ui2/src/components/popover_menu.rs b/crates/ui2/src/components/popover_menu.rs index 0f2fa6d23f147ba846695d1177ca5189f9130b3e..1a02dd526e464a7c9ef7483bf575afc21f24c753 100644 --- a/crates/ui2/src/components/popover_menu.rs +++ b/crates/ui2/src/components/popover_menu.rs @@ -18,19 +18,19 @@ pub struct PopoverMenu { Box< dyn FnOnce( Rc>>>, - Option View + 'static>>, + Option Option> + 'static>>, ) -> AnyElement + 'static, >, >, - menu_builder: Option View + 'static>>, + menu_builder: Option Option> + 'static>>, anchor: AnchorCorner, attach: Option, offset: Option>, } impl PopoverMenu { - pub fn menu(mut self, f: impl Fn(&mut WindowContext) -> View + 'static) -> Self { + pub fn menu(mut self, f: impl Fn(&mut WindowContext) -> Option> + 'static) -> Self { self.menu_builder = Some(Rc::new(f)); self } @@ -42,7 +42,9 @@ impl PopoverMenu { .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();