Fix dismissing context menu when clicking on an item dispatched action (#3779)

Antonio Scandurra created

Release Notes:

- N/A

Change summary

crates/ui2/src/components/context_menu.rs |  29 ++++-
crates/workspace2/src/pane.rs             | 140 ++++++++++++------------
2 files changed, 94 insertions(+), 75 deletions(-)

Detailed changes

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

@@ -29,6 +29,7 @@ pub struct ContextMenu {
     focus_handle: FocusHandle,
     selected_index: Option<usize>,
     delayed: bool,
+    clicked: bool,
     _on_blur_subscription: Subscription,
 }
 
@@ -56,6 +57,7 @@ impl ContextMenu {
                     focus_handle,
                     selected_index: None,
                     delayed: false,
+                    clicked: false,
                     _on_blur_subscription,
                 },
                 cx,
@@ -187,6 +189,11 @@ impl ContextMenu {
     }
 
     pub fn on_action_dispatch(&mut self, dispatched: &Box<dyn Action>, cx: &mut ViewContext<Self>) {
+        if self.clicked {
+            cx.propagate();
+            return;
+        }
+
         if let Some(ix) = self.items.iter().position(|item| {
             if let ContextMenuItem::Entry {
                 action: Some(action),
@@ -269,6 +276,7 @@ impl Render for ContextMenu {
                             action,
                         } => {
                             let handler = handler.clone();
+                            let menu = cx.view().downgrade();
 
                             let label_element = if let Some(icon) = icon {
                                 h_stack()
@@ -283,10 +291,14 @@ impl Render for ContextMenu {
                             ListItem::new(ix)
                                 .inset(true)
                                 .selected(Some(ix) == self.selected_index)
-                                .on_click(cx.listener(move |_, _, cx| {
+                                .on_click(move |_, cx| {
                                     handler(cx);
-                                    cx.emit(DismissEvent);
-                                }))
+                                    menu.update(cx, |menu, cx| {
+                                        menu.clicked = true;
+                                        cx.emit(DismissEvent);
+                                    })
+                                    .ok();
+                                })
                                 .child(
                                     h_stack()
                                         .w_full()
@@ -304,13 +316,18 @@ impl Render for ContextMenu {
                             handler,
                         } => {
                             let handler = handler.clone();
+                            let menu = cx.view().downgrade();
                             ListItem::new(ix)
                                 .inset(true)
                                 .selected(Some(ix) == self.selected_index)
-                                .on_click(cx.listener(move |_, _, cx| {
+                                .on_click(move |_, cx| {
                                     handler(cx);
-                                    cx.emit(DismissEvent);
-                                }))
+                                    menu.update(cx, |menu, cx| {
+                                        menu.clicked = true;
+                                        cx.emit(DismissEvent);
+                                    })
+                                    .ok();
+                                })
                                 .child(entry_render(cx))
                                 .into_any_element()
                         }

crates/workspace2/src/pane.rs 🔗

@@ -1571,78 +1571,80 @@ impl Pane {
             }
         };
 
-        let pane = cx.view().clone();
+        let pane = cx.view().downgrade();
         right_click_menu(ix).trigger(tab).menu(move |cx| {
             let pane = pane.clone();
-            ContextMenu::build(cx, move |menu, cx| {
-                let menu = menu
-                    .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()
-                    .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()
-                    .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",
-                        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 {
-                    let entry_id = entry.to_proto();
-                    menu.separator().entry(
-                        "Reveal In Project Panel",
-                        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
+            ContextMenu::build(cx, move |mut menu, cx| {
+                if let Some(pane) = pane.upgrade() {
+                    menu = menu
+                        .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()
+                        .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()
+                        .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",
+                            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 {
+                        let entry_id = entry.to_proto();
+                        menu = menu.separator().entry(
+                            "Reveal In Project Panel",
+                            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),
+                                    ))
+                                });
+                            }),
+                        );
+                    }
                 }
+
+                menu
             })
         })
     }