-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
let Qt4 backend ignore extra mouse buttons (fixes exceptions) #890
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
Were you intending to support the extra mouse buttons, or simply avoid the backend blowing up? |
buttond = {QtCore.Qt.LeftButton : 1, | ||
QtCore.Qt.MidButton : 2, | ||
QtCore.Qt.RightButton : 3, | ||
# QtCore.Qt.XButton1 : None, |
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 don't see why we can't provide magic numbers (or maybe even strings) for these keys.
In my eyes, since you have used the {}.get()
method further on, this commented out section should either be removed or the mouse buttons supported.
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.
Yeah -- we probably should use strings, but that would require changing all backends to be consistent and would require any third-party code using the event system to update. Should be on a list of good "backward incompatible" changes to make some day.
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.
As far as I can see, strings would just work for the new values whilst keeping the integers for the old would avoid breaking any backwards compatibility?
None the less, this doesn't need to be done here for this pull request to be useful. In its current form this pull should be considered for merging into the 1.1.x branch.
First and foremost, I wanted to avoid the exceptions. (The first commit, introducing the .get, indeed fixed this already.) Then, I introduced the constants for readability, and figured that I should also add the two missing constants in case someone wanted to pick this up later and introduce support for the extra buttons. Leaving the commented lines in would make life easier then (in particular for someone not familiar with Qt). I could have left away the comment characters (the effect would be exactly the same), but this way it looks like unfinished code (which it should, because it is). I am no expert in mouse buttons, so I don’t know which new constants would make sense (e.g. 4/5 vs. "x1"/"x2"). Of course one could as well define them now and at least the Qt backend would support them. |
+1 |
I agree. Could you close this PR and reopen it rebased against the v1.1.x branch? |
Never did that before, but I'll do my very best. ;-) |
I need to rebase my changed (probably locally) onto upstream/v1.1.x first, right? |
Yes. And then create a new PR and edit the base revision so it points to matplotlib:v1.1.x |
My mouse has more than three buttons; the current qt4 backend does not cope with mouse events on the canvas for these buttons.