Fix #487: Keyboard Shortcuts records chords with modifiers

egui reports a modifier's own key press (ShiftLeft, ControlLeft…) as a key event, and the
recorder took the first key press it saw, so pressing Shift+A saved `ShiftLeft` as the chord: a
shortcut that can never fire. The recorder now skips modifier key presses, so the chord completes
on the first other key with the modifiers held (Esc still cancels). A modifier is never a chord's
key in the shortcut parser either, so `shortcuts.set` refuses one, and overrides saved that way by
older versions are dropped on start, giving the default back.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
bflatastic
2026-10-08 18:55:02 +07:00
co-authored by Claude Opus 5.5
parent 4974d658e5
commit 6dc6b8600b
4 changed files with 50 additions and 4 deletions
+36 -2
View File
@@ -515,11 +515,12 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) {
let mut ok = false;
let mut cancel = false;
let recording = d.str("__recording");
// Record a chord (before widgets see the keys).
// Record a chord (before widgets see the keys). Modifiers held down are part of the next key's
// chord, not a chord of their own.
if !recording.is_empty() {
let ev = ctx.input(|i| {
i.events.iter().find_map(|e| match e {
egui::Event::Key { key, pressed: true, modifiers, .. } => Some((*key, *modifiers)),
egui::Event::Key { key, pressed: true, modifiers, .. } if !crate::shortcuts::is_modifier(*key) => Some((*key, *modifiers)),
_ => None,
})
});
@@ -789,6 +790,39 @@ mod tests {
assert_eq!(chord_from_event(Key::K, Modifiers::COMMAND | Modifiers::SHIFT).as_deref(), Some("Cmd+Shift+K"));
assert_eq!(chord_from_event(Key::P, Modifiers::NONE).as_deref(), Some("P"));
assert_eq!(chord_from_event(Key::Escape, Modifiers::NONE), None);
// A modifier's own key press is no chord (#487).
assert_eq!(chord_from_event(Key::ShiftLeft, Modifiers::SHIFT), None);
assert!(normalize("ShiftLeft").is_none() && normalize("Cmd+ControlRight").is_none());
}
#[test]
fn a_modifier_pressed_while_recording_waits_for_the_key() {
let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());
open(&mut app);
app.ui.dialog.as_mut().unwrap().fields.insert("__recording".into(), json!("tool:groupSelection"));
let ctx = egui::Context::default();
theme::install_fonts(&ctx);
let mut press = |key, modifiers| {
let events = vec![egui::Event::Key { key, physical_key: None, pressed: true, repeat: false, modifiers }];
let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| show(&mut app, ui.ctx()));
out.textures_delta.clear();
app.ui.dialog.clone().unwrap()
};
// egui reports Shift's own key press first (#487): the recording goes on.
assert_eq!(press(Key::ShiftLeft, Modifiers::SHIFT).str("__recording"), "tool:groupSelection");
let d = press(Key::A, Modifiers::SHIFT);
assert_eq!(d.str("__recording"), "");
assert_eq!(dialog_overrides(&d).get("tool:groupSelection").map(String::as_str), Some("Shift+A"));
}
#[test]
fn saved_modifier_only_overrides_give_the_default_back() {
let mut ui = UiState::default();
ui.shortcut_overrides.insert("tool:groupSelection".into(), "Shift+ShiftLeft".into());
ui.shortcut_overrides.insert("edit.preferences".into(), String::new());
let ui = ui.sanitized();
assert!(!ui.shortcut_overrides.contains_key("tool:groupSelection"));
assert_eq!(ui.shortcut_overrides.get("edit.preferences").map(String::as_str), Some(""));
}
#[test]
+10 -1
View File
@@ -40,11 +40,20 @@ pub fn parse(s: &str) -> Option<KeyboardShortcut> {
"Backspace" => Key::Backspace,
"Tab" => Key::Tab,
"~" => Key::Backtick,
k => Key::from_name(k)?,
// A modifier alone is never a chord's key: it couldn't fire (#487).
k => Key::from_name(k).filter(|k| !is_modifier(*k))?,
};
Some(KeyboardShortcut::new(m, key))
}
/// The modifier keys, which egui also reports as key presses of their own.
pub(crate) fn is_modifier(key: Key) -> bool {
matches!(
key,
Key::ShiftLeft | Key::ShiftRight | Key::ControlLeft | Key::ControlRight | Key::AltLeft | Key::AltRight | Key::SuperLeft | Key::SuperRight
)
}
/// Every command shortcut in effect (user overrides from Edit → Keyboard Shortcuts win), with the
/// command's params: `{}`, or `{panel}` for the panel shortcuts (`window.panel`).
pub(crate) fn all_shortcuts() -> Vec<(KeyboardShortcut, &'static str, serde_json::Value)> {
+3
View File
@@ -407,6 +407,9 @@ impl UiState {
f.tools.retain(|id| vectorcraft_tools::tool_info(id).is_some());
f.tools.first().is_some_and(|k| seen.insert(k.clone()))
});
// Overrides that can't fire (modifier-only chords recorded by older versions, #487) give
// the default back.
self.shortcut_overrides.retain(|_, c| c.is_empty() || crate::shortcut_editor::normalize(c).is_some());
self
}
}