From d48bcaf0433377b7bc985cc168798bf0af7fa1cc Mon Sep 17 00:00:00 2001 From: DCM4711 <107349915+DCM4711@users.noreply.github.com> Date: Wed, 14 Oct 2026 21:55:20 +0200 Subject: [PATCH] Context menu: the right-click that opens it doesn't close it A quick right-click delivers press and release in one frame. The menu opens at the pointer, and its corner snaps to the nearest physical pixel (252.46 -> 252.5 on a Retina display), which can leave the pointer just outside it. The close check then saw that frame's press as a click outside the menu and closed it right away, so the menu often flashed and vanished on macOS. The menu remembers the frame it opened in, and that frame's press no longer counts as a click elsewhere. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/grid.rs | 10 +- .../ui-egui/tests/context_menu_stays_open.rs | 102 ++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/context_menu_stays_open.rs diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 97b787ef3..bf90b5f7c 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -114,6 +114,8 @@ pub struct GridState { /// Column and row of the clicked header cell, and where to open the menu. pub filter_menu: Option<(u32, u32, Pos2)>, pub context_menu: Option, + /// The frame a right-click opened `context_menu` in: that click's own press doesn't close it. + pub context_menu_frame: u64, pub marching_phase: f32, /// The selection outline on its way to a new selection. pub glide: crate::selection_glide::SelectionGlide, @@ -2321,6 +2323,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & app.grid.header_menu = Some((geo.mirror_pos(p), rows)); } else { app.grid.context_menu = Some(geo.mirror_pos(p)); + app.grid.context_menu_frame = ui.ctx().cumulative_frame_nr(); } } @@ -2913,9 +2916,14 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { } }); }); + // A quick right-click presses and releases in one frame, and the menu's corner snaps to the + // nearest physical pixel, which can put the pointer just outside it: that press opened the + // menu, so it doesn't count as a click elsewhere. + let opened_now = app.grid.context_menu_frame == ui.ctx().cumulative_frame_nr(); if close || ui.input(|i| i.key_pressed(egui::Key::Escape)) - || (ui.input(|i| i.pointer.any_pressed()) + || (!opened_now + && ui.input(|i| i.pointer.any_pressed()) && !resp.response.contains_pointer() && !resp.response.hovered() && ui.input(|i| i.pointer.interact_pos()).is_some_and(|p| !resp.response.rect.contains(p))) diff --git a/crates/ui-egui/tests/context_menu_stays_open.rs b/crates/ui-egui/tests/context_menu_stays_open.rs new file mode 100644 index 000000000..dada039a6 --- /dev/null +++ b/crates/ui-egui/tests/context_menu_stays_open.rs @@ -0,0 +1,102 @@ +//! A right-click on a cell opens its context menu, and the menu stays open until the user picks +//! an item, presses Escape or clicks elsewhere. + +use egui::{Event, Modifiers, PointerButton, Pos2}; +use gridcraft_engine::Session; +use gridcraft_engine::core::CellRef; +use gridcraft_ui_egui::{SheetApp, grid}; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + harness_at(1.0) +} + +/// `ppp` physical pixels per point: 2 on a Retina display. +fn harness_at(ppp: f32) -> Harness { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).with_pixels_per_point(ppp).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +fn center(h: &Harness, cell: &str) -> Pos2 { + let app = h.state(); + let sh = app.session.active().unwrap().wb.active().unwrap(); + grid::Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll).cell_rect(sh, CellRef::parse(cell).unwrap()).center() +} + +fn button(h: &mut Harness, pos: Pos2, pressed: bool) { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Secondary, pressed, modifiers: Modifiers::NONE }); +} + +/// Press and release arrive in the same frame (a quick click). +fn quick_right_click(h: &mut Harness, cell: &str) { + let pos = center(h, cell); + h.input_mut().events.push(Event::PointerMoved(pos)); + button(h, pos, true); + button(h, pos, false); + h.run_steps(4); +} + +/// Press and release in separate frames (a slower click). +fn slow_right_click(h: &mut Harness, cell: &str) { + let pos = center(h, cell); + h.input_mut().events.push(Event::PointerMoved(pos)); + h.run_steps(1); + button(h, pos, true); + h.run_steps(1); + button(h, pos, false); + h.run_steps(4); +} + +#[test] +fn a_quick_right_click_keeps_the_menu_open() { + let mut h = harness(); + quick_right_click(&mut h, "C5"); + assert!(h.state().grid.context_menu.is_some(), "the menu stays open"); +} + +#[test] +fn a_slow_right_click_keeps_the_menu_open() { + let mut h = harness(); + slow_right_click(&mut h, "C5"); + assert!(h.state().grid.context_menu.is_some(), "the menu stays open"); +} + +#[test] +fn right_clicking_another_cell_moves_the_menu_there() { + for quick in [true, false] { + let mut h = harness(); + slow_right_click(&mut h, "C5"); + if quick { + quick_right_click(&mut h, "M2"); + } else { + slow_right_click(&mut h, "M2"); + } + let at = h.state().grid.context_menu; + assert!(at.is_some(), "the menu stays open (quick: {quick})"); + assert_eq!(at, Some(center(&h, "M2")), "at the second cell (quick: {quick})"); + } +} + +/// A real pointer sits between pixels (x = 252.46), and the menu's corner snaps to the nearest +/// physical pixel (252.5 on a Retina display), just past the pointer. The press that comes in the +/// same frame as the release then counted as a press outside the menu and closed it at once. +#[test] +fn a_quick_right_click_between_pixels_keeps_the_menu_open_on_retina() { + let mut h = harness_at(2.0); + let pos = center(&h, "C5") + egui::vec2(0.46, 0.37); + h.input_mut().events.push(Event::PointerMoved(pos)); + button(&mut h, pos, true); + button(&mut h, pos, false); + h.run_steps(4); + assert!(h.state().grid.context_menu.is_some(), "the menu stays open"); +}