Enable keyboard on_activate in Table and Tree
#3781
Conversation
|
Winforms failure is unrelated. c.f. the latest run on #3769 |
|
I've restarted CI; if it passes and you think the PR is ready for review, please post a comment. |
|
I think this PR is ready for review. |
mhsmith
changed the title
activateon_activate in Table and Tree
|
I'm currently preparing a demo for Wednesday, so I may not get a chance to look at this until Thursday. |
There was a problem hiding this comment.
We recently switched the documentation from RST to Markdown, so please rename this file to .md.
| on_select_handler.assert_called_once_with(widget) | ||
| on_select_handler.reset_mock() | ||
| await probe.type_character("a") | ||
| await probe.redraw("Letter pressed - forth row selected") |
There was a problem hiding this comment.
| await probe.redraw("Letter pressed - forth row selected") | |
| await probe.redraw("Letter pressed - fourth row selected") |
| # Activate row | ||
| assert widget.selection == source[3] | ||
| await probe.type_character("\n") | ||
| await probe.redraw("Return key pressed - forth row activated") |
There was a problem hiding this comment.
| await probe.redraw("Return key pressed - forth row activated") | |
| await probe.redraw("Return key pressed - fourth row activated") |
|
|
||
| async def test_activate_keyboard( | ||
| widget, | ||
| probe, |
There was a problem hiding this comment.
This partly duplicates test_keyboard_navigation, so I think it would be better to merge them all into a single test, which covers activation of at least two different items.
There was a problem hiding this comment.
My comments on test_table also apply to test_tree.
Also, there are several places in this file where "table" and "row" should be replaced by "tree" and "node".
There was a problem hiding this comment.
Duplicate code between Tree and Table should be merged.
There was a problem hiding this comment.
The Tree and Table implementations are duplicated, so please merge them into one. Don't worry about the type annotations, because we only use them for documentation of the public API.
There was a problem hiding this comment.
Where should this code shared between table and tree live?
Is it OK to add an from toga.widgets.table import ... in tree.py?
There was a problem hiding this comment.
Yes, if there isn't any more obvious place, then it's fine to put it in one file and import it from the other.
3 participants
Pressing the Return key on a table or tree should be semantically identical to double clicking. This PR aims to do that, by invoking
on_activate, just like double clicking does.On GTK, this is the default behaviour, so nothing to do there.
There's a small snag when
multiple_selectis enabled.Currently,
on_activatehas an argument: the row/node that was double clicked.When pressing Return when multiple rows are selected, there isn't a single row that should be activated. I chose to not invoke the event in this case.
I placed the code that helps handling this on the core widgets: the
_selection_singlemethod. This way it can be shared across implementations.PR Checklist: