Skip to content

Conversation

@pushfoo
Copy link
Member

@pushfoo pushfoo commented Jul 29, 2024

  • Add relative position helpers
  • Use position helpers to clean up UITextInput.on_event
  • Some light refactoring for readability
  • Improve related docstrings and comments

pushfoo added 3 commits July 28, 2024 19:29
* Add UIWidget._event_pos_relative_to_self

* Add UITextInput._event_pos_relative_to_self

* Use new method to clean up UITextInput.on_event
* Use self._event_pos_relative_to_self

* Restructure it to put the type check

* Add comments explaining when / why things are handled
* Use inside_xy in the lower block as well

* Improve UITextInput.on_event docstring + comments
@pushfoo pushfoo requested a review from eruvanos July 29, 2024 00:22
@pushfoo
Copy link
Member Author

pushfoo commented Jul 29, 2024

@DigiDuncan Let me know if the relative position helper seems to belong in Rect or if I somehow missed one like it already existing.

@pushfoo pushfoo added this to the Future milestone Sep 30, 2024
Comment on lines +181 to +194
def _event_pos_relative_to_self(self, mouse_event: UIMouseEvent) -> Vec2 | None:
"""Gets coords relative to bottom left if inside the widget.
Args:
mouse_event: Any :py:class:`UIMouseEvent`.
Returns:
``None`` if outside the widget or coords relative to
the widget's bottom left.
"""
pos = mouse_event.pos
if not self.rect.point_in_rect(pos):
return None
return Vec2(pos[0] - self.left, pos[1] - self.bottom)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to use the new Rect method.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't update it yet because I want to give it more thought after rereading the UI code and considering the following:

  1. 3.9 features will be our new minimum
  2. The new rect method seems to perform more ops
  3. It may add even more call overhead

@pushfoo pushfoo marked this pull request as draft October 1, 2024 00:46
@eruvanos
Copy link
Member

Hi, I am unsure about the state of the changes, due to the conflicts and the lag of an open issue, I would maybe close this PR?
@pushfoo let me know, if you thing the UITextInput requires the refactoring, from my point of view, right now it is kind of explicit. But I am always open for a second opinion.

@pushfoo
Copy link
Member Author

pushfoo commented Feb 15, 2025

I would close this PR
TL;DR: Agreed.

This thing was an exploratory kludge. I want to review your recent UI event proposals for controllers before attempting this + thoroughly review Rect since:

  1. I need to in general
  2. Dragon's "use rect" suggestion seemed to consume more operations at first glance
  3. The fact I said "seemed" means we need to bench it

@pushfoo pushfoo closed this Feb 15, 2025
@einarf einarf deleted the add-relative-widget-position-helper branch June 7, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants