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

Need icon for merge database action #122

Closed
droidmonkey opened this issue Dec 3, 2016 · 18 comments
Closed

Need icon for merge database action #122

droidmonkey opened this issue Dec 3, 2016 · 18 comments

Comments

@droidmonkey
Copy link
Member

droidmonkey commented Dec 3, 2016

Expected Behavior

Icons should be added for better usability and recognition of actions.

Current Behavior

No icons are present.

@droidmonkey droidmonkey added this to the v2.2.0 milestone Dec 3, 2016
droidmonkey added a commit that referenced this issue Jan 28, 2017
KeePassX PR Migration: #122 Twofish cypher support
@droidmonkey droidmonkey modified the milestones: v2.2.0, v2.3.0 May 7, 2017
@phoerious phoerious modified the milestones: v2.3.0, v2.4.0 Jan 28, 2018
@droidmonkey droidmonkey changed the title Need icon for merge and repair database actions Need icon for merge database action Sep 24, 2018
@droidmonkey droidmonkey modified the milestones: v2.4.0, v2.5.0 Mar 14, 2019
@phoerious phoerious modified the milestones: v2.5.0, v2.6.0 Oct 26, 2019
@wolframroesler
Copy link
Contributor

With Material Design icons (cf. #475/#4066) we have a plethora to chose from. How about this one:

Screenshot from 2019-12-27 16-53-13

https://materialdesignicons.com/icon/database-refresh

@droidmonkey
Copy link
Member Author

droidmonkey commented Dec 27, 2019

That is more like sync. Is there one with just an incoming arrow? Or perhaps one database with an arrow to another one? We can use this one for the upcoming synchronize feature.

@wolframroesler
Copy link
Contributor

You're right, the Auryn arrows are very similar to what Nextcloud displays while syncing.

How about this one:

Screenshot from 2019-12-27 22-09-40

https://materialdesignicons.com/icon/database-import

Alas, nothing with two databases exists yet, but giving the existing icons, creating a new one shouldn't be too difficult for someone familiar with Inkscape (I'm not).

@droidmonkey
Copy link
Member Author

droidmonkey commented Dec 27, 2019

That one is nice, what about a standard merge icon:

merge

https://materialdesignicons.com/icon/merge

@wolframroesler
Copy link
Contributor

Too generic IMHO. We have "open database" and "new database" already, "merge databases" should graphically connect with them.

Anyway, make your choice, I'll add it to #4066 and configure "Database -> Merge from database" to use it.

@droidmonkey
Copy link
Member Author

Agree, lets use the one you proposed

wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Dec 28, 2019
@wolframroesler
Copy link
Contributor

Done, pushed to #4066.

dbmerge

@droidmonkey
Copy link
Member Author

These are very nice!

wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Dec 28, 2019
wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Dec 28, 2019
wolframroesler added a commit to wolframroesler/keepassxc that referenced this issue Dec 28, 2019
@wolframroesler
Copy link
Contributor

Glad you like them @droidmonkey.

While I was at it, I added another bunch of missing menu icons ("OpenURL" in "Entry", "Sort A-Z/Z-A" in "Group", and almost all in "Help"). Pushed to #4066. We now have icons for all menu items that don't open, or are in, a sub-menu.

entries

groups

tools

help

@phoerious
Copy link
Member

Can we find adequate replacements for the entry and group icons as well? That used to be the blocker in my previous attempts at doing this. They are extremely low-resolution and its an enormous amount of work to find alternatives.

BTW with this change we should finally enable pixmap scaling on all platforms.

@phoerious phoerious added the EPIC label Dec 29, 2019
@wolframroesler
Copy link
Contributor

@phoerious That would be great. The default icons for groups/entries (share/icons/database in the source code) are ancient, hideous, unintellegible, and 16x16 only, and really should be redesigned. I see some possible issues however:

First, users consider these icons part of their database, and it could be off-putting for them if a new KeePassXC release suddenly made everything look different. Not all of our users are IT experts who are tolerant about such changes.

Second, our redesign could mess up people's use of the icons because the redesign would be based on the "semantic" meaning of the icons, but people may have used them "literally" based on their current graphical appearance. For example, there's the C33_Run.png icon that's supposed to resemble a start flag, however I'm using it for games because it also happens to resemble a chess board. C59_Package_Development.png looks like a hammer and C34_Configure.png looks like a wrench; what would break if we changed them into something completely different, e. g. a box and cogwheel?

Third, people use different KeePass clients across platforms (e. g. KeePassXC on macOS and Strongbox on iOS) and may expect icons to look identical (or at least similar enough be be recognizable) on all of them.

AFAIK all of these clients are open source and under active development, and I suppose their maintainers are as annoyed by the icons as we are. Maybe we can try and find a solution together with the others, so at least we maintain some consistency across clients.

Finally, custom icons (e. g. downloaded favicons) don't look too nice next to icons that follow other styles like Material Design. Have a look at this screenshot of KeePassium, a KeePass client for iOS that already uses redesigned default icons:

2019-12-29 12-08-33-1

It's not too bad in this case because this database consistently uses default icons for groups and favicons for entries, but it looks totally messy if they are used in a mix.

For these reasons, I think that redesigning the default icons should go into an issue of its own where we can discuss the solution in detail. IMHO it's independent of the ongoing redesign of the application icons (#475, #4066, #122) which is entirely a question of our own UI design.

If you agree, I'll create a new issue for the default icon redesign and jot down some ideas there.

@wolframroesler
Copy link
Contributor

@phoerious

BTW with this change we should finally enable pixmap scaling on all platforms.

For clarification, does "this change" refer to the existing one (SVG instead of PNG for the application icons as of #475/#4066) or the one you proposed (redesigning the default group/entry icons)?

Also, could you elaborate on how to enable pixmap scaling?

@droidmonkey
Copy link
Member Author

We already did that scaling fix in the pr

@phoerious
Copy link
Member

phoerious commented Dec 29, 2019

KeePass2Android also uses material icons instead of Nuvola. I for myself completely stopped using these icons for entries (except for the generic key icon if there is no favicon) and I am extremely annoyed by the fact that there are so many. We could easily downscale to let's say a handful and 90% of our users would probably still be fine.

@droidmonkey
Copy link
Member Author

droidmonkey commented Dec 29, 2019

Let's make a new issue for this because it does need to be addressed, ideally in 2.6.0. See #4071

droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 6, 2020
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 6, 2020
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 6, 2020
droidmonkey pushed a commit to wolframroesler/keepassxc that referenced this issue Jan 6, 2020
@wolframroesler
Copy link
Contributor

After some rework, here's what the "merge database" icon and the others in the Database menu currently look like (cf. #475/#4066 ):

Screenshot from 2020-01-10 22 48 04

@droidmonkey
Copy link
Member Author

I like that a lot

@wolframroesler
Copy link
Contributor

Since #4066 has been merged, I think this issue can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants