Purge unused custom icons and bulk deletion of icons #5970
Conversation
schlimmchen
changed the title
|
Please squash all your commits. This looks great, although I'm not in favor of a dedicated tab for custom icons. I think adding it to a "Database Maintenance" tab would be good. We can include other cleanup actions on there. |
|
I agree that a dedicated tab just to manage icons is very bold. I tried adding the list view and button to the general tab, but that one ist too cluttered then and there would be no space to add anything else that actually belongs there. A "Maintenance" tab is a good idea. I can see buttons like "Trim history entries now" and "Refresh database root group ID" (currently in Browser Integration) there. I'll rename the icons tab and squash my commits. |
|
Yup my thoughts exactly. |
schlimmchen
force-pushed
the
feature/2110-purge-unused-icons
branch
3 times, most recently
from
bf7a756 to
164c542
Compare
|
Notice the renamed tab, the new icon, and the added spacer so that the icon list view does not span the whole widget vertically. Commits were squashed. Needed another commit update after noticing that I did not add the new icon to the COPYING file, and then noticing I chose an icon from the wrong set. The icon in use now is "hammer-wrench" from the material design icon set. |
|
Yes! Very nice |
droidmonkey
changed the title
This change adds a new database settings widget named "maintenance", using a wrench icon. This widget is designated to be the home for database related maintenance tasks. Initially, managing custom icons is now possible from that new tab. The feature includes bulk removing of any number of selected custom icons and automatic purging of unused custom icons by the click of a button. Fixes keepassxreboot#2110
droidmonkey
force-pushed
the
feature/2110-purge-unused-icons
branch
from
164c542 to
28d4291
Compare
|
Great work this is ready to merge |
|
This is a GLORIOUS feature! Thank you! |
phoerious
added
pr: new feature
[This is a copy of my previous pull request #5969, which I fucked up by accidentally deleting my remote branch to be merged here.]
This change implements purging custom icons that are not in use by any entry or group. These orphaned icons are easily created when downloading new favicons, or switching to new custom icons. Which icons are obsolete is hard to identify by hand, and clicking every icon and trying to delete it waiting for a confirmation not to delete if the icon is in use is very tedious.
It is now also possible to select multiple icons and bulk delete them. The user is prompted (only once) to confirm deletion of icons that are still in use by any group or entry.
Icons that are exclusively used by history entries are purged and deleted without prompting. This logic was already in place for single deletion of icons and was adopted for the purging feature.
A new database settings dialog was created for managing the icons (purging and deleting) for multiple reasons: Given the use case that a user wants to delete/purge icons, it is not intuitive to edit an arbitrary entry/group, switch to the Icon pane, and delete/purge icons from there. There are also special cases for the purging scenario in which it becomes non-trivial to decide if the icon of the currently edited entry/group is obsolete or not depending on the current list view selection (see commit messages). Moreover, the feature to select multiple icons for bulk deletion makes no sense in context of the entry/group editing dialog (what should happen when multiple icons are selected (by accident) while editing an entry).
Entry/Group edit UI: Select an icon, add acustom icon, or download a favicon.
Database settings UI: Purge or (bulk) delete icons.
Fixes #2110.
Screenshots
Testing strategy
Manually using a dummy database with three orphaned icons, one of which is a history-entry-only icon. See
icontests.zip (password: "icontests").
Type of change