graphic design tool
git clone https://git.lucas.co/cce-designer.git
refactor: the menubars dispatch only what no command does
The five menubars are never drawn, so MCP's menu_click was the one thing
that could click them, and their index matches had drifted from their
item lists: the header's Save opened a project. Everything they listed
but the active camera and the parameter presets is a registry command,
so those arms go, and menu_click refuses a menu it does not dispatch.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
CLAUDE.md | 12 ++
src/api.rs | 2 +-
src/main.rs | 24 ++++
src/window.rs | 350 ++++------------------------------------------------------
4 files changed, 61 insertions(+), 327 deletions(-)
diff --git a/CLAUDE.md b/CLAUDE.md
index deebd54..21978cd 100644
--- a/CLAUDE.md
+++ b/CLAUDE.md
@@ -100,6 +100,18 @@ the window is off-screen. The
tool list lives in `mcp_tools()` in `src/api.rs`; the protocol layer is
`cce_ui::mcp` (tools-only Streamable HTTP). Keep the enum, the tool list, and
the schemas in sync — `test_mcp_tools_map_to_actions` enforces the mapping.
+**`menu_click` dispatches three menus and refuses the rest** (since
+2026-09-29). The five menubars are roster slots that are never drawn —
+`HEADER_H` and `MENUBAR_H` are 0 — kept as pane identities and for their
+checkmarks, so the MCP tool is the only thing that can click one. What
+`process_window_event` still dispatches by index is what no registry command
+does: the viewport menubar's Camera menu, and the parameters menubar's Preset
+and Reset (`window::menu_is_dispatched`). Everything else they list is a
+command, reached through `run_command`; a click on it is an error saying so,
+where it used to be accepted and, the item lists having drifted from the
+matches, ran the wrong item (the header's Save opened a project).
+`a_menubar_click_is_dispatched_or_refused` is the test.
+
(The former bespoke HTTP API on port 3000 was retired in favor of this;
app-internal threads like the cce-files choosers now return results via
`CustomEvent::RunAction` instead of POSTing to it.)
diff --git a/src/api.rs b/src/api.rs
index 740f400..1e45797 100644
--- a/src/api.rs
+++ b/src/api.rs
@@ -236,7 +236,7 @@ pub(crate) fn mcp_tools() -> Vec<McpTool> {
),
tool(
"menu_click",
- "Click a menubar item by indices (widget_idx must be a menubar widget slot).",
+ "Click a menubar item by indices. Only the viewport menubar's Camera menu (menu 0) and the parameters menubar's Preset and Reset menus (0, 1) are dispatched this way; everything else is a command, see run_command.",
json!({
"type": "object",
"properties": {
diff --git a/src/main.rs b/src/main.rs
index f764ccc..4581726 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -6629,6 +6629,30 @@ mod tests {
}
}
+ /// The menubars are not drawn, and what they listed is commands. A
+ /// click by index is dispatched for the two things no command does —
+ /// the active camera, the parameter presets — and refused for the rest,
+ /// where it used to be accepted and, for the header's File menu, run
+ /// the item one below the one named.
+ #[test]
+ fn a_menubar_click_is_dispatched_or_refused() {
+ use crate::app::McpAction;
+ let mut state = State::new(false);
+ let mut redraw = false;
+ state.set_active_camera("camera1");
+ state
+ .apply_action(McpAction::MenuClick { widget_idx: RIGHT_MENUBAR_IDX, menu_idx: 0, item_idx: 0 }, &mut redraw)
+ .expect("the Camera menu is dispatched");
+ assert_eq!(state.active_camera, "Default Camera");
+
+ let nodes = state.fs_root.children.len();
+ for (widget_idx, menu_idx) in [(crate::slots::HEADER_IDX, 0), (LEFT_MENUBAR_IDX, 0), (RIGHT_MENUBAR_IDX, 2)] {
+ let res = state.apply_action(McpAction::MenuClick { widget_idx, menu_idx, item_idx: 0 }, &mut redraw);
+ assert!(res.is_err(), "menubar {widget_idx} menu {menu_idx} was accepted: {res:?}");
+ }
+ assert_eq!(state.fs_root.children.len(), nodes, "a refused click ran New Project");
+ }
+
/// New Project from the palette starts a project. The command named a
/// label no arm dispatched, so the row ran and nothing happened.
#[test]
diff --git a/src/window.rs b/src/window.rs
index 1280583..7874477 100644
--- a/src/window.rs
+++ b/src/window.rs
@@ -9,8 +9,7 @@
use std::path::Path;
use cce_ui::widget::WidgetHost;
-use crate::shortcut::Action;
-use crate::app::{State, CustomEvent, McpAction, get_next_visible_pane, Project, ParamDef};
+use crate::app::{State, CustomEvent, McpAction, Project, ParamDef};
use crate::slots::{LEFT_MENUBAR_IDX, RIGHT_MENUBAR_IDX, PARAM_MENUBAR_IDX, SPREADSHEET_MENUBAR_IDX, HEADER_IDX, PARAM_IDX, WIDGET_COUNT};
#[derive(Debug, Clone, Copy)]
@@ -26,6 +25,17 @@ pub enum WindowEvent {
KeyboardInput { event: cce_ui::widget::KeyEvent },
}
+/// The menubar menus `process_window_event` still dispatches by index: the
+/// viewport's Camera menu and the parameters' Preset and Reset. The rest of
+/// what the menubars list is a registry command.
+pub(crate) fn menu_is_dispatched(widget_idx: usize, menu_idx: usize) -> bool {
+ match widget_idx {
+ RIGHT_MENUBAR_IDX => menu_idx == 0,
+ PARAM_MENUBAR_IDX => menu_idx <= 1,
+ _ => false,
+ }
+}
+
impl State {
/// Route a window event through `handle_event`, then run the post-event
/// side-effect pass (menu clicks, pane toggles, pending actions).
@@ -141,241 +151,11 @@ impl State {
}
}
- if let Some((menu_idx, item_idx)) = state.menu_mut(HEADER_IDX).menu_click() {
- if menu_idx == 0 { // File
- match item_idx {
- 0 => { // New Project
- state.new_project();
- changed = true;
- }
- 1 => { // Open
- state.open_file_chooser();
- changed = true;
- }
- 2 => { // Save
- let path_opt = state.loaded_project_path.clone();
- if let Some(path) = path_opt {
- if let Err(e) = state.save_to_file(&path) {
- eprintln!("Failed to save project: {:?}", e);
- state.update_status_text(&format!("Failed to save: {:?}", e));
- } else {
- state.update_status_text(&format!("Project saved to {}", path.display()));
- state.add_recent_file(path);
- }
- } else {
- state.save_file_chooser();
- }
- changed = true;
- }
- 3 => { // Save As
- state.save_file_chooser();
- changed = true;
- }
- 4 => { // Exit
- state.exit_requested = true;
- }
- _ => {}
- }
- } else if menu_idx == 2 { // View
- match item_idx {
- 0 => { // Zoom In
- state.zoom(1.15, None);
- changed = true;
- }
- 1 => { // Zoom Out
- state.zoom(1.0 / 1.15, None);
- changed = true;
- }
- 2 => { // Reset Zoom
- state.set_grid_geometry(crate::app::configured_grid_geometry());
- state.sync_grid_settings();
- changed = true;
- }
- 3 => { // Detach Circular Window
- state.execute_action(Action::DetachCircularWindow);
- changed = true;
- }
- 4 => { // Show Network Pane
- state.show_network = !state.show_network;
- state.slots.content.set_visible(state.show_network);
- state.slots.left_menubar.set_visible(state.show_network);
- state.slots.breadcrumb.set_visible(state.show_network);
- let val = state.show_network;
- state.menu_mut(HEADER_IDX).set_item_checked(2, 4, val);
- if !state.show_network && state.focused_pane == LEFT_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- 5 => { // Show Viewport Pane
- state.show_viewport = !state.show_viewport;
- state.slots.viewport.set_visible(state.show_viewport);
- state.slots.right_menubar.set_visible(state.show_viewport);
- let val = state.show_viewport;
- state.menu_mut(HEADER_IDX).set_item_checked(2, 5, val);
- if !state.show_viewport && state.focused_pane == RIGHT_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- 6 => { // Show Parameters Pane
- state.show_parameters = !state.show_parameters;
- state.slots.param.set_visible(state.show_parameters);
- state.slots.param_menubar.set_visible(state.show_parameters);
- let val = state.show_parameters;
- state.menu_mut(HEADER_IDX).set_item_checked(2, 6, val);
- if !state.show_parameters && state.focused_pane == PARAM_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- 7 => { // Show Spreadsheet Pane
- state.show_spreadsheet = !state.show_spreadsheet;
- state.slots.spreadsheet.set_visible(state.show_spreadsheet);
- state.slots.spreadsheet_menubar.set_visible(state.show_spreadsheet);
- let val = state.show_spreadsheet;
- state.menu_mut(HEADER_IDX).set_item_checked(2, 7, val);
- if !state.show_spreadsheet && state.focused_pane == SPREADSHEET_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- 8 => { // Show Playbar Pane
- state.execute_menu_action("Show Playbar Pane");
- state.sync_nodes();
- changed = true;
- }
- _ => {}
- }
- }
- }
-
- if let Some((menu_idx, item_idx)) = state.menu_mut(LEFT_MENUBAR_IDX).menu_click() {
- if menu_idx == 0 { // File
- match item_idx {
- 0 => { // New
- state.new_project();
- changed = true;
- }
- 1 => { // Open
- state.open_file_chooser();
- changed = true;
- }
- 2 => { // Save
- let path_opt = state.loaded_project_path.clone();
- if let Some(path) = path_opt {
- if let Err(e) = state.save_to_file(&path) {
- eprintln!("Failed to save project: {:?}", e);
- state.update_status_text(&format!("Failed to save: {:?}", e));
- } else {
- state.update_status_text(&format!("Project saved to {}", path.display()));
- state.add_recent_file(path);
- }
- } else {
- state.save_file_chooser();
- }
- changed = true;
- }
- 3 => { // Save As
- state.save_file_chooser();
- changed = true;
- }
- _ => {}
- }
- } else if menu_idx == 2 { // View
- match item_idx {
- 0 => { // Zoom In
- state.zoom(1.15, None);
- changed = true;
- }
- 1 => { // Zoom Out
- state.zoom(1.0 / 1.15, None);
- changed = true;
- }
- 2 => {
- state.circular_network_pane = !state.circular_network_pane;
- let val = state.circular_network_pane;
- state.menu_mut(LEFT_MENUBAR_IDX).set_item_checked(2, 2, val);
- state.rebuild_positions();
- state.apply_layout();
- state.sync_grid_settings();
- changed = true;
- }
- 3 => { // Detach Pane
- state.execute_action(Action::DetachCircularWindow);
- changed = true;
- }
- 4 => { // Close Pane
- state.show_network = false;
- state.slots.content.set_visible(false);
- state.slots.left_menubar.set_visible(false);
- state.slots.breadcrumb.set_visible(false);
- state.menu_mut(HEADER_IDX).set_item_checked(2, 4, false);
- if state.focused_pane == LEFT_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- _ => {}
- }
- }
- }
-
+ // The menubars are not drawn (their bars have no height), so a
+ // click reaches these only through MCP's `menu_click`. What is
+ // dispatched here is what no registry command does: choosing the
+ // active camera, and the parameter presets. Everything else the
+ // menubars list is a command, and is run as one.
if let Some((menu_idx, item_idx)) = state.menu_mut(RIGHT_MENUBAR_IDX).menu_click() {
if menu_idx == 0 {
let camera_nodes: Vec<String> = state.current_dir().children.iter()
@@ -392,43 +172,6 @@ impl State {
}
changed = true;
}
- } else if menu_idx == 3 { // View
- if item_idx == 0 { // Close Pane
- state.show_viewport = false;
- state.slots.viewport.set_visible(false);
- state.slots.right_menubar.set_visible(false);
- state.menu_mut(HEADER_IDX).set_item_checked(2, 5, false);
- if state.focused_pane == RIGHT_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- } else {
- let action = if menu_idx == 1 {
- Some(Action::ToggleSquareViewport)
- } else if menu_idx == crate::app::GUIDES_MENU {
- match item_idx {
- crate::app::GUIDE_GRID => Some(Action::ToggleGrid),
- crate::app::GUIDE_ORIGIN => Some(Action::ToggleOrigin),
- crate::app::GUIDE_CAMERA_PIVOT => Some(Action::ToggleCameraPivot),
- _ => None,
- }
- } else { None };
- if let Some(a) = action {
- state.execute_action(a);
- changed = true;
- }
}
}
@@ -501,54 +244,6 @@ impl State {
}
}
}
- } else if menu_idx == 2 { // View
- if item_idx == 0 { // Close Pane
- state.show_parameters = false;
- state.slots.param.set_visible(false);
- state.slots.param_menubar.set_visible(false);
- state.menu_mut(HEADER_IDX).set_item_checked(2, 6, false);
- if state.focused_pane == PARAM_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
- }
- }
-
- if let Some((menu_idx, item_idx)) = state.menu_mut(SPREADSHEET_MENUBAR_IDX).menu_click() {
- if menu_idx == 0 { // View
- if item_idx == 0 { // Close Pane
- state.show_spreadsheet = false;
- state.slots.spreadsheet.set_visible(false);
- state.slots.spreadsheet_menubar.set_visible(false);
- state.menu_mut(HEADER_IDX).set_item_checked(2, 7, false);
- if state.focused_pane == SPREADSHEET_MENUBAR_IDX {
- state.focused_pane = get_next_visible_pane(
- state.focused_pane,
- state.show_network,
- state.show_viewport,
- state.show_parameters,
- state.show_spreadsheet,
- false,
- );
- }
- state.rebuild_positions();
- state.apply_layout();
- state.sync_pane_focus();
- state.sync_nodes();
- changed = true;
- }
}
}
@@ -1007,12 +702,15 @@ impl State {
McpAction::MenuClick { widget_idx, menu_idx, item_idx } => {
// Validate before touching menu_mut(): a non-menubar widget_idx
// panics its MenuBar downcast, and out-of-range menu/item indices
- // used to reply "Menu clicked" while dispatching nowhere. NB the
- // pane-toggle items ("Show Spreadsheet Pane", ...) are NOT in these
- // menubars — they are toggle params in the menu pane, drained by
- // sync_parameters_to_project's label match, unreachable from here.
+ // used to reply "Menu clicked" while dispatching nowhere.
let validated: Result<String, String> = if widget_idx >= WIDGET_COUNT {
Err(format!("widget_idx {widget_idx} out of range (widget slots: 0..{WIDGET_COUNT})"))
+ } else if state.menubar_at(widget_idx).is_some() && !menu_is_dispatched(widget_idx, menu_idx) {
+ // Everything else a menubar lists is a registry command;
+ // a click that was accepted here would dispatch nowhere.
+ Err(format!(
+ "menu {menu_idx} of menubar {widget_idx} is not dispatched by index: use run_command"
+ ))
} else if let Some(menubar) = state.menubar_at(widget_idx) {
match menubar.menu_dropdowns.get(menu_idx) {
None => Err(format!(