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

only display advanced settings checkbox on tabs that have them (partially fixes #6190) #6513

Merged
merged 1 commit into from
Jul 9, 2023

Conversation

xvallspl
Copy link
Contributor

@xvallspl xvallspl commented May 12, 2021

Current implementation has it disabled instead of hidden. Fixes #6190

In addition, it moves the checkbox into the only tab that has use for it, the Security->Encryption settings tab.

The scrolling bar is not working (yet)

Screenshots

advancedSettings

Testing strategy

Went to each tab and checked every configuration

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@michaelk83
Copy link

From the linked issue:

If you only provide advanced settings for encryption, you should consider to move the checkbox away from the statusbar into the "Encryption Settings" panel, because the current UI design creates the impression this checkbox has a global effect on the entire settings dialog.

I think moving the checkbox to inside the relevant tab(s) would make more sense.

@xvallspl
Copy link
Contributor Author

I am still trying to figure out how difficult it would be to move it inside the tabs. While these changes doesn't solve the awkwardness, they improve on the current solution. Maybe "Fixes" is a strong word.

@xvallspl xvallspl changed the title only display advanced settings checkbox on tabs that have them (Fixes #6190) only display advanced settings checkbox on tabs that have them (partially fixes #6190) May 12, 2021
@droidmonkey
Copy link
Member

I would like it to be part of the Encryption page as well.

image

@xvallspl xvallspl force-pushed the fix/only-show-advanced-settings-if-exist branch from ebd8b87 to 1840f6e Compare May 13, 2021 16:03
@xvallspl
Copy link
Contributor Author

xvallspl commented May 13, 2021

Moved the checkbox into the encryption tab, similar to Jonathan's suggestion. I removed several useless checks on the way.

I would still prefer to have all the settings in the same view, so I'll take a look at that.

@xvallspl
Copy link
Contributor Author

xvallspl commented May 14, 2021

Now simple and advanced settings share view with the checkbox toggling the visibility of the advanced ones. There's a need for a scroll area now, which would be in the next commit.

I was aiming for deleting more code than I introduce, but right now I'm exactly at 0. Make a wish.

@droidmonkey
Copy link
Member

You'll want to disable the slide bar choosing the encryption time in your implementation when the advanced settings are shown.

@xvallspl xvallspl force-pushed the fix/only-show-advanced-settings-if-exist branch from fa11c78 to 0c930e3 Compare May 20, 2021 05:04
@droidmonkey
Copy link
Member

This has a crash on Linux ci that needs to be fixed.

@xvallspl xvallspl force-pushed the fix/only-show-advanced-settings-if-exist branch from 0c930e3 to 11b3595 Compare May 31, 2021 06:17
@droidmonkey droidmonkey added this to the v2.7.0 milestone May 31, 2021
@droidmonkey
Copy link
Member

I made some emergency changes to stop the crashes. I think this needs a fair amount of work to be ready. There are a lot of extra calls to build the version drop down list and the UI elements are not properly sizing for me.

@droidmonkey droidmonkey marked this pull request as draft May 31, 2021 14:50
@xvallspl
Copy link
Contributor Author

xvallspl commented Jun 1, 2021

Ok! I was taking. Alook into this as well

@droidmonkey droidmonkey removed this from the v2.7.0 milestone Dec 23, 2021
@droidmonkey
Copy link
Member

droidmonkey commented Mar 20, 2022

Rebased onto develop and tweaked the layout quite a bit:

image

@droidmonkey droidmonkey force-pushed the fix/only-show-advanced-settings-if-exist branch from 2473114 to db05d66 Compare March 20, 2022 19:08
@droidmonkey droidmonkey force-pushed the fix/only-show-advanced-settings-if-exist branch from db05d66 to abc84fc Compare June 24, 2023 03:19
@droidmonkey
Copy link
Member

Made some more tweaks, this should be final:

image

image

Also removed the global setting that stored whether advanced was shown or not. That could inadvertently cause you to change encryption settings between multiple databases.

@droidmonkey droidmonkey added this to the v2.8.0 milestone Jun 24, 2023
@droidmonkey droidmonkey force-pushed the fix/only-show-advanced-settings-if-exist branch from 785147c to d056d2a Compare June 24, 2023 17:42
@droidmonkey droidmonkey marked this pull request as ready for review June 24, 2023 17:42
@codecov
Copy link

codecov bot commented Jun 24, 2023

Codecov Report

Patch coverage: 97.56% and project coverage change: -0.01 ⚠️

Comparison is base (190a1fa) 64.82% compared to head (142744d) 64.81%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #6513      +/-   ##
===========================================
- Coverage    64.82%   64.81%   -0.01%     
===========================================
  Files          343      337       -6     
  Lines        44613    44555      -58     
===========================================
- Hits         28918    28876      -42     
+ Misses       15695    15679      -16     
Impacted Files Coverage Δ
src/core/Config.cpp 89.76% <ø> (ø)
src/core/Config.h 100.00% <ø> (ø)
src/gui/dbsettings/DatabaseSettingsDialog.h 100.00% <ø> (ø)
...gui/dbsettings/DatabaseSettingsWidgetMaintenance.h 0.00% <ø> (ø)
src/gui/settings/SettingsWidget.cpp 66.67% <ø> (-33.33%) ⬇️
src/gui/wizard/NewDatabaseWizardPage.cpp 83.33% <ø> (+2.25%) ⬆️
...ui/dbsettings/DatabaseSettingsWidgetEncryption.cpp 75.59% <97.50%> (+0.93%) ⬆️
src/gui/dbsettings/DatabaseSettingsDialog.cpp 93.55% <100.00%> (+0.50%) ⬆️

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@droidmonkey droidmonkey force-pushed the fix/only-show-advanced-settings-if-exist branch from d056d2a to da551a8 Compare June 26, 2023 11:41
Fixes #6190

Remove the advanced settings checkbox and replace with a dedicated tab widget interface to toggle between basic and advanced encryption settings.
@droidmonkey droidmonkey force-pushed the fix/only-show-advanced-settings-if-exist branch from da551a8 to 142744d Compare July 9, 2023 18:39
@droidmonkey droidmonkey merged commit 3cf1497 into develop Jul 9, 2023
@droidmonkey droidmonkey deleted the fix/only-show-advanced-settings-if-exist branch July 9, 2023 19:29
@droidmonkey droidmonkey modified the milestones: v2.8.0, v2.7.8 Apr 28, 2024
libf-de pushed a commit to libf-de/keepassxc-secretservice-dbus that referenced this pull request May 6, 2024
Release 2.7.8

Changes
- Add hotkey for showing search help [keepassxreboot#10591]
- Add hotkey for group switching (Ctrl+Shift+PgUp/PgDown) [keepassxreboot#10625]
- Add per-database auto-save delay setting [keepassxreboot#9100]
- Add setting to hide menubar [keepassxreboot#10341]
- Improve Bitwarden 1PUX import and support organization collections [keepassxreboot#10499]
- Show advanced settings checkbox only for settings that have them [keepassxreboot#6513]
- Remove obsolete setting for requiring repeated password entry [keepassxreboot#9722]
- Passkeys: Allow registering Passkeys to existing entries [keepassxreboot#10408]
- Passkeys: Show warning about data being unencrypted before Passkey export [keepassxreboot#10411]
- Passkeys: Support NFC and USB transports [keepassxreboot#10402]
- Passkeys: Pass extension JSON data to browser [keepassxreboot#10615]
- SSH Agent: Do not use entries from recycle bin [keepassxreboot#10518]
- Linux: Change hotkey sequence used for {CLEARFIELD} Auto-Type [keepassxreboot#10008]
- Windows: Improve DACL memory access protection [keepassxreboot#10618]

Fixes
- Fix crash when deleting history items [keepassxreboot#10451]
- Fix crash on screen lock or computer sleep [keepassxreboot#10458]
- Fix search field not being focused after unlock [keepassxreboot#10459]
- Fix loss of window focus when Auto-Type needs to unlock a database [keepassxreboot#10555]
- Fix inconsistent TOTP visibility on unlock [keepassxreboot#10009]
- Fix CSV import skipping over single-name groups [keepassxreboot#10575]
- Fix key file folder being remembered even if disabled in settings [keepassxreboot#10636]
- Fix issues with entry editing and database locking [keepassxreboot#10667]
- Fix key file text when provided on command line [keepassxreboot#10642]
- Fix issues with hardware key auto detection [keepassxreboot#10663]
- Do not override monospace font size [keepassxreboot#10282]
- Perform group sort only when group view is in focus [keepassxreboot#10202]
- Do not show decimals for attachment sizes in Bytes [keepassxreboot#10595]
- Prevent merging of global custom data when merging databases [keepassxreboot#10452]
- Fix minor translation issues [keepassxreboot#10635]
- Passkeys: Fix StrongBox incompatibility [keepassxreboot#10420]
- Passkeys: Set RP ID to effective domain if unset instead of returning an error [keepassxreboot#10384]
- Passkeys: Various UI fixes and improvements [keepassxreboot#10427, keepassxreboot#10608, keepassxreboot#10609]
- AppImage: Fix URL opening [keepassxreboot#10624]
- Flatpak: Fix application autostart [keepassxreboot#10563]
- Linux/macOS: Fix button sizes on modal alert popups [keepassxreboot#10500]
- Linux: Fix clipboard clear on Wayland [keepassxreboot#10500]
- Windows: Preserve file-hidden attribute [keepassxreboot#10343]

# -----BEGIN PGP SIGNATURE-----
#
# iQIzBAABCAAdFiEE6A9nU7OzJX8m9B8ILPQdKqhDj5kFAmY4A30ACgkQLPQdKqhD
# j5npgBAApBCGfhdugBE3X9iCkGQ69LKKWizgp44AzmezxU2ee7KEoZgSmZpOCPyO
# bg9EIgwac+3yCh4i4hJrTvnwIemrUKNsNLE18Kn/Uw3HJBCtsb40CeIFcZktOegu
# RQ5G7jhBtnAopnTKQhdwcwJ0Yq6ZSTSiSuo+miDAN22DjnWVd7BLMOioSBPgxFUT
# td+2MAPeydLoMdFRmkuBaDSStLWThdCz6DrWcBYQSK2b6Mu+3mzmtE24zNM1jCKu
# Tl0t6fRkOhqWSRyWBSMzIH3uMuV95yQNudjDMnuOVWVE9Ai+A1RPFHtf8Zj1ydh9
# n9JGPDyloWRcYQdDBgbn6lFHWnwSaYVCRpRPPmjpmXVwt5/AdtB8wN+6uGbcYTzw
# u9l0YYWx84W0kNPkJ0ZejF33qioQ7FaZruJv2ej++NtO0FJP48UVyrQ4EMG6V+17
# AcQ0aoSWWTb5AYhJXLjImDG7DNY1mbgW6deJLKVS7pkoRke1uSLGqYTUAJCFaXnq
# d9uZt4HRUUMeq6x8dvFNvIcZhsfRUaO/iXjp81nl8hlWIeTYNTj22eww3yapFs+S
# cdmdCmfGZAx5FWCXaszXwD3gLF8Bg6S63l9TvbjEHGR2riYKOO1IbFz8JXXjWpdN
# l4SIcWJfdO2mNz0MWfzNtmMYNu9LBfU2Hod5JHJQYiQh3dh4EG4=
# =MrBi
# -----END PGP SIGNATURE-----
# gpg: directory '/home/runner/.gnupg' created
# gpg: keybox '/home/runner/.gnupg/pubring.kbx' created
# gpg: Signature made Sun May  5 22:09:01 2024 UTC
# gpg:                using RSA key E80F6753B3B3257F26F41F082CF41D2AA8438F99
# gpg: Can't check signature: No public key
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.

Database Security -> Advanced Settings checkbox not clickable
3 participants