beeware / beeware/toga

Event handler cleanups

Open
#3,185 1 comment 0 reactions 0 assignees View on GitHub
enhancement
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.