-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Allow Enter key to select recent DB on OS X #2331
Allow Enter key to select recent DB on OS X #2331
Conversation
The build failed since |
This would be better handled if you just gave the selection box the focus when the WelcomeWidget is activated. Plus, don't limit this to just OSX, we only do platform specific code if it is actually platform specific. |
What do you mean giving the selection box focus? I can select things in the list with arrow keys, but Enter does not open the DB. I have not tested this on other OSes, but the Qt docs claim that Enter will activate things on Windows/X11. I believe this is an OS X specific problem. |
Looks like the Windows build passed. Code change should have been identical for Windows and Linux. Guessing the Windows build isn't as strict regarding warnings. |
I was considering getting rid of the |
Just realized this broke tabbing over to a button (e.g. Create new database) and pressing Enter. Overriding |
Please don't spend time on this until I dive into the documentation. |
Windows does release builds only, which don't have -Werror set. |
@gshakhn you need to call down into the baseclass keyPressEvent so everything else works. Do this outside of the Q_MAC_OS guards. |
* Override `keyPressEvent`on WelcomeWidget on OS X to `openDatabaseFromFile`. openDatabaseFromFile is already invoked via the QListWidget::itemActivated signal, but this signal doesn't fire on OS X for Enter. QListWidget::itemActivated activates on an OS specific activation key. [1] On Windows/X11, this is Enter, which lets the user easily navigate with just the keyboard. On OS X, this is Ctrl+O, which is already bound to Open Database. This means that itemActivated cannot fire via the keyboard. Per StackOverflow [2], the recommended solution is to catch the enter/return key press manually. This seems like a common problem with Qt. [3] [4] [1] https://doc.qt.io/archives/qt-4.8/qlistwidget.html#itemActivated [2] https://stackoverflow.com/questions/31650780/when-does-a-qtreeview-emit-the-activated-signal-on-mac [3] https://forum.qt.io/topic/36147/pyside-itemactivated-not-triggered-on-mac-os-x-with-return-key [4] dolphin-emu/dolphin#6099
f0716d3
to
129c2cd
Compare
- New Database Wizard [#1952] - Advanced Search [#1797] - Automatic update checker [#2648] - KeeShare database synchronization [#2109, #1992, #2738, #2742, #2746, #2739] - Improve favicon fetching; transition to Duck-Duck-Go [#2795, #2011, #2439] - Remove KeePassHttp support [#1752] - CLI: output info to stderr for easier scripting [#2558] - CLI: Add --quiet option [#2507] - CLI: Add create command [#2540] - CLI: Add recursive listing of entries [#2345] - CLI: Fix stdin/stdout encoding on Windows [#2425] - SSH Agent: Support OpenSSH for Windows [#1994] - macOS: TouchID Quick Unlock [#1851] - macOS: Multiple improvements; include CLI in DMG [#2165, #2331, #2583] - Linux: Prevent Klipper from storing secrets in clipboard [#1969] - Linux: Use polling based file watching for NFS [#2171] - Linux: Enable use of browser plugin in Snap build [#2802] - TOTP QR Code Generator [#1167] - High-DPI Scaling for 4k screens [#2404] - Make keyboard shortcuts more consistent [#2431] - Warn user if deleting referenced entries [#1744] - Allow toolbar to be hidden and repositioned [#1819, #2357] - Increase max allowed database timeout to 12 hours [#2173] - Password generator uses existing password length by default [#2318] - Improve alert message box button labels [#2376] - Show message when a database merge makes no changes [#2551] - Browser Integration Enhancements [#1497, #2253, #1904, #2232, #1850, #2218, #2391, #2396, #2542, #2622, #2637, #2790] - Overall Code Improvements [#2316, #2284, #2351, #2402, #2410, #2419, #2422, #2443, #2491, #2506, #2610, #2667, #2709, #2731]
Description
Override
keyPressEvent
on WelcomeWidget on OS X toopenDatabaseFromFile
.Motivation and context
I work with a few different password DBs. I like to use the keyboard heavily in KeePassXC. I can close an existing DB with Ctrl+W, but I have to use the mouse to open a recent one. Hitting Enter does not work on OS X. Details are below.
My C++ is extremely rusty, so any feedback would be appreciated.
openDatabaseFromFile
is already invoked via the QListWidget::itemActivated signal, but this signal doesn't fire on OS X for Enter. QListWidget::itemActivated activates on an OS specific activation key. [1] On Windows/X11, this is Enter, which lets the user easily navigate with just the keyboard. On OS X, this is Ctrl+O, which is already bound to Open Database. This means thatitemActivated
cannot fire via the keyboard.Per StackOverflow [2], a recommended solution is to catch the enter/return key press manually.
This seems like a common problem with Qt. [3] [4] I'm guessing other
QListWidget
s also exhibit this problem, but I don't use them enough for it to be a pain point. This PR fixes this particular widget, but there is likely a more holistic approach for the other widgets.[1] https://doc.qt.io/archives/qt-4.8/qlistwidget.html#itemActivated
[2] https://stackoverflow.com/questions/31650780/when-does-a-qtreeview-emit-the-activated-signal-on-mac
[3] https://forum.qt.io/topic/36147/pyside-itemactivated-not-triggered-on-mac-os-x-with-return-key
[4] dolphin-emu/dolphin#6099
How has this been tested?
Manually tested on OS X 10.11.6 with Qt 5.11.2. I have not tested on a Windows/X11 environment to verify that the
#ifdef
is working.Screenshots (if appropriate):
Types of changes
Checklist:
make format
. Code change is small and looks like the other code as far as I can tell though)-DWITH_ASAN=ON
. [REQUIRED]