From e1267a713949277e9ceac712cfc8ae18b26a578e Mon Sep 17 00:00:00 2001 From: Lucas Lepski Date: Sat, 10 Oct 2026 21:55:57 +0200 Subject: [PATCH] UI-MODAL: block background input while application dialogs are open Co-Authored-By: GPT-6.1 Sol --- crates/ui-egui/src/lib.rs | 3 + crates/ui-egui/src/panels/dialogs.rs | 18 ++++- crates/ui-egui/src/shortcuts.rs | 4 ++ crates/ui-egui/src/tests_dialogs.rs | 98 ++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/src/tests_dialogs.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6b5033e0c..578b09b94 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -47,6 +47,8 @@ mod tests_curve; #[cfg(test)] mod tests_date_picker; #[cfg(test)] +mod tests_dialogs; +#[cfg(test)] mod tests_filmstrip; #[cfg(test)] mod tests_grid; @@ -1013,6 +1015,7 @@ impl LightcraftApp { ctx.request_repaint(); return; } + panels::dialogs::prepare(self, ui); let t0 = now_ms(); if std::mem::take(&mut self.ui.quit) { ctx.send_viewport_cmd(egui::ViewportCommand::Close); diff --git a/crates/ui-egui/src/panels/dialogs.rs b/crates/ui-egui/src/panels/dialogs.rs index aa287c952..95e3ca7bb 100644 --- a/crates/ui-egui/src/panels/dialogs.rs +++ b/crates/ui-egui/src/panels/dialogs.rs @@ -60,6 +60,21 @@ pub const ABOUT_TABS: &[(&str, &str)] = &[("about", "About"), ("contributors", " /// Help ▸ What's New (docs/whats-new.md). pub const WHATS_NEW: &str = include_str!("../../../../docs/whats-new.md"); +fn window_id(dialog: &Dialog) -> egui::Id { + egui::Id::new(if matches!(dialog, Dialog::Import { .. }) { "lightcraft-import-dialog" } else { "lightcraft-dialog" }) +} + +/// Block the underlying panels from the first frame, before the window establishes its modal +/// layer. Close background menus on entry; popups subsequently opened inside the dialog stay usable. +pub(crate) fn prepare(app: &LightcraftApp, ui: &mut egui::Ui) { + let Some(dialog) = &app.ui.dialog else { return }; + ui.disable(); + let layer = egui::LayerId::new(egui::Order::Middle, window_id(dialog)); + if ui.ctx().memory(|m| m.top_modal_layer()) != Some(layer) { + egui::Popup::close_all(ui.ctx()); + } +} + pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { let Some(mut dlg) = app.ui.dialog.clone() else { return }; let at_start = dlg.clone(); @@ -127,7 +142,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { // own so the size it is given doesn't carry over to the other dialogs. Its content scrolls // rather than growing the window when the options below the grid get taller (e.g. Copy). let import = matches!(dlg, Dialog::Import { .. }); - let window_id = egui::Id::new(if import { "lightcraft-import-dialog" } else { "lightcraft-dialog" }); + let window_id = window_id(&dlg); let shown = egui::Window::new(crate::i18n::tr(&title)).id(window_id) .collapsible(false) .resizable(import) @@ -1022,6 +1037,7 @@ pub fn show(app: &mut LightcraftApp, ctx: &egui::Context) { } }); if let Some(w) = shown { + ctx.memory_mut(|m| m.set_modal_layer(w.response.layer_id)); ctx.move_to_top(w.response.layer_id); crate::widgets::register(ctx, "dialog:window", w.response.rect); } diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 2abea5fa4..877ad6b83 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -304,6 +304,10 @@ pub fn handle(app: &mut LightcraftApp, ctx: &egui::Context) { if !matches!(app.ui.dialog, Some(crate::state::Dialog::Shortcuts)) { app.recording_shortcut = None; } + // Dialogs own their keys (including Esc and text editing); background photo actions wait. + if app.ui.dialog.is_some() { + return; + } // don't steal keys from text fields or from the keymap editor recording a shortcut if ctx.egui_wants_keyboard_input() || app.recording_shortcut.is_some() || app.ui.name_edit.is_some() { return; diff --git a/crates/ui-egui/src/tests_dialogs.rs b/crates/ui-egui/src/tests_dialogs.rs new file mode 100644 index 000000000..6581640ec --- /dev/null +++ b/crates/ui-egui/src/tests_dialogs.rs @@ -0,0 +1,98 @@ +//! Modal application dialogs block background input while retaining their own controls. + +use crate::headless::Headless; +use crate::{LightcraftApp, Services}; +use serde_json::json; +use std::time::Duration; + +const T: Duration = Duration::from_secs(20); + +fn demo(size: [f32; 2], ui: serde_json::Value) -> Headless { + let services = Services { png: None, ..Default::default() }; + let app = LightcraftApp::new(lightcraft_engine::Session::with_demo(), services); + let mut h = Headless::new(app, size, 1.0); + let r = h.request("ui.set", ui, T); + assert_eq!(r["ok"], true, "{r}"); + h.settle(Duration::from_secs(120)); + h +} + +/// Application dialogs block the toolbar, including the gear that would reopen Copy Settings +/// and discard its choices. Their own controls work, and closing restores background input. +#[test] +fn modal_dialogs_block_background_clicks_and_preserve_choices() { + let mut h = demo([1400.0, 1000.0], json!({"view": "detail", "right": "edit"})); + let active = h.app.session.active().unwrap(); + let rating = h.app.session.catalog.photo(active).unwrap().rating; + h.app.run("dialog.copySettings", json!({})).unwrap(); + // Also exercise the first frame of a newly opened dialog. + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, rating, "the dialog blocks background ratings"); + let r = h.request("ui.clickWidget", json!({"id": "copyGroup:none"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + let choices = h.app.ui.dialog.clone(); + assert!(matches!(&choices, Some(crate::state::Dialog::CopySettings { groups }) if groups.is_empty())); + let r = h.request("ui.clickWidget", json!({"id": "icon:copyGear"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.ui.dialog, choices, "a background click cannot reopen the dialog and reset its choices"); + let r = h.request("ui.clickWidget", json!({"id": "button:dialogCancel"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_none()); + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, 5, "closing restores toolbar input"); + // A different dialog uses the same barrier. + h.app.run("app.settings", json!({})).unwrap(); + h.step(); + let r = h.request("ui.clickWidget", json!({"id": "star:bottom:1"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, 5); + let r = h.request("ui.clickWidget", json!({"id": "button:dialogOk"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_none()); +} + +#[test] +fn modal_dialogs_block_background_shortcuts_and_keep_escape() { + let mut h = demo([1400.0, 1000.0], json!({"view": "detail", "right": "edit"})); + let active = h.app.session.active().unwrap(); + let rating = h.app.session.catalog.photo(active).unwrap().rating; + h.app.run("dialog.copySettings", json!({})).unwrap(); + h.step(); + for key in ["5", "ArrowRight", "J"] { + let r = h.request("ui.key", json!({"key": key}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + } + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, rating); + assert_eq!(h.app.session.active(), Some(active)); + assert!(!h.app.ui.show_clipping); + let r = h.request("ui.key", json!({"key": "Escape"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(h.app.ui.dialog.is_none()); + let r = h.request("ui.key", json!({"key": "5"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert_eq!(h.app.session.catalog.photo(active).unwrap().rating, 5, "closing restores shortcuts"); +} + +#[test] +fn modal_dialogs_close_background_menus() { + let mut h = demo([1400.0, 1000.0], json!({"view": "detail", "right": "edit"})); + let r = h.request("ui.clickWidget", json!({"id": "menu:Photo"}), T); + assert_eq!(r["ok"], true, "{r}"); + h.step(); + assert!(egui::Popup::is_any_open(&h.view.ctx)); + h.app.run("dialog.copySettings", json!({})).unwrap(); + h.step(); + assert!(!egui::Popup::is_any_open(&h.view.ctx), "background menus cannot remain above the modal window"); +}