shifathosn
/
gridcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Keep keyboard navigation separate from pointer clicks (#87)

This commit is contained in:
harrisonoconnorhover 2026-10-09 21:26:06 -04:00
parent 13664fe66b
commit 698a2a8009
2 changed files with 64 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
crates/ui-egui/src/grid.rs
View File

@ -983,6 +983,8 @@ fn paint_headers(p: &Painter, geo: &Geo, sh: &Sheet, sel: &gridcraft_engine::Sel
fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &Geo, sh: &Sheet, wb: &Workbook) {
let ctx = ui.ctx().clone();
// Enter/Space synthesize egui clicks; only real pointer clicks use sheet coordinates.
let primary_clicked = resp.clicked_by(egui::PointerButton::Primary);
let pointer = ctx.input(|i| i.pointer.clone());
let mods = ctx.input(|i| i.modifiers);
let pos = pointer.interact_pos().or(pointer.hover_pos());
@ -1048,7 +1050,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
}
return;
}
if app.session.draw_tool == "eraser" && (resp.clicked() || resp.dragged()) {
if app.session.draw_tool == "eraser" && (primary_clicked || resp.dragged()) {
if let Some(p) = pos
&& let Some(("shape", id, _)) = crate::chartview::hit(app, geo, sh, p)
&& sh.shapes.iter().any(|s| s.id == id && s.kind == gridcraft_engine::model::ShapeKind::Ink)
@ -1059,7 +1061,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
}
}
// Objects (charts, pictures, shapes) take clicks first.
if resp.drag_started() || resp.clicked() {
if resp.drag_started() || primary_clicked {
if let Some(p) = pos
&& let Some((kind, id, rect)) = crate::chartview::hit(app, geo, sh, p)
{
@ -1075,13 +1077,13 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
}
}
return;
} else if resp.clicked() || resp.drag_started() {
} else if primary_clicked || resp.drag_started() {
app.selected_chart = None;
}
}
// Press: decide the drag mode.
if resp.drag_started() || (resp.clicked() && app.grid.drag == Drag::None) {
if resp.drag_started() || (primary_clicked && app.grid.drag == Drag::None) {
// Where the press began (a drag is only recognised after the pointer has moved).
let Some(p) = pointer.press_origin().or(pos) else { return };
if in_corner {
@ -1177,12 +1179,12 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
let _ = app.session.run("selection.set", json!({"range": ranges.join(","), "active": c.a1()}));
} else {
let _ = app.session.run("selection.set", json!({"cell": c.a1()}));
if resp.clicked() && wb.styles.get(sh.style_id(c)).num_fmt.as_str() == "checkbox" {
if primary_clicked && wb.styles.get(sh.style_id(c)).num_fmt.as_str() == "checkbox" {
let _ = app.run("cell.toggleCheckbox", json!({"cell": c.a1()}));
}
// A click on a hyperlink's text follows it (like Excel); elsewhere in the cell selects.
if let Some(h) = sh.hyperlinks.get(&c).cloned()
&& resp.clicked()
&& primary_clicked
&& !mods.any()
{
let r = geo.cell_rect(sh, c);
@ -1386,7 +1388,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: &
}
}
if resp.clicked() || resp.drag_started() {
if primary_clicked || resp.drag_started() {
resp.request_focus();
}
keyboard(app, &ctx, resp, geo, sh);

57
crates/ui-egui/tests/interaction.rs
View File

@ -1,9 +1,9 @@
//! UI interaction tests: real egui input through the whole app (no GPU needed).
use egui::{Event, Key, Modifiers};
use egui::{Event, Key, Modifiers, PointerButton};
use gridcraft_engine::Session;
use gridcraft_engine::core::{CellRef, Value};
use gridcraft_ui_egui::SheetApp;
use gridcraft_ui_egui::{SheetApp, grid::Geo};
use serde_json::json;
fn harness(session: Session) -> egui_kittest::Harness<'static, SheetApp> {
@ -40,6 +40,59 @@ fn value(h: &egui_kittest::Harness<'static, SheetApp>, a: &str) -> Value {
h.state().session.active().and_then(|d| d.wb.active().map(|s| s.value(CellRef::parse(a).unwrap_or_default()))).unwrap_or_default()
}
fn click_cell(h: &mut egui_kittest::Harness<'static, SheetApp>, cell: &str) {
let app = h.state();
let sheet = app.session.active().unwrap().wb.active().unwrap();
let geo = Geo::new(sheet, app.grid.rect.unwrap(), app.view().scroll);
let pos = geo.cell_rect(sheet, CellRef::parse(cell).unwrap()).center();
h.input_mut().events.extend([
Event::PointerMoved(pos),
Event::PointerButton { pos, button: PointerButton::Primary, pressed: true, modifiers: Modifiers::NONE },
Event::PointerButton { pos, button: PointerButton::Primary, pressed: false, modifiers: Modifiers::NONE },
]);
h.run_steps(2);
}
#[test]
fn keyboard_navigation_does_not_replay_the_last_pointer_click() {
let mut h = harness(blank());
click_cell(&mut h, "B2");
for (key_code, modifiers, expected) in [
(Key::Enter, Modifiers::NONE, "B3"),
(Key::Enter, Modifiers::NONE, "B4"),
(Key::Enter, Modifiers::NONE, "B5"),
(Key::Enter, Modifiers::SHIFT, "B4"),
(Key::Tab, Modifiers::NONE, "C4"),
(Key::Enter, Modifiers::NONE, "C5"),
] {
key(&mut h, key_code, modifiers);
let selection = &h.state().session.active().unwrap().selection;
assert_eq!(selection.active.a1(), expected);
assert_eq!(selection.anchor, selection.active);
assert!(h.state().editor.is_none(), "navigation must not start editing at the old pointer location");
}
// A subsequent real pointer click must still select the clicked cell.
click_cell(&mut h, "D6");
assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "D6");
key(&mut h, Key::Enter, Modifiers::NONE);
assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "D7");
}
#[test]
fn space_toggles_the_active_checkbox_without_clicking_the_hovered_cell() {
let mut session = blank();
session.execute("insert.checkbox", json!({"range": "A1:B1"})).unwrap();
let mut h = harness(session);
click_cell(&mut h, "A1");
assert_eq!(value(&h, "A1"), Value::Bool(true));
key(&mut h, Key::ArrowRight, Modifiers::NONE);
key(&mut h, Key::Space, Modifiers::NONE);
assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "B1");
assert_eq!(value(&h, "A1"), Value::Bool(true), "the old pointer location must remain unchanged");
assert_eq!(value(&h, "B1"), Value::Bool(true));
assert!(h.state().editor.is_none());
}
#[test]
fn typing_enters_values_and_moves_down() {
let mut h = harness(blank());