diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index 81fa581e4..96e17336f 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -1,6 +1,6 @@ use super::*; -/// Shows enabled commands and their platform-specific keyboard shortcuts. +/// Shows enabled commands and their shortcuts from engine metadata. pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool) { let (down, up, home, end, pgup, pgdn) = ui.input_mut(|i| { ( @@ -20,15 +20,14 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial d.focus_initialized = true; } - let mac = ui.ctx().os() == egui::os::OperatingSystem::Mac; let q = d.search.to_ascii_lowercase(); - let cmds: Vec<(String, String, String, Option<&str>)> = app + let cmds: Vec<(String, String, String, Option)> = app .session .commands() .into_iter() .filter(|c| c.enabled) .filter(|c| q.is_empty() || c.label.to_ascii_lowercase().contains(&q) || c.id.to_ascii_lowercase().contains(&q)) - .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), crate::widgets::command_shortcut(c.id, mac))) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), c.shortcut.map(str::to_string))) .take(40) .collect(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index a43784930..2b2f40e95 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -15,54 +15,33 @@ fn tr<'a>(ui: &Ui, text: &'a str) -> &'a str { i18n::current(ui.ctx()).tr(text) } -/// Verified shortcut labels shared by Ribbon tooltips and Command Palette. -/// Each entry is (command ID, optional Ribbon tooltip, macOS keys, other OS keys). -/// The dialog ID is included because Insert Link opens a UI dialog directly. -const SHORTCUTS: &[(&str, &str, &str, Option<&str>)] = &[ - ("file.new", "", "⌘N", Some("Ctrl+N")), - ("file.open", "", "⌘O", Some("Ctrl+O")), - ("file.save", "Save", "⌘S", Some("Ctrl+S")), - ("file.saveAs", "", "⌘⇧S", Some("Ctrl+Shift+S")), - ("file.close", "", "⌘W", Some("Ctrl+W")), - ("edit.copy", "Copy", "⌘C", Some("Ctrl+C")), - ("edit.cut", "Cut", "⌘X", Some("Ctrl+X")), - ("edit.paste", "Paste", "⌘V", Some("Ctrl+V")), - ("edit.undo", "Undo", "⌘Z", Some("Ctrl+Z")), - ("edit.redo", "Redo", "⌘Y", Some("Ctrl+Y")), - ("edit.selectAll", "", "⌘A", Some("Ctrl+A")), - ("edit.fillDown", "", "⌘D", Some("Ctrl+D")), - ("edit.fillRight", "", "⌘R", Some("Ctrl+R")), - ("home.bold", "Bold", "⌘B", Some("Ctrl+B")), - ("home.italic", "Italic", "⌘I", Some("Ctrl+I")), - ("home.underline", "Underline", "⌘U", Some("Ctrl+U")), - ("formulas.autoSum", "AutoSum", "⌘⇧T", Some("Alt+=")), - ("insert.table", "Table", "⌘T", Some("Ctrl+T")), - ("dialog:insertLink", "Insert Link", "⌘K", Some("Ctrl+K")), - ("data.filter", "Filter", "⌘⇧F", Some("Ctrl+Shift+L")), - ("edit.flashFill", "Flash Fill", "Control+E", Some("Ctrl+E")), - ("edit.pasteSpecial", "", "Ctrl+Cmd+V", Some("Ctrl+Alt+V")), - ("home.alignLeft", "", "⌘L", None), - ("home.alignCenter", "", "⌘E", None), - ("selection.row", "", "Shift+Space", Some("Shift+Space")), - ("selection.currentRegion", "", "Control+Shift+8", Some("Ctrl+Shift+8")), - ("selection.column", "", "Control+Space", Some("Ctrl+Space")), - ("view.showObjects", "", "Control+6", Some("Ctrl+6")), - ("formulas.showFormulas", "Show Formulas", "⌘`", Some("Ctrl+`")), +/// Keyboard shortcuts shown after a ribbon tooltip's command name: Excel for Mac's on macOS, +/// Excel for Windows' elsewhere (each one GridCraft binds). Only the name is translated. +const TOOLTIP_SHORTCUTS: &[(&str, &str, &str)] = &[ + ("Save", "⌘S", "Ctrl+S"), + ("Undo", "⌘Z", "Ctrl+Z"), + ("Redo", "⌘Y", "Ctrl+Y"), + ("Paste", "⌘V", "Ctrl+V"), + ("Cut", "⌘X", "Ctrl+X"), + ("Copy", "⌘C", "Ctrl+C"), + ("Bold", "⌘B", "Ctrl+B"), + ("Italic", "⌘I", "Ctrl+I"), + ("Underline", "⌘U", "Ctrl+U"), + ("AutoSum", "⌘⇧T", "Alt+="), + ("Table", "⌘T", "Ctrl+T"), + ("Insert Link", "⌘K", "Ctrl+K"), + ("Filter", "⌘⇧F", "Ctrl+Shift+L"), + ("Flash Fill", "⌘E", "Ctrl+E"), + // The Control key, written the way each platform names it. macOS's ⌃ is supplied by its + // system fonts; elsewhere the Windows spelling avoids a glyph no bundled face has. + ("Show Formulas", "⌃`", "Ctrl+`"), ]; -/// Returns a verified platform-specific shortcut for a command ID. -pub fn command_shortcut(id: &str, mac: bool) -> Option<&'static str> { - SHORTCUTS.iter().find(|(command, _, _, _)| *command == id).and_then(|(_, _, mac_keys, other)| if mac { Some(*mac_keys) } else { *other }) +/// The shortcut a ribbon tooltip shows after `tip`, for this platform (`mac`). +pub fn tooltip_shortcut(tip: &str, mac: bool) -> Option<&'static str> { + TOOLTIP_SHORTCUTS.iter().find(|(name, _, _)| *name == tip).map(|(_, m, other)| if mac { *m } else { *other }) } -/// Returns the same shortcut for a Ribbon tooltip label. -pub fn tooltip_shortcut(tip: &str, mac: bool) -> Option<&'static str> { - SHORTCUTS.iter().find(|(_, label, _, _)| !label.is_empty() && *label == tip).and_then( - |(_, _, mac_keys, other)| { - if mac { Some(*mac_keys) } else { *other } - }, - ) -} /// A ribbon tooltip: the translated command name, then its shortcut for this platform, as Excel /// shows them ("Bold (Ctrl+B)" on Windows, "Bold (⌘B)" on macOS). fn tip_text(ui: &Ui, tip: &str) -> String { @@ -735,17 +714,9 @@ mod tooltip_shortcut_tests { assert_eq!(tooltip_shortcut("Filter", true), Some("⌘⇧F")); // Search commands has no shortcut of its own (Ctrl+Shift+Space selects all). assert_eq!(tooltip_shortcut("Search commands", false), None); - for (id, tip, mac, other) in super::SHORTCUTS { - assert!(!mac.is_empty(), "{id}: empty macOS mapping"); - if let Some(keys) = other { - assert!(!keys.contains(['⌘', '⇧', '⌥']), "{id}: {keys}"); - } - assert_eq!(super::command_shortcut(id, true), Some(*mac), "{id}"); - assert_eq!(super::command_shortcut(id, false), *other, "{id}"); - if !tip.is_empty() { - assert_eq!(tooltip_shortcut(tip, true), Some(*mac), "{id}"); - assert_eq!(tooltip_shortcut(tip, false), *other, "{id}"); - } + for (name, mac, other) in super::TOOLTIP_SHORTCUTS { + assert!(!other.contains(['⌘', '⇧', '⌥']), "{name}: {other}"); + assert!(mac.starts_with('⌘') || mac.starts_with('⌃'), "{name}: {mac}"); } } @@ -753,7 +724,7 @@ mod tooltip_shortcut_tests { fn ribbon_tip_text_obeys_runtime_os() { use egui::os::OperatingSystem; - for (os, expected) in [(OperatingSystem::Windows, "Flash Fill (Ctrl+E)"), (OperatingSystem::Mac, "Flash Fill (Control+E)")] { + for (os, expected) in [(OperatingSystem::Windows, "Flash Fill (Ctrl+E)"), (OperatingSystem::Mac, "Flash Fill (⌘E)")] { let ctx = egui::Context::default(); ctx.set_os(os); diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 6aba1c2f3..63850a64e 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -83,87 +83,27 @@ fn command_search_arrow_and_page_keys_move_the_selection_and_keep_the_search_foc assert!(h.ctx.memory(|m| m.focused()).is_some(), "the search field kept focus"); } -/// Checks platform-specific labels for the Bold and Filter shortcuts. +/// The palette displays the engine metadata unchanged on either runtime OS. #[test] -fn command_search_shows_shortcuts_for_current_platform() { - let mut h = harness(); - h.state_mut().open_dialog("commandSearch", json!({})); - h.run_steps(6); - - h.state_mut().dialog.as_mut().unwrap().search = "bold".into(); - h.run_steps(3); - - let bold = if h.ctx.os() == egui::os::OperatingSystem::Mac { "Bold (⌘B)" } else { "Bold (Ctrl+B)" }; - h.get_by_label_contains(bold); - - h.state_mut().dialog.as_mut().unwrap().search = "filter".into(); - h.run_steps(3); - - let filter = if h.ctx.os() == egui::os::OperatingSystem::Mac { "Filter (⌘⇧F)" } else { "Filter (Ctrl+Shift+L)" }; - h.get_by_label_contains(filter); -} - -/// Checks that Paste Special shows a usable shortcut after copying a cell. -#[test] -fn command_search_shows_paste_special_shortcut() { - let mut h = harness(); - - h.state_mut().session.execute("cell.set", json!({"cell": "C3", "input": "copied"})).unwrap(); - h.state_mut().session.execute("edit.copy", json!({"range": "C3"})).unwrap(); - - h.state_mut().open_dialog("commandSearch", json!({})); - h.run_steps(6); - - h.state_mut().dialog.as_mut().unwrap().search = "pasteSpecial".into(); - h.run_steps(3); - - let expected = if h.ctx.os() == egui::os::OperatingSystem::Mac { "Paste Special… (Ctrl+Cmd+V)" } else { "Paste Special… (Ctrl+Alt+V)" }; - - h.get_by_label_contains(expected); -} - -/// Checks both OS mappings on one host, including commands with conflicting keys. -#[test] -fn command_search_uses_runtime_os_and_verified_shortcuts() { +fn command_search_shows_engine_shortcuts() { use egui::os::OperatingSystem; - for (os, bold, filter, flash, paste, left, center) in [ - ( - OperatingSystem::Windows, - "Bold (Ctrl+B)", - "Filter (Ctrl+Shift+L)", - "Flash Fill (Ctrl+E)", - "Paste Special… (Ctrl+Alt+V)", - "Align Left Home", - "Center Home", - ), - ( - OperatingSystem::Mac, - "Bold (⌘B)", - "Filter (⌘⇧F)", - "Flash Fill (Control+E)", - "Paste Special… (Ctrl+Cmd+V)", - "Align Left (⌘L)", - "Center (⌘E)", - ), - ] { + for os in [OperatingSystem::Windows, OperatingSystem::Mac] { let mut h = harness(); h.ctx.set_os(os); h.run_steps(2); - h.state_mut().session.execute("cell.set", json!({"cell": "C3", "input": "copied"})).unwrap(); - h.state_mut().session.execute("edit.copy", json!({"range": "C3"})).unwrap(); - h.state_mut().open_dialog("commandSearch", json!({})); h.run_steps(6); for (query, expected) in [ - ("bold", bold), - ("filter", filter), - ("edit.flashFill", flash), - ("edit.pasteSpecial", paste), - ("home.alignLeft", left), - ("home.alignCenter", center), + ("home.bold", "Bold (Cmd+B)"), + ("data.filter", "Filter (Ctrl+Shift+L)"), + ("edit.flashFill", "Flash Fill (Ctrl+E)"), + ("home.alignLeft", "Align Left (Cmd+L)"), + ("home.alignCenter", "Center (Cmd+E)"), + ("selection.row", "Select Entire Row (Shift+Space)"), + ("selection.column", "Select Entire Column (Ctrl+Space)"), ] { h.state_mut().dialog.as_mut().unwrap().search = query.into(); h.run_steps(3); @@ -171,72 +111,59 @@ fn command_search_uses_runtime_os_and_verified_shortcuts() { } } } -/// Selection shortcuts use the same bindings as the runtime keyboard handler. + +/// Paste Special uses the engine's shortcut metadata when enabled. #[test] -fn command_search_selection_shortcuts_match_runtime_os() { - for (os, column_label) in [ - (egui::os::OperatingSystem::Windows, "Select Entire Column (Ctrl+Space)"), - (egui::os::OperatingSystem::Mac, "Select Entire Column (Control+Space)"), - ] { +fn command_search_shows_paste_special_metadata() { + use egui::os::OperatingSystem; + + for os in [OperatingSystem::Windows, OperatingSystem::Mac] { let mut h = harness(); h.ctx.set_os(os); h.run_steps(2); + h.state_mut().session.execute("cell.set", json!({"cell": "C3", "input": "copied"})).unwrap(); + + h.state_mut().session.execute("edit.copy", json!({"range": "C3"})).unwrap(); + h.state_mut().open_dialog("commandSearch", json!({})); h.run_steps(6); - h.state_mut().dialog.as_mut().unwrap().search = "selection.row".into(); + h.state_mut().dialog.as_mut().unwrap().search = "edit.pasteSpecial".into(); h.run_steps(3); - h.get_by_label_contains("Select Entire Row (Shift+Space)"); - h.state_mut().dialog.as_mut().unwrap().search = "selection.column".into(); - h.run_steps(3); - h.get_by_label_contains(column_label); + h.get_by_label_contains("Paste Special… (Ctrl+Cmd+V)"); } } -/// Ribbon tooltips and the Command Palette read the same shortcut metadata. +/// The engine registry is the authoritative shortcut source. #[test] -fn command_palette_and_ribbon_tooltips_share_shortcuts() { - use gridcraft_ui_egui::widgets::{command_shortcut, tooltip_shortcut}; +fn command_palette_shortcuts_come_from_engine_metadata() { + let session = gridcraft_engine::Session::new(); + let commands = session.commands(); - for (id, tip) in [ - ("file.save", "Save"), - ("edit.undo", "Undo"), - ("edit.redo", "Redo"), - ("edit.copy", "Copy"), - ("edit.cut", "Cut"), - ("edit.paste", "Paste"), - ("home.bold", "Bold"), - ("home.italic", "Italic"), - ("home.underline", "Underline"), - ("formulas.autoSum", "AutoSum"), - ("insert.table", "Table"), - ("data.filter", "Filter"), - ("edit.flashFill", "Flash Fill"), - ("formulas.showFormulas", "Show Formulas"), + for (id, expected) in [ + ("home.bold", "Cmd+B"), + ("data.filter", "Ctrl+Shift+L"), + ("edit.flashFill", "Ctrl+E"), + ("edit.pasteSpecial", "Ctrl+Cmd+V"), + ("selection.row", "Shift+Space"), + ("selection.column", "Ctrl+Space"), + ("selection.currentRegion", "Ctrl+Shift+8"), + ("home.alignLeft", "Cmd+L"), + ("home.alignCenter", "Cmd+E"), ] { - for mac in [false, true] { - assert_eq!(command_shortcut(id, mac), tooltip_shortcut(tip, mac), "{id} and {tip} diverged on mac={mac}"); - } + let command = commands.iter().find(|c| c.id == id).unwrap(); + assert_eq!(command.shortcut, Some(expected), "{id}"); } - - assert_eq!(command_shortcut("edit.flashFill", true), Some("Control+E")); - assert_eq!(command_shortcut("home.alignCenter", true), Some("⌘E")); - assert_eq!(command_shortcut("home.alignCenter", false), None); - assert_eq!(command_shortcut("formulas.showFormulas", true), Some("⌘`")); - assert_eq!(command_shortcut("edit.pasteSpecial", false), Some("Ctrl+Alt+V")); - assert_eq!(command_shortcut("selection.row", false), Some("Shift+Space")); - assert_eq!(command_shortcut("unverified.command", false), None); } - -/// Current Region must show the physical Control shortcut on both platforms. +/// Current Region displays the shortcut declared in engine metadata. #[test] fn command_search_shows_current_region_shortcut() { use egui::os::OperatingSystem; for (os, expected) in - [(OperatingSystem::Windows, "Select Current Region (Ctrl+Shift+8)"), (OperatingSystem::Mac, "Select Current Region (Control+Shift+8)")] + [(OperatingSystem::Windows, "Select Current Region (Ctrl+Shift+8)"), (OperatingSystem::Mac, "Select Current Region (Ctrl+Shift+8)")] { let mut h = harness(); h.ctx.set_os(os);