diff --git a/crates/ui-egui/src/keytips/keytip_table.rs b/crates/ui-egui/src/keytips/keytip_table.rs index d14af141b..5303298bc 100644 --- a/crates/ui-egui/src/keytips/keytip_table.rs +++ b/crates/ui-egui/src/keytips/keytip_table.rs @@ -92,6 +92,25 @@ pub fn item_letter(prefix: &str, target: &str) -> Option<&'static str> { sequences_for(target).into_iter().find(|seq| seq.len() == prefix.len() + 1 && seq.starts_with(prefix)) } +/// True when `prefix` is the group level of a menu control: some of `target`'s sequences finish +/// one key below it (Borders' "HB" before HBA..HBW). The menu opens there, as Excel's does, so its +/// item letters show. +pub fn opens_menu(prefix: &str, target: &str) -> bool { + !prefix.is_empty() && is_group_level(prefix) && sequences_for(target).iter().any(|seq| seq.len() == prefix.len() + 1 && seq.starts_with(prefix)) +} + +/// The badge for a menu item that runs `target` with `params` (compact JSON, as `keytips.json` +/// writes it): its finish sequence one key below the prefix. Items sharing one command (the +/// Borders presets) are told apart by their params. +pub fn item_letter_with(prefix: &str, target: &str, params: &str) -> Option<&'static str> { + SEQUENCES.iter().find_map(|(seq, entry)| match entry { + Entry::Finish(Action::CommandWith(id, p)) if *id == target && *p == params && seq.len() == prefix.len() + 1 && seq.starts_with(prefix) => { + Some(*seq) + } + _ => None, + }) +} + /// Badge sequences for a command control: every sequence for `target` that continues from the /// current prefix (the painter strips the prefix, so the remaining letters show). pub fn control_badges(prefix: &str, target: &str) -> Vec<&'static str> { @@ -197,6 +216,16 @@ mod tests { assert!(group_extension("E", &["dialog:pasteSpecial"]).is_empty(), "the finish letter paints on the menu item"); } + #[test] + fn menu_levels_and_item_letters_with_params() { + assert!(opens_menu("HB", "home.borders")); + assert!(!opens_menu("H", "home.borders"), "the tab level shows the HB badge, not the menu"); + assert!(!opens_menu("", "home.borders")); + assert_eq!(item_letter_with("HB", "home.borders", "{\"preset\":\"all\"}"), Some("HBA")); + assert_eq!(item_letter_with("HB", "home.borders", "{\"preset\":\"bottom\"}"), Some("HBO")); + assert_eq!(item_letter_with("H", "home.borders", "{\"preset\":\"all\"}"), None); + } + #[test] fn item_letter_is_one_level_deep() { assert_eq!(item_letter("HV", "dialog:pasteSpecial"), Some("HVS")); diff --git a/crates/ui-egui/src/ribbon/home.rs b/crates/ui-egui/src/ribbon/home.rs index c061dfac3..34369be51 100644 --- a/crates/ui-egui/src/ribbon/home.rs +++ b/crates/ui-egui/src/ribbon/home.rs @@ -158,7 +158,11 @@ pub(super) fn home(app: &mut SheetApp, ui: &mut Ui) { act(app, "home.borders", json!({"preset": app.grid.last_border.clone()})); } ba.widget_info(|| egui::WidgetInfo::labeled(egui::Role::Button, ui.is_enabled(), "Border presets")); - egui::Popup::menu(&ba).show(|ui| { + // Alt, H, B opens the menu with each item's letter, as Excel's does (#2964); a letter + // or Esc changes the prefix, which closes it again (`ribbon_keys`). + let prefix = app.keytips.prefix().unwrap_or_default().to_owned(); + let keyed = keytip_table::opens_menu(&prefix, "home.borders"); + egui::Popup::menu(&ba).open_memory(popup_command(keyed, ba.clicked())).show(|ui| { for (label, preset) in [ ("Bottom Border", "bottom"), ("Top Border", "top"), @@ -175,13 +179,21 @@ pub(super) fn home(app: &mut SheetApp, ui: &mut Ui) { ("Top and Double Bottom Border", "topDoubleBottom"), ("Inside Borders", "inside"), ] { - if crate::border_preview::preset_button(ui, label, preset).clicked() { + let item = crate::border_preview::preset_button(ui, label, preset); + if let Some(sequence) = keytip_table::item_letter_with(&prefix, "home.borders", &json!({"preset": preset}).to_string()) { + keytip(app, ui, sequence, egui::pos2(item.rect.right() - 10.0, item.rect.center().y)); + } + if item.clicked() { app.grid.last_border = preset.to_string(); act(app, "home.borders", json!({"preset": preset})); } } ui.separator(); - if ui.button("More Borders…").clicked() { + let more = ui.button("More Borders…"); + if let Some(sequence) = keytip_table::item_letter(&prefix, "dialog:formatCells") { + keytip(app, ui, sequence, egui::pos2(more.rect.right() - 10.0, more.rect.center().y)); + } + if more.clicked() { app.open_dialog("formatCells", json!({"tab": "Border"})); } }); diff --git a/crates/ui-egui/tests/keytips.rs b/crates/ui-egui/tests/keytips.rs index b4418018a..68056f9ea 100644 --- a/crates/ui-egui/tests/keytips.rs +++ b/crates/ui-egui/tests/keytips.rs @@ -768,3 +768,29 @@ fn border_keytips_without_a_command_stay_silent() { assert_eq!(borders(&h, "A1"), Default::default(), "H,B,{k:?} must not draw a border"); } } + +/// #2964: Alt, H, B opens the Borders menu (Excel shows it with each item's letter) instead of +/// leaving nothing on screen; a letter then picks its item and the menu closes, and Esc backs out. +#[test] +fn borders_keytip_opens_the_borders_menu() { + let mut h = harness(selected("A1:B2")); + sequence(&mut h, &[Key::H, Key::B], false); + h.run_steps(2); + assert_eq!(h.state().keytips.prefix(), Some("HB")); + assert!(egui::Popup::is_any_open(&h.ctx), "Alt, H, B opens the Borders menu"); + assert!(h.query_by_label("All Borders").is_some(), "the menu's items show"); + letter(&mut h, Key::A, false); + h.run_steps(2); + assert!(!egui::Popup::is_any_open(&h.ctx), "picking an item closes the menu"); + assert_eq!(h.state().keytips.prefix(), None); + assert!(sides(&h, "B2").iter().all(|s| *s != gridcraft_engine::model::BorderStyle::None), "A applied All Borders"); + + let mut h = harness(blank()); + sequence(&mut h, &[Key::H, Key::B], false); + h.run_steps(2); + assert!(egui::Popup::is_any_open(&h.ctx)); + key(&mut h, Key::Escape); + h.run_steps(2); + assert_eq!(h.state().keytips.prefix(), Some("H"), "Esc backs out to the Home letters"); + assert!(!egui::Popup::is_any_open(&h.ctx), "and closes the menu"); +}