diff --git a/contributors/people.toml b/contributors/people.toml index 8ba666cc..c685b584 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -71,3 +71,6 @@ sync_github_name = true [people.Hathor875] real_name = "Krzysztof Cieślik" sync_github_name = true + +[people.jay-tau] +sync_github_name = true diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 6438c903..15560090 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1772,7 +1772,8 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { None => {} } // No dialog, close prompt or palette over the page: only then does page input count. - let unobstructed = app.dialog.is_none() && app.close_request.is_none() && !app.palette_open; + let unobstructed = !app.modal_open() && !app.palette_open; + let view = &mut app.views[index]; if view.organize { organize_grid(view, info, doc.allows_assembly(), doc.dirty, unobstructed, ui, &t); return; @@ -3273,7 +3274,7 @@ fn quick_bar(app: &mut PdfCraftApp, area: Rect, ui: &mut egui::Ui) { } /// The organize toolbar: page operations on the selection (Acrobat's Organize Pages bar). -fn organize_toolbar(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool, ui: &mut egui::Ui, t: &Tokens) { +fn organize_toolbar(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool, unobstructed: bool, ui: &mut egui::Ui, t: &Tokens) { let targets = view.target_pages(); let n = info.pages.len(); let (first, last) = (targets.first().copied().unwrap_or(0), targets.last().copied().unwrap_or(0)); @@ -3379,8 +3380,9 @@ fn organize_toolbar(view: &mut DocView, info: &DocInfo, editable: bool, dirty: b }); }); }); - // Keys act on the selection unless a text field has focus. - if editable && !ui.ctx().egui_wants_keyboard_input() { + // Keys act on the selection unless a text field or a dialog has them (Delete under + // Preferences deleted the selected pages). + if editable && unobstructed && !ui.ctx().egui_wants_keyboard_input() { use egui::{Key, Modifiers}; let (del, esc) = ui.input_mut(|i| { ( @@ -3431,16 +3433,16 @@ fn gap_button(ui: &mut egui::Ui, gap: usize, at: Pos2, height: f32, label: Strin resp.on_hover_text(label).clicked() } -fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool, auto_scroll_enabled: bool, ui: &mut egui::Ui, t: &Tokens) { +fn organize_grid(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool, unobstructed: bool, ui: &mut egui::Ui, t: &Tokens) { let ppp = ui.ctx().pixels_per_point(); // The page image scales with the zoom; the padding and the page number don't. let cell = vec2(146.0 * view.grid_zoom + 44.0, 194.0 * view.grid_zoom + 56.0); let anchor = view.grid_anchor.take(); let mut open_page = None; - organize_toolbar(view, info, editable, dirty, ui, t); + organize_toolbar(view, info, editable, dirty, unobstructed, ui, t); let viewport = ui.available_rect_before_wrap(); view.viewport_screen = viewport; - let auto_delta = if auto_scroll_enabled { + let auto_delta = if unobstructed { view.auto_scroll.update(ui, viewport, true) } else { view.auto_scroll.cancel(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 039103ec..70e14a8d 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1194,6 +1194,12 @@ impl PdfCraftApp { self.active.and_then(|i| self.views.get(i).map(|v| (i, v.id))) } + /// A dialog or prompt is open over the window: the keyboard is its own, and nothing may act + /// on the document underneath it. + pub(crate) fn modal_open(&self) -> bool { + self.dialog.is_some() || self.close_request.is_some() || self.password_prompt.is_some() || self.pending_link.is_some() || self.updates.open + } + /// Enable the UI control channel on `ctx` (opt-in; see [`control`]). Returns a client that /// sends requests to this app; [`control::serve`] exposes it on loopback. pub fn attach_control(&mut self, ctx: &egui::Context) -> control::ControlClient { @@ -1769,6 +1775,11 @@ impl PdfCraftApp { } return; } + // The other dialogs and prompts are modal too (⌘W closed the file behind Preferences, + // #870). + if self.modal_open() { + return; + } if let Some(view) = self.active.and_then(|i| self.views.get_mut(i)) && view.auto_scroll.escape(ctx) { @@ -1958,7 +1969,7 @@ impl eframe::App for PdfCraftApp { self.shortcuts(ctx); // Scrolling is transient: never resume after changing tabs, opening a modal/palette, // or returning to a window that lost focus. - let blocked = self.dialog.is_some() || self.close_request.is_some() || self.palette_open || !ctx.input(|i| i.focused); + let blocked = self.modal_open() || self.palette_open || !ctx.input(|i| i.focused); for (index, view) in self.views.iter_mut().enumerate() { if blocked || self.active != Some(index) { view.auto_scroll.cancel(); diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index 93c244af..518399f9 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -6,7 +6,7 @@ use egui::{Key, Modifiers}; use egui_kittest::Harness; use egui_kittest::kittest::{NodeT, Queryable}; use pdfcraft_render::{PageRenderer, RenderRequest, RequestKind}; -use pdfcraft_ui_egui::{CloseRequest, PdfCraftApp}; +use pdfcraft_ui_egui::{CloseRequest, Dialog, PdfCraftApp}; /// An `n`-page document with a proper xref table; page `i` shows "Page i+1". fn fixture(n: usize) -> Vec { @@ -356,6 +356,46 @@ fn the_save_prompt_answers_to_the_keyboard() { let _ = std::fs::remove_file(out); } +#[test] +fn keys_leave_the_document_under_a_dialog_alone() { + // Issue #870: ⌘W with Preferences open closed the file underneath it, and in the page grid + // Delete deleted the selected page. While a dialog is open its keys are its own. + let mut h = organize(3); + h.get_by_label("Page 2").click(); + h.run_steps(2); + h.get_by_label("Rotate clockwise").click(); + h.run_steps(3); + assert!(h.state_mut().execute("app.preferences")); + h.run_steps(3); + let shift = Modifiers::COMMAND | Modifiers::SHIFT; + for (m, key) in [ + (Modifiers::COMMAND, Key::W), + (shift, Key::W), + (Modifiers::COMMAND, Key::Z), + (Modifiers::COMMAND, Key::D), + (Modifiers::COMMAND, Key::F), + (Modifiers::NONE, Key::Delete), + (Modifiers::NONE, Key::Backspace), + ] { + h.key_press_modifiers(m, key); + h.run_steps(3); + let app = h.state(); + assert_eq!(app.views.len(), 1, "{m:?}+{key:?} closed the file"); + assert!(app.close_request.is_none(), "{m:?}+{key:?} asked to close the file"); + assert!(app.session.get(app.views[0].id).unwrap().can_undo().is_some(), "{m:?}+{key:?} undid the rotation"); + assert!(app.views[0].find.is_none(), "{m:?}+{key:?} opened Find"); + assert_eq!(app.dialog, Some(Dialog::Preferences), "{m:?}+{key:?} replaced Preferences"); + } + assert_eq!(page_texts(h.state()), ["Page 1", "Page 2", "Page 3"], "a page was deleted under the dialog"); + h.key_press(Key::Escape); + h.run_steps(3); + assert!(h.state().dialog.is_none(), "Escape closes Preferences"); + assert_eq!(h.state().views[0].target_pages(), [1], "Escape went to the dialog, not the page selection"); + h.key_press_modifiers(Modifiers::COMMAND, Key::W); + h.run_steps(3); + assert!(h.state().close_request.is_some(), "with the dialog closed, ⌘W closes the file again"); +} + #[test] fn clean_tabs_close_without_asking() { let mut h = harness(1, |_| {});