fix: support other than 'left' click - #49
Conversation
| if button == 1 and not modifiers: | ||
| self._on_button_press(click_event) | ||
| elif kind := self._pick(click_event): | ||
| self._release_owner(click_event, kind=kind) |
There was a problem hiding this comment.
I find it slightly strange that we release the owner immediately.
Now that you added the functionality for programmatically firing pick events (which was lacking), it opens up the door for being able to simulate things like click, drag and then release, which would vastly improve test coverage.
Maybe we need two methods? A click and a click_and_drag (or a better name) where we provide both start and end positions, and the mouse button is held during the move?
There was a problem hiding this comment.
Makes sense. I changed .click() so that it 1. uses the matplotlib pick machinery and we don't have to implement our own ._pick(). 2. explicitly does one button press and one release at the same position. I also added .click_and_drag() for "press-drag-release" style interactions.
| assert events == [] | ||
|
|
||
|
|
||
| def test_canvas_middle_click_removes_point(): |
There was a problem hiding this comment.
Not sure what this test is adding? Isn't the code that creates the event here basically the same code as in the _make_click_event function?
|
|
||
| assert len(points.children) == 0 | ||
|
|
||
|
|
There was a problem hiding this comment.
Like the Rectangles, can we add a test that checks that the on_remove callback is called when the point is removed?
There was a problem hiding this comment.
This is tested for various types in tests/tool_test.py::test_middle_click_removes_owner_and_calls_callback
| rects.click(x=50, y=60, button=2) | ||
| assert len(ax.patches) == 1 | ||
| rects.start() | ||
| rects.click(x=30, y=60) |
There was a problem hiding this comment.
It would be great if the tests were done not only on the Points and Rectangles, but the other artists as well. Maybe we can parametrize the tests and put them in common files? It could be in the lines_test for Lines and Points, and a new patch_test for the Rectangles, Ellipses, Vspans and Hspans. And polygons_test for polygons.
2 participants
Fixes scipp/plopp#548