Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Separately track modifier keys for mouse events.#23473
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
d625e11 to38c9039CompareI haven't had a chance to look at the details here, but I just pulled and tested quick on macosx and hit some errors that you can probably fix pretty quick: Clicking anywhere: TypeError:MouseEvent.__init__() got an unexpected keyword argument'num' ctrl + clicking anywhere: SystemError: NULL object passed to Py_BuildValue |
Oops, thanks for trying. Hopefully fixed now? |
Looks like the failures with macosx are real and a |
anntzer commentedDec 13, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Oops, probably a careless rebase, thanks for the ping. |
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.
tk seems to be double registering the modifiers in key_press now. (I seeshift+shift and maybe more illuminating isctrl+control rather thancontrol+control although I only expect a singlecontrol).
The modifiers appear with the mouse presses as expected in macosx, qt, tk. However, I think it'd be nice to also add these to the keypress events because currently it is only an empty frozenset on those events.
Test script I was using:
importmatplotlib.pyplotaspltfig,ax=plt.subplots(1,1)fig.canvas.mpl_connect('key_press_event',lambdakey_event:print(f'press{key_event.key!r}'))fig.canvas.mpl_connect('key_release_event',lambdakey_event:print(f'release{key_event.key!r}'))fig.canvas.mpl_connect('key_press_event',lambdakey_event:print(f'modifiers press{key_event.modifiers!r}'))fig.canvas.mpl_connect('button_press_event',lambdakey_event:print(f'modifiers press{key_event.modifiers!r}'))plt.show()
Ah, I had forgotten to implement the exclude kwarg to _mpl_modifiers for some backends, thanks for catching that, the doubling of modifiers should now be fixed. |
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.
Fine to save some work for follow-ups and break this up. This is a good start!
Works as expected for me now, but there are some differences between backends which is unrelated to this PR, but it might be nice to fix up the inconsistencies later. It is likely related to mac's keyboards.
macosx -> good
qt5 -> good
tk -> Pressing command, I get a KeyPress "super+meta" (KeyRelease is only "meta" which is odd), but only "super" in the modifiers. I would expect "cmd" in the modifiers.
gtk3 -> Pressing command, I get a KeyPress "meta" but nothing in the modifiers when I expect a "cmd" in the modifiers
I suspect it's just a matter of updating (if needed) the table in _mpl_modifiers (previously _get_key) (perhaps by manually inspecting event.state in various cases); as you have access to a mac, can you give it a try (perhaps both with a laptop keyboard and with an independent one just to be sure)? |
Uh oh!
There was an error while loading.Please reload this page.
I tested qt + tk on linux and osx on osx. Also sawsomething funny going on with 'meta' on tk. I saw it in the out put from the snippet Greg posted, but I could not sort out which keys I hit to get it. |
Anyone can merge on green (again, excluding the patch coverage). |
Whether the event modifiers are directly available on enter/leave eventsdepends on the backend, but all are handled here (except possibly formacos, which I haven't checked).
Thank you@anntzer ! |
@greglucas I got my hands on a mac :) and had a look at#23473 (review). where the last tuple item must be "meta" (to match tk reporting the cmd key with event.keysym = "Meta_L" (or "Meta_R")) and the first entry is how you think the modifier should actually be called (I guess "cmd", or perhaps "meta", I don't really have a good intuition here)? Can you decide on that last point and pick up the patch? |
Uh oh!
There was an error while loading.Please reload this page.
PR Summary
Ensure that modifier keys are correctly reported for mouse events even if the last key press occurred out of the canvas.
Whether the event modifiers are directly available on enter/leave events
depends on the backend, but all are handled here (except possibly for
macos, which I haven't checked).
There's a first separate commit for the macosx backend for some preliminary work with ObjC.
Followup to#16931 (it's the reason for that big PR...). Supersedes#6159. See also the side note at#23436 (comment).
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).