diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 9d190083e..762a8c704 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -107,8 +107,9 @@ impl SheetApp { pub fn logic(&mut self, ctx: &egui::Context) { self.ensure_text_width(ctx); // A keytip control request lives one frame: the ribbon consumes it during `ui`, and - // anything that never reached a control (collapsed ribbon, tab switch) is dropped. - self.ui.keytip_invoke = None; + // anything that never reached a control (collapsed ribbon, tab switch) is dropped. One + // made with a switch to the control's tab waits for the frame that draws that tab. + self.ui.keytip_invoke = self.ui.keytip_invoke_next.take(); // Floating pictures' textures go with their workbook, even when no sheet is painted. let open: Vec = self.session.documents().iter().map(|d| d.uid).collect(); floating_pictures::forget_closed(ctx, &open); diff --git a/crates/ui-egui/src/app/keys.rs b/crates/ui-egui/src/app/keys.rs index 3490bd01c..b490d8557 100644 --- a/crates/ui-egui/src/app/keys.rs +++ b/crates/ui-egui/src/app/keys.rs @@ -59,7 +59,18 @@ impl SheetApp { // Route to the ribbon control that owns this keytip: it opens its dropdown or // runs its click handler with the right params (the engine command alone would // be missing them). - keytips::Action::Control(id) => self.ui.keytip_invoke = Some(id), + // + // A control on another tab (a legacy menu key such as Alt+I, S for the Symbol + // gallery) shows its tab first, so the control is drawn and takes the request. + keytips::Action::Control(id) => { + if let Some(tab) = gridcraft_engine::find_command(id).and_then(|c| c.menu.first()).filter(|t| **t != self.ui.ribbon_tab) { + self.run_or_alert("ui.ribbonTab", json!({"tab": tab})); + self.ui.ribbon_collapsed = false; + self.ui.keytip_invoke_next = Some(id); + } else { + self.ui.keytip_invoke = Some(id); + } + } } } } diff --git a/crates/ui-egui/src/keytips/keytip_table/generated.rs b/crates/ui-egui/src/keytips/keytip_table/generated.rs index c3a343671..6c397f26c 100644 --- a/crates/ui-egui/src/keytips/keytip_table/generated.rs +++ b/crates/ui-egui/src/keytips/keytip_table/generated.rs @@ -378,7 +378,7 @@ pub const SEQUENCES: &[(&str, Entry)] = &[ ("IW", Entry::Finish(Action::Command("home.insertSheet"))), // Excel: Insert > Worksheet; Legacy Excel 2003 menu path (#941). ("IF", Entry::Finish(Action::Dialog("insertFunction"))), // Excel: Insert > Function; Legacy Excel 2003 menu path (#941). ("IND", Entry::Finish(Action::Dialog("defineName"))), // Excel: Insert > Name > Define; Legacy Excel 2003 menu path (#941). - ("IS", Entry::Finish(Action::Command("insert.symbol"))), // Excel: Insert > Symbol; Legacy Excel 2003 menu path (#941). + ("IS", Entry::Finish(Action::Control("insert.symbol"))), // Excel: Insert > Symbol; Legacy Excel 2003 menu path (#941). ("ED", Entry::Finish(Action::Dialog("deleteCells"))), // Excel: Edit > Delete; Legacy Excel 2003 menu path (#941). ("EM", Entry::Finish(Action::Dialog("moveSheet"))), // Excel: Edit > Move or Copy Sheet; Legacy Excel 2003 menu path (#941). ("EL", Entry::Finish(Action::Command("home.deleteSheet"))), // Excel: Edit > Delete Sheet; Legacy Excel 2003 menu path (#941). diff --git a/crates/ui-egui/src/keytips/keytips.json b/crates/ui-egui/src/keytips/keytips.json index 5c96d1d9f..174d519eb 100644 --- a/crates/ui-egui/src/keytips/keytips.json +++ b/crates/ui-egui/src/keytips/keytips.json @@ -2626,7 +2626,7 @@ }, { "seq": "IS", - "kind": "command", + "kind": "control", "target": "insert.symbol", "excel": "Insert > Symbol", "note": "Legacy Excel 2003 menu path (#941)." diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 95f861f14..a0637d966 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -102,6 +102,10 @@ pub struct UiState { /// [`keytips::Action::Control`], consumed (and cleared) by the control's render site. #[serde(skip)] pub keytip_invoke: Option<&'static str>, + /// A keytip control request for the next frame: its tab was switched to this frame, so its + /// control is drawn only then. + #[serde(skip)] + pub keytip_invoke_next: Option<&'static str>, /// Interface language. English by default (so tests and headless renders never depend on the /// host's locale); the desktop app starts from [`i18n::Language::system`] when `ui.json` has /// no usable language (see [`i18n::saved_language`]). An unknown saved value reads as English @@ -143,6 +147,7 @@ impl Default for UiState { autosave: false, recent: vec![], keytip_invoke: None, + keytip_invoke_next: None, language: i18n::Language::En, interface_scale: 100, } diff --git a/crates/ui-egui/src/ribbon/insert.rs b/crates/ui-egui/src/ribbon/insert.rs index 595b74657..c347c896a 100644 --- a/crates/ui-egui/src/ribbon/insert.rs +++ b/crates/ui-egui/src/ribbon/insert.rs @@ -213,7 +213,9 @@ pub(super) fn insert(app: &mut SheetApp, ui: &mut Ui) { }); let sym = big_button(ui, Icon::Symbol, "Symbol", "Symbol", true); control_keytip(app, ui, &sym, "insert.symbol"); - egui::Popup::menu(&sym).show(|ui| { + // Opened by its key tip too (Alt+I, S): the gallery picks the symbol, nothing is inserted before. + let forced = invoke(app, "insert.symbol"); + egui::Popup::menu(&sym).open_memory(popup_command(forced, sym.clicked())).show(|ui| { ui.set_width(260.0); ui.horizontal_wrapped(|ui| { for ch in [ diff --git a/crates/ui-egui/tests/it/keytips_generated.rs b/crates/ui-egui/tests/it/keytips_generated.rs index 976e1fdbf..600a36319 100644 --- a/crates/ui-egui/tests/it/keytips_generated.rs +++ b/crates/ui-egui/tests/it/keytips_generated.rs @@ -2800,7 +2800,8 @@ fn keytip_ind_() { fn keytip_is_() { let mut h = harness(blank()); sequence(&mut h, "IS", false); - assert!(h.state().session.journal.iter().any(|(id, _)| id == "insert.symbol"), "IS must journal insert.symbol"); + assert!(h.state().message.is_none() && h.state().toast.is_none(), "IS must route to its control without an error box"); + assert_eq!(h.state().keytips.prefix(), None); } #[test] diff --git a/crates/ui-egui/tests/it/legacy_keytips.rs b/crates/ui-egui/tests/it/legacy_keytips.rs new file mode 100644 index 000000000..fa6ca37ed --- /dev/null +++ b/crates/ui-egui/tests/it/legacy_keytips.rs @@ -0,0 +1,33 @@ +//! Excel 2003 menu access keys that open a ribbon control rather than run a command (#941). + +use egui::os::OperatingSystem; +use egui::{Event, Key, Modifiers}; +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +fn press(h: &mut egui_kittest::Harness<'static, SheetApp>, key: Key) { + let ev = |pressed| Event::Key { key, physical_key: None, pressed, repeat: false, modifiers: Modifiers::NONE }; + h.input_mut().events.extend([ev(true), Event::Text(key.name().to_ascii_lowercase()), ev(false)]); + h.run_steps(2); +} + +#[test] +fn alt_i_s_opens_the_symbol_gallery_and_leaves_the_cell_alone() { + // Insert > Symbol picks a symbol; running insert.symbol with no symbol set the active cell + // to its own text again, re-reading it (a Python cell lost its code, text could turn into a + // number). From the Home tab the keys show the Insert tab's gallery instead. + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "'007"})).unwrap(); + let mut h = crate::common::os_harness(s, OperatingSystem::Nix); + h.run_steps(4); + for key in [Key::F10, Key::I, Key::S] { + press(&mut h, key); + } + h.run_steps(4); + assert!(!h.state().session.journal.iter().any(|(id, _)| id == "insert.symbol"), "nothing is inserted until a symbol is picked"); + assert_eq!(h.state().ui.ribbon_tab, "Insert"); + assert!(h.query_by_label("€").is_some(), "the Symbol gallery is open"); +} diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index f1240dcd6..eedef2148 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -63,6 +63,7 @@ mod interaction; mod interface_scale; mod keyboard_shortcuts; mod keytips_generated; +mod legacy_keytips; mod macro_editor; mod macros_ui; mod menu_hover;