Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add "Apply" button to entry and group edit windows #624

Merged
merged 2 commits into from
Jun 14, 2017

Conversation

droidmonkey
Copy link
Member

Description

Add's an "Apply" buttons to both the entry and group edit windows. Implements #509.

Motivation and context

Often times you want to save your changes without exiting the edit windows.

How has this been tested?

Manually and with updated GUI tests.

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]
  • ✅ I have added tests to cover my changes.

@droidmonkey droidmonkey added this to the v2.2.0 milestone Jun 7, 2017
@droidmonkey droidmonkey requested review from phoerious and louib June 7, 2017 02:39
@@ -102,7 +103,10 @@ void EditWidget::setReadOnly(bool readOnly)
m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Close);
}
else {
m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel | QDialogButtonBox::Apply);
Copy link
Member

Choose a reason for hiding this comment

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

@droidmonkey maybe nitpicking, but I'd place the Apply before the Cancel.

Copy link
Contributor

Choose a reason for hiding this comment

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

@louib It seems Qt ignores the order of the values and sets it according to the platform defaults:
https://doc.qt.io/qt-5/qmessagebox.html#the-property-based-api

m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel);
m_ui->buttonBox->setStandardButtons(QDialogButtonBox::Ok | QDialogButtonBox::Cancel | QDialogButtonBox::Apply);
// Find and connect the apply button
QPushButton *applyButton = m_ui->buttonBox->button(QDialogButtonBox::Apply);
Copy link
Member

Choose a reason for hiding this comment

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

@droidmonkey QPushButton* applyButton

@@ -66,7 +66,7 @@
<item>
<widget class="QDialogButtonBox" name="buttonBox">
<property name="standardButtons">
<set>QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
<set>QDialogButtonBox::Apply|QDialogButtonBox::Cancel|QDialogButtonBox::Ok</set>
Copy link
Member

Choose a reason for hiding this comment

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

@droidmonkey we could also preserve the same order for the Ok Apply and Cancel buttons here.

@louib
Copy link
Member

louib commented Jun 12, 2017

@droidmonkey the new button works great! However, it appears the button stays pushed until a click is made somewhere in the window. Would it be possible to never leave it pushed?

screenshot from 2017-06-11 21-30-58

Notice also that the Ok button is always pushed. I tested and it's the same thing on the current develop branch. Do you recall if there was a reason for that?
screenshot from 2017-06-11 21-30-41

@droidmonkey
Copy link
Member Author

@louib the QDialogButtonBox handles all the particulars when it comes to button behavior and ordering. I am not aware of anything special we are doing that would cause the always pushed look to occur.

@droidmonkey droidmonkey merged commit 6ffca84 into develop Jun 14, 2017
@droidmonkey droidmonkey deleted the feature/apply-button branch June 14, 2017 00:55
droidmonkey added a commit that referenced this pull request Jun 25, 2017
- Added YubiKey 2FA integration for unlocking databases [#127]
- Added TOTP support [#519]
- Added CSV import tool [#146, #490]
- Added KeePassXC CLI tool [#254]
- Added diceware password generator [#373]
- Added support for entry references [#370, #378]
- Added support for Twofish encryption [#167]
- Enabled DEP and ASLR for in-memory protection [#371]
- Enabled single instance mode [#510]
- Enabled portable mode [#645]
- Enabled database lock on screensaver and session lock [#545]
- Redesigned welcome screen with common features and recent databases [#292]
- Multiple updates to search behavior [#168, #213, #374, #471, #603, #654]
- Added auto-type fields {CLEARFIELD}, {SPACE}, {{}, {}} [#267, #427, #480]
- Fixed auto-type errors on Linux [#550]
- Prompt user prior to executing a cmd:// URL [#235]
- Entry attributes can be protected (hidden) [#220]
- Added extended ascii to password generator [#538]
- Added new database icon to toolbar [#289]
- Added context menu entry to empty recycle bin in databases [#520]
- Added "apply" button to entry and group edit windows [#624]
- Added macOS tray icon and enabled minimize on close [#583]
- Fixed issues with unclean shutdowns [#170, #580]
- Changed keyboard shortcut to create new database to CTRL+SHIFT+N [#515]
- Compare window title to entry URLs [#556]
- Implemented inline error messages [#162]
- Ignore group expansion and other minor changes when making database "dirty" [#464]
- Updated license and copyright information on souce files [#632]
- Added contributors list to about dialog [#629]
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants