From 83a41dcd68111d8a8ae16230fa4e4abd8fe55d88 Mon Sep 17 00:00:00 2001 From: dantebertuzzi Date: Tue, 20 Oct 2026 18:56:04 -0300 Subject: [PATCH] Right-click on a chart opens the chart's menu, not the cell's (fixes #2067) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grid's right-click handler never hit-tested objects, so right-clicking a chart opened the menu of the cell underneath it. And since any button's drag start grabbed an object, a right press that moved a little started dragging the chart and no menu opened at all. Right-clicking a chart, picture, shape or slicer now selects it and opens its own menu (Delete; for charts also Select Data… and Format Chart Area…). Only the primary button moves objects. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/grid.rs | 68 ++++++++++++++++++++++++++-- crates/ui-egui/src/i18n/de.rs | 2 + crates/ui-egui/src/i18n/pt.rs | 2 + crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/tests/chart_resize.rs | 48 ++++++++++++++++++++ 5 files changed, 116 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 106bc6684..51c653e22 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 right-clicked chart, picture, shape or slicer: where to open its menu, its kind and id. + pub object_menu: Option<(Pos2, &'static str, u32)>, pub marching_phase: f32, /// The selection outline on its way to a new selection. pub glide: crate::selection_glide::SelectionGlide, @@ -790,6 +792,7 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) { list_picker(app, ui, &geo, previous_popups.1 == app.grid.list_picker); header_menu(app, ui, previous_popups.2 == app.grid.header_menu); context_menu(app, ui); + object_menu(app, ui); let track_x = if rtl { (available.left(), rect.left()) } else { (rect.right(), available.right()) }; vertical_scrollbar(app, ui, sh, &geo, track_x, max_y, &t); if app.session.clipboard.is_some() { @@ -1940,10 +1943,12 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & } return; } - // Objects (charts, pictures, shapes, slicers) take clicks first. - if resp.drag_started() || primary_clicked { + // Objects (charts, pictures, shapes, slicers) take clicks first. Only the primary button moves + // them: a right-click that wobbles into a drag still opens the object's menu (#2067). + let primary_drag = resp.drag_started_by(egui::PointerButton::Primary); + if primary_drag || primary_clicked { // A drag is recognised only after the pointer has moved, so grab the object where the press began. - let at = if resp.drag_started() { pointer.press_origin().or(pos) } else { pos }; + let at = if primary_drag { pointer.press_origin().or(pos) } else { pos }; if let Some(p) = at && let Some((kind, id, rect)) = crate::chartview::hit(app, geo, sh, p) { @@ -1958,7 +1963,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & if kind == "chart" && resp.double_clicked() { app.grid.pane = Some("formatChart".into()); } - if resp.drag_started() { + if primary_drag { let a = object_anchor(sh, kind, id); let resize = (p - rect.right_bottom()).length() < OBJECT_RESIZE_RADIUS; if let Some(a) = a { @@ -1966,10 +1971,21 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & } } return; - } else if primary_clicked || resp.drag_started() { + } else { app.selected_chart = None; } } + // A right-click on a chart, picture, shape or slicer opens the object's menu, not the cell's + // under it; a right press that wobbles into a drag counts too (#2067). + let secondary_drag = resp.drag_started_by(egui::PointerButton::Secondary); + if (resp.secondary_clicked() || secondary_drag) + && let Some(p) = if secondary_drag { pointer.press_origin().map(|p| geo.mirror_pos(p)).or(pos) } else { pos } + && let Some((kind, id, _)) = crate::chartview::hit(app, geo, sh, p) + { + app.selected_chart = Some(id); + app.grid.object_menu = Some((geo.mirror_pos(p), kind, id)); + return; + } // A double-click is also a click: handle header AutoFit before resize setup returns. if resp.double_clicked() @@ -2970,6 +2986,48 @@ fn context_menu(app: &mut SheetApp, ui: &mut egui::Ui) { } } +/// The right-click menu of a chart, picture, shape or slicer. +fn object_menu(app: &mut SheetApp, ui: &mut egui::Ui) { + let Some((at, kind, id)) = app.grid.object_menu else { return }; + let mut close = false; + let resp = egui::Area::new(egui::Id::new("object_context")).fixed_pos(at).order(egui::Order::Foreground).show(ui.ctx(), |ui| { + egui::Frame::popup(ui.style()).show(ui, |ui| { + ui.set_min_width(200.0); + let mut items = vec![("Delete", "object.delete")]; + if kind == "chart" { + items.extend([("-", ""), ("Select Data…", "ui:selectData"), ("Format Chart Area…", "ui:formatChart")]); + } + let lang = crate::i18n::current(ui.ctx()); + for (label, action) in items { + if label == "-" { + ui.separator(); + continue; + } + if ui.add(egui::Button::new(lang.tr(label)).frame(false).min_size(vec2(190.0, 20.0))).clicked() { + close = true; + match action { + "ui:selectData" => app.open_dialog("selectData", json!({"chart": id})), + "ui:formatChart" => app.grid.pane = Some("formatChart".into()), + _ => { + app.run_or_alert("object.delete", json!({"kind": kind, "id": id})); + app.selected_chart = None; + } + } + } + } + }); + }); + if close + || ui.input(|i| i.key_pressed(egui::Key::Escape)) + || (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))) + { + app.grid.object_menu = None; + } +} + /// Used by `EditState` users to keep the active cell visible while pointing. pub fn point_move(app: &mut SheetApp, ed: &mut EditState, dr: i64, dc: i64, extend: bool) { let Some(d) = app.session.active() else { return }; diff --git a/crates/ui-egui/src/i18n/de.rs b/crates/ui-egui/src/i18n/de.rs index 371ea7981..ce60ab2d8 100644 --- a/crates/ui-egui/src/i18n/de.rs +++ b/crates/ui-egui/src/i18n/de.rs @@ -61,6 +61,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Insert" => "Einfügen", "Insert Cells…" => "Zellen einfügen…", "Delete" => "Löschen", + "Select Data…" => "Daten auswählen…", + "Format Chart Area…" => "Diagrammbereich formatieren…", "Delete Cells…" => "Zellen löschen…", "Insert Sheet Rows" => "Blattzeilen einfügen", "Insert Sheet Columns" => "Blattspalten einfügen", diff --git a/crates/ui-egui/src/i18n/pt.rs b/crates/ui-egui/src/i18n/pt.rs index fb9236cdb..82c405a2a 100644 --- a/crates/ui-egui/src/i18n/pt.rs +++ b/crates/ui-egui/src/i18n/pt.rs @@ -426,6 +426,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Select a chart first." => "Selecione um gráfico primeiro.", "Text Box" => "Caixa de Texto", "Select Data" => "Selecionar Dados", + "Select Data…" => "Selecionar Dados…", + "Format Chart Area…" => "Formatar Área do Gráfico…", "Row Height" => "Altura da Linha", "Column Width" => "Largura da Coluna", "Standard Width" => "Largura Padrão", diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f549aec2b..3c428e4a2 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1218,6 +1218,7 @@ impl SheetApp { && self.name_box.is_none() && !ctx.text_edit_focused() && self.grid.context_menu.is_none() + && self.grid.object_menu.is_none() && self.grid.header_menu.is_none() && self.grid.filter_menu.is_none() && self.grid.list_picker.is_none() diff --git a/crates/ui-egui/tests/chart_resize.rs b/crates/ui-egui/tests/chart_resize.rs index 5aa07e080..fb3e99165 100644 --- a/crates/ui-egui/tests/chart_resize.rs +++ b/crates/ui-egui/tests/chart_resize.rs @@ -117,3 +117,51 @@ fn dragging_the_size_field_resizes_the_chart() { let (w1, _) = size(&h, id); assert!(w1 > w0 + 20.0, "width went from {w0} to {w1}"); } + +fn right_click(h: &mut Harness, pos: egui::Pos2, wobble: egui::Vec2) { + h.input_mut().events.push(Event::PointerMoved(pos)); + h.run_steps(2); + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Secondary, pressed: true, modifiers: Modifiers::NONE }); + h.run_steps(2); + let end = pos + wobble; + if wobble != vec2(0.0, 0.0) { + h.input_mut().events.push(Event::PointerMoved(end)); + h.run_steps(2); + } + h.input_mut().events.push(Event::PointerButton { pos: end, button: PointerButton::Secondary, pressed: false, modifiers: Modifiers::NONE }); + h.run_steps(2); +} + +#[test] +fn right_clicking_a_chart_opens_its_menu_not_the_cells() { + // #2067: a right-click on a chart opened the menu of the cell under it, or nothing at all. + for wobble in [vec2(0.0, 0.0), vec2(20.0, 10.0)] { + let (mut h, id) = fixture(); + let (w0, h0) = size(&h, id); + let inside = corner(&h, id) - vec2(100.0, 80.0); + right_click(&mut h, inside, wobble); + assert_eq!(h.state().selected_chart, Some(id), "the right-clicked chart is selected ({wobble:?})"); + assert!(matches!(h.state().grid.object_menu, Some((_, "chart", i)) if i == id), "the chart's menu opens ({wobble:?})"); + assert!(h.state().grid.context_menu.is_none(), "the cell menu stays closed ({wobble:?})"); + assert_eq!(size(&h, id), (w0, h0)); + let a = h.state().session.doc().unwrap().wb.active().unwrap().charts.iter().find(|c| c.id == id).unwrap().anchor; + assert_eq!(a.cell, CellRef::parse("D3").unwrap(), "a right-button drag does not move the chart ({wobble:?})"); + } + // Delete from the menu removes the chart. + let (mut h, id) = fixture(); + let inside = corner(&h, id) - vec2(100.0, 80.0); + right_click(&mut h, inside, vec2(0.0, 0.0)); + h.get_by_label("Delete").click(); + h.run_steps(3); + assert!(h.state().session.doc().unwrap().wb.active().unwrap().charts.is_empty()); + assert!(h.state().grid.object_menu.is_none()); + // Outside the chart a right-click still opens the cell menu. + let (mut h, _) = fixture(); + let cell = { + let app = h.state(); + let sh = app.session.doc().unwrap().wb.active().unwrap(); + Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll).cell_rect(sh, CellRef::parse("A10").unwrap()).center() + }; + right_click(&mut h, cell, vec2(0.0, 0.0)); + assert!(h.state().grid.context_menu.is_some() && h.state().grid.object_menu.is_none()); +}