From 9cb20d3e2e77d78af2f51828eb751818ac6f8569 Mon Sep 17 00:00:00 2001 From: lorenzomicheli Date: Wed, 7 Oct 2026 07:58:30 +0000 Subject: [PATCH 1/2] menu: Open a context menu with a long press on touch A finger has no right button, so ContextMenuExt could not be opened on a touch-only device. A long press on the trigger now opens the same menu at the press position. The listener is registered before the trigger's children paint, so an Input inside the trigger keeps its own long press. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/component/src/menu/context_menu.rs | 288 ++++++++++++---------- crates/kit/tests/menu.rs | 101 +++++++- 2 files changed, 250 insertions(+), 139 deletions(-) diff --git a/crates/component/src/menu/context_menu.rs b/crates/component/src/menu/context_menu.rs index d5ef6d81e1..68c77f7508 100644 --- a/crates/component/src/menu/context_menu.rs +++ b/crates/component/src/menu/context_menu.rs @@ -6,9 +6,9 @@ use std::{ use gpui::{ Anchor, AnyElement, App, Bounds, Context, DismissEvent, Element, ElementId, Entity, FocusHandle, Focusable, GlobalElementId, Hitbox, HitboxBehavior, InspectorElementId, - InteractiveElement, IntoElement, LayoutId, MouseButton, MouseDownEvent, ParentElement, Pixels, - Point, Position, Style, StyleRefinement, Styled, Subscription, Window, anchored, deferred, div, - px, + InteractiveElement, IntoElement, LayoutId, LongPressEvent, MouseButton, MouseDownEvent, + ParentElement, Pixels, Point, Position, Style, StyleRefinement, Styled, Subscription, + TouchPhase, Window, anchored, deferred, div, px, }; use crate::{input::AnyInputState, menu::PopupMenu, root::WindowState}; @@ -46,7 +46,7 @@ impl ContextMenuExt for E {} pub struct ContextMenu { id: ElementId, element: Option, - menu: Option) -> PopupMenu>>, + menu: Option>, // This is not in use, just for style refinement forwarding. _ignore_style: StyleRefinement, anchor: Anchor, @@ -370,7 +370,7 @@ impl Element for ContextMenu< fn paint( &mut self, - id: Option<&gpui::GlobalElementId>, + _: Option<&gpui::GlobalElementId>, _: Option<&InspectorElementId>, _: gpui::Bounds, request_layout: &mut Self::RequestLayoutState, @@ -378,144 +378,158 @@ impl Element for ContextMenu< window: &mut Window, cx: &mut App, ) { + let builder = self.menu.clone(); + let shared_state = request_layout.shared_state.clone(); + + // A finger has no right button, so a long press opens the menu. This + // listener goes in before the children paint, so in the bubble phase + // it runs after theirs: selectable text or an input inside the + // trigger claims its long press first, and then the menu stays shut. + window.on_mouse_event({ + let shared_state = shared_state.clone(); + let builder = builder.clone(); + let hitbox = hitbox.clone(); + move |event: &LongPressEvent, phase, window, cx| { + if phase.bubble() + && event.phase == TouchPhase::Started + && !window.default_prevented() + && hitbox.is_hovered(window) + { + window.prevent_default(); + open_menu(&shared_state, &builder, event.start_position, window, cx); + } + } + }); + if let Some(element) = &mut request_layout.element { element.paint(window, cx); } - // Take the builder before setting up element state to avoid borrow issues - let builder = self.menu.clone(); + // When right mouse click, to build content menu, and show it at the mouse position. + let hitbox = hitbox.clone(); + window.on_mouse_event(move |event: &MouseDownEvent, phase, window, cx| { + if phase.bubble() && event.button == MouseButton::Right && hitbox.is_hovered(window) { + open_menu(&shared_state, &builder, event.position, window, cx); + } + }); + } +} - self.with_element_state( - id.unwrap(), - window, - cx, - |_view, state: &mut ContextMenuState, window, _| { - let shared_state = state.shared_state.clone(); - - 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.is_hovered(window) - { - // Capture the focused element to restore focus to on dismiss. - // If focus is still on the previous menu, keep its captured focus. - let previous_focus_handle = window.focused(cx).and_then(|focused| { - let shared_state = shared_state.borrow(); - match shared_state.menu_view.as_ref() { - Some(menu) if menu.read(cx).focus_handle == focused => { - menu.read(cx).previous_focus_handle.clone() - } - _ => Some(focused), - } - }); - - // Retain the old menu's owner before clearing replacement state. - let previous_selection_input = { - let state = shared_state.borrow(); - let menu_focused = state.menu_view.as_ref().is_some_and(|menu| { - let focus = menu.focus_handle(cx); - focus.is_focused(window) || focus.contains_focused(window, cx) - }); - state - .selection_input - .as_ref() - .filter(|input| { - menu_focused || input.focus_handle(cx).is_focused(window) - }) - .cloned() - }; - let position = event.position; - - { - 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.selection_input = 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| { - // Resolve after pointer dispatch: the inner input has - // now taken focus and published its state, even if it - // was unfocused in the last rendered frame. - let selection_input = - WindowState::try_update(window, cx, |state, _, _| { - state.focused_input.clone() - }) - .flatten() - .filter(|input| input.focus_handle(cx).is_focused(window)) - .or(previous_selection_input); - let menu = PopupMenu::build(window, cx, move |menu, window, cx| { - let Some(build) = &builder else { - return menu; - }; - build(menu, window, cx) - }); - let trigger_focus_handle = - shared_state.borrow().trigger_focus_handle.clone(); - menu.update(cx, |menu, cx| { - menu.set_trigger_focus(trigger_focus_handle, cx); - menu.set_previous_focus(previous_focus_handle, cx); - }); - - // A menu on another element must not keep the last - // focused input highlighted. An explicit action target - // takes precedence over where the press landed. - let selection_input = selection_input.filter(|input| { - let Some(bounds) = input.input_bounds(cx) else { - return false; - }; - match menu.read(cx).action_context.as_ref() { - Some(target) => target == &input.focus_handle(cx), - None => bounds.contains(&position), - } - }); - if let Some(input) = selection_input.as_ref() { - input.set_selection_focus(Some(menu.focus_handle(cx)), cx); - } - - // Set up the subscription for dismiss handling. - // Hold a Weak here, not a strong clone: the closure - // would otherwise close the cycle - // `shared_state -> _subscription -> closure -> - // shared_state`, so a menu left open when the window - // closes leaks its PopupMenu entity. - let _subscription = window.subscribe(&menu, cx, { - let shared_state = Rc::downgrade(&shared_state); - move |_, _: &DismissEvent, window, _cx| { - if let Some(shared_state) = shared_state.upgrade() { - let mut state = shared_state.borrow_mut(); - state.open = false; - state.selection_input = None; - 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.selection_input = selection_input; - state._subscription = Some(_subscription); - window.refresh(); - } - } - }); - } - }); - }, - ); +type MenuBuilder = dyn Fn(PopupMenu, &mut Window, &mut Context) -> PopupMenu; + +/// Builds the menu and shows it at `position`. +fn open_menu( + shared_state: &Rc>, + builder: &Option>, + position: Point, + window: &mut Window, + cx: &mut App, +) { + // Capture the focused element to restore focus to on dismiss. + // If focus is still on the previous menu, keep its captured focus. + let previous_focus_handle = window.focused(cx).and_then(|focused| { + let shared_state = shared_state.borrow(); + match shared_state.menu_view.as_ref() { + Some(menu) if menu.read(cx).focus_handle == focused => { + menu.read(cx).previous_focus_handle.clone() + } + _ => Some(focused), + } + }); + + // Retain the old menu's owner before clearing replacement state. + let previous_selection_input = { + let state = shared_state.borrow(); + let menu_focused = state.menu_view.as_ref().is_some_and(|menu| { + let focus = menu.focus_handle(cx); + focus.is_focused(window) || focus.contains_focused(window, cx) + }); + state + .selection_input + .as_ref() + .filter(|input| menu_focused || input.focus_handle(cx).is_focused(window)) + .cloned() + }; + { + 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.selection_input = None; + shared_state._subscription = None; + shared_state.position = 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| { + // Resolve after pointer dispatch: the inner input has + // now taken focus and published its state, even if it + // was unfocused in the last rendered frame. + let selection_input = + WindowState::try_update(window, cx, |state, _, _| state.focused_input.clone()) + .flatten() + .filter(|input| input.focus_handle(cx).is_focused(window)) + .or(previous_selection_input); + let menu = PopupMenu::build(window, cx, move |menu, window, cx| { + let Some(build) = &builder else { + return menu; + }; + build(menu, window, cx) + }); + let trigger_focus_handle = shared_state.borrow().trigger_focus_handle.clone(); + menu.update(cx, |menu, cx| { + menu.set_trigger_focus(trigger_focus_handle, cx); + menu.set_previous_focus(previous_focus_handle, cx); + }); + + // A menu on another element must not keep the last + // focused input highlighted. An explicit action target + // takes precedence over where the press landed. + let selection_input = selection_input.filter(|input| { + let Some(bounds) = input.input_bounds(cx) else { + return false; + }; + match menu.read(cx).action_context.as_ref() { + Some(target) => target == &input.focus_handle(cx), + None => bounds.contains(&position), + } + }); + if let Some(input) = selection_input.as_ref() { + input.set_selection_focus(Some(menu.focus_handle(cx)), cx); + } + + // Set up the subscription for dismiss handling. + // Hold a Weak here, not a strong clone: the closure + // would otherwise close the cycle + // `shared_state -> _subscription -> closure -> + // shared_state`, so a menu left open when the window + // closes leaks its PopupMenu entity. + let _subscription = window.subscribe(&menu, cx, { + let shared_state = Rc::downgrade(&shared_state); + move |_, _: &DismissEvent, window, _cx| { + if let Some(shared_state) = shared_state.upgrade() { + let mut state = shared_state.borrow_mut(); + state.open = false; + state.selection_input = None; + 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.selection_input = selection_input; + state._subscription = Some(_subscription); + window.refresh(); + } + } + }); } #[cfg(test)] diff --git a/crates/kit/tests/menu.rs b/crates/kit/tests/menu.rs index e39f3e65c6..4d519e6c9e 100644 --- a/crates/kit/tests/menu.rs +++ b/crates/kit/tests/menu.rs @@ -7,8 +7,9 @@ use gpui_kit::component::{ use gpui_kit::test::{TestAppContextExt, TestSupportExt, TestWindowExt}; use gpui_kit::{ AnyWindowHandle, App, AppContext, ClipboardItem, Context, Entity, FocusHandle, Focusable as _, - InputEvent, KeyDownEvent, KeyUpEvent, Keystroke, MouseButton, MouseDownEvent, MouseUpEvent, - TestAppContext, Window, actions, div, point, prelude::*, px, size, + InputEvent, KeyDownEvent, KeyUpEvent, Keystroke, LongPressEvent, MouseButton, MouseDownEvent, + MouseUpEvent, Pixels, Point, TestAppContext, TouchPhase, Window, WindowHandle, actions, + base::Root, div, point, prelude::*, px, size, }; use std::time::Duration; @@ -604,3 +605,99 @@ fn submenu_select_all_uses_the_parent_input_action_target(cx: &mut TestAppContex }) .unwrap(); } + +struct LongPressMenus { + input: Entity, +} + +impl Render for LongPressMenus { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div() + .size_full() + .p_4() + .flex() + .flex_col() + .gap_4() + .child( + div() + .id("row") + .test_support() + .w(px(320.)) + .h(px(40.)) + .child("Long press me") + .context_menu(|menu, _, _| menu.menu("Copy", Box::new(Copy))), + ) + .child( + div() + .w(px(320.)) + .child(Input::new(&self.input).id("row-input")) + .context_menu(|menu, _, _| menu.menu("Copy", Box::new(Copy))), + ) + } +} + +fn long_press(window: &mut Window, cx: &mut App, position: Point) { + for phase in [TouchPhase::Started, TouchPhase::Ended] { + window.dispatch_event( + LongPressEvent { + phase, + start_position: position, + position, + } + .to_platform_input(), + cx, + ); + window.render_frame(cx); + } +} + +fn long_press_menus(cx: &mut TestAppContext) -> (WindowHandle, Entity) { + cx.update(gpui_kit::init); + common::open_window(cx, Some(size(px(640.), px(480.))), |window, cx| { + cx.new(|cx| LongPressMenus { + input: cx.new(|cx| InputState::new(window, cx).default_value("quick select")), + }) + }) +} + +/// A finger has no right button, so a long press opens the same menu. +#[gpui_kit::test] +fn long_press_opens_the_context_menu(cx: &mut TestAppContext) { + let (handle, _) = long_press_menus(cx); + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + let row = window.find("row").bounds(); + long_press(window, cx, row.center()); + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + assert!(window.try_find("popup-menu").is_some()); + }) + .unwrap(); +} + +/// An input inside the trigger keeps its own long press. +#[gpui_kit::test] +fn long_press_on_an_input_in_a_context_menu_trigger_selects(cx: &mut TestAppContext) { + let (handle, view) = long_press_menus(cx); + let input = view.read_with(cx, |view, _| view.input.clone()); + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + let bounds = window.find("row-input").bounds(); + long_press( + window, + cx, + point(bounds.left() + px(24.), bounds.center().y), + ); + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(handle.into(), |_, window, cx| { + window.render_frame(cx); + assert_eq!(input.read(cx).selected_range(), 0..5); + assert!(window.try_find("popup-menu").is_none()); + }) + .unwrap(); +} From a1568c5e283cbb89e1ae0eff2220b7a8dfb73a0d Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Wed, 7 Oct 2026 23:53:54 +0800 Subject: [PATCH 2/2] menu: Preserve touch text selection inside context menus Yield long presses over registered selectable text to window-level selection while retaining object menus on blank space. Add text-selection and blank-space regression coverage and document the new query in both locales. AI-assisted fix generated with Codex. --- crates/base/src/text_selection.rs | 17 +++++++ crates/component/src/menu/context_menu.rs | 7 +-- crates/kit/tests/menu.rs | 62 +++++++++++++++++++++++ website/docs/text-system.md | 2 + website/zh-CN/docs/text-system.md | 2 + 5 files changed, 87 insertions(+), 3 deletions(-) diff --git a/crates/base/src/text_selection.rs b/crates/base/src/text_selection.rs index a7719ddd66..a8ce67a98a 100644 --- a/crates/base/src/text_selection.rs +++ b/crates/base/src/text_selection.rs @@ -2308,6 +2308,23 @@ fn with_text_selection_scope( pub struct TextSelection; impl TextSelection { + /// Returns whether `position` hits selectable text in the active scope. + /// + /// Uses the current frame's registered text geometry and hitboxes without + /// starting or changing a selection. Blank space and text outside the active + /// selection scope do not match. A containing control can yield a long press + /// to the window's text selection before claiming the gesture itself. + /// Call during pointer dispatch with the event's position, so the hitboxes + /// reflect the point being queried. Touch-selection controls do not match. + pub fn is_selectable_at(position: Point, window: &Window, cx: &mut App) -> bool { + live_text_selection_state(window, cx).is_some_and(|state| { + state.update(cx, |state, cx| { + !state.touch.covers(position) + && state.endpoint(position, Some(window), cx).inside_text + }) + }) + } + /// Returns the currently selected text in logical document order. pub fn selected_text(window: &mut Window, cx: &mut App) -> String { let Some(state) = live_text_selection_state(window, cx) else { diff --git a/crates/component/src/menu/context_menu.rs b/crates/component/src/menu/context_menu.rs index 68c77f7508..5e13fd3588 100644 --- a/crates/component/src/menu/context_menu.rs +++ b/crates/component/src/menu/context_menu.rs @@ -382,9 +382,9 @@ impl Element for ContextMenu< let shared_state = request_layout.shared_state.clone(); // A finger has no right button, so a long press opens the menu. This - // listener goes in before the children paint, so in the bubble phase - // it runs after theirs: selectable text or an input inside the - // trigger claims its long press first, and then the menu stays shut. + // listener goes in before the children paint, so an input inside the + // trigger claims its long press first. Rich-text selection is handled + // by the window layer, which runs later; yield over its text geometry. window.on_mouse_event({ let shared_state = shared_state.clone(); let builder = builder.clone(); @@ -394,6 +394,7 @@ impl Element for ContextMenu< && event.phase == TouchPhase::Started && !window.default_prevented() && hitbox.is_hovered(window) + && !gpui_base::TextSelection::is_selectable_at(event.start_position, window, cx) { window.prevent_default(); open_menu(&shared_state, &builder, event.start_position, window, cx); diff --git a/crates/kit/tests/menu.rs b/crates/kit/tests/menu.rs index 4d519e6c9e..8cff9f9f89 100644 --- a/crates/kit/tests/menu.rs +++ b/crates/kit/tests/menu.rs @@ -3,6 +3,7 @@ use gpui_kit::component::{ button::Button, input::{Copy, Input, InputState, SelectAll, Textarea, TextareaState}, menu::{ContextMenuExt, DropdownMenu, PopupMenuItem}, + text::{TextView, TextViewState}, }; use gpui_kit::test::{TestAppContextExt, TestSupportExt, TestWindowExt}; use gpui_kit::{ @@ -608,6 +609,7 @@ fn submenu_select_all_uses_the_parent_input_action_target(cx: &mut TestAppContex struct LongPressMenus { input: Entity, + text: Entity, } impl Render for LongPressMenus { @@ -633,6 +635,15 @@ impl Render for LongPressMenus { .child(Input::new(&self.input).id("row-input")) .context_menu(|menu, _, _| menu.menu("Copy", Box::new(Copy))), ) + .child( + div() + .id("text-row") + .test_support() + .w(px(320.)) + .h(px(80.)) + .child(TextView::new(&self.text).selectable(true)) + .context_menu(|menu, _, _| menu.menu("Copy", Box::new(Copy))), + ) } } @@ -656,6 +667,7 @@ fn long_press_menus(cx: &mut TestAppContext) -> (WindowHandle, Entity