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.
This commit is contained in:
jeromehzl
2026-10-30 09:04:39 +00:00
committed by GitHub
parent 966b10e731
commit 7e22aed739
6 changed files with 133 additions and 1 deletions
+97
View File
@@ -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<Item> {
.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::<Vec<_>>(), ["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};
+2
View File
@@ -42,6 +42,8 @@ fn take(app: &SolveApp, out: &mut BTreeSet<String>) {
Target::Workspace,
Target::NamedViews,
Target::InsertDesign,
Target::VisualStyle,
Target::ViewTool,
] {
take_items(out, &context_menu::items(app, &target));
}
+23
View File
@@ -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<egui::Rect>, 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<egui::Rect>) {
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;
+2
View File
@@ -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,
+8
View File
@@ -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",
+1 -1
View File
@@ -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;