From 45dccb22d03ea2aa9b59b387b47196a6edf5f4ce Mon Sep 17 00:00:00 2001 From: alehel Date: Fri, 23 Oct 2026 17:26:53 +0200 Subject: [PATCH] M3: All tools search finds menu commands; Escape stays with the palette Local CodeRabbit review follow-ups for the expanded All tools view: - The search field promises tools and commands, but an entry with a menu (Watermark: Add, Update, Remove) matched only by its own name, so "remove watermark" found nothing. Entries now also match by their menu commands' names, in English or the UI language. - With the command palette open over the view, Escape closed the view as well as the palette. The palette now owns Escape, as dialogs do. Each new test was seen to fail with its fix reverted. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/all_tools_ui.rs | 19 +++++++++++++++---- crates/ui-egui/tests/it/ui.rs | 11 +++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/all_tools_ui.rs b/crates/ui-egui/src/all_tools_ui.rs index 53aa7ebbd7..c8c510a11c 100644 --- a/crates/ui-egui/src/all_tools_ui.rs +++ b/crates/ui-egui/src/all_tools_ui.rs @@ -25,14 +25,19 @@ fn label_hit(label: &str, translated: &str, query: &str, rtl: bool) -> bool { /// Whether `group` matches `query` (already trimmed and lower-cased), in English or the UI /// language. `None` is no match; otherwise the items that matched, empty when the tool's own -/// name did. +/// name did. An entry with a menu (Watermark: Add, Update, Remove) also matches by its commands' +/// names, so "remove watermark" finds it. fn matches(query: &str, group: &ToolGroup) -> Option> { let rtl = crate::i18n::current().rtl(); let hit = |label: &str| label_hit(label, tl!(label), query, rtl); if query.is_empty() || hit(group.label) { return Some(Vec::new()); } - let items: Vec<_> = group.sections.iter().flat_map(|s| s.items.iter()).map(|i| i.label).filter(|l| hit(l)).collect(); + let action_hit = |id: &&str| { + pdfcraft_engine::commands::command(id).is_some_and(|spec| label_hit(spec.label, &crate::i18n::menu_label(spec.id, spec.label), query, rtl)) + }; + let items: Vec<_> = + group.sections.iter().flat_map(|s| s.items.iter()).filter(|i| hit(i.label) || i.actions.iter().any(action_hit)).map(|i| i.label).collect(); (!items.is_empty()).then_some(items) } @@ -40,8 +45,8 @@ fn matches(query: &str, group: &ToolGroup) -> Option> { /// category's tiles that match the search. Choosing a tile opens that tool's panel. pub fn show(app: &mut PdfCraftApp, ui: &mut egui::Ui) { let t = Tokens::get(ui.ctx()); - // A dialog or prompt over the view owns Escape. - if !app.modal_open() && ui.input(|i| i.key_pressed(egui::Key::Escape)) { + // A dialog, prompt or the command palette over the view owns Escape. + if !app.modal_open() && !app.palette_open && ui.input(|i| i.key_pressed(egui::Key::Escape)) { app.all_tools_full = false; return; } @@ -173,6 +178,12 @@ mod tests { assert_eq!(matches("no such tool anywhere", group("organize")), None); } + #[test] + fn search_finds_an_entry_by_its_menus_commands() { + let found = matches("remove watermark", group("edit")).expect("Watermark's menu has Remove watermark"); + assert!(found.contains(&"Watermark"), "{found:?}"); + } + #[test] fn right_to_left_labels_match_a_query_typed_in_logical_order() { let shown = crate::bidi::display_rtl("تنظيم الصفحات"); diff --git a/crates/ui-egui/tests/it/ui.rs b/crates/ui-egui/tests/it/ui.rs index 056a7c3ddd..52e22017b3 100644 --- a/crates/ui-egui/tests/it/ui.rs +++ b/crates/ui-egui/tests/it/ui.rs @@ -179,6 +179,17 @@ fn escape_in_a_dialog_over_the_expanded_all_tools_leaves_the_view_open() { assert!(h.state().all_tools_full, "Escape belonged to the dialog"); } +#[test] +fn escape_in_the_command_palette_over_the_expanded_all_tools_leaves_the_view_open() { + let mut h = harness(|app| app.set_option("tools", "full").unwrap()); + h.state_mut().palette_open = true; + h.run_steps(3); + h.key_press(egui::Key::Escape); + h.run_steps(3); + assert!(!h.state().palette_open, "Escape closed the palette"); + assert!(h.state().all_tools_full, "and only the palette"); +} + #[test] fn another_workspace_from_the_mode_option_puts_the_expanded_all_tools_away() { let mut app = PdfCraftApp::new();