Skip to content

[UI] Some standard Bitcoin icons beautified#764

Merged
eduffield222 merged 3 commits into
dashpay:v0.12.1.xfrom
crowning-:UI_Icons
May 9, 2016
Merged

[UI] Some standard Bitcoin icons beautified#764
eduffield222 merged 3 commits into
dashpay:v0.12.1.xfrom
crowning-:UI_Icons

Conversation

@crowning-
Copy link
Copy Markdown

The new black/transparent icons introduced by the Bitcoin-devs for the menu items "Open URI", "Command-line options", "About Dash Core" and "About Qt" are almost invisible on some Linux flavors, and don't look good on other operating systems depending on the OS-version and/or chosen UI-theme of the operating system. No idea why they did this, especially why they did this only for a couple of icons and not ALL of them. Makes the look quite inconsistent, and makes theme-dependent icons impossible.

#762 already disables overpainting the existing icons, however they also changed some icons to black with transparent background, e.g. https://github.com/dashpay/dash/blob/v0.12.1.x/src/qt/res/icons/drkblue/about_qt.png

Depending on the UI-theme of your operating system you'll end with black icons on a black background. Not good.

I've changed the icons (or, where possible, used the standard Qt-build-in icons) to have the much better and more consistent look we had before.
Test on several Linux flavors, Windows and OSX.

Before:

(Ubuntu 14.04):
before

After:

Ubuntu/Linux:
linux

Windows:
win

OSX (no icons):
osx

Unfortunately this renders the PR of @taw00 (#759) useless.
Sorry about this.
@taw00, it's still nice to have those Bitcoin icons dashified, it's probably a good idea you keep them locally in case Bitcoin will continue to replace all icons and we might decide to follow them here.

Comment thread src/qt/bitcoingui.cpp
usedReceivingAddressesAction->setStatusTip(tr("Show the list of used receiving addresses and labels"));

openAction = new QAction(QIcon(":/icons/" + theme + "/open"), tr("Open &URI..."), this);
openAction = new QAction(QApplication::style()->standardIcon(QStyle::SP_DirOpenIcon), tr("Open &URI..."), this);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice if we would also:

  1. remove references like https://github.com/dashpay/dash/blob/v0.12.1.x/src/qt/dash.qrc#L50, https://github.com/dashpay/dash/blob/v0.12.1.x/src/Makefile.qt.include#L213 etc for all themes
  2. remove resources themselves (i.e. file info.png in this case, for all themes too)

@crowning-
Copy link
Copy Markdown
Author

Yeah, good find. I'll do some general icon-cleanup tomorrow.

@crowning-
Copy link
Copy Markdown
Author

crowning- commented Apr 25, 2016

Done, builds on all 3 OS flavors and still looks good :-)

Ready to merge IMO.

When I should have some more time this week I'll probably write a little shell-script which shows unused icons/GFX and obsolete dependency entries so we won't have to find them by hand next time.

@taw00
Copy link
Copy Markdown

taw00 commented Apr 25, 2016

re: My PR. It's fine. It's "code". ;) Just trying to be consistent with themes of the BTC icons.
Really need to have a suite of dash icons in their own contrib directory or somesuch and not have them named "bitcoin.*" But, this is the annoying part of forking from bitcoin core We are brute-force skinning. :)

@UdjinM6
Copy link
Copy Markdown

UdjinM6 commented Apr 25, 2016

Looks good for me 👍
not tested ACK ddfc3e9

@eduffield222
Copy link
Copy Markdown

nice!!

@eduffield222 eduffield222 merged commit 684994c into dashpay:v0.12.1.x May 9, 2016
@crowning- crowning- deleted the UI_Icons branch May 11, 2016 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants