Change the definition of clicked_by (#4192)

This is a refactor on the way to add support for opening context menus
on touch screens via press-and-hold.

This PR changes what `InputState::button_clicked` does (it was ver badly
named before), and also changes `Response::clicked_by` to no longer be
true if clicking with keyboard (i.e. a widget has keyboard focus and the
user presses Space or Enter).

CHANGELOG.md

@@ -7,6 +7,13 @@ This file is updated upon each release.
 Changes since the last release can be found at <https://github.com/emilk/egui/compare/latest...HEAD> or by running the `scripts/generate_changelog.py` script.
 
 
+## Unreleased
+
+### ⚠️ BREAKING
+* `Response::clicked*` and `Response::dragged*` may lock the `Context`, so don't call it from a `Context`-locking closure.
+* `Response::clicked_by` will no longer be true if clicked with keyboard. Use `Response::clicked` instead.
+
+
 ## 0.26.2 - 2024-02-14
 * Avoid interacting twice when not required [#4041](https://github.com/emilk/egui/pull/4041) (thanks [@abey79](https://github.com/abey79)!)
 

crates/egui/src/context.rs

@@ -1104,9 +1104,8 @@ impl Context {
             contains_pointer: false,
             hovered: false,
             highlighted,
-            clicked: Default::default(),
-            double_clicked: Default::default(),
-            triple_clicked: Default::default(),
+            clicked: false,
+            fake_primary_click: false,
             drag_started: false,
             dragged: false,
             drag_stopped: false,
@@ -1131,15 +1130,15 @@ impl Context {
                 && (input.key_pressed(Key::Space) || input.key_pressed(Key::Enter))
             {
                 // Space/enter works like a primary click for e.g. selected buttons
-                res.clicked[PointerButton::Primary as usize] = true;
+                res.fake_primary_click = true;
             }
 
             #[cfg(feature = "accesskit")]
             if enabled
                 && sense.click
                 && input.has_accesskit_action_request(id, accesskit::Action::Default)
             {
-                res.clicked[PointerButton::Primary as usize] = true;
+                res.fake_primary_click = true;
             }
 
             let interaction = memory.interaction();
@@ -1157,13 +1156,9 @@ impl Context {
             let clicked = Some(id) == viewport.interact_widgets.clicked;
 
             for pointer_event in &input.pointer.pointer_events {
-                if let PointerEvent::Released { click, button } = pointer_event {
-                    if enabled && sense.click && clicked {
-                        if let Some(click) = click {
-                            res.clicked[*button as usize] = true;
-                            res.double_clicked[*button as usize] = click.is_double();
-                            res.triple_clicked[*button as usize] = click.is_triple();
-                        }
+                if let PointerEvent::Released { click, .. } = pointer_event {
+                    if enabled && sense.click && clicked && click.is_some() {
+                        res.clicked = true;
                     }
 
                     res.is_pointer_button_down_on = false;

crates/egui/src/input_state.rs

@@ -489,11 +489,7 @@ impl InputState {
     /// delivers a synthetic zoom factor based on ctrl-scroll events, as a fallback.
     pub fn multi_touch(&self) -> Option<MultiTouchInfo> {
         // In case of multiple touch devices simply pick the touch_state of the first active device
-        if let Some(touch_state) = self.touch_states.values().find(|t| t.is_active()) {
-            touch_state.info()
-        } else {
-            None
-        }
+        self.touch_states.values().find_map(|t| t.info())
     }
 
     /// True if there currently are any fingers touching egui.
@@ -976,11 +972,13 @@ impl PointerState {
         self.pointer_events.iter().any(|event| event.is_click())
     }
 
-    /// Was the button given clicked this frame?
+    /// Was the given pointer button given clicked this frame?
+    ///
+    /// Returns true on double- and triple- clicks too.
     pub fn button_clicked(&self, button: PointerButton) -> bool {
         self.pointer_events
             .iter()
-            .any(|event| matches!(event, &PointerEvent::Pressed { button: b, .. } if button == b))
+            .any(|event| matches!(event, &PointerEvent::Released { button: b, click: Some(_) } if button == b))
     }
 
     /// Was the button given double clicked this frame?

crates/egui/src/input_state/touch_state.rs

@@ -163,6 +163,7 @@ impl TouchState {
                 _ => (),
             }
         }
+
         // This needs to be called each frame, even if there are no new touch events.
         // Otherwise, we would send the same old delta information multiple times:
         self.update_gesture(time, pointer_pos);
@@ -176,10 +177,6 @@ impl TouchState {
         }
     }
 
-    pub fn is_active(&self) -> bool {
-        self.gesture_state.is_some()
-    }
-
     pub fn info(&self) -> Option<MultiTouchInfo> {
         self.gesture_state.as_ref().map(|state| {
             // state.previous can be `None` when the number of simultaneous touches has just

crates/egui/src/menu.rs

@@ -367,6 +367,9 @@ impl MenuRoot {
     /// Interaction with a context menu (secondary click).
     fn context_interaction(response: &Response, root: &mut Option<Self>) -> MenuResponse {
         let response = response.interact(Sense::click());
+        let hovered = response.hovered();
+        let secondary_clicked = response.secondary_clicked();
+
         response.ctx.input(|input| {
             let pointer = &input.pointer;
             if let Some(pos) = pointer.interact_pos() {
@@ -377,9 +380,9 @@ impl MenuRoot {
                     destroy = !in_old_menu && pointer.any_pressed() && root.id == response.id;
                 }
                 if !in_old_menu {
-                    if response.hovered() && response.secondary_clicked() {
+                    if hovered && secondary_clicked {
                         return MenuResponse::Create(pos, response.id);
-                    } else if (response.hovered() && pointer.primary_down()) || destroy {
+                    } else if destroy || hovered && pointer.primary_down() {
                         return MenuResponse::Close;
                     }
                 }

crates/egui/src/response.rs

@@ -3,7 +3,6 @@ use std::{any::Any, sync::Arc};
 use crate::{
     emath::{Align, Pos2, Rect, Vec2},
     menu, Context, CursorIcon, Id, LayerId, PointerButton, Sense, Ui, WidgetRect, WidgetText,
-    NUM_POINTER_BUTTONS,
 };
 
 // ----------------------------------------------------------------------------
@@ -15,7 +14,10 @@ use crate::{
 ///
 /// Whenever something gets added to a [`Ui`], a [`Response`] object is returned.
 /// [`ui.add`] returns a [`Response`], as does [`ui.button`], and all similar shortcuts.
-// TODO(emilk): we should be using bit sets instead of so many bools
+///
+/// ⚠️ The `Response` contains a clone of [`Context`], and many methods lock the `Context`.
+/// It can therefor be a deadlock to use `Context` from within a context-locking closures,
+/// such as [`Context::input`].
 #[derive(Clone, Debug)]
 pub struct Response {
     // CONTEXT:
@@ -69,18 +71,23 @@ pub struct Response {
     #[doc(hidden)]
     pub highlighted: bool,
 
-    /// The pointer clicked this thing this frame.
-    #[doc(hidden)]
-    pub clicked: [bool; NUM_POINTER_BUTTONS],
-
-    // TODO(emilk): `released` for sliders
-    /// The thing was double-clicked.
+    /// This widget was clicked this frame.
+    ///
+    /// Which pointer and how many times we don't know,
+    /// and ask [`crate::InputState`] about at runtime.
+    ///
+    /// This is only set to true if the widget was clicked
+    /// by an actual mouse.
     #[doc(hidden)]
-    pub double_clicked: [bool; NUM_POINTER_BUTTONS],
+    pub clicked: bool,
 
-    /// The thing was triple-clicked.
+    /// This widget should act as if clicked due
+    /// to something else than a click.
+    ///
+    /// This is set to true if the widget has keyboard focus and
+    /// the user hit the Space or Enter key.
     #[doc(hidden)]
-    pub triple_clicked: [bool; NUM_POINTER_BUTTONS],
+    pub fake_primary_click: bool,
 
     /// The widget started being dragged this frame.
     #[doc(hidden)]
@@ -118,55 +125,62 @@ impl Response {
     /// A click is registered when the mouse or touch is released within
     /// a certain amount of time and distance from when and where it was pressed.
     ///
+    /// This will also return true if the widget was clicked via accessibility integration,
+    /// or if the widget had keyboard focus and the use pressed Space/Enter.
+    ///
     /// Note that the widget must be sensing clicks with [`Sense::click`].
     /// [`crate::Button`] senses clicks; [`crate::Label`] does not (unless you call [`crate::Label::sense`]).
     ///
     /// You can use [`Self::interact`] to sense more things *after* adding a widget.
     #[inline(always)]
     pub fn clicked(&self) -> bool {
-        self.clicked[PointerButton::Primary as usize]
+        self.fake_primary_click || self.clicked_by(PointerButton::Primary)
     }
 
-    /// Returns true if this widget was clicked this frame by the given button.
+    /// Returns true if this widget was clicked this frame by the given mouse button.
+    ///
+    /// This will NOT return true if the widget was "clicked" via
+    /// some accessibility integration, or if the widget had keyboard focus and the
+    /// user pressed Space/Enter. For that, use [`Self::clicked`] instead.
     #[inline]
     pub fn clicked_by(&self, button: PointerButton) -> bool {
-        self.clicked[button as usize]
+        self.clicked && self.ctx.input(|i| i.pointer.button_clicked(button))
     }
 
     /// Returns true if this widget was clicked this frame by the secondary mouse button (e.g. the right mouse button).
     #[inline]
     pub fn secondary_clicked(&self) -> bool {
-        self.clicked[PointerButton::Secondary as usize]
+        self.clicked_by(PointerButton::Secondary)
     }
 
     /// Returns true if this widget was clicked this frame by the middle mouse button.
     #[inline]
     pub fn middle_clicked(&self) -> bool {
-        self.clicked[PointerButton::Middle as usize]
+        self.clicked_by(PointerButton::Middle)
     }
 
     /// Returns true if this widget was double-clicked this frame by the primary button.
     #[inline]
     pub fn double_clicked(&self) -> bool {
-        self.double_clicked[PointerButton::Primary as usize]
+        self.double_clicked_by(PointerButton::Primary)
     }
 
     /// Returns true if this widget was triple-clicked this frame by the primary button.
     #[inline]
     pub fn triple_clicked(&self) -> bool {
-        self.triple_clicked[PointerButton::Primary as usize]
+        self.triple_clicked_by(PointerButton::Primary)
     }
 
     /// Returns true if this widget was double-clicked this frame by the given button.
     #[inline]
     pub fn double_clicked_by(&self, button: PointerButton) -> bool {
-        self.double_clicked[button as usize]
+        self.clicked && self.ctx.input(|i| i.pointer.button_double_clicked(button))
     }
 
     /// Returns true if this widget was triple-clicked this frame by the given button.
     #[inline]
     pub fn triple_clicked_by(&self, button: PointerButton) -> bool {
-        self.triple_clicked[button as usize]
+        self.clicked && self.ctx.input(|i| i.pointer.button_triple_clicked(button))
     }
 
     /// `true` if there was a click *outside* this widget this frame.
@@ -917,27 +931,8 @@ impl Response {
             contains_pointer: self.contains_pointer || other.contains_pointer,
             hovered: self.hovered || other.hovered,
             highlighted: self.highlighted || other.highlighted,
-            clicked: [
-                self.clicked[0] || other.clicked[0],
-                self.clicked[1] || other.clicked[1],
-                self.clicked[2] || other.clicked[2],
-                self.clicked[3] || other.clicked[3],
-                self.clicked[4] || other.clicked[4],
-            ],
-            double_clicked: [
-                self.double_clicked[0] || other.double_clicked[0],
-                self.double_clicked[1] || other.double_clicked[1],
-                self.double_clicked[2] || other.double_clicked[2],
-                self.double_clicked[3] || other.double_clicked[3],
-                self.double_clicked[4] || other.double_clicked[4],
-            ],
-            triple_clicked: [
-                self.triple_clicked[0] || other.triple_clicked[0],
-                self.triple_clicked[1] || other.triple_clicked[1],
-                self.triple_clicked[2] || other.triple_clicked[2],
-                self.triple_clicked[3] || other.triple_clicked[3],
-                self.triple_clicked[4] || other.triple_clicked[4],
-            ],
+            clicked: self.clicked || other.clicked,
+            fake_primary_click: self.fake_primary_click || other.fake_primary_click,
             drag_started: self.drag_started || other.drag_started,
             dragged: self.dragged || other.dragged,
             drag_stopped: self.drag_stopped || other.drag_stopped,

crates/egui_demo_lib/src/demo/pan_zoom.rs

@@ -11,7 +11,7 @@ impl Eq for PanZoom {}
 
 impl super::Demo for PanZoom {
     fn name(&self) -> &'static str {
-        "🗖 Pan Zoom"
+        "🔍 Pan Zoom"
     }
 
     fn show(&mut self, ctx: &egui::Context, open: &mut bool) {