Escape stays in the sketch (#1804) (#1836)

* claim: #1804

* Escape no longer leaves an open sketch (#1804)

A second Escape after putting a tool down, or after clearing the selection, stays in the sketch. Finish Sketch is what leaves it.

crates/ui-egui/src/dialogs.rs

@@ -114,6 +114,8 @@ pub enum Kind {
     OffsetPlane {
         offset: String,
     },
+    /// Halfway between two planes or planar faces.
+    Midplane,
     /// A construction plane through an axis at an angle to a plane.
     AnglePlane {
         angle: String,
@@ -521,6 +523,10 @@ impl Dialog {
             "construct.plane.offset" => {
                 Dialog::new(Kind::OffsetPlane { offset: "10 mm".into() }, vec![SelInput::new("Plane", PLANES | PLANAR_FACES, false)])
             }
+            "construct.plane.midplane" => Dialog::new(
+                Kind::Midplane,
+                vec![SelInput::new("First plane", PLANES | PLANAR_FACES, false), SelInput::new("Second plane", PLANES | PLANAR_FACES, false)],
+            ),
             "construct.plane.along_path" => {
                 Dialog::new(Kind::PathPlane { t: "0.5".into(), sketch: None }, vec![SelInput::new("Path", CURVES | EDGES, false)])
             }
@@ -790,6 +796,7 @@ impl Dialog {
                 | Kind::Loft { .. }
                 | Kind::Sweep { .. }
                 | Kind::PathPlane { .. }
+                | Kind::Midplane
                 | Kind::Move { .. }
                 | Kind::Hole { .. }
                 | Kind::Primitive { .. }
@@ -1379,6 +1386,7 @@ fn title(k: &Kind) -> &'static str {
         Kind::Measure { .. } => "MEASURE",
         Kind::Material { .. } => "PHYSICAL MATERIAL",
         Kind::OffsetPlane { .. } => "OFFSET PLANE",
+        Kind::Midplane => "MIDPLANE",
         Kind::PathPlane { .. } => "PLANE ALONG PATH",
         Kind::Rib { web: false, .. } => "RIB",
         Kind::Rib { web: true, .. } => "WEB",
@@ -2686,7 +2694,7 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) {
                         enter |= field(ui, t);
                         ui.end_row();
                     }
-                    Kind::Split | Kind::ReplaceFace | Kind::Remove => {}
+                    Kind::Split | Kind::ReplaceFace | Kind::Remove | Kind::Midplane => {}
                     Kind::Rib { thickness, depth, flip, .. } => {
                         row_label(ui, "Thickness");
                         enter |= field(ui, thickness);
@@ -3897,15 +3905,16 @@ fn dialog_commands(app: &SolveApp, d: &Dialog) -> Result<Vec<(String, Value)>, S
         }
         Kind::OffsetPlane { offset } => {
             need(0, "a plane")?;
-            let base = match sels(d, 0).first() {
-                Some(Sel::Plane { name }) => json!(name),
-                Some(face @ Sel::Face { .. }) => {
-                    crate::sketch_attachment_ui::face_plane(app, face).ok_or("the picked face is no longer in the model")?
-                }
-                _ => return Err("pick an origin or construction plane, or a planar face".into()),
-            };
+            let base = plane_arg(app, sels(d, 0).first())?;
             ("construct.plane.offset", json!({"base": base, "offset": offset}))
         }
+        Kind::Midplane => {
+            need(0, "the first plane")?;
+            need(1, "the second plane")?;
+            let a = plane_arg(app, sels(d, 0).first())?;
+            let b = plane_arg(app, sels(d, 1).first())?;
+            ("construct.plane.midplane", json!({"a": a, "b": b}))
+        }
         Kind::PathPlane { t, sketch } => {
             need(0, "a path")?;
             let path = match sels(d, 0).first() {
@@ -4495,6 +4504,24 @@ fn emboss_face_sel(s: &Session, component: u64, face: &solidcraft_engine::doc::E
     Some(Sel::Face { body: world_name, index, point })
 }
 
+/// A picked plane or planar face as `construct.plane.*` takes it.
+fn plane_arg(app: &SolveApp, sel: Option<&Sel>) -> Result<Value, String> {
+    match sel {
+        Some(Sel::Plane { name }) => Ok(json!(name)),
+        Some(face @ Sel::Face { .. }) => crate::sketch_attachment_ui::face_plane(app, face).ok_or("the picked face is no longer in the model".into()),
+        _ => Err("pick an origin or construction plane, or a planar face".into()),
+    }
+}
+
+/// A stored construction reference back into a dialog pick.
+fn geo_sel(s: &Session, g: &solidcraft_engine::doc::construct::GeoRef) -> Option<Sel> {
+    match g {
+        solidcraft_engine::doc::construct::GeoRef::Plane { plane } => plane_sel(s, plane),
+        solidcraft_engine::doc::construct::GeoRef::Face { at, .. } => face_sel(s, *at),
+        _ => None,
+    }
+}
+
 pub(crate) fn plane_sel(s: &Session, pl: &PlaneRef) -> Option<Sel> {
     match pl {
         PlaneRef::Origin { name } => Some(Sel::Plane { name: name.clone() }),
@@ -5022,6 +5049,16 @@ pub fn for_feature(app: &SolveApp, id: u64, marker: Option<usize>) -> Option<Dia
             }
             d
         }
+        FeatureKind::ConstructionPlane { plane: PlaneRef::Midplane { a, b } } => {
+            let mut d = start("construct.plane.midplane")?;
+            if let Some(inp) = d.inputs.first_mut() {
+                inp.items = geo_sel(s, a).into_iter().collect();
+            }
+            if let Some(inp) = d.inputs.get_mut(1) {
+                inp.items = geo_sel(s, b).into_iter().collect();
+            }
+            d
+        }
         FeatureKind::ConstructionPlane { plane: PlaneRef::AtAngle { base, axis_origin, axis_dir, angle } } => {
             let mut d = start("construct.plane.at_angle")?;
             d.kind = Kind::AnglePlane { angle: angle.to_string() };

crates/ui-egui/src/preselect_tests.rs

@@ -1649,3 +1649,26 @@ fn patch_dialog_rows_are_translated() {
         assert!(h.widget(through).is_some(), "{lang}: the points input must show {through:?}");
     }
 }
+
+/// Midplane opens a two-plane dialog. Starting it with nothing selected used to run the
+/// command and report that `a` is required.
+#[test]
+fn midplane_asks_for_two_planes_then_builds_the_one_between_them() {
+    let mut h = crate::scenario::Harness::new();
+    h.app.run("construct.plane.offset", json!({"base":"XY","offset":"20 mm","name":"Shelf"})).unwrap();
+    h.app.start("construct.plane.midplane");
+    h.frames(2);
+    let d = h.app.dialog.as_ref().unwrap();
+    assert!(matches!(d.kind, Kind::Midplane));
+    assert_eq!(d.inputs.len(), 2);
+    let status = h.app.status.as_ref().map(|s| s.0.clone()).unwrap_or_default();
+    assert!(!status.contains("is required"), "{status}");
+    h.app.dialog.as_mut().unwrap().inputs[0].items = vec![Sel::Plane { name: "XY".into() }];
+    h.app.dialog.as_mut().unwrap().inputs[1].items = vec![Sel::Plane { name: "Shelf".into() }];
+    h.step(&json!({"key":"Enter"})).unwrap();
+    assert!(h.app.dialog.is_none(), "{:?}", h.app.dialog.as_ref().and_then(|d| d.error.clone()));
+    let (_, _, pl) =
+        solidcraft_engine::view::construction_planes(&h.app.session).into_iter().find(|(_, name, _)| name != "Shelf").expect("the midplane");
+    assert!(pl.normal().cross(Vec3::Z).len() < 1e-6, "{pl:?}");
+    assert!((pl.origin.z - 10.0).abs() < 1e-6, "{pl:?}");
+}

crates/ui-egui/src/sketch_shortcut_tests.rs

@@ -67,6 +67,29 @@ fn a_sketch_key_with_nothing_selected_asks_for_the_plane_then_starts_the_tool()
     assert!(h.app.dialog.is_none());
 }
 
+/// #1804: Escape puts down the sketch tool. Another Escape does not leave the sketch.
+#[test]
+fn a_second_escape_stays_in_the_sketch() {
+    let mut h = Harness::new();
+    h.app.run("sketch.create", json!({"plane":"XY"})).unwrap();
+    let sketch = h.app.session.active_sketch;
+    h.app.start("sketch.line");
+    h.settle();
+    assert_eq!(tool(&h), Some("sketch.line"));
+    press(&mut h, "Escape");
+    assert!(tool(&h).is_none(), "the first Escape puts the tool down");
+    assert_eq!(h.app.session.active_sketch, sketch);
+    press(&mut h, "Escape");
+    assert_eq!(h.app.session.active_sketch, sketch, "a second Escape stays in the sketch");
+    h.app.run("sketch.line", json!({"points":[[0,0],[10,0]]})).unwrap();
+    h.app.session.selection = vec![Sel::SketchCurve { id: "l1".into() }];
+    press(&mut h, "Escape");
+    assert!(h.app.session.selection.is_empty(), "Escape still clears a selection");
+    assert_eq!(h.app.session.active_sketch, sketch);
+    press(&mut h, "Escape");
+    assert_eq!(h.app.session.active_sketch, sketch, "Escape after clearing the selection stays in the sketch");
+}
+
 #[test]
 fn escape_drops_the_tool_waiting_for_a_sketch() {
     let mut h = Harness::new();

crates/ui-egui/src/toolbar.rs

@@ -1348,10 +1348,9 @@ pub fn shortcuts(app: &mut SolveApp, ctx: &egui::Context) {
                 } else if !app.session.selection.is_empty() || app.session.active_sketch.is_none() {
                     app.tree.picked_joints.clear();
                     let _ = app.run("select.clear", json!({}));
-                } else {
-                    // Nothing left to cancel: Esc finishes the sketch.
-                    app.finish_sketch();
                 }
+                // An open sketch with nothing left to cancel stays open (#1804). Finish Sketch
+                // is what leaves it, so a second Escape after putting a tool down is harmless.
             }
             Key::S if !mods.shift && !mods.alt => {
                 let at = ctx.input(|i| i.pointer.latest_pos());