Merge PR #1363: Context menu: the right-click that opens it doesn't close it

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<Pos2>,
+    /// 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,
@@ -2393,6 +2395,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();
         }
     }
 
@@ -3043,9 +3046,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)))

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");
+}