mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-31 15:49:36 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
2 files changed
+26
-4
No files matched your search
@@ -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<Vec<&'static str>> {
|
||||
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<Vec<&'static str>> {
|
||||
/// 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("تنظيم الصفحات");
|
||||
|
||||
@@ -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();
|
||||
|
||||
Reference in new issue
Block a user