Fix context menu in tab bar (#3776)

Antonio Scandurra created

Release Notes:

- N/A

Change summary

crates/collab_ui2/src/collab_panel.rs               | 11 ++
crates/collab_ui2/src/collab_panel/channel_modal.rs |  6 
crates/copilot_button2/src/copilot_button.rs        | 10 +
crates/language_tools2/src/lsp_log.rs               |  1 
crates/language_tools2/src/syntax_tree_view.rs      |  1 
crates/project_panel2/src/project_panel.rs          |  1 
crates/ui2/src/components/context_menu.rs           | 13 ++
crates/ui2/src/components/stories/context_menu.rs   | 11 -
crates/workspace2/src/dock.rs                       |  2 
crates/workspace2/src/pane.rs                       | 74 ++++++++++++--
10 files changed, 100 insertions(+), 30 deletions(-)

Detailed changes

crates/collab_ui2/src/collab_panel.rs 🔗

@@ -993,6 +993,7 @@ impl CollabPanel {
                 };
                 context_menu = context_menu.entry(
                     expand_action_name,
+                    None,
                     cx.handler_for(&this, move |this, cx| {
                         this.toggle_channel_collapsed(channel_id, cx)
                     }),
@@ -1002,18 +1003,21 @@ impl CollabPanel {
             context_menu = context_menu
                 .entry(
                     "Open Notes",
+                    None,
                     cx.handler_for(&this, move |this, cx| {
                         this.open_channel_notes(channel_id, cx)
                     }),
                 )
                 .entry(
                     "Open Chat",
+                    None,
                     cx.handler_for(&this, move |this, cx| {
                         this.join_channel_chat(channel_id, cx)
                     }),
                 )
                 .entry(
                     "Copy Channel Link",
+                    None,
                     cx.handler_for(&this, move |this, cx| {
                         this.copy_channel_link(channel_id, cx)
                     }),
@@ -1024,14 +1028,17 @@ impl CollabPanel {
                     .separator()
                     .entry(
                         "New Subchannel",
+                        None,
                         cx.handler_for(&this, move |this, cx| this.new_subchannel(channel_id, cx)),
                     )
                     .entry(
                         "Rename",
+                        None,
                         cx.handler_for(&this, move |this, cx| this.rename_channel(channel_id, cx)),
                     )
                     .entry(
                         "Move this channel",
+                        None,
                         cx.handler_for(&this, move |this, cx| {
                             this.start_move_channel(channel_id, cx)
                         }),
@@ -1040,6 +1047,7 @@ impl CollabPanel {
                 if let Some(channel_name) = clipboard_channel_name {
                     context_menu = context_menu.separator().entry(
                         format!("Move '#{}' here", channel_name),
+                        None,
                         cx.handler_for(&this, move |this, cx| {
                             this.move_channel_on_clipboard(channel_id, cx)
                         }),
@@ -1050,14 +1058,17 @@ impl CollabPanel {
                     .separator()
                     .entry(
                         "Invite Members",
+                        None,
                         cx.handler_for(&this, move |this, cx| this.invite_members(channel_id, cx)),
                     )
                     .entry(
                         "Manage Members",
+                        None,
                         cx.handler_for(&this, move |this, cx| this.manage_members(channel_id, cx)),
                     )
                     .entry(
                         "Delete",
+                        None,
                         cx.handler_for(&this, move |this, cx| this.remove_channel(channel_id, cx)),
                     );
             }

crates/collab_ui2/src/collab_panel/channel_modal.rs 🔗

@@ -532,7 +532,7 @@ impl ChannelModalDelegate {
         let user_id = user.id;
         let picker = cx.view().clone();
         let context_menu = ContextMenu::build(cx, |mut menu, _cx| {
-            menu = menu.entry("Remove Member", {
+            menu = menu.entry("Remove Member", None, {
                 let picker = picker.clone();
                 move |cx| {
                     picker.update(cx, |picker, cx| {
@@ -544,7 +544,7 @@ impl ChannelModalDelegate {
             let picker = picker.clone();
             match role {
                 ChannelRole::Admin => {
-                    menu = menu.entry("Revoke Admin", move |cx| {
+                    menu = menu.entry("Revoke Admin", None, move |cx| {
                         picker.update(cx, |picker, cx| {
                             picker
                                 .delegate
@@ -553,7 +553,7 @@ impl ChannelModalDelegate {
                     });
                 }
                 ChannelRole::Member => {
-                    menu = menu.entry("Make Admin", move |cx| {
+                    menu = menu.entry("Make Admin", None, move |cx| {
                         picker.update(cx, |picker, cx| {
                             picker
                                 .delegate

crates/copilot_button2/src/copilot_button.rs 🔗

@@ -133,8 +133,11 @@ impl CopilotButton {
     pub fn build_copilot_start_menu(&mut self, cx: &mut ViewContext<Self>) -> View<ContextMenu> {
         let fs = self.fs.clone();
         ContextMenu::build(cx, |menu, cx| {
-            menu.entry("Sign In", initiate_sign_in)
-                .entry("Disable Copilot", move |cx| hide_copilot(fs.clone(), cx))
+            menu.entry("Sign In", None, initiate_sign_in).entry(
+                "Disable Copilot",
+                None,
+                move |cx| hide_copilot(fs.clone(), cx),
+            )
         })
     }
 
@@ -154,6 +157,7 @@ impl CopilotButton {
                         if language_enabled { "Hide" } else { "Show" },
                         language.name()
                     ),
+                    None,
                     move |cx| toggle_copilot_for_language(language.clone(), fs.clone(), cx),
                 );
             }
@@ -169,6 +173,7 @@ impl CopilotButton {
                         "{} Suggestions for This Path",
                         if path_enabled { "Hide" } else { "Show" }
                     ),
+                    None,
                     move |cx| {
                         if let Some(workspace) = cx.window_handle().downcast::<Workspace>() {
                             if let Ok(workspace) = workspace.root_view(cx) {
@@ -194,6 +199,7 @@ impl CopilotButton {
                 } else {
                     "Show Suggestions for All Files"
                 },
+                None,
                 move |cx| toggle_copilot_globally(fs.clone(), cx),
             )
             .separator()

crates/language_tools2/src/lsp_log.rs 🔗

@@ -763,6 +763,7 @@ impl Render for LspLogToolbarItemView {
                             ))
                             .entry(
                                 SERVER_LOGS,
+                                None,
                                 cx.handler_for(&log_view, move |view, cx| {
                                     view.show_logs_for_server(row.server_id, cx);
                                 }),

crates/project_panel2/src/project_panel.rs 🔗

@@ -400,6 +400,7 @@ impl ProjectPanel {
                     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)

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

@@ -76,13 +76,14 @@ impl ContextMenu {
     pub fn entry(
         mut self,
         label: impl Into<SharedString>,
+        action: Option<Box<dyn Action>>,
         handler: impl Fn(&mut WindowContext) + 'static,
     ) -> Self {
         self.items.push(ContextMenuItem::Entry {
             label: label.into(),
             handler: Rc::new(handler),
             icon: None,
-            action: None,
+            action,
         });
         self
     }
@@ -282,7 +283,10 @@ impl Render for ContextMenu {
                             ListItem::new(ix)
                                 .inset(true)
                                 .selected(Some(ix) == self.selected_index)
-                                .on_click(move |_, cx| handler(cx))
+                                .on_click(cx.listener(move |_, _, cx| {
+                                    handler(cx);
+                                    cx.emit(DismissEvent);
+                                }))
                                 .child(
                                     h_stack()
                                         .w_full()
@@ -303,7 +307,10 @@ impl Render for ContextMenu {
                             ListItem::new(ix)
                                 .inset(true)
                                 .selected(Some(ix) == self.selected_index)
-                                .on_click(move |_, cx| handler(cx))
+                                .on_click(cx.listener(move |_, _, cx| {
+                                    handler(cx);
+                                    cx.emit(DismissEvent);
+                                }))
                                 .child(entry_render(cx))
                                 .into_any_element()
                         }

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

@@ -1,4 +1,4 @@
-use gpui::{actions, Action, AnchorCorner, Div, Render, View};
+use gpui::{actions, AnchorCorner, Div, Render, View};
 use story::Story;
 
 use crate::prelude::*;
@@ -10,12 +10,9 @@ fn build_menu(cx: &mut WindowContext, header: impl Into<SharedString>) -> View<C
     ContextMenu::build(cx, |menu, _| {
         menu.header(header)
             .separator()
-            .entry("Print current time", |cx| {
-                println!("dispatching PrintCurrentTime action");
-                cx.dispatch_action(PrintCurrentDate.boxed_clone())
-            })
-            .entry("Print best foot", |cx| {
-                cx.dispatch_action(PrintBestFood.boxed_clone())
+            .action("Print current time", Box::new(PrintCurrentDate))
+            .entry("Print best food", Some(Box::new(PrintBestFood)), |cx| {
+                cx.dispatch_action(Box::new(PrintBestFood))
             })
     })
 }

crates/workspace2/src/dock.rs 🔗

@@ -625,7 +625,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(), None, move |cx| {
                                             panel.set_position(position, cx);
                                         })
                                     }

crates/workspace2/src/pane.rs 🔗

@@ -990,7 +990,7 @@ impl Pane {
         &mut self,
         cx: &mut ViewContext<Pane>,
         mut save_intent: SaveIntent,
-        should_close: impl 'static + Fn(EntityId) -> bool,
+        should_close: impl Fn(EntityId) -> bool,
     ) -> Task<Result<()>> {
         // Find the items to close.
         let mut items_to_close = Vec::new();
@@ -1571,28 +1571,74 @@ impl Pane {
             }
         };
 
+        let pane = cx.view().clone();
         right_click_menu(ix).trigger(tab).menu(move |cx| {
-            ContextMenu::build(cx, |menu, _| {
+            let pane = pane.clone();
+            ContextMenu::build(cx, move |menu, cx| {
                 let menu = menu
-                    .action("Close", CloseActiveItem { save_intent: None }.boxed_clone())
-                    .action("Close Others", CloseInactiveItems.boxed_clone())
+                    .entry(
+                        "Close",
+                        Some(Box::new(CloseActiveItem { save_intent: None })),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.close_item_by_id(item_id, SaveIntent::Close, cx)
+                                .detach_and_log_err(cx);
+                        }),
+                    )
+                    .entry(
+                        "Close Others",
+                        Some(Box::new(CloseInactiveItems)),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.close_items(cx, SaveIntent::Close, |id| id != item_id)
+                                .detach_and_log_err(cx);
+                        }),
+                    )
                     .separator()
-                    .action("Close Left", CloseItemsToTheLeft.boxed_clone())
-                    .action("Close Right", CloseItemsToTheRight.boxed_clone())
+                    .entry(
+                        "Close Left",
+                        Some(Box::new(CloseItemsToTheLeft)),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.close_items_to_the_left_by_id(item_id, cx)
+                                .detach_and_log_err(cx);
+                        }),
+                    )
+                    .entry(
+                        "Close Right",
+                        Some(Box::new(CloseItemsToTheRight)),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.close_items_to_the_right_by_id(item_id, cx)
+                                .detach_and_log_err(cx);
+                        }),
+                    )
                     .separator()
-                    .action("Close Clean", CloseCleanItems.boxed_clone())
-                    .action(
+                    .entry(
+                        "Close Clean",
+                        Some(Box::new(CloseCleanItems)),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.close_clean_items(&CloseCleanItems, cx)
+                                .map(|task| task.detach_and_log_err(cx));
+                        }),
+                    )
+                    .entry(
                         "Close All",
-                        CloseAllItems { save_intent: None }.boxed_clone(),
+                        Some(Box::new(CloseAllItems { save_intent: None })),
+                        cx.handler_for(&pane, |pane, cx| {
+                            pane.close_all_items(&CloseAllItems { save_intent: None }, cx)
+                                .map(|task| task.detach_and_log_err(cx));
+                        }),
                     );
 
                 if let Some(entry) = single_entry_to_resolve {
-                    menu.separator().action(
+                    let entry_id = entry.to_proto();
+                    menu.separator().entry(
                         "Reveal In Project Panel",
-                        RevealInProjectPanel {
-                            entry_id: entry.to_proto(),
-                        }
-                        .boxed_clone(),
+                        Some(Box::new(RevealInProjectPanel { entry_id })),
+                        cx.handler_for(&pane, move |pane, cx| {
+                            pane.project.update(cx, |_, cx| {
+                                cx.emit(project::Event::RevealInProjectPanel(
+                                    ProjectEntryId::from_proto(entry_id),
+                                ))
+                            });
+                        }),
                     )
                 } else {
                     menu