diff --git a/AGENTS.md b/AGENTS.md index 6126d4887f..297105c4c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -175,6 +175,7 @@ PdfCraft opens files from strangers, and people trust it with their work. A malf - **Page input.** Page modules read `PagePointer` (`crates/ui-egui/src/page_pointer.rs`), never egui's raw pointer state. `page_pointer::tests::page_modules_read_the_pointer_only_through_page_pointer` enforces it. - **Page cursor.** Page modules claim the cursor for what is under the pointer (`view.cursor.claim`, `crates/ui-egui/src/canvas/page_cursor.rs`) and never set it themselves. `page_pointer::tests::page_modules_claim_the_cursor_and_never_set_it` enforces it. - **Page presses.** A page module decides what is under a point in one hit test (`Hit` in `crates/ui-egui/src/canvas/page_router.rs`) and acts on a press only through that router. Modules never read the router; they are told. `page_pointer::tests::page_modules_act_on_a_press_only_through_the_router` enforces it. +- **Page keys.** Page code (everything under `canvas/` and `tools/`, `app/keys.rs`, `app/clipboard.rs` and the Pages panel's `panels/pages.rs`) reads keys and clipboard events through `PageKeys` (`crates/ui-egui/src/canvas/page_keys.rs`), which has none while something is over the pages (a dialog, the palette, a popup, a menu: `PdfCraftApp::over_pages`), never egui's raw key state. The functions `NOT_PAGE_KEYS` lists, each with why, read raw: a window's or a field's own keys, which `PageKeys` would hide, and the few page reads it can't make (chords that wait for the menu bar instead, Escape ending what the pages hold, any key stopping a glide, the clipboard's hand-off to text fields). `page_pointer::tests::page_modules_read_keys_only_through_page_keys` enforces it. - **Fidelity.** The PDF object graph is the model. Preserve unknown data. Saves are incremental unless a full rewrite is required. Never silently drop or repair data without recording it. - **Fonts** are the optional build input `CRAFT_FONTS_DIR`. Code using `pdfcraft_fonts::CRAFT_FONTS` must work when it is empty. - **Quality gates** before every commit: `cargo fmt --check`, clippy with `-D warnings`, the tests, `cargo xtask layers`, `cargo xtask assets`, and the wasm check. New workspace crates include `workspace = true` under `[lints]`. Scope `cargo clippy -p --all-targets -- -D warnings` and `cargo test -p ` to the crates you touched; the workspace sweep is CI's job. diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index db5f2edd6d..7a4dacd183 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -299,6 +299,8 @@ impl PdfCraftApp { deferred_commands: Vec::new(), native_menu: None, menu_input: Vec::new(), + menu_pastes: 0, + page_keys: Default::default(), dialog_seen: None, dialog_epoch: 0, ctx: None, @@ -377,6 +379,61 @@ impl PdfCraftApp { self.active.and_then(|i| self.views.get(i).map(|v| (i, v.id))) } + /// Whether something over the pages has the keyboard: what covers them + /// ([`Self::covers_pages`]), the hotbar's tools window, a popup, a menu of the menu bar, or the + /// bar holding the keyboard (Alt, F10). + pub(crate) fn over_pages(&self, ctx: &egui::Context) -> bool { + self.covers_pages() + || self.hotbar.picker_over_pages(ctx) + || egui::Popup::is_any_open(ctx) + || craft_menu::bar::is_open(ctx) + || craft_menu::bar::is_focused(ctx) + } + + /// Edit ▸ Copy, Cut, Select all and Delete stand for keys, and Edit ▸ Paste for the paste the + /// platform answers with. They go where the user's would: to a text field that has the + /// keyboard, else to the pages. They join the input once the menu bar has let go of the keys, + /// which an in-window menu's click does a pass later (craft-menu stores the menu as open on + /// the pass its click closes it). A native menu's click leaves egui's popups and the Hotbar + /// tools window open, which hold the pages' keys; a click anywhere else closes them, and so + /// does this. Edit ▸ Paste asks the platform for the clipboard only then, so its answer, on a + /// later pass, is an ordinary paste. With a text field focused, a native click on Copy, Cut, + /// Paste or Select all doesn't come here: craft-menu taps the shortcut for the field instead. + fn release_menu_input(&mut self, ctx: &egui::Context) { + if self.menu_input.is_empty() && self.menu_pastes == 0 { + return; + } + if craft_menu::bar::is_open(ctx) || craft_menu::bar::is_focused(ctx) { + ctx.request_repaint(); + return; + } + if !ctx.text_edit_focused() { + egui::Popup::close_all(ctx); + self.hotbar.close_picker(); + } + let events = std::mem::take(&mut self.menu_input); + ctx.input_mut(|i| i.events.extend(events)); + for _ in 0..std::mem::take(&mut self.menu_pastes) { + ctx.send_viewport_cmd(egui::ViewportCommand::RequestPaste); + } + } + + /// Read mode and full screen hide the hotbar (the quick tools), and its tools window with it. + pub(crate) fn hotbar_hidden(&self) -> bool { + self.mode == crate::Mode::Read || self.full_screen + } + + /// The pages are covered, and no page key reaches them, a menu click's included: by a dialog or + /// prompt that turns the menus off (`modal_open`), by the full-screen and update prompts, which + /// leave the menus on, by the palette, which turns them off, or by the OCR Verify screen, which + /// takes the pages' place. + pub(crate) fn covers_pages(&self) -> bool { + // Full screen draws the page alone: not the palette, nor the OCR Verify screen it was + // entered over. Dialogs and prompts show there too. + let shown = !(self.full_screen && self.active.is_some()); + self.modal_open() || self.pending_full_screen.is_some() || self.updates.prompt_open() || (self.palette_open || self.ocr_verify.open) && shown + } + /// 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 { @@ -674,11 +731,13 @@ impl PdfCraftApp { // Shortcuts deferred last frame: the text field has taken that frame's typing since. let deferred = std::mem::take(&mut self.deferred_commands); self.native_menu_clicks(ctx); - // The keys a menu click stands for (⌘C, Delete…), before anything reads this frame's keys. - if !self.menu_input.is_empty() { - let events = std::mem::take(&mut self.menu_input); - ctx.input_mut(|i| i.events.extend(events)); - } + // The input a menu click stands for (⌘C, Delete…), before anything reads this frame's keys. + self.release_menu_input(ctx); + // A command's chord pressed with the keyboard on the menu bar (Alt, F10) takes it off the + // bar, so the chord runs in the same press, the pages' as well as the registry's. + craft_menu::bar::leave_for_shortcut(ctx); + self.hotbar.close_hidden_picker(ctx, self.hotbar_hidden()); + self.page_keys = page_keys::PageKeys::new(self.over_pages(ctx)); self.shortcuts(ctx); // Scrolling and group-drag previews are transient: never resume after changing tabs, // opening a modal/palette, or returning to a window that lost focus. diff --git a/crates/ui-egui/src/app/clipboard.rs b/crates/ui-egui/src/app/clipboard.rs index 3de7164f49..ce4b60bcd3 100644 --- a/crates/ui-egui/src/app/clipboard.rs +++ b/crates/ui-egui/src/app/clipboard.rs @@ -46,7 +46,8 @@ impl PdfCraftApp { } /// ⌘C / ⌘X / ⌘V copy, cut and paste text, images or comments on canvas (unless a text field - /// has focus). With no text selected, ⌘C copies the selected comments, each with its replies. + /// has focus, or something is over the pages). With no text selected, ⌘C copies the selected + /// comments, each with its replies. pub(super) fn clipboard_keys(&mut self, i: usize, ctx: &egui::Context) { use egui::{Key, KeyboardShortcut, Modifiers}; let pastes: Vec<_> = ctx.input(|input| { @@ -60,7 +61,7 @@ impl PdfCraftApp { }) .collect() }); - let free = self.dialog.is_none() && !self.palette_open && !ctx.egui_wants_keyboard_input() && !self.is_preparing(); + let free = self.dialog.is_none() && !self.palette_open && !self.page_keys.over() && !ctx.egui_wants_keyboard_input() && !self.is_preparing(); if !free || self.views.get(i).is_some_and(|v| v.forms.focus.is_some() || v.organize) { self.receive_pastes(i, pastes, false); return; diff --git a/crates/ui-egui/src/app/keys.rs b/crates/ui-egui/src/app/keys.rs index 419f7a9ed5..7894643a4b 100644 --- a/crates/ui-egui/src/app/keys.rs +++ b/crates/ui-egui/src/app/keys.rs @@ -1,4 +1,5 @@ use super::*; +use crate::page_keys::PageKeys; impl PdfCraftApp { pub(super) fn shortcuts(&mut self, ctx: &egui::Context) { @@ -31,57 +32,42 @@ impl PdfCraftApp { return; } // The in-window menu bar holds the keys while one of its menus is open or it has the - // keyboard (Alt, F10); a command shortcut leaves it and runs. - craft_menu::bar::leave_for_shortcut(ctx); - if !craft_menu::bar::is_open(ctx) && !craft_menu::bar::is_focused(ctx) { + // keyboard (Alt, F10); a command shortcut has left it already (`logic`) and runs. The + // chords wait meanwhile: the registered commands', Select all and the view's. + let chords = !craft_menu::bar::is_open(ctx) && !craft_menu::bar::is_focused(ctx); + // The pages' own chords (Select all, the view's) don't reach pages something covers: the + // full-screen and update prompts, or the OCR Verify screen in their place (a dialog has + // returned above, and the palette holds the keyboard). + let page_chords = !self.covers_pages(); + if chords { self.registry_shortcuts(ctx); } // (Esc closes a Combine file shown large first.) - if self.full_screen && self.combine_tab.preview.is_none() && ctx.input(|i| i.key_pressed(Key::Escape)) { + if self.full_screen && self.combine_tab.preview.is_none() && self.page_keys.pressed(ctx, Key::Escape) { self.set_full_screen(ctx, false); } if let Some(i) = self.active { - // Select all belongs to the document or page grid, unless a text field or - // overlay owns the keyboard. Other canvas shortcuts keep their own handling. - let pages_owns_keyboard = - self.right == Some(RightPanel::Pages) && self.views[i].pages_focus_id.is_some_and(|id| ctx.memory(|m| m.has_focus(id))); - if self.dialog.is_none() - && !self.palette_open - && (pages_owns_keyboard || !ctx.egui_wants_keyboard_input()) - && ctx.input_mut(|input| input.consume_shortcut(&egui::KeyboardShortcut::new(egui::Modifiers::COMMAND, Key::A))) - { - let view = &self.views[i]; - // A paragraph or text box open for typing keeps ⌘A for its own text. - let edit_objects = self.quick_tool == QuickTool::EditText && view.line_editor.is_none() && view.content.draft.is_none(); - if pages_owns_keyboard { - self.views[i].select_all_pages(); - } else if let Some(doc) = self.session.get(view.id).filter(|d| edit_objects && d.allows_modification()) { - object_ui::select_all(&mut self.views[i], doc); - } else if self.is_preparing() { - if let Some(doc) = self.session.get(self.views[i].id) { - prepare::select_all(&mut self.views[i], &doc.form); - } - } else { - self.views[i].select_all(); - } + if page_chords && chords { + self.select_all_keys(i, ctx); } self.tool_keys(i, ctx); + let keys = self.page_keys; if self.quick_tool == QuickTool::EditText && let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_modification()) { - object_ui::nudge_keys(ctx, &mut self.views[i], doc); - object_ui::delete_keys(ctx, &mut self.views[i]); + object_ui::nudge_keys(ctx, keys, &mut self.views[i], doc); + object_ui::delete_keys(ctx, keys, &mut self.views[i]); } else if let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_annotation()) { - comments::nudge_keys(ctx, &mut self.views[i], &doc.info); + comments::nudge_keys(ctx, keys, &mut self.views[i], &doc.info); } // A selected field moves with the arrows too (Prepare a form, #2608), before the view // turns the page with them. if self.is_preparing() && let Some(doc) = self.session.get(self.views[i].id).filter(|d| d.allows_modification()) { - prepare::nudge_keys(ctx, &mut self.views[i], doc); + prepare::nudge_keys(ctx, keys, &mut self.views[i], doc); } - canvas::shortcuts(&mut self.views[i], ctx, self.scroll_speed); + canvas::shortcuts(&mut self.views[i], ctx, keys, chords && page_chords, self.scroll_speed); self.clipboard_keys(i, ctx); self.image_pick_keys(i, ctx); } @@ -89,13 +75,43 @@ impl PdfCraftApp { } impl PdfCraftApp { + /// ⌘A selects all of the document or the page grid, unless a text field or overlay owns the + /// keyboard. + fn select_all_keys(&mut self, i: usize, ctx: &egui::Context) { + use egui::Key; + let pages_owns_keyboard = + self.right == Some(RightPanel::Pages) && self.views[i].pages_focus_id.is_some_and(|id| ctx.memory(|m| m.has_focus(id))); + if self.dialog.is_none() + && !self.palette_open + && (pages_owns_keyboard || !ctx.egui_wants_keyboard_input()) + && ctx.input_mut(|input| input.consume_shortcut(&egui::KeyboardShortcut::new(egui::Modifiers::COMMAND, Key::A))) + { + let view = &self.views[i]; + // A paragraph or text box open for typing keeps ⌘A for its own text. + let edit_objects = self.quick_tool == QuickTool::EditText && view.line_editor.is_none() && view.content.draft.is_none(); + if pages_owns_keyboard { + self.views[i].select_all_pages(); + } else if let Some(doc) = self.session.get(view.id).filter(|d| edit_objects && d.allows_modification()) { + object_ui::select_all(&mut self.views[i], doc); + } else if self.is_preparing() { + if let Some(doc) = self.session.get(self.views[i].id) { + prepare::select_all(&mut self.views[i], &doc.form); + } + } else { + self.views[i].select_all(); + } + } + } + /// The quick tools' keys, as in Acrobat: V selects, H pans, and holding Space pans until it - /// is released. Plain letters, so not while a text field, a form field, a dialog or the - /// palette has the keyboard. + /// is released. Plain letters, so not while a text field, a form field, a dialog, the palette + /// or a menu has the keyboard. fn tool_keys(&mut self, i: usize, ctx: &egui::Context) { use egui::Key; + let keys = self.page_keys; let free = self.dialog.is_none() && !self.palette_open && !ctx.egui_wants_keyboard_input() && self.views[i].forms.focus.is_none(); - let (plain, space) = ctx.input(|input| (input.modifiers.is_none(), input.key_down(Key::Space))); + let free = free && !keys.over(); + let (plain, space) = (ctx.input(|input| input.modifiers.is_none()), PageKeys::held(ctx, Key::Space)); if let Some(previous) = self.space_hand && !space { @@ -107,11 +123,11 @@ impl PdfCraftApp { if !free || !plain { return; } - if ctx.input(|input| input.key_pressed(Key::V)) { + if keys.pressed(ctx, Key::V) { self.quick_tool = QuickTool::Select; self.space_hand = None; } - if ctx.input(|input| input.key_pressed(Key::H)) { + if keys.pressed(ctx, Key::H) { self.quick_tool = QuickTool::Hand; self.space_hand = None; } diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 587917cef9..251c29cdeb 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -30,6 +30,7 @@ mod navigation; mod organize; pub(super) mod page_cursor; mod page_input; +pub(super) mod page_keys; pub(super) mod page_pointer; pub(super) mod page_router; mod presenter; @@ -59,7 +60,7 @@ use cache::{ use cache::{OLD_TILES, PageTex, STALE_TAG, TEXT_TAG, TILE_BYTES, scale_tag, tag_scale, texture_bytes, tile_side}; pub use find::Find; use find::find_bar; -pub use navigation::shortcuts; +pub(crate) use navigation::shortcuts; pub(crate) use organize::drop_gap; #[cfg(test)] pub(crate) use organize::grid_page_rect; @@ -1345,6 +1346,8 @@ struct PageEnv<'a> { initials: &'a Option, key_scroll: f32, key_side: f32, + /// This frame's keys as the pages see them. + keys: crate::page_keys::PageKeys, middle_gesture: bool, modal: bool, navigation_enabled: bool, @@ -1697,7 +1700,7 @@ fn cached_edit_images<'a>(view: &'a mut DocView, doc: &pdfcraft_engine::Document } fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i: usize) { - let PageEnv { allowed, comments_hidden, doc, editing_content, info, ppp, prefs, rects, redact_mark, scale, t, tool, .. } = frame.env; + let PageEnv { allowed, comments_hidden, doc, editing_content, info, keys, ppp, prefs, rects, redact_mark, scale, t, tool, .. } = frame.env; let PageFrame { ptr, origin, visible, pressed, pointer, .. } = *frame; let r = snap_to_pixels(rects[i].translate(origin.to_vec2()), ppp); if !r.intersects(visible.expand(400.0)) { @@ -1738,7 +1741,7 @@ fn draw_page(ui: &mut egui::Ui, view: &mut DocView, frame: &mut PageFrame<'_>, i if frame.env.text_links { view.refresh_text_links(i); } - let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark }; + let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark, keys }; // A cancelled press takes nothing until the button is up, though the page's menu still opens // where a right-click is. A page full screen doesn't show yet takes nothing at all. (A press // there gives nothing on the page an owner, so its release, once the page is in, takes nothing @@ -2793,8 +2796,9 @@ fn scroll_direction(dir: i8, dy: f32, resized: bool) -> i8 { /// The hover tip, find bar, size hint and the in-place editors drawn over the pages. Whether the /// text editor was finished. fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, primary_down: bool, hover_text: Option<(Pos2, String)>) -> bool { - let PageEnv { added, allowed, author, avail, can_modify, comments_hidden, doc, form, info, prefs, t, today, tool, units, unobstructed, .. } = - *env; + let PageEnv { + added, allowed, author, avail, can_modify, comments_hidden, doc, form, info, keys, prefs, t, today, tool, units, unobstructed, .. + } = *env; if let Some((pos, text)) = hover_text { // Click-through, as a tooltip is: the pages lose the pointer under anything that isn't. let tip = egui::Area::new(egui::Id::unique("canvas-hover")).order(egui::Order::Tooltip).interactable(false); @@ -2810,7 +2814,7 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim if tool == QuickTool::EnhanceCamera { crate::camera_ui::bar(ui.ctx(), view, avail, t, can_modify); } - if let Some(e) = comments::composer(ui.ctx(), view, info, prefs) { + if let Some(e) = comments::composer(ui.ctx(), keys, view, info, prefs) { view.pending_edit = Some(e); } if let Some(e) = comments::popups(ui.ctx(), view, info, prefs, comments_hidden, allowed) { @@ -2821,17 +2825,17 @@ fn frame_overlays(ui: &mut egui::Ui, view: &mut DocView, env: &PageEnv<'_>, prim None => e, }); } - if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), view, info) { + if let Some(e) = crate::edit_text_ui::overlay(ui.ctx(), keys, view, info) { view.pending_edit = Some(e); } let text_box = crate::forms_ui::text_box(doc, view, form); - if let Some(e) = crate::forms_ui::overlay(ui.ctx(), view, info, form, primary_down, text_box.as_ref(), today) { + if let Some(e) = crate::forms_ui::overlay(ui.ctx(), keys, view, info, form, primary_down, text_box.as_ref(), today) { view.pending_edit = Some(e); } - if let Some(e) = crate::fill_sign::type_box(ui.ctx(), view, info, author) { + if let Some(e) = crate::fill_sign::type_box(ui.ctx(), keys, view, info, author) { view.pending_edit = Some(e); } - let (typed, text_done) = crate::content_ui::editor(ui.ctx(), view, info, added); + let (typed, text_done) = crate::content_ui::editor(ui.ctx(), keys, view, info, added); if typed.is_some() { view.pending_edit = typed; } @@ -3205,13 +3209,16 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // No dialog, close prompt or palette over the page: only then does page input count. let modal = app.modal_open(); let unobstructed = !modal && !app.palette_open; + // A menu click can open a dialog after the frame's first look. + app.page_keys = app.page_keys.also(app.covers_pages()); + let keys = app.page_keys; let organize_zoom = app.organize_zoom; let invert_middle = app.invert_middle_drag; let view = &mut app.views[index]; if view.organize { // One remembered grid size for every document, kept in the settings. view.set_grid_zoom(organize_zoom); - organize_grid(view, doc, unobstructed, app.show_page_edge_shadows, invert_middle, ui, &t); + organize_grid(view, doc, unobstructed, keys, app.show_page_edge_shadows, invert_middle, ui, &t); app.organize_zoom = view.grid_zoom(); return; } @@ -3316,7 +3323,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { } else { view.prepare.selected = None; } - view.objects.prepare(doc, tool == QuickTool::EditText && can_modify, ui.ctx()); + view.objects.prepare(doc, tool == QuickTool::EditText && can_modify, ui.ctx(), keys); view.text_background.prepare(ui.ctx(), doc, view.line_editor.as_ref(), view.content.draft.as_ref(), scale); let added = if editing_content { doc.added().to_vec() } else { Vec::new() }; let doc_links = if tool == QuickTool::Link { doc.links().to_vec() } else { Vec::new() }; @@ -3364,6 +3371,7 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { initials: &initials, key_scroll, key_side, + keys, middle_gesture, modal, navigation_enabled, @@ -3470,19 +3478,19 @@ pub fn document_area(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui) { // One crop, then back to selecting (as Acrobat does). let cropped = view.pending_edit.as_ref().is_some_and(|e| matches!(e, pdfcraft_engine::Edit::SetPageBox { .. })); let mut tool = app.quick_tool; - comments::keys(ui.ctx(), view, info, &mut tool, allowed); + comments::keys(ui.ctx(), keys, view, info, &mut tool, allowed); if preparing { - crate::prepare::keys(ui.ctx(), view); + crate::prepare::keys(ui.ctx(), keys, view); } if tool != QuickTool::EditText { view.image_selection = None; crate::content_ui::release_image_selection(view, &added); } if editing_content { - crate::content_ui::keys(ui.ctx(), view, tool == QuickTool::EditText); + crate::content_ui::keys(ui.ctx(), keys, view, tool == QuickTool::EditText); } if tool == QuickTool::Link { - crate::link_ui::keys(ui.ctx(), view); + crate::link_ui::keys(ui.ctx(), keys, view); } else { view.links.selected = None; } @@ -3805,7 +3813,7 @@ fn apply_frame_requests(app: &mut PdfCraftApp, index: usize, ui: &mut egui::Ui, // Read mode hides the tools (#2646), and full screen is the page and nothing else (#1459); // the quick tools come back with the window. // A document that hides the toolbars (`HideToolbar`) hides this one too. - if app.mode != crate::Mode::Read && !app.full_screen && !app.views.get(index).is_some_and(|v| v.interface.hidden.toolbar) { + if !app.hotbar_hidden() && !app.views.get(index).is_some_and(|v| v.interface.hidden.toolbar) { // The page nearest the left edge, whose margin the bar sits in. let page_left = app.views[index].screen_rects.iter().map(|(_, r)| r.left()).reduce(f32::min); quick_bar(app, avail, page_left, ui); diff --git a/crates/ui-egui/src/canvas/image_pick.rs b/crates/ui-egui/src/canvas/image_pick.rs index 81ae3ba757..d36bfdd88e 100644 --- a/crates/ui-egui/src/canvas/image_pick.rs +++ b/crates/ui-egui/src/canvas/image_pick.rs @@ -422,7 +422,7 @@ impl PdfCraftApp { if ctx.egui_wants_keyboard_input() { return; } - if ctx.input(|inp| inp.key_pressed(egui::Key::Escape)) { + if self.page_keys.pressed(ctx, egui::Key::Escape) { if let Some(view) = self.views.get_mut(i) { view.picked_image = None; view.pending_pick = None; @@ -430,7 +430,7 @@ impl PdfCraftApp { return; } let Some(picked) = self.views.get(i).and_then(|v| v.picked_image) else { return }; - let copy = ctx.input(|inp| inp.events.iter().any(|e| matches!(e, egui::Event::Copy))); + let copy = self.page_keys.clipboard(ctx).copy; if !copy || self.views[i].selected_text().is_some() { return; } diff --git a/crates/ui-egui/src/canvas/navigation.rs b/crates/ui-egui/src/canvas/navigation.rs index 47196ca140..0fdb2d3bfd 100644 --- a/crates/ui-egui/src/canvas/navigation.rs +++ b/crates/ui-egui/src/canvas/navigation.rs @@ -379,16 +379,33 @@ fn zoom_shortcuts(view: &mut DocView, pressed: &impl Fn(egui::KeyboardShortcut) } } -/// The document view's keyboard shortcuts: find, rotation, zoom (or the page grid's size), -/// history, Escape, and the paging and scrolling keys. An open form field keeps the paging -/// and scrolling keys for itself. -pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_speed::ScrollSpeed) { +/// The view's keys: its chords, then its plain keys. `chords`: no menu of the menu bar has the +/// keyboard and nothing covers the pages, so chords may run (they wait for a menu, as the +/// registered commands' do). An open form field keeps the paging and scrolling keys for itself. +pub(crate) fn shortcuts( + view: &mut DocView, + ctx: &egui::Context, + keys: crate::page_keys::PageKeys, + chords: bool, + speed: crate::scroll_speed::ScrollSpeed, +) { + if let Some((arrows, field_open)) = view_chords(view, ctx, chords) { + view_keys(view, ctx, keys, speed, arrows, field_open); + } +} + +/// The view's chords: Find and its steps, rotation, zoom and the view history, unless a menu of the +/// menu bar has the keyboard or something covers the pages (`chords`). `None` while a text field +/// has the keyboard, and then it has the plain keys too; else whether the arrows are the view's (a +/// focused radio button keeps them), and whether an open form field (a drop-down's or a list box's +/// items) keeps the plain keys. +fn view_chords(view: &mut DocView, ctx: &egui::Context, chords: bool) -> Option<(bool, bool)> { use egui::{Key, KeyboardShortcut, Modifiers}; - if ctx.input_mut(|i| i.consume_shortcut(&KeyboardShortcut::new(Modifiers::COMMAND, Key::F))) { + let pressed = |s: KeyboardShortcut| chords && ctx.input_mut(|i| i.consume_shortcut(&s)); + if pressed(KeyboardShortcut::new(Modifiers::COMMAND, Key::F)) { view.open_find(); } let cmd = |k| KeyboardShortcut::new(Modifiers::COMMAND, k); - let pressed = |s: KeyboardShortcut| ctx.input_mut(|i| i.consume_shortcut(&s)); // ⌘G / ⇧⌘G step through the matches from the find field too, as in Acrobat and Preview // (#2669): ahead of the text-input check, but only while there is a search to step. if view.find.is_some() { @@ -402,7 +419,7 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s // radio button for the arrows that move within its group: the other page keys still work. let form_box = view.forms.keys.filter(|(id, _)| ctx.memory(|m| m.focused()) == Some(*id)); if ctx.egui_wants_keyboard_input() && form_box.is_none() { - return; + return None; } let arrows = !form_box.is_some_and(|(_, radio)| radio); // Rotation first: egui matches ⌘+ loosely with respect to Shift. @@ -418,11 +435,26 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s if pressed(cmd(Key::CloseBracket)) || pressed(alt(Key::ArrowRight)) { view.view_history(true); } - let key = |k| (arrows || !matches!(k, Key::ArrowLeft | Key::ArrowRight | Key::ArrowUp | Key::ArrowDown)) && ctx.input(|i| i.key_pressed(k)); // An open form field (a drop-down's or a list box's items) takes the arrows, Page Up / Page // Down, Home, End and Escape. A focused check box or radio button uses none of them, so the // page keys still work there. let field_open = view.forms.focus.is_some() && form_box.is_none(); + Some((arrows, field_open)) +} + +/// The view's plain keys: Escape, Home and End, full screen's keys, the arrows and Page Up / Page +/// Down. Not while something is over the pages or a form field is open (`field_open`), which keeps +/// them. +fn view_keys( + view: &mut DocView, + ctx: &egui::Context, + keys: crate::page_keys::PageKeys, + speed: crate::scroll_speed::ScrollSpeed, + arrows: bool, + field_open: bool, +) { + use egui::Key; + let key = |k| (arrows || !matches!(k, Key::ArrowLeft | Key::ArrowRight | Key::ArrowUp | Key::ArrowDown)) && keys.pressed(ctx, k); // An open menu takes Escape first and closes; the selection stays for the next one (#3535). // So does an open field, whose list closes. if key(Key::Escape) && !egui::Popup::is_any_open(ctx) && !field_open { @@ -443,9 +475,16 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context, speed: crate::scroll_s // (Shift+⌘ on macOS) with ↑ / ↓ or Page Up / Page Down (#6434; *Adobe Acrobat Help*, 2019, // "Keys for navigating a PDF", p. 83). These take the key from the page turning and // scrolling below. - let shift_cmd = |k| KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, k); - let to_start = pressed(shift_cmd(Key::ArrowUp)) || pressed(shift_cmd(Key::PageUp)); - let to_end = pressed(shift_cmd(Key::ArrowDown)) || pressed(shift_cmd(Key::PageDown)); + let (mut to_start, mut to_end) = (false, false); + keys.take_presses(ctx, |k, m| { + let shift_cmd = m.matches_logically(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT); + match k { + Key::ArrowUp | Key::PageUp if shift_cmd => to_start = true, + Key::ArrowDown | Key::PageDown if shift_cmd => to_end = true, + _ => return false, + } + true + }); if key(Key::Home) || to_start { view.go_to_page(0); } diff --git a/crates/ui-egui/src/canvas/organize.rs b/crates/ui-egui/src/canvas/organize.rs index 25c99543e4..2917ffdcae 100644 --- a/crates/ui-egui/src/canvas/organize.rs +++ b/crates/ui-egui/src/canvas/organize.rs @@ -4,6 +4,7 @@ use super::{ Align2, Color32, CornerRadius, DocInfo, DocView, Edit, Pos2, Rect, RenderRequest, RequestKind, Sense, Stroke, Tokens, Vec2, ViewAction, device_pixels, icons, pos2, theme, vec2, widgets, }; +use crate::page_keys::PageKeys; /// How far a sharp grid render may be from the size drawn before it is redone: a pinch changes /// the size on every frame, and a slightly soft or slightly large image is fine meanwhile. @@ -92,7 +93,7 @@ pub(crate) fn grid_page_rect(cell: Rect, page: Vec2) -> Rect { } /// 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, unobstructed: bool, ui: &mut egui::Ui, t: &Tokens) { +fn organize_toolbar(view: &mut DocView, info: &DocInfo, editable: bool, dirty: bool, keys: PageKeys, 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)); @@ -153,19 +154,12 @@ fn organize_toolbar(view: &mut DocView, info: &DocInfo, editable: bool, dirty: b }); // 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| { - ( - i.consume_key(Modifiers::NONE, Key::Delete) || i.consume_key(Modifiers::NONE, Key::Backspace), - i.consume_key(Modifiers::NONE, Key::Escape), - ) - }); + if editable && !ui.ctx().egui_wants_keyboard_input() { + use egui::Key; + let ctx = ui.ctx(); + let (del, esc) = (keys.take(ctx, Key::Delete) || keys.take(ctx, Key::Backspace), keys.take(ctx, Key::Escape)); // ⌘C / ⌘X / ⌘V copy, cut and paste pages (egui delivers them as clipboard events). - let (copy, cut, paste) = ui.input(|i| { - let any = |f: &dyn Fn(&egui::Event) -> bool| i.events.iter().any(f); - (any(&|e| matches!(e, egui::Event::Copy)), any(&|e| matches!(e, egui::Event::Cut)), any(&|e| matches!(e, egui::Event::Paste(_)))) - }); + let crate::page_keys::Clipboard { copy, cut, paste } = keys.clipboard(ctx); if copy || cut { view.pending_action = Some(ViewAction::CopyPages { cut }); } @@ -267,10 +261,12 @@ fn request_sharper( } } +#[allow(clippy::too_many_arguments)] pub(super) fn organize_grid( view: &mut DocView, doc: &pdfcraft_engine::Document, unobstructed: bool, + keys: PageKeys, show_page_edge_shadows: bool, invert_middle: bool, ui: &mut egui::Ui, @@ -285,7 +281,7 @@ pub(super) fn organize_grid( let anchor = view.grid_anchor.take(); let reveal = view.grid_reveal.take(); let mut open_page = None; - organize_toolbar(view, info, editable, dirty, unobstructed, ui, t); + organize_toolbar(view, info, editable, dirty, keys, ui, t); let viewport = ui.available_rect_before_wrap(); view.viewport_screen = viewport; let auto_delta = if unobstructed { diff --git a/crates/ui-egui/src/canvas/page_input.rs b/crates/ui-egui/src/canvas/page_input.rs index fa0a15562b..b39c16fde0 100644 --- a/crates/ui-egui/src/canvas/page_input.rs +++ b/crates/ui-egui/src/canvas/page_input.rs @@ -10,6 +10,7 @@ use super::{ signature_badges, verdict_mark_rect, }; use crate::page_cursor::{Layer, Subject}; +use crate::page_keys::PageKeys; use crate::page_pointer::PagePointer; use crate::page_router::{Owner, PagePress, top}; use crate::{QuickTool, comments, icons}; @@ -45,6 +46,8 @@ pub(crate) struct PageToolContext<'a> { pub page: usize, pub doc: &'a pdfcraft_engine::Document, pub info: &'a DocInfo, + /// This frame's keys as the pages see them. + pub keys: PageKeys, } /// The tools' input on one page. The hits at the press point decide who holds the press, and the @@ -64,6 +67,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut editing_content, form, info, + keys, placing, prefs, preparing, @@ -76,8 +80,8 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut } = frame.env; let PageSlot { i, xf, .. } = *slot; let PageFrame { resp, ptr, pointer, .. } = *frame; - let cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info }; - let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark }; + let cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info, keys }; + let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark, keys }; let (lines, images) = if matches!(tool, QuickTool::AddText | QuickTool::EditText) && can_modify { edit_inventory(view, doc, i) } else { Default::default() }; let field_tool = match tool { @@ -210,7 +214,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut // the palette, another window or another tab has it (`gate`), as before. let gate = !(tool == QuickTool::EditText && view.objects.input_blocked()); if tool == QuickTool::EditText && can_modify && gate { - crate::edit_text_ui::picture_keys(ui.ctx(), view, i, &images, ptr.any_down); + crate::edit_text_ui::picture_keys(ui.ctx(), keys, view, i, &images, ptr.any_down); } let on_objects = tool == QuickTool::EditText && can_modify && crate::object_ui::page_input(cx, press, view); let picks_up = matches!(tool, QuickTool::Select | QuickTool::AddText | QuickTool::EditText); @@ -346,6 +350,7 @@ fn tool_input( doc, info, initials, + keys, signature, tool, units, @@ -356,7 +361,7 @@ fn tool_input( } = frame.env; let PageSlot { i, xf, .. } = *slot; let PageFrame { resp, ptr, .. } = *frame; - let cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info }; + let cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info, keys }; let free = press == PagePress::Free; if let QuickTool::Measure(measure_tool) = tool { crate::measure_ui::page_input(ui, crate::measure_ui::Clicks::of(resp, free), doc, view, pcx, measure_tool, units); @@ -892,6 +897,7 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P form, generation, info, + keys, prepare_tab_order, prefs, preparing, @@ -905,8 +911,8 @@ pub(super) fn page_overlays(ui: &mut egui::Ui, view: &mut DocView, frame: &mut P let PageSlot { i, r, xf } = *slot; let PageFrame { resp, ptr, pointer, .. } = *frame; let painter = ui.painter(); - let tool_cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info }; - let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark }; + let tool_cx = PageToolContext { ui, resp, xf: &xf, ptr, page: i, doc, info, keys }; + let pcx = comments::PageCx { page: i, xf: &xf, ptr, info, tool, prefs, allowed, hidden: comments_hidden, redact_mark, keys }; // A press nobody holds picks the picture under it: a link's click follows the link only (ruling // 5), though its drag or double-click still lets the picked picture go, as does a drag the page // it is over first sees, whichever page it began on. diff --git a/crates/ui-egui/src/canvas/page_keys.rs b/crates/ui-egui/src/canvas/page_keys.rs new file mode 100644 index 0000000000..c70188e6cc --- /dev/null +++ b/crates/ui-egui/src/canvas/page_keys.rs @@ -0,0 +1,173 @@ +//! The keyboard as the pages see it. +//! +//! A dialog, the palette or a menu over the pages has the keys, but the pages are drawn before the +//! dialogs and read egui's keys first. So page modules read keys through [`PageKeys`], which has +//! none while something is over the pages: Delete under the Keyboard shortcuts window deleted the +//! comment selected behind it, and the Escape that closed it deselected the comment too. +//! +//! Each module still asks for itself whether a text field has the keyboard. + +use egui::{Context, Event, Key, Modifiers}; + +/// A frame's Copy, Cut and Paste, as egui delivers them (⌘C, ⌘X, ⌘V, or a menu's). +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct Clipboard { + pub copy: bool, + pub cut: bool, + pub paste: bool, +} + +/// One frame's keyboard, as the pages see it: their keys, or none while something is over them. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct PageKeys { + /// A dialog, the palette or a menu is over the pages. + over: bool, +} + +impl PageKeys { + pub(crate) fn new(over: bool) -> Self { + Self { over } + } + + /// The keys once something more is found over the pages this frame. Only ever more covered: a + /// menu that this frame's Escape closes still had that Escape. + #[must_use] + pub(crate) fn also(self, over: bool) -> Self { + Self { over: self.over || over } + } + + /// `key` was pressed for the pages this frame. It stays for whatever reads it next. + pub(crate) fn pressed(self, ctx: &Context, key: Key) -> bool { + !self.over && ctx.input(|i| i.key_pressed(key)) + } + + /// Take `key`, pressed without ⌘ or Ctrl (egui lets an extra Shift or Alt through), for the + /// pages: nothing after reads it. + pub(crate) fn take(self, ctx: &Context, key: Key) -> bool { + !self.over && ctx.input_mut(|i| i.consume_key(Modifiers::NONE, key)) + } + + /// Take the frame's key presses that `take` wants for the pages, each judged by its own + /// modifiers (the frame's can differ when a key is let go in the same frame). `take` says + /// whether it took the press; a press taken is gone for whatever reads keys next. + pub(crate) fn take_presses(self, ctx: &Context, mut take: impl FnMut(Key, Modifiers) -> bool) { + if self.over { + return; + } + ctx.input_mut(|i| { + i.events.retain(|e| match e { + Event::Key { key, pressed: true, modifiers, .. } => !take(*key, *modifiers), + _ => true, + }); + }); + } + + /// The frame's Copy, Cut and Paste for the pages: none while something is over them. They stay + /// for whatever reads them next. + pub(crate) fn clipboard(self, ctx: &Context) -> Clipboard { + if self.over { + return Clipboard::default(); + } + ctx.input(|i| { + let any = |f: fn(&Event) -> bool| i.events.iter().any(f); + Clipboard { copy: any(|e| matches!(e, Event::Copy)), cut: any(|e| matches!(e, Event::Cut)), paste: any(|e| matches!(e, Event::Paste(_))) } + }) + } + + /// Something is over the pages: their keys wait. + pub(crate) fn over(self) -> bool { + self.over + } + + /// `key` is held down. Not the pages' alone: a key let go while something is over them is let + /// go all the same. + pub(crate) fn held(ctx: &Context, key: Key) -> bool { + ctx.input(|i| i.key_down(key)) + } +} + +#[cfg(test)] +mod tests { + use egui::RawInput; + + use super::*; + + fn press(key: Key) -> Event { + Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE } + } + + /// Run one frame with `events`, calling `f` with its context. + fn frame(events: Vec, f: impl FnOnce(&Context)) { + let ctx = Context::default(); + let mut f = Some(f); + let mut output = ctx.run_ui(RawInput { events, ..Default::default() }, |ui| { + if let Some(f) = f.take() { + f(ui.ctx()); + } + }); + // An input-only test has no renderer to take the font texture. + output.textures_delta.clear(); + } + + #[test] + fn something_found_over_the_pages_stays_over_them_for_the_frame() { + assert_eq!(PageKeys::new(false).also(true), PageKeys::new(true)); + assert_eq!(PageKeys::new(true).also(false), PageKeys::new(true)); + assert_eq!(PageKeys::new(false).also(false), PageKeys::new(false)); + } + + #[test] + fn with_something_over_the_pages_they_have_no_keys_and_leave_them_there() { + frame(vec![press(Key::Delete)], |ctx| { + let keys = PageKeys::new(true); + assert!(!keys.pressed(ctx, Key::Delete)); + assert!(!keys.take(ctx, Key::Delete)); + assert!(ctx.input(|i| i.key_pressed(Key::Delete)), "the key stays for the dialog"); + }); + } + + #[test] + fn the_pages_take_the_presses_they_want_with_their_own_modifiers() { + let shifted = Event::Key { key: Key::ArrowLeft, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::SHIFT }; + frame(vec![press(Key::ArrowRight), shifted, Event::Copy, press(Key::Escape)], |ctx| { + let mut seen = Vec::new(); + PageKeys::new(true).take_presses(ctx, |key, _| { + seen.push(key); + true + }); + assert!(seen.is_empty(), "nothing while something is over the pages"); + PageKeys::new(false).take_presses(ctx, |key, modifiers| { + seen.push(key); + key == Key::ArrowLeft && modifiers.shift + }); + assert_eq!(seen, [Key::ArrowRight, Key::ArrowLeft, Key::Escape]); + assert!(!ctx.input(|i| i.key_pressed(Key::ArrowLeft)), "the press taken is gone"); + assert!(ctx.input(|i| i.key_pressed(Key::ArrowRight) && i.key_pressed(Key::Escape)), "the others stay"); + assert!(ctx.input(|i| i.events.contains(&Event::Copy)), "and so does what isn't a key"); + }); + } + + #[test] + fn the_clipboard_is_the_pages_only_with_nothing_over_them() { + frame(vec![Event::Copy, Event::Paste("text".into())], |ctx| { + assert_eq!(PageKeys::new(true).clipboard(ctx), Clipboard::default()); + assert_eq!(PageKeys::new(false).clipboard(ctx), Clipboard { copy: true, cut: false, paste: true }); + }); + frame(vec![Event::Cut], |ctx| { + assert_eq!(PageKeys::new(true).clipboard(ctx), Clipboard::default()); + assert_eq!(PageKeys::new(false).clipboard(ctx), Clipboard { copy: false, cut: true, paste: false }); + }); + } + + #[test] + fn the_pages_peek_at_a_key_or_take_it() { + frame(vec![press(Key::Delete)], |ctx| { + let keys = PageKeys::new(false); + assert!(keys.pressed(ctx, Key::Delete)); + assert!(keys.pressed(ctx, Key::Delete), "a peek leaves it"); + assert!(keys.take(ctx, Key::Delete)); + assert!(!keys.pressed(ctx, Key::Delete), "a take doesn't"); + assert!(!keys.take(ctx, Key::Escape)); + }); + } +} diff --git a/crates/ui-egui/src/canvas/page_pointer/tests.rs b/crates/ui-egui/src/canvas/page_pointer/tests.rs index 4868b22e4a..b0bf54e3b8 100644 --- a/crates/ui-egui/src/canvas/page_pointer/tests.rs +++ b/crates/ui-egui/src/canvas/page_pointer/tests.rs @@ -797,3 +797,120 @@ fn the_page_guards_read_hit_tests_split_calls_and_aliases() { let aliases: Vec<(String, String)> = [("hit", "corner_under"), ("picture_hit", "under")].map(|(a, b)| (a.to_owned(), b.to_owned())).to_vec(); assert_eq!(aliased_hit_tests(source), aliases); } + +/// egui's raw key state: a key pressed, held, let go or taken, and the key, text and clipboard +/// events themselves, however they are imported. +const RAW_KEY_READS: [&str; 17] = [ + "Event::Copy", + "Event::Cut", + "Event::Paste", + "Event::Text", + "Event::{", + "Event::*", + "Event as ", + "InputState::", + "key_pressed(", + "key_released(", + "key_down(", + "keys_down", + "num_presses(", + "consume_key(", + "consume_shortcut(", + "count_and_consume_key(", + "Event::Key", +]; + +/// Functions in the files the key guard scans ([`key_files`]) that read egui's keys themselves: +/// a window's or a field's own keys, which `PageKeys` would hide, and the pages' reads that +/// `PageKeys` can't make (chords that wait for the menu bar instead, Escape ending what the pages +/// hold, any key stopping a glide, the clipboard's hand-off to text fields): (file, function, why). +const NOT_PAGE_KEYS: [(&str, &str, &str); 17] = [ + ( + "app/clipboard.rs", + "clipboard_keys", + "the pages' clipboard: Copy, Cut and Paste (events and chords), which act only when `page_keys.over()` is false, \ + and the pastes it hands on to text fields while something is over the pages", + ), + ( + "app/keys.rs", + "select_all_keys", + "Select all, a chord that waits for a menu of the menu bar as the registered commands' do, and isn't called while something covers the pages", + ), + ("canvas/area.rs", "glide_interrupted", "any key stops the touchpad's glide"), + ("canvas/autoscroll.rs", "update_latched", "Escape ends the middle-button scroll, which holds the whole view"), + ("canvas/autoscroll.rs", "escape", "Escape ends the middle-button scroll, which holds the whole view"), + ("canvas/find.rs", "find_bar", "the Find field's own keys, while it has the focus"), + ( + "canvas/navigation.rs", + "view_chords", + "the view's chords, which wait for a menu of the menu bar as the registered commands' do, and for whatever covers the pages", + ), + ("canvas/page_input.rs", "cancel_on_escape", "Escape cancels a press the pages hold, which goes on under a dialog opened during it"), + ("tools/calc_order_ui.rs", "panel", "the Calculation order panel's own Alt+arrows"), + ("tools/comments/composer.rs", "submit_key", "the comment field's own Enter, while it has the focus (the Comments panel's too)"), + ("tools/field_table_ui.rs", "show", "the Text field table window's own Escape"), + ("tools/field_table_ui.rs", "column_dividers", "the Text field table window's own arrows"), + ("tools/fill_sign.rs", "signature_pad", "the signature dialog's own ⌘Z"), + ("tools/forms_ui.rs", "combo_editor", "the combo box's own Enter, while its field has the focus"), + ("tools/prepare/properties.rs", "body", "the field properties' own Enter, while one of their fields has the focus"), + ("tools/redact_ui.rs", "search_body", "the redaction search field's own Enter, while it has the focus"), + ("tools/sign_ui/dialog.rs", "sign_as", "the signing dialog's own Enter"), +]; + +/// The files whose key reads the key guard checks, relative to `src`: every file under `canvas/` +/// and `tools/` and the canvas's root, whether or not it is a page module yet, the app's own keys +/// and clipboard, and the Pages panel's thumbnails; not their tests, and not `PageKeys` itself. A +/// helper in another file that page code hands egui's input to isn't seen. +fn key_files() -> Vec { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let mut files = ["canvas.rs", "app/keys.rs", "app/clipboard.rs", "panels/pages.rs"].map(str::to_owned).to_vec(); + let mut dirs = vec![src.join("canvas"), src.join("tools")]; + while let Some(dir) = dirs.pop() { + for entry in std::fs::read_dir(dir).expect("source directory") { + let path = entry.expect("source entry").path(); + let stem = path.file_stem().map(|s| s.to_string_lossy().into_owned()).unwrap_or_default(); + if stem == "tests" || stem.ends_with("_tests") { + continue; + } + if path.is_dir() { + dirs.push(path); + } else if path.extension().is_some_and(|ext| ext == "rs") { + let file = + path.strip_prefix(&src).expect("under src").components().map(|c| c.as_os_str().to_string_lossy()).collect::>().join("/"); + if file != "canvas/page_keys.rs" { + files.push(file); + } + } + } + } + files.sort(); + files +} + +/// Page code reads keys only through `PageKeys`. The pages are drawn before the dialogs and the +/// palette and read egui's keys first, so a raw read acts on what is selected or typed behind +/// whatever is over them: Delete under a dialog deleted the comment selected behind it, and the +/// Escape that closed one threw away the paragraph being typed. +#[test] +fn page_modules_read_keys_only_through_page_keys() { + let src = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); + let files = key_files(); + let mut found = Vec::new(); + for file in &files { + let source = std::fs::read_to_string(src.join(file)).expect("source file").replace("\r\n", "\n"); + for (n, function, text) in code_lines(&source) { + for name in RAW_KEY_READS.iter().filter(|name| uses(&text, name)) { + found.push((file.as_str(), function.map(str::to_owned), n, (*name).to_owned())); + } + } + } + let problems = misuses_in( + found, + &NOT_PAGE_KEYS, + "reads egui's keys in page code. Read them through `PageKeys` (src/canvas/page_keys.rs) instead: it has none \ + while a dialog, the palette or a menu is over the pages, which read keys before those are drawn. Not the \ + pages' keys? Add the function to NOT_PAGE_KEYS in src/canvas/page_pointer/tests.rs, with why.", + "reads no keys of its own any more: remove it from NOT_PAGE_KEYS.", + ); + assert!(problems.is_empty(), "{}", problems.join("\n")); +} diff --git a/crates/ui-egui/src/canvas/tests.rs b/crates/ui-egui/src/canvas/tests.rs index dab411bdf0..87bda69e86 100644 --- a/crates/ui-egui/src/canvas/tests.rs +++ b/crates/ui-egui/src/canvas/tests.rs @@ -763,7 +763,7 @@ fn keyboard_find_step_is_explicit_and_a_late_text_layer_cannot_undo_it() { ctx.input_mut(|i| { i.events.push(egui::Event::Key { key: egui::Key::G, physical_key: None, pressed: true, repeat: false, modifiers: egui::Modifiers::COMMAND }); }); - shortcuts(&mut v, &ctx, Default::default()); + shortcuts(&mut v, &ctx, Default::default(), true, Default::default()); assert_eq!(v.current, 2, "the keyboard step chose the next match"); assert_eq!(v.find.as_ref().unwrap().auto_from, None, "the keyboard step is the user's choice"); diff --git a/crates/ui-egui/src/hotbar.rs b/crates/ui-egui/src/hotbar.rs index 34b3480b40..4d4539a465 100644 --- a/crates/ui-egui/src/hotbar.rs +++ b/crates/ui-egui/src/hotbar.rs @@ -23,6 +23,35 @@ pub(crate) struct Hotbar { search: String, #[serde(skip)] picker_open: bool, + /// The pass the Hotbar tools window was last drawn on. + #[serde(skip)] + picker_shown: Option, +} + +impl Hotbar { + /// The Hotbar tools window is over the pages: open, and drawn on the last pass. Read mode, full + /// screen, Organize and a document that hides the toolbars don't draw the hotbar, and a window + /// they hide holds no keys. + pub(crate) fn picker_over_pages(&self, ctx: &egui::Context) -> bool { + self.picker_open && self.picker_shown.is_some_and(|pass| pass.saturating_add(1) >= ctx.cumulative_pass_nr()) + } + + /// Closes the Hotbar tools window when the hotbar is `hidden` on this pass (Read mode or full + /// screen, which a native menu's click may have turned on just now), or the last pass didn't + /// draw it (Organize, or a document that hides the toolbars). Left open, it would hold the keys of the pass that hides it, and come + /// back over the pages on the pass that shows the hotbar again, after that pass's page keys + /// had been read without it. + pub(crate) fn close_hidden_picker(&mut self, ctx: &egui::Context, hidden: bool) { + if self.picker_open && (hidden || !self.picker_over_pages(ctx)) { + self.close_picker(); + } + } + + /// Closes the Hotbar tools window. + pub(crate) fn close_picker(&mut self) { + self.picker_open = false; + self.picker_shown = None; + } } impl Default for Hotbar { @@ -38,6 +67,7 @@ impl Default for Hotbar { placed: false, search: String::new(), picker_open: false, + picker_shown: None, } } } @@ -871,6 +901,7 @@ pub(crate) fn show(app: &mut PdfCraftApp, area: Rect, page_left: Option, ui data.insert_temp(egui::Id::unique("custom-hotbar"), shown.response.rect.size()); }); bar.picker_open = picker_open; + bar.picker_shown = picker_open.then(|| ctx.cumulative_pass_nr()); app.hotbar = bar; if let Some(id) = run { app.execute(id); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 78435914ee..d36298ef4e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -104,6 +104,7 @@ use canvas::crop; use canvas::fling; use canvas::image_pick; use canvas::page_cursor; +use canvas::page_keys; use canvas::page_pointer; use canvas::page_router; pub use canvas::scroll_speed; @@ -731,8 +732,14 @@ pub struct PdfCraftApp { deferred_commands: Vec<(&'static str, Option)>, /// The native menu bar (macOS), while it is installed; the in-window bar shows otherwise. pub(crate) native_menu: Option, - /// Input a menu click stands for (⌘C, Delete…), added to the next frame's input in `logic`. + /// Input a menu click stands for (⌘C, Delete…), waiting to join a pass's input (see + /// `release_menu_input`). pub(crate) menu_input: Vec, + /// Edit ▸ Paste clicks waiting to ask the platform for the clipboard (see + /// `release_menu_input`). + pub(crate) menu_pastes: usize, + /// This frame's keys as the pages see them: none while something is over them. + pub(crate) page_keys: page_keys::PageKeys, /// The dialog seen at the last check, and a counter bumped whenever it changes (see /// [`Self::dialog_epoch`]). dialog_seen: Option, diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 066a914e0d..5efbba309d 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -552,7 +552,7 @@ impl PdfCraftApp { } "edit.copy" => self.menu_input.push(egui::Event::Copy), CUT => self.menu_input.push(egui::Event::Cut), - PASTE => ctx.send_viewport_cmd(egui::ViewportCommand::RequestPaste), + PASTE => self.menu_pastes = self.menu_pastes.saturating_add(1), SELECT_ALL => self.menu_input.extend(key(egui::Key::A, egui::Modifiers::COMMAND)), DELETE => self.menu_input.extend(key(egui::Key::Delete, egui::Modifiers::NONE)), PANEL => self.choose_right_panel(panel(command)), diff --git a/crates/ui-egui/src/panels/comments_panel.rs b/crates/ui-egui/src/panels/comments_panel.rs index 102da43db8..62614b3173 100644 --- a/crates/ui-egui/src/panels/comments_panel.rs +++ b/crates/ui-egui/src/panels/comments_panel.rs @@ -611,6 +611,11 @@ fn card(ui: &mut egui::Ui, view: &mut DocView, context: CardContext<'_>) -> Opti let menu_id = ui.scope_id().with(("card-menu", key)); if open_menu { egui::Popup::open_id(ui.ctx(), menu_id); + } else if more_rect.is_none() { + // The button is drawn on the selected card alone. Without it (Undo deselected the card + // with its menu open), egui would keep the menu open with nowhere to show it, and an + // open popup holds every page key (`PageKeys`). + egui::Popup::close_id(ui.ctx(), menu_id); } // Hang the menu from the "…" button (a pointer anchor would follow the mouse to the items). let anchor = more_rect.map_or(egui::PopupAnchor::PointerFixed, egui::PopupAnchor::ParentRect); diff --git a/crates/ui-egui/src/panels/pages.rs b/crates/ui-egui/src/panels/pages.rs index 1d16331eb9..b53fb12575 100644 --- a/crates/ui-egui/src/panels/pages.rs +++ b/crates/ui-egui/src/panels/pages.rs @@ -87,7 +87,17 @@ pub(super) fn thumbnail_size(ui: &mut egui::Ui, t: &Tokens, view: &mut crate::Do /// (the selection page commands and Print act on) without moving the document. Right-click /// offers Extract, Cut, Copy and Paste on the selection, or on the page clicked; dragging /// moves the selection, or the page grabbed, to the gap it is dropped on. -pub(super) fn pages(ui: &mut egui::Ui, t: &Tokens, info: &DocInfo, view: &mut crate::DocView, modal: bool, editable: bool, nav: &mut Option