From 7e22aed7391ad5aade70454d3e181c34a9baebea Mon Sep 17 00:00:00 2001 From: jeromehzl <51324719+jeromehzl@users.noreply.github.com> Date: Fri, 30 Oct 2026 09:04:39 +0000 Subject: [PATCH] View ribbon: Visual style and Navigation tool offer their choices instead of an error (#8868) (#9127) Both commands need a choice, but start_with_variant_memory only special- cased the parameterless display toggles, so a click on the ribbon (or command search, a shortcut, ui.start) ran them with {} and showed only the engine's "style must be ..." / "tool must be ..." error. They now open a short menu, the way Units does: under the ribbon row that started it (a second click closes it), or in the middle of the view. Visual style lists the four styles with the current one checked; the Navigation tool lists Orbit, Pan and Zoom, and picking the latched one releases it, as its navigation-bar button does. The four styles come from one list shared with the navigation bar's menu. --- crates/ui-egui/src/context_menu.rs | 97 +++++++++++++++++++++++ crates/ui-egui/src/i18n_menu_tests.rs | 2 + crates/ui-egui/src/lib.rs | 23 ++++++ crates/ui-egui/src/menu_tests.rs | 2 + crates/ui-egui/src/navigation_bar.rs | 8 ++ crates/ui-egui/src/navigation_bar_view.rs | 2 +- 6 files changed, 133 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/context_menu.rs b/crates/ui-egui/src/context_menu.rs index b072194e54..993bd040d5 100644 --- a/crates/ui-egui/src/context_menu.rs +++ b/crates/ui-egui/src/context_menu.rs @@ -65,6 +65,10 @@ pub enum Target { Units, /// Insert › Insert Design: the designs open in other tabs, and Browse… (#8570). InsertDesign, + /// View › Visual style: the four styles, the current one marked (#8868). + VisualStyle, + /// View › Navigation tool: Orbit, Pan, Zoom; the latched one marked (#8868). + ViewTool, /// The mouse mappings (Preferences › Navigation), from the status bar (#7023). Navigation, /// An assembly joint. @@ -431,6 +435,38 @@ pub fn items(app: &SolveApp, target: &Target) -> Vec { .collect(), Target::NamedView { name } => crate::browser::view_items(app, name), Target::InsertDesign => insert_design_items(app), + Target::VisualStyle => crate::navigation_bar::VISUAL_STYLES + .iter() + .enumerate() + .map(|(index, (style, label))| { + let current = usize::from(app.ui.visual_style) == index; + let mut item = cmd(app, "view.visual_style", label).with(json!({"style": style})).key(if current { "current" } else { "" }); + item.icon = crate::navigation_bar::visual_style_icon(u8::try_from(index).unwrap_or_default()).into(); + item + }) + .collect(), + Target::ViewTool => { + let latched = match app.viewport.nav { + Some(crate::viewport::NavMode::Orbit) => "orbit", + Some(crate::viewport::NavMode::Pan) => "pan", + Some(crate::viewport::NavMode::Zoom) => "zoom", + None => "", + }; + [("orbit", "Orbit"), ("pan", "Pan"), ("zoom", "Zoom")] + .into_iter() + .map(|(tool, label)| { + let current = latched == tool; + // Picking the latched tool again releases it, as its navigation-bar button does. + let mut item = cmd(app, "view.tool", label).with(json!({"tool": if current { "none" } else { tool }})).key(if current { + "current" + } else { + "" + }); + item.icon = tool.into(); + item + }) + .collect() + } }; if !crate::render_workspace::active(app) { v.extend(crate::browser::folded_folder_items(app, target)); @@ -2587,6 +2623,67 @@ mod tests { assert!(h.app.viewport.context_menu.is_none()); } + #[test] + fn visual_style_toolbar_start_offers_the_four_styles_not_an_error() { + let mut h = crate::scenario::Harness::new(); + h.app.home.open = false; + let error = |h: &crate::scenario::Harness| h.app.status.as_ref().is_some_and(|(_, _, error)| *error); + h.app.status = None; + h.call("ui.start", json!({"command":"view.visual_style"})); + assert!(!error(&h), "{:?}", h.app.status); + assert_eq!(h.app.menu.target, Target::VisualStyle); + assert!(h.app.viewport.context_menu.is_some()); + let menu = describe(&h.app); + let labels: Vec<_> = menu["items"].as_array().unwrap().iter().map(|i| i["label"].as_str().unwrap().to_owned()).collect(); + assert_eq!(labels, ["Shaded with edges", "Shaded", "Wireframe", "Shaded with hidden edges"]); + let rows = items(&h.app, &Target::VisualStyle); + assert_eq!(rows.iter().filter(|i| i.shortcut == "current").map(|i| i.label.as_str()).collect::>(), ["Shaded with edges"]); + h.call("ui.menuPick", json!({"item":"Wireframe"})); + assert_eq!(h.app.ui.visual_style, 2); + assert!(h.app.viewport.context_menu.is_none()); + assert!(!error(&h)); + assert!(h.app.session.undo.is_empty(), "a display style is not a design edit"); + // The real ribbon path: the VIEW tab's panel drop-down, then the Visual style row. + h.step(&json!({"click":{"handle":"tab:VIEW"}})).unwrap(); + h.step(&json!({"click":{"handle":"panel:VIEW:VIEWS"}})).unwrap(); + h.step(&json!({"click":{"handle":"toolbar:view.visual_style"}})).unwrap(); + assert!(!error(&h), "{:?}", h.app.status); + assert_eq!(h.app.menu.target, Target::VisualStyle); + assert!(h.app.viewport.context_menu.is_some()); + assert_eq!(items(&h.app, &Target::VisualStyle).iter().find(|i| i.label == "Wireframe").unwrap().shortcut, "current"); + // The drop-down closes on a pick, so the toggle is checked with the opener's rect directly. + let button = egui::Rect::from_min_size(egui::pos2(40.0, 60.0), egui::vec2(28.0, 28.0)); + h.app.start_at("view.visual_style", Some(button)); + assert_eq!(h.app.viewport.context_menu, Some(button.left_bottom())); + h.app.start_at("view.visual_style", Some(button)); + assert!(h.app.viewport.context_menu.is_none(), "a second click closes the menu"); + let missing = h.call("engine.execute", json!({"command":"view.visual_style","params":{}})); + assert_eq!(missing["ok"], false); + assert!(h.app.viewport.context_menu.is_none()); + } + + #[test] + fn view_tool_toolbar_start_offers_orbit_pan_zoom_and_releases_the_latched_one() { + let mut h = crate::scenario::Harness::new(); + h.app.home.open = false; + h.app.status = None; + let error = |h: &crate::scenario::Harness| h.app.status.as_ref().is_some_and(|(_, _, error)| *error); + h.call("ui.start", json!({"command":"view.tool"})); + assert!(!error(&h), "{:?}", h.app.status); + assert_eq!(h.app.menu.target, Target::ViewTool); + let menu = describe(&h.app); + let labels: Vec<_> = menu["items"].as_array().unwrap().iter().map(|i| i["label"].as_str().unwrap().to_owned()).collect(); + assert_eq!(labels, ["Orbit", "Pan", "Zoom"]); + assert!(items(&h.app, &Target::ViewTool).iter().all(|i| i.shortcut != "current")); + h.call("ui.menuPick", json!({"item":"Pan"})); + assert_eq!(h.app.viewport.nav, Some(crate::viewport::NavMode::Pan)); + h.call("ui.start", json!({"command":"view.tool"})); + assert_eq!(items(&h.app, &Target::ViewTool).iter().find(|i| i.label == "Pan").unwrap().shortcut, "current"); + h.call("ui.menuPick", json!({"item":"Pan"})); + assert_eq!(h.app.viewport.nav, None, "picking the latched tool releases it, as its navigation-bar button does"); + assert!(!error(&h)); + } + #[test] fn line_offset_context_menu_breaks_association_and_allows_trim() { use solidcraft_engine::{geom::Vec2, sketch::Shape}; diff --git a/crates/ui-egui/src/i18n_menu_tests.rs b/crates/ui-egui/src/i18n_menu_tests.rs index 247e7ec2b2..4845ec4563 100644 --- a/crates/ui-egui/src/i18n_menu_tests.rs +++ b/crates/ui-egui/src/i18n_menu_tests.rs @@ -42,6 +42,8 @@ fn take(app: &SolveApp, out: &mut BTreeSet) { Target::Workspace, Target::NamedViews, Target::InsertDesign, + Target::VisualStyle, + Target::ViewTool, ] { take_items(out, &context_menu::items(app, &target)); } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c5c976dd17..c68cc8dd69 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1409,6 +1409,18 @@ impl SolveApp { }); } + /// A command that needs one choice from a short list: its menu under the button that started it + /// (a second click closes it), or in the middle of the viewport when started without one + /// (command search, a shortcut, `ui.start`). + fn open_choice_menu(&mut self, opener: Option, target: context_menu::Target) { + if let Some(rect) = opener { + context_menu::toggle_for(self, rect, rect.left_bottom(), target); + } else { + let at = self.viewport.rect.map(|rect| rect.center()).unwrap_or(egui::pos2(300.0, 200.0)); + context_menu::open_for(self, at, target); + } + } + /// Start an interactive command with its menu anchored below the invoking button. pub(crate) fn start_at(&mut self, id: &str, opener: Option) { self.start_with_variant_memory(id, true, opener); @@ -1540,6 +1552,17 @@ impl SolveApp { let _ = self.run(id, json!({"mode":mode})); return; } + // A display choice opens its choices, as the navigation bar's menus do: run with no + // parameters it could only fail (#8868). + let choice = match id { + "view.visual_style" => Some(context_menu::Target::VisualStyle), + "view.tool" => Some(context_menu::Target::ViewTool), + _ => None, + }; + if let Some(target) = choice { + self.open_choice_menu(opener, target); + return; + } if id == "view.render_focus" { let _ = self.run(id, json!({"on": !self.shell.render_focus})); return; diff --git a/crates/ui-egui/src/menu_tests.rs b/crates/ui-egui/src/menu_tests.rs index d8a63a2cc0..d3c1b7cb76 100644 --- a/crates/ui-egui/src/menu_tests.rs +++ b/crates/ui-egui/src/menu_tests.rs @@ -1466,6 +1466,8 @@ fn every_context_target_has_identified_icon_rows_including_submenus() { Target::Group { id: group_id, scope: "c0".into() }, Target::Origin, Target::Units, + Target::VisualStyle, + Target::ViewTool, Target::ToolbarCommand { id: "solid.extrude".into() }, Target::Toolbar, Target::HistoryControls, diff --git a/crates/ui-egui/src/navigation_bar.rs b/crates/ui-egui/src/navigation_bar.rs index 9bcba8bc91..d833582038 100644 --- a/crates/ui-egui/src/navigation_bar.rs +++ b/crates/ui-egui/src/navigation_bar.rs @@ -17,6 +17,14 @@ pub(crate) const SELECTION_MODE_ICONS: &[(&str, &str)] = &[ ("sketch", "select_sketch"), ]; +/// The engine's style ids in index order (`ui.visual_style` 0..3), with the English menu label. +pub(crate) const VISUAL_STYLES: [(&str, &str); 4] = [ + ("shaded_with_edges", "Shaded with edges"), + ("shaded", "Shaded"), + ("wireframe", "Wireframe"), + ("shaded_with_hidden_edges", "Shaded with hidden edges"), +]; + pub(crate) fn visual_style_icon(style: u8) -> &'static str { match style { 1 => "style_shaded", diff --git a/crates/ui-egui/src/navigation_bar_view.rs b/crates/ui-egui/src/navigation_bar_view.rs index 3668ec8a13..99d8f050dd 100644 --- a/crates/ui-egui/src/navigation_bar_view.rs +++ b/crates/ui-egui/src/navigation_bar_view.rs @@ -433,7 +433,7 @@ pub(crate) fn context_menu(app: &mut SolveApp, response: &egui::Response, index: fn style_menu(app: &mut SolveApp, ui: &mut egui::Ui) -> bool { let mut chosen = false; - for (index, style) in ["shaded_with_edges", "shaded", "wireframe", "shaded_with_hidden_edges"].into_iter().enumerate() { + for (index, style) in navigation_bar::VISUAL_STYLES.iter().map(|(style, _)| *style).enumerate() { let label = format!("navbar.style.{style}"); let mut item = ui_kit::NavigationMenuRow::new(style, app.tr(&label), navigation_bar::visual_style_icon(index as u8)); item.selected = app.ui.visual_style == index as u8;