diff --git a/crates/ui-egui/src/keys.rs b/crates/ui-egui/src/keys.rs index 784ee5dbc..394643a50 100644 --- a/crates/ui-egui/src/keys.rs +++ b/crates/ui-egui/src/keys.rs @@ -6,7 +6,7 @@ use std::collections::{HashMap, HashSet}; use std::sync::OnceLock; use std::sync::atomic::{AtomicU8, Ordering}; -use craft_keymap::{Bindable, Binding, Chord, CommandRef, Key, Keymap, Kind, Mods, Overrides, Platform, Policy, Scope, Scopes}; +use craft_keymap::{AssignError, Bindable, Binding, Chord, CommandRef, Key, Keymap, Kind, Mods, Overrides, Platform, Policy, Scope, Scopes}; use serde_json::{Value, json}; use crate::DesignApp; @@ -135,7 +135,15 @@ pub fn note_os(os: egui::os::OperatingSystem) { /// The keymap with the user's overrides, for [`platform`]. pub fn new_keymap(overrides: Overrides) -> Keymap { - Keymap::new(catalogue().to_vec(), Scopes::new().with_context(TEXT_CONTEXT), platform()).with_overrides(overrides) + keymap_for(platform(), overrides) +} + +/// The keymap for `platform`: the system's own chords are reserved, so neither a user edit nor an +/// imported set can take them. +pub fn keymap_for(platform: Platform, overrides: Overrides) -> Keymap { + Keymap::new(catalogue().to_vec(), Scopes::new().with_context(TEXT_CONTEXT), platform) + .with_reserved(craft_keymap::reserved::system(platform)) + .with_overrides(overrides) } /// Every bindable command, in dispatch order: where two share keys the earlier one runs. UI @@ -280,7 +288,7 @@ pub fn set(app: &mut DesignApp, cmd: &CommandRef, shortcut: Option<&str>, policy } Some(sc) => { let binding = Binding::parse(sc).map_err(|e| format!("can't read shortcut `{sc}`: {e}"))?; - let done = app.keymap.set_bindings(cmd, vec![binding], policy).map_err(|e| format!("can't assign `{sc}`: {e}"))?; + let done = app.keymap.set_bindings(cmd, vec![binding], policy).map_err(|e| assign_error(&app.keymap, sc, &e))?; taken = done.taken_from.into_iter().map(|h| h.command).collect(); } } @@ -288,6 +296,15 @@ pub fn set(app: &mut DesignApp, cmd: &CommandRef, shortcut: Option<&str>, policy Ok(taken) } +/// Why the keymap refused `sc`. A chord the system takes is named as the menus show it +/// ("⌘Tab is reserved by the system"). +fn assign_error(keymap: &Keymap, sc: &str, e: &AssignError) -> String { + match e { + AssignError::Reserved(t) => format!("{} is reserved by the system", t.display(keymap.platform())), + e => format!("can't assign `{sc}`: {e}"), + } +} + /// Other commands on the keys `cmd` holds, by command key. pub fn conflicts(keymap: &Keymap, cmd: &CommandRef) -> Vec { let mut out: Vec = Vec::new(); @@ -309,6 +326,59 @@ pub fn label(keymap: &Keymap, key: &str) -> String { mod tests { use super::*; + const PLATFORMS: [Platform; 3] = [Platform::Mac, Platform::Windows, Platform::Linux]; + + /// No default shortcut sits on a chord an operating system takes before the app sees it. + #[test] + fn defaults_avoid_reserved_chords() { + for p in PLATFORMS { + let km = keymap_for(p, Overrides::new()); + let reserved: Vec = km + .catalogue() + .iter() + .flat_map(|b| km.default_bindings(&b.command).into_iter().map(move |d| (b.command.key(), d))) + .filter(|(_, d)| km.is_reserved(&d.trigger.chord)) + .map(|(k, d)| format!("{k} {}", d.trigger)) + .collect(); + assert_eq!(reserved, Vec::::new(), "{p:?}: defaults on reserved chords"); + } + } + + /// A chord the operating system takes, per platform. + fn os_chord(p: Platform) -> &'static str { + match p { + Platform::Mac => "Cmd+Tab", + Platform::Windows => "Alt+F4", + Platform::Linux => "Cmd+Alt+F2", + } + } + + /// Binding a chord the system takes is refused with a message naming it, and a saved override + /// on one leaves the command its default. + #[test] + fn a_reserved_chord_is_refused() { + let cmd = CommandRef::new("app.palette"); + for p in PLATFORMS { + let mut km = keymap_for(p, Overrides::new()); + let before = km.bindings(&cmd); + let sc = os_chord(p); + let binding = Binding::parse(sc).unwrap(); + let shown = binding.trigger.display(p); + let err = km.set_bindings(&cmd, vec![binding], Policy::Steal).unwrap_err(); + assert_eq!(assign_error(&km, sc, &err), format!("{shown} is reserved by the system"), "{p:?}"); + assert_eq!(km.bindings(&cmd), before, "{p:?}"); + let mut saved = Overrides::new(); + saved.insert(cmd.key(), vec![sc.to_string()]); + assert_eq!(keymap_for(p, saved).bindings(&cmd), before, "{p:?}: a saved reserved chord"); + } + // Through `window.setShortcut`, as the Keyboard Shortcuts dialog and the control channel set keys. + let mut app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + let sc = os_chord(platform()); + let r = menus::run_ui(&mut app, "window.setShortcut", &json!({"id": "app.palette", "shortcut": sc, "steal": true})).unwrap(); + assert!(r.unwrap_err().ends_with("is reserved by the system")); + assert_eq!(menus::shortcut_of(&app, "app.palette").as_deref(), Some("Cmd+Enter")); + } + /// The web build follows the operating system in the browser's user agent. #[test] fn browser_user_agent_picks_the_platform() { diff --git a/crates/ui-egui/src/shortcut_import.rs b/crates/ui-egui/src/shortcut_import.rs index f7b30ecac..a4c57d3c4 100644 --- a/crates/ui-egui/src/shortcut_import.rs +++ b/crates/ui-egui/src/shortcut_import.rs @@ -260,11 +260,19 @@ pub fn summary(lang: &str, report: &Value) -> String { let len = |k: &str| report[k].as_array().map_or(0, Vec::len).to_string(); // Rows for keys in contexts DesignCraft lacks belong to items that did match. let unmatched = report["unmatched"].as_array().map_or(0, |rows| rows.iter().filter(|r| r["reason"] != "context").count()); - crate::i18n::tr(lang, "Imported {count} shortcuts from “{set}”; {unmatched} items not matched, {unreadable} unreadable.") + let mut out = crate::i18n::tr(lang, "Imported {count} shortcuts from “{set}”; {unmatched} items not matched, {unreadable} unreadable.") .replace("{count}", &n("imported")) .replace("{set}", report["set"].as_str().unwrap_or_default()) .replace("{unmatched}", &unmatched.to_string()) - .replace("{unreadable}", &len("unreadable")) + .replace("{unreadable}", &len("unreadable")); + let reserved = report["skipped"].as_array().map_or(0, |rows| rows.iter().filter(|r| r["reason"] == "reserved").count()); + if reserved > 0 { + out.push(' '); + out.push_str( + &crate::i18n::tr(lang, "{count} shortcuts are reserved by the system and were left out.").replace("{count}", &reserved.to_string()), + ); + } + out } /// The Keyboard Shortcuts dialog's Import button: a native file picker imports at once and @@ -330,7 +338,7 @@ mod tests { Product area : Tools\r\n\ \tPen Tool --- Default: Shift+Ctrl+F3\r\n\ Product area : File Menu\r\n\ - \tExit --- Default: Ctrl+Alt+F12\r\n\ + \tExit --- Default: Ctrl+Alt+F13\r\n\ garbage line without a separator\r\n"; let r = menus::run_ui(&mut app, "window.importShortcuts", &json!({"text": set})).unwrap().unwrap(); assert_eq!(r["set"], "My Set"); @@ -362,7 +370,7 @@ mod tests { assert_eq!(crate::keys::shortcut(&app.keymap, &crate::keys::panel("swatches")).as_deref(), Some("Cmd+Shift+F2")); assert_eq!(crate::keys::shortcut(&app.keymap, &crate::keys::tool("pen")).as_deref(), Some("Cmd+Shift+F3")); // InDesign's name for a command, as an alias. - assert_eq!(menus::shortcut_of(&app, "app.quit").as_deref(), Some("Cmd+Alt+F12")); + assert_eq!(menus::shortcut_of(&app, "app.quit").as_deref(), Some("Cmd+Alt+F13")); // The numeric keypad key is bound as the main key of the same name, and reported. assert_eq!(r["approximated"][0]["item"], "File > Close", "{r}"); assert_eq!(r["imported"], 7, "{r}"); @@ -371,6 +379,20 @@ mod tests { assert_eq!(menus::shortcut_of(&app, "app.palette").as_deref(), Some("Cmd+Enter")); } + /// A set's key on a chord the system takes is left out and the summary says so. + #[test] + fn a_reserved_chord_in_a_set_is_left_out() { + let mut app = app(); + // Ctrl in a Windows set is ⌘ on a Mac. + let keys = if crate::keys::platform().is_mac() { "Ctrl+Tab" } else { "Alt+F4" }; + let set = format!("Shortcut Set : OS\r\nProduct area : File Menu\r\n\tExit --- Default: {keys}\r\n"); + let r = menus::run_ui(&mut app, "window.importShortcuts", &json!({ "text": set })).unwrap().unwrap(); + assert_eq!(r["skipped"][0]["reason"], "reserved", "{r}"); + assert_eq!(r["imported"], 0, "{r}"); + assert_eq!(menus::shortcut_of(&app, "app.quit"), menus::default_shortcut("app.quit").map(str::to_string)); + assert!(summary("en", &r).ends_with("1 shortcuts are reserved by the system and were left out."), "{}", summary("en", &r)); + } + #[test] fn a_panel_item_shares_its_key_with_the_command_that_opens_the_panel() { let mut app = app();