mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-01 22:56:54 -06:00
#2964: Alt, H, B opens the Borders menu with its item letters
At the "HB" level every Borders sequence (HBA..HBW) shares the prefix, so control_badge drew no badge and nothing opened: after B the screen showed no key tips at all, although the next letter still applied its preset. The Borders menu now opens at its group level, as Excel's does, with each preset's letter (told apart by its params) and More Borders' M. A letter or Esc changes the prefix, which closes the menu as before; clicking the arrow still toggles it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
a85b1aea3a
commit
c55a7e35aa
@@ -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"));
|
||||
|
||||
@@ -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"}));
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user