Event handler cleanups
- Dominant language
- Python
- Stars
- 5.4k
- Forks
- 827
- Avg merge
- 15h 39m
- Merged PRs (30d)
- 40
Description
While working on the Invent prototype in #3157, I found it necessary to pass each event handler a keyword argument identifying the event type. Although we don't have any need for that feature now, the way in which I did it may be useful for other reasons.
Specifically, I created an `EventProperty` class which allowed all this boilerplate:
```py
@property
def on_press(self) -> OnPressHandler:
"""The handler to invoke when the button is pressed."""
return self._on_press
@on_press.setter
def on_press(self, handler: toga.widgets.button.OnPressHandler) -> None:
self._on_press = wrapped_handler(self, handler)
```
To be simplified to this:
```py
on_press = EventProperty("The handler to invoke when the button is pressed.")
```
The only thing this omits is the handler protocol type. But as discussed in the links below, these didn't actually give the type-checking benefits we’d hoped for, because we couldn't find any clean way of making them accept both a regular and an async callable. What's more, the protocol classes add a huge amount of useless noise to the documentation – see [Slider](https://toga.readthedocs.io/en/latest/reference/api/widgets/slider.html) for a particularly bad example.
So I'd like to suggest the following simplifications:
* Remove all the protocol classes. In the few cases where their docstring contains some information that isn't covered elsewhere, move it to the associated property. Or if the protocol is shared by multiple properties, put the details in a documentation section and link all the properties to it.
* Switch to the `EventProperty` syntax as shown above.
* Implement `EventProperty` such that, for type-checking and documentation purposes, all event properties would have a type called `Handler`. I'm not sure whether `Handler` should be a `Protocol`, `TypeAlias`, `TypeVar`, or something else, but it should:
* Have a documentation section which explains the general rules about event handlers, which we're currently working on in [#3170](https://github.com/beeware/toga/pull/3170).
* If possible, tell type-checkers that the property accepts both a regular and an async callable, with a single positional argument, and any keyword arguments.
Previous discussion of this subject:
* #2192
* #2252
Contributor guide
Research direction
Start by reviewing the existing EventProperty implementation and the event-handler protocol classes, then read the discussion in #3170, #2192, and #2252. Done means the protocol classes are removed, event properties use the simplified syntax, and the resulting Handler type and documentation cover the stated regular and async handler requirements.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- desktop-dev
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100