diff --git a/crates/ui-egui/src/grid/interact.rs b/crates/ui-egui/src/grid/interact.rs index 65214813b..deb75d338 100644 --- a/crates/ui-egui/src/grid/interact.rs +++ b/crates/ui-egui/src/grid/interact.rs @@ -109,10 +109,14 @@ pub(super) fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Respo return; } // A plain click on a slicer value tile toggles it (Ctrl/Cmd for multi-select); the clear - // button resets it. Runs before the generic object select/drag so tiles stay clickable. + // button resets it. Runs before the generic object select/drag so tiles stay clickable, but + // only when the slicer is what the press lands on: a form control over a tile (Ctrl or + // Cmd+clicked, or clicked while selected, so `form_controls::interact` left the click) and a + // selected object's handle keep the generic path, which selects, moves or resizes them. if primary_clicked && !resp.drag_started() && let Some(p) = pos + && object_under(app, geo, sh, p).is_some_and(|(o, _, handle)| o.kind == ObjKind::Slicer && handle.is_none()) && let Some((id, act)) = crate::chartview::slicer_click(geo, sh, wb, wb.active_sheet(), p) { app.selected_object = Some(ObjRef::new(ObjKind::Slicer, id)); @@ -135,10 +139,10 @@ pub(super) fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Respo // The selected object's handles reach a little outside it, so they win over hit-testing. // Only they resize: a press anywhere on another object selects and moves it, as the // hover cursor shows. - let on_handle = at.and_then(|p| crate::chartview::selected_handle(app, geo, sh, p)); - let handle = on_handle.map(|(.., h)| h); + let under = at.and_then(|p| object_under(app, geo, sh, p)); + let handle = under.and_then(|(.., h)| h); if let Some(p) = at - && let Some((obj, _)) = on_handle.map(|(o, r, _)| (o, r)).or_else(|| crate::chartview::hit(app, geo, sh, p)) + && let Some((obj, _, _)) = under { app.selected_object = Some(obj); if resp.double_clicked() && app.open_object(obj) != crate::objects::OpenAction::Select { @@ -544,6 +548,15 @@ fn handle_cell_double_click(app: &mut SheetApp, input: &InteractionInput<'_>, c: false } +/// What a press at `p` lands on, for slicer tiles and the generic object handling alike, so the +/// z-order decides once: the selected object's handle (handles reach a little past the frame and +/// win over hit-testing), else the topmost object under `p` and its rect. +fn object_under(app: &SheetApp, geo: &Geo, sh: &Sheet, p: Pos2) -> Option<(ObjRef, Rect, Option)> { + crate::chartview::selected_handle(app, geo, sh, p) + .map(|(o, r, h)| (o, r, Some(h))) + .or_else(|| crate::chartview::hit(app, geo, sh, p).map(|(o, r)| (o, r, None))) +} + fn handle_context_menu_and_keyboard(app: &mut SheetApp, input: &InteractionInput<'_>) { let (geo, sh, sel, pos) = (input.geo, input.sheet, &input.selection, input.position); let resp = input.response; diff --git a/crates/ui-egui/tests/it/objects.rs b/crates/ui-egui/tests/it/objects.rs index e0c272817..8870f0709 100644 --- a/crates/ui-egui/tests/it/objects.rs +++ b/crates/ui-egui/tests/it/objects.rs @@ -108,6 +108,18 @@ fn click(h: &mut Harness, p: Pos2) { h.run_steps(2); } +fn click_with(h: &mut Harness, p: Pos2, modifiers: Modifiers) { + h.run_steps(30); + for pressed in [true, false] { + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); + h.input_mut().events.push(Event::PointerMoved(p)); + h.input_mut().events.push(Event::PointerButton { pos: p, button: PointerButton::Primary, pressed, modifiers }); + h.step(); + } + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); + h.run_steps(2); +} + fn double_click(h: &mut Harness, p: Pos2) { // Separate the gesture from earlier clicks (egui's triple-click window). h.run_steps(45); @@ -136,6 +148,54 @@ fn kept(h: &Harness) -> Value { h.state().session.doc().unwrap().wb.active().unwrap().value(at("R30")) } +/// A form control over a slicer's value tile is the topmost object there, so a click that +/// `form_controls::interact` leaves to the grid (Cmd+click selects the control; a click on a +/// selected control moves it) takes the generic object path and never toggles the tile beneath. +#[test] +fn a_control_over_a_slicer_tile_keeps_the_generic_path_on_a_selecting_click() { + let (mut h, o) = fixture(); + let slicer = rect(&h, o.slicer); + // A button exactly over the slicer's first value tile: the tile in screen units at the + // grid's scale, then the button's anchor in sheet units from the slicer's own. + let (z, anchor, n, columns) = { + let app = h.state(); + let d = app.session.doc().unwrap(); + let sh = d.wb.active().unwrap(); + let sl = sh.slicers.iter().find(|s| s.id == o.slicer.id).unwrap(); + let geo = Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll); + (geo.z, sl.anchor, gridcraft_engine::cmd::slicer::slicer_items(&d.wb, d.wb.active_sheet(), sl).len(), sl.columns) + }; + let layout = chartview::slicer_layout(slicer, z, n, columns); + let tile = layout.tiles[0]; + let params = json!({ + "at": anchor.cell.a1(), + "dx": anchor.dx + (tile.left() - slicer.left()) / z, + "dy": anchor.dy + (tile.top() - slicer.top()) / z, + "width": tile.width() / z, + "height": tile.height() / z, + "text": "Over", + }); + let button = ObjRef::new(ObjKind::Control, id(&h.state_mut().run("insert.button", params).unwrap(), "control")); + h.run_steps(2); + assert!(rect(&h, button).contains(tile.center()), "the button covers the tile: {:?} vs {tile:?}", rect(&h, button)); + let filter = + |h: &Harness| h.state().session.doc().unwrap().wb.active().unwrap().slicers.iter().find(|s| s.id == o.slicer.id).unwrap().selected.clone(); + assert_eq!(filter(&h), None); + // Cmd+click selects the control without working it, and the tile beneath stays as it was. + click_with(&mut h, tile.center(), Modifiers::COMMAND); + assert_eq!(h.state().selected_object, Some(button), "the control is selected"); + assert_eq!(filter(&h), None, "the tile under the control was not toggled"); + // A click on the selected control is for moving it, not for the tile beneath either. + click(&mut h, tile.center()); + assert_eq!(h.state().selected_object, Some(button)); + assert_eq!(filter(&h), None, "still no toggle"); + // The tile itself, clear of the control, still toggles. + let free = layout.tiles[1].center(); + h.state_mut().selected_object = None; + click(&mut h, free); + assert!(filter(&h).is_some(), "a click on an uncovered tile toggles it"); +} + #[test] fn a_click_selects_every_kind_and_slicers_win_over_charts() { let (mut h, o) = fixture();