Check pressed mouse buttons in pan/zoom drag handlers.#29066
Merged
QuLogic merged 1 commit intomatplotlib:mainfrom Nov 15, 2024
Merged
Check pressed mouse buttons in pan/zoom drag handlers.#29066QuLogic merged 1 commit intomatplotlib:mainfrom
QuLogic merged 1 commit intomatplotlib:mainfrom
Conversation
timhoffm
reviewed
Nov 4, 2024
|
|
||
| def drag_pan(self, event): | ||
| """Callback for dragging in pan/zoom mode.""" | ||
| if self._pan_info.button not in event.buttons: |
Member
There was a problem hiding this comment.
Shouldn't this be an exact match? I believe the action should terminate as soon as any button is pressed or released.
Contributor
Author
There was a problem hiding this comment.
Sure, I don't really mind either way.
timhoffm
approved these changes
Nov 4, 2024
QuLogic
approved these changes
Nov 9, 2024
QuLogic
requested changes
Nov 9, 2024
Member
QuLogic
left a comment
There was a problem hiding this comment.
Actually, the test failures appear relevant.
Contributor
Author
|
Indeed, should be fixed now (synthetic events in tests needed some adjustment). |
Sometimes, the mouse_release_event ending a pan/zoom can be lost, if it
occurs while the canvas does not have focus (a typical case is when a
context menu is implemented on top of the canvas, see example below);
this can result in rather confusing behavior as the pan/zoom continues
which no mouse button is pressed. To fix this, always check that the
correct button is still pressed in the motion_notify_event handlers.
To test, use e.g.
```
from matplotlib import pyplot as plt
from matplotlib.backends.qt_compat import QtWidgets
def on_button_press(event):
if event.button != 3: # Right-click.
return
menu = QtWidgets.QMenu()
menu.addAction("Some menu action", lambda: None)
menu.exec(event.guiEvent.globalPosition().toPoint())
fig = plt.figure()
fig.canvas.mpl_connect("button_press_event", on_button_press)
fig.add_subplot()
plt.show()
```
enter pan/zoom mode, right-click to open the context menu, exit the
menu, and continue moving the mouse.
QuLogic
approved these changes
Nov 15, 2024
This was referenced Nov 15, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Sometimes, the mouse_release_event ending a pan/zoom can be lost, if it occurs while the canvas does not have focus (a typical case is when a context menu is implemented on top of the canvas, see example below); this can result in rather confusing behavior as the pan/zoom continues which no mouse button is pressed. To fix this, always check that the correct button is still pressed in the motion_notify_event handlers.
To test, use e.g.
enter pan/zoom mode, right-click to open the context menu, exit the menu, and continue moving the mouse.
Followup to #28453 (and the original motivation for it).
PR summary
PR checklist