Skip to content
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

Add keyboard shortcuts actions to VisualizationFrame directly #1117

Merged

Conversation

Kukanani
Copy link

This fixes a known issue (#1116) where keyboard shortcuts to trigger menu actions do not work.
The action has to be added to the parent frame as well as the menu itself. The Fullscreen action (F11) was already doing that correctly.

I also added a (pretty standard) keyboard shortcut for Save As.

QAction * file_menu_save_action = file_menu_->addAction( "&Save Config", this, SLOT( onSave() ), QKeySequence( "Ctrl+S" ));
this->addAction(file_menu_save_action);
QAction * file_menu_save_as_action = file_menu_->addAction( "Save Config &As", this, SLOT( onSaveAs() ), QKeySequence( "Ctrl+Shift+S"));
this->addAction(file_menu_save_as_action);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's interesting that this fixes it for you. Can you elaborate on why this is necessary? The Qt examples don't do that, but then again it could be related to the relationship between the UI elements in this particular scenario.

http://doc.qt.io/qt-5/qtwidgets-mainwindows-menus-example.html (I was looking at the section void MainWindow::createMenus())

Maybe the createMenus() function is being overloaded and all we need to do is to change the name of this function.

I'm not certain about any of those ideas, I'm just throwing things out there incase you don't have a good explanation for why this fixes the problem and is the "correct" way to do it.

@wjwwood
Copy link
Member

wjwwood commented Jun 28, 2017

Also, maybe we should switch to using the standard key sequences provided by Qt:

http://doc.qt.io/qt-5/qkeysequence.html#StandardKey-enum

E.g. QKeySequence::SaveAs.

@Kukanani
Copy link
Author

My guess is that the RenderPanel has focus by default, and as long is it does, all keyboard events are routed there (which is required for tool selection etc.) instead of to the menu object. By adding the event to the parent window, I assume this allows the event to be captured even when the RenderPanel has focus.

Without this fix, if you switch focus to the menu (for example, enter and exit fullscreen with F11), then try Ctrl+S, it works. It's pretty odd behavior, but this hand-wavey explanation is the best hypothesis I have.

@wjwwood
Copy link
Member

wjwwood commented Jun 28, 2017

Even with your explanation. It isn't clear to me why it's necessary. You'd think that the Qt example I linked would work even if the menu doesn't have focus (though I'm not entirely clear on that mechanism either). Perhaps there is an issue with the passing of keypress events up from the render window which causes the visualization frame itself to be handled, but not it's children (like the menu). Since this is an incremental improvement, I'll consider it for merge, but I'll keep an eye out for a better solution.

@wjwwood wjwwood merged commit f23ae4f into ros-visualization:kinetic-devel Jun 28, 2017
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.

2 participants