Merge pull request #2031 from tareq-halaby/fix/toolbar-alt-click-cycle-tools

Toolbar: Alt/Option-click cycles the flyout tool group

crates/ui-egui/src/toolbar.rs

@@ -103,23 +103,61 @@ pub fn show(app: &mut DesignApp, ui: &mut egui::Ui) {
                                 // on the tool (egui reports releases within 0.8 s as clicks).
                                 let hold_id = egui::Id::unique(("flyout_hold", gi));
                                 let ended_hold = resp.clicked() && ui.data_mut(|d| d.remove_temp::<bool>(hold_id)).unwrap_or(false);
-                                if resp.clicked() && !ended_hold {
-                                    chosen = Some(shown.id);
+                                // Plain click selects the shown tool; Alt/Option-click cycles the flyout
+                                // group, as the letter-key cycle does. Read the modifiers from the
+                                // primary-release event (Alt may already be up by the time this frame
+                                // reads i.modifiers), preferring a release on this slot; egui still
+                                // counts a click when the pointer drifts a little outside, so a release
+                                // whose press started here counts too, and one pressed elsewhere doesn't.
+                                let click_mods = ui.input(|i| {
+                                    let press_here = i.pointer.press_origin().is_some_and(|o| resp.rect.contains(o));
+                                    i.events
+                                        .iter()
+                                        .rev()
+                                        .find_map(|e| match e {
+                                            egui::Event::PointerButton {
+                                                pos,
+                                                button: egui::PointerButton::Primary,
+                                                pressed: false,
+                                                modifiers,
+                                                ..
+                                            } if resp.rect.contains(*pos) || press_here => Some(*modifiers),
+                                            _ => None,
+                                        })
+                                        .unwrap_or(i.modifiers)
+                                });
+                                let alt = click_mods.alt;
+                                let modified = click_mods.alt || click_mods.shift || click_mods.ctrl || click_mods.command;
+                                // egui's double-click ignores modifiers: remember whether the previous
+                                // click was modified so dialogs only open for two plain clicks.
+                                let prev_mod_id = egui::Id::unique(("tool-click-mod", gi));
+                                let prev_modified = ui.data(|d| d.get_temp::<bool>(prev_mod_id)).unwrap_or(false);
+                                let slot_click = resp.clicked() && !ended_hold;
+                                if slot_click {
+                                    chosen = Some(if g.len() > 1 && alt {
+                                        let from = g.iter().position(|tl| tl.id == current).unwrap_or(0);
+                                        g[(from + 1) % g.len()].id
+                                    } else {
+                                        shown.id
+                                    });
                                 }
                                 if !resp.is_pointer_button_down_on() {
                                     ui.data_mut(|d| d.remove::<bool>(hold_id));
                                 }
                                 // Double-click a polygon tool: Polygon Settings.
-                                if resp.double_clicked() && matches!(shown.id, "polygon" | "polygonFrame") {
+                                // Modified clicks only cycle / select: exclude them from dialogs.
+                                let plain_double = resp.double_clicked() && !modified && !prev_modified;
+                                if plain_double && matches!(shown.id, "polygon" | "polygonFrame") {
                                     let cur = app.session.execute("tool.polygonSettings", &json!({})).unwrap_or_default();
                                     app.ui.dialog = Some(crate::dialogs::Dialog::new("polygonSettings", cur));
                                 }
                                 // Double-click a transform tool: its Object › Transform dialog.
-                                if resp.double_clicked()
-                                    && let Some(cmd) = transform_dialog_of(shown.id)
-                                {
+                                if plain_double && let Some(cmd) = transform_dialog_of(shown.id) {
                                     crate::menus::activate(app, cmd, &Value::Null);
                                 }
+                                if slot_click {
+                                    ui.data_mut(|d| d.insert_temp(prev_mod_id, modified));
+                                }
                                 let held =
                                     resp.is_pointer_button_down_on() && ui.input(|i| i.pointer.press_start_time().is_some_and(|s| i.time - s > 0.35));
                                 if g.len() > 1 && held {

crates/ui-egui/src/toolbar/tests.rs

@@ -378,3 +378,54 @@ fn fill_stroke_proxy_keys() {
     press(&mut app, &ctx, Key::X, Modifiers::NONE);
     assert_eq!(app.ui.stroke_front, front, "X while typing leaves the proxy alone");
 }
+
+/// Alt/Option-click cycles a tool group's flyout without opening the menu.
+#[test]
+fn alt_click_cycles_the_tool_flyout_group() {
+    use egui::{Event, Modifiers, PointerButton};
+    use egui_kittest::kittest::Queryable;
+    let app = crate::DesignApp::new(designcraft_engine::Session::new(), crate::Services::default());
+    // 1 ms frames so an Alt-click and a following plain click can form a real double-click
+    // (default open() spaces frames by 0.25 s, outside egui's double-click window).
+    let mut h = crate::test_window::open_stepped(app, vec2(1400.0, 900.0), 1.0, 0.001);
+    h.state_mut().app.select_tool("rectangleFrame");
+    h.run_steps(2);
+    let at = h.get_by_label("Rectangle Frame Tool (F)").rect().center();
+    let alt = Modifiers { alt: true, ..Default::default() };
+    // 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));
+        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);
+    };
+    // Break the double-click window between independent clicks (~0.4 s at 1 ms/step).
+    let settle = |h: &mut egui_kittest::Harness<'static, crate::test_window::Window>| h.run_steps(400);
+    click(&mut h, Modifiers::NONE, Modifiers::NONE);
+    assert_eq!(h.state().app.session.tool_id(), "rectangleFrame");
+    assert!(h.state().app.ui.flyout.is_none());
+    settle(&mut h);
+    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");
+    settle(&mut h);
+    click(&mut h, alt, Modifiers::NONE);
+    assert_eq!(h.state().app.session.tool_id(), "polygonFrame");
+    settle(&mut h);
+    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");
+    settle(&mut h);
+    // Alt-click onto polygonFrame, then a plain click inside the double-click window: egui
+    // reports double_clicked, but prev_modified must keep Polygon Settings closed.
+    h.state_mut().app.select_tool("ellipseFrame");
+    h.run_steps(2);
+    click(&mut h, alt, Modifiers::NONE);
+    assert_eq!(h.state().app.session.tool_id(), "polygonFrame");
+    click(&mut h, Modifiers::NONE, Modifiers::NONE);
+    assert!(h.state().app.ui.dialog.is_none(), "plain click after Alt-click must not open Polygon Settings");
+    assert_eq!(h.state().app.session.tool_id(), "polygonFrame");
+}