From 8f9fe5226bfb25da700fe505ef767c95ff715e34 Mon Sep 17 00:00:00 2001 From: Hakan Seven Date: Fri, 29 May 2026 01:23:25 +0300 Subject: [PATCH] fix(ribbon): clear blue tool state when its popup window closes (#40) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A ribbon tool that opens a popup window (LAYERS, PAGESETUP, ABOUT, the style managers, …) marks itself blue via \`activate_tool\` when the user clicks it, but nothing un-marked the tool after the window was dismissed — \`OsWindowClosed\` cleared the matching \`_window\` slot, the active-tool string stayed put, and the button kept rendering as if the panel were still open. Add \`Ribbon::deactivate_tool_if(id)\` (clears only when the active tool matches) and call it for every popup-window slot in \`OsWindowClosed\`. Also clear inside the \`ToggleLayers\` close branch — that path \`take()\`s \`layer_window\` to \`None\` before \`OsWindowClosed\` fires, so the conditional in the \`OsWindowClosed\` handler would miss the close otherwise. Mapped IDs match the matching ribbon \`ToolDef.id\`s (LAYERS, PAGESETUP, ABOUT) plus command names for the popups normally opened via the command line (TEXTSTYLE, TABLESTYLE, MLSTYLE, LAYOUTMANAGER / LAYOUTPANEL, PLOTSTYLE / STYLESMANAGER, DIMSTYLE, SHORTCUTS / KEYBOARD) so a future ribbon tool with one of those ids gets the same correct deactivate. Closes #40 --- src/app/update.rs | 25 +++++++++++++++++++++++++ src/ui/ribbon/mod.rs | 9 +++++++++ 2 files changed, 34 insertions(+) diff --git a/src/app/update.rs b/src/app/update.rs index d504b056b..a1c10f722 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -1215,6 +1215,11 @@ impl OpenCADStudio { Message::ToggleLayers => { if let Some(id) = self.layer_window.take() { + // Close path: `OsWindowClosed`'s deactivate guard + // sees `layer_window == None` after the take(), so do + // it here so the button flips off in the same frame + // (#40). + self.ribbon.deactivate_tool_if("LAYERS"); window::close(id) } else { self.sync_ribbon_layers(); @@ -1255,35 +1260,55 @@ impl OpenCADStudio { self.save_dialog_window = None; return Task::none(); } + // Each popup window that's launched from a ribbon tool + // also turns that tool blue (`activate_tool`); when the + // window closes, the matching tool needs to be cleared + // or the button stays highlighted with no window behind + // it. The mapped IDs are the ribbon `ToolDef.id`s that + // dispatched the open in the first place. See #40. if self.layer_window == Some(id) { self.layer_window = None; + self.ribbon.deactivate_tool_if("LAYERS"); } if self.page_setup_window == Some(id) { self.page_setup_window = None; + self.ribbon.deactivate_tool_if("PAGESETUP"); } if self.textstyle_window == Some(id) { self.textstyle_window = None; + self.ribbon.deactivate_tool_if("STYLE"); + self.ribbon.deactivate_tool_if("TEXTSTYLE"); } if self.tablestyle_window == Some(id) { self.tablestyle_window = None; + self.ribbon.deactivate_tool_if("TABLESTYLE"); } if self.mlstyle_window == Some(id) { self.mlstyle_window = None; + self.ribbon.deactivate_tool_if("MLSTYLE"); } if self.layout_manager_window == Some(id) { self.layout_manager_window = None; + self.ribbon.deactivate_tool_if("LAYOUTMANAGER"); + self.ribbon.deactivate_tool_if("LAYOUTPANEL"); } if self.plotstyle_window == Some(id) { self.plotstyle_window = None; + self.ribbon.deactivate_tool_if("PLOTSTYLE"); + self.ribbon.deactivate_tool_if("STYLESMANAGER"); } if self.dimstyle_window == Some(id) { self.dimstyle_window = None; + self.ribbon.deactivate_tool_if("DIMSTYLE"); } if self.shortcuts_window == Some(id) { self.shortcuts_window = None; + self.ribbon.deactivate_tool_if("SHORTCUTS"); + self.ribbon.deactivate_tool_if("KEYBOARD"); } if self.about_window == Some(id) { self.about_window = None; + self.ribbon.deactivate_tool_if("ABOUT"); } if self.update_notice_window == Some(id) { self.update_notice_window = None; diff --git a/src/ui/ribbon/mod.rs b/src/ui/ribbon/mod.rs index fbce9a0dc..c8a48f640 100644 --- a/src/ui/ribbon/mod.rs +++ b/src/ui/ribbon/mod.rs @@ -144,6 +144,15 @@ impl Ribbon { pub fn deactivate_tool(&mut self) { self.active_tool = None; } + /// Clear `active_tool` only when it currently equals `id`. Used by the + /// window-close path to deactivate the tool that owned a popup window + /// without disturbing a different tool the user picked in the + /// meantime. See #40. + pub fn deactivate_tool_if(&mut self, id: &str) { + if self.active_tool.as_deref() == Some(id) { + self.active_tool = None; + } + } pub fn set_wireframe(&mut self, w: bool) { self.wireframe = w; }