Dismiss worksheet popups on outside clicks and focus loss (#41)

This commit is contained in:
harrisonoconnorhover
2026-10-09 10:32:07 -04:00
parent 51b531be3e
commit 1d2df6126b
2 changed files with 197 additions and 15 deletions
+21 -15
View File
@@ -416,13 +416,14 @@ pub fn show(app: &mut SheetApp, ui: &mut egui::Ui) {
paint_headers(&painter, &geo, sh, &sel, &t, &quads);
}
// Interaction.
let previous_popups = (app.grid.filter_menu, app.grid.list_picker, app.grid.header_menu);
interact(app, ui, &resp, &geo, sh, &wb);
if app.editor.as_ref().is_some_and(|e| !e.from_formula_bar) {
in_cell_editor(app, ui, &geo, sh, &wb);
}
filter_menu(app, ui, &geo);
list_picker(app, ui, &geo);
header_menu(app, ui);
filter_menu(app, ui, &geo, previous_popups.0 == app.grid.filter_menu);
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);
if app.session.clipboard.is_some() {
app.grid.marching_phase = (app.grid.marching_phase + 0.5) % 8.0;
@@ -1643,7 +1644,13 @@ fn in_cell_editor(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, sh: &Sheet,
crate::formula_bar::editor_widget(app, &mut child, id, font, false, erect.width() - 4.0);
}
fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
/// Test the popup itself, not whether the pointer is over any part of the app.
/// A trigger can sit outside its popup, so its opening click must be ignored.
fn popup_dismissed(response: &egui::Response, was_open: bool) -> bool {
response.ctx.input(|i| !i.focused || i.key_pressed(egui::Key::Escape)) || (was_open && response.clicked_elsewhere())
}
fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, was_open: bool) {
let Some((col, at)) = app.grid.filter_menu else { return };
let Some(d) = app.session.active() else { return };
let wb = d.wb.clone();
@@ -1676,9 +1683,10 @@ fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
}
let wb = app.session.active().map(|d| d.wb.clone()).unwrap_or(wb);
let values = gridcraft_engine::cmd::data::filter_values(&wb, si, col);
let key = egui::Id::new(("filter_sel", col));
let mut close = false;
let area = egui::Area::new(egui::Id::new("filter_menu")).fixed_pos(at + vec2(-180.0, 10.0)).order(egui::Order::Foreground);
area.show(ui.ctx(), |ui| {
let response = area.show(ui.ctx(), |ui| {
egui::Frame::popup(ui.style()).show(ui, |ui| {
ui.set_width(230.0);
if ui.button("↑ Sort Ascending").clicked() {
@@ -1691,7 +1699,6 @@ fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
}
ui.separator();
ui.label(egui::RichText::new("Filter").strong());
let key = egui::Id::new(("filter_sel", col));
let mut checks: Vec<(String, bool)> = ui.ctx().data_mut(|d| d.get_temp::<Vec<(String, bool)>>(key)).unwrap_or(values.clone());
let all = checks.iter().all(|(_, b)| *b);
let mut all_new = all;
@@ -1710,7 +1717,6 @@ fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
ui.horizontal(|ui| {
if ui.button("Clear Filter").clicked() {
let _ = app.run("data.filterBy", json!({"column": col_to_letters(col), "clear": true}));
ui.ctx().data_mut(|d| d.remove::<Vec<(String, bool)>>(key));
close = true;
}
if ui.button("Apply").clicked() {
@@ -1722,13 +1728,13 @@ fn filter_menu(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
json!({"column": col_to_letters(col), "values": vals, "blanks": blanks})
};
let _ = app.run("data.filterBy", p);
ui.ctx().data_mut(|d| d.remove::<Vec<(String, bool)>>(key));
close = true;
}
});
});
});
if close || ui.input(|i| i.key_pressed(egui::Key::Escape)) || (ui.input(|i| i.pointer.any_pressed()) && !ui.ctx().is_pointer_over_egui()) {
if close || popup_dismissed(&response.response, was_open) {
ui.ctx().data_mut(|d| d.remove::<Vec<(String, bool)>>(key));
app.grid.filter_menu = None;
}
}
@@ -1939,7 +1945,7 @@ fn paint_overlays(
let _ = (wb, si);
}
fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo, was_open: bool) {
let Some(c) = app.grid.list_picker else { return };
let Some(d) = app.session.active() else { return };
let wb = d.wb.clone();
@@ -1952,7 +1958,7 @@ fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
let items = gridcraft_engine::cmd::data::list_items(&wb, si, &dv);
let r = geo.cell_rect(sh, c);
let mut close = false;
egui::Area::new(egui::Id::new("dv_list")).fixed_pos(r.left_bottom()).order(egui::Order::Foreground).show(ui.ctx(), |ui| {
let response = egui::Area::new(egui::Id::new("dv_list")).fixed_pos(r.left_bottom()).order(egui::Order::Foreground).show(ui.ctx(), |ui| {
egui::Frame::popup(ui.style()).inner_margin(2.0).show(ui, |ui| {
ui.set_min_width(r.width().max(100.0));
egui::ScrollArea::vertical().max_height(200.0).show(ui, |ui| {
@@ -1965,15 +1971,15 @@ fn list_picker(app: &mut SheetApp, ui: &mut egui::Ui, geo: &Geo) {
});
});
});
if close || ui.input(|i| i.key_pressed(egui::Key::Escape)) || (ui.input(|i| i.pointer.any_pressed()) && !ui.ctx().is_pointer_over_egui()) {
if close || popup_dismissed(&response.response, was_open) {
app.grid.list_picker = None;
}
}
fn header_menu(app: &mut SheetApp, ui: &mut egui::Ui) {
fn header_menu(app: &mut SheetApp, ui: &mut egui::Ui, was_open: bool) {
let Some((at, rows)) = app.grid.header_menu else { return };
let mut close = false;
egui::Area::new(egui::Id::new("header_menu")).fixed_pos(at).order(egui::Order::Foreground).show(ui.ctx(), |ui| {
let response = egui::Area::new(egui::Id::new("header_menu")).fixed_pos(at).order(egui::Order::Foreground).show(ui.ctx(), |ui| {
egui::Frame::popup(ui.style()).show(ui, |ui| {
ui.set_min_width(190.0);
let items: Vec<(&str, &str)> = if rows {
@@ -2033,7 +2039,7 @@ fn header_menu(app: &mut SheetApp, ui: &mut egui::Ui) {
}
});
});
if close || ui.input(|i| i.key_pressed(egui::Key::Escape)) || (ui.input(|i| i.pointer.any_pressed()) && !ui.ctx().is_pointer_over_egui()) {
if close || popup_dismissed(&response.response, was_open) {
app.grid.header_menu = None;
}
}
+176
View File
@@ -0,0 +1,176 @@
//! Custom worksheet popups must keep inside interactions and dismiss outside clicks.
use egui::{Event, Key, Modifiers, PointerButton, Pos2, pos2};
use egui_kittest::kittest::Queryable;
use gridcraft_engine::Session;
use gridcraft_engine::core::{CellRef, Value};
use gridcraft_ui_egui::{SheetApp, grid};
use serde_json::json;
type Harness = egui_kittest::Harness<'static, SheetApp>;
#[derive(Clone, Copy, Debug)]
enum Menu {
Header,
Filter,
Validation,
}
fn harness() -> Harness {
let mut session = Session::new();
session.new_workbook();
session.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Qty"], ["Red", 1], ["Blue", 2]]})).unwrap();
session.execute("data.filter", json!({"range": "A1:B3"})).unwrap();
session.execute("insert.table", json!({"range": "A1:B3"})).unwrap();
session.execute("range.setValues", json!({"range": "H1", "values": [["Other", "Qty"], ["Green", 3]]})).unwrap();
session.execute("insert.table", json!({"range": "H1:I2"})).unwrap();
session.execute("data.validation", json!({"range": "E3", "type": "list", "formula1": "\"First,Second\""})).unwrap();
session.execute("home.insertSheet", json!({})).unwrap();
session.execute("sheet.activate", json!({"sheet": 0})).unwrap();
let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).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 geo(h: &Harness) -> grid::Geo {
let app = h.state();
let sh = app.session.active().unwrap().wb.active().unwrap();
grid::Geo::new(sh, app.grid.rect.unwrap(), app.view().scroll)
}
fn cell_pos(h: &Harness, cell: &str) -> Pos2 {
geo(h).cell_rect(h.state().session.active().unwrap().wb.active().unwrap(), CellRef::parse(cell).unwrap()).center()
}
fn click(h: &mut Harness, pos: Pos2, button: PointerButton) {
h.input_mut().events.push(Event::PointerMoved(pos));
h.input_mut().events.push(Event::PointerButton { pos, button, pressed: true, modifiers: Modifiers::NONE });
h.input_mut().events.push(Event::PointerButton { pos, button, pressed: false, modifiers: Modifiers::NONE });
h.run_steps(2);
}
fn open(h: &mut Harness, menu: Menu) {
let geometry = geo(h);
let sh = h.state().session.active().unwrap().wb.active().unwrap();
let (pos, button) = match menu {
Menu::Header => (pos2(cell_pos(h, "A1").x, (geometry.rect.top() + geometry.cells.top()) / 2.0), PointerButton::Secondary),
Menu::Filter => (grid::filter_button_rect(&geometry, sh, CellRef::parse("A1").unwrap()).center(), PointerButton::Primary),
Menu::Validation => {
let cell = CellRef::parse("E3").unwrap();
let pos = grid::validation_arrow(sh, &geometry, cell).unwrap().center();
h.state_mut().run("selection.set", json!({"cell": "E3"})).unwrap();
h.run_steps(2);
(pos, PointerButton::Primary)
}
};
click(h, pos, button);
assert!(is_open(h, menu), "{menu:?} must survive its opening click");
}
fn is_open(h: &Harness, menu: Menu) -> bool {
match menu {
Menu::Header => h.state().grid.header_menu.is_some(),
Menu::Filter => h.state().grid.filter_menu.is_some(),
Menu::Validation => h.state().grid.list_picker.is_some(),
}
}
#[test]
fn worksheet_popups_close_on_grid_and_ribbon_clicks() {
for menu in [Menu::Header, Menu::Filter, Menu::Validation] {
for ribbon in [false, true] {
let mut h = harness();
open(&mut h, menu);
let outside = if ribbon { pos2(1050.0, 52.0) } else { cell_pos(&h, "H7") };
click(&mut h, outside, PointerButton::Primary);
assert!(!is_open(&h, menu), "{menu:?} stayed open after outside click (ribbon={ribbon})");
if !ribbon {
assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "H7");
}
}
}
}
#[test]
fn filter_closes_when_clicking_another_table_or_sheet() {
for sheet in [false, true] {
let mut h = harness();
open(&mut h, Menu::Filter);
let outside = if sheet { pos2(44.0, h.state().grid.rect.unwrap().bottom() + 16.0) } else { cell_pos(&h, "H2") };
click(&mut h, outside, PointerButton::Primary);
assert!(!is_open(&h, Menu::Filter), "filter stayed open after switching table/sheet");
if sheet {
assert_eq!(h.state().session.active().unwrap().wb.active_sheet, 1);
} else {
assert_eq!(h.state().session.active().unwrap().selection.active.a1(), "H2");
}
}
}
#[test]
fn popup_actions_still_execute_and_filter_checkboxes_stay_open() {
let mut h = harness();
open(&mut h, Menu::Header);
h.get_by_label("Copy").click();
h.run_steps(2);
assert!(!is_open(&h, Menu::Header));
assert!(h.state().session.clipboard.is_some());
open(&mut h, Menu::Filter);
h.get_by_label("(Select All)").click();
h.run_steps(2);
assert!(is_open(&h, Menu::Filter), "clicking a checkbox must keep the filter open");
let outside = cell_pos(&h, "H7");
click(&mut h, outside, PointerButton::Primary);
open(&mut h, Menu::Filter);
h.get_by_label("Apply").click();
h.run_steps(2);
let sh = h.state().session.active().unwrap().wb.active().unwrap();
assert!(!sh.is_row_hidden(1) && !sh.is_row_hidden(2), "cancelled checkbox edits must not be applied after reopening");
open(&mut h, Menu::Filter);
h.get_by_label("(Select All)").click();
h.run_steps(2);
h.get_by_label("Apply").click();
h.run_steps(2);
assert!(!is_open(&h, Menu::Filter));
assert!(h.state().session.active().unwrap().wb.active().unwrap().is_row_hidden(1));
h.state_mut().run("data.clearFilter", json!({})).unwrap();
h.run_steps(2);
open(&mut h, Menu::Validation);
h.get_by_label("Second").click();
h.run_steps(2);
assert!(!is_open(&h, Menu::Validation));
assert_eq!(h.state().session.active().unwrap().wb.active().unwrap().value(CellRef::parse("E3").unwrap()), Value::from("Second"));
}
#[test]
fn worksheet_popups_close_on_escape_or_window_focus_loss() {
for menu in [Menu::Header, Menu::Filter, Menu::Validation] {
for focus_loss in [false, true] {
let mut h = harness();
open(&mut h, menu);
if focus_loss {
h.input_mut().focused = false;
h.input_mut().events.push(Event::WindowFocused(false));
} else {
h.input_mut().events.push(Event::Key {
key: Key::Escape,
physical_key: None,
pressed: true,
repeat: false,
modifiers: Modifiers::NONE,
});
}
h.run_steps(2);
assert!(!is_open(&h, menu), "{menu:?} stayed open (focus_loss={focus_loss})");
}
}
}