Keyboard shortcuts work when toolbar not displayed #1830

Closed
wants to merge 1 commit into from

3 participants

@warrd

Following from issue #1829, I've implemented a fix for keyboard shortcuts when the toolbar is not displayed.

In the existing code, state/logic of pan/zoom tools are coupled to the existence of a toolbar, so I went for the easiest fix possible: keeping the toolbar but setting visibility to false when None is chosen in matplotlibrc.

So far it's only implemented in the Qt and GTK backends: I don't have the others so no way to test.

@efiring
Matplotlib Developers member

I haven't tried it out, but offhand, I don't like the approach taken. If I specify no toolbar, I really mean no toolbar, not just "don't draw it".

What might be needed is a refactoring to separate the actual functionality of zoom, pan, keypress-handling, etc. from the particular Toolbar user interface.

@warrd

Yes, fair enough, I agree it's slightly hacky. I originally thought about a major refactor but it's my first time with the codebase, so wanted to make changes to application logic as minor as possible.

The refactor would have to involve moving a lot of the behaviour in NavigationToolbar2 to the main FigureCanvasBase, or more likely a new class that keeps track of cursor state etc.
What do you think?

@efiring
Matplotlib Developers member

Yes, it looks to me like most of the necessary functionality is already in NavigationToolbarBase, so that could be renamed (Navigation? Interaction? Control?) with the relatively small toolbar-specific parts moved into a ToolbarBase.

Or maybe there are really three components: plot manipulation state and actions, which are backend-independent; keypress-handling, which is backend-dependent; and the specific Toolbar interface, which is very backend dependent.

@dmcdougall
Matplotlib Developers member

What might be needed is a refactoring to separate the actual functionality of zoom, pan, keypress-handling, etc. from the particular Toolbar user interface.

This gels well with the Model-View-Controller mentality in Apple's frameworks.

@warrd

I'm closing this pull request and putting in a new one with a more significant refactor

@warrd warrd closed this Mar 23, 2013
@warrd

See pull request #1849

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment