Form controls: a slicer tile under a control is not toggled by a click meant for the control

A click on a form control that form_controls::interact leaves to the grid (Ctrl or Cmd+click,
which selects the control; a click on a selected control, which moves it) fell through to the
slicer tile branch, which looks only at slicers, so a control placed over a slicer's value tile
had the tile toggled and the slicer selected before the generic object handling could select or
drag the control. The slicer tile branch now runs only when the press lands on a slicer by the
same test the generic path uses (the selected object's handle first, then the topmost object), so
the z-order decides once; a control over a tile, and a selected object's handle over one, keep
the generic path. Charts, pictures and shapes lie beneath slicers in that order, so they were not
affected.

Test (kittest): a_control_over_a_slicer_tile_keeps_the_generic_path_on_a_selecting_click (a button
over the first tile; Cmd+click selects the button and the slicer's filter is unchanged; a click
on the selected button still toggles nothing; an uncovered tile still toggles).
This commit is contained in:
prxnvt committed 2026-11-02 04:46:13 -05:00
1 parent 76c70c2b89
commit d09694ff61
2 files changed
+77 -4

No files matched your search

+17 -4
View File
@@ -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::objects::Handle>)> {
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;
+60
View File
@@ -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();