From 16766bdbc79a4b9ec46bb8b9879587f38ce2994d Mon Sep 17 00:00:00 2001 From: William Dunn Date: Tue, 13 Oct 2026 13:37:54 -0400 Subject: [PATCH] Edit Sketch in the viewport menu over a sketch curve (#795) Right-clicking a curve or point of a sketch that isn't being edited now opens that sketch's own menu, with Edit Sketch, instead of the body menu. Solid geometry in the selection keeps the body menu. --- crates/ui-egui/src/context_menu.rs | 23 ++++++++- crates/ui-egui/src/menu_tests.rs | 49 +++++++++++++++++++ .../s52_edit_sketch_from_viewport.json | 19 +++++++ 3 files changed, 90 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/scenarios/s52_edit_sketch_from_viewport.json diff --git a/crates/ui-egui/src/context_menu.rs b/crates/ui-egui/src/context_menu.rs index a8c465be..62141012 100644 --- a/crates/ui-egui/src/context_menu.rs +++ b/crates/ui-egui/src/context_menu.rs @@ -211,6 +211,18 @@ fn sel_bodies(app: &SolveApp) -> Vec { v } +/// The sketch a selected sketch curve or point belongs to. Geometry of a sketch that is not +/// being edited selects as `ExternalSketchCurve` or `ExternalSketchPoint` +/// (`viewport::hit_candidate`). Those two carry the owning sketch's feature id. The entities of +/// the sketch being edited are `SketchCurve` and `SketchPoint` instead. The sketch-editing menu +/// already covers that case. +fn sel_sketch(app: &SolveApp) -> Option { + app.session.selection.iter().find_map(|s| match s { + Sel::ExternalSketchCurve { sketch, .. } | Sel::ExternalSketchPoint { sketch, .. } => Some(*sketch), + _ => None, + }) +} + /// Bodies the selection touches (bodies, and the bodies of selected faces and edges). fn touched_bodies(app: &SolveApp) -> Vec { let mut v: Vec = Vec::new(); @@ -367,7 +379,16 @@ fn viewport_items(app: &SolveApp) -> Vec { v.push(cmd(app, "edit.redo", "Redo").on(!app.session.redo.is_empty())); return v; } - if has(|s| matches!(s, Sel::Face { .. })) { + // A sketch curve or point in the viewport gets the sketch's own menu, the one its browser row + // opens, with Edit Sketch among it. Solid geometry in the selection keeps the body menu: a + // right-click on a face still offers the face's commands. The picker already prefers a sketch + // curve to the face it lies on (`pick_hits` offers curves before faces). A right-click that + // resolved to a sketch curve therefore replaces any face in the selection with that curve. + // The menu then follows the pick. + let solid = has(|s| matches!(s, Sel::Face { .. } | Sel::Edge { .. } | Sel::Vertex { .. } | Sel::Body { .. })); + if let Some(sketch) = sel_sketch(app).filter(|_| !solid) { + v.extend(sketch_items(app, sketch)); + } else if has(|s| matches!(s, Sel::Face { .. })) { let face = sel.iter().find_map(|s| if let Sel::Face { body, .. } = s { Some(body.clone()) } else { None }).unwrap_or_default(); let (feature, sketch) = body_feature_and_sketch(app, &face); let hidden = app.session.visibility.hidden_bodies.contains(&face); diff --git a/crates/ui-egui/src/menu_tests.rs b/crates/ui-egui/src/menu_tests.rs index 1e356019..95b01dc4 100644 --- a/crates/ui-egui/src/menu_tests.rs +++ b/crates/ui-egui/src/menu_tests.rs @@ -91,6 +91,55 @@ fn appearance_component_action_assigns_an_inherited_look() { assert!(app.session.doc.appearances.components.contains_key(&id)); } +/// A right-click on a sketch curve in the viewport reaches the sketch's own menu, the one the +/// browser row opens, Edit Sketch among it. +#[test] +fn a_sketch_curve_in_the_viewport_offers_the_sketch_menu() { + let mut app = SolveApp::new(Session::default(), Services::default()); + app.run("sketch.create", json!({"plane": "XY", "name": "Base"})).unwrap(); + app.run("sketch.rectangle.two_point", json!({"p0": [0, 0], "p1": [40, 30]})).unwrap(); + app.run("sketch.finish", json!({})).unwrap(); + let (sketch, curve) = { + let st = app.session.world_state(); + let ss = st.sketches.first().expect("the design has one sketch"); + (ss.feature, ss.sketch.curves.first().expect("the rectangle drew curves").id.clone()) + }; + app.run("select.set", json!({"items": [{"type": "external_sketch_curve", "sketch": sketch, "id": curve}]})).unwrap(); + let items = context_menu::items(&app, &Target::Viewport); + // Item for item the browser row's menu, with the viewport's Clear Selection after it. + let row = context_menu::items(&app, &Target::Sketch { id: sketch }); + let labels = |v: &[Item]| v.iter().map(|i| i.label.clone()).collect::>(); + assert_eq!(labels(&items), [labels(&row), vec![String::new(), "Clear Selection".to_string()]].concat()); + let edit = find(&items, "Edit Sketch"); + assert_eq!(edit.id, "ui.editSketch"); + assert_eq!(edit.params, Some(json!({"sketch": sketch}))); + assert!(edit.enabled); + context_menu::run_item(&mut app, &edit, pos2(0.0, 0.0)); + assert_eq!(app.session.active_sketch, Some(sketch), "Edit Sketch opens the sketch that was right-clicked"); + // The sketch is open now: the viewport offers the sketch-editing menu, not a second Edit Sketch. + let editing = context_menu::items(&app, &Target::Viewport); + assert!(find(&editing, "Finish Sketch").enabled); + assert!(!editing.iter().any(|i| i.label == "Edit Sketch"), "{:?}", labels(&editing)); +} + +/// Solid geometry in the selection keeps the body menu: a right-click on a face offers the +/// face's commands even when a sketch curve is selected as well. +#[test] +fn a_selected_face_keeps_the_body_menu_beside_a_sketch_curve() { + let mut app = sample_app(); + let b = body(&app); + let sketch = app.session.world_state().sketches.first().expect("the sample design has sketches").feature; + let items = json!([ + {"type": "face", "body": b, "index": 0, "point": [0, 0, 0]}, + {"type": "external_sketch_curve", "sketch": sketch, "id": "l1"}, + ]); + app.run("select.set", json!({"items": items})).unwrap(); + let items = context_menu::items(&app, &Target::Viewport); + assert!(find(&items, "Press Pull").enabled); + assert!(find(&items, "Edit Profile Sketch").enabled); + assert!(!items.iter().any(|i| i.label == "Redefine Sketch Plane"), "the face menu, not the sketch menu"); +} + #[test] fn face_menu_hides_and_finds_the_body() { let mut app = sample_app(); diff --git a/crates/ui-egui/tests/scenarios/s52_edit_sketch_from_viewport.json b/crates/ui-egui/tests/scenarios/s52_edit_sketch_from_viewport.json new file mode 100644 index 00000000..0f005806 --- /dev/null +++ b/crates/ui-egui/tests/scenarios/s52_edit_sketch_from_viewport.json @@ -0,0 +1,19 @@ +[ + {"note": "A right-click on a sketch curve in the work area must offer the sketch's own menu. It used to give the empty-space view menu instead. Edit Sketch was then reachable only from the browser. The `menu:ui.editSketch` handle is published by context_menu::list_row. Clicking that handle therefore fails unless the row is drawn."}, + {"start": "sketch.create"}, + {"click": {"plane": "XY"}}, + {"start": "sketch.rectangle.two_point"}, + {"click": {"sketch": [0, 0]}}, + {"click": {"sketch": [40, 30]}}, + {"key": "Escape"}, + {"start": "sketch.finish"}, + {"call": "ui.view", "params": {"view": "iso"}}, + {"expect": {"sketching": false}}, + {"click": {"world": [20, 0, 0]}, "button": "right"}, + {"expect": {"selection": 1}}, + {"shot": "s52_sketch_menu"}, + {"click": {"handle": "menu:ui.editSketch"}}, + {"expect": {"sketching": true}}, + {"start": "sketch.finish"}, + {"expect": {"errors": 0, "sketching": false}} +]