Merge pull request #5110 from mockydevs/feat/effect-shortcuts-3296

Fix #3296: Effect menu items take keyboard shortcuts of their own

crates/ui-egui/src/keymap.rs

@@ -38,28 +38,38 @@ pub const TOGGLES: &[&str] = &[
 
 /// The command an entry key runs.
 pub fn command_of(key: &str) -> CommandRef {
-    match key.strip_prefix("panel:") {
-        Some(panel) => CommandRef::with_params("window.panel", json!({ "panel": panel })),
-        None => CommandRef::new(key),
+    if let Some(panel) = key.strip_prefix("panel:") {
+        return CommandRef::with_params("window.panel", json!({ "panel": panel }));
     }
+    // An Effect menu item (#3296): what its item runs, its dialog unless it has no options.
+    if let Some(effect) = key.strip_prefix("effect:") {
+        let command = shortcut_editor::effect_items().iter().find(|e| e.effect == effect).map_or("effect.dialog", |e| e.command);
+        return CommandRef::with_params(command, json!({ "effect": effect }));
+    }
+    CommandRef::new(key)
 }
 
 /// The entry key of a command: `panel:<id>` for `window.panel {panel}`.
 pub fn entry_key(command: &CommandRef) -> String {
     match (command.id.as_str(), command.params.get("panel").and_then(Value::as_str)) {
         ("window.panel", Some(panel)) => format!("panel:{panel}"),
+        ("effect.dialog" | "effect.apply", _) if command.params.get("effect").and_then(Value::as_str).is_some() => {
+            format!("effect:{}", command.params.get("effect").and_then(Value::as_str).unwrap_or_default())
+        }
         _ => command.key(),
     }
 }
 
 /// The [`Overrides`] key of an entry key.
 pub fn stored_key(key: &str) -> String {
-    if key.starts_with("panel:") { command_of(key).key() } else { key.to_string() }
+    if key.starts_with("panel:") || key.starts_with("effect:") { command_of(key).key() } else { key.to_string() }
 }
 
 /// The entry key of an [`Overrides`] key.
 pub fn entry_of_stored(key: &str) -> String {
-    CommandRef::parse_key(key).filter(|c| c.id == "window.panel").map_or_else(|| key.to_string(), |c| entry_key(&c))
+    CommandRef::parse_key(key)
+        .filter(|c| matches!(c.id.as_str(), "window.panel" | "effect.dialog" | "effect.apply"))
+        .map_or_else(|| key.to_string(), |c| entry_key(&c))
 }
 
 // ---------- chord texts ----------

crates/ui-egui/src/menus/labels.rs

@@ -211,11 +211,12 @@ pub(super) fn unlock_item_label(app: &VectorcraftApp, id: &str, p: &Value, lang:
     Some(crate::panels::layers::shown_name(app, n, lang))
 }
 
-/// The shortcut a menu item shows: its command's (items without params), or a panel's for the
-/// Window menu's `window.panel {panel}` items.
+/// The shortcut a menu item shows: its command's (items without params), a panel's for the
+/// Window menu's `window.panel {panel}` items, or an effect's for the Effect menu's items.
 pub fn item_shortcut(id: &str, p: &Value) -> Option<&'static str> {
     match (id, p.get("panel").and_then(Value::as_str)) {
         ("window.panel", Some(panel)) => crate::shortcut_editor::panel_shortcut(panel),
+        ("effect.dialog" | "effect.apply", _) => p.get("effect").and_then(Value::as_str).and_then(crate::shortcut_editor::effect_shortcut),
         _ if p.is_null() => shortcut_of(id),
         _ => None,
     }

crates/ui-egui/src/shortcut_editor.rs

@@ -363,6 +363,13 @@ pub fn entries() -> &'static [Entry] {
             group: "Window".into(),
             is_tool: false,
         }));
+        // Each effect of the Effect menu, by its own key: they all run `effect.dialog` (#3296).
+        v.extend(effect_items().iter().map(|e| Entry {
+            key: format!("effect:{}", e.effect),
+            label: e.label.clone(),
+            group: e.group.clone(),
+            is_tool: false,
+        }));
         for (id, label, sc, params) in menus::UI_COMMANDS {
             if !seen.insert(id) || (sc.is_empty() && !params.starts_with("{}") && !params.starts_with("{path")) {
                 continue;
@@ -373,6 +380,55 @@ pub fn entries() -> &'static [Entry] {
     })
 }
 
+/// One Effect menu item a shortcut can be given (#3296): the effect's id, the command its item runs
+/// (`effect.dialog`, or `effect.apply` for an effect without options), its label and its menu path.
+#[derive(Clone, Debug)]
+pub struct EffectItem {
+    pub effect: String,
+    pub command: &'static str,
+    pub label: String,
+    pub group: String,
+}
+
+/// The Effect menu's effect items, in menu order, each once.
+pub fn effect_items() -> &'static [EffectItem] {
+    static E: OnceLock<Vec<EffectItem>> = OnceLock::new();
+    E.get_or_init(|| {
+        fn walk(items: &[menus::Item], path: &mut Vec<String>, seen: &mut HashSet<String>, out: &mut Vec<EffectItem>) {
+            for it in items {
+                match it {
+                    menus::Item::Cmd(label, id @ ("effect.dialog" | "effect.apply"), p) => {
+                        if let Some(effect) = p.get("effect").and_then(serde_json::Value::as_str)
+                            && seen.insert(effect.to_string())
+                        {
+                            out.push(EffectItem { effect: effect.to_string(), command: id, label: label.to_string(), group: path.join(" › ") });
+                        }
+                    }
+                    menus::Item::Sub(name, children) => {
+                        path.push(name.to_string());
+                        walk(children, path, seen, out);
+                        path.pop();
+                    }
+                    _ => {}
+                }
+            }
+        }
+        let mut out = vec![];
+        let mut seen = HashSet::new();
+        for (name, items) in menus::menu_tree() {
+            if name == "Effect" {
+                walk(&items, &mut vec![name.to_string()], &mut seen, &mut out);
+            }
+        }
+        out
+    })
+}
+
+/// Effective shortcut of the Effect menu item of `effect` (`effect:<id>`).
+pub fn effect_shortcut(effect: &str) -> Option<&'static str> {
+    effective(&format!("effect:{effect}"))
+}
+
 /// The group of a command no menu holds: the first part of its id, capitalised (`edit.x` → "Edit").
 fn id_group(id: &str) -> String {
     let mut c = id.split('.').next().unwrap_or_default().chars();

crates/ui-egui/src/shortcuts.rs

@@ -112,6 +112,13 @@ pub(crate) fn all_shortcuts() -> Vec<(KeyboardShortcut, &'static str, serde_json
             v.push((sc, "window.panel", json!({ "panel": panel })));
         }
     }
+    // Effect menu items with shortcuts of their own (#3296): a chord bound to one is a command's
+    // too, so the tool doesn't take Cmd+Enter bound to an effect before the keymap runs it.
+    for e in crate::shortcut_editor::effect_items() {
+        if let Some(sc) = crate::shortcut_editor::effect_shortcut(&e.effect).and_then(parse) {
+            v.push((sc, e.command, json!({ "effect": e.effect })));
+        }
+    }
     // Most specific (most modifiers) first: `paste_command` takes the first V chord the held keys
     // match logically, so Cmd+Shift+V isn't taken by Cmd+V.
     v.sort_by_key(|(sc, ..)| {

crates/ui-egui/src/shortcuts/tests.rs

@@ -2007,3 +2007,49 @@ fn all_registered_shortcuts_parse() {
         }
     }
 }
+
+/// #3296: each Effect menu item can be given a shortcut of its own (`effect:<id>`): the editor
+/// lists it under its menu path, the menu shows the shortcut, and pressing it opens that effect's
+/// dialog for the selection.
+#[test]
+fn an_effect_menu_item_takes_a_shortcut_of_its_own() {
+    let entry = crate::shortcut_editor::entries().iter().find(|e| e.key == "effect:distort.transform").expect("Transform is listed");
+    assert_eq!(entry.group, "Effect › Distort & Transform");
+    assert_eq!(crate::keymap::command_of("effect:distort.transform").id, "effect.dialog");
+    let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());
+    app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap();
+    let id = app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap()["id"].clone();
+    app.session.execute("select.set", &json!({"ids": [id]})).unwrap();
+    crate::keymap::set_override(&mut app.ui.shortcut_overrides, "effect:distort.transform", "Cmd+Alt+Shift+F11");
+    crate::shortcut_editor::sync(&app.ui);
+    assert_eq!(crate::menus::item_shortcut("effect.dialog", &json!({"effect": "distort.transform"})), Some("Cmd+Alt+Shift+F11"));
+    let f11 =
+        egui::Event::Key { key: Key::F11, physical_key: None, pressed: true, repeat: false, modifiers: cmd() | Modifiers::ALT | Modifiers::SHIFT };
+    frame(&mut app, vec![f11]);
+    let d = app.ui.dialog.as_ref().expect("the effect's dialog opens");
+    assert_eq!(d.fields.get("__effect").and_then(serde_json::Value::as_str), Some("distort.transform"), "{:?}", d.kind);
+    crate::shortcut_editor::sync(&crate::state::UiState::default());
+}
+
+/// #3296 review: an effect bound to Cmd+Enter runs the effect; the Selection tool doesn't take the
+/// Enter first and open its Move dialog.
+#[test]
+fn cmd_enter_bound_to_an_effect_runs_the_effect_not_the_tool() {
+    let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());
+    app.session.execute("file.new", &json!({"width": 200, "height": 200})).unwrap();
+    let id = app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 20, "height": 20})).unwrap()["id"].clone();
+    app.select_tool("selection");
+    app.session.execute("select.set", &json!({"ids": [id]})).unwrap();
+    crate::keymap::set_override(&mut app.ui.shortcut_overrides, "effect:distort.transform", "Cmd+Enter");
+    crate::shortcut_editor::sync(&app.ui);
+    assert!(
+        crate::shortcuts::all_shortcuts()
+            .iter()
+            .any(|(sc, c, p)| sc.logical_key == Key::Enter && *c == "effect.dialog" && p["effect"] == "distort.transform"),
+        "the effect's chord is among the command shortcuts"
+    );
+    frame(&mut app, press(Key::Enter, cmd()));
+    let d = app.ui.dialog.as_ref().expect("a dialog opens");
+    assert_eq!(d.fields.get("__effect").and_then(serde_json::Value::as_str), Some("distort.transform"), "the effect's, not Move: {:?}", d.kind);
+    crate::shortcut_editor::sync(&crate::state::UiState::default());
+}

docs/target-app-parity.md

@@ -1505,10 +1505,15 @@ A VectorCraft addition, not a feature of the reference app, so no area score mov
 
 A VectorCraft addition (it replaces a script some users ran), so no area score moves. Object › Plug-ins › Color Effects › Create Swatch Legend… (`addon.color.swatchLegend`) opens a dialog (columns, chip size, gap, values, selected swatches only, with a preview) and draws a chip per colour swatch (the selected ones, else all) filled with and linked to its swatch, labelled with its name and values (HEX, RGB, CMYK, Lab or Grayscale; by default its own model; spot swatches marked), black on light chips and white on dark ones, in a grid of chosen columns, chip size and spacing, as one selected group in one undo step (#4919). Tests: `cmd::addons::color::legend::tests` (engine), `addons::color::legend::tests::create_swatch_legend_opens_its_dialog_previews_and_runs_on_ok` (ui-egui).
 
+## Keyboard shortcuts for Effect menu items (2026-11-01)
+
+Edit › Keyboard Shortcuts lists every item of the Effect menu under its menu path (Effect › Distort & Transform › Transform…) and gives each one a shortcut of its own (stored as `effect:<id>`; they all run `effect.dialog {effect}`, or `effect.apply` for an effect without options, so a shortcut on the command alone couldn't tell them apart). The menu shows the shortcut, and pressing it opens that effect's dialog for the selection (#3296). Test: `shortcuts::tests::an_effect_menu_item_takes_a_shortcut_of_its_own`.
+
 ## Revision history
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-11-01 | minor | Shortcuts for Effect menu items (#3296); evidence added; scores unchanged. |
 | 2026-11-01 | minor | Color Effects add-on (a VectorCraft addition): Find Nearest Swatch (#4923); evidence added; scores unchanged. |
 | 2026-11-01 | minor | Create Swatch Legend add-on under Object › Plug-ins › Color Effects (`addon.color.swatchLegend`, #4919); evidence added; scores unchanged. |
 | 2026-11-01 | minor | Document Tools add-on (a VectorCraft addition): Export All Open Documents as PDF or SVG (#4920); evidence added; scores unchanged. |

docs/ui-parity.md

@@ -400,6 +400,7 @@ separate. Scores and hours unchanged. [Behavior and command details](development
 
 | Date | Change | Summary |
 |---|---|---|
+| 2026-11-01 | minor | Keyboard Shortcuts: every Effect menu item (Distort & Transform › Transform…, Stylize › Drop Shadow…, …) is listed under its menu path and takes a shortcut of its own, which the menu shows and which opens that effect's dialog (or applies an effect without options) (#3296); scores and estimates unchanged |
 | 2026-10-31 | minor | Units › East Asian Type acts (#394): Q and Ha (a quarter of a millimetre each) are units that preference offers; 4 Preferences options remain stored but unused; scores and estimates unchanged |
 | 2026-11-01 | minor | Color Effects add-on (a VectorCraft addition): Find Nearest Swatch (#4923); scores and estimates unchanged |
 | 2026-11-01 | minor | Artboards panel: a row dragged above or below another reorders the artboards (#4687); scores and estimates unchanged |