From b3c0188940d17d3b293048a4aa59811cdee27e2d Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Wed, 12 Nov 2025 14:19:52 +0800 Subject: [PATCH] context_menu: Fix ContextMenu to cover parent element area. (#1566) Close #1541 --- crates/story/src/lib.rs | 4 +- crates/story/src/menu_story.rs | 104 ++++++++++++++++------ crates/ui/src/menu/context_menu.rs | 138 ++++++++++++++++------------- crates/ui/src/table/mod.rs | 10 ++- 4 files changed, 164 insertions(+), 92 deletions(-) diff --git a/crates/story/src/lib.rs b/crates/story/src/lib.rs index d358930f..a519237d 100644 --- a/crates/story/src/lib.rs +++ b/crates/story/src/lib.rs @@ -117,7 +117,7 @@ use gpui_component::{ dock::{Panel, PanelControl, PanelEvent, PanelInfo, PanelState, TitleStyle, register_panel}, group_box::GroupBox, h_flex, - menu::{ContextMenuExt, PopupMenu}, + menu::PopupMenu, notification::Notification, scroll::ScrollbarShow, v_flex, @@ -436,8 +436,6 @@ impl RenderOnce for StorySection { } } -impl ContextMenuExt for StorySection {} - pub(crate) fn section(title: impl Into) -> StorySection { StorySection { title: title.into(), diff --git a/crates/story/src/menu_story.rs b/crates/story/src/menu_story.rs index eda174dd..45d585d1 100644 --- a/crates/story/src/menu_story.rs +++ b/crates/story/src/menu_story.rs @@ -3,7 +3,7 @@ use gpui::{ ParentElement as _, Render, SharedString, Styled as _, Window, actions, div, px, }; use gpui_component::{ - ActiveTheme as _, IconName, + ActiveTheme as _, IconName, StyledExt, button::Button, h_flex, menu::{ContextMenuExt, DropdownMenu as _, PopupMenuItem}, @@ -206,31 +206,85 @@ impl Render for MenuStory { ) .child( section("Context Menu") - .child("Right click to open ContextMenu") - .min_h_20() - .context_menu({ - move |this, window, cx| { - this.external_link_icon(false) - .link("About", "https://github.com/longbridge/gpui-component") - .separator() - .menu("Cut", Box::new(Cut)) - .menu("Copy", Box::new(Copy)) - .menu("Paste", Box::new(Paste)) - .separator() - .label("This is a label") - .menu_with_check("Toggle Check", checked, Box::new(ToggleCheck)) - .separator() - .submenu("Settings", window, cx, move |menu, _, _| { - menu.menu("Info 0", Box::new(Info(0))) + .v_flex() + .gap_4() + .child( + v_flex() + .w_full() + .p_4() + .items_center() + .justify_center() + .min_h_20() + .rounded_lg() + .border_2() + .border_dashed() + .border_color(cx.theme().border) + .child("Right click to open ContextMenu") + .context_menu({ + move |this, window, cx| { + this.external_link_icon(false) + .link( + "About", + "https://github.com/longbridge/gpui-component", + ) .separator() - .menu("Item 1", Box::new(Info(1))) - .menu("Item 2", Box::new(Info(2))) - }) - .separator() - .menu("Search All", Box::new(SearchAll)) - .separator() - } - }), + .menu("Cut", Box::new(Cut)) + .menu("Copy", Box::new(Copy)) + .menu("Paste", Box::new(Paste)) + .separator() + .label("This is a label") + .menu_with_check( + "Toggle Check", + checked, + Box::new(ToggleCheck), + ) + .separator() + .submenu("Settings", window, cx, move |menu, _, _| { + menu.menu("Info 0", Box::new(Info(0))) + .separator() + .menu("Item 1", Box::new(Info(1))) + .menu("Item 2", Box::new(Info(2))) + }) + .separator() + .menu("Search All", Box::new(SearchAll)) + .separator() + } + }) + .child( + div() + .text_sm() + .text_color(cx.theme().muted_foreground) + .child( + "You can right click anywhere in \ + this area to open the context menu.", + ), + ), + ) + .child( + div() + .id("other") + .flex() + .w_full() + .p_4() + .items_center() + .justify_center() + .min_h_20() + .rounded_lg() + .border_2() + .border_dashed() + .border_color(cx.theme().border) + .child("Here is another area with context menu.") + .context_menu({ + move |this, _, _| { + this.link( + "About", + "https://github.com/longbridge/gpui-component", + ) + .separator() + .menu("Item 1", Box::new(Info(1))) + } + }), + ), ) .child( section("Menu with scrollbar") diff --git a/crates/ui/src/menu/context_menu.rs b/crates/ui/src/menu/context_menu.rs index a65a2e93..111dfcad 100644 --- a/crates/ui/src/menu/context_menu.rs +++ b/crates/ui/src/menu/context_menu.rs @@ -1,48 +1,55 @@ use std::{cell::RefCell, rc::Rc}; use gpui::{ - anchored, deferred, div, prelude::FluentBuilder, px, relative, AnyElement, App, Context, - Corner, DismissEvent, Element, ElementId, Entity, Focusable, GlobalElementId, - InspectorElementId, InteractiveElement, IntoElement, MouseButton, MouseDownEvent, - ParentElement, Pixels, Point, Position, Stateful, Style, Subscription, Window, + anchored, deferred, div, prelude::FluentBuilder, px, AnyElement, App, Context, Corner, + DismissEvent, Element, ElementId, Entity, Focusable, GlobalElementId, InspectorElementId, + InteractiveElement, IntoElement, MouseButton, MouseDownEvent, ParentElement, Pixels, Point, + StyleRefinement, Styled, Subscription, Window, }; use crate::menu::PopupMenu; /// A extension trait for adding a context menu to an element. -pub trait ContextMenuExt: ParentElement + Sized { +pub trait ContextMenuExt: ParentElement + Styled { /// Add a context menu to the element. + /// + /// This will changed the element to be `relative` positioned, and add a child `ContextMenu` element. + /// Because the `ContextMenu` element is positioned `absolute`, it will not affect the layout of the parent element. fn context_menu( self, f: impl Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu + 'static, - ) -> Self { - self.child(ContextMenu::new("context-menu").menu(f)) + ) -> ContextMenu { + ContextMenu::new("context-menu", self).menu(f) } } -impl ContextMenuExt for Stateful where E: ParentElement {} +impl ContextMenuExt for E {} /// A context menu that can be shown on right-click. -pub struct ContextMenu { +pub struct ContextMenu { id: ElementId, - menu: - Option) -> PopupMenu + 'static>>, + element: Option, + menu: Option) -> PopupMenu>>, + // This is not in use, just for style refinement forwarding. + _ignore_style: StyleRefinement, anchor: Corner, } -impl ContextMenu { +impl ContextMenu { /// Create a new context menu with the given ID. - pub fn new(id: impl Into) -> Self { + pub fn new(id: impl Into, element: E) -> Self { Self { id: id.into(), + element: Some(element), menu: None, anchor: Corner::TopLeft, + _ignore_style: StyleRefinement::default(), } } /// Build the context menu using the given builder function. #[must_use] - pub fn menu(mut self, builder: F) -> Self + fn menu(mut self, builder: F) -> Self where F: Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu + 'static, { @@ -68,7 +75,25 @@ impl ContextMenu { } } -impl IntoElement for ContextMenu { +impl ParentElement for ContextMenu { + fn extend(&mut self, elements: impl IntoIterator) { + if let Some(element) = &mut self.element { + element.extend(elements); + } + } +} + +impl Styled for ContextMenu { + fn style(&mut self) -> &mut StyleRefinement { + if let Some(element) = &mut self.element { + element.style() + } else { + &mut self._ignore_style + } + } +} + +impl IntoElement for ContextMenu { type Element = Self; fn into_element(self) -> Self::Element { @@ -84,14 +109,14 @@ struct ContextMenuSharedState { } pub struct ContextMenuState { - menu_element: Option, + element: Option, shared_state: Rc>, } impl Default for ContextMenuState { fn default() -> Self { Self { - menu_element: None, + element: None, shared_state: Rc::new(RefCell::new(ContextMenuSharedState { menu_view: None, open: false, @@ -102,7 +127,7 @@ impl Default for ContextMenuState { } } -impl Element for ContextMenu { +impl Element for ContextMenu { type RequestLayoutState = ContextMenuState; type PrepaintState = (); @@ -121,71 +146,60 @@ impl Element for ContextMenu { window: &mut Window, cx: &mut App, ) -> (gpui::LayoutId, Self::RequestLayoutState) { - let mut style = Style::default(); - // Set the layout style relative to the table view to get same size. - style.position = Position::Absolute; - style.flex_grow = 1.0; - style.flex_shrink = 1.0; - style.size.width = relative(1.).into(); - style.size.height = relative(1.).into(); - let anchor = self.anchor; self.with_element_state( id.unwrap(), window, cx, - |_, state: &mut ContextMenuState, window, cx| { + |this, state: &mut ContextMenuState, window, cx| { let (position, open) = { let shared_state = state.shared_state.borrow(); (shared_state.position, shared_state.open) }; let menu_view = state.shared_state.borrow().menu_view.clone(); - let (menu_element, menu_layout_id) = if open { + let mut menu_element = None; + if open { let has_menu_item = menu_view .as_ref() .map(|menu| !menu.read(cx).is_empty()) .unwrap_or(false); if has_menu_item { - let mut menu_element = deferred( - anchored() - .position(position) - .snap_to_window_with_margin(px(8.)) - .anchor(anchor) - .when_some(menu_view, |this, menu| { - // Focus the menu, so that can be handle the action. - if !menu.focus_handle(cx).contains_focused(window, cx) { - menu.focus_handle(cx).focus(window); - } + menu_element = Some( + deferred( + anchored() + .position(position) + .snap_to_window_with_margin(px(8.)) + .anchor(anchor) + .when_some(menu_view, |this, menu| { + // Focus the menu, so that can be handle the action. + if !menu.focus_handle(cx).contains_focused(window, cx) { + menu.focus_handle(cx).focus(window); + } - this.child(div().occlude().child(menu.clone())) - }), - ) - .with_priority(1) - .into_any(); - - let menu_layout_id = menu_element.request_layout(window, cx); - (Some(menu_element), Some(menu_layout_id)) - } else { - (None, None) + this.child(div().occlude().child(menu.clone())) + }), + ) + .with_priority(1) + .into_any(), + ); } - } else { - (None, None) - }; - - let mut layout_ids = vec![]; - if let Some(menu_layout_id) = menu_layout_id { - layout_ids.push(menu_layout_id); } - let layout_id = window.request_layout(style, layout_ids, cx); + let mut element = this + .element + .take() + .expect("Element should exists.") + .children(menu_element) + .into_any_element(); + + let layout_id = element.request_layout(window, cx); ( layout_id, ContextMenuState { - menu_element, - + element: Some(element), ..Default::default() }, ) @@ -202,8 +216,8 @@ impl Element for ContextMenu { window: &mut Window, cx: &mut App, ) -> Self::PrepaintState { - if let Some(menu_element) = &mut request_layout.menu_element { - menu_element.prepaint(window, cx); + if let Some(element) = &mut request_layout.element { + element.prepaint(window, cx); } } @@ -217,8 +231,8 @@ impl Element for ContextMenu { window: &mut Window, cx: &mut App, ) { - if let Some(menu_element) = &mut request_layout.menu_element { - menu_element.paint(window, cx); + if let Some(element) = &mut request_layout.element { + element.paint(window, cx); } let Some(builder) = self.menu.take() else { diff --git a/crates/ui/src/table/mod.rs b/crates/ui/src/table/mod.rs index 5e27e179..6c6d814c 100644 --- a/crates/ui/src/table/mod.rs +++ b/crates/ui/src/table/mod.rs @@ -1170,7 +1170,12 @@ where } /// Calculate the extra rows needed to fill the table empty space when `stripe` is true. - fn calculate_extra_rows_needed(&self, total_height: Pixels, actual_height: Pixels, row_height: Pixels) -> usize { + fn calculate_extra_rows_needed( + &self, + total_height: Pixels, + actual_height: Pixels, + row_height: Pixels, + ) -> usize { let mut extra_rows_needed = 0; let remaining_height = total_height - actual_height; @@ -1303,7 +1308,8 @@ where .size .height; let actual_height = row_height * rows_count as f32; - let extra_rows_count = self.calculate_extra_rows_needed(total_height, actual_height, row_height); + let extra_rows_count = + self.calculate_extra_rows_needed(total_height, actual_height, row_height); let render_rows_count = if self.options.stripe { rows_count + extra_rows_count } else {