From b6bf2d8c5ec8548a091bb4ef99799dad305e10fa Mon Sep 17 00:00:00 2001
From: Andreas Johansson <z@zanmato.se>
Date: Fri, 21 Nov 2025 03:27:21 +0100
Subject: [PATCH] menu: Use defer to avoid race conditions with click listeners
 (#1651)

When using `.context_menu` on a parent element, the
`window.on_mouse_event` would fire before, for example, the tables
`on_mouse_down` so it'd be like this:
1. First right click = no context menu, right_clicked_row is set
2. Second right click = context menu, but with the row from the first
click

Before: (Look at `Selected row`)


https://github.com/user-attachments/assets/5eb89a00-f1ce-4423-b8d7-3ec61b848b83

After:


https://github.com/user-attachments/assets/10033ca4-4c2c-47b5-ace5-6ba745fcfee5

---------

Co-authored-by: Jason Lee <huacnlee@gmail.com>
---
 crates/ui/src/menu/context_menu.rs | 53 ++++++++++++++++++++----------
 1 file changed, 35 insertions(+), 18 deletions(-)

diff --git a/crates/ui/src/menu/context_menu.rs b/crates/ui/src/menu/context_menu.rs
index 111dfcad..ea08b8cd 100644
--- a/crates/ui/src/menu/context_menu.rs
+++ b/crates/ui/src/menu/context_menu.rs
@@ -29,7 +29,7 @@ impl<E: ParentElement + Styled> ContextMenuExt for E {}
 pub struct ContextMenu<E: ParentElement + Styled + Sized> {
     id: ElementId,
     element: Option<E>,
-    menu: Option<Box<dyn Fn(PopupMenu, &mut Window, &mut Context<PopupMenu>) -> PopupMenu>>,
+    menu: Option<Rc<dyn Fn(PopupMenu, &mut Window, &mut Context<PopupMenu>) -> PopupMenu>>,
     // This is not in use, just for style refinement forwarding.
     _ignore_style: StyleRefinement,
     anchor: Corner,
@@ -53,7 +53,7 @@ impl<E: ParentElement + Styled> ContextMenu<E> {
     where
         F: Fn(PopupMenu, &mut Window, &mut Context<PopupMenu>) -> PopupMenu + 'static,
     {
-        self.menu = Some(Box::new(builder));
+        self.menu = Some(Rc::new(builder));
         self
     }
 
@@ -235,9 +235,8 @@ impl<E: ParentElement + Styled + IntoElement + 'static> Element for ContextMenu<
             element.paint(window, cx);
         }
 
-        let Some(builder) = self.menu.take() else {
-            return;
-        };
+        // Take the builder before setting up element state to avoid borrow issues
+        let builder = self.menu.clone();
 
         self.with_element_state(
             id.unwrap(),
@@ -254,26 +253,44 @@ impl<E: ParentElement + Styled + IntoElement + 'static> Element for ContextMenu<
                     {
                         {
                             let mut shared_state = shared_state.borrow_mut();
+                            // Clear any existing menu view to allow immediate replacement
+                            // Set the new position and open the menu
+                            shared_state.menu_view = None;
+                            shared_state._subscription = None;
                             shared_state.position = event.position;
                             shared_state.open = true;
                         }
 
-                        let menu = PopupMenu::build(window, cx, |menu, window, cx| {
-                            (builder)(menu, window, cx)
-                        })
-                        .into_element();
-
-                        let _subscription = window.subscribe(&menu, cx, {
+                        // Use defer to build the menu in the next frame, avoiding race conditions
+                        window.defer(cx, {
                             let shared_state = shared_state.clone();
-                            move |_, _: &DismissEvent, window, _| {
-                                shared_state.borrow_mut().open = false;
-                                window.refresh();
+                            let builder = builder.clone();
+                            move |window, cx| {
+                                let menu = PopupMenu::build(window, cx, move |menu, window, cx| {
+                                    let Some(build) = &builder else {
+                                        return menu;
+                                    };
+                                    build(menu, window, cx)
+                                });
+
+                                // Set up the subscription for dismiss handling
+                                let _subscription = window.subscribe(&menu, cx, {
+                                    let shared_state = shared_state.clone();
+                                    move |_, _: &DismissEvent, window, _cx| {
+                                        shared_state.borrow_mut().open = false;
+                                        window.refresh();
+                                    }
+                                });
+
+                                // Update the shared state with the built menu and subscription
+                                {
+                                    let mut state = shared_state.borrow_mut();
+                                    state.menu_view = Some(menu.clone());
+                                    state._subscription = Some(_subscription);
+                                    window.refresh();
+                                }
                             }
                         });
-
-                        shared_state.borrow_mut().menu_view = Some(menu.clone());
-                        shared_state.borrow_mut()._subscription = Some(_subscription);
-                        window.refresh();
                     }
                 });
             },