Donate, Send Feedback, OCS Web and Manage > About all answered "No drawing open — use New or Open first." and did nothing. Two doors guarded the welcome tab and the outer one shadowed the inner. `dispatch_command` already knew which commands stand alone — About, Donate, Report and the links were all on its list — but `on_ribbon_tool_click` refused every ribbon event before dispatch was ever reached. The welcome page's own buttons emit RibbonToolClick, so they are only ever clickable while `is_start` holds: the refusal covered exactly the case they exist in, and they had never worked. New Drawing / Open File / Plugins send their own messages, which is why the page still looked alive. So the ribbon door now defers commands to dispatch and keeps refusing only what touches the scene. The policy stays in one place instead of being half-remembered in two. A drawing tool is still turned away on the welcome tab (#299) — the wording just comes from dispatch now. The same gap was swallowing tools the issues never mentioned: CUI and ALIASEDIT configure the application, not a drawing, and were missing from the list, so fixing only the ribbon door would have moved their refusal one door down rather than removing it. HELP and CUILOAD / CUIIMPORT join them. The new test drives the real message path and fails on the old code with the reported wording. It reads the allowlist from source rather than dispatching DONATE / REPORT / WEBVERSION, which shell out to a browser — that also fails the day someone adds a welcome-page button and forgets the list. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
b3bd93d8c5
commit
f028e93a33
3 changed files with 132 additions and 6 deletions
|
|
@ -541,6 +541,116 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn start_page_runs_tools_that_need_no_drawing_but_still_refuses_the_rest() {
|
||||
// The welcome page's own buttons (Donate / Send Feedback / OCS Web) and
|
||||
// Manage > About route through RibbonToolClick, so a blanket is_start
|
||||
// refusal killed them outright — by definition they are only ever
|
||||
// clickable while is_start holds. `dispatch_command` owns the list of
|
||||
// commands that stand alone; this door must not shadow it. (#388, #389)
|
||||
use crate::app::Message;
|
||||
use crate::modules::ModuleEvent;
|
||||
|
||||
// Fresh app = welcome tab, no drawing.
|
||||
let mut app = OpenCADStudio::new_for_test();
|
||||
assert!(
|
||||
app.tabs[app.active_tab].is_start,
|
||||
"test needs the welcome tab"
|
||||
);
|
||||
|
||||
// ABOUT is safe to drive: it opens a modal. DONATE / WEBVERSION / REPORT
|
||||
// take the same path but shell out to a browser, so they are covered by
|
||||
// the allowlist assertion below rather than by dispatching them here.
|
||||
let start = app.command_line.history.len();
|
||||
let _ = app.update(Message::RibbonToolClick {
|
||||
tool_id: "ABOUT".to_string(),
|
||||
event: ModuleEvent::Command("ABOUT".to_string()),
|
||||
});
|
||||
let out: String = app.command_line.history[start..]
|
||||
.iter()
|
||||
.map(|e| e.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
!out.contains("No drawing open"),
|
||||
"ABOUT needs no drawing and must not be refused on the welcome page: {out:?}"
|
||||
);
|
||||
|
||||
// …but a tool that does need a drawing is still turned away (#299).
|
||||
let start = app.command_line.history.len();
|
||||
let _ = app.update(Message::RibbonToolClick {
|
||||
tool_id: "LINE".to_string(),
|
||||
event: ModuleEvent::Command("LINE".to_string()),
|
||||
});
|
||||
let out: String = app.command_line.history[start..]
|
||||
.iter()
|
||||
.map(|e| e.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
out.contains("No drawing open"),
|
||||
"LINE must still be refused on the welcome page: {out:?}"
|
||||
);
|
||||
assert!(
|
||||
app.tabs[app.active_tab].active_cmd.is_none(),
|
||||
"LINE must not have started"
|
||||
);
|
||||
|
||||
// A non-command tool event touches the scene, so it stays inert too.
|
||||
let start = app.command_line.history.len();
|
||||
let _ = app.update(Message::RibbonToolClick {
|
||||
tool_id: "LAYERS".to_string(),
|
||||
event: ModuleEvent::ToggleLayers,
|
||||
});
|
||||
let out: String = app.command_line.history[start..]
|
||||
.iter()
|
||||
.map(|e| e.text.as_str())
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
out.contains("No drawing open"),
|
||||
"a scene-touching event must stay inert on the welcome page: {out:?}"
|
||||
);
|
||||
|
||||
// Every welcome-page button must also clear dispatch's own gate, or the
|
||||
// fix above just moves the refusal one door down. Asserted on the source
|
||||
// rather than by dispatching: DONATE / REPORT / WEBVERSION shell out to
|
||||
// a real browser, which a test must not do.
|
||||
let dispatch_src = include_str!("commands/mod.rs");
|
||||
let gate = dispatch_src
|
||||
.split("is_start")
|
||||
.nth(1)
|
||||
.and_then(|s| s.split('{').next())
|
||||
.expect("the is_start gate moved — re-point this test");
|
||||
// DONATE/REPORT/WEBVERSION are the welcome page's own buttons; the rest
|
||||
// are ribbon tools that configure the application, not a drawing.
|
||||
let standalone = [
|
||||
"DONATE",
|
||||
"REPORT",
|
||||
"WEBVERSION",
|
||||
"ABOUT",
|
||||
"CHANGELOG",
|
||||
"CUI",
|
||||
"ALIASEDIT",
|
||||
];
|
||||
for cmd in standalone {
|
||||
assert!(
|
||||
gate.contains(&format!("\"{cmd}\"")),
|
||||
"{cmd} needs no drawing but is missing from dispatch's standalone \
|
||||
list, so it is refused on the welcome page"
|
||||
);
|
||||
}
|
||||
// …and each must actually have somewhere to land.
|
||||
let view_src = include_str!("commands/view.rs");
|
||||
for cmd in standalone {
|
||||
assert!(
|
||||
view_src.contains(&format!("\"{cmd}\" =>"))
|
||||
|| view_src.contains(&format!("\"{cmd}\" |")),
|
||||
"{cmd} has no dispatch arm"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn handing_over_an_already_open_drawing_switches_to_its_tab() {
|
||||
// Double-clicking a drawing that is already open should land on the tab
|
||||
|
|
|
|||
|
|
@ -103,6 +103,11 @@ impl OpenCADStudio {
|
|||
// command would silently do nothing. Allow only the commands that
|
||||
// make sense there (create / open a document, or quit) and tell the
|
||||
// user otherwise instead of running a no-op. See #96.
|
||||
// Anything that acts on the application rather than a drawing belongs
|
||||
// here: document lifecycle, the links, and the app-wide configuration
|
||||
// editors (shortcuts, aliases) — none of them read the scene. This is
|
||||
// the single place that decides; `on_ribbon_tool_click` defers to it
|
||||
// rather than keeping a second, blunter copy (#388, #389).
|
||||
if self.tabs[i].is_start
|
||||
&& !matches!(
|
||||
cmd,
|
||||
|
|
@ -117,6 +122,11 @@ impl OpenCADStudio {
|
|||
| "PLUGINMANAGER"
|
||||
| "DONATE"
|
||||
| "WEBVERSION"
|
||||
| "HELP"
|
||||
| "CUI"
|
||||
| "ALIASEDIT"
|
||||
| "CUILOAD"
|
||||
| "CUIIMPORT"
|
||||
)
|
||||
{
|
||||
self.command_line
|
||||
|
|
|
|||
|
|
@ -91,12 +91,18 @@ impl OpenCADStudio {
|
|||
|
||||
|
||||
pub(super) fn on_ribbon_tool_click(&mut self, tool_id: String, event: ModuleEvent) -> Task<Message> {
|
||||
// On the Start page there is no drawing to act on — ribbon
|
||||
// commands are inert. Point the user at New / Open instead of
|
||||
// running a command into the empty welcome tab (#299). The
|
||||
// quick-access New / Open / Save buttons use a separate path and
|
||||
// stay available.
|
||||
if self.tabs[self.active_tab].is_start {
|
||||
// On the Start page there is no drawing to act on, so a tool that
|
||||
// touches the scene is inert — point the user at New / Open
|
||||
// instead of running it into the empty welcome tab (#299).
|
||||
//
|
||||
// Commands are exempt: `dispatch_command` already decides which
|
||||
// ones stand alone (About, Donate, Report, the web link…) and
|
||||
// reports the rest. Refusing them here shadowed that list and
|
||||
// killed the welcome page's own buttons, which by definition can
|
||||
// only ever be clicked while `is_start` holds (#388, #389).
|
||||
// Keep the policy in one place — this door must not second-guess
|
||||
// it. Every other event below mutates the scene or its panels.
|
||||
if self.tabs[self.active_tab].is_start && !matches!(event, ModuleEvent::Command(_)) {
|
||||
self.ribbon.close_dropdown();
|
||||
self.command_line
|
||||
.push_info("No drawing open — use New or Open first.");
|
||||
|
|
|
|||
Loading…
Reference in a new issue