-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
expose key press handler #717
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
…cess it; illustrate in gtk, tk and qt examples
@@ -76,6 +76,8 @@ The ``Save`` button | |||
``svg`` and ``pdf``. | |||
|
|||
|
|||
.. _key-event-handling: |
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.
Personally, I think I would rather see these tags done like so:
.. _gallery_tags::
:key-event-handling:
:foo-tag:
:spam-tag:
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.
nm, I had a brain fart and didn't realize it was a reference tag.
Would this be a good opportunity to force keymap values to be lists (currently they are both lists or scalar string)? Are there plans to extend this a bit further to allow for users to super-impose their own keymaps? |
Is there a problem with the current validate_stringlist approach for the rc keymap? I don't feel compelled to force it to be a list if it is not broken in the current form. On the question of allowing users to impose their own keymaps, I am not sure I know exactly what you mean. The can already do this by connecting to the key_press_events through mpl_connect. Can you elaborate on what you mean by super-imposing their own keymaps? An rc like registry of keys -> functions that would automatically get loaded by mpl? |
In issue #497, I outline the issues that can occur when there is confusion between a list and a scalar string. It is technically broken in the current form. As for your second point, I am talking about dealing with keymap collisions. Let's say I make an app that does something when the 'z' key is pressed and I didn't know that mpl already has this mapped to activating the zoom tool, that is a little irritating. Obviously fixable once discovered. But perhaps it wouldn't be that difficult for functionality that would allow users to define their own keymaps (and mpl would have its default keymap) and then allow them to be mixed together. Perhaps through the dictionary idiom of dict.update()? Currently, I have to do something like this: Regardless, the primary issue is the conflation of scalar strings with lists. |
It looks like the comments have gone off on a tangent. The pull request doesn't make anything worse with respect the the list/string problem, does it? If not, and if it does what it is supposed to do, and if that is worth doing, then I suggest that this be merged before it gets stale. The list/string problem can be dealt with separately. |
I agree the list/string issue, and the customization of keymaps should be handled outside this pull request as all I am doing is exposing the defaults to the mpl embedder. Nonetheless, this patch does make it easier for mpl embedders who subclass FigureCanvasBase to intercept the keys, handle what they want, and pass the rest on to the default key_press_handler. |
The mpl key press handler is tied into the implementation of FigureManagerBase, and so is not accessible to most GUI implementations which decouple the FigureCanvas and toolbar from the manager. In this patch, I factored out the key press handling code into backend_bases.key_press_handler and update the tk, qt and gtk examples to show how to use it