Revert "fix: handle context menu on mouse release"

This reverts commit fd0b64e.

Author: feigeCode

crates/db_view/src/db_tree_view.rs

@@ -2744,11 +2744,9 @@ impl DbTreeView {
         let view_for_click = view.clone();
         let view_for_double_click = view.clone();
         let view_for_context = view.clone();
-        let view_for_context_up = view.clone();
         let node_id_for_click = node_id.clone();
         let node_id_for_double_click = node_id.clone();
         let node_id_for_context = node_id.clone();
-        let node_id_for_context_up = node_id.clone();
         let view_for_filter = view.clone();
         let node_id_for_filter = node_id.clone();
 
@@ -2802,12 +2800,6 @@ impl DbTreeView {
                     cx.notify();
                 });
             })
-            .on_mouse_up(MouseButton::Right, move |_event, _window, cx| {
-                view_for_context_up.update(cx, |this, cx| {
-                    this.set_context_menu_target(&node_id_for_context_up, ix);
-                    cx.notify();
-                });
-            })
             // 内容
             .child(
                 h_flex()

crates/mongodb_view/src/mongo_tree_view.rs

@@ -1252,9 +1252,7 @@ impl MongoTreeView {
         let view_for_double_click = cx.entity().clone();
         let node_id_for_double_click = node_id.clone();
         let view_for_context = cx.entity().clone();
-        let view_for_context_up = cx.entity().clone();
         let node_id_for_context = node_id.clone();
-        let node_id_for_context_up = node_id.clone();
         let view_for_arrow = cx.entity().clone();
         let node_id_for_arrow = node_id.clone();
 
@@ -1286,12 +1284,6 @@ impl MongoTreeView {
                     cx.notify();
                 });
             })
-            .on_mouse_up(MouseButton::Right, move |_event, _window, cx| {
-                view_for_context_up.update(cx, |view, cx| {
-                    view.set_context_menu_target(&node_id_for_context_up);
-                    cx.notify();
-                });
-            })
             .child(div().w(indent))
             .child(
                 div()

crates/one_ui/src/edit_table/state.rs

@@ -1030,10 +1030,6 @@ where
         _: &mut Window,
         _: &mut Context<Self>,
     ) {
-        self.set_row_context_menu_target(row_ix);
-    }
-
-    fn set_row_context_menu_target(&mut self, row_ix: usize) {
         self.right_clicked_row = Some(row_ix);
     }
 
@@ -2826,12 +2822,6 @@ where
                         this.on_row_right_click(e, row_ix, window, cx);
                     }),
                 )
-                .on_mouse_up(
-                    MouseButton::Right,
-                    cx.listener(move |this, _, _, _| {
-                        this.set_row_context_menu_target(row_ix);
-                    }),
-                )
         } else {
             self.delegate
                 .render_tr(row_ix, window, cx)

crates/redis_view/src/redis_tree_view.rs

@@ -1975,10 +1975,8 @@ impl RedisTreeView {
         let view = cx.entity().clone();
         let view_for_db_switch = cx.entity().clone();
         let view_for_context = cx.entity().clone();
-        let view_for_context_up = cx.entity().clone();
         let view_for_dbl = cx.entity().clone();
         let node_id_for_context = node_id.clone();
-        let node_id_for_context_up = node_id.clone();
         let node_id_for_dbl = node_id.clone();
         let connection_id_for_load_more = node_connection_id.clone();
 
@@ -2044,12 +2042,6 @@ impl RedisTreeView {
                     cx.notify();
                 });
             })
-            .on_mouse_up(MouseButton::Right, move |_event, _window, cx| {
-                view_for_context_up.update(cx, |view, cx| {
-                    view.set_context_menu_target(&node_id_for_context_up);
-                    cx.notify();
-                });
-            })
             // 展开/折叠箭头
             .child(
                 div()

crates/ui/src/input/popovers/context_menu.rs

@@ -1,7 +1,7 @@
 use gpui::{
-    Anchor, App, AppContext as _, Context, DismissEvent, Entity, IntoElement, ParentElement as _,
-    Pixels, Point, Render, Styled, Subscription, Window, anchored, deferred, div,
-    prelude::FluentBuilder as _, px,
+    Anchor, App, AppContext as _, Context, DismissEvent, Entity, IntoElement, MouseDownEvent,
+    ParentElement as _, Pixels, Point, Render, Styled, Subscription, Window, anchored, deferred,
+    div, prelude::FluentBuilder as _, px,
 };
 use rust_i18n::t;
 
@@ -98,7 +98,7 @@ impl InputState {
 
     pub(crate) fn handle_right_click_menu(
         &mut self,
-        position: Point<Pixels>,
+        event: &MouseDownEvent,
         offset: usize,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -130,7 +130,7 @@ impl InputState {
 
         let action_context = self.focus_handle.clone();
         self.context_menu.update(cx, |this, cx| {
-            this.mouse_position = position;
+            this.mouse_position = event.position;
             this.menu.update(cx, |menu, cx| {
                 let new_menu = if let Some(builder) = &self.context_menu_builder {
                     builder(PopupMenu::new(cx), window, cx)

crates/ui/src/input/state.rs

@@ -1577,7 +1577,7 @@ impl InputState {
         // Show Mouse context menu
         if event.button == MouseButton::Right {
             if self.enable_context_menu || self.context_menu_builder.is_some() {
-                self.handle_right_click_menu(event.position, offset, window, cx);
+                self.handle_right_click_menu(event, offset, window, cx);
             }
             return;
         }
@@ -1591,18 +1591,10 @@ impl InputState {
 
     pub(super) fn on_mouse_up(
         &mut self,
-        event: &MouseUpEvent,
-        window: &mut Window,
-        cx: &mut Context<Self>,
+        _: &MouseUpEvent,
+        _window: &mut Window,
+        _cx: &mut Context<Self>,
     ) {
-        if event.button == MouseButton::Right && self.context_menu_content.is_none() {
-            if self.enable_context_menu || self.context_menu_builder.is_some() {
-                let offset = self.index_for_mouse_position(event.position);
-                self.handle_right_click_menu(event.position, offset, window, cx);
-            }
-            return;
-        }
-
         if self.selected_range.is_empty() {
             self.selection_reversed = false;
         }

crates/ui/src/menu/context_menu.rs

@@ -3,14 +3,12 @@ use std::{cell::RefCell, rc::Rc};
 use gpui::{
     Anchor, AnyElement, App, Context, DismissEvent, Element, ElementId, Entity, Focusable,
     GlobalElementId, Hitbox, HitboxBehavior, InspectorElementId, InteractiveElement, IntoElement,
-    MouseButton, MouseDownEvent, MouseUpEvent, ParentElement, Pixels, Point, StyleRefinement,
-    Styled, Subscription, Window, anchored, deferred, div, prelude::FluentBuilder, px,
+    MouseButton, MouseDownEvent, ParentElement, Pixels, Point, StyleRefinement, Styled,
+    Subscription, Window, anchored, deferred, div, prelude::FluentBuilder, px,
 };
 
 use crate::menu::PopupMenu;
 
-type PopupMenuBuilder = Rc<dyn Fn(PopupMenu, &mut Window, &mut Context<PopupMenu>) -> PopupMenu>;
-
 /// A extension trait for adding a context menu to an element.
 pub trait ContextMenuExt: InteractiveElement + ParentElement + Styled {
     /// Add a context menu to the element.
@@ -42,7 +40,7 @@ impl<E: InteractiveElement + ParentElement + Styled> ContextMenuExt for E {}
 pub struct ContextMenu<E: ParentElement + Styled + Sized> {
     id: ElementId,
     element: Option<E>,
-    menu: Option<PopupMenuBuilder>,
+    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: Anchor,
@@ -121,57 +119,6 @@ struct ContextMenuSharedState {
     _subscription: Option<Subscription>,
 }
 
-fn should_open_context_menu(state: &ContextMenuSharedState, position: Point<Pixels>) -> bool {
-    !state.open || state.position != position
-}
-
-fn mark_context_menu_open(state: &mut ContextMenuSharedState, position: Point<Pixels>) {
-    state.menu_view = None;
-    state._subscription = None;
-    state.position = position;
-    state.open = true;
-}
-
-fn open_context_menu_at(
-    shared_state: Rc<RefCell<ContextMenuSharedState>>,
-    builder: Option<PopupMenuBuilder>,
-    position: Point<Pixels>,
-    window: &mut Window,
-    cx: &mut App,
-) {
-    {
-        let mut state = shared_state.borrow_mut();
-        if !should_open_context_menu(&state, position) {
-            return;
-        }
-        mark_context_menu_open(&mut state, position);
-    }
-
-    window.defer(cx, move |window, cx| {
-        let menu = PopupMenu::build(window, cx, move |menu, window, cx| {
-            let Some(build) = &builder else {
-                return menu;
-            };
-            build(menu, window, cx)
-        });
-
-        let _subscription = window.subscribe(&menu, cx, {
-            let shared_state = shared_state.clone();
-            move |_, _: &DismissEvent, window, _cx| {
-                shared_state.borrow_mut().open = false;
-                window.refresh();
-            }
-        });
-
-        {
-            let mut state = shared_state.borrow_mut();
-            state.menu_view = Some(menu.clone());
-            state._subscription = Some(_subscription);
-            window.refresh();
-        }
-    });
-}
-
 pub struct ContextMenuState {
     element: Option<AnyElement>,
     shared_state: Rc<RefCell<ContextMenuSharedState>>,
@@ -323,86 +270,56 @@ impl<E: ParentElement + Styled + IntoElement + 'static> Element for ContextMenu<
             |_view, state: &mut ContextMenuState, window, _| {
                 let shared_state = state.shared_state.clone();
 
-                let hitbox_for_down = hitbox.clone();
-                let state_for_down = shared_state.clone();
-                let builder_for_down = builder.clone();
-                // Build the context menu from mouse down on platforms that emit it.
+                let hitbox = hitbox.clone();
+                // When right mouse click, to build content menu, and show it at the mouse position.
                 window.on_mouse_event(move |event: &MouseDownEvent, phase, window, cx| {
                     if phase.bubble()
                         && event.button == MouseButton::Right
-                        && hitbox_for_down.is_hovered(window)
+                        && hitbox.is_hovered(window)
                     {
-                        open_context_menu_at(
-                            state_for_down.clone(),
-                            builder_for_down.clone(),
-                            event.position,
-                            window,
-                            cx,
-                        );
-                    }
-                });
-
-                let hitbox_for_up = hitbox.clone();
-                let state_for_up = shared_state.clone();
-                let builder_for_up = builder.clone();
-                // Some platforms, including the OHOS backend, surface context-menu
-                // clicks on button release instead of button press.
-                window.on_mouse_event(move |event: &MouseUpEvent, phase, window, cx| {
-                    if phase.bubble()
-                        && event.button == MouseButton::Right
-                        && hitbox_for_up.is_hovered(window)
-                    {
-                        open_context_menu_at(
-                            state_for_up.clone(),
-                            builder_for_up.clone(),
-                            event.position,
-                            window,
-                            cx,
-                        );
+                        {
+                            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;
+                        }
+
+                        // Use defer to build the menu in the next frame, avoiding race conditions
+                        window.defer(cx, {
+                            let shared_state = shared_state.clone();
+                            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();
+                                }
+                            }
+                        });
                     }
                 });
             },
         );
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-    use gpui::px;
-
-    fn test_state(position: Point<Pixels>, open: bool) -> ContextMenuSharedState {
-        ContextMenuSharedState {
-            menu_view: None,
-            open,
-            position,
-            _subscription: None,
-        }
-    }
-
-    fn point(x: f32, y: f32) -> Point<Pixels> {
-        Point { x: px(x), y: px(y) }
-    }
-
-    #[test]
-    fn context_menu_suppresses_same_click_release_after_press() {
-        let position = point(10.0, 20.0);
-        let mut state = test_state(position, false);
-
-        assert!(should_open_context_menu(&state, position));
-        mark_context_menu_open(&mut state, position);
-
-        assert!(!should_open_context_menu(&state, position));
-    }
-
-    #[test]
-    fn context_menu_allows_release_only_or_new_position() {
-        let position = point(10.0, 20.0);
-        let new_position = point(20.0, 20.0);
-        let open_state = test_state(position, true);
-        let closed_state = test_state(position, false);
-
-        assert!(should_open_context_menu(&closed_state, position));
-        assert!(should_open_context_menu(&open_state, new_position));
-    }
-}