diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index be1a28f4d..1f5597a6f 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -41,6 +41,7 @@ pub struct CommandSpec { /// Ribbon/menu placement, e.g. `["Home", "Font"]`. Empty = not in menus. pub menu: &'static [&'static str], pub shortcut: Option<&'static str>, + pub shortcut_mac: Option<&'static str>, pub params: &'static str, pub enabled: Enabled, pub run: Run, @@ -54,12 +55,32 @@ pub struct CommandInfo { pub label: &'static str, pub menu: Vec<&'static str>, pub shortcut: Option<&'static str>, + pub shortcut_mac: Option<&'static str>, pub params: &'static str, pub enabled: bool, #[serde(skip_serializing_if = "Option::is_none")] pub disabled_reason: Option, } +impl CommandInfo { + /// Format the portable shortcut notation for the runtime operating system. + pub fn shortcut_for_platform(&self, mac: bool) -> Option { + let keys = if mac { + self.shortcut_mac.or(self.shortcut) + } else if self.shortcut_mac.is_some() && self.shortcut_mac == self.shortcut { + // An identical Mac override marks a Mac-only display shortcut. + // The portable field remains unchanged for legacy CLI consumers. + None + } else { + self.shortcut + }?; + if mac { + return Some(keys.to_owned()); + } + Some(keys.split('+').map(|part| if part == "Cmd" { "Ctrl" } else { part }).collect::>().join("+")) + } +} + impl CommandSpec { pub fn info(&self, s: &Session) -> CommandInfo { let e = (self.enabled)(s); @@ -68,6 +89,7 @@ impl CommandSpec { label: self.label, menu: self.menu.to_vec(), shortcut: self.shortcut, + shortcut_mac: self.shortcut_mac, params: self.params, enabled: e.is_ok(), disabled_reason: e.err(), @@ -118,13 +140,13 @@ pub fn has_clipboard(s: &Session) -> std::result::Result<(), String> { macro_rules! cmd { ($id:literal, $label:literal, [$($m:literal),*], $sc:expr, $params:literal, $en:expr, $run:expr) => { - $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, params: $params, enabled: $en, run: $run, journal: true, undoable: true } + $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, shortcut_mac: None, params: $params, enabled: $en, run: $run, journal: true, undoable: true } }; (query $id:literal, $label:literal, [$($m:literal),*], $sc:expr, $params:literal, $en:expr, $run:expr) => { - $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, params: $params, enabled: $en, run: $run, journal: false, undoable: false } + $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, shortcut_mac: None, params: $params, enabled: $en, run: $run, journal: false, undoable: false } }; (noundo $id:literal, $label:literal, [$($m:literal),*], $sc:expr, $params:literal, $en:expr, $run:expr) => { - $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, params: $params, enabled: $en, run: $run, journal: true, undoable: false } + $crate::cmd::CommandSpec { id: $id, label: $label, menu: &[$($m),*], shortcut: $sc, shortcut_mac: None, params: $params, enabled: $en, run: $run, journal: true, undoable: false } }; } pub(crate) use cmd; @@ -180,6 +202,7 @@ pub fn register_test_command(id: &'static str, run: Run) { label: id, menu: &[], shortcut: None, + shortcut_mac: None, params: "", enabled: |_| Ok(()), run, @@ -197,6 +220,7 @@ pub(crate) fn register_undoable_test_command(id: &'static str, run: Run) { label: id, menu: &[], shortcut: None, + shortcut_mac: None, params: "", enabled: |_| Ok(()), run, diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 143763a5e..0906f4893 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -99,18 +99,30 @@ pub fn specs() -> Vec { ), cmd!(query "data.validate", "Check Value Against Validation", [], None, "{cell?, input} → {ok, message?}", has_doc, validate_cmd), cmd!(noundo "data.circleInvalid", "Circle Invalid Data", ["Data", "Data Tools", "Data Validation"], None, "{} → invalid cells", has_doc, circle_invalid), - cmd!( - "data.group", - "Group", - ["Data", "Outline"], - Some("Alt+Shift+Right"), - "{rows?: \"2:5\" | cols?: \"B:C\" | by?: rows|cols (the selection's rows or columns; a block of cells groups rows by default), sheet?}", - has_doc, - |s, p| group(s, p, true) - ), - cmd!("data.ungroup", "Ungroup", ["Data", "Outline"], Some("Alt+Shift+Left"), "{rows? | cols? | by?: rows|cols, sheet?}", has_doc, |s, p| { - group(s, p, false) - }), + CommandSpec { + shortcut_mac: Some("Cmd+Shift+K"), + ..cmd!( + "data.group", + "Group", + ["Data", "Outline"], + Some("Alt+Shift+Right"), + "{rows?: \"2:5\" | cols?: \"B:C\" | by?: rows|cols (the selection's rows or columns; a block of cells groups rows by default), sheet?}", + has_doc, + |s, p| group(s, p, true) + ) + }, + CommandSpec { + shortcut_mac: Some("Cmd+Shift+J"), + ..cmd!( + "data.ungroup", + "Ungroup", + ["Data", "Outline"], + Some("Alt+Shift+Left"), + "{rows? | cols? | by?: rows|cols, sheet?}", + has_doc, + |s, p| { group(s, p, false) } + ) + }, cmd!( "data.hideDetail", "Hide Detail", diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 6e4e04613..70dcf0c5a 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -81,15 +81,18 @@ pub fn specs() -> Vec { has_doc, paste ), - cmd!( - "edit.pasteSpecial", - "Paste Special…", - ["Home", "Clipboard"], - Some("Ctrl+Cmd+V"), - "{what: all|formulas|values|formats|comments|validation|allExceptBorders|columnWidths|formulasAndNumberFormats|valuesAndNumberFormats, operation?: none|add|subtract|multiply|divide, skipBlanks?, transpose?, link?}", - has_clipboard, - paste_special - ), + CommandSpec { + shortcut_mac: Some("Ctrl+Cmd+V"), + ..cmd!( + "edit.pasteSpecial", + "Paste Special…", + ["Home", "Clipboard"], + Some("Ctrl+Alt+V"), + "{what: all|formulas|values|formats|comments|validation|allExceptBorders|columnWidths|formulasAndNumberFormats|valuesAndNumberFormats, operation?: none|add|subtract|multiply|divide, skipBlanks?, transpose?, link?}", + has_clipboard, + paste_special + ) + }, cmd!(noundo "edit.clearClipboard", "Cancel Copy", [], Some("Escape"), "{}", has_doc, clear_clipboard), cmd!("edit.clearAll", "Clear All", ["Home", "Editing", "Clear"], None, "{range?}", has_doc, |s, p| clear(s, p, "all")), cmd!("edit.clearFormats", "Clear Formats", ["Home", "Editing", "Clear"], None, "{range?}", has_doc, |s, p| clear(s, p, "formats")), diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 69123dcfa..f0635d320 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -90,8 +90,14 @@ pub fn specs() -> Vec { format_cells ), // Alignment - cmd!("home.alignLeft", "Align Left", ["Home", "Alignment"], Some("Cmd+L"), "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)), - cmd!("home.alignCenter", "Center", ["Home", "Alignment"], Some("Cmd+E"), "{range?}", has_doc, |s, p| halign(s, p, HAlign::Center)), + CommandSpec { + shortcut_mac: Some("Cmd+L"), + ..cmd!("home.alignLeft", "Align Left", ["Home", "Alignment"], Some("Cmd+L"), "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)) + }, + CommandSpec { + shortcut_mac: Some("Cmd+E"), + ..cmd!("home.alignCenter", "Center", ["Home", "Alignment"], Some("Cmd+E"), "{range?}", has_doc, |s, p| halign(s, p, HAlign::Center)) + }, cmd!("home.alignRight", "Align Right", ["Home", "Alignment"], None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Right)), cmd!("home.justify", "Justify", ["Home", "Alignment"], None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Justify)), cmd!("home.alignTop", "Top Align", ["Home", "Alignment"], None, "{range?}", has_doc, |s, p| valign(s, p, VAlign::Top)), diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index d1aea3cc7..de9c98ecd 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -35,6 +35,7 @@ pub fn specs() -> Vec { label: "Allow Edit Ranges…", menu: &["Review", "Protect"], shortcut: None, + shortcut_mac: None, params: "{action?: list (default) | add | modify | delete | unlock, title?, range?, password?, newTitle?, sheet?} → ranges", enabled: has_doc, run: allow_edit_ranges, diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index aed9843fc..44b518ff9 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -1,5 +1,6 @@ use super::{Dialog, Key, SheetApp, json, vec2}; +/// 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| { ( @@ -19,14 +20,15 @@ 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)> = 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(" › "))) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), c.shortcut_for_platform(mac))) .take(40) .collect(); @@ -62,8 +64,12 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial egui::ScrollArea::vertical().max_height(LIST_HEIGHT).auto_shrink([true, false]).show(ui, |ui| { let scroll_area_width = ui.available_width(); - for (i, (id, label, menu)) in cmds.iter().enumerate() { - let r = ui.add(egui::Button::selectable(i == d.list_index, format!("{label} {menu}")).min_size(vec2(scroll_area_width, 20.0))); + for (i, (id, label, menu, shortcut)) in cmds.iter().enumerate() { + let text = match shortcut { + Some(keys) => format!("{label} ({keys}) {menu}"), + None => format!("{label} {menu}"), + }; + let r = ui.add(egui::Button::selectable(i == d.list_index, text.clone()).truncate().min_size(vec2(scroll_area_width, 20.0))); if (down || up || home || end || pgup || pgdn) && i == d.list_index && !ui.clip_rect().contains_rect(r.rect) { r.scroll_to_me(Some(if down || end || pgdn { egui::Align::Max } else { egui::Align::Min })); @@ -74,7 +80,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial *confirm = true; } - r.on_hover_text(id); + r.on_hover_text(format!("{text}\n{id}")); } }); if ui.input(|i| i.key_pressed(Key::Enter)) { diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index a2e749921..28c02840e 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -47,7 +47,7 @@ pub fn tooltip_shortcut(tip: &str, mac: bool) -> Option<&'static str> { fn tip_text(ui: &Ui, tip: &str) -> String { // A name shared with a two-line ribbon label (Czech "Automatický\nsoučet") reads as one line. let name = tr(ui, tip).replace('\n', " "); - let text = match tooltip_shortcut(tip, cfg!(target_os = "macos")) { + let text = match tooltip_shortcut(tip, ui.ctx().os() == egui::os::OperatingSystem::Mac) { Some(keys) => format!("{name} ({keys})"), None => name, }; @@ -754,4 +754,23 @@ mod tooltip_shortcut_tests { assert!(mac.starts_with('⌘') || mac.starts_with('⌃'), "{name}: {mac}"); } } + + #[test] + 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 (⌘E)")] { + let ctx = egui::Context::default(); + ctx.set_os(os); + + let mut actual = String::new(); + + let mut output = ctx.run_ui(egui::RawInput::default(), |ui| { + actual = super::tip_text(ui, "Flash Fill"); + }); + output.textures_delta.clear(); + + assert_eq!(actual, expected, "{os:?}"); + } + } } diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 98133c371..ca3bfe925 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -2,6 +2,7 @@ //! column width without clicking into the field first. use egui::{Event, Key, vec2}; +use egui_kittest::kittest::Queryable; use gridcraft_engine::Session; use gridcraft_engine::core::CellRef; use gridcraft_ui_egui::SheetApp; @@ -81,3 +82,168 @@ fn command_search_arrow_and_page_keys_move_the_selection_and_keep_the_search_foc assert!(after_page > 2, "PageDown moved past the third row, got {after_page}"); assert!(h.ctx.memory(|m| m.focused()).is_some(), "the search field kept focus"); } + +/// The palette displays the engine metadata unchanged on either runtime OS. +#[test] +fn command_search_shows_engine_shortcuts() { + 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().open_dialog("commandSearch", json!({})); + h.run_steps(6); + + for (query, expected) in [ + ("home.bold", if os == OperatingSystem::Mac { "Bold (Cmd+B)" } else { "Bold (Ctrl+B)" }), + ("data.filter", "Filter (Ctrl+Shift+L)"), + ("edit.copy", if os == OperatingSystem::Mac { "Copy (Cmd+C)" } else { "Copy (Ctrl+C)" }), + ("edit.paste", if os == OperatingSystem::Mac { "Paste (Cmd+V)" } else { "Paste (Ctrl+V)" }), + ("home.italic", if os == OperatingSystem::Mac { "Italic (Cmd+I)" } else { "Italic (Ctrl+I)" }), + ("home.underline", if os == OperatingSystem::Mac { "Underline (Cmd+U)" } else { "Underline (Ctrl+U)" }), + ("edit.fillDown", if os == OperatingSystem::Mac { "Fill Down (Cmd+D)" } else { "Fill Down (Ctrl+D)" }), + ("edit.flashFill", "Flash Fill (Ctrl+E)"), + ("home.alignLeft", if os == OperatingSystem::Mac { "Align Left (Cmd+L)" } else { "Align Left Home" }), + ("home.alignCenter", if os == OperatingSystem::Mac { "Center (Cmd+E)" } else { "Center Home" }), + ("selection.row", "Select Entire Row (Shift+Space)"), + ("selection.column", "Select Entire Column (Ctrl+Space)"), + ("data.group", if os == OperatingSystem::Mac { "Group (Cmd+Shift+K)" } else { "Group (Alt+Shift+Right)" }), + ("data.ungroup", if os == OperatingSystem::Mac { "Ungroup (Cmd+Shift+J)" } else { "Ungroup (Alt+Shift+Left)" }), + ] { + h.state_mut().dialog.as_mut().unwrap().search = query.into(); + h.run_steps(3); + h.get_by_label_contains(expected); + } + } +} + +/// Paste Special uses the engine's shortcut metadata when enabled. +#[test] +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 = "edit.pasteSpecial".into(); + h.run_steps(3); + + h.get_by_label_contains(if os == OperatingSystem::Mac { "Paste Special… (Ctrl+Cmd+V)" } else { "Paste Special… (Ctrl+Alt+V)" }); + } +} + +/// The engine registry is the authoritative shortcut source. +#[test] +fn command_palette_shortcuts_come_from_engine_metadata() { + let session = gridcraft_engine::Session::new(); + let commands = session.commands(); + + for (id, mac, other) in [ + ("home.bold", Some("Cmd+B"), Some("Ctrl+B")), + ("edit.find", Some("Cmd+F"), Some("Ctrl+F")), + ("edit.selectAll", Some("Cmd+A"), Some("Ctrl+A")), + ("edit.undo", Some("Cmd+Z"), Some("Ctrl+Z")), + ("edit.redo", Some("Cmd+Y"), Some("Ctrl+Y")), + ("edit.copy", Some("Cmd+C"), Some("Ctrl+C")), + ("edit.cut", Some("Cmd+X"), Some("Ctrl+X")), + ("edit.paste", Some("Cmd+V"), Some("Ctrl+V")), + ("edit.fillDown", Some("Cmd+D"), Some("Ctrl+D")), + ("edit.fillRight", Some("Cmd+R"), Some("Ctrl+R")), + ("home.italic", Some("Cmd+I"), Some("Ctrl+I")), + ("home.underline", Some("Cmd+U"), Some("Ctrl+U")), + ("edit.pasteSpecial", Some("Ctrl+Cmd+V"), Some("Ctrl+Alt+V")), + ("home.alignLeft", Some("Cmd+L"), None), + ("home.alignCenter", Some("Cmd+E"), None), + ("data.group", Some("Cmd+Shift+K"), Some("Alt+Shift+Right")), + ("data.ungroup", Some("Cmd+Shift+J"), Some("Alt+Shift+Left")), + ("edit.flashFill", Some("Ctrl+E"), Some("Ctrl+E")), + ("data.filter", Some("Ctrl+Shift+L"), Some("Ctrl+Shift+L")), + ("selection.row", Some("Shift+Space"), Some("Shift+Space")), + ("selection.column", Some("Ctrl+Space"), Some("Ctrl+Space")), + ] { + let command = commands.iter().find(|c| c.id == id).unwrap(); + assert_eq!(command.shortcut_for_platform(true), mac.map(str::to_string), "{id}: macOS"); + assert_eq!(command.shortcut_for_platform(false), other.map(str::to_string), "{id}: Windows/Linux"); + } + + for (id, legacy) in + [("data.group", "Alt+Shift+Right"), ("data.ungroup", "Alt+Shift+Left"), ("home.alignLeft", "Cmd+L"), ("home.alignCenter", "Cmd+E")] + { + let command = commands.iter().find(|c| c.id == id).unwrap(); + assert_eq!(command.shortcut, Some(legacy), "{id}: legacy CLI shortcut"); + } + + let command = commands.iter().find(|c| c.id == "home.alignRight").unwrap(); + assert_eq!(command.shortcut_for_platform(true), None); + assert_eq!(command.shortcut_for_platform(false), None); +} +/// 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 (Ctrl+Shift+8)")] + { + let mut h = harness(); + h.ctx.set_os(os); + h.run_steps(2); + + h.state_mut().open_dialog("commandSearch", json!({})); + h.run_steps(6); + + h.state_mut().dialog.as_mut().unwrap().search = "selection.currentRegion".into(); + + h.run_steps(3); + h.get_by_label_contains(expected); + } +} + +/// Disabled commands are absent; commands without shortcuts show no invented keys. +#[test] +fn command_search_hides_disabled_and_handles_missing_shortcuts() { + use egui::os::OperatingSystem; + + for os in [OperatingSystem::Windows, OperatingSystem::Mac] { + let mut h = harness(); + h.ctx.set_os(os); + h.run_steps(2); + + // Undo is disabled in a fresh workbook with no undo history. + let undo = h.state().session.commands().into_iter().find(|c| c.id == "edit.undo").unwrap(); + + assert!(!undo.enabled, "Undo should initially be disabled"); + + h.state_mut().open_dialog("commandSearch", json!({})); + h.run_steps(6); + + h.state_mut().dialog.as_mut().unwrap().search = "edit.undo".into(); + h.run_steps(3); + + assert!( + h.query_by_label("Undo (Cmd+Z) Edit").is_none() && h.query_by_label("Undo (Ctrl+Z) Edit").is_none(), + "Disabled Undo must not appear in the palette" + ); + + // Align Right has no shortcut on either platform. + h.state_mut().dialog.as_mut().unwrap().search = "home.alignRight".into(); + h.run_steps(3); + + let align = h.state().session.commands().into_iter().find(|c| c.id == "home.alignRight").unwrap(); + + assert_eq!(align.shortcut_for_platform(os == OperatingSystem::Mac), None); + + h.get_by_label_contains("Align Right Home"); + } +}