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

Overhaul action states and add icons to toolbar #11047

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

droidmonkey
Copy link
Member

@droidmonkey droidmonkey commented Jul 8, 2024

Screenshots

image

Testing strategy

Added unit tests and tested extensively manually

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)
  • ✅ New feature (change that adds functionality)

@varjolintu
Copy link
Member

varjolintu commented Jul 9, 2024

Something I noticed while debugging:
https://github.com/keepassxreboot/keepassxc/blob/develop/src/gui/EntryPreviewWidget.cpp#L544 This is constantly updated even if the config setting has disabled showing the TOTP in the Preview Panel.

EDIT: Maybe the whole Preview Panel shouldn't get updated if it's not even visible.

@michaelk83
Copy link

Consider grouping the new Database Settings toolbar icon into the existing Settings icon, with a dropdown. And in that case, you can remove the separator between the Passkeys and Password Generator, since these icons can be considered as just four tools: passkeys, reports, generator, settings.

@droidmonkey droidmonkey force-pushed the feature/toolbar-upgrade branch 2 times, most recently from 3b7b666 to 135eedc Compare September 7, 2024 18:38
@droidmonkey droidmonkey marked this pull request as ready for review September 7, 2024 18:39
src/gui/MainWindow.cpp Fixed Show fixed Hide fixed
src/gui/MainWindow.cpp Fixed Show fixed Hide fixed
@droidmonkey droidmonkey force-pushed the feature/toolbar-upgrade branch 2 times, most recently from bd31d84 to 1f0525d Compare September 8, 2024 02:03
src/gui/MainWindow.cpp Fixed Show fixed Hide fixed
Copy link

codecov bot commented Sep 8, 2024

Codecov Report

Attention: Patch coverage is 94.28571% with 10 lines in your changes missing coverage. Please review.

Project coverage is 63.36%. Comparing base (c8fc25e) to head (e591dbc).
Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
src/gui/reports/ReportsDialog.cpp 0.00% 6 Missing ⚠️
src/gui/DatabaseTabWidget.cpp 77.78% 2 Missing ⚠️
src/fdosecrets/objects/Service.cpp 0.00% 1 Missing ⚠️
src/gui/DatabaseWidget.cpp 96.43% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #11047      +/-   ##
===========================================
+ Coverage    62.97%   63.36%   +0.39%     
===========================================
  Files          362      362              
  Lines        37783    37729      -54     
===========================================
+ Hits         23793    23906     +113     
+ Misses       13990    13823     -167     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@droidmonkey droidmonkey force-pushed the feature/toolbar-upgrade branch 3 times, most recently from d24142a to 389f740 Compare September 9, 2024 03:08
@droidmonkey
Copy link
Member Author

@phoerious this is ready for review

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.

Going to application settings and back to entry view disables several context menu items
3 participants