From d3832e98e6d7b81ca9a410c07eccca9f4579a243 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Tue, 27 Oct 2026 16:56:49 +0700 Subject: [PATCH 01/12] fix(ui): show shortcuts in command palette (#2725) --- crates/ui-egui/src/dialogs/command_search.rs | 20 ++++++++++++++++---- crates/ui-egui/tests/it/dialog_focus.rs | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index 509ca4700..1aefdb9b1 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -20,13 +20,20 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial } let q = d.search.to_ascii_lowercase(); - let cmds: Vec<(String, String, String)> = app + let cmds: Vec<(String, String, String, Option<&str>)> = 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(" › "), + crate::widgets::tooltip_shortcut(c.label, cfg!(target_os = "macos")).or(c.shortcut), + ) + }) .take(40) .collect(); @@ -62,8 +69,13 @@ 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 shortcut = shortcut.map(|keys| if cfg!(target_os = "macos") { keys.to_string() } else { keys.replace("Cmd+", "Ctrl+") }); + 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).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 })); diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 98133c371..9ed72fe76 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,22 @@ 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"); } + +#[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 cfg!(target_os = "macos") { "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 cfg!(target_os = "macos") { "Filter (⌘⇧F)" } else { "Filter (Ctrl+Shift+L)" }; + h.get_by_label_contains(filter); +} From 6e7fb6ce0be7e77ea52b387f45208a5b6418b353 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:13:33 +0700 Subject: [PATCH 02/12] fix(ui): correct Paste Special shortcut in command palette --- crates/ui-egui/src/dialogs/command_search.rs | 11 ++++++++++- crates/ui-egui/tests/it/dialog_focus.rs | 20 ++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index 1aefdb9b1..c03d0a0f8 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::*; +/// Shows enabled commands and their platform-specific keyboard shortcuts. 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| { ( @@ -70,7 +71,15 @@ 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, shortcut)) in cmds.iter().enumerate() { - let shortcut = shortcut.map(|keys| if cfg!(target_os = "macos") { keys.to_string() } else { keys.replace("Cmd+", "Ctrl+") }); + let shortcut = shortcut.map(|keys| { + if cfg!(target_os = "macos") { + keys.to_string() + } else if keys == "Ctrl+Cmd+V" { + "Ctrl+Alt+V".to_string() + } else { + keys.strip_prefix("Cmd+").map_or_else(|| keys.to_string(), |rest| format!("Ctrl+{rest}")) + } + }); let text = match shortcut { Some(keys) => format!("{label} ({keys}) {menu}"), None => format!("{label} {menu}"), diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 9ed72fe76..44b726958 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -83,6 +83,7 @@ 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. #[test] fn command_search_shows_shortcuts_for_current_platform() { let mut h = harness(); @@ -101,3 +102,22 @@ fn command_search_shows_shortcuts_for_current_platform() { let filter = if cfg!(target_os = "macos") { "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 cfg!(target_os = "macos") { "Paste Special… (Ctrl+Cmd+V)" } else { "Paste Special… (Ctrl+Alt+V)" }; + + h.get_by_label_contains(expected); +} From f77d70502aca3f655a0b3c19a029a28c23748252 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:39:37 +0700 Subject: [PATCH 03/12] fix(ui): verify command shortcuts for runtime OS --- crates/ui-egui/src/dialogs/command_search.rs | 54 +++++++++++++------ crates/ui-egui/tests/it/dialog_focus.rs | 56 ++++++++++++++++++-- 2 files changed, 90 insertions(+), 20 deletions(-) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index c03d0a0f8..e43a0cb43 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -20,6 +20,7 @@ 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 .session @@ -27,14 +28,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial .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::tooltip_shortcut(c.label, cfg!(target_os = "macos")).or(c.shortcut), - ) - }) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), command_search_shortcut(c.id, mac))) .take(40) .collect(); @@ -71,15 +65,6 @@ 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, shortcut)) in cmds.iter().enumerate() { - let shortcut = shortcut.map(|keys| { - if cfg!(target_os = "macos") { - keys.to_string() - } else if keys == "Ctrl+Cmd+V" { - "Ctrl+Alt+V".to_string() - } else { - keys.strip_prefix("Cmd+").map_or_else(|| keys.to_string(), |rest| format!("Ctrl+{rest}")) - } - }); let text = match shortcut { Some(keys) => format!("{label} ({keys}) {menu}"), None => format!("{label} {menu}"), @@ -107,3 +92,38 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial app.run_or_alert(&id, json!({})); } } + +/// Only advertises shortcuts verified against the grid keymap and ribbon. +/// Commands without a confirmed binding are shown without a shortcut. +fn command_search_shortcut(id: &str, mac: bool) -> Option<&'static str> { + let (mac_keys, other_keys) = match id { + "file.new" => ("⌘N", Some("Ctrl+N")), + "file.open" => ("⌘O", Some("Ctrl+O")), + "file.save" => ("⌘S", Some("Ctrl+S")), + "file.saveAs" => ("⌘⇧S", Some("Ctrl+Shift+S")), + "file.close" => ("⌘W", Some("Ctrl+W")), + "edit.copy" => ("⌘C", Some("Ctrl+C")), + "edit.cut" => ("⌘X", Some("Ctrl+X")), + "edit.paste" => ("⌘V", Some("Ctrl+V")), + "edit.undo" => ("⌘Z", Some("Ctrl+Z")), + "edit.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" => ("⌘B", Some("Ctrl+B")), + "home.italic" => ("⌘I", Some("Ctrl+I")), + "home.underline" => ("⌘U", Some("Ctrl+U")), + "formulas.autoSum" => ("⌘⇧T", Some("Alt+=")), + "insert.table" => ("⌘T", Some("Ctrl+T")), + "data.filter" => ("⌘⇧F", Some("Ctrl+Shift+L")), + "edit.flashFill" => ("Control+E", Some("Ctrl+E")), + "edit.pasteSpecial" => ("Ctrl+Cmd+V", Some("Ctrl+Alt+V")), + "home.alignLeft" => ("⌘L", None), + "home.alignCenter" => ("⌘E", None), + "view.showObjects" => ("Ctrl+6", Some("Ctrl+6")), + "formulas.showFormulas" => ("⌘`", Some("Ctrl+`")), + _ => return None, + }; + + if mac { Some(mac_keys) } else { other_keys } +} diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 44b726958..902bb8e4c 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -93,13 +93,13 @@ fn command_search_shows_shortcuts_for_current_platform() { h.state_mut().dialog.as_mut().unwrap().search = "bold".into(); h.run_steps(3); - let bold = if cfg!(target_os = "macos") { "Bold (⌘B)" } else { "Bold (Ctrl+B)" }; + 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 cfg!(target_os = "macos") { "Filter (⌘⇧F)" } else { "Filter (Ctrl+Shift+L)" }; + let filter = if h.ctx.os() == egui::os::OperatingSystem::Mac { "Filter (⌘⇧F)" } else { "Filter (Ctrl+Shift+L)" }; h.get_by_label_contains(filter); } @@ -117,7 +117,57 @@ fn command_search_shows_paste_special_shortcut() { h.state_mut().dialog.as_mut().unwrap().search = "pasteSpecial".into(); h.run_steps(3); - let expected = if cfg!(target_os = "macos") { "Paste Special… (Ctrl+Cmd+V)" } else { "Paste Special… (Ctrl+Alt+V)" }; + 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() { + 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)", + ), + ] { + 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), + ] { + h.state_mut().dialog.as_mut().unwrap().search = query.into(); + h.run_steps(3); + h.get_by_label_contains(expected); + } + } +} From bf4a3969e8bbbe37e2126d70cc352530d200f5e0 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Wed, 28 Oct 2026 08:56:28 +0700 Subject: [PATCH 04/12] fix(ui): show verified selection shortcuts in command palette --- crates/ui-egui/src/dialogs/command_search.rs | 2 ++ crates/ui-egui/tests/it/dialog_focus.rs | 23 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index e43a0cb43..e7c59b150 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -120,6 +120,8 @@ fn command_search_shortcut(id: &str, mac: bool) -> Option<&'static str> { "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.column" => ("Control+Space", Some("Ctrl+Space")), "view.showObjects" => ("Ctrl+6", Some("Ctrl+6")), "formulas.showFormulas" => ("⌘`", Some("Ctrl+`")), _ => return None, diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 902bb8e4c..878ea9bd8 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -171,3 +171,26 @@ fn command_search_uses_runtime_os_and_verified_shortcuts() { } } } +/// Selection shortcuts use the same bindings as the runtime keyboard handler. +#[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)"), + ] { + 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.row".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); + } +} From 712f45965b5942226f6e73f4a1648603a5f78ff4 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:07:56 +0700 Subject: [PATCH 05/12] fix(ui): share verified shortcuts between palette and ribbon --- crates/ui-egui/src/dialogs/command_search.rs | 43 +--------- crates/ui-egui/src/widgets.rs | 82 +++++++++++++------- crates/ui-egui/tests/it/dialog_focus.rs | 35 +++++++++ 3 files changed, 93 insertions(+), 67 deletions(-) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index e7c59b150..81fa581e4 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -28,7 +28,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial .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(" › "), command_search_shortcut(c.id, mac))) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), crate::widgets::command_shortcut(c.id, mac))) .take(40) .collect(); @@ -69,7 +69,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial Some(keys) => format!("{label} ({keys}) {menu}"), None => format!("{label} {menu}"), }; - let r = ui.add(egui::Button::selectable(i == d.list_index, text).truncate().min_size(vec2(scroll_area_width, 20.0))); + 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 })); @@ -80,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)) { @@ -92,40 +92,3 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial app.run_or_alert(&id, json!({})); } } - -/// Only advertises shortcuts verified against the grid keymap and ribbon. -/// Commands without a confirmed binding are shown without a shortcut. -fn command_search_shortcut(id: &str, mac: bool) -> Option<&'static str> { - let (mac_keys, other_keys) = match id { - "file.new" => ("⌘N", Some("Ctrl+N")), - "file.open" => ("⌘O", Some("Ctrl+O")), - "file.save" => ("⌘S", Some("Ctrl+S")), - "file.saveAs" => ("⌘⇧S", Some("Ctrl+Shift+S")), - "file.close" => ("⌘W", Some("Ctrl+W")), - "edit.copy" => ("⌘C", Some("Ctrl+C")), - "edit.cut" => ("⌘X", Some("Ctrl+X")), - "edit.paste" => ("⌘V", Some("Ctrl+V")), - "edit.undo" => ("⌘Z", Some("Ctrl+Z")), - "edit.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" => ("⌘B", Some("Ctrl+B")), - "home.italic" => ("⌘I", Some("Ctrl+I")), - "home.underline" => ("⌘U", Some("Ctrl+U")), - "formulas.autoSum" => ("⌘⇧T", Some("Alt+=")), - "insert.table" => ("⌘T", Some("Ctrl+T")), - "data.filter" => ("⌘⇧F", Some("Ctrl+Shift+L")), - "edit.flashFill" => ("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.column" => ("Control+Space", Some("Ctrl+Space")), - "view.showObjects" => ("Ctrl+6", Some("Ctrl+6")), - "formulas.showFormulas" => ("⌘`", Some("Ctrl+`")), - _ => return None, - }; - - if mac { Some(mac_keys) } else { other_keys } -} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 63490a36e..11ec30a23 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -15,39 +15,59 @@ fn tr<'a>(ui: &Ui, text: &'a str) -> &'a str { i18n::current(ui.ctx()).tr(text) } -/// 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+`"), +/// 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.column", "", "Control+Space", Some("Ctrl+Space")), + ("view.showObjects", "", "Control+6", Some("Ctrl+6")), + ("formulas.showFormulas", "Show Formulas", "⌘`", Some("Ctrl+`")), ]; -/// 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 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 }) } +/// 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 { // 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, }; @@ -714,9 +734,17 @@ 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 (name, mac, other) in super::TOOLTIP_SHORTCUTS { - assert!(!other.contains(['⌘', '⇧', '⌥']), "{name}: {other}"); - assert!(mac.starts_with('⌘') || mac.starts_with('⌃'), "{name}: {mac}"); + 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}"); + } } } } diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 878ea9bd8..de004d621 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -194,3 +194,38 @@ fn command_search_selection_shortcuts_match_runtime_os() { h.get_by_label_contains(column_label); } } + +/// Ribbon tooltips and the Command Palette read the same shortcut metadata. +#[test] +fn command_palette_and_ribbon_tooltips_share_shortcuts() { + use gridcraft_ui_egui::widgets::{command_shortcut, tooltip_shortcut}; + + 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 mac in [false, true] { + assert_eq!(command_shortcut(id, mac), tooltip_shortcut(tip, mac), "{id} and {tip} diverged on mac={mac}"); + } + } + + 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); +} From dd48d096050576e0be1d320bce41635acceca027 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Wed, 28 Oct 2026 10:31:19 +0700 Subject: [PATCH 06/12] test(ui): cover runtime tooltips and current region shortcut --- crates/ui-egui/src/widgets.rs | 20 ++++++++++++++++++++ crates/ui-egui/tests/it/dialog_focus.rs | 22 ++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 11ec30a23..a43784930 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -44,6 +44,7 @@ const SHORTCUTS: &[(&str, &str, &str, Option<&str>)] = &[ ("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+`")), @@ -747,4 +748,23 @@ mod tooltip_shortcut_tests { } } } + + #[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 (Control+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 de004d621..6aba1c2f3 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -229,3 +229,25 @@ fn command_palette_and_ribbon_tooltips_share_shortcuts() { 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. +#[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)")] + { + 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); + } +} From e675efd307df41a77b383f7df6c967f58b83e6e7 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Sat, 31 Oct 2026 13:46:01 +0700 Subject: [PATCH 07/12] fix(ui): source command palette shortcuts from engine metadata --- crates/ui-egui/src/dialogs/command_search.rs | 7 +- crates/ui-egui/src/widgets.rs | 83 ++++------ crates/ui-egui/tests/it/dialog_focus.rs | 153 +++++-------------- 3 files changed, 70 insertions(+), 173 deletions(-) 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); From 2d5be2c57a59958653dd98a58392ed9afd7eb1e3 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Sat, 31 Oct 2026 14:46:11 +0700 Subject: [PATCH 08/12] fix(ui): correct platform-specific command shortcut labels --- crates/ui-egui/src/dialogs/command_search.rs | 16 +++++++++++++++- crates/ui-egui/src/widgets.rs | 6 +++--- crates/ui-egui/tests/it/dialog_focus.rs | 8 ++++---- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index 96e17336f..c2396b345 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -20,6 +20,7 @@ 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)> = app .session @@ -27,7 +28,20 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial .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(" › "), c.shortcut.map(str::to_string))) + .map(|c| { + ( + c.id.to_string(), + c.label.to_string(), + c.menu.join(" › "), + match (mac, c.id, c.shortcut) { + (false, "home.bold", Some(_)) => Some("Ctrl+B".to_owned()), + (false, "edit.pasteSpecial", Some(_)) => Some("Ctrl+Alt+V".to_owned()), + // These metadata shortcuts are only bound on macOS. + (false, "home.alignLeft" | "home.alignCenter", Some(_)) => None, + (_, _, keys) => keys.map(str::to_string), + }, + ) + }) .take(40) .collect(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 2b2f40e95..110a1d032 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -31,7 +31,7 @@ const TOOLTIP_SHORTCUTS: &[(&str, &str, &str)] = &[ ("Table", "⌘T", "Ctrl+T"), ("Insert Link", "⌘K", "Ctrl+K"), ("Filter", "⌘⇧F", "Ctrl+Shift+L"), - ("Flash Fill", "⌘E", "Ctrl+E"), + ("Flash Fill", "Control+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+`"), @@ -716,7 +716,7 @@ mod tooltip_shortcut_tests { assert_eq!(tooltip_shortcut("Search commands", false), None); for (name, mac, other) in super::TOOLTIP_SHORTCUTS { assert!(!other.contains(['⌘', '⇧', '⌥']), "{name}: {other}"); - assert!(mac.starts_with('⌘') || mac.starts_with('⌃'), "{name}: {mac}"); + assert!(mac.starts_with('⌘') || mac.starts_with('⌃') || (*name == "Flash Fill" && *mac == "Control+E"), "{name}: {mac}"); } } @@ -724,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 (⌘E)")] { + for (os, expected) in [(OperatingSystem::Windows, "Flash Fill (Ctrl+E)"), (OperatingSystem::Mac, "Flash Fill (Control+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 63850a64e..f3c5db9ec 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -97,11 +97,11 @@ fn command_search_shows_engine_shortcuts() { h.run_steps(6); for (query, expected) in [ - ("home.bold", "Bold (Cmd+B)"), + ("home.bold", if os == OperatingSystem::Mac { "Bold (Cmd+B)" } else { "Bold (Ctrl+B)" }), ("data.filter", "Filter (Ctrl+Shift+L)"), ("edit.flashFill", "Flash Fill (Ctrl+E)"), - ("home.alignLeft", "Align Left (Cmd+L)"), - ("home.alignCenter", "Center (Cmd+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)"), ] { @@ -132,7 +132,7 @@ fn command_search_shows_paste_special_metadata() { h.state_mut().dialog.as_mut().unwrap().search = "edit.pasteSpecial".into(); h.run_steps(3); - h.get_by_label_contains("Paste Special… (Ctrl+Cmd+V)"); + h.get_by_label_contains(if os == OperatingSystem::Mac { "Paste Special… (Ctrl+Cmd+V)" } else { "Paste Special… (Ctrl+Alt+V)" }); } } From 513f19ded300e31267750591d8a32a3cf411f5ff Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Sat, 31 Oct 2026 15:14:23 +0700 Subject: [PATCH 09/12] fix(ui): show macOS group shortcuts in command palette --- crates/ui-egui/src/dialogs/command_search.rs | 3 +++ crates/ui-egui/tests/it/dialog_focus.rs | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/crates/ui-egui/src/dialogs/command_search.rs b/crates/ui-egui/src/dialogs/command_search.rs index c2396b345..7a80985b3 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -38,6 +38,9 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial (false, "edit.pasteSpecial", Some(_)) => Some("Ctrl+Alt+V".to_owned()), // These metadata shortcuts are only bound on macOS. (false, "home.alignLeft" | "home.alignCenter", Some(_)) => None, + // macOS uses Command+Shift+K/J, unlike the engine's Windows metadata. + (true, "data.group", Some(_)) => Some("Cmd+Shift+K".to_owned()), + (true, "data.ungroup", Some(_)) => Some("Cmd+Shift+J".to_owned()), (_, _, keys) => keys.map(str::to_string), }, ) diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index f3c5db9ec..53fb6c933 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -104,6 +104,8 @@ fn command_search_shows_engine_shortcuts() { ("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); @@ -152,6 +154,8 @@ fn command_palette_shortcuts_come_from_engine_metadata() { ("selection.currentRegion", "Ctrl+Shift+8"), ("home.alignLeft", "Cmd+L"), ("home.alignCenter", "Cmd+E"), + ("data.group", "Alt+Shift+Right"), + ("data.ungroup", "Alt+Shift+Left"), ] { let command = commands.iter().find(|c| c.id == id).unwrap(); assert_eq!(command.shortcut, Some(expected), "{id}"); From 94f8410ccfec27a25e15b75c08cf82df04410bf0 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Mon, 2 Nov 2026 11:26:40 +0700 Subject: [PATCH 10/12] fix(ui): centralize platform-specific command shortcuts and test palette --- crates/engine/src/cmd.rs | 54 ++++++++++++- crates/engine/src/cmd/data.rs | 6 +- crates/engine/src/cmd/edit.rs | 23 +++--- crates/engine/src/cmd/format.rs | 13 ++- crates/engine/src/cmd/review.rs | 2 + crates/ui-egui/src/dialogs/command_search.rs | 18 +---- crates/ui-egui/src/widgets.rs | 6 +- crates/ui-egui/tests/it/dialog_focus.rs | 83 +++++++++++++++++--- 8 files changed, 147 insertions(+), 58 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index ef6714eea..3bd25929e 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -40,6 +40,8 @@ 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 shortcut_other: Option<&'static str>, pub params: &'static str, pub enabled: Enabled, pub run: Run, @@ -53,12 +55,21 @@ pub struct CommandInfo { pub label: &'static str, pub menu: Vec<&'static str>, pub shortcut: Option<&'static str>, + pub shortcut_mac: Option<&'static str>, + pub shortcut_other: Option<&'static str>, pub params: &'static str, pub enabled: bool, #[serde(skip_serializing_if = "Option::is_none")] pub disabled_reason: Option, } +impl CommandInfo { + /// Shortcut declared by the engine for the active platform. + pub fn shortcut_for_platform(&self, mac: bool) -> Option<&'static str> { + if mac { self.shortcut_mac } else { self.shortcut_other } + } +} + impl CommandSpec { pub fn info(&self, s: &Session) -> CommandInfo { let e = (self.enabled)(s); @@ -67,6 +78,8 @@ impl CommandSpec { label: self.label, menu: self.menu.to_vec(), shortcut: self.shortcut, + shortcut_mac: self.shortcut_mac, + shortcut_other: self.shortcut_other, params: self.params, enabled: e.is_ok(), disabled_reason: e.err(), @@ -116,14 +129,45 @@ pub fn has_clipboard(s: &Session) -> std::result::Result<(), String> { } macro_rules! cmd { + (platform $id:literal, $label:literal, [$($m:literal),*], $mac:expr, $other:expr, $params:literal, $en:expr, $run:expr) => { + $crate::cmd::CommandSpec { + id: $id, + label: $label, + menu: &[$($m),*], + shortcut: $mac, + shortcut_mac: $mac, + shortcut_other: $other, + params: $params, + enabled: $en, + run: $run, + journal: true, + undoable: true, + } + }; ($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: $sc, shortcut_other: $sc, 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: $sc, shortcut_other: $sc, 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: $sc, shortcut_other: $sc, params: $params, enabled: $en, run: $run, journal: true, undoable: false } + }; + (platform_noundo $id:literal, $label:literal, [$($m:literal),*], + $mac:expr, $other:expr, $params:literal, $en:expr, $run:expr) => { + $crate::cmd::CommandSpec { + id: $id, + label: $label, + menu: &[$($m),*], + shortcut: $mac, + shortcut_mac: $mac, + shortcut_other: $other, + params: $params, + enabled: $en, + run: $run, + journal: true, + undoable: false, + } }; } pub(crate) use cmd; @@ -178,6 +222,8 @@ pub fn register_test_command(id: &'static str, run: Run) { label: id, menu: &[], shortcut: None, + shortcut_mac: None, + shortcut_other: None, params: "", enabled: |_| Ok(()), run, @@ -195,6 +241,8 @@ pub(crate) fn register_undoable_test_command(id: &'static str, run: Run) { label: id, menu: &[], shortcut: None, + shortcut_mac: None, + shortcut_other: None, params: "", enabled: |_| Ok(()), run, diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 5fa076e64..6bc8deaf3 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -93,16 +93,16 @@ 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!( + cmd!(platform "data.group", "Group", ["Data", "Outline"], - Some("Alt+Shift+Right"), + Some("Cmd+Shift+K"), 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| { + cmd!(platform "data.ungroup", "Ungroup", ["Data", "Outline"], Some("Cmd+Shift+J"), Some("Alt+Shift+Left"), "{rows? | cols? | by?: rows|cols, sheet?}", has_doc, |s, p| { group(s, p, false) }), cmd!( diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index e48fe8838..90ead5023 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -59,30 +59,29 @@ pub fn specs() -> Vec { cmd!(noundo "selection.set", "Select", [], None, "{range: \"A1:B2,D4\" | cell, active?: \"A1\", sheet?}", has_doc, selection_set), cmd!(noundo "selection.move", "Move Selection", [], None, "{dr, dc, extend?: bool, jump?: bool (Ctrl+arrow), page?: bool}", has_doc, selection_move), cmd!(noundo "selection.next", "Next Cell in Selection", [], None, "{forward?: true, byRow?: false} (Enter/Tab inside a selection)", has_doc, selection_next), - cmd!(noundo "edit.selectAll", "Select All", ["Edit"], Some("Cmd+A"), "{} (current region first, then the whole sheet)", has_doc, select_all), + cmd!(platform_noundo "edit.selectAll", "Select All", ["Edit"], Some("Cmd+A"), Some("Ctrl+A"), "{} (current region first, then the whole sheet)", has_doc, select_all), cmd!(noundo "selection.currentRegion", "Select Current Region", [], Some("Ctrl+Shift+8"), "{}", has_doc, select_region), cmd!(noundo "selection.row", "Select Entire Row", [], Some("Shift+Space"), "{}", has_doc, select_row), cmd!(noundo "selection.column", "Select Entire Column", [], Some("Ctrl+Space"), "{}", has_doc, select_col), cmd!(noundo "edit.goTo", "Go To…", ["Home", "Editing", "Find & Select"], Some("Ctrl+G"), "{reference: \"B5\" | \"Sheet2!A1:C3\" | name | \"Sheet2!Name\" | \"B14:C22,F19:G30\" | \"Price, Type\", record?: bool} (record: keep a reference without names among the recent ones) or {list: true} → {names, recent}", has_doc, go_to), cmd!(noundo "edit.goToSpecial", "Go To Special…", ["Home", "Editing", "Find & Select"], None, "{kind: blanks|constants|formulas|comments|lastCell|currentRegion|visible|conditionalFormats|dataValidation|errors|numbers|text}", has_doc, go_to_special), - cmd!(noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), "{steps?: 1}", can_undo, undo), - cmd!(noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Y"), "{steps?: 1}", can_redo, redo), - cmd!(noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, copy), - cmd!(noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, cut), - cmd!( - "edit.paste", + cmd!(platform_noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), Some("Ctrl+Z"), "{steps?: 1}", can_undo, undo), + cmd!(platform_noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Y"), Some("Ctrl+Y"), "{steps?: 1}", can_redo, redo), + cmd!(platform_noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), Some("Ctrl+C"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, copy), + cmd!(platform_noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), Some("Ctrl+X"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, cut), + cmd!(platform "edit.paste", "Paste", ["Home", "Clipboard"], - Some("Cmd+V"), + Some("Cmd+V"), Some("Ctrl+V"), "{at?: \"C3\", text?: \"tab-separated text from the system clipboard\", html?: \"the clipboard's HTML flavour; cells the source recorded as numbers (x:num, sdval) stay numbers\"}", has_doc, paste ), - cmd!( + cmd!(platform "edit.pasteSpecial", "Paste Special…", ["Home", "Clipboard"], - Some("Ctrl+Cmd+V"), + Some("Ctrl+Cmd+V"), 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 @@ -97,8 +96,8 @@ pub fn specs() -> Vec { s, p, "comments" )), cmd!("edit.clearHyperlinks", "Clear Hyperlinks", ["Home", "Editing", "Clear"], None, "{range?}", has_doc, |s, p| clear(s, p, "hyperlinks")), - cmd!("edit.fillDown", "Fill Down", ["Home", "Editing", "Fill"], Some("Cmd+D"), "{range?}", has_doc, |s, p| fill_dir(s, p, 1, 0)), - cmd!("edit.fillRight", "Fill Right", ["Home", "Editing", "Fill"], Some("Cmd+R"), "{range?}", has_doc, |s, p| fill_dir(s, p, 0, 1)), + cmd!(platform "edit.fillDown", "Fill Down", ["Home", "Editing", "Fill"], Some("Cmd+D"), Some("Ctrl+D"), "{range?}", has_doc, |s, p| fill_dir(s, p, 1, 0)), + cmd!(platform "edit.fillRight", "Fill Right", ["Home", "Editing", "Fill"], Some("Cmd+R"), Some("Ctrl+R"), "{range?}", has_doc, |s, p| fill_dir(s, p, 0, 1)), cmd!("edit.fillUp", "Fill Up", ["Home", "Editing", "Fill"], None, "{range?}", has_doc, |s, p| fill_dir(s, p, -1, 0)), cmd!("edit.fillLeft", "Fill Left", ["Home", "Editing", "Fill"], None, "{range?}", has_doc, |s, p| fill_dir(s, p, 0, -1)), cmd!( diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 744b6d136..854ccc08b 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -9,23 +9,22 @@ use super::*; pub fn specs() -> Vec { vec![ // Font - cmd!("home.bold", "Bold", ["Home", "Font"], Some("Cmd+B"), "{range?, on?: bool}", has_doc, |s, p| toggle( + cmd!(platform "home.bold", "Bold", ["Home", "Font"], Some("Cmd+B"), Some("Ctrl+B"), "{range?, on?: bool}", has_doc, |s, p| toggle( s, p, |st| st.font.bold, |st, v| st.font.bold = v )), - cmd!("home.italic", "Italic", ["Home", "Font"], Some("Cmd+I"), "{range?, on?}", has_doc, |s, p| toggle( + cmd!(platform "home.italic", "Italic", ["Home", "Font"], Some("Cmd+I"), Some("Ctrl+I"), "{range?, on?}", has_doc, |s, p| toggle( s, p, |st| st.font.italic, |st, v| st.font.italic = v )), - cmd!( - "home.underline", + cmd!(platform "home.underline", "Underline", ["Home", "Font"], - Some("Cmd+U"), + Some("Cmd+U"), Some("Ctrl+U"), "{range?, on?, style?: single|double|singleAccounting|doubleAccounting}", has_doc, underline @@ -87,8 +86,8 @@ 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)), + cmd!(platform "home.alignLeft", "Align Left", ["Home", "Alignment"], Some("Cmd+L"), None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)), + cmd!(platform "home.alignCenter", "Center", ["Home", "Alignment"], Some("Cmd+E"), None, "{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 9811fde9e..e1fc61efd 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -35,6 +35,8 @@ pub fn specs() -> Vec { label: "Allow Edit Ranges…", menu: &["Review", "Protect"], shortcut: None, + shortcut_mac: None, + shortcut_other: 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 7a80985b3..3c75e2575 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -28,23 +28,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial .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(" › "), - match (mac, c.id, c.shortcut) { - (false, "home.bold", Some(_)) => Some("Ctrl+B".to_owned()), - (false, "edit.pasteSpecial", Some(_)) => Some("Ctrl+Alt+V".to_owned()), - // These metadata shortcuts are only bound on macOS. - (false, "home.alignLeft" | "home.alignCenter", Some(_)) => None, - // macOS uses Command+Shift+K/J, unlike the engine's Windows metadata. - (true, "data.group", Some(_)) => Some("Cmd+Shift+K".to_owned()), - (true, "data.ungroup", Some(_)) => Some("Cmd+Shift+J".to_owned()), - (_, _, keys) => keys.map(str::to_string), - }, - ) - }) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), c.shortcut_for_platform(mac).map(str::to_string))) .take(40) .collect(); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 110a1d032..2b2f40e95 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -31,7 +31,7 @@ const TOOLTIP_SHORTCUTS: &[(&str, &str, &str)] = &[ ("Table", "⌘T", "Ctrl+T"), ("Insert Link", "⌘K", "Ctrl+K"), ("Filter", "⌘⇧F", "Ctrl+Shift+L"), - ("Flash Fill", "Control+E", "Ctrl+E"), + ("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+`"), @@ -716,7 +716,7 @@ mod tooltip_shortcut_tests { assert_eq!(tooltip_shortcut("Search commands", false), None); for (name, mac, other) in super::TOOLTIP_SHORTCUTS { assert!(!other.contains(['⌘', '⇧', '⌥']), "{name}: {other}"); - assert!(mac.starts_with('⌘') || mac.starts_with('⌃') || (*name == "Flash Fill" && *mac == "Control+E"), "{name}: {mac}"); + assert!(mac.starts_with('⌘') || mac.starts_with('⌃'), "{name}: {mac}"); } } @@ -724,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 53fb6c933..a131002e5 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -99,6 +99,11 @@ fn command_search_shows_engine_shortcuts() { 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" }), @@ -144,22 +149,36 @@ fn command_palette_shortcuts_come_from_engine_metadata() { let session = gridcraft_engine::Session::new(); let commands = session.commands(); - 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"), - ("data.group", "Alt+Shift+Right"), - ("data.ungroup", "Alt+Shift+Left"), + for (id, mac, other) in [ + ("home.bold", Some("Cmd+B"), Some("Ctrl+B")), + ("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, Some(expected), "{id}"); + assert_eq!(command.shortcut_for_platform(true), mac, "{id}: macOS"); + assert_eq!(command.shortcut_for_platform(false), other, "{id}: Windows/Linux"); } + + 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] @@ -182,3 +201,41 @@ fn command_search_shows_current_region_shortcut() { 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"); + } +} From e945d7b76da909fb16fb21d5c092c14a6329a788 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:28:18 +0700 Subject: [PATCH 11/12] fix(ui): preserve portable shortcuts and macOS overrides --- crates/engine/src/cmd.rs | 52 ++++---------------- crates/engine/src/cmd/data.rs | 36 +++++++++----- crates/engine/src/cmd/edit.rs | 40 ++++++++------- crates/engine/src/cmd/format.rs | 19 ++++--- crates/engine/src/cmd/review.rs | 1 - crates/ui-egui/src/dialogs/command_search.rs | 2 +- crates/ui-egui/tests/it/dialog_focus.rs | 10 +++- 7 files changed, 78 insertions(+), 82 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 3bd25929e..9ef498ea9 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -41,7 +41,6 @@ pub struct CommandSpec { pub menu: &'static [&'static str], pub shortcut: Option<&'static str>, pub shortcut_mac: Option<&'static str>, - pub shortcut_other: Option<&'static str>, pub params: &'static str, pub enabled: Enabled, pub run: Run, @@ -56,7 +55,6 @@ pub struct CommandInfo { pub menu: Vec<&'static str>, pub shortcut: Option<&'static str>, pub shortcut_mac: Option<&'static str>, - pub shortcut_other: Option<&'static str>, pub params: &'static str, pub enabled: bool, #[serde(skip_serializing_if = "Option::is_none")] @@ -64,9 +62,13 @@ pub struct CommandInfo { } impl CommandInfo { - /// Shortcut declared by the engine for the active platform. - pub fn shortcut_for_platform(&self, mac: bool) -> Option<&'static str> { - if mac { self.shortcut_mac } else { self.shortcut_other } + /// 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 { self.shortcut }?; + if mac { + return Some(keys.to_owned()); + } + Some(keys.split('+').map(|part| if part == "Cmd" { "Ctrl" } else { part }).collect::>().join("+")) } } @@ -79,7 +81,6 @@ impl CommandSpec { menu: self.menu.to_vec(), shortcut: self.shortcut, shortcut_mac: self.shortcut_mac, - shortcut_other: self.shortcut_other, params: self.params, enabled: e.is_ok(), disabled_reason: e.err(), @@ -129,45 +130,14 @@ pub fn has_clipboard(s: &Session) -> std::result::Result<(), String> { } macro_rules! cmd { - (platform $id:literal, $label:literal, [$($m:literal),*], $mac:expr, $other:expr, $params:literal, $en:expr, $run:expr) => { - $crate::cmd::CommandSpec { - id: $id, - label: $label, - menu: &[$($m),*], - shortcut: $mac, - shortcut_mac: $mac, - shortcut_other: $other, - params: $params, - enabled: $en, - run: $run, - journal: true, - undoable: true, - } - }; ($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, shortcut_mac: $sc, shortcut_other: $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, shortcut_mac: $sc, shortcut_other: $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, shortcut_mac: $sc, shortcut_other: $sc, params: $params, enabled: $en, run: $run, journal: true, undoable: false } - }; - (platform_noundo $id:literal, $label:literal, [$($m:literal),*], - $mac:expr, $other:expr, $params:literal, $en:expr, $run:expr) => { - $crate::cmd::CommandSpec { - id: $id, - label: $label, - menu: &[$($m),*], - shortcut: $mac, - shortcut_mac: $mac, - shortcut_other: $other, - 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; @@ -223,7 +193,6 @@ pub fn register_test_command(id: &'static str, run: Run) { menu: &[], shortcut: None, shortcut_mac: None, - shortcut_other: None, params: "", enabled: |_| Ok(()), run, @@ -242,7 +211,6 @@ pub(crate) fn register_undoable_test_command(id: &'static str, run: Run) { menu: &[], shortcut: None, shortcut_mac: None, - shortcut_other: None, params: "", enabled: |_| Ok(()), run, diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 6bc8deaf3..5e0d4c4c1 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -93,18 +93,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!(platform - "data.group", - "Group", - ["Data", "Outline"], - Some("Cmd+Shift+K"), 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!(platform "data.ungroup", "Ungroup", ["Data", "Outline"], Some("Cmd+Shift+J"), 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 90ead5023..4f6eeb247 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -59,33 +59,37 @@ pub fn specs() -> Vec { cmd!(noundo "selection.set", "Select", [], None, "{range: \"A1:B2,D4\" | cell, active?: \"A1\", sheet?}", has_doc, selection_set), cmd!(noundo "selection.move", "Move Selection", [], None, "{dr, dc, extend?: bool, jump?: bool (Ctrl+arrow), page?: bool}", has_doc, selection_move), cmd!(noundo "selection.next", "Next Cell in Selection", [], None, "{forward?: true, byRow?: false} (Enter/Tab inside a selection)", has_doc, selection_next), - cmd!(platform_noundo "edit.selectAll", "Select All", ["Edit"], Some("Cmd+A"), Some("Ctrl+A"), "{} (current region first, then the whole sheet)", has_doc, select_all), + cmd!(noundo "edit.selectAll", "Select All", ["Edit"], Some("Cmd+A"), "{} (current region first, then the whole sheet)", has_doc, select_all), cmd!(noundo "selection.currentRegion", "Select Current Region", [], Some("Ctrl+Shift+8"), "{}", has_doc, select_region), cmd!(noundo "selection.row", "Select Entire Row", [], Some("Shift+Space"), "{}", has_doc, select_row), cmd!(noundo "selection.column", "Select Entire Column", [], Some("Ctrl+Space"), "{}", has_doc, select_col), cmd!(noundo "edit.goTo", "Go To…", ["Home", "Editing", "Find & Select"], Some("Ctrl+G"), "{reference: \"B5\" | \"Sheet2!A1:C3\" | name | \"Sheet2!Name\" | \"B14:C22,F19:G30\" | \"Price, Type\", record?: bool} (record: keep a reference without names among the recent ones) or {list: true} → {names, recent}", has_doc, go_to), cmd!(noundo "edit.goToSpecial", "Go To Special…", ["Home", "Editing", "Find & Select"], None, "{kind: blanks|constants|formulas|comments|lastCell|currentRegion|visible|conditionalFormats|dataValidation|errors|numbers|text}", has_doc, go_to_special), - cmd!(platform_noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), Some("Ctrl+Z"), "{steps?: 1}", can_undo, undo), - cmd!(platform_noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Y"), Some("Ctrl+Y"), "{steps?: 1}", can_redo, redo), - cmd!(platform_noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), Some("Ctrl+C"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, copy), - cmd!(platform_noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), Some("Ctrl+X"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, cut), - cmd!(platform "edit.paste", + cmd!(noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), "{steps?: 1}", can_undo, undo), + cmd!(noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Y"), "{steps?: 1}", can_redo, redo), + cmd!(noundo "edit.copy", "Copy", ["Home", "Clipboard"], Some("Cmd+C"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, copy), + cmd!(noundo "edit.cut", "Cut", ["Home", "Clipboard"], Some("Cmd+X"), "{range?, html?: bool} → {range, text, html?} (html: a styled HTML table, only when requested and small enough)", has_doc, cut), + cmd!( + "edit.paste", "Paste", ["Home", "Clipboard"], - Some("Cmd+V"), Some("Ctrl+V"), + Some("Cmd+V"), "{at?: \"C3\", text?: \"tab-separated text from the system clipboard\", html?: \"the clipboard's HTML flavour; cells the source recorded as numbers (x:num, sdval) stay numbers\"}", has_doc, paste ), - cmd!(platform - "edit.pasteSpecial", - "Paste Special…", - ["Home", "Clipboard"], - Some("Ctrl+Cmd+V"), 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 - ), + 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")), @@ -96,8 +100,8 @@ pub fn specs() -> Vec { s, p, "comments" )), cmd!("edit.clearHyperlinks", "Clear Hyperlinks", ["Home", "Editing", "Clear"], None, "{range?}", has_doc, |s, p| clear(s, p, "hyperlinks")), - cmd!(platform "edit.fillDown", "Fill Down", ["Home", "Editing", "Fill"], Some("Cmd+D"), Some("Ctrl+D"), "{range?}", has_doc, |s, p| fill_dir(s, p, 1, 0)), - cmd!(platform "edit.fillRight", "Fill Right", ["Home", "Editing", "Fill"], Some("Cmd+R"), Some("Ctrl+R"), "{range?}", has_doc, |s, p| fill_dir(s, p, 0, 1)), + cmd!("edit.fillDown", "Fill Down", ["Home", "Editing", "Fill"], Some("Cmd+D"), "{range?}", has_doc, |s, p| fill_dir(s, p, 1, 0)), + cmd!("edit.fillRight", "Fill Right", ["Home", "Editing", "Fill"], Some("Cmd+R"), "{range?}", has_doc, |s, p| fill_dir(s, p, 0, 1)), cmd!("edit.fillUp", "Fill Up", ["Home", "Editing", "Fill"], None, "{range?}", has_doc, |s, p| fill_dir(s, p, -1, 0)), cmd!("edit.fillLeft", "Fill Left", ["Home", "Editing", "Fill"], None, "{range?}", has_doc, |s, p| fill_dir(s, p, 0, -1)), cmd!( diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 854ccc08b..166afc72d 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -9,22 +9,23 @@ use super::*; pub fn specs() -> Vec { vec![ // Font - cmd!(platform "home.bold", "Bold", ["Home", "Font"], Some("Cmd+B"), Some("Ctrl+B"), "{range?, on?: bool}", has_doc, |s, p| toggle( + cmd!("home.bold", "Bold", ["Home", "Font"], Some("Cmd+B"), "{range?, on?: bool}", has_doc, |s, p| toggle( s, p, |st| st.font.bold, |st, v| st.font.bold = v )), - cmd!(platform "home.italic", "Italic", ["Home", "Font"], Some("Cmd+I"), Some("Ctrl+I"), "{range?, on?}", has_doc, |s, p| toggle( + cmd!("home.italic", "Italic", ["Home", "Font"], Some("Cmd+I"), "{range?, on?}", has_doc, |s, p| toggle( s, p, |st| st.font.italic, |st, v| st.font.italic = v )), - cmd!(platform "home.underline", + cmd!( + "home.underline", "Underline", ["Home", "Font"], - Some("Cmd+U"), Some("Ctrl+U"), + Some("Cmd+U"), "{range?, on?, style?: single|double|singleAccounting|doubleAccounting}", has_doc, underline @@ -86,8 +87,14 @@ pub fn specs() -> Vec { format_cells ), // Alignment - cmd!(platform "home.alignLeft", "Align Left", ["Home", "Alignment"], Some("Cmd+L"), None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)), - cmd!(platform "home.alignCenter", "Center", ["Home", "Alignment"], Some("Cmd+E"), None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Center)), + CommandSpec { + shortcut_mac: Some("Cmd+L"), + ..cmd!("home.alignLeft", "Align Left", ["Home", "Alignment"], None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)) + }, + CommandSpec { + shortcut_mac: Some("Cmd+E"), + ..cmd!("home.alignCenter", "Center", ["Home", "Alignment"], None, "{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 e1fc61efd..879704a1e 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -36,7 +36,6 @@ pub fn specs() -> Vec { menu: &["Review", "Protect"], shortcut: None, shortcut_mac: None, - shortcut_other: 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 3c75e2575..55953ecef 100644 --- a/crates/ui-egui/src/dialogs/command_search.rs +++ b/crates/ui-egui/src/dialogs/command_search.rs @@ -28,7 +28,7 @@ pub(super) fn command_search(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dial .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(" › "), c.shortcut_for_platform(mac).map(str::to_string))) + .map(|c| (c.id.to_string(), c.label.to_string(), c.menu.join(" › "), c.shortcut_for_platform(mac))) .take(40) .collect(); diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index a131002e5..33cb26b7f 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -151,6 +151,7 @@ fn command_palette_shortcuts_come_from_engine_metadata() { 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")), @@ -172,8 +173,13 @@ fn command_palette_shortcuts_come_from_engine_metadata() { ("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, "{id}: macOS"); - assert_eq!(command.shortcut_for_platform(false), other, "{id}: Windows/Linux"); + 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")] { + 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(); From b408b709863f4c9c4e968efa43f42e7f79228e69 Mon Sep 17 00:00:00 2001 From: AnhNgoo <110978888+AnhNgoo@users.noreply.github.com> Date: Mon, 2 Nov 2026 13:43:29 +0700 Subject: [PATCH 12/12] fix(ui): preserve alignment shortcuts for CLI consumers --- crates/engine/src/cmd.rs | 10 +++++++++- crates/engine/src/cmd/format.rs | 4 ++-- crates/ui-egui/tests/it/dialog_focus.rs | 4 +++- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 9ef498ea9..99da8f67b 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -64,7 +64,15 @@ pub struct CommandInfo { 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 { self.shortcut }?; + 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()); } diff --git a/crates/engine/src/cmd/format.rs b/crates/engine/src/cmd/format.rs index 166afc72d..d8b453891 100644 --- a/crates/engine/src/cmd/format.rs +++ b/crates/engine/src/cmd/format.rs @@ -89,11 +89,11 @@ pub fn specs() -> Vec { // Alignment CommandSpec { shortcut_mac: Some("Cmd+L"), - ..cmd!("home.alignLeft", "Align Left", ["Home", "Alignment"], None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Left)) + ..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"], None, "{range?}", has_doc, |s, p| halign(s, p, HAlign::Center)) + ..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)), diff --git a/crates/ui-egui/tests/it/dialog_focus.rs b/crates/ui-egui/tests/it/dialog_focus.rs index 33cb26b7f..ca3bfe925 100644 --- a/crates/ui-egui/tests/it/dialog_focus.rs +++ b/crates/ui-egui/tests/it/dialog_focus.rs @@ -177,7 +177,9 @@ fn command_palette_shortcuts_come_from_engine_metadata() { 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")] { + 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"); }