-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Allow Selectors to be dragged from anywhere within their patch #19657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
78160ab
to
e8905ad
Compare
|
e8905ad
to
bbf32b8
Compare
832d55e
to
0c35d09
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you may have rebased and reverted the changes I suggested.
@@ -44,6 +44,36 @@ def test_rectangle_selector(): | |||
check_rectangle(rectprops=dict(fill=True)) | |||
|
|||
|
|||
@pytest.mark.parametrize('drag_from_anywhere, new_center', | |||
[[True, (60, 75)], | |||
[False, (30, 20)]]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you're not dragging from the center, and you didn't enable the new option, why is the center now (30, 20)
instead of remaining at (50, 65)
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because dragging with drag_from_anywhere=False
creates a new rectangle, which has a center (30, 20). I'll add some more comments to the test to make it clearer what's going on.
0c35d09
to
17c6f8c
Compare
Sorry about the review comments, they should all be implemented now. I've added some more comments to the test too - let me know if anything remains unclear. |
PR Summary
As it says in the what's new entry:
The
~matplotlib.widgets.RectangleSelector
and~matplotlib.widgets.EllipseSelector
have a new keyword argument,select_whole_region
, which when set toTrue
allows you to click and drag from anywhere inside the selector to move it. Previously it was only possible to move it by either activating the move modifier button, or clicking on the central handle.Questions:
select_whole_region
is the best I came up with, but it doesn't feel great.Here's an example:
Screen.Recording.2021-03-06.at.16.53.58.mov
PR Checklist
pytest
passes).flake8
on changed files to check).flake8-docstrings
and runflake8 --docstring-convention=all
).doc/users/next_whats_new/
(follow instructions in README.rst there).doc/api/next_api_changes/
(follow instructions in README.rst there).