Merge pull request #2740 from pythonarcade/gui/fix-double-caret

Gui/fix double caret

Author: eruvanos

CHANGELOG.md

@@ -12,6 +12,14 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page.
 - Added `Text.visible` (bool) property to control the visibility of text objects.
 - Fixed an issue causing points and lines to draw random primitives when
   passing in an empty list.
+- GUI
+  - Fix caret did not deactivate because of consumed mouse events. [2725](https://github.com/pythonarcade/arcade/issues/2725)
+  - Property listener can now receive:
+      - no args
+      - instance
+      - instance, value
+      - instance, value, old value
+    >   Listener accepting `*args` receive `instance, value` like in previous versions.
 
 ## 3.3.0
 
@@ -36,12 +44,6 @@ Arcade [PyPi Release History](https://pypi.org/project/arcade/#history) page.
     - Added property setters for `center_x` and `center_y`
     - Added property setters for `left`, `right`, `top`, and `bottom`
     - Users can now set widget position and size more intuitively without needing to access the `rect` property
-  - Property listener can now receive:
-    - no args
-    - instance
-    - instance, value
-    - instance, value, old value
-    > Listener accepting `*args` receive `instance, value` like in previous versions.
 
 - Rendering:
   - The `arcade.gl` package was restructured to be more modular in preparation for

arcade/gui/property.py

@@ -230,8 +230,10 @@ class MyObject:
     """
     t = type(instance)
     prop = getattr(t, property)
-    if isinstance(prop, Property):
-        prop.bind(instance, callback)
+    if not isinstance(prop, Property):
+        raise ValueError(f"{t.__name__}.{property} is not an arcade.gui.Property")
+
+    prop.bind(instance, callback)
 
 
 def unbind(instance, property: str, callback):

arcade/gui/ui_manager.py

@@ -99,6 +99,12 @@ def on_draw():
     """Experimental feature to pixelate the UI, all textures will be rendered pixelated,
     which will mostly influence scaled background images.
     This property has to be set right after the UIManager is created."""
+    _active_widget: UIWidget | None = None
+    """The currently active widget. Any widget, which consumes mouse press or release events
+    should set itself as active widget.
+    UIManager ensures that only one widget can be active at a time,
+    which can be used by widgets like text fields to detect when they are disabled,
+    without relying on unconsumed mouse press or release events."""
 
     DEFAULT_LAYER = 0
     OVERLAY_LAYER = 10
@@ -518,6 +524,19 @@ def rect(self) -> Rect:
         """The rect of the UIManager, which is the window size."""
         return LBWH(0, 0, *self.window.get_size())
 
+    def _set_active_widget(self, widget: UIWidget | None):
+        if self._active_widget == widget:
+            return
+
+        if self._active_widget:
+            print(f"Deactivating widget {self._active_widget.__class__.__name__}")
+            self._active_widget._active = False
+
+        self._active_widget = widget
+        if self._active_widget:
+            print(f"Activating widget {self._active_widget.__class__.__name__}")
+            self._active_widget._active = True
+
     def debug(self):
         """Walks through all widgets of a UIManager and prints out layout information."""
         for index, layer in self.children.items():

arcade/gui/widgets/__init__.py

@@ -94,6 +94,8 @@ class UIWidget(EventDispatcher, ABC):
     This is not part of the public API and subject to change.
     UILabel have a strong background if set.
     """
+    _active = Property[bool](False)
+    """If True, the widget is active"""
 
     def __init__(
         self,
@@ -167,6 +169,23 @@ def add(self, child: W, **kwargs) -> W:
 
         return child
 
+    # TODO "focus" would be more intuative but clashes with the UIFocusGroups :/
+    # maybe the two systems should be merged?
+    def _grap_active(self):
+        """Sets itself as the single active widget in the UIManager."""
+        ui_manager: UIManager | None = self.get_ui_manager()
+        if ui_manager:
+            ui_manager._set_active_widget(self)
+
+    def _release_active(self):
+        """Make this widget inactive in the UIManager."""
+        if not self._active:
+            return
+
+        ui_manager: UIManager | None = self.get_ui_manager()
+        if ui_manager and ui_manager._active_widget is self:
+            ui_manager._set_active_widget(None)
+
     def remove(self, child: UIWidget) -> dict | None:
         """Removes a child from the UIManager which was directly added to it.
         This will not remove widgets which are added to a child of UIManager.
@@ -694,6 +713,7 @@ def on_event(self, event: UIEvent) -> bool | None:
             and event.button in self.interaction_buttons
         ):
             self.pressed = True
+            self._grap_active()  # make this the active widget
             return EVENT_HANDLED
 
         if (
@@ -705,6 +725,7 @@ def on_event(self, event: UIEvent) -> bool | None:
             if self.rect.point_in_rect(event.pos):
                 if not self.disabled:
                     # Dispatch new on_click event, source is this widget itself
+                    self._grap_active()  # make this the active widget
                     self.dispatch_event(
                         "on_click",
                         UIOnClickEvent(
@@ -715,7 +736,7 @@ def on_event(self, event: UIEvent) -> bool | None:
                             modifiers=event.modifiers,
                         ),
                     )
-                    return EVENT_HANDLED
+                    return EVENT_HANDLED  # TODO should we return the result from on_click?
 
         return EVENT_UNHANDLED
 

arcade/gui/widgets/text.py

@@ -16,6 +16,7 @@
     UIMouseDragEvent,
     UIMouseEvent,
     UIMousePressEvent,
+    UIMouseReleaseEvent,
     UIMouseScrollEvent,
     UIOnChangeEvent,
     UIOnClickEvent,
@@ -544,7 +545,6 @@ def __init__(
             **kwargs,
         )
 
-        self._active = False
         self._text_color = Color.from_iterable(text_color)
 
         self.doc: AbstractDocument = pyglet.text.decode_text(text)
@@ -574,10 +574,17 @@ def __init__(
         bind(self, "pressed", self._apply_style)
         bind(self, "invalid", self._apply_style)
         bind(self, "disabled", self._apply_style)
+        bind(self, "_active", self._on_active_changed)
 
         # initial style application
         self._apply_style()
 
+    def _on_active_changed(self):
+        """Handle the active state change of the input
+        text field to care about loosing active state."""
+        if not self._active:
+            self.deactivate()
+
     def _apply_style(self):
         style = self.get_current_style()
 
@@ -630,12 +637,25 @@ def on_event(self, event: UIEvent) -> bool | None:
 
         Text input is only active when the user clicks on the input field."""
         # If active check to deactivate
-        if self._active and isinstance(event, UIMousePressEvent):
-            if self.rect.point_in_rect(event.pos):
-                x = int(event.x - self.left - self.LAYOUT_OFFSET)
-                y = int(event.y - self.bottom)
-                self.caret.on_mouse_press(x, y, event.button, event.modifiers)
-            else:
+        if self._active and isinstance(event, UIMouseEvent):
+            event_in_rect = self.rect.point_in_rect(event.pos)
+
+            # mouse press
+            if isinstance(event, UIMousePressEvent):
+                # inside the input field
+                if event_in_rect:
+                    x = int(event.x - self.left - self.LAYOUT_OFFSET)
+                    y = int(event.y - self.bottom)
+                    self.caret.on_mouse_press(x, y, event.button, event.modifiers)
+                else:
+                    # outside the input field
+                    self.deactivate()
+                    # return unhandled to allow other widgets to activate
+                    return EVENT_UNHANDLED
+
+            # mouse release outside the input field,
+            # which could be a click on another widget, which handles the press event
+            if isinstance(event, UIMouseReleaseEvent) and not event_in_rect:
                 self.deactivate()
                 # return unhandled to allow other widgets to activate
                 return EVENT_UNHANDLED
@@ -683,18 +703,20 @@ def activate(self):
         if self._active:
             return
 
-        self._active = True
+        self._grap_active()  # will set _active to True
         self.trigger_full_render()
         self.caret.on_activate()
         self.caret.position = len(self.doc.text)
 
     def deactivate(self):
         """Programmatically deactivate the text input field."""
 
-        if not self._active:
-            return
+        if self._active:
+            print("Release active text input field")
+            self._release_active()  # will set _active to False
+        else:
+            print("Text input field is not active, cannot deactivate")
 
-        self._active = False
         self.trigger_full_render()
         self.caret.on_deactivate()
 

tests/unit/gui/test_property.py

@@ -1,5 +1,7 @@
 import gc
 
+import pytest
+
 from arcade.gui.property import Property, bind, unbind
 
 
@@ -241,3 +243,15 @@ def callback(*args, **kwargs):
     del callback
 
     assert len(MyObject.name.obs[obj]._listeners) == 1
+
+
+def test_bind_raise_if_attr_not_a_ui_property():
+    class BadObject:
+        @property
+        def name(self):
+            return
+
+    obj = BadObject()
+
+    with pytest.raises(ValueError):
+        bind(obj, "name", lambda *args: None)