Edit Sketch: open an existing sketch from the ribbon (#9705) (#9829)

* claim: #9705

* Fix Edit Sketch ribbon selection and chooser (#9705)

assets/locales/pending/edit-sketch-choice.json

@@ -0,0 +1,10 @@
+{
+  "ar": { "ui.edit_sketch_empty": "لا توجد رسومات لتعديلها" },
+  "de": { "ui.edit_sketch_empty": "Keine Skizzen zum Bearbeiten" },
+  "en": { "ui.edit_sketch_empty": "No sketches to edit" },
+  "he": { "ui.edit_sketch_empty": "אין סקיצות לעריכה" },
+  "ja": { "ui.edit_sketch_empty": "編集できるスケッチはありません" },
+  "ko": { "ui.edit_sketch_empty": "편집할 스케치가 없습니다" },
+  "vi": { "ui.edit_sketch_empty": "Không có phác thảo để chỉnh sửa" },
+  "zh": { "ui.edit_sketch_empty": "没有可编辑的草图" }
+}

crates/ui-egui/src/context_menu.rs

@@ -15,7 +15,7 @@ use crate::ui_kit::{Container, Surface};
 use egui::{Color32, Pos2, Rect, Sense, Vec2, pos2, vec2};
 use serde::Serialize;
 use serde_json::{Value, json};
-use solidcraft_engine::Sel;
+use solidcraft_engine::{Sel, doc::FeatureKind};
 
 use crate::SolveApp;
 
@@ -65,6 +65,8 @@ pub enum Target {
     Units,
     /// Insert › Insert Design: the designs open in other tabs, and Browse… (#8570).
     InsertDesign,
+    /// Model › Edit Sketch: choose an existing sketch when none is uniquely selected.
+    EditSketch,
     /// View › Visual style: the four styles, the current one marked (#8868).
     VisualStyle,
     /// View › Navigation tool: Orbit, Pan, Zoom; the latched one marked (#8868).
@@ -435,6 +437,17 @@ pub fn items(app: &SolveApp, target: &Target) -> Vec<Item> {
             .collect(),
         Target::NamedView { name } => crate::browser::view_items(app, name),
         Target::InsertDesign => insert_design_items(app),
+        Target::EditSketch => {
+            let items: Vec<Item> = app
+                .session
+                .doc
+                .features
+                .iter()
+                .filter(|f| matches!(f.kind, FeatureKind::Sketch { .. }))
+                .map(|f| act("ui.editSketch", &f.name, "sketch").with(json!({"sketch": f.id})))
+                .collect();
+            if items.is_empty() { vec![act("ui.editSketch", app.tr("ui.edit_sketch_empty"), "sketch").on(false)] } else { items }
+        }
         Target::VisualStyle => crate::navigation_bar::VISUAL_STYLES
             .iter()
             .enumerate()

crates/ui-egui/src/lib.rs

@@ -1792,6 +1792,10 @@ impl SolveApp {
             design_properties::open_selected(self);
             return;
         }
+        if id == "sketch.edit" {
+            self.start_sketch_edit(opener);
+            return;
+        }
         if id == "sketch.finish" {
             self.finish_sketch();
             return;
@@ -1967,6 +1971,37 @@ impl SolveApp {
         tools::Tool::for_command(id).is_some() || dialogs::Dialog::for_command(self, id).is_some()
     }
 
+    /// The ribbon has no sketch argument: use one unambiguous sketch pick, otherwise ask.
+    fn start_sketch_edit(&mut self, opener: Option<egui::Rect>) {
+        use solidcraft_engine::{Sel, doc::FeatureKind};
+        let mut sketches: Vec<u64> = self
+            .session
+            .selection
+            .iter()
+            .filter_map(|s| match s {
+                Sel::Feature { id } => Some(*id),
+                Sel::Profile { sketch, .. } | Sel::ExternalSketchCurve { sketch, .. } | Sel::ExternalSketchPoint { sketch, .. } => Some(*sketch),
+                Sel::SketchCurve { sketch, .. } => *sketch,
+                _ => None,
+            })
+            .filter(|id| self.session.doc.feature(*id).is_some_and(|f| matches!(f.kind, FeatureKind::Sketch { .. })))
+            .collect();
+        sketches.sort_unstable();
+        sketches.dedup();
+        let sketch = match sketches.as_slice() {
+            [id] => Some(*id),
+            [] => self.session.active_sketch,
+            _ => None,
+        };
+        if let Some(id) = sketch {
+            self.edit_feature(id);
+        } else {
+            dialogs::cancel(self);
+            self.tool = None;
+            self.open_choice_menu(opener, context_menu::Target::EditSketch);
+        }
+    }
+
     /// Edit a feature: sketches open in sketch mode; other features roll the
     /// timeline back to just before themselves and reopen their dialog, filled in.
     pub fn edit_feature(&mut self, id: u64) {

crates/ui-egui/src/sketch_edit_tests.rs

@@ -83,6 +83,93 @@ fn toolbar_finish_and_other_feature_edit_leave_the_sketch() {
     assert_eq!(app.session.active_sketch, None);
 }
 
+fn two_finished_sketches() -> (crate::scenario::Harness, Vec<u64>) {
+    use serde_json::json;
+    let mut h = crate::scenario::Harness::new();
+    h.app.home.open = false;
+    let mut ids = Vec::new();
+    for name in ["First", "Second"] {
+        let created = h.app.run("sketch.create", json!({"plane": "XY", "name": name})).unwrap();
+        ids.push(created["sketch"].as_u64().unwrap());
+        h.app.run("sketch.line", json!({"points": [[0, 0], [20, 10]]})).unwrap();
+        h.app.run("sketch.finish", json!({})).unwrap();
+    }
+    h.settle();
+    (h, ids)
+}
+
+#[test]
+fn ribbon_edit_sketch_opens_the_selected_sketch_and_finish_restores_the_marker() {
+    use serde_json::json;
+    let (mut h, ids) = two_finished_sketches();
+    let marker = h.app.session.doc.marker;
+    h.app.run("select.set", json!({"items": [{"type": "feature", "id": ids[0]}]})).unwrap();
+    h.step(&json!({"start": "sketch.edit"})).unwrap();
+    h.settle();
+    assert_eq!(h.app.session.active_sketch, Some(ids[0]));
+    assert_eq!(h.app.session.doc.marker, Some(1));
+    h.step(&json!({"start": "sketch.finish"})).unwrap();
+    h.settle();
+    assert_eq!(h.app.session.active_sketch, None);
+    assert_eq!(h.app.session.doc.marker, marker);
+}
+
+#[test]
+fn ribbon_edit_sketch_offers_a_choice_without_a_unique_selection() {
+    use serde_json::json;
+    for selected in [false, true] {
+        let (mut h, ids) = two_finished_sketches();
+        if selected {
+            h.app
+                .run(
+                    "select.set",
+                    json!({"items": [
+                        {"type": "feature", "id": ids[0]}, {"type": "feature", "id": ids[1]}
+                    ]}),
+                )
+                .unwrap();
+        }
+        h.step(&json!({"start": "sketch.edit"})).unwrap();
+        h.settle();
+        assert_eq!(h.app.session.active_sketch, None, "do not choose an arbitrary sketch");
+        assert!(h.app.viewport.context_menu.is_some(), "offer the sketches to edit");
+        h.step(&json!({"call": "ui.menuPick", "params": {"item": "Second"}})).unwrap();
+        h.settle();
+        assert_eq!(h.app.session.active_sketch, Some(ids[1]));
+        h.step(&json!({"start": "sketch.finish"})).unwrap();
+        h.settle();
+        assert_eq!(h.app.session.doc.marker, None);
+    }
+}
+
+#[test]
+fn ribbon_edit_sketch_uses_the_owner_of_selected_sketch_geometry() {
+    use serde_json::json;
+    use solidcraft_engine::Sel;
+    let (mut h, ids) = two_finished_sketches();
+    h.app.session.selection =
+        vec![Sel::ExternalSketchCurve { sketch: ids[1], id: "l1".into() }, Sel::ExternalSketchPoint { sketch: ids[1], id: "p1".into() }];
+    h.step(&json!({"start": "sketch.edit"})).unwrap();
+    h.settle();
+    assert_eq!(h.app.session.active_sketch, Some(ids[1]));
+}
+
+#[test]
+fn ribbon_edit_sketch_handles_an_empty_design_without_running_a_command() {
+    use serde_json::json;
+    let mut h = crate::scenario::Harness::new();
+    h.app.home.open = false;
+    h.step(&json!({"start": "sketch.edit"})).unwrap();
+    h.settle();
+    let items = crate::context_menu::items(&h.app, &h.app.menu.target);
+    assert_eq!(items.len(), 1);
+    assert_eq!(items[0].label, "No sketches to edit");
+    assert!(!items[0].enabled);
+    assert_eq!(h.app.session.active_sketch, None);
+    assert!(h.app.session.doc.features.is_empty());
+    assert!(h.app.session.execute("sketch.edit", &json!({})).is_err(), "the programmatic command still requires a sketch");
+}
+
 /// Leaving a sketch any way leaves the camera exactly where it is; entering still faces the plane.
 mod exit_keeps_view {
     use crate::scenario::Harness;