From 9476b39e89db970b9b3bbae5a2b4f1da68c2a836 Mon Sep 17 00:00:00 2001
From: tareq <tareq.halaby@gmail.com>
Date: Fri, 30 Oct 2026 13:56:25 +0100
Subject: [PATCH] Toolbar: bind click modifiers to the slot rect.

Ignore primary releases outside this tool button so another widget cannot steer Alt-cycle or suppress dialogs; harden the Alt-click test to clear i.modifiers before release.
---
 crates/ui-egui/src/toolbar.rs       |  5 ++++-
 crates/ui-egui/src/toolbar/tests.rs | 31 ++++++++++++++++++++---------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/crates/ui-egui/src/toolbar.rs b/crates/ui-egui/src/toolbar.rs
index 8362d2083..1ba172004 100644
--- a/crates/ui-egui/src/toolbar.rs
+++ b/crates/ui-egui/src/toolbar.rs
@@ -107,17 +107,20 @@ pub fn show(app: &mut DesignApp, ui: &mut egui::Ui) {
                                 // group (InDesign / Photoshop parity with the letter-key cycle).
                                 // Prefer the primary-release event's modifiers: Alt may already be up
                                 // by the time this frame reads i.modifiers.
+                                // Only a release on this slot: the frame's event list is shared,
+                                // so a later release elsewhere must not steer Alt-cycle / dialogs.
                                 let click_mods = ui.input(|i| {
                                     i.events
                                         .iter()
                                         .rev()
                                         .find_map(|e| match e {
                                             egui::Event::PointerButton {
+                                                pos,
                                                 button: egui::PointerButton::Primary,
                                                 pressed: false,
                                                 modifiers,
                                                 ..
-                                            } => Some(*modifiers),
+                                            } if resp.rect.contains(*pos) => Some(*modifiers),
                                             _ => None,
                                         })
                                         .unwrap_or(i.modifiers)
diff --git a/crates/ui-egui/src/toolbar/tests.rs b/crates/ui-egui/src/toolbar/tests.rs
index 5db130f63..5551ba2dd 100644
--- a/crates/ui-egui/src/toolbar/tests.rs
+++ b/crates/ui-egui/src/toolbar/tests.rs
@@ -390,23 +390,36 @@ fn alt_click_cycles_the_tool_flyout_group() {
     h.run_steps(2);
     let at = h.get_by_label("Rectangle Frame Tool (F)").rect().center();
     let alt = Modifiers { alt: true, ..Default::default() };
-    let click = |h: &mut egui_kittest::Harness<'static, crate::test_window::Window>, modifiers: Modifiers| {
-        h.input_mut().events.push(Event::ModifiersChanged(modifiers));
+    // Clear i.modifiers before the release while keeping Alt on the release event, so a
+    // regression that only reads i.modifiers (and ignores the pointer-release modifiers) fails.
+    let click = |h: &mut egui_kittest::Harness<'static, crate::test_window::Window>, press_modifiers: Modifiers, current_modifiers: Modifiers| {
+        h.input_mut().events.push(Event::ModifiersChanged(press_modifiers));
         h.input_mut().events.push(Event::PointerMoved(at));
-        for pressed in [true, false] {
-            h.input_mut().events.push(Event::PointerButton { pos: at, button: PointerButton::Primary, pressed, modifiers });
-        }
+        h.input_mut().events.push(Event::PointerButton {
+            pos: at,
+            button: PointerButton::Primary,
+            pressed: true,
+            modifiers: press_modifiers,
+        });
+        h.input_mut().events.push(Event::ModifiersChanged(current_modifiers));
+        h.input_mut().events.push(Event::PointerButton {
+            pos: at,
+            button: PointerButton::Primary,
+            pressed: false,
+            modifiers: press_modifiers,
+        });
         h.run_steps(3);
     };
-    click(&mut h, Modifiers::NONE);
+    click(&mut h, Modifiers::NONE, Modifiers::NONE);
     assert_eq!(h.state().app.session.tool_id(), "rectangleFrame");
     assert!(h.state().app.ui.flyout.is_none());
-    click(&mut h, alt);
+    click(&mut h, alt, Modifiers::NONE);
     assert_eq!(h.state().app.session.tool_id(), "ellipseFrame", "Alt-click advances the frame group");
     assert!(h.state().app.ui.flyout.is_none(), "Alt-click must not open the flyout");
-    click(&mut h, alt);
+    click(&mut h, alt, Modifiers::NONE);
     assert_eq!(h.state().app.session.tool_id(), "polygonFrame");
-    click(&mut h, alt);
+    assert!(h.state().app.ui.dialog.is_none(), "modified double-click must not open a dialog");
+    click(&mut h, alt, Modifiers::NONE);
     assert_eq!(h.state().app.session.tool_id(), "rectangleFrame", "Alt-click after the last group tool wraps to the first");
     assert!(h.state().app.ui.flyout.is_none(), "wrap-around Alt-click must not open the flyout");
 }