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

Quit autotype dialog with esc key #1412

Merged
merged 2 commits into from
Jan 22, 2018
Merged

Conversation

TheZ3ro
Copy link
Contributor

@TheZ3ro TheZ3ro commented Jan 22, 2018

Description

Quit autotype dialog with esc key

Motivation and context

Fixes #907

Types of changes

  • ✅ New feature (non-breaking change which adds functionality)

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]
  • ✅ I have compiled and verified my code with -DWITH_ASAN=ON. [REQUIRED]

@TheZ3ro TheZ3ro added this to the v2.3.0 milestone Jan 22, 2018
@TheZ3ro TheZ3ro requested a review from a team January 22, 2018 14:47
qDebug() << e->key();

if (!e->modifiers() || (e->modifiers() & Qt::KeypadModifier && e->key() == Qt::Key_Enter)) {
if (e->key() == Qt::Key_Escape) {
Copy link
Member

@phoerious phoerious Jan 22, 2018

Choose a reason for hiding this comment

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

This if structure makes no sense to me. This should be

if (!e->modifiers() && e->key() == Qt::Key_Escape) {
    emit rejected();
} else {
    e->ignore();
}

or

if (!e->modifiers() && e->key() == Qt::Key_Escape) {
    emit rejected();
    return;
}
e->ignore();

which are both logically equivalent to your complicated-looking construct if I didn't miss anything. Also, you should remove the qDebug() call.

Copy link
Member

Choose a reason for hiding this comment

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

BTW another thing you might add to this branch, while you're at it: enable cyclic arrow key navigation. So if you use arrow up on the top entry, you come out at the bottom and vice versa. A friend of mine suggested this to me earlier today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied this code from the PasswordGeneratorWidget but actually the first if seems useful in both cases.
I will fix them

Copy link
Member

@phoerious phoerious Jan 22, 2018

Choose a reason for hiding this comment

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

What seems useful? The check for KeyadModifier and Enter? It does absolutely nothing.

Copy link
Contributor Author

@TheZ3ro TheZ3ro Jan 22, 2018

Choose a reason for hiding this comment

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

lol, I was meaning useless 🤦‍♂️

@TheZ3ro TheZ3ro force-pushed the feature/exit-autotype-dialog branch from 5614583 to 74fc601 Compare January 22, 2018 20:53
@@ -121,6 +121,20 @@ void EntryView::keyPressEvent(QKeyEvent* event)
#endif
}

int last = m_model->rowCount()-1;
Copy link
Member

Choose a reason for hiding this comment

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

Spaces please. ;-)

Same for if below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with a rebase

@TheZ3ro TheZ3ro force-pushed the feature/exit-autotype-dialog branch from 74fc601 to ae0a06b Compare January 22, 2018 23:11
@TheZ3ro TheZ3ro force-pushed the feature/exit-autotype-dialog branch from ae0a06b to 745d255 Compare January 22, 2018 23:12
@phoerious phoerious merged commit 28ebdb7 into develop Jan 22, 2018
@phoerious phoerious deleted the feature/exit-autotype-dialog branch January 22, 2018 23:21
phoerious added a commit that referenced this pull request Feb 27, 2018
- Add support for KDBX 4.0, Argon2 and ChaCha20 [#148, #1179, #1230, #1494]
- Add SSH Agent feature [#1098, #1450, #1463]
- Add preview panel with details of the selected entry [#879, #1338]
- Add more and configurable columns to entry table and allow copying of values by double click [#1305]
- Add KeePassXC-Browser API as a replacement for KeePassHTTP [#608]
- Deprecate KeePassHTTP [#1392]
- Add support for Steam one-time passwords [#1206]
- Add support for multiple Auto-Type sequences for a single entry [#1390]
- Adjust YubiKey HMAC-SHA1 challenge-response key generation for KDBX 4.0 [#1060]
- Replace qHttp with cURL for website icon downloads [#1460]
- Remove lock file [#1231]
- Add option to create backup file before saving [#1385]
- Ask to save a generated password before closing the entry password generator [#1499]
- Resolve placeholders recursively [#1078]
- Add Auto-Type button to the toolbar [#1056]
- Improve window focus handling for Auto-Type dialogs [#1204, #1490]
- Auto-Type dialog and password generator can now be exited with ESC [#1252, #1412]
- Add optional dark tray icon [#1154]
- Add new "Unsafe saving" option to work around saving problems with file sync services [#1385]
- Add IBus support to AppImage and additional image formats to Windows builds [#1534, #1537]
- Add diceware password generator to CLI [#1406]
- Add --key-file option to CLI [#816, #824]
- Add DBus interface for opening and closing KeePassXC databases [#283]
- Add KDBX compression options to database settings [#1419]
- Discourage use of old fixed-length key files in favor of arbitrary files [#1326, #1327]
- Correct reference resolution in entry fields [#1486]
- Fix window state and recent databases not being remembered on exit [#1453]
- Correct history item generation when configuring TOTP for an entry [#1446]
- Correct multiple TOTP bugs [#1414]
- Automatic saving after every change is now a default [#279]
- Allow creation of new entries during search [#1398]
- Correct menu issues on macOS [#1335]
- Allow compilation on OpenBSD [#1328]
- Improve entry attachments view [#1139, #1298]
- Fix auto lock for Gnome and Xfce [#910, #1249]
- Don't remember key files in file dialogs when the setting is disabled [#1188]
- Improve database merging and conflict resolution [#807, #1165]
- Fix macOS pasteboard issues [#1202]
- Improve startup times on some platforms [#1205]
- Hide the notes field by default [#1124]
- Toggle main window by clicking tray icon with the middle mouse button [#992]
- Fix custom icons not copied over when databases are merged [#1008]
- Allow use of DEL key to delete entries [#914]
- Correct intermittent crash due to stale history items [#1527]
- Sanitize newline characters in title, username and URL fields [#1502]
- Reopen previously opened databases in correct order [#774]
- Use system's zxcvbn library if available [#701]
- Implement various i18n improvements [#690, #875, #1436]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

#improvement Make ESC work as "cancel" in the auto-type selection window
2 participants