Improve entry table display/functionality in GUI #849

fonic · 2017-08-05T12:50:07Z

Description

This adds to the entry table:

additional columns Password, Notes, Expires, Created, Modified, Accessed and Attachments (Add sortable last-modified column to entry list view #129)
a menu to customize columns (toggle visibility, resize, reset) (Enable configurable columns in the entries grid #17, Add sortable last-modified column to entry list view #129, A way to edit list of attributes in the upper part #397)
'copy-on-doubleclick' for columns Username, Password and Notes (Copy column field value on double-click #360)

With this, the entry table gets mostly in par with KeePassX 0.4.x.

All changes are currently prefixed with /** @author Fonic [...] */ comments and there are in-code TODOs to be resolved. Those will of course be cleaned/removed once this is ready to be shipped.

Please note that this is work in progress and that I'm still familiarizing myself with the project and its guidelines. Thus, if something is not the way it's supposed to be, please be so kind and point it out.

Motivation

In my opinion, this addresses a major disadvantage of KeePassXC. Not only the additional columns, but more importantly the 'copy-on-doubleclick' really adds to the usability.

I am aware that redesigning the entry table already is a goal within this project (#129). My contribution may either be a step towards this or at least fill the gap until this is done.

DONE

Add additional columns to entry table
Implement 'copy-on-doubleclick' for certain columns
Use date/time format from locale instead of hard-coded format string to display timestamps
Add header menu (right click on header):
- add actions for toggling column visibility
- add actions for auto-resizing columns (fit to window, fit to contents)
- add actions to reset header configuration (reset to last session state, reset to defaults)
- add checkbox actions to toggle hidden/cleartext display of username and password
Make settings persistent (save/load to/from configuration):
- header state (column order/sizes/visibility)
- hidden/cleartext display of username and password
Change EntryModel::columnCount() to comply to Qt documentation advisory

TODO

Decide whether overall view state (splitter, entry list header configuration) should be synced across tabs or should be kept separate; separation would reduce complexity and improve usability [discussion]
Decide whether view state of entry list should be synced or kept separate when switching between normal mode (i.e. list entries of selected group) and search mode; my opinion: sync state from list to search, but not the other way (i.e. search looks like list, but changes when in search mode are discarded when search mode ends) [discussion]
Decide whether to force sort column/order when entering/leaving search mode or to save/restore previous sort column/order; my opinion: backup ordering before entering search mode, force ordering for search mode, restore ordering when leaving search mode [discussion]
Add header menu as submenu of menu 'View'
Resolve minor in-code TODOs
Remove development comments

How has this been tested?

locally

Screenshots:

Types of changes

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

Checklist (in progress):

✅ 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]
❎ My change requires a change to the documentation and I have updated it accordingly.
❎ I have added tests to cover my changes.

Add additional columns 'Password', 'Notes', 'Expires', 'Created', 'Modified', 'Accessed' and 'Attachments' to entry table display. With this, the entry display is in par with KeePassX 0.4.x.

Add 'copy-on-doubleclick' functionality for entry table columns 'Username', 'Password' and 'Notes'.

On Travis CI, entry->attachments()->keys().join() does not exist when it should (http://doc.qt.io/qt-5/qlist.html#more-members). Using for loop construct for now.

louib · 2017-08-05T17:32:39Z

src/gui/entry/EntryModel.cpp

+     *       instead this should be global and user-configurable (or derived automatically
+     *       from current regional settings)
+     */
+    QString timeFormat("d MMM yyyy HH:mm:ss");


@fonic since this is a constant, this could be declared as such in the EntryModel header file.

static const QString DefaultTimeFormat("d MMM yyyy HH:mm:ss");

We could do that, but I think this should be a global, user-configurable setting as the way timestamps are displayed varies from one country/language to another.

How about putting it in src/core/Config.cpp:

void Config::init(const QString& fileName) { m_defaults.insert("DateTimeFormat", "d MMM yyyy HH:mm:ss"); }

and then fetching it from the config whenever it's needed?

@fonic personally, I'd leave it non-configurable, and set the format to ISO 8601. We can always make it configurable if we ever get that request.

It should read the locale display preference from the OS and use that.

Let's do that. Quite easy actually: QDateTime.toString(Qt::DefaultLocaleShortDate). Uses the application's locale and falls back to the system's locale if nothing is set specifically for the application.

To allow for correct sorting, I'd implement Qt::UserRole like done here.

Update entry model test to account for additional columns 'Password', 'Notes', 'Expires', 'Created', 'Modified', 'Accessed' and 'Attachments'.

Instead of using a hard-coded date/time format string to display timestamps, use format defined by current application locale (defaults to system locale if not explicitely set). Separate display (uses QStrings) and sorting (uses QDateTimes) of timestamps by adding a custom role (Qt::UserRole) to the model.

Add header menu (right click on header): - actions to toggle columns visibility - actions to resize all columns - actions to reset column configuration Make changes persistent by saving/loading to/from configuration Change EntryModel::columnCount() to comply to Qt documentation advisory Lots of code cleanup (comments, TODOs, coding rules)

Use old-style QMenu->addAction(const QString &text, const QObject *receiver, const char *member, const QKeySequence &shortcut = 0) to add actions to header menu to fix compile errors on Travis CI.

Column 'Group' is no longer added/deleted when switching between list and search mode, but shown/hidden instead. Therefore, when in list mode, column 'Group' with index 0 is hidden and cannot be clicked. Alter test to click column 'Title' with index 1 instead.

fonic · 2017-08-07T08:23:28Z

So, lots of progress here. Please test and let me know what you think. When testing, please focus on the functionality for now. I think I'll restructure this with clean commits once the functionality is defined/decided, as the current approach of keeping track of the view state is inefficient.

Btw:

testgui reports lots of leaks which are not related to my changes (I compared with branch 'develop', same thing there). Is that the way it's supposed to be?
The build system here seems to use a rather old Qt version. Is there a specific reason for this? I'm using Qt 5.7.1 and the sources build just fine with it

Add functionality for hidden/cleartext display of usernames and passwords: - modify data providers of entry view model - add settings to entry view / entry view model - add corresponding checkbox actions to entry view header menu - make settings persistent by saving/loading to/from configuration Again lots of code cleanup (variable names, includes, comments, TODOs, code formatting)

fonic · 2017-08-08T19:47:37Z

I'll stop development at the current state until things listed in the TODO section have been discussed and decided as those greatly affect implementation.

From my point of view, all functionality is there - now it's only a matter of deciding together which pieces to integrate in what way and how (separate pull requests, all in one, ...).

louib · 2017-08-10T23:51:24Z

@fonic tested that quickly on linux. This is pretty neat, good job!

I'd personally vote for enabling only a subset of the available fields by default (maybe title & username & url & modified date?)

Decide whether overall view state (splitter, entry list header configuration) should be synced across tabs or should be kept separate; separation would reduce complexity and improve usability [discussion]

You mean if the header state should be global or per-database file? I'd go for a global header state. AFAICT it's easier to implement and I feel like the convenience of being able to set it once for all databases wins over being able to customize it for specific databases.

TheZ3ro · 2017-08-12T08:54:54Z

Generally I would prefer KISS

Decide whether overall view state (splitter, entry list header configuration) should be synced across tabs or should be kept separate; separation would reduce complexity and improve usability [discussion]

I'd go for a global header state as well. This needs to be saved in the configuration to restore the state when keepassxc is re-opened

Decide whether view state of entry list should be synced or kept separate when switching between normal mode (i.e. list entries of selected group) and search mode; my opinion: sync state from list to search, but not the other way (i.e. search looks like list, but changes when in search mode are discarded when search mode ends) [discussion]

I think it should be separate but if we save the state in the configuration the search state should be saved as well

Decide whether to force sort column/order when entering/leaving search mode or to save/restore previous sort column/order; my opinion: backup ordering before entering search mode, force ordering for search mode, restore ordering when leaving search mode [discussion]

I agree with your opinion

fonic · 2017-08-28T15:16:32Z

Hi there,

sorry that it took me so long to respond, I was on badly needed vacation ;) I'll continue working on this shortly.

I'd personally vote for enabling only a subset of the available fields by default (maybe title & username & url & modified date?)

I would suggest displaying all available fields by default so new users see what is available and can customize to their needs. Just my opinion, of course we can do it your way, too.

Decide whether overall view state (splitter, entry list header configuration) should be synced across tabs or should be kept separate; separation would reduce complexity and improve usability [discussion]

You mean if the header state should be global or per-database file? I'd go for a global header state. AFAICT it's easier to implement and I feel like the convenience of being able to set it once for all databases wins over being able to customize it for specific databases.

Yes, that's exactly what I meant. I already implemented both ways, the complexity of having separate states (i.e. per database) is actually not that bad. Syncing the view, on the other hand, is quite difficult (see below).

I'd go for a global header state as well. This needs to be saved in the configuration to restore the state when keepassxc is re-opened

Good. I agree, I think having separate states is just confusing to users, because every time a tab is selected the view changes. Saving/loading to/from configuration is already implemented, but not yet uploaded.

I think it should be separate but if we save the state in the configuration the search state should be saved as well

Why do you vote for separate? Just asking.

Generally I would prefer KISS

I love KISS, the question in this case is KISS for whom:

KISS for the user (same view for each database) means complexity for the code
KISS for the code (separate view for each database) might mean complexity for the user

Just so that you understand: syncing the view states is actually quite difficult due to the way the header state is represented in Qt (binary stream).

To make the whole decision process easier, I coded a version that allows toggling of everything we discuss here (e.g. sync splitter yes/no, sync header state yes/no). I think actually testing this is better than words. Will upload that shortly. After that, I'll start submitting the changes cleaned up and piece by piece within a new pull request-

cjsheets · 2017-08-29T02:47:30Z

Thank you very much for taking on this feature @fonic , It's the only functionality I find myself missing with keepassxc.

Just thought I'd +1 your thought to retain the option to show all fields. I happen to primarily use the Note field (since, for auto-type, we don't have much say on what goes in Title) but different people will have different styles.

frostasm · 2017-11-02T19:17:11Z

There is a chance that these changes will be in version 2.3.0?

droidmonkey · 2017-11-02T20:03:45Z

Absolutely

TheZ3ro · 2017-11-27T14:46:04Z

@fonic any news on this?

fonic · 2017-12-17T08:44:50Z

Sorry for replying so late. Q3/Q4 is always the busiest time of the year at my workplace, I just couldn't find the time to work on anything else over the last couple of months. Thankfully, that's behind me now :)

I'll check what has changed in the code base since then and continue to work on this.

fonic · 2017-12-19T10:25:22Z

Closing this pull request in favor of new pull request #1305.

fonic added 3 commits August 5, 2017 13:52

Add additional columns to entry table

fa0dd62

Add additional columns 'Password', 'Notes', 'Expires', 'Created', 'Modified', 'Accessed' and 'Attachments' to entry table display. With this, the entry display is in par with KeePassX 0.4.x.

Add 'copy-on-doubleclick' functionality to entry table

809b84e

Add 'copy-on-doubleclick' functionality for entry table columns 'Username', 'Password' and 'Notes'.

Fix compile error on Travis CI

1cc488b

On Travis CI, entry->attachments()->keys().join() does not exist when it should (http://doc.qt.io/qt-5/qlist.html#more-members). Using for loop construct for now.

droidmonkey added needs work user interface labels Aug 5, 2017

droidmonkey added this to the v2.3.0 milestone Aug 5, 2017

fonic mentioned this pull request Aug 5, 2017

Enable configurable columns in the entries grid #17

Closed

louib reviewed Aug 5, 2017

View reviewed changes

fonic added 7 commits August 6, 2017 05:12

Update in-code TODOs

d2f1273

Update entry model test to account for additional columns

9d6a269

Update entry model test to account for additional columns 'Password', 'Notes', 'Expires', 'Created', 'Modified', 'Accessed' and 'Attachments'.

Update in-code TODO

5f08f92

Use old-style QMenu->addAction() to add actions to header menu

17c460d

Use old-style QMenu->addAction(const QString &text, const QObject *receiver, const char *member, const QKeySequence &shortcut = 0) to add actions to header menu to fix compile errors on Travis CI.

This was referenced Aug 7, 2017

Copy column field value on double-click #360

Closed

Add sortable last-modified column to entry list view #129

Closed

fonic added 3 commits August 7, 2017 10:51

Update in-code TODOs/comments

3ff8ed2

Update in-code TODOs

304e500

TheZ3ro mentioned this pull request Sep 23, 2017

option to display passwords, notes in main window #982

Closed

droidmonkey added the in progress label Oct 11, 2017

droidmonkey removed the in progress label Oct 19, 2017

droidmonkey added high priority 🚨 and removed needs work labels Nov 2, 2017

droidmonkey self-assigned this Nov 27, 2017

droidmonkey mentioned this pull request Nov 27, 2017

Add column for entries: last modify date / creation date #1233

Closed

fonic mentioned this pull request Dec 19, 2017

Improve and extend entry view table #1305

Merged

fonic closed this Dec 19, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve entry table display/functionality in GUI #849

Improve entry table display/functionality in GUI #849

fonic commented Aug 5, 2017 •

edited by droidmonkey

Loading

louib Aug 5, 2017

fonic Aug 5, 2017

louib Aug 5, 2017 •

edited

Loading

droidmonkey Aug 5, 2017

fonic Aug 6, 2017 •

edited

Loading

fonic commented Aug 7, 2017 •

edited

Loading

fonic commented Aug 8, 2017 •

edited

Loading

louib commented Aug 10, 2017

TheZ3ro commented Aug 12, 2017

fonic commented Aug 28, 2017 •

edited

Loading

cjsheets commented Aug 29, 2017

frostasm commented Nov 2, 2017

droidmonkey commented Nov 2, 2017

TheZ3ro commented Nov 27, 2017

fonic commented Dec 17, 2017

fonic commented Dec 19, 2017

Improve entry table display/functionality in GUI #849

Improve entry table display/functionality in GUI #849

Conversation

fonic commented Aug 5, 2017 • edited by droidmonkey Loading

Description

Motivation

DONE

TODO

How has this been tested?

Screenshots:

Types of changes

Checklist (in progress):

louib Aug 5, 2017

Choose a reason for hiding this comment

fonic Aug 5, 2017

Choose a reason for hiding this comment

louib Aug 5, 2017 • edited Loading

Choose a reason for hiding this comment

droidmonkey Aug 5, 2017

Choose a reason for hiding this comment

fonic Aug 6, 2017 • edited Loading

Choose a reason for hiding this comment

fonic commented Aug 7, 2017 • edited Loading

fonic commented Aug 8, 2017 • edited Loading

louib commented Aug 10, 2017

TheZ3ro commented Aug 12, 2017

fonic commented Aug 28, 2017 • edited Loading

cjsheets commented Aug 29, 2017

frostasm commented Nov 2, 2017

droidmonkey commented Nov 2, 2017

TheZ3ro commented Nov 27, 2017

fonic commented Dec 17, 2017

fonic commented Dec 19, 2017

fonic commented Aug 5, 2017 •

edited by droidmonkey

Loading

louib Aug 5, 2017 •

edited

Loading

fonic Aug 6, 2017 •

edited

Loading

fonic commented Aug 7, 2017 •

edited

Loading

fonic commented Aug 8, 2017 •

edited

Loading

fonic commented Aug 28, 2017 •

edited

Loading