From 3a15384070827a94f595c96de82c246c70c125de Mon Sep 17 00:00:00 2001 From: Danilo Leal Date: Mon, 29 Dec 2025 11:40:25 -0300 Subject: [PATCH] Simplify and clean it up --- crates/ui/src/components/context_menu.rs | 235 ++++++++--------------- 1 file changed, 85 insertions(+), 150 deletions(-) diff --git a/crates/ui/src/components/context_menu.rs b/crates/ui/src/components/context_menu.rs index 2445a393f9b574dfc89669ca07f13db9bc2615c1..6feb825e1538ce652dada7eab6c3301f1aa1fc8f 100644 --- a/crates/ui/src/components/context_menu.rs +++ b/crates/ui/src/components/context_menu.rs @@ -10,8 +10,7 @@ use gpui::{ use menu::{SelectChild, SelectFirst, SelectLast, SelectNext, SelectParent, SelectPrevious}; use settings::Settings; use std::{ - cell::{Cell, RefCell}, - collections::HashMap, + cell::Cell, rc::Rc, time::{Duration, Instant}, }; @@ -26,11 +25,9 @@ enum SubmenuOpenTrigger { struct OpenSubmenu { item_index: usize, entity: Entity, - // Bounds of the trigger row that opened this submenu. trigger_bounds: Option>, // Capture the submenu's vertical offset once and keep it stable while the submenu is open. offset: Option, - _dismiss_subscription: Subscription, } @@ -39,37 +36,12 @@ enum SubmenuState { Open(OpenSubmenu), } -struct SubmenuHoverSafetyHeuristic { - last_mouse_position: Option>, - trigger_left_x: Option, -} - -impl SubmenuHoverSafetyHeuristic { - fn new() -> Self { - Self { - last_mouse_position: None, - trigger_left_x: None, - } - } - - fn clear(&mut self) { - self.last_mouse_position = None; - self.trigger_left_x = None; - } - - fn update_mouse_position(&mut self, position: Point) { - self.last_mouse_position = Some(position); - } - - fn update_trigger_left_x(&mut self, trigger_left_x: Pixels) { - self.trigger_left_x = Some(trigger_left_x); - } - - fn should_allow_close_from_parent_area(&self, mouse_position: Point) -> bool { - self.trigger_left_x - .map(|trigger_left_x| mouse_position.x < trigger_left_x) - .unwrap_or(true) - } +#[derive(Clone, Copy, PartialEq, Eq, Default)] +enum HoverTarget { + #[default] + None, + MainMenu, + Submenu, } pub enum ContextMenuItem { @@ -246,17 +218,14 @@ pub struct ContextMenu { documentation_aside: Option<(usize, DocumentationAside)>, fixed_width: Option, // Submenu-related fields + parent_menu: Option>, submenu_state: SubmenuState, - submenu_hover_safety_heuristic: SubmenuHoverSafetyHeuristic, + hover_target: HoverTarget, + submenu_safety_threshold_x: Option, submenu_observed_bounds: Rc>>>, - submenu_trigger_observed_bounds_by_item: Rc>>>, - is_submenu: bool, - submenu_hovered: bool, + submenu_trigger_bounds: Rc>>>, submenu_trigger_mouse_down: bool, - submenu_generation: u64, - ignore_blur_cancel_until: Option, - parent_menu: Option>, - menu_hovered: bool, + ignore_blur_until: Option, } #[derive(Copy, Clone, PartialEq, Eq)] @@ -310,15 +279,15 @@ impl ContextMenu { &focus_handle, window, |this: &mut ContextMenu, window, cx| { - if let Some(ignore_until) = this.ignore_blur_cancel_until { + if let Some(ignore_until) = this.ignore_blur_until { if Instant::now() < ignore_until { return; } else { - this.ignore_blur_cancel_until = None; + this.ignore_blur_until = None; } } - if !this.is_submenu { + if this.parent_menu.is_none() { if let SubmenuState::Open(open_submenu) = &this.submenu_state { let submenu_focus = open_submenu.entity.read(cx).focus_handle.clone(); if submenu_focus.contains_focused(window, cx) { @@ -347,17 +316,14 @@ impl ContextMenu { keep_open_on_confirm: false, documentation_aside: None, fixed_width: None, + parent_menu: None, submenu_state: SubmenuState::Closed, - submenu_hover_safety_heuristic: SubmenuHoverSafetyHeuristic::new(), + hover_target: HoverTarget::MainMenu, + submenu_safety_threshold_x: None, submenu_observed_bounds: Rc::new(Cell::new(None)), - submenu_trigger_observed_bounds_by_item: Rc::new(RefCell::new(HashMap::new())), - is_submenu: false, - submenu_hovered: false, + submenu_trigger_bounds: Rc::new(Cell::new(None)), submenu_trigger_mouse_down: false, - submenu_generation: 0, - ignore_blur_cancel_until: None, - parent_menu: None, - menu_hovered: true, + ignore_blur_until: None, }, window, cx, @@ -389,15 +355,15 @@ impl ContextMenu { &focus_handle, window, |this: &mut ContextMenu, window, cx| { - if let Some(ignore_until) = this.ignore_blur_cancel_until { + if let Some(ignore_until) = this.ignore_blur_until { if Instant::now() < ignore_until { return; } else { - this.ignore_blur_cancel_until = None; + this.ignore_blur_until = None; } } - if !this.is_submenu { + if this.parent_menu.is_none() { if let SubmenuState::Open(open_submenu) = &this.submenu_state { let submenu_focus = open_submenu.entity.read(cx).focus_handle.clone(); if submenu_focus.contains_focused(window, cx) { @@ -426,17 +392,14 @@ impl ContextMenu { keep_open_on_confirm: true, documentation_aside: None, fixed_width: None, + parent_menu: None, submenu_state: SubmenuState::Closed, - submenu_hover_safety_heuristic: SubmenuHoverSafetyHeuristic::new(), + hover_target: HoverTarget::MainMenu, + submenu_safety_threshold_x: None, submenu_observed_bounds: Rc::new(Cell::new(None)), - submenu_trigger_observed_bounds_by_item: Rc::new(RefCell::new(HashMap::new())), - is_submenu: false, - submenu_hovered: false, + submenu_trigger_bounds: Rc::new(Cell::new(None)), submenu_trigger_mouse_down: false, - submenu_generation: 0, - ignore_blur_cancel_until: None, - parent_menu: None, - menu_hovered: true, + ignore_blur_until: None, }, window, cx, @@ -473,15 +436,15 @@ impl ContextMenu { &focus_handle, window, |this: &mut ContextMenu, window, cx| { - if let Some(ignore_until) = this.ignore_blur_cancel_until { + if let Some(ignore_until) = this.ignore_blur_until { if Instant::now() < ignore_until { return; } else { - this.ignore_blur_cancel_until = None; + this.ignore_blur_until = None; } } - if !this.is_submenu { + if this.parent_menu.is_none() { if let SubmenuState::Open(open_submenu) = &this.submenu_state { let submenu_focus = open_submenu.entity.read(cx).focus_handle.clone(); @@ -497,17 +460,14 @@ impl ContextMenu { keep_open_on_confirm: false, documentation_aside: None, fixed_width: None, + parent_menu: None, submenu_state: SubmenuState::Closed, - submenu_hover_safety_heuristic: SubmenuHoverSafetyHeuristic::new(), + hover_target: HoverTarget::MainMenu, + submenu_safety_threshold_x: None, submenu_observed_bounds: Rc::new(Cell::new(None)), - submenu_trigger_observed_bounds_by_item: Rc::new(RefCell::new(HashMap::new())), - is_submenu: false, - submenu_hovered: false, + submenu_trigger_bounds: Rc::new(Cell::new(None)), submenu_trigger_mouse_down: false, - submenu_generation: 0, - ignore_blur_cancel_until: None, - parent_menu: None, - menu_hovered: true, + ignore_blur_until: None, }, window, cx, @@ -877,7 +837,7 @@ impl ContextMenu { (handler)(context, window, cx) } - if self.is_submenu && !self.keep_open_on_confirm { + if self.parent_menu.is_some() && !self.keep_open_on_confirm { self.clicked = true; } @@ -889,7 +849,7 @@ impl ContextMenu { } pub fn cancel(&mut self, _: &menu::Cancel, window: &mut Window, cx: &mut Context) { - if self.is_submenu { + if self.parent_menu.is_some() { cx.emit(DismissEvent); // Restore keyboard focus to the parent menu so arrow keys / Escape / Enter work again. @@ -897,8 +857,7 @@ impl ContextMenu { let parent_focus = parent.read(cx).focus_handle.clone(); parent.update(cx, |parent, _cx| { - parent.ignore_blur_cancel_until = - Some(Instant::now() + Duration::from_millis(200)); + parent.ignore_blur_until = Some(Instant::now() + Duration::from_millis(200)); }); window.focus(&parent_focus, cx); @@ -1027,7 +986,7 @@ impl ContextMenu { window: &mut Window, cx: &mut Context, ) { - if !self.is_submenu { + if self.parent_menu.is_none() { return; } @@ -1133,17 +1092,14 @@ impl ContextMenu { keep_open_on_confirm: false, documentation_aside: None, fixed_width: None, + parent_menu: Some(parent_entity), submenu_state: SubmenuState::Closed, - submenu_hover_safety_heuristic: SubmenuHoverSafetyHeuristic::new(), + hover_target: HoverTarget::MainMenu, + submenu_safety_threshold_x: None, submenu_observed_bounds: Rc::new(Cell::new(None)), - submenu_trigger_observed_bounds_by_item: Rc::new(RefCell::new(HashMap::new())), - is_submenu: true, - submenu_hovered: false, + submenu_trigger_bounds: Rc::new(Cell::new(None)), submenu_trigger_mouse_down: false, - submenu_generation: 0, - ignore_blur_cancel_until: None, - parent_menu: Some(parent_entity), - menu_hovered: true, + ignore_blur_until: None, }; menu = (builder)(menu, window, cx); @@ -1152,14 +1108,11 @@ impl ContextMenu { } fn close_submenu(&mut self, clear_selection: bool, cx: &mut Context) { - self.submenu_generation = self.submenu_generation.wrapping_add(1); self.submenu_state = SubmenuState::Closed; - self.submenu_hovered = false; - self.submenu_hover_safety_heuristic.clear(); + self.hover_target = HoverTarget::MainMenu; + self.submenu_safety_threshold_x = None; self.submenu_observed_bounds.set(None); - self.submenu_trigger_observed_bounds_by_item - .borrow_mut() - .clear(); + self.submenu_trigger_bounds.set(None); if clear_selection { self.selected_index = None; @@ -1191,27 +1144,19 @@ impl ContextMenu { // offset so we don't reuse a stale position. if matches!(reason, SubmenuOpenTrigger::Keyboard) { self.submenu_observed_bounds.set(None); - self.submenu_trigger_observed_bounds_by_item - .borrow_mut() - .clear(); + self.submenu_trigger_bounds.set(None); } - self.submenu_hover_safety_heuristic.clear(); - self.submenu_hovered = false; + self.submenu_safety_threshold_x = None; + self.hover_target = HoverTarget::MainMenu; // When opening a submenu via keyboard, there is a brief moment where focus/hover can // transition in a way that triggers the parent menu's `on_blur` dismissal. if matches!(reason, SubmenuOpenTrigger::Keyboard) { - self.ignore_blur_cancel_until = Some(Instant::now() + Duration::from_millis(150)); + self.ignore_blur_until = Some(Instant::now() + Duration::from_millis(150)); } - let _ = reason; - - let trigger_bounds = self - .submenu_trigger_observed_bounds_by_item - .borrow() - .get(&item_index) - .copied(); + let trigger_bounds = self.submenu_trigger_bounds.get(); self.submenu_state = SubmenuState::Open(OpenSubmenu { item_index, @@ -1224,11 +1169,6 @@ impl ContextMenu { cx.notify(); } - fn update_last_mouse_position(&mut self, position: Point) { - self.submenu_hover_safety_heuristic - .update_mouse_position(position); - } - pub fn on_action_dispatch( &mut self, dispatched: &dyn Action, @@ -1403,13 +1343,10 @@ impl ContextMenu { } })) .on_mouse_move(cx.listener(move |this, event: &MouseMoveEvent, _, cx| { - this.update_last_mouse_position(event.position); - if matches!(&this.submenu_state, SubmenuState::Open(_)) || this.selected_index == Some(ix) { - this.submenu_hover_safety_heuristic - .update_trigger_left_x(event.position.x - px(100.0)); + this.submenu_safety_threshold_x = Some(event.position.x - px(100.0)); } cx.notify(); @@ -1421,11 +1358,10 @@ impl ContextMenu { .child( canvas( { - let bounds_by_item = - self.submenu_trigger_observed_bounds_by_item.clone(); + let trigger_bounds_cell = self.submenu_trigger_bounds.clone(); move |bounds, _window, _cx| { if toggle_state { - bounds_by_item.borrow_mut().insert(ix, bounds); + trigger_bounds_cell.set(Some(bounds)); } } }, @@ -1442,10 +1378,8 @@ impl ContextMenu { if *hovered { this.clear_selected(); window.focus(&this.focus_handle.clone(), cx); - this.menu_hovered = true; - this.submenu_hovered = false; - this.submenu_hover_safety_heuristic - .update_trigger_left_x(mouse_pos.x - px(50.0)); + this.hover_target = HoverTarget::MainMenu; + this.submenu_safety_threshold_x = Some(mouse_pos.x - px(50.0)); if let Some(ContextMenuItem::Submenu { builder, .. }) = this.items.get(ix) @@ -1470,7 +1404,7 @@ impl ContextMenu { SubmenuState::Open(open_submenu) if open_submenu.item_index == ix ); - if is_open_for_this_item && !this.submenu_hovered { + if is_open_for_this_item && this.hover_target != HoverTarget::Submenu { this.close_submenu(false, cx); this.clear_selected(); cx.notify(); @@ -1557,8 +1491,10 @@ impl ContextMenu { div() .id(("submenu-container", ix)) - .on_hover(cx.listener(|this, _, _, _| { - this.submenu_hovered = true; + .on_hover(cx.listener(|this, hovered, _, _| { + if *hovered { + this.hover_target = HoverTarget::Submenu; + } })) .absolute() .left_full() @@ -1702,7 +1638,7 @@ impl ContextMenu { .inset(true) .disabled(*disabled) .toggle_state(Some(ix) == self.selected_index) - .when(!self.is_submenu && !*disabled, |item| { + .when(self.parent_menu.is_none() && !*disabled, |item| { item.on_hover(cx.listener(move |this, hovered, window, cx| { if *hovered { this.clear_selected(); @@ -1717,7 +1653,7 @@ impl ContextMenu { } })) }) - .when(self.is_submenu, |item| { + .when(self.parent_menu.is_some(), |item| { item.on_click(cx.listener(move |this, _, window, cx| { if matches!( &this.submenu_state, @@ -1752,7 +1688,7 @@ impl ContextMenu { if *hovered { parent.update(cx, |parent, _| { parent.clear_selected(); - parent.submenu_hovered = true; + parent.hover_target = HoverTarget::Submenu; }); } else { parent_clone.update(cx, |parent, cx| { @@ -1760,9 +1696,12 @@ impl ContextMenu { &parent.submenu_state, SubmenuState::Open(_) ) { + // Only close if mouse is to the left of the safety threshold + // (prevents accidental close when moving diagonally toward submenu) let should_close = parent - .submenu_hover_safety_heuristic - .should_allow_close_from_parent_area(mouse_pos); + .submenu_safety_threshold_x + .map(|threshold_x| mouse_pos.x < threshold_x) + .unwrap_or(true); if should_close { parent.close_submenu(true, cx); @@ -1920,12 +1859,9 @@ impl Render for ContextMenu { let computed_offset = if is_initializing { let menu_bounds = self.submenu_observed_bounds.get(); - let trigger_bounds = open_submenu.trigger_bounds.or_else(|| { - self.submenu_trigger_observed_bounds_by_item - .borrow() - .get(&open_submenu.item_index) - .copied() - }); + let trigger_bounds = open_submenu + .trigger_bounds + .or_else(|| self.submenu_trigger_bounds.get()); match (menu_bounds, trigger_bounds) { (Some(menu_bounds), Some(trigger_bounds)) => { @@ -2009,7 +1945,9 @@ impl Render for ContextMenu { .on_action(cx.listener(ContextMenu::confirm)) .on_action(cx.listener(ContextMenu::cancel)) .on_hover(cx.listener(|this, hovered: &bool, _, _| { - this.menu_hovered = *hovered; + if *hovered { + this.hover_target = HoverTarget::MainMenu; + } })) .on_mouse_down_out(cx.listener( |this, event: &MouseDownEvent, window, cx| { @@ -2021,17 +1959,14 @@ impl Render for ContextMenu { } } - if this.is_submenu { - if let Some(parent) = &this.parent_menu { - let overriden_by_parent_trigger = parent - .read(cx) - .submenu_trigger_observed_bounds_by_item - .borrow() - .values() - .any(|bounds| bounds.contains(&event.position)); - if overriden_by_parent_trigger { - return; - } + if let Some(parent) = &this.parent_menu { + let overridden_by_parent_trigger = parent + .read(cx) + .submenu_trigger_bounds + .get() + .is_some_and(|bounds| bounds.contains(&event.position)); + if overridden_by_parent_trigger { + return; } }