Review fix-up: switch to Home only when the legacy keytip prefix changes

ribbon_keys ran ui.ribbonTab every frame while the prefix was E/O/OC,
forcing the Home tab continuously. Run it once on the prefix change and
add a regression test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Brandon Thomas
2026-10-10 12:14:34 -04:00
co-authored by Claude Opus 5.5
parent cecbd6cfb3
commit 699b95288a
2 changed files with 18 additions and 2 deletions
+4 -2
View File
@@ -519,8 +519,10 @@ impl SheetApp {
// Leave pointer cancellation to the popup so clicking a menu item still works.
egui::Popup::close_all(ctx);
}
// The legacy Edit/Format paths lead to the same existing Home menus.
if matches!(self.keytips.prefix(), Some("E" | "O" | "OC")) {
// The legacy Edit/Format paths lead to the same existing Home menus. Switch once, when
// the prefix changes, not every frame (that would reset a tab the user picked meanwhile).
let after = self.keytips.prefix();
if after != before.as_deref() && matches!(after, Some("E" | "O" | "OC")) {
self.run_or_alert("ui.ribbonTab", json!({"tab": "Home"}));
self.ui.ribbon_collapsed = false;
}
+14
View File
@@ -208,6 +208,20 @@ fn ribbon_and_legacy_autofit_sequences_resize_the_selected_column() {
}
}
#[test]
fn legacy_prefix_switches_to_home_once_not_every_frame() {
let mut h = harness(blank());
h.state_mut().ui.ribbon_tab = "Insert".into();
sequence(&mut h, &[Key::E], false);
assert_eq!(h.state().keytips.prefix(), Some("E"));
assert_eq!(h.state().ui.ribbon_tab, "Home");
// While the prefix stays the same, later frames must not keep forcing the Home tab.
h.state_mut().ui.ribbon_tab = "Insert".into();
h.run_steps(3);
assert_eq!(h.state().keytips.prefix(), Some("E"));
assert_eq!(h.state().ui.ribbon_tab, "Insert");
}
#[test]
fn escape_backs_out_without_clearing_copy_and_f10_toggles_off() {
let mut h = harness(copied_row());