#941: Alt+I, S opens the Symbol gallery instead of re-setting the active cell

Addresses CodeRabbit review on #3250, "Open a symbol picker instead of
re-parsing the cell": the legacy key ran insert.symbol with no symbol,
which set the active cell to its own text again (re-reading it: text
could turn into a number, a Python cell lost its code). It now routes to
the ribbon's Symbol control: a control key tip whose control is on
another tab shows that tab first and hands the request to the next
frame, when the control is drawn, and the Symbol button opens its
gallery on its key tip. Nothing is inserted until a symbol is picked.

Test: legacy_keytips::alt_i_s_opens_the_symbol_gallery_and_leaves_the_cell_alone
(keytips regenerated with cargo xtask keytips).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Hector VillarrealandClaude Opus 5.5 committed 2026-11-03 03:03:58 -06:00
1 parent 3c0749e685
commit 6fcb94f7e3
9 files changed
+61 -7

No files matched your search

+3 -2
View File
@@ -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<u64> = self.session.documents().iter().map(|d| d.uid).collect();
floating_pictures::forget_closed(ctx, &open);
+12 -1
View File
@@ -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);
}
}
}
}
}
@@ -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).
+1 -1
View File
@@ -2626,7 +2626,7 @@
},
{
"seq": "IS",
"kind": "command",
"kind": "control",
"target": "insert.symbol",
"excel": "Insert > Symbol",
"note": "Legacy Excel 2003 menu path (#941)."
+5
View File
@@ -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,
}
+3 -1
View File
@@ -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 [
+2 -1
View File
@@ -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]
+33
View File
@@ -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");
}
+1
View File
@@ -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;