Skip to content

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

Merged
merged 3 commits into from
Mar 10, 2012
Merged

Conversation

jdh2358
Copy link
Collaborator

@jdh2358 jdh2358 commented Feb 26, 2012

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

@@ -76,6 +76,8 @@ The ``Save`` button
``svg`` and ``pdf``.


.. _key-event-handling:
Copy link
Member

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:

Copy link
Member

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.

@WeatherGod
Copy link
Member

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?

@jdh2358
Copy link
Collaborator Author

jdh2358 commented Feb 29, 2012

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?

@WeatherGod
Copy link
Member

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:
https://github.com/WeatherGod/BRadar/blob/master/lib/BRadar/plotutils.py#L320
(note, this class gets subclassed in other projects of mine that extends the keymap).

Regardless, the primary issue is the conflation of scalar strings with lists.

@efiring
Copy link
Member

efiring commented Mar 10, 2012

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.

@jdh2358
Copy link
Collaborator Author

jdh2358 commented Mar 10, 2012

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.

jdh2358 added a commit that referenced this pull request Mar 10, 2012
@jdh2358 jdh2358 merged commit 43efa34 into matplotlib:master Mar 10, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants