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

Hotkeydialog #99

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Hotkeydialog #99

wants to merge 13 commits into from

Conversation

Yida-Lin
Copy link
Contributor

@Yida-Lin Yida-Lin commented Jul 1, 2017

Fix #98

@Yida-Lin
Copy link
Contributor Author

Yida-Lin commented Jul 1, 2017

@cbrnr Ready! Please let me know what do you think! :)

@cbrnr
Copy link
Owner

cbrnr commented Jul 3, 2017

Thanks @Yida-Lin.

  • I would call the dialog "Keyboard and Mouse Shortcuts"
  • There should be a new menu entry in the Help menu
  • The action should always be enabled, not just if a file is open
  • The keyboard shortcuts are currently hard-coded in the dialog - can you extract them directly from the actions?
  • Zoom in/out all channels is missing
  • The help icon in the toolbar should be the rightmost icon

@Yida-Lin Yida-Lin force-pushed the hotkeyDialog branch 2 times, most recently from a0bec12 to 2162516 Compare July 29, 2017 19:36
@Yida-Lin
Copy link
Contributor Author

@cbrnr Hi Clemens,

Ready, except 2 issues:

  • I don't know how to extract mouse wheel event from QKeySequence
  • I don't know how to enable it when not opening a file

@cbrnr
Copy link
Owner

cbrnr commented Jul 31, 2017

Extracting the key sequences doesn't work like that - the dialog still shows Windows shortcuts on a Mac. I think you could use the shortcut and text methods of a QAction to extract the keyboard shortcut and the name of the action.

@Yida-Lin
Copy link
Contributor Author

@cbrnr hmm QActions are scattered everywhere... It would be difficult to include a bunch of headers just for the dialog. :/ Plus GuiActionCommand::getQActions () is abstract... which makes it even more complicated (need to instantiate a bunch of objects to get all actions, I assume).

I currently use:

#if defined(Q_OS_MACOS)
    QString ctrl = "Cmd";
    QString alt = "Option";
#else
    QString ctrl = "Ctrl";
    QString alt = "Alt";
#endif

Could you please let me know whether it works on a mac?

@cbrnr
Copy link
Owner

cbrnr commented Sep 13, 2017

Yes, this works on a Mac. Can you please fix these final issues before I merge:

  • "Scale all channels up" and "Scale all channels down" are missing (Shift+Mousewheel Up/Down)
  • Decrease the dialog width and make sure everything fits into the dialog without having to scroll. Currently, there is a horizontal scrollbar because the table view is too wide.
  • The "Close File" shortcut is missing (Cmd+W on a Mac).
  • "Undo" and "Redo" shortcuts are missing (Cmd+Z, Shift+Cmd+Z).
  • The "Exit" shortcut is Cmd+Q on a Mac and not Cmd+F4.
  • Selection in the dialog should be disabled (no need to highlight anything).
  • The dialog should be always enabled (even when there is no file open).

@Yida-Lin
Copy link
Contributor Author

@cbrnr Hi Clemens! :)

All finished and ready to merge! Right before the Graz BCI Conference :)

@cbrnr
Copy link
Owner

cbrnr commented Sep 18, 2017

Thanks @Yida-Lin. I think we shouldn't make the dialogs fixed size though (see my comment in #104).

@Yida-Lin
Copy link
Contributor Author

@cbrnr Hi Clemens,

Sure. I didn't realize they weren't resizable. Just fixed the problem.

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