From 9299c64d42fcc5fafd02bf787003a054a8355a61 Mon Sep 17 00:00:00 2001
From: Don Omondi <282319807+mockydevs@users.noreply.github.com>
Date: Sun, 1 Nov 2026 17:15:48 +0300
Subject: [PATCH] Effect shortcuts: count them among command shortcuts for
 Enter's precedence

An effect bound to Cmd+Enter wasn't among all_shortcuts, so the active tool took the Enter
before the keymap ran the effect. Effect items' own shortcuts are now listed with the rest.
---
 crates/ui-egui/src/shortcuts.rs       |  7 +++++++
 crates/ui-egui/src/shortcuts/tests.rs | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs
index 0d4023963..f93bde770 100644
--- a/crates/ui-egui/src/shortcuts.rs
+++ b/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, ..)| {
diff --git a/crates/ui-egui/src/shortcuts/tests.rs b/crates/ui-egui/src/shortcuts/tests.rs
index baeda1791..804e8f9b8 100644
--- a/crates/ui-egui/src/shortcuts/tests.rs
+++ b/crates/ui-egui/src/shortcuts/tests.rs
@@ -2030,3 +2030,26 @@ fn an_effect_menu_item_takes_a_shortcut_of_its_own() {
     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());
+}