From c5a80ff7918dbb169b524b572ddffdf8209cacf8 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sun, 25 Oct 2026 02:13:09 -0400 Subject: [PATCH] UI: Insert Function lets Tab reach the category list The search box asked for the keyboard focus every frame, so Tab moved to the category list for one frame and the focus went back to the search box. The dialog now gives the search box the focus once, when it opens, through the module's take_first_focus, as the other dialogs do. Test: insert_function_lets_tab_reach_the_category_list (the search box has the focus and takes typing; after Tab the category list keeps it over several frames; Space opens the list). --- crates/ui-egui/src/dialogs.rs | 4 +++- crates/ui-egui/src/dialogs/insert_function.rs | 5 ++-- crates/ui-egui/src/dialogs/show.rs | 2 +- crates/ui-egui/tests/it/function_library.rs | 24 +++++++++++++++++++ 4 files changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 6eccc48eb..2bbb2eba2 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -856,7 +856,9 @@ impl Dialog { "insertFunction" => { // A Function Library menu opens it on its own category. let category = p("category").as_str().unwrap_or(crate::function_library::MOST_RECENTLY_USED).to_string(); - Dialog::custom("insertFunction", "Insert Function", json!({"category": category})) + let mut d = Dialog::custom("insertFunction", "Insert Function", json!({"category": category})); + d.focus_first_field = true; + d } "find" => { let mut d = Dialog::custom("find", "Find and Replace", json!({"what": "", "with": "", "within": "sheet", "lookIn": "formulas"})); diff --git a/crates/ui-egui/src/dialogs/insert_function.rs b/crates/ui-egui/src/dialogs/insert_function.rs index a77de272b..6a93530f7 100644 --- a/crates/ui-egui/src/dialogs/insert_function.rs +++ b/crates/ui-egui/src/dialogs/insert_function.rs @@ -1,4 +1,4 @@ -use super::show::ok_cancel; +use super::show::{ok_cancel, take_first_focus}; use super::{Dialog, Json, SheetApp, json, vec2}; /// Insert Function: search names (as the formula language spells them) and descriptions, or pick @@ -10,7 +10,8 @@ pub(super) fn insert_function(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dia ui.horizontal(|ui| { ui.label(language.tr("Search:")); let r = ui.add(egui::TextEdit::singleline(&mut d.search).desired_width(300.0).hint_text(language.tr("e.g. lookup, average, date"))); - r.request_focus(); + // Once only, so Tab can move on to the category list. + take_first_focus(d, &r); }); // A new search (typed, or set through the control channel) looks through every function, as // Excel's does. diff --git a/crates/ui-egui/src/dialogs/show.rs b/crates/ui-egui/src/dialogs/show.rs index 071a94e5b..a2e452fb0 100644 --- a/crates/ui-egui/src/dialogs/show.rs +++ b/crates/ui-egui/src/dialogs/show.rs @@ -464,7 +464,7 @@ fn form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) /// Enter sets the width, as in Excel. /// Keeps asking until the field really has focus: a new window's first frame is an invisible /// sizing pass where a focus request is ignored. -fn take_first_focus(d: &mut Dialog, r: &egui::Response) { +pub(super) fn take_first_focus(d: &mut Dialog, r: &egui::Response) { if !d.focus_first_field { return; } diff --git a/crates/ui-egui/tests/it/function_library.rs b/crates/ui-egui/tests/it/function_library.rs index 3d8a37d5e..dd86f4652 100644 --- a/crates/ui-egui/tests/it/function_library.rs +++ b/crates/ui-egui/tests/it/function_library.rs @@ -239,6 +239,30 @@ fn insert_function_has_its_title_and_a_working_category_list() { assert_ne!(h.state().session.prefs.recent_functions[0], "XLOOKUP"); } +#[test] +fn insert_function_lets_tab_reach_the_category_list() { + use egui::accesskit::Role; + let mut h = formulas_tab(1440.0); + click(&mut h, "Insert Function"); + h.run_steps(3); + let search_focused = |h: &Harness| h.get_all_by_role(Role::TextInput).any(|n| n.is_focused()); + assert!(search_focused(&h), "the search box takes the focus when the dialog opens"); + h.input_mut().events.push(Event::Text("npv".into())); + h.run_steps(2); + assert_eq!(h.state().dialog.as_ref().map(|d| d.search.as_str()), Some("npv")); + // Tab moves on to the category list, and the search box does not take the focus back. + h.input_mut().events.extend([key_event(Key::Tab, true), key_event(Key::Tab, false)]); + h.run_steps(5); + assert!(h.get_by_role(Role::ComboBox).is_focused(), "Tab reaches the category list"); + assert!(!search_focused(&h)); + // The list opens from the keyboard (its Date & Time joins the ribbon's). + let listed = |h: &Harness| h.get_all_by_label("Date & Time").count(); + let before = listed(&h); + h.input_mut().events.extend([key_event(Key::Space, true), key_event(Key::Space, false)]); + h.run_steps(3); + assert_eq!(listed(&h), before + 1, "the list is open"); +} + /// The text shapes the last frame painted, with their left edge and width. fn painted_text(h: &Harness) -> Vec<(String, f32, f32)> { let mut out = Vec::new();