Merge pull request #386 from theironchef/fix/selection-tools-open-move-dialog

Enter opens the tool's dialog; the selection tools open Move

ROADMAP.md

@@ -119,7 +119,7 @@ grade by behaviour against `plan/illustrator/`, not by whether a menu item exist
   - **Editing type:** double-clicking type with a selection tool switches to the Type tool with the caret where it was clicked.
   - **Temporary tools:** Cmd with any other tool drags with the selection tool used last (Selection, Direct Selection or Group Selection). Held Space pans and Cmd+Space zooms (Cmd+Alt+Space out) whatever the tool.
   - **Panel drag and drop:** art dragged off the canvas onto the Symbols panel becomes a symbol, onto the Swatches panel a pattern swatch and onto the Brushes panel an Art brush; a symbol dragged onto the canvas places an instance where it is dropped, and a brush dragged onto a path is applied to it.
-  - **Tool double-clicks:** double-clicking a tool's button opens its options: Hand fits the artboard in the window, Zoom shows 100 %, Rotate, Scale, Reflect and Shear open their Object › Transform dialogs, Gradient, Eyedropper, Blend and the Liquify tools their panels or options.
+  - **Tool double-clicks:** double-clicking a tool's button opens its options: Hand fits the artboard in the window, Zoom shows 100 %, Rotate, Scale, Reflect and Shear open their Object › Transform dialogs, the Selection, Direct Selection and Group Selection tools the Move dialog (Enter opens these seven tools' dialogs too), Gradient, Eyedropper, Blend and the Liquify tools their panels or options.
 - **Layers panel:** every layer, sublayer, group and object is a clickable row at any depth: a click highlights it and makes its layer current (new art goes there), Shift-click a range, Ctrl/Cmd-click toggles. Sublayers are no longer treated as objects: clicks, marquees, Select All and the Select menu take the art inside them, and each sublayer selects in its own colour. Selection squares on rows with selected art (click selects a row's art, Shift adds or removes; dragging the square moves the selection to another row, Alt copies), the eye and lock on every row with drag-down-the-column (one undo step), Ctrl/Cmd-click an eye for per-layer Outline, Alt-click an eye or lock to hide or lock the other layers (again: show or unlock them all), triangles with Alt to open or close everything inside, drag and drop rows above, below or into layers and groups with a drop line (Alt copies, the trash deletes), inline rename, Delete key on highlighted rows. Buttons and the full panel menu: New Layer/Sublayer (Alt: options, Ctrl: on top), Duplicate, Delete, Options for Selection, clipping mask, isolation, Locate Object, Merge Selected, Flatten Artwork, Collect in New Layer, Release to Layers (Sequence and Build), Reverse Order, Template, Hide/Outline/Lock Others and Show/Preview/Unlock All, Paste Remembers Layers, Panel Options (row size, thumbnails per kind, Show Layers Only). Layer Options: name, colour (presets or custom), Template, Lock, Show, Print, Preview (outline drawing and clicking) and Dim Images to %. Every operation is a command (`layer.move`, `layer.highlight`, `layer.merge`…) with validation against cycles and invalid nesting.
 - **Drawing aids:** Smart Guides and snapping (moving, and resizing by the bounding-box handles: the dragged edges align with the other objects; the Artboard tool snaps artboards, and their bleed, to other artboards, bleeds and art, the grid or whole pixels while moving, copying and resizing them, and the Transform panel shows and edits the active artboard's X/Y/W/H), guides dragged out of the rulers, and Draw Normal / Behind / Inside modes.
 - **Views:** the status bar's artboard navigator (first, previous, next, last; `view.goToArtboard`) goes to an artboard and fits it, and Fit Artboard in Window and Actual Size show the navigator's artboard. The Artboards panel highlights that active artboard: clicking a row makes it active (the Artboard tool's too, while it is chosen) as does clicking an artboard with the Artboard tool, double-clicking its number also fits it in the window and double-clicking its name renames it.

crates/ui-egui/src/menus.rs

@@ -276,7 +276,7 @@ pub const UI_COMMANDS: &[(&str, &str, &str, &str)] = &[
         "tool.options",
         "Tool Options…",
         "",
-        "{tool: id} what double-clicking a tool button opens: hand → fits the artboard in the window (view.fitArtboard), zoom → 100% (view.actualSize), rotate|scale|reflect|shear → that Object › Transform dialog (dialog `rotate`, `scale`, `reflect` or `shear`; error `nothing selected` without a selection), gradient → the Gradient panel (window.panel), eyedropper → Eyedropper Options (dialog `eyedropperOptions`, fields sampleSize, pickUp, apply; OK runs eyedropper.setOptions), printTiling → resets the print tiling (print.tiling.set {reset: true}), warp|twirl|pucker|bloat|scallop|crystallize|wrinkle → that tool's Tool Options (dialog `liquifyOptions`, fields tool, width, height, angle, intensity %, usePressure, detail, simplify, simplifyOn, rate, complexity, horizontal %, vertical %, affectAnchors, affectIn, affectOut, showBrush; OK runs tool.setOption {tool, values})",
+        "{tool: id} what double-clicking a tool button opens: hand → fits the artboard in the window (view.fitArtboard), zoom → 100% (view.actualSize), rotate|scale|reflect|shear → that Object › Transform dialog (dialog `rotate`, `scale`, `reflect` or `shear`; error `nothing selected` without a selection), selection|directSelection|groupSelection → the Move dialog (dialog `move`; the same error), gradient → the Gradient panel (window.panel), eyedropper → Eyedropper Options (dialog `eyedropperOptions`, fields sampleSize, pickUp, apply; OK runs eyedropper.setOptions), printTiling → resets the print tiling (print.tiling.set {reset: true}), warp|twirl|pucker|bloat|scallop|crystallize|wrinkle → that tool's Tool Options (dialog `liquifyOptions`, fields tool, width, height, angle, intensity %, usePressure, detail, simplify, simplifyOn, rate, complexity, horizontal %, vertical %, affectAnchors, affectIn, affectOut, showBrush; OK runs tool.setOption {tool, values})",
     ),
     (
         "ui.colorGuideOptions",

crates/ui-egui/src/shortcuts.rs

@@ -211,7 +211,9 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context) {
             // A key the tool claims (Esc with a loaded place cursor) is only the tool's.
             let claimed = app.session.tool_claims_key(tk, view);
             let r = app.session.tool_key(tk, Mods::default(), view);
-            if claimed {
+            // Enter also takes what the tool asks of the UI: Rotate, Scale, Reflect and Shear open
+            // their dialog with it.
+            if claimed || (k == Key::Enter && !busy) {
                 crate::canvas::apply_requests(app, r);
             }
             if k == Key::Escape && !busy && !claimed {
@@ -223,6 +225,19 @@ pub fn handle(app: &mut VectorcraftApp, ctx: &egui::Context) {
                     app.ui.flyout = None;
                 }
             }
+            if k == Key::Enter && !busy && !claimed {
+                // Enter with a selection tool opens the Move dialog, as a double-click on its button
+                // does.
+                let tool = app.session.tool_id();
+                if app.ui.dialog.is_none() && crate::canvas::is_selection_tool(tool) {
+                    // Nothing selected: it fails and nothing opens (the menu item is disabled then too).
+                    let _ = crate::toolbar::open_options(app, tool);
+                }
+                // The Enter that opened a dialog isn't also its OK: dialogs take Enter, this frame too.
+                if app.ui.dialog.is_some() {
+                    ctx.input_mut(|i| i.consume_key(Modifiers::NONE, Key::Enter));
+                }
+            }
         }
     }
     if typing {
@@ -526,6 +541,54 @@ mod tests {
         assert_eq!(grid(&mut app, &[Key::ArrowLeft, Key::ArrowDown]), (rows, columns + 1));
     }
 
+    /// Enter opens the tool's dialog: the Move dialog for the selection tools, the tool's own for
+    /// Rotate, Scale, Reflect and Shear. The Enter that opens it isn't also its OK.
+    #[test]
+    fn enter_opens_the_tools_dialog_and_leaves_it_open() {
+        let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());
+        app.session.execute("file.new", &json!({"width": 400, "height": 300})).unwrap();
+        let ctx = egui::Context::default();
+        let enter = || egui::Event::Key { key: Key::Enter, physical_key: None, pressed: true, repeat: false, modifiers: Modifiers::NONE };
+        // A whole app frame: the shortcuts first, then the UI, where a dialog takes its Enter.
+        let frame = |app: &mut VectorcraftApp, events: Vec<egui::Event>| {
+            let screen_rect = Some(egui::Rect::from_min_size(egui::Pos2::ZERO, egui::vec2(1280.0, 800.0)));
+            let mut out = ctx.run_ui(egui::RawInput { events, screen_rect, ..Default::default() }, |ui| {
+                app.logic(ui.ctx());
+                app.ui(ui);
+            });
+            out.textures_delta.clear();
+        };
+        let kind = |app: &VectorcraftApp| app.ui.dialog.as_ref().map(|d| d.kind.clone());
+        let steps = |app: &VectorcraftApp| app.session.active().unwrap().history.undo.len();
+        app.select_tool("selection");
+        frame(&mut app, vec![enter()]);
+        assert_eq!(kind(&app), None, "nothing is selected");
+        app.session.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 30})).unwrap();
+        for (tool, dialog) in [
+            ("selection", "move"),
+            ("directSelection", "move"),
+            ("groupSelection", "move"),
+            ("rotate", "rotate"),
+            ("scale", "scale"),
+            ("reflect", "reflect"),
+            ("shear", "shear"),
+        ] {
+            app.select_tool(tool);
+            let before = steps(&app);
+            frame(&mut app, vec![enter()]);
+            assert_eq!(kind(&app).as_deref(), Some(dialog), "{tool}: Enter opens it");
+            frame(&mut app, vec![]);
+            assert_eq!(kind(&app).as_deref(), Some(dialog), "{tool}: and it stays open");
+            assert_eq!(steps(&app), before, "{tool}: the Enter that opened it didn't confirm it");
+            // The next Enter is its OK.
+            frame(&mut app, vec![enter()]);
+            assert_eq!((kind(&app), steps(&app)), (None, before + 1), "{tool}: Enter confirms");
+        }
+        app.select_tool("rectangle");
+        frame(&mut app, vec![enter()]);
+        assert_eq!(kind(&app), None, "another tool has its own Enter");
+    }
+
     #[test]
     fn comma_and_period_reapply_the_last_colour_and_gradient() {
         let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), Default::default());

crates/ui-egui/src/toolbar.rs

@@ -218,19 +218,21 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) {
 /// the Gradient panel, the Eyedropper's the Eyedropper Options dialog, a Liquify tool's its Tool
 /// Options dialog, the Blend tool's Blend
 /// Options; the Print Tiling tool's resets the print tiling. As in the reference app, the Hand
-/// tool's fits the artboard in the window, the Zoom tool's shows it at 100%, and the Rotate, Scale,
-/// Reflect and Shear tools' are their Object › Transform dialogs.
+/// tool's fits the artboard in the window, the Zoom tool's shows it at 100%, the Rotate, Scale,
+/// Reflect and Shear tools' are their Object › Transform dialogs, and the selection tools' are the
+/// Move dialog.
 pub fn open_options(app: &mut VectorcraftApp, tool: &str) -> Result<serde_json::Value, String> {
     match tool {
         "hand" => app.run("view.fitArtboard", json!({})),
         "zoom" => app.run("view.actualSize", json!({})),
-        "rotate" | "scale" | "reflect" | "shear" => {
-            let id = format!("object.{tool}");
+        "rotate" | "scale" | "reflect" | "shear" | "selection" | "directSelection" | "groupSelection" => {
+            let dialog = if crate::canvas::is_selection_tool(tool) { "move" } else { tool };
+            let id = format!("object.{dialog}");
             if let Some(c) = vectorcraft_engine::find_command(&id) {
                 (c.enabled)(&app.session)?;
             }
             crate::menus::invoke(app, &id, json!({}));
-            Ok(json!({ "dialog": tool }))
+            Ok(json!({ "dialog": dialog }))
         }
         "gradient" if app.ui.open_panel.as_deref() == Some("gradient") => Ok(json!({ "open": "gradient" })),
         "gradient" => app.run("window.panel", json!({ "panel": "gradient" })),
@@ -538,6 +540,33 @@ pub(crate) mod tests {
         assert!((b.x0 - 20.0).abs() < 1e-9 && (b.y0 - 0.0).abs() < 1e-9 && (b.width() - 20.0).abs() < 1e-9, "{b:?}");
     }
 
+    #[test]
+    fn double_clicking_a_selection_tool_opens_the_move_dialog() {
+        let mut app = VectorcraftApp::new(Session::new(), Default::default());
+        app.run("file.new", json!({"width": 300, "height": 200})).unwrap();
+        let ctx = egui::Context::default();
+        // The Selection tool's button is the first one.
+        let selection = frame(&mut app, &ctx, 0.0, vec![])[0];
+        // Nothing selected: Move is disabled, so nothing opens.
+        double_click(&mut app, &ctx, 1.0, selection.center());
+        assert!(app.ui.dialog.is_none());
+        assert_eq!(app.run("tool.options", json!({"tool": "directSelection"})), Err("nothing selected".into()));
+        let id = app.run("shape.rectangle", json!({"x": 10, "y": 10, "width": 40, "height": 20})).unwrap()["id"].as_u64().unwrap();
+        double_click(&mut app, &ctx, 2.0, selection.center());
+        assert_eq!(app.ui.dialog.take().map(|d| d.kind), Some("move".to_string()));
+        // The Direct Selection and Group Selection tools open it too.
+        for tool in ["directSelection", "groupSelection"] {
+            app.run("tool.options", json!({"tool": tool})).unwrap();
+            assert_eq!(app.ui.dialog.take().map(|d| d.kind), Some("move".to_string()), "{tool}");
+        }
+        // OK moves the selection by what was typed.
+        app.run("tool.options", json!({"tool": "selection"})).unwrap();
+        app.ui.dialog.as_mut().unwrap().fields.insert("dx".into(), json!("25 pt"));
+        crate::dialogs::confirm(&mut app).unwrap();
+        let b = app.session.active().unwrap().doc.node(vectorcraft_doc::NodeId(id)).unwrap().geometric_bounds().unwrap();
+        assert_eq!((b.x0, b.y0), (35.0, 10.0));
+    }
+
     #[test]
     fn right_clicking_a_tool_group_opens_its_flyout() {
         let mut app = VectorcraftApp::new(Session::new(), Default::default());

docs/control-protocol.md

@@ -89,8 +89,11 @@ for `eyedropper` it opens Eyedropper Options, an `eyedropperOptions` dialog (fie
 `apply`, the attribute trees of `eyedropper.setOptions`) whose `ui.dialog.confirm` runs `eyedropper.setOptions` (what
 `appearance.copyFrom` copies). For `hand` it fits the artboard in the window (`view.fitArtboard`) and for `zoom` it
 shows 100% (`view.actualSize`). For `rotate`, `scale`, `reflect` and `shear` it opens the same dialog as Object ›
-Transform (dialog kind = the tool id), or fails with `nothing selected`. Gradient tool handles snap to
-anchors, edges and smart guides; Shift constrains them to 45° steps from the `constrainAngle` preference.
+Transform (dialog kind = the tool id), or fails with `nothing selected`. For `selection`, `directSelection` and
+`groupSelection` it opens the Move dialog (kind `move`), with the same failure. `ui.key` Enter with one of those seven
+tools opens its dialog too (nothing happens without a selection), with the transform tools' `origin` at their reference
+point. Gradient tool handles snap to anchors, edges and smart guides; Shift constrains them to 45° steps from the
+`constrainAngle` preference.
 
 Effect dialogs: `engine.execute {command: "effect.dialog", params: {effect, index?, item?}}` opens the `effect` dialog
 (fields: the effect's parameters, `preview`). With `index` it edits that applied effect of `item` (null: the object's