From 1aa99c04d586198e16099e67785abb59c3f3ab06 Mon Sep 17 00:00:00 2001 From: choupeanut Date: Tue, 27 Oct 2026 12:16:16 +0800 Subject: [PATCH 01/13] feat(ui): customize keyboard shortcuts --- crates/ui-egui/src/chrome.rs | 29 ++- crates/ui-egui/src/keymap.rs | 85 +++++++ crates/ui-egui/src/lib.rs | 6 + crates/ui-egui/src/ribbon.rs | 1 + crates/ui-egui/src/shortcutui.rs | 228 ++++++++++++++++++ .../ui-egui/tests/shortcut_customization.rs | 61 +++++ docs/user-guide/workspace.md | 7 + 7 files changed, 414 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/src/shortcutui.rs create mode 100644 crates/ui-egui/tests/shortcut_customization.rs diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 99c83758d..e2e40d219 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -489,7 +489,7 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { // Behind a dialog (Save changes?, Recover unsaved work?, Command Search…) the keys are the // dialog's: Escape answers it rather than switching tools, and Ctrl+N can't start what it // asks about. - if ctx.memory(|m| m.top_modal_layer()).is_some() { + if ctx.memory(|m| m.top_modal_layer()).is_some() || app.ui.shortcut_editor.capturing() { return; } crate::modelbuilder::shortcuts(app, ctx); @@ -507,7 +507,10 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { // While a text field has focus, it keeps the keys it acts on: those without Ctrl (⌘), // which type text, and undo and redo, whatever they're bound to (#685), as ModelBuilder // leaves them. - if (typing && !b.chord.command) || (text_field && b.chord.is_text_undo()) { + if (typing && !b.chord.command) + || (text_field && b.chord.is_text_undo()) + || (app.session.map().view.first_person && !b.chord.command && !b.chord.alt && !b.chord.shift) + { continue; } // Duplicating needs a selected layout element, and not a text field being typed in; @@ -634,6 +637,8 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { /// The `ui.*` commands Command Search lists: (id, label, ribbon location). pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ + ("ui.customizeShortcuts", "Customize Shortcuts…", "Settings › Customize"), + ("ui.identify", "Identify Features", "Home › Inspect"), ("ui.addData", "Add Data…", "Home › Add"), ("ui.addDelimited", "Add Delimited Text…", "Home › Add › Add Data"), ("ui.addUrl", "Add Data From URL…", "Home › Add › Add Data"), @@ -966,7 +971,11 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { let on = why.is_none(); let mut b = egui::Button::new(egui::RichText::new(label).size(13.0)).frame(false).min_size(vec2(row_w, 26.0)); // The command's shortcut after its location. - let keys = if id.contains(':') { String::new() } else { crate::keymap::shortcut_text(ui.ctx(), app, id, &Value::Null) }; + let keys = if id.contains(':') && !id.starts_with("plugin:") { + String::new() + } else { + crate::keymap::shortcut_text(ui.ctx(), app, id, &Value::Null) + }; let right = match (show_loc, keys.is_empty()) { (true, false) => format!("{loc} {keys}"), (true, true) => loc.clone(), @@ -980,6 +989,20 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { if let Some(why) = why { resp = resp.on_disabled_hover_text(crate::i18n::fmt("Unavailable: {reason}", &[("reason", tl_raw!(why))])); } + let mut customize = false; + if !id.contains(':') || id.starts_with("plugin:") { + resp.context_menu(|ui| { + if ui.button(tl!("Customize shortcut…")).clicked() { + customize = true; + ui.close(); + } + }); + } + if customize { + app.ui.shortcut_editor.open = true; + app.ui.shortcut_editor.filter = id.clone(); + keep = false; + } let enter = on && first && ui.input(|i| i.key_pressed(Key::Enter)); first &= !on; if resp.clicked() || enter { diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 098d2625b..847d5023d 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -7,6 +7,7 @@ use egui::Key; use rustgis_engine::Registry; +use serde::{Deserialize, Serialize}; use serde_json::Value; use crate::RustGisApp; @@ -174,6 +175,17 @@ impl Binding { } } +/// One user change to the defaults, stored in preferences. +#[derive(Serialize, Deserialize)] +struct Change { + keys: String, + command: String, + #[serde(default)] + params: Value, + #[serde(default)] + remove: bool, +} + /// Every shortcut. #[derive(Clone, Debug, PartialEq)] pub struct Keymap { @@ -214,6 +226,57 @@ impl Keymap { map } + /// Apply stored user changes to this version's defaults. + pub fn apply_preferences(&mut self, text: &str) { + if text.len() > 100_000 { + return; + } + let Ok(changes) = serde_json::from_str::>(text) else { return }; + for change in changes.into_iter().take(1_000) { + let Some(chord) = Chord::parse(&change.keys) else { continue }; + if is_clipboard(chord) || change.command.is_empty() || change.command.len() > 200 { + continue; + } + if change.remove { + self.bindings.retain(|b| b.chord != chord || !b.runs(&change.command, &change.params)); + } else { + self.bindings.retain(|b| b.chord != chord); + self.bindings.push(Binding { chord, command: change.command, params: change.params }); + } + } + } + + /// Store differences only, so defaults added by a later version still reach the user. + pub fn preferences(&self, defaults: &Keymap) -> String { + let mut changes = Vec::new(); + for b in &defaults.bindings { + if !self.bindings.contains(b) { + changes.push(Change { keys: b.chord.to_string(), command: b.command.clone(), params: b.params.clone(), remove: true }); + } + } + for b in &self.bindings { + if !defaults.bindings.contains(b) { + changes.push(Change { keys: b.chord.to_string(), command: b.command.clone(), params: b.params.clone(), remove: false }); + } + } + serde_json::to_string(&changes).unwrap_or_default() + } + + /// Assign a chord, replacing its previous assignment. Clipboard chords are reserved. + pub fn assign(&mut self, chord: Chord, command: &str, params: Value) -> bool { + if is_clipboard(chord) || command.is_empty() { + return false; + } + self.bindings.retain(|b| b.chord != chord); + self.bindings.push(Binding { chord, command: command.to_owned(), params }); + true + } + + /// Remove a particular chord. + pub fn remove(&mut self, chord: Chord) { + self.bindings.retain(|b| b.chord != chord); + } + /// The binding a key combination runs. pub fn command_for(&self, chord: Chord) -> Option<&Binding> { self.bindings.iter().find(|b| b.chord == chord) @@ -297,6 +360,28 @@ pub fn label(app: &RustGisApp, command: &str, params: &Value) -> String { mod tests { use super::*; + #[test] + fn shortcut_preferences_round_trip_and_keep_new_defaults() { + let defaults = Keymap::default(); + let mut changed = defaults.clone(); + let table = Chord::parse("Mod+T").unwrap(); + changed.remove(table); + let d = Chord::parse("D").unwrap(); + assert!(changed.assign(d, "tool.set", serde_json::json!({"tool": "select"}))); + assert!(changed.assign(table, "select.clear", Value::Null)); + assert!(!changed.assign(Chord::parse("Mod+C").unwrap(), "select.clear", Value::Null)); + let saved = changed.preferences(&defaults); + let mut newer = defaults.clone(); + let fresh = Chord::parse("F8").unwrap(); + assert!(newer.assign(fresh, "new.command", Value::Null)); + newer.apply_preferences(&saved); + assert_eq!(newer.command_for(d).unwrap().params, serde_json::json!({"tool": "select"})); + assert_eq!(newer.command_for(table).unwrap().command, "select.clear"); + assert_eq!(newer.command_for(fresh).unwrap().command, "new.command"); + newer.apply_preferences("not json"); + assert_eq!(newer.command_for(table).unwrap().command, "select.clear"); + } + /// Undo and redo are a text field's own, an extra Alt or Shift aside; nothing else is. #[test] fn text_undo_chords_are_undo_and_redo() { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index cb98b88e3..005ae7c57 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -67,6 +67,7 @@ mod scriptcolor; mod scriptcomplete; mod searchbox; mod serviceui; +mod shortcutui; mod showme; mod sourcespane; mod sqlbox; @@ -543,6 +544,7 @@ pub struct UiState { pub shortcuts: bool, /// The keyboard shortcuts. pub keymap: keymap::Keymap, + pub(crate) shortcut_editor: shortcutui::Editor, pub tab_strip_rect: Option, /// Where the left and right panes were drawn on the last frame, while they show. pub pane_rects: [Option; 2], @@ -735,6 +737,7 @@ impl Default for UiState { about: false, shortcuts: false, keymap: keymap::Keymap::default(), + shortcut_editor: shortcutui::Editor::default(), tab_strip_rect: None, pane_rects: [None; 2], copy_out: None, @@ -996,6 +999,7 @@ impl RustGisApp { // Both saved database passwords and S3 keys use the same store, installed before prefs. self.session.database_logins.set_store(services.secrets.clone()); self.services = services; + shortcutui::load(&mut self); sourcespane::load_connections(&mut self); autosave::load(&mut self); if let Some(lang) = self.services.load_pref.as_ref().and_then(|load| load("language")) { @@ -2163,6 +2167,7 @@ impl RustGisApp { } } "ui.shortcuts" => self.ui.shortcuts = true, + "ui.customizeShortcuts" => self.ui.shortcut_editor.open = true, // Copy, cut and copy path also put their text on the system clipboard. "ui.copy" | "ui.cut" | "ui.copyPath" => { let cmd = match id { @@ -3727,6 +3732,7 @@ impl RustGisApp { update::show(self, &ctx); showme::tick(self, &ctx); chrome::command_search(self, &ctx); + shortcutui::show(self, &ctx); chrome::about(self, &ctx); #[cfg(not(target_arch = "wasm32"))] agentthreads::windows(self, &ctx); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 4e02bd2aa..a01351fbd 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -3241,6 +3241,7 @@ fn settings_tab(app: &mut RustGisApp, ui: &mut Ui) { } } }); + big(ui, app, "command", "Shortcuts…", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, false, false); let qa = app.ui.quick_access.clone(); big_form_menu(ui, app, "command", "Quick\nAccess", "Choose extra buttons for the Quick Access Toolbar.", move |ui, app| { for (id, label) in QUICK { diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs new file mode 100644 index 000000000..d87d2bc94 --- /dev/null +++ b/crates/ui-egui/src/shortcutui.rs @@ -0,0 +1,228 @@ +//! User customization of keyboard shortcuts. +use crate::{ + RustGisApp, + keymap::{self, Chord, Keymap}, +}; +use egui::{Context, Event}; +use serde_json::{Value, json}; + +const PREF: &str = "shortcut_changes"; + +#[derive(Clone)] +struct Target { + command: String, + params: Value, +} + +#[derive(Default)] +pub struct Editor { + pub open: bool, + pub filter: String, + capture: Option, + pending: Option<(Chord, Target)>, + notice: Option<&'static str>, +} + +impl Editor { + pub fn capturing(&self) -> bool { + self.capture.is_some() || self.pending.is_some() + } +} + +fn save(app: &RustGisApp) { + let defaults = Keymap::new(&app.session.registry); + if let Some(save) = &app.services.save_pref { + save(PREF, &app.ui.keymap.preferences(&defaults)); + } +} + +pub fn load(app: &mut RustGisApp) { + if let Some(changes) = app.services.load_pref.as_ref().and_then(|load| load(PREF)) { + app.ui.keymap.apply_preferences(&changes); + } +} + +fn assign(app: &mut RustGisApp, editor: &mut Editor, chord: Chord, target: Target) { + if keymap::is_clipboard(chord) { + editor.notice = Some("Copy, Cut and Paste keys cannot be reassigned."); + } else if app.ui.keymap.command_for(chord).is_some_and(|b| !b.runs(&target.command, &target.params)) { + editor.pending = Some((chord, target)); + } else if app.ui.keymap.assign(chord, &target.command, target.params) { + save(app); + } +} + +fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { + let mut rows = Vec::new(); + for c in &app.session.registry.commands { + if !c.needs_input() && !matches!(c.id, "edit.copy" | "edit.cut" | "edit.paste") { + rows.push((c.id.to_string(), Value::Null, tl!(c.label).to_string())); + } + } + for &(id, label, _) in crate::chrome::UI_COMMANDS { + rows.push((id.to_string(), Value::Null, tl!(label).to_string())); + } + for c in rustgis_engine::plugins::commands() { + let label = c.plugin.manifest.translate(app.lang().code(), &c.decl.label).to_string(); + rows.push((c.id, Value::Null, label)); + } + for &(tool, label) in keymap::MAP_TOOLS { + rows.push(("tool.set".to_string(), json!({"tool": tool}), crate::i18n::fmt("Map Tool: {tool}", &[("tool", tl_raw!(label))]))); + } + for (command, params, _) in app.ui.keymap.grouped() { + if !rows.iter().any(|(c, p, _)| c == command && p == params) { + rows.push((command.to_string(), params.clone(), keymap::label(app, command, params))); + } + } + rows.sort_by_key(|a| a.2.to_lowercase()); + rows.dedup_by(|a, b| a.0 == b.0 && a.1 == b.1); + rows +} + +/// Draws the editor after the normal map and dialogs. +pub fn show(app: &mut RustGisApp, ctx: &Context) { + let mut editor = std::mem::take(&mut app.ui.shortcut_editor); + if !editor.open { + app.ui.shortcut_editor = editor; + return; + } + let mut open = true; + let mut remove = None; + let mut restore = None; + let mut reset_all = false; + let defaults = Keymap::new(&app.session.registry); + let rows = rows(app); + egui::Window::new(tl!("Customize Shortcuts")).open(&mut open).default_width(620.0).default_height(500.0).resizable(true).show( + ctx, + |ui| { + ui.horizontal(|ui| { + let label = ui.label(tl!("Search commands")); + ui.add_enabled(!editor.capturing(), egui::TextEdit::singleline(&mut editor.filter)).labelled_by(label.id); + if ui.button(tl!("Reset All")).clicked() { + reset_all = true; + } + }); + if cfg!(target_arch = "wasm32") { + ui.label(tl!("Your browser may reserve some shortcuts.")); + } + ui.label(tl!("Single keys are paused during first-person navigation and while typing.")); + if let Some(notice) = editor.notice { + ui.label(tl!(notice)); + } + if editor.capture.is_some() { + ui.horizontal(|ui| { + ui.label(tl!("Press a key combination to assign it.")); + if ui.button(tl!("Cancel")).clicked() { + editor.capture = None; + } + }); + } + let filter = editor.filter.to_lowercase(); + egui::ScrollArea::vertical().auto_shrink([false, false]).show(ui, |ui| { + for (command, params, label) in &rows { + if !filter.is_empty() && !label.to_lowercase().contains(&filter) && !command.to_lowercase().contains(&filter) { + continue; + } + ui.horizontal_wrapped(|ui| { + ui.label(label); + for chord in app.ui.keymap.keys_for(command, params).collect::>() { + if ui + .small_button(format!("{} ×", chord.text(ctx.os().is_mac()))) + .on_hover_text(tl!("Remove shortcut")) + .clicked() + { + remove = Some(chord); + } + } + if ui.small_button(tl!("Add…")).clicked() { + editor.notice = None; + editor.capture = Some(Target { command: command.clone(), params: params.clone() }); + } + for chord in defaults.keys_for(command, params) { + if app.ui.keymap.command_for(chord).is_none_or(|b| !b.runs(command, params)) + && ui.small_button(crate::i18n::fmt("Reset {key}", &[("key", &chord.text(ctx.os().is_mac()))])).clicked() + { + restore = Some((chord, Target { command: command.clone(), params: params.clone() })); + } + } + }); + ui.separator(); + } + }); + }, + ); + if !open { + editor = Editor::default(); + } + if reset_all { + app.ui.keymap = defaults; + editor.capture = None; + editor.pending = None; + editor.notice = None; + save(app); + } + if let Some(chord) = remove { + app.ui.keymap.remove(chord); + save(app); + } + if let Some((chord, target)) = restore { + assign(app, &mut editor, chord, target); + } + if editor.capture.is_some() { + let chord = ctx.input(|i| { + i.events.iter().find_map(|e| match e { + Event::Key { key, pressed: true, repeat: false, modifiers, .. } => { + Some(Chord { command: modifiers.command, alt: modifiers.alt, shift: modifiers.shift, key: *key }) + } + _ => None, + }) + }); + if let Some(chord) = chord + && let Some(target) = editor.capture.take() + { + assign(app, &mut editor, chord, target); + } + } + if let Some((chord, target)) = editor.pending.clone() { + let existing = app.ui.keymap.command_for(chord).map(|b| keymap::label(app, &b.command, &b.params)).unwrap_or_default(); + egui::Modal::new(egui::Id::unique("shortcut_conflict")).show(ctx, |ui| { + ui.heading(tl!("Shortcut already assigned")); + ui.label(crate::i18n::fmt("{key} is used by {command}.", &[("key", &chord.text(ctx.os().is_mac())), ("command", &existing)])); + ui.horizontal(|ui| { + if ui.button(tl!("Replace")).clicked() { + if app.ui.keymap.assign(chord, &target.command, target.params.clone()) { + save(app); + } + editor.pending = None; + } + if ui.button(tl!("Cancel")).clicked() { + editor.pending = None; + } + }); + }); + } + app.ui.shortcut_editor = editor; +} + +#[cfg(test)] +mod tests { + use super::*; + use rustgis_engine::{Project, Session}; + + #[test] + fn conflicting_and_clipboard_keys_are_not_changed_without_confirmation() { + let mut app = RustGisApp::new(Session::new(Project::default())); + let mut editor = Editor::default(); + let target = Target { command: "select.clear".into(), params: Value::Null }; + let chord = Chord::parse("Mod+T").unwrap(); + let before = app.ui.keymap.command_for(chord).unwrap().command.clone(); + assign(&mut app, &mut editor, chord, target.clone()); + assert!(editor.pending.is_some()); + assert_eq!(app.ui.keymap.command_for(chord).unwrap().command, before); + editor.pending = None; + let clipboard = Chord::parse("Mod+C").unwrap(); + assign(&mut app, &mut editor, clipboard, target); + assert!(editor.notice.is_some()); + assert!(app.ui.keymap.command_for(clipboard).is_none()); + } +} diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs new file mode 100644 index 000000000..5b82265c8 --- /dev/null +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -0,0 +1,61 @@ +//! Interaction tests for customizable shortcuts. +use egui_kittest::kittest::Queryable; +use rustgis_engine::{Project, Session}; +use rustgis_ui_egui::{RustGisApp, Services}; +use serde_json::Value; +use std::{ + collections::HashMap, + sync::{Arc, Mutex}, +}; + +fn harness(prefs: Arc>>) -> egui_kittest::Harness<'static, RustGisApp> { + let reads = prefs.clone(); + let writes = prefs; + let services = Services { + load_pref: Some(Box::new(move |key| reads.lock().unwrap().get(key).cloned())), + save_pref: Some(Box::new(move |key, value| { + writes.lock().unwrap().insert(key.to_string(), value.to_string()); + })), + ..Services::default() + }; + let app = RustGisApp::new(Session::new(Project::default())).with_services(services); + egui_kittest::Harness::builder().with_size(egui::vec2(1400.0, 900.0)).with_max_steps(64).build_ui_state( + |ui, app: &mut RustGisApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + app, + ) +} + +#[test] +fn assigns_a_key_confirms_conflicts_and_restores_saved_preferences() { + let prefs = Arc::new(Mutex::new(HashMap::new())); + let mut h = harness(prefs.clone()); + h.state_mut().run("ui.customizeShortcuts", Value::Null).unwrap(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search commands").focus(); + h.event(egui::Event::Text("ui.theme".into())); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press(egui::Key::T); + h.run_steps(3); + assert!(prefs.lock().unwrap().get("shortcut_changes").is_some_and(|p| p.contains("ui.theme"))); + + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press_modifiers(egui::Modifiers::COMMAND, egui::Key::T); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Cancel").click(); + h.run_steps(3); + let before = prefs.lock().unwrap().get("shortcut_changes").cloned().unwrap(); + assert!(!before.contains("Mod+T")); + + let restored = harness(prefs); + assert!(restored.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == egui::Key::T)); + assert!( + restored.state().ui.keymap.bindings.iter().any(|b| b.command == "table.open" && b.chord.key == egui::Key::T && b.chord.command) + ); +} diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 4d9ce0c61..ca367d305 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -156,6 +156,13 @@ Use scripts or the assistant for parameterized commands without an interactive e ## Keyboard shortcuts +**Settings › Customize › Shortcuts** assigns multiple keys to commands and map tools. Click **Add…** +and press the desired key combination; if it is already in use, choose **Replace** or **Cancel**. +Click a displayed key to remove it, **Reset** to restore a default key, or **Reset All** to restore +all defaults. These changes are saved as a user preference on this device. Copy, Cut and Paste stay +fixed. Unmodified keys do not run while typing or during first-person navigation; browsers may +reserve some shortcuts. In Command Search, right-click a command to open it in the editor. + See the [keyboard shortcut reference](index.md#keyboard-shortcuts) for project, map and editing shortcuts, macOS bindings and text-field undo behavior. **Help › Shortcuts** lists the active bindings in the app. Feature editing is described in [Editing](editing.md); layout deletion, From 7a150af8b90b94b518e4339246587c6c17d520d6 Mon Sep 17 00:00:00 2001 From: choupeanut Date: Tue, 27 Oct 2026 13:37:58 +0800 Subject: [PATCH 02/13] fix(ui): address shortcut editor review --- crates/ui-egui/src/chrome.rs | 3 +-- crates/ui-egui/src/keymap.rs | 12 ++++++++++-- crates/ui-egui/src/shortcutui.rs | 31 +++++++++++++++++++++++++++---- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index e2e40d219..855affc97 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -638,7 +638,6 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { /// The `ui.*` commands Command Search lists: (id, label, ribbon location). pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ ("ui.customizeShortcuts", "Customize Shortcuts…", "Settings › Customize"), - ("ui.identify", "Identify Features", "Home › Inspect"), ("ui.addData", "Add Data…", "Home › Add"), ("ui.addDelimited", "Add Delimited Text…", "Home › Add › Add Data"), ("ui.addUrl", "Add Data From URL…", "Home › Add › Add Data"), @@ -990,7 +989,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { resp = resp.on_disabled_hover_text(crate::i18n::fmt("Unavailable: {reason}", &[("reason", tl_raw!(why))])); } let mut customize = false; - if !id.contains(':') || id.starts_with("plugin:") { + if (!id.contains(':') || id.starts_with("plugin:")) && crate::shortcutui::can_customize(app, id) { resp.context_menu(|ui| { if ui.button(tl!("Customize shortcut…")).clicked() { customize = true; diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 847d5023d..42f9627c5 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -231,8 +231,8 @@ impl Keymap { if text.len() > 100_000 { return; } - let Ok(changes) = serde_json::from_str::>(text) else { return }; - for change in changes.into_iter().take(1_000) { + let Ok(changes) = serde_json::from_str::>(text) else { return }; + for change in changes.into_iter().take(1_000).filter_map(|record| serde_json::from_value::(record).ok()) { let Some(chord) = Chord::parse(&change.keys) else { continue }; if is_clipboard(chord) || change.command.is_empty() || change.command.len() > 200 { continue; @@ -382,6 +382,14 @@ mod tests { assert_eq!(newer.command_for(table).unwrap().command, "select.clear"); } + #[test] + fn malformed_preference_records_do_not_hide_valid_bindings() { + let mut keymap = Keymap::default(); + keymap.apply_preferences(r#"[{"keys":"D","command":"select.clear"},{"keys":42},{"keys":"W","command":"table.open"}]"#); + assert_eq!(keymap.command_for(Chord::parse("D").unwrap()).unwrap().command, "select.clear"); + assert_eq!(keymap.command_for(Chord::parse("W").unwrap()).unwrap().command, "table.open"); + } + /// Undo and redo are a text field's own, an extra Alt or Shift aside; nothing else is. #[test] fn text_undo_chords_are_undo_and_redo() { diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index d87d2bc94..4e13f76ea 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -74,14 +74,26 @@ fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { rows.push((command.to_string(), params.clone(), keymap::label(app, command, params))); } } - rows.sort_by_key(|a| a.2.to_lowercase()); + rows.sort_by_cached_key(|a| a.2.to_lowercase()); rows.dedup_by(|a, b| a.0 == b.0 && a.1 == b.1); rows } +/// Whether Command Search can open a matching row in the editor. +pub fn can_customize(app: &RustGisApp, id: &str) -> bool { + if matches!(id, "edit.copy" | "edit.cut" | "edit.paste") { + return false; + } + crate::chrome::UI_COMMANDS.iter().any(|(command, _, _)| *command == id) + || app.session.registry.commands.iter().any(|c| c.id == id && !c.needs_input()) + || rustgis_engine::plugins::commands().iter().any(|c| c.id == id) + || app.ui.keymap.grouped().iter().any(|(command, params, _)| *command == id && params.is_null()) +} + /// Draws the editor after the normal map and dialogs. pub fn show(app: &mut RustGisApp, ctx: &Context) { let mut editor = std::mem::take(&mut app.ui.shortcut_editor); + let was_capturing = editor.capture.is_some(); if !editor.open { app.ui.shortcut_editor = editor; return; @@ -106,8 +118,8 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { ui.label(tl!("Your browser may reserve some shortcuts.")); } ui.label(tl!("Single keys are paused during first-person navigation and while typing.")); - if let Some(notice) = editor.notice { - ui.label(tl!(notice)); + if editor.notice.is_some() { + ui.label(tl!("Copy, Cut and Paste keys cannot be reassigned.")); } if editor.capture.is_some() { ui.horizontal(|ui| { @@ -168,7 +180,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { if let Some((chord, target)) = restore { assign(app, &mut editor, chord, target); } - if editor.capture.is_some() { + if was_capturing && editor.capture.is_some() { let chord = ctx.input(|i| { i.events.iter().find_map(|e| match e { Event::Key { key, pressed: true, repeat: false, modifiers, .. } => { @@ -225,4 +237,15 @@ mod tests { assert!(editor.notice.is_some()); assert!(app.ui.keymap.command_for(clipboard).is_none()); } + + #[test] + fn search_customize_requires_an_editor_row() { + let mut app = RustGisApp::new(Session::new(Project::default())); + assert!(can_customize(&app, "ui.theme")); + assert!(!can_customize(&app, "layer.addData")); + assert!(!can_customize(&app, "edit.copy")); + assert!(!can_customize(&app, "ui.identify")); + assert!(app.ui.keymap.assign(Chord::parse("D").unwrap(), "layer.addData", Value::Null)); + assert!(can_customize(&app, "layer.addData")); + } } From 0bcf9aab86f04d5d47fe6d5975205d5d84f2437f Mon Sep 17 00:00:00 2001 From: choupeanut Date: Tue, 27 Oct 2026 14:48:21 +0800 Subject: [PATCH 03/13] fix(ui): use direct plugin command lookup --- crates/ui-egui/src/shortcutui.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index 4e13f76ea..c1543f36d 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -86,7 +86,7 @@ pub fn can_customize(app: &RustGisApp, id: &str) -> bool { } crate::chrome::UI_COMMANDS.iter().any(|(command, _, _)| *command == id) || app.session.registry.commands.iter().any(|c| c.id == id && !c.needs_input()) - || rustgis_engine::plugins::commands().iter().any(|c| c.id == id) + || rustgis_engine::plugins::command(id).is_some() || app.ui.keymap.grouped().iter().any(|(command, params, _)| *command == id && params.is_null()) } From 67d85f9d9b4f60b5eb561bfa6e7748808a04ef1a Mon Sep 17 00:00:00 2001 From: giswqs <5016453+giswqs@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:52:49 +0000 Subject: [PATCH 04/13] fix(shortcuts): let Escape cancel a shortcut capture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While the shortcut editor waits for a key, a bare Escape now cancels the capture instead of being assigned to the command. Clarify the first-person comment (unmodified keys are paused, Escape included), and name the ribbon button "Customize Shortcuts…" to match Command Search. The editor's notice is a flag, since its text was never read, and the notice string sits inside tl!() where it is drawn. Co-Authored-By: Claude Haiku 5.5 Applied by the claude-autofix workflow. --- crates/ui-egui/src/chrome.rs | 1 + crates/ui-egui/src/ribbon.rs | 2 +- crates/ui-egui/src/shortcutui.rs | 23 +++++++++++++---------- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 855affc97..90ca035cc 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -507,6 +507,7 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { // While a text field has focus, it keeps the keys it acts on: those without Ctrl (⌘), // which type text, and undo and redo, whatever they're bound to (#685), as ModelBuilder // leaves them. + // First-person navigation: W A S D walk, so unmodified keys are paused (Escape and Delete too). if (typing && !b.chord.command) || (text_field && b.chord.is_text_undo()) || (app.session.map().view.first_person && !b.chord.command && !b.chord.alt && !b.chord.shift) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index a01351fbd..d28caa2e8 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -3241,7 +3241,7 @@ fn settings_tab(app: &mut RustGisApp, ui: &mut Ui) { } } }); - big(ui, app, "command", "Shortcuts…", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, false, false); + big(ui, app, "command", "Customize\nShortcuts…", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, false, false); let qa = app.ui.quick_access.clone(); big_form_menu(ui, app, "command", "Quick\nAccess", "Choose extra buttons for the Quick Access Toolbar.", move |ui, app| { for (id, label) in QUICK { diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index c1543f36d..396385ce1 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -20,7 +20,7 @@ pub struct Editor { pub filter: String, capture: Option, pending: Option<(Chord, Target)>, - notice: Option<&'static str>, + notice: bool, } impl Editor { @@ -44,7 +44,7 @@ pub fn load(app: &mut RustGisApp) { fn assign(app: &mut RustGisApp, editor: &mut Editor, chord: Chord, target: Target) { if keymap::is_clipboard(chord) { - editor.notice = Some("Copy, Cut and Paste keys cannot be reassigned."); + editor.notice = true; } else if app.ui.keymap.command_for(chord).is_some_and(|b| !b.runs(&target.command, &target.params)) { editor.pending = Some((chord, target)); } else if app.ui.keymap.assign(chord, &target.command, target.params) { @@ -118,7 +118,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { ui.label(tl!("Your browser may reserve some shortcuts.")); } ui.label(tl!("Single keys are paused during first-person navigation and while typing.")); - if editor.notice.is_some() { + if editor.notice { ui.label(tl!("Copy, Cut and Paste keys cannot be reassigned.")); } if editor.capture.is_some() { @@ -147,7 +147,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { } } if ui.small_button(tl!("Add…")).clicked() { - editor.notice = None; + editor.notice = false; editor.capture = Some(Target { command: command.clone(), params: params.clone() }); } for chord in defaults.keys_for(command, params) { @@ -170,7 +170,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { app.ui.keymap = defaults; editor.capture = None; editor.pending = None; - editor.notice = None; + editor.notice = false; save(app); } if let Some(chord) = remove { @@ -189,10 +189,13 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { _ => None, }) }); - if let Some(chord) = chord - && let Some(target) = editor.capture.take() - { - assign(app, &mut editor, chord, target); + if let Some(chord) = chord { + // A bare Escape backs out, as Cancel does, rather than being assigned. + if chord.key == egui::Key::Escape && !chord.command && !chord.alt && !chord.shift { + editor.capture = None; + } else if let Some(target) = editor.capture.take() { + assign(app, &mut editor, chord, target); + } } } if let Some((chord, target)) = editor.pending.clone() { @@ -234,7 +237,7 @@ mod tests { editor.pending = None; let clipboard = Chord::parse("Mod+C").unwrap(); assign(&mut app, &mut editor, clipboard, target); - assert!(editor.notice.is_some()); + assert!(editor.notice); assert!(app.ui.keymap.command_for(clipboard).is_none()); } From 5e78ae9139199255ffa176e9210f1b09400d9bc1 Mon Sep 17 00:00:00 2001 From: choupeanut Date: Tue, 27 Oct 2026 21:31:27 +0800 Subject: [PATCH 05/13] fix(ui): finish shortcut customization review fixes --- crates/ui-egui/src/chrome.rs | 61 ++++++++++++++++++- crates/ui-egui/src/ribbon.rs | 12 +++- crates/ui-egui/src/shortcutui.rs | 39 ++++++++---- .../ui-egui/tests/shortcut_customization.rs | 49 +++++++++++++++ docs/user-guide/workspace.md | 8 ++- 5 files changed, 151 insertions(+), 18 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index 855affc97..7b1c2fa19 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -509,7 +509,11 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { // leaves them. if (typing && !b.chord.command) || (text_field && b.chord.is_text_undo()) - || (app.session.map().view.first_person && !b.chord.command && !b.chord.alt && !b.chord.shift) + || (app.session.map().view.first_person + && !b.chord.command + && !b.chord.alt + && !b.chord.shift + && !matches!(b.chord.key, Key::Escape | Key::Home | Key::Delete)) { continue; } @@ -1297,6 +1301,61 @@ mod tests { assert_eq!(h.state().session.undo_depth(), depth - 1, "no field focused: the project"); } + #[test] + fn first_person_preserves_default_controls_and_pauses_custom_single_keys() { + let mut app = RustGisApp::with_sample(); + app.session.map_mut().view.first_person = true; + app.run("tool.set", json!({"tool": "select"})).unwrap(); + app.ui.keymap.assign(crate::keymap::Chord::parse("T").unwrap(), "ui.theme", Value::Null); + let before = app.ui.dark; + let mut h = egui_kittest::Harness::builder().build_ui_state( + |ui, app: &mut RustGisApp| { + shortcuts(app, ui.ctx()); + }, + app, + ); + h.key_press(Key::T); + h.run(); + assert_eq!(h.state().ui.dark, before); + h.key_press(Key::Escape); + h.run(); + assert_eq!(h.state().session.tool.code(), "explore"); + // Home and Delete must also reach the keymap while navigating. + for key in ["Home", "Delete"] { + h.state_mut().ui.keymap.assign(crate::keymap::Chord::parse(key).unwrap(), "ui.theme", Value::Null); + let before = h.state().ui.dark; + h.key_press(Key::from_name(key).unwrap()); + h.run(); + assert_ne!(h.state().ui.dark, before); + } + } + + #[test] + fn typing_keeps_shift_and_alt_shortcuts_in_the_text_field() { + use egui_kittest::kittest::Queryable; + let mut app = RustGisApp::with_sample(); + for key in ["Shift+T", "Alt+T"] { + app.ui.keymap.assign(crate::keymap::Chord::parse(key).unwrap(), "ui.theme", Value::Null); + } + let before = app.ui.dark; + let mut h = egui_kittest::Harness::builder().build_ui_state( + |ui, app: &mut RustGisApp| { + shortcuts(app, ui.ctx()); + let label = ui.label("Script"); + ui.text_edit_singleline(&mut app.workflow.script).labelled_by(label.id); + }, + app, + ); + h.run(); + h.get_by_role(egui::accesskit::Role::TextInput).focus(); + h.run(); + for modifiers in [Modifiers::SHIFT, Modifiers::ALT] { + h.key_press_modifiers(modifiers, Key::T); + h.run(); + assert_eq!(h.state().ui.dark, before); + } + } + #[test] fn header_without_a_unified_title_bar_keeps_its_usual_size() { let l = header_layout(None); diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index a01351fbd..39aa35301 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -3241,7 +3241,17 @@ fn settings_tab(app: &mut RustGisApp, ui: &mut Ui) { } } }); - big(ui, app, "command", "Shortcuts…", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, false, false); + big( + ui, + app, + "command", + "Customize\nShortcuts…", + "Customize keyboard shortcuts.", + "ui.customizeShortcuts", + Value::Null, + false, + false, + ); let qa = app.ui.quick_access.clone(); big_form_menu(ui, app, "command", "Quick\nAccess", "Choose extra buttons for the Quick Access Toolbar.", move |ui, app| { for (id, label) in QUICK { diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index c1543f36d..edf458124 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -20,7 +20,7 @@ pub struct Editor { pub filter: String, capture: Option, pending: Option<(Chord, Target)>, - notice: Option<&'static str>, + notice_clipboard: bool, } impl Editor { @@ -44,7 +44,7 @@ pub fn load(app: &mut RustGisApp) { fn assign(app: &mut RustGisApp, editor: &mut Editor, chord: Chord, target: Target) { if keymap::is_clipboard(chord) { - editor.notice = Some("Copy, Cut and Paste keys cannot be reassigned."); + editor.notice_clipboard = true; } else if app.ui.keymap.command_for(chord).is_some_and(|b| !b.runs(&target.command, &target.params)) { editor.pending = Some((chord, target)); } else if app.ui.keymap.assign(chord, &target.command, target.params) { @@ -74,8 +74,9 @@ fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { rows.push((command.to_string(), params.clone(), keymap::label(app, command, params))); } } + let mut seen = std::collections::HashSet::new(); + rows.retain(|(command, params, _)| seen.insert((command.clone(), params.to_string()))); rows.sort_by_cached_key(|a| a.2.to_lowercase()); - rows.dedup_by(|a, b| a.0 == b.0 && a.1 == b.1); rows } @@ -87,7 +88,7 @@ pub fn can_customize(app: &RustGisApp, id: &str) -> bool { crate::chrome::UI_COMMANDS.iter().any(|(command, _, _)| *command == id) || app.session.registry.commands.iter().any(|c| c.id == id && !c.needs_input()) || rustgis_engine::plugins::command(id).is_some() - || app.ui.keymap.grouped().iter().any(|(command, params, _)| *command == id && params.is_null()) + || app.ui.keymap.bindings.iter().any(|b| b.command == id) } /// Draws the editor after the normal map and dialogs. @@ -102,6 +103,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { let mut remove = None; let mut restore = None; let mut reset_all = false; + let mut assign_escape = false; let defaults = Keymap::new(&app.session.registry); let rows = rows(app); egui::Window::new(tl!("Customize Shortcuts")).open(&mut open).default_width(620.0).default_height(500.0).resizable(true).show( @@ -117,13 +119,14 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { if cfg!(target_arch = "wasm32") { ui.label(tl!("Your browser may reserve some shortcuts.")); } - ui.label(tl!("Single keys are paused during first-person navigation and while typing.")); - if editor.notice.is_some() { + ui.label(tl!("While typing, shortcuts without Ctrl/⌘ are paused. First-person navigation keeps Escape, Home and Delete.")); + if editor.notice_clipboard { ui.label(tl!("Copy, Cut and Paste keys cannot be reassigned.")); } if editor.capture.is_some() { ui.horizontal(|ui| { - ui.label(tl!("Press a key combination to assign it.")); + ui.label(tl!("Press a key combination to assign it. Escape cancels.")); + assign_escape = ui.button(tl!("Assign Escape")).clicked(); if ui.button(tl!("Cancel")).clicked() { editor.capture = None; } @@ -147,7 +150,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { } } if ui.small_button(tl!("Add…")).clicked() { - editor.notice = None; + editor.notice_clipboard = false; editor.capture = Some(Target { command: command.clone(), params: params.clone() }); } for chord in defaults.keys_for(command, params) { @@ -170,7 +173,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { app.ui.keymap = defaults; editor.capture = None; editor.pending = None; - editor.notice = None; + editor.notice_clipboard = false; save(app); } if let Some(chord) = remove { @@ -180,7 +183,13 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { if let Some((chord, target)) = restore { assign(app, &mut editor, chord, target); } - if was_capturing && editor.capture.is_some() { + if assign_escape { + if let Some(target) = editor.capture.take() + && let Some(chord) = Chord::parse("Escape") + { + assign(app, &mut editor, chord, target); + } + } else if was_capturing && editor.capture.is_some() { let chord = ctx.input(|i| { i.events.iter().find_map(|e| match e { Event::Key { key, pressed: true, repeat: false, modifiers, .. } => { @@ -192,7 +201,11 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { if let Some(chord) = chord && let Some(target) = editor.capture.take() { - assign(app, &mut editor, chord, target); + if chord == (Chord { command: false, alt: false, shift: false, key: egui::Key::Escape }) { + editor.capture = None; + } else { + assign(app, &mut editor, chord, target); + } } } if let Some((chord, target)) = editor.pending.clone() { @@ -234,7 +247,7 @@ mod tests { editor.pending = None; let clipboard = Chord::parse("Mod+C").unwrap(); assign(&mut app, &mut editor, clipboard, target); - assert!(editor.notice.is_some()); + assert!(editor.notice_clipboard); assert!(app.ui.keymap.command_for(clipboard).is_none()); } @@ -245,7 +258,7 @@ mod tests { assert!(!can_customize(&app, "layer.addData")); assert!(!can_customize(&app, "edit.copy")); assert!(!can_customize(&app, "ui.identify")); - assert!(app.ui.keymap.assign(Chord::parse("D").unwrap(), "layer.addData", Value::Null)); + assert!(app.ui.keymap.assign(Chord::parse("D").unwrap(), "layer.addData", json!({"path": "example.geojson"}))); assert!(can_customize(&app, "layer.addData")); } } diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs index 5b82265c8..e82ccd1c6 100644 --- a/crates/ui-egui/tests/shortcut_customization.rs +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -59,3 +59,52 @@ fn assigns_a_key_confirms_conflicts_and_restores_saved_preferences() { restored.state().ui.keymap.bindings.iter().any(|b| b.command == "table.open" && b.chord.key == egui::Key::T && b.chord.command) ); } + +#[test] +fn escape_cancels_capture_but_can_be_assigned_explicitly_and_reset() { + let prefs = Arc::new(Mutex::new(HashMap::new())); + let mut h = harness(prefs.clone()); + h.state_mut().run("ui.customizeShortcuts", Value::Null).unwrap(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search commands").focus(); + h.event(egui::Event::Text("ui.theme".into())); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").focus(); + h.run_steps(3); + h.key_press(egui::Key::Enter); + h.step(); + h.key_press(egui::Key::Escape); + h.run_steps(3); + assert!(h.query_by_label("Assign Escape").is_none()); + assert!(prefs.lock().unwrap().get("shortcut_changes").is_none()); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Assign Escape").click(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Replace").click(); + h.run_steps(3); + assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == egui::Key::Escape)); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Reset All").click(); + h.run_steps(3); + assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "tool.set" && b.chord.key == egui::Key::Escape)); + assert!(!h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme")); + assert_eq!(prefs.lock().unwrap().get("shortcut_changes").unwrap(), "[]"); +} + +#[test] +fn command_search_context_menu_opens_matching_editor_row() { + let mut h = harness(Arc::new(Mutex::new(HashMap::new()))); + h.state_mut().run("ui.commandSearch", Value::Null).unwrap(); + h.run_steps(3); + h.event(egui::Event::Text("Toggle Dark Mode".into())); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Toggle Dark Mode View").click_secondary(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Customize shortcut…").click(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press(egui::Key::T); + h.run_steps(3); + assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == egui::Key::T)); +} diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index ca367d305..d30a3dc38 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -156,11 +156,13 @@ Use scripts or the assistant for parameterized commands without an interactive e ## Keyboard shortcuts -**Settings › Customize › Shortcuts** assigns multiple keys to commands and map tools. Click **Add…** -and press the desired key combination; if it is already in use, choose **Replace** or **Cancel**. +**Settings › Customize › Customize Shortcuts** assigns multiple keys to commands and map tools. +Click **Add…** and press the desired key combination. Escape cancels; **Assign Escape** binds Escape. +If the combination is already in use, choose **Replace** or **Cancel**. Click a displayed key to remove it, **Reset** to restore a default key, or **Reset All** to restore all defaults. These changes are saved as a user preference on this device. Copy, Cut and Paste stay -fixed. Unmodified keys do not run while typing or during first-person navigation; browsers may +fixed. While typing, shortcuts without Ctrl/⌘ are paused, including Shift and Alt combinations. During +first-person navigation, unmodified shortcuts are paused except Escape, Home and Delete. Browsers may reserve some shortcuts. In Command Search, right-click a command to open it in the editor. See the [keyboard shortcut reference](index.md#keyboard-shortcuts) for project, map and editing From dedb10d7c2ff0f678af029fe252f49b7d076ca9e Mon Sep 17 00:00:00 2001 From: choupeanut Date: Wed, 28 Oct 2026 07:14:36 +0800 Subject: [PATCH 06/13] fix(ui): preserve rejected shortcut preferences and filter web commands --- ROADMAP.md | 3 +- crates/ui-egui/src/chrome.rs | 9 +++- crates/ui-egui/src/keymap.rs | 46 +++++++++++++++---- crates/ui-egui/src/lib.rs | 3 ++ crates/ui-egui/src/shortcutui.rs | 38 ++++++++++++--- .../ui-egui/tests/shortcut_customization.rs | 32 ++++++++++++- docs/user-guide/workspace.md | 7 ++- 7 files changed, 115 insertions(+), 23 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 24d95fd13..0d324cdb1 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -39,6 +39,8 @@ comment on its issue or pull request and open a draft pull request early. - Georeferencer: side-by-side window, transform picker, more formats ([#609](https://github.com/opengeos/RustGIS/issues/609)). +- Rebindable keyboard shortcuts ([#562](https://github.com/opengeos/RustGIS/issues/562)). + ## Next ### Fixes and quick wins @@ -154,7 +156,6 @@ built-in version must justify itself on one of those. only used by tests ([#557](https://github.com/opengeos/RustGIS/issues/557)). - A catalog of data cubes for the time slider ([#683](https://github.com/opengeos/RustGIS/issues/683)). -- Rebindable keyboard shortcuts ([#562](https://github.com/opengeos/RustGIS/issues/562)). ## Exploratory Open questions rather than plans; discuss before building. diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index d01def4f0..f32a0e4c8 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -640,6 +640,11 @@ pub fn shortcuts(app: &mut RustGisApp, ctx: &egui::Context) { } } +/// Whether a UI command is available on the current platform. +pub(crate) fn command_available(id: &str, web: bool) -> bool { + !web || !matches!(id, "ui.checkUpdate" | "ui.mcpRelay") +} + /// The `ui.*` commands Command Search lists: (id, label, ribbon location). pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ ("ui.customizeShortcuts", "Customize Shortcuts…", "Settings › Customize"), @@ -951,7 +956,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { for &(id, label, loc) in UI_COMMANDS { // The web app always serves the latest release (no Check for Updates button) and is // one process a loopback relay could not reach (`ui.mcpRelay` is desktop only). - if matches!(id, "ui.checkUpdate" | "ui.mcpRelay") && cfg!(target_arch = "wasm32") { + if !command_available(id, cfg!(target_arch = "wasm32")) { continue; } if ql.is_empty() || label.to_lowercase().contains(&ql) || tl!(label).to_lowercase().contains(&ql) { @@ -996,7 +1001,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { let mut customize = false; if (!id.contains(':') || id.starts_with("plugin:")) && crate::shortcutui::can_customize(app, id) { resp.context_menu(|ui| { - if ui.button(tl!("Customize shortcut…")).clicked() { + if ui.button(tl!("Customize Shortcuts…")).clicked() { customize = true; ui.close(); } diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 4fa37752b..14f49e9c6 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -12,6 +12,9 @@ use serde_json::Value; use crate::RustGisApp; +const MAX_PREF_BYTES: usize = 100_000; +const MAX_PREF_CHANGES: usize = 1_000; + /// The default shortcuts: (keys, command, parameters as JSON, or empty for none). Clipboard /// keys are not here: they arrive as copy, cut and paste events, not key presses. const DEFAULTS: &[(&str, &str, &str)] = &[ @@ -227,12 +230,15 @@ impl Keymap { } /// Apply stored user changes to this version's defaults. - pub fn apply_preferences(&mut self, text: &str) { - if text.len() > 100_000 { - return; + pub fn apply_preferences(&mut self, text: &str) -> bool { + if text.len() > MAX_PREF_BYTES { + return false; + } + let Ok(changes) = serde_json::from_str::>(text) else { return false }; + if changes.len() > MAX_PREF_CHANGES { + return false; } - let Ok(changes) = serde_json::from_str::>(text) else { return }; - for change in changes.into_iter().take(1_000).filter_map(|record| serde_json::from_value::(record).ok()) { + for change in changes.into_iter().filter_map(|record| serde_json::from_value::(record).ok()) { let Some(chord) = Chord::parse(&change.keys) else { continue }; if is_clipboard(chord) || change.command.is_empty() || change.command.len() > 200 { continue; @@ -244,10 +250,11 @@ impl Keymap { self.bindings.push(Binding { chord, command: change.command, params: change.params }); } } + true } - /// Store differences only, so defaults added by a later version still reach the user. - pub fn preferences(&self, defaults: &Keymap) -> String { + /// Store differences only; return None if they would exceed the load limits. + pub fn preferences(&self, defaults: &Keymap) -> Option { let mut changes = Vec::new(); for b in &defaults.bindings { if !self.bindings.contains(b) { @@ -259,7 +266,10 @@ impl Keymap { changes.push(Change { keys: b.chord.to_string(), command: b.command.clone(), params: b.params.clone(), remove: false }); } } - serde_json::to_string(&changes).unwrap_or_default() + if changes.len() > MAX_PREF_CHANGES { + return None; + } + serde_json::to_string(&changes).ok().filter(|text| text.len() <= MAX_PREF_BYTES) } /// Assign a chord, replacing its previous assignment. Clipboard chords are reserved. @@ -360,6 +370,22 @@ pub fn label(app: &RustGisApp, command: &str, params: &Value) -> String { mod tests { use super::*; + #[test] + fn over_limit_preferences_are_rejected_without_partial_changes() { + let defaults = Keymap::default(); + let too_many = + serde_json::to_string(&vec![serde_json::json!({"keys":"D","command":"select.clear"}); MAX_PREF_CHANGES + 1]).unwrap(); + let too_large = " ".repeat(MAX_PREF_BYTES + 1); + for saved in [&too_many, &too_large] { + let mut keymap = defaults.clone(); + assert!(!keymap.apply_preferences(saved)); + assert_eq!(keymap, defaults); + } + let mut changed = defaults.clone(); + changed.assign(Chord::parse("D").unwrap(), "test.command", Value::String("x".repeat(MAX_PREF_BYTES))); + assert!(changed.preferences(&defaults).is_none(), "never save a value that cannot be loaded"); + } + #[test] fn removed_registry_shortcut_stays_removed_after_preference_reload() { let registry = @@ -369,7 +395,7 @@ mod tests { assert_eq!(defaults.command_for(chord).unwrap().command, "test.action"); let mut changed = defaults.clone(); changed.remove(chord); - let saved = changed.preferences(&defaults); + let saved = changed.preferences(&defaults).unwrap(); let mut reloaded = Keymap::new(®istry); reloaded.apply_preferences(&saved); assert!(reloaded.command_for(chord).is_none()); @@ -385,7 +411,7 @@ mod tests { assert!(changed.assign(d, "tool.set", serde_json::json!({"tool": "select"}))); assert!(changed.assign(table, "select.clear", Value::Null)); assert!(!changed.assign(Chord::parse("Mod+C").unwrap(), "select.clear", Value::Null)); - let saved = changed.preferences(&defaults); + let saved = changed.preferences(&defaults).unwrap(); let mut newer = defaults.clone(); let fresh = Chord::parse("F8").unwrap(); assert!(newer.assign(fresh, "new.command", Value::Null)); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index ea03e18f8..4526feda6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -546,6 +546,8 @@ pub struct UiState { /// The keyboard shortcuts. pub keymap: keymap::Keymap, pub(crate) shortcut_editor: shortcutui::Editor, + /// Preserve unreadable saved shortcuts until the user explicitly resets them. + pub(crate) shortcut_preferences_blocked: bool, pub tab_strip_rect: Option, /// Where the left and right panes were drawn on the last frame, while they show. pub pane_rects: [Option; 2], @@ -741,6 +743,7 @@ impl Default for UiState { shortcuts: false, keymap: keymap::Keymap::default(), shortcut_editor: shortcutui::Editor::default(), + shortcut_preferences_blocked: false, tab_strip_rect: None, pane_rects: [None; 2], copy_out: None, diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index 16fb89e0f..b98bd8169 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -29,16 +29,23 @@ impl Editor { } } -fn save(app: &RustGisApp) { +fn save(app: &mut RustGisApp) { + if app.ui.shortcut_preferences_blocked { + return; + } let defaults = Keymap::new(&app.session.registry); - if let Some(save) = &app.services.save_pref { - save(PREF, &app.ui.keymap.preferences(&defaults)); + if let Some(changes) = app.ui.keymap.preferences(&defaults) { + if let Some(save) = &app.services.save_pref { + save(PREF, &changes); + } + } else { + app.ui.shortcut_preferences_blocked = true; } } pub fn load(app: &mut RustGisApp) { if let Some(changes) = app.services.load_pref.as_ref().and_then(|load| load(PREF)) { - app.ui.keymap.apply_preferences(&changes); + app.ui.shortcut_preferences_blocked = !app.ui.keymap.apply_preferences(&changes); } } @@ -52,7 +59,7 @@ fn assign(app: &mut RustGisApp, editor: &mut Editor, chord: Chord, target: Targe } } -fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { +fn rows(app: &RustGisApp, web: bool) -> Vec<(String, Value, String)> { let mut rows = Vec::new(); for c in &app.session.registry.commands { if !c.needs_input() && !matches!(c.id, "edit.copy" | "edit.cut" | "edit.paste") { @@ -74,6 +81,7 @@ fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { rows.push((command.to_string(), params.clone(), keymap::label(app, command, params))); } } + rows.retain(|(id, _, _)| crate::chrome::command_available(id, web)); let mut seen = std::collections::HashSet::new(); rows.retain(|(command, params, _)| seen.insert((command.clone(), params.to_string()))); rows.sort_by_cached_key(|a| a.2.to_lowercase()); @@ -82,7 +90,7 @@ fn rows(app: &RustGisApp) -> Vec<(String, Value, String)> { /// Whether Command Search can open a matching row in the editor. pub fn can_customize(app: &RustGisApp, id: &str) -> bool { - if matches!(id, "edit.copy" | "edit.cut" | "edit.paste") { + if !crate::chrome::command_available(id, cfg!(target_arch = "wasm32")) || matches!(id, "edit.copy" | "edit.cut" | "edit.paste") { return false; } crate::chrome::UI_COMMANDS.iter().any(|(command, _, _)| *command == id) @@ -105,7 +113,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { let mut reset_all = false; let mut assign_escape = false; let defaults = Keymap::new(&app.session.registry); - let rows = rows(app); + let rows = rows(app, cfg!(target_arch = "wasm32")); egui::Window::new(tl!("Customize Shortcuts")).open(&mut open).default_width(620.0).default_height(500.0).resizable(true).show( ctx, |ui| { @@ -116,6 +124,9 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { reset_all = true; } }); + if app.ui.shortcut_preferences_blocked { + ui.label(tl!("Saved shortcuts are unreadable or exceed the limit. Changes apply only to this session; Reset All replaces the saved shortcuts.")); + } if cfg!(target_arch = "wasm32") { ui.label(tl!("Your browser may reserve some shortcuts.")); } @@ -175,6 +186,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { } if reset_all { app.ui.keymap = defaults; + app.ui.shortcut_preferences_blocked = false; editor.capture = None; editor.pending = None; editor.notice_clipboard = false; @@ -238,6 +250,18 @@ mod tests { use super::*; use rustgis_engine::{Project, Session}; + #[test] + fn web_rows_exclude_desktop_commands_even_with_existing_bindings() { + let mut app = RustGisApp::new(Session::new(Project::default())); + for id in ["ui.checkUpdate", "ui.mcpRelay"] { + app.ui.keymap.assign(Chord::parse("F8").unwrap(), id, Value::Null); + assert!(!crate::chrome::command_available(id, true)); + assert!(!rows(&app, true).iter().any(|(command, _, _)| command == id)); + assert!(rows(&app, false).iter().any(|(command, _, _)| command == id)); + } + assert!(crate::chrome::command_available("ui.theme", true)); + } + #[test] fn conflicting_and_clipboard_keys_are_not_changed_without_confirmation() { let mut app = RustGisApp::new(Session::new(Project::default())); diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs index e82ccd1c6..f79982bf9 100644 --- a/crates/ui-egui/tests/shortcut_customization.rs +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -100,7 +100,7 @@ fn command_search_context_menu_opens_matching_editor_row() { h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Toggle Dark Mode View").click_secondary(); h.run_steps(3); - h.get_by_role_and_label(egui::accesskit::Role::Button, "Customize shortcut…").click(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Customize Shortcuts…").click(); h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); h.step(); @@ -108,3 +108,33 @@ fn command_search_context_menu_opens_matching_editor_row() { h.run_steps(3); assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == egui::Key::T)); } + +#[test] +fn unreadable_preferences_survive_edits_until_an_explicit_reset() { + let too_many = serde_json::to_string(&vec![serde_json::json!({"keys":"D","command":"select.clear"}); 1_001]).unwrap(); + for original in ["not json".to_string(), " ".repeat(100_001), too_many] { + let prefs = Arc::new(Mutex::new(HashMap::from([("shortcut_changes".into(), original.clone())]))); + let mut h = harness(prefs.clone()); + h.state_mut().run("ui.customizeShortcuts", Value::Null).unwrap(); + h.run_steps(3); + assert!(h.query_by_label("Saved shortcuts are unreadable or exceed the limit. Changes apply only to this session; Reset All replaces the saved shortcuts.").is_some()); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search commands").focus(); + h.event(egui::Event::Text("ui.theme".into())); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press(egui::Key::T); + h.run_steps(3); + assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme")); + assert_eq!(prefs.lock().unwrap().get("shortcut_changes").unwrap(), &original); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Reset All").click(); + h.run_steps(3); + assert_eq!(prefs.lock().unwrap().get("shortcut_changes").unwrap(), "[]"); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press(egui::Key::T); + h.run_steps(3); + let reloaded = harness(prefs); + assert!(reloaded.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == egui::Key::T)); + } +} diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 1c49a4a5f..47d0d5010 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -164,11 +164,14 @@ Use scripts or the assistant for parameterized commands without an interactive e Click **Add…** and press the desired key combination. Escape cancels; **Assign Escape** binds Escape. If the combination is already in use, choose **Replace** or **Cancel**. Tab, Enter, Space and arrow key bindings can override their usual keyboard navigation actions. -Click a displayed key to remove it, **Reset** to restore a default key, or **Reset All** to restore +Click a displayed key to remove it, **Reset {key}** (for example, **Reset Ctrl+T**) to restore a default key, or **Reset All** to restore all defaults. These changes are saved as a user preference on this device. Copy, Cut and Paste stay fixed. While typing, shortcuts without Ctrl/⌘ are paused, including Shift and Alt combinations. During first-person navigation, unmodified shortcuts are paused except Escape, Home and Delete. Browsers may -reserve some shortcuts. In Command Search, right-click a command to open it in the editor. +reserve some shortcuts. In Command Search, right-click a command and choose **Customize Shortcuts…** +to open it in the editor. If saved preferences are unreadable or exceed the supported limits, the +original value is preserved and edits apply only to the current session. **Reset All** explicitly +replaces the saved shortcuts with the defaults. See the [keyboard shortcut reference](index.md#keyboard-shortcuts) for project, map and editing shortcuts, macOS bindings and text-field undo behavior. **Help › Shortcuts** lists the active From d558db0ecd83b46ebc9f85072f3b9bee3ff9b915 Mon Sep 17 00:00:00 2001 From: choupeanut Date: Wed, 28 Oct 2026 09:52:11 +0800 Subject: [PATCH 07/13] fix(ui): rename shortcut editor to Keyboard Shortcuts --- crates/ui-egui/src/chrome.rs | 4 ++-- crates/ui-egui/src/ribbon.rs | 2 +- crates/ui-egui/src/shortcutui.rs | 2 +- crates/ui-egui/tests/shortcut_customization.rs | 2 +- docs/user-guide/workspace.md | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index f32a0e4c8..cea70d303 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -647,7 +647,7 @@ pub(crate) fn command_available(id: &str, web: bool) -> bool { /// The `ui.*` commands Command Search lists: (id, label, ribbon location). pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ - ("ui.customizeShortcuts", "Customize Shortcuts…", "Settings › Customize"), + ("ui.customizeShortcuts", "Keyboard Shortcuts…", "Settings › Customize"), ("ui.addData", "Add Data…", "Home › Add"), ("ui.addDelimited", "Add Delimited Text…", "Home › Add › Add Data"), ("ui.addUrl", "Add Data From URL…", "Home › Add › Add Data"), @@ -1001,7 +1001,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { let mut customize = false; if (!id.contains(':') || id.starts_with("plugin:")) && crate::shortcutui::can_customize(app, id) { resp.context_menu(|ui| { - if ui.button(tl!("Customize Shortcuts…")).clicked() { + if ui.button(tl!("Keyboard Shortcuts…")).clicked() { customize = true; ui.close(); } diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 39aa35301..e181599c8 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -3245,7 +3245,7 @@ fn settings_tab(app: &mut RustGisApp, ui: &mut Ui) { ui, app, "command", - "Customize\nShortcuts…", + "Keyboard\nShortcuts…", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index b98bd8169..07cb9480a 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -114,7 +114,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { let mut assign_escape = false; let defaults = Keymap::new(&app.session.registry); let rows = rows(app, cfg!(target_arch = "wasm32")); - egui::Window::new(tl!("Customize Shortcuts")).open(&mut open).default_width(620.0).default_height(500.0).resizable(true).show( + egui::Window::new(tl!("Keyboard Shortcuts")).id(egui::Id::unique("shortcut_editor")).open(&mut open).default_width(620.0).default_height(500.0).resizable(true).show( ctx, |ui| { ui.horizontal(|ui| { diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs index f79982bf9..75b3d573e 100644 --- a/crates/ui-egui/tests/shortcut_customization.rs +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -100,7 +100,7 @@ fn command_search_context_menu_opens_matching_editor_row() { h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Toggle Dark Mode View").click_secondary(); h.run_steps(3); - h.get_by_role_and_label(egui::accesskit::Role::Button, "Customize Shortcuts…").click(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Keyboard Shortcuts…").click(); h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); h.step(); diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 47d0d5010..7d407c1cc 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -160,7 +160,7 @@ Use scripts or the assistant for parameterized commands without an interactive e ## Keyboard shortcuts -**Settings › Customize › Customize Shortcuts** assigns multiple keys to commands and map tools. +**Settings › Customize › Keyboard Shortcuts** assigns multiple keys to commands and map tools. Click **Add…** and press the desired key combination. Escape cancels; **Assign Escape** binds Escape. If the combination is already in use, choose **Replace** or **Cancel**. Tab, Enter, Space and arrow key bindings can override their usual keyboard navigation actions. @@ -168,7 +168,7 @@ Click a displayed key to remove it, **Reset {key}** (for example, **Reset Ctrl+T all defaults. These changes are saved as a user preference on this device. Copy, Cut and Paste stay fixed. While typing, shortcuts without Ctrl/⌘ are paused, including Shift and Alt combinations. During first-person navigation, unmodified shortcuts are paused except Escape, Home and Delete. Browsers may -reserve some shortcuts. In Command Search, right-click a command and choose **Customize Shortcuts…** +reserve some shortcuts. In Command Search, right-click a command and choose **Keyboard Shortcuts…** to open it in the editor. If saved preferences are unreadable or exceed the supported limits, the original value is preserved and edits apply only to the current session. **Reset All** explicitly replaces the saved shortcuts with the defaults. From e0394423e98a3a7529fa26a1733ea6744663e351 Mon Sep 17 00:00:00 2001 From: giswqs <5016453+giswqs@users.noreply.github.com> Date: Wed, 28 Oct 2026 03:05:54 +0000 Subject: [PATCH 08/13] fix(shortcuts): keep shortcut capture from reaching the map On macOS, Ctrl+T pressed while capturing was stored as a bare T, since Ctrl is not the command key there. Capture now skips a Ctrl-only press. While a shortcut is being captured, first-person walking and the Escape sketch cancel are paused, so they no longer act on the same keys. Saved shortcuts that cannot be read now block saving over the stored text, instead of being dropped by the next edit. Too many changes to save get their own notice rather than being reported as unreadable. Co-Authored-By: Claude Haiku 5.5 Applied by the claude-autofix workflow. --- crates/ui-egui/src/keymap.rs | 21 +++++++++++++---- crates/ui-egui/src/lib.rs | 3 +++ crates/ui-egui/src/mapview.rs | 5 ++-- crates/ui-egui/src/shortcutui.rs | 40 +++++++++++++++++++++++++------- 4 files changed, 53 insertions(+), 16 deletions(-) diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 14f49e9c6..142019a59 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -229,7 +229,8 @@ impl Keymap { map } - /// Apply stored user changes to this version's defaults. + /// Apply stored user changes to this version's defaults. Returns false if any of them could + /// not be read: the valid ones still apply, but the stored text must not be overwritten. pub fn apply_preferences(&mut self, text: &str) -> bool { if text.len() > MAX_PREF_BYTES { return false; @@ -238,9 +239,18 @@ impl Keymap { if changes.len() > MAX_PREF_CHANGES { return false; } - for change in changes.into_iter().filter_map(|record| serde_json::from_value::(record).ok()) { - let Some(chord) = Chord::parse(&change.keys) else { continue }; + let mut readable = true; + for record in changes { + let Ok(change) = serde_json::from_value::(record) else { + readable = false; + continue; + }; + let Some(chord) = Chord::parse(&change.keys) else { + readable = false; + continue; + }; if is_clipboard(chord) || change.command.is_empty() || change.command.len() > 200 { + readable = false; continue; } if change.remove { @@ -250,7 +260,7 @@ impl Keymap { self.bindings.push(Binding { chord, command: change.command, params: change.params }); } } - true + readable } /// Store differences only; return None if they would exceed the load limits. @@ -426,7 +436,8 @@ mod tests { #[test] fn malformed_preference_records_do_not_hide_valid_bindings() { let mut keymap = Keymap::default(); - keymap.apply_preferences(r#"[{"keys":"D","command":"select.clear"},{"keys":42},{"keys":"W","command":"table.open"}]"#); + let readable = keymap.apply_preferences(r#"[{"keys":"D","command":"select.clear"},{"keys":42},{"keys":"W","command":"table.open"}]"#); + assert!(!readable, "an unreadable record blocks saving over the stored text"); assert_eq!(keymap.command_for(Chord::parse("D").unwrap()).unwrap().command, "select.clear"); assert_eq!(keymap.command_for(Chord::parse("W").unwrap()).unwrap().command, "table.open"); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 4526feda6..bea4b1f48 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -548,6 +548,8 @@ pub struct UiState { pub(crate) shortcut_editor: shortcutui::Editor, /// Preserve unreadable saved shortcuts until the user explicitly resets them. pub(crate) shortcut_preferences_blocked: bool, + /// The shortcut changes are too many to save: they apply only to this session. + pub(crate) shortcut_preferences_full: bool, pub tab_strip_rect: Option, /// Where the left and right panes were drawn on the last frame, while they show. pub pane_rects: [Option; 2], @@ -744,6 +746,7 @@ impl Default for UiState { keymap: keymap::Keymap::default(), shortcut_editor: shortcutui::Editor::default(), shortcut_preferences_blocked: false, + shortcut_preferences_full: false, tab_strip_rect: None, pane_rects: [None; 2], copy_out: None, diff --git a/crates/ui-egui/src/mapview.rs b/crates/ui-egui/src/mapview.rs index 0621c09f9..1d98142e9 100644 --- a/crates/ui-egui/src/mapview.rs +++ b/crates/ui-egui/src/mapview.rs @@ -1720,7 +1720,8 @@ fn finish_area_selection(app: &mut RustGisApp, ui: &Ui, rect: Rect) { fn selection_interact(app: &mut RustGisApp, ui: &Ui, resp: &egui::Response, rect: Rect) { let tool = app.session.tool; - if ui.input(|i| i.key_pressed(egui::Key::Escape)) { + // While a shortcut is being captured, Escape cancels the capture, not the sketch. + if !app.ui.shortcut_editor.capturing() && ui.input(|i| i.key_pressed(egui::Key::Escape)) { app.map.cancel_sketch(); return; } @@ -1996,7 +1997,7 @@ fn interact(app: &mut RustGisApp, ui: &mut Ui, resp: &egui::Response, rect: Rect } } // First-person navigation: W A S D walk, Q and E fly up and down. - if app.session.map().view.first_person && !ui.ctx().egui_wants_keyboard_input() { + if app.session.map().view.first_person && !app.ui.shortcut_editor.capturing() && !ui.ctx().egui_wants_keyboard_input() { let (mut fw, mut rt, mut up) = (0.0, 0.0, 0.0); ui.input(|i| { // Shortcuts (Ctrl+S, Ctrl+E…) aren't steps. diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index 07cb9480a..b7b3a331b 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -35,11 +35,13 @@ fn save(app: &mut RustGisApp) { } let defaults = Keymap::new(&app.session.registry); if let Some(changes) = app.ui.keymap.preferences(&defaults) { + app.ui.shortcut_preferences_full = false; if let Some(save) = &app.services.save_pref { save(PREF, &changes); } } else { - app.ui.shortcut_preferences_blocked = true; + // Too many changes to store: the saved ones stay as they are, and these apply to this session. + app.ui.shortcut_preferences_full = true; } } @@ -99,6 +101,17 @@ pub fn can_customize(app: &RustGisApp, id: &str) -> bool { || app.ui.keymap.bindings.iter().any(|b| b.command == id) } +/// The chord a key press assigns. On a Mac, Ctrl alone is not one: ⌘ is the command key there +/// and Ctrl is a separate modifier, so Ctrl+T is skipped rather than stored as T. +fn captured_chord(event: &Event) -> Option { + match event { + Event::Key { key, pressed: true, repeat: false, modifiers, .. } if !(modifiers.ctrl && !modifiers.command) => { + Some(Chord { command: modifiers.command, alt: modifiers.alt, shift: modifiers.shift, key: *key }) + } + _ => None, + } +} + /// Draws the editor after the normal map and dialogs. pub fn show(app: &mut RustGisApp, ctx: &Context) { let mut editor = std::mem::take(&mut app.ui.shortcut_editor); @@ -126,6 +139,8 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { }); if app.ui.shortcut_preferences_blocked { ui.label(tl!("Saved shortcuts are unreadable or exceed the limit. Changes apply only to this session; Reset All replaces the saved shortcuts.")); + } else if app.ui.shortcut_preferences_full { + ui.label(tl!("Too many shortcut changes to save. Changes apply only to this session.")); } if cfg!(target_arch = "wasm32") { ui.label(tl!("Your browser may reserve some shortcuts.")); @@ -206,14 +221,7 @@ pub fn show(app: &mut RustGisApp, ctx: &Context) { assign(app, &mut editor, chord, target); } } else if was_capturing && editor.capture.is_some() { - let chord = ctx.input(|i| { - i.events.iter().find_map(|e| match e { - Event::Key { key, pressed: true, repeat: false, modifiers, .. } => { - Some(Chord { command: modifiers.command, alt: modifiers.alt, shift: modifiers.shift, key: *key }) - } - _ => None, - }) - }); + let chord = ctx.input(|i| i.events.iter().find_map(captured_chord)); if let Some(chord) = chord && let Some(target) = editor.capture.take() { @@ -279,6 +287,20 @@ mod tests { assert!(app.ui.keymap.command_for(clipboard).is_none()); } + #[test] + fn capture_takes_the_command_key_not_a_bare_ctrl() { + let press = |modifiers: egui::Modifiers| Event::Key { + key: egui::Key::T, + physical_key: None, + pressed: true, + repeat: false, + modifiers, + }; + assert_eq!(captured_chord(&press(egui::Modifiers::COMMAND)), Chord::parse("Mod+T")); + // On a Mac, Ctrl+T is Ctrl alone: it is skipped, not stored as a bare T. + assert_eq!(captured_chord(&press(egui::Modifiers::CTRL)), None); + } + #[test] fn search_customize_requires_an_editor_row() { let mut app = RustGisApp::new(Session::new(Project::default())); From e133a5ce25a4cefdcddb7903a20c2ffc853c38da Mon Sep 17 00:00:00 2001 From: giswqs Date: Tue, 27 Oct 2026 23:14:08 -0400 Subject: [PATCH 09/13] fix(shortcuts): drop the ellipsis from Keyboard Shortcuts The command opens a window, not a dialog asking for more input, so its label reads Keyboard Shortcuts in the ribbon, the Settings menu, Command Search and the docs. --- crates/ui-egui/src/chrome.rs | 4 ++-- crates/ui-egui/src/ribbon.rs | 12 +----------- crates/ui-egui/tests/shortcut_customization.rs | 2 +- docs/user-guide/workspace.md | 2 +- 4 files changed, 5 insertions(+), 15 deletions(-) diff --git a/crates/ui-egui/src/chrome.rs b/crates/ui-egui/src/chrome.rs index d949a39a5..7a211bc3a 100644 --- a/crates/ui-egui/src/chrome.rs +++ b/crates/ui-egui/src/chrome.rs @@ -647,7 +647,7 @@ pub(crate) fn command_available(id: &str, web: bool) -> bool { /// The `ui.*` commands Command Search lists: (id, label, ribbon location). pub(crate) const UI_COMMANDS: &[(&str, &str, &str)] = &[ - ("ui.customizeShortcuts", "Keyboard Shortcuts…", "Settings › Customize"), + ("ui.customizeShortcuts", "Keyboard Shortcuts", "Settings › Customize"), ("ui.addData", "Add Data…", "Home › Add"), ("ui.addDelimited", "Add Delimited Text…", "Home › Add › Add Data"), ("ui.addUrl", "Add Data From URL…", "Home › Add › Add Data"), @@ -1005,7 +1005,7 @@ pub fn command_search(app: &mut RustGisApp, ctx: &egui::Context) { let mut customize = false; if (!id.contains(':') || id.starts_with("plugin:")) && crate::shortcutui::can_customize(app, id) { resp.context_menu(|ui| { - if ui.button(tl!("Keyboard Shortcuts…")).clicked() { + if ui.button(tl!("Keyboard Shortcuts")).clicked() { customize = true; ui.close(); } diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index e30ff7add..d922f3a4a 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -3290,17 +3290,7 @@ fn settings_tab(app: &mut RustGisApp, ui: &mut Ui) { } } }); - big( - ui, - app, - "command", - "Keyboard\nShortcuts…", - "Customize keyboard shortcuts.", - "ui.customizeShortcuts", - Value::Null, - false, - false, - ); + big(ui, app, "command", "Keyboard\nShortcuts", "Customize keyboard shortcuts.", "ui.customizeShortcuts", Value::Null, false, false); let qa = app.ui.quick_access.clone(); big_form_menu(ui, app, "command", "Quick\nAccess", "Choose extra buttons for the Quick Access Toolbar.", move |ui, app| { for (id, label) in QUICK { diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs index 75b3d573e..0034bb0b5 100644 --- a/crates/ui-egui/tests/shortcut_customization.rs +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -100,7 +100,7 @@ fn command_search_context_menu_opens_matching_editor_row() { h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Toggle Dark Mode View").click_secondary(); h.run_steps(3); - h.get_by_role_and_label(egui::accesskit::Role::Button, "Keyboard Shortcuts…").click(); + h.get_by_role_and_label(egui::accesskit::Role::Button, "Keyboard Shortcuts").click(); h.run_steps(3); h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); h.step(); diff --git a/docs/user-guide/workspace.md b/docs/user-guide/workspace.md index 8abac8e36..17b531dc3 100644 --- a/docs/user-guide/workspace.md +++ b/docs/user-guide/workspace.md @@ -178,7 +178,7 @@ Click a displayed key to remove it, **Reset {key}** (for example, **Reset Ctrl+T all defaults. These changes are saved as a user preference on this device. Copy, Cut and Paste stay fixed. While typing, shortcuts without Ctrl/⌘ are paused, including Shift and Alt combinations. During first-person navigation, unmodified shortcuts are paused except Escape, Home and Delete. Browsers may -reserve some shortcuts. In Command Search, right-click a command and choose **Keyboard Shortcuts…** +reserve some shortcuts. In Command Search, right-click a command and choose **Keyboard Shortcuts** to open it in the editor. If saved preferences are unreadable or exceed the supported limits, the original value is preserved and edits apply only to the current session. **Reset All** explicitly replaces the saved shortcuts with the defaults. From 8ca540494d64cbb1dda34f138090a77d5347f712 Mon Sep 17 00:00:00 2001 From: giswqs Date: Tue, 27 Oct 2026 23:14:20 -0400 Subject: [PATCH 10/13] style: cargo fmt --- crates/ui-egui/src/keymap.rs | 3 ++- crates/ui-egui/src/shortcutui.rs | 9 ++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 142019a59..004741ae4 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -436,7 +436,8 @@ mod tests { #[test] fn malformed_preference_records_do_not_hide_valid_bindings() { let mut keymap = Keymap::default(); - let readable = keymap.apply_preferences(r#"[{"keys":"D","command":"select.clear"},{"keys":42},{"keys":"W","command":"table.open"}]"#); + let readable = + keymap.apply_preferences(r#"[{"keys":"D","command":"select.clear"},{"keys":42},{"keys":"W","command":"table.open"}]"#); assert!(!readable, "an unreadable record blocks saving over the stored text"); assert_eq!(keymap.command_for(Chord::parse("D").unwrap()).unwrap().command, "select.clear"); assert_eq!(keymap.command_for(Chord::parse("W").unwrap()).unwrap().command, "table.open"); diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index b7b3a331b..c5af277d5 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -289,13 +289,8 @@ mod tests { #[test] fn capture_takes_the_command_key_not_a_bare_ctrl() { - let press = |modifiers: egui::Modifiers| Event::Key { - key: egui::Key::T, - physical_key: None, - pressed: true, - repeat: false, - modifiers, - }; + let press = + |modifiers: egui::Modifiers| Event::Key { key: egui::Key::T, physical_key: None, pressed: true, repeat: false, modifiers }; assert_eq!(captured_chord(&press(egui::Modifiers::COMMAND)), Chord::parse("Mod+T")); // On a Mac, Ctrl+T is Ctrl alone: it is skipped, not stored as a bare T. assert_eq!(captured_chord(&press(egui::Modifiers::CTRL)), None); From c130c2698496f6413b65f98ec6601f1444851b7f Mon Sep 17 00:00:00 2001 From: giswqs Date: Tue, 27 Oct 2026 23:20:30 -0400 Subject: [PATCH 11/13] Address Claude review feedback - Saving shortcut preferences now rejects empty or over-long command ids, the same per-record check loading applies, so a saved file can always be read back (shared MAX_COMMAND_LEN, with a test). --- crates/ui-egui/src/keymap.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/keymap.rs b/crates/ui-egui/src/keymap.rs index 004741ae4..c2a956b72 100644 --- a/crates/ui-egui/src/keymap.rs +++ b/crates/ui-egui/src/keymap.rs @@ -14,6 +14,7 @@ use crate::RustGisApp; const MAX_PREF_BYTES: usize = 100_000; const MAX_PREF_CHANGES: usize = 1_000; +const MAX_COMMAND_LEN: usize = 200; /// The default shortcuts: (keys, command, parameters as JSON, or empty for none). Clipboard /// keys are not here: they arrive as copy, cut and paste events, not key presses. @@ -249,7 +250,7 @@ impl Keymap { readable = false; continue; }; - if is_clipboard(chord) || change.command.is_empty() || change.command.len() > 200 { + if is_clipboard(chord) || change.command.is_empty() || change.command.len() > MAX_COMMAND_LEN { readable = false; continue; } @@ -276,7 +277,7 @@ impl Keymap { changes.push(Change { keys: b.chord.to_string(), command: b.command.clone(), params: b.params.clone(), remove: false }); } } - if changes.len() > MAX_PREF_CHANGES { + if changes.len() > MAX_PREF_CHANGES || changes.iter().any(|c| c.command.is_empty() || c.command.len() > MAX_COMMAND_LEN) { return None; } serde_json::to_string(&changes).ok().filter(|text| text.len() <= MAX_PREF_BYTES) @@ -394,6 +395,9 @@ mod tests { let mut changed = defaults.clone(); changed.assign(Chord::parse("D").unwrap(), "test.command", Value::String("x".repeat(MAX_PREF_BYTES))); assert!(changed.preferences(&defaults).is_none(), "never save a value that cannot be loaded"); + let mut long_id = defaults.clone(); + long_id.assign(Chord::parse("D").unwrap(), &"x".repeat(MAX_COMMAND_LEN + 1), Value::Null); + assert!(long_id.preferences(&defaults).is_none(), "never save a command id that loading rejects"); } #[test] From 52ef5dc2b243f921109213bd93b7e732767f7719 Mon Sep 17 00:00:00 2001 From: giswqs Date: Tue, 27 Oct 2026 23:42:56 -0400 Subject: [PATCH 12/13] Address review feedback - On a Mac, shortcut capture skips Ctrl+Cmd presses as well as Ctrl alone, so the dropped Ctrl never turns Ctrl+Cmd+T into Cmd+T (CodeRabbit). - Import Shapefile with Projection is left out of the shortcut editor and Command Search's customize entry: its source CRS comes only from its dialog, so a bare shortcut would always fail (CodeRabbit). - A test captures Tab and an arrow key, which the editor must still see after the widgets drawn earlier in the frame (Claude). --- crates/ui-egui/src/shortcutui.rs | 19 +++++++++++++++---- .../ui-egui/tests/shortcut_customization.rs | 17 +++++++++++++++++ 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index c5af277d5..14f503f97 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -61,6 +61,9 @@ fn assign(app: &mut RustGisApp, editor: &mut Editor, chord: Chord, target: Targe } } +/// UI commands that need parameters only their dialog supplies, so a bare shortcut would fail. +const NEEDS_DIALOG: &[&str] = &["ui.importShapefile"]; + fn rows(app: &RustGisApp, web: bool) -> Vec<(String, Value, String)> { let mut rows = Vec::new(); for c in &app.session.registry.commands { @@ -69,7 +72,9 @@ fn rows(app: &RustGisApp, web: bool) -> Vec<(String, Value, String)> { } } for &(id, label, _) in crate::chrome::UI_COMMANDS { - rows.push((id.to_string(), Value::Null, tl!(label).to_string())); + if !NEEDS_DIALOG.contains(&id) { + rows.push((id.to_string(), Value::Null, tl!(label).to_string())); + } } for c in rustgis_engine::plugins::commands() { let label = c.plugin.manifest.translate(app.lang().code(), &c.decl.label).to_string(); @@ -92,7 +97,10 @@ fn rows(app: &RustGisApp, web: bool) -> Vec<(String, Value, String)> { /// Whether Command Search can open a matching row in the editor. pub fn can_customize(app: &RustGisApp, id: &str) -> bool { - if !crate::chrome::command_available(id, cfg!(target_arch = "wasm32")) || matches!(id, "edit.copy" | "edit.cut" | "edit.paste") { + if !crate::chrome::command_available(id, cfg!(target_arch = "wasm32")) + || matches!(id, "edit.copy" | "edit.cut" | "edit.paste") + || NEEDS_DIALOG.contains(&id) + { return false; } crate::chrome::UI_COMMANDS.iter().any(|(command, _, _)| *command == id) @@ -102,10 +110,13 @@ pub fn can_customize(app: &RustGisApp, id: &str) -> bool { } /// The chord a key press assigns. On a Mac, Ctrl alone is not one: ⌘ is the command key there -/// and Ctrl is a separate modifier, so Ctrl+T is skipped rather than stored as T. +/// and Ctrl is a separate modifier, so Ctrl+T and Ctrl+⌘+T are skipped rather than stored as T +/// and ⌘T. Elsewhere `mac_cmd` is never set and `command` mirrors Ctrl. fn captured_chord(event: &Event) -> Option { match event { - Event::Key { key, pressed: true, repeat: false, modifiers, .. } if !(modifiers.ctrl && !modifiers.command) => { + Event::Key { key, pressed: true, repeat: false, modifiers, .. } + if !(modifiers.ctrl && (!modifiers.command || modifiers.mac_cmd)) => + { Some(Chord { command: modifiers.command, alt: modifiers.alt, shift: modifiers.shift, key: *key }) } _ => None, diff --git a/crates/ui-egui/tests/shortcut_customization.rs b/crates/ui-egui/tests/shortcut_customization.rs index 0034bb0b5..c6de2cfc1 100644 --- a/crates/ui-egui/tests/shortcut_customization.rs +++ b/crates/ui-egui/tests/shortcut_customization.rs @@ -60,6 +60,23 @@ fn assigns_a_key_confirms_conflicts_and_restores_saved_preferences() { ); } +#[test] +fn tab_and_arrow_keys_can_be_captured() { + let mut h = harness(Arc::new(Mutex::new(HashMap::new()))); + h.state_mut().run("ui.customizeShortcuts", Value::Null).unwrap(); + h.run_steps(3); + h.get_by_role_and_label(egui::accesskit::Role::TextInput, "Search commands").focus(); + h.event(egui::Event::Text("ui.theme".into())); + h.run_steps(3); + for key in [egui::Key::Tab, egui::Key::ArrowLeft] { + h.get_by_role_and_label(egui::accesskit::Role::Button, "Add…").click(); + h.step(); + h.key_press(key); + h.run_steps(3); + assert!(h.state().ui.keymap.bindings.iter().any(|b| b.command == "ui.theme" && b.chord.key == key), "{key:?} was not captured"); + } +} + #[test] fn escape_cancels_capture_but_can_be_assigned_explicitly_and_reset() { let prefs = Arc::new(Mutex::new(HashMap::new())); From 5e53b61e12851f16af66f3573038f27365314d26 Mon Sep 17 00:00:00 2001 From: giswqs <5016453+giswqs@users.noreply.github.com> Date: Wed, 28 Oct 2026 03:49:01 +0000 Subject: [PATCH 13/13] test(shortcuts): keep Command Search and the editor rows in step Command Search's Keyboard Shortcuts entry uses can_customize, and the editor lists rows() separately, so the two could drift. A new test checks that every editor row can be customized, and that every customizable command has a row. Co-Authored-By: Claude Haiku 5.5 Applied by the claude-autofix workflow. --- crates/ui-egui/src/shortcutui.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/crates/ui-egui/src/shortcutui.rs b/crates/ui-egui/src/shortcutui.rs index 14f503f97..3a2a22286 100644 --- a/crates/ui-egui/src/shortcutui.rs +++ b/crates/ui-egui/src/shortcutui.rs @@ -317,4 +317,22 @@ mod tests { assert!(app.ui.keymap.assign(Chord::parse("D").unwrap(), "layer.addData", json!({"path": "example.geojson"}))); assert!(can_customize(&app, "layer.addData")); } + + /// Command Search's menu entry and the editor's rows must agree: every row can be customized, + /// and every command that can be customized has a row. + #[test] + fn search_customize_agrees_with_the_editor_rows() { + let app = RustGisApp::new(Session::new(Project::default())); + let ids: std::collections::HashSet = rows(&app, false).into_iter().map(|(id, _, _)| id).collect(); + for id in &ids { + assert!(can_customize(&app, id), "{id} has an editor row but Command Search cannot customize it"); + } + let registry = app.session.registry.commands.iter().map(|c| c.id.to_string()); + let ui = crate::chrome::UI_COMMANDS.iter().map(|&(id, _, _)| id.to_string()); + let plugins = rustgis_engine::plugins::commands().into_iter().map(|c| c.id); + let bound = app.ui.keymap.bindings.iter().map(|b| b.command.clone()); + for id in registry.chain(ui).chain(plugins).chain(bound) { + assert_eq!(can_customize(&app, &id), ids.contains(&id), "{id}"); + } + } }