From 724f13cced3d84441660217d2cfb81cb5798e7cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Cie=C5=9Blik?= <132496025+Hathor875@users.noreply.github.com> Date: Sat, 10 Oct 2026 21:08:55 +0200 Subject: [PATCH] fix(view): check Shift-Command-G before Command-G for find navigation (#810) (#835) * view: check Shift-Command-G before Command-G for find navigation (#810) * Tests: the find shortcut test waits for find by a deadline (cherry picked from commit eaf3180bd8ebb353b3d8c5ca7b9ef7548ed55c37) --------- Co-authored-by: Brandon Thomas --- crates/ui-egui/src/canvas.rs | 7 +++--- crates/ui-egui/tests/viewer.rs | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 2dc7c57651..6a3193e805 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -1612,7 +1612,9 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context) { view.fit = Fit::Width; view.goto = Some((view.current, 0.0)); } - if pressed(cmd(Key::G)) { + if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::G)) { + view.find_step(false); + } else if pressed(cmd(Key::G)) { view.find_step(true); } if pressed(cmd(Key::OpenBracket)) { @@ -1621,9 +1623,6 @@ pub fn shortcuts(view: &mut DocView, ctx: &egui::Context) { if pressed(cmd(Key::CloseBracket)) { view.view_history(true); } - if pressed(KeyboardShortcut::new(Modifiers::COMMAND | Modifiers::SHIFT, Key::G)) { - view.find_step(false); - } // ⌘C arrives as a Copy event on most platforms. let copy = ctx.input(|i| i.events.iter().any(|e| matches!(e, egui::Event::Copy))); if copy && let Some(text) = view.selected_text() { diff --git a/crates/ui-egui/tests/viewer.rs b/crates/ui-egui/tests/viewer.rs index 46c04cc0f7..e6465d2cb9 100644 --- a/crates/ui-egui/tests/viewer.rs +++ b/crates/ui-egui/tests/viewer.rs @@ -130,6 +130,45 @@ fn view_history_select_all_and_find_options() { assert_eq!(h.state().views[0].find.as_ref().unwrap().matches.len(), 5); } +#[test] +fn find_shortcuts_advance_and_retreat_matches() { + let mut h = harness(); + h.state_mut().active = Some(0); + h.run_steps(2); + h.state_mut().views[0].open_find(); + h.state_mut().views[0].find.as_mut().unwrap().query = "page".into(); + h.state_mut().views[0].rerun_find(); + // Find runs in the background: wait by a deadline, not a frame count, which a loaded machine + // can use up before the last page is searched. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20); + while std::time::Instant::now() < deadline { + h.run_steps(2); + std::thread::sleep(std::time::Duration::from_millis(5)); + if h.state().views[0].find.as_ref().unwrap().matches.len() >= 15 { + break; + } + } + let n = h.state().views[0].find.as_ref().unwrap().matches.len(); + assert_eq!(n, 15); + // Surrender text edit focus so canvas shortcut handler receives key events. + h.state_mut().views[0].find.as_mut().unwrap().focus = false; + h.ctx.memory_mut(|m| m.surrender_focus(egui::Id::new("find-input"))); + h.run_steps(2); + let start = h.state().views[0].find.as_ref().unwrap().current.unwrap(); + // ⌘G advances to next match. + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::G); + h.run_steps(2); + assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some((start + 1) % n)); + // ⇧⌘G retreats to previous match. + h.key_press_modifiers(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, egui::Key::G); + h.run_steps(2); + assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some(start)); + // ⇧⌘G again wraps backwards. + h.key_press_modifiers(egui::Modifiers::COMMAND | egui::Modifiers::SHIFT, egui::Key::G); + h.run_steps(2); + assert_eq!(h.state().views[0].find.as_ref().unwrap().current, Some((start + n - 1) % n)); +} + #[test] fn layouts_fit_height_labels_and_system_theme() { use pdfcraft_ui_egui::canvas::{Fit, PageLayout};