Use text-color icons for system tray Send/Receive menu entries #5649

pull luke-jr wants to merge 1 commits into bitcoin:master from luke-jr:systray_text_icons changing 2 files +20 −2

luke-jr commented at 9:00 PM on January 12, 2015: member

Fixes #5625
jonasschnelli commented at 7:25 AM on January 13, 2015: contributor

@zander please have a look at: #5625. I think the problem addresses in the PR is win only.
zander commented at 7:31 AM on January 13, 2015: none

I suggest using #ifdef Q_OS_WIN then for this code; so it won't make things worse on other OS's (esp linux) where this behavior change is not wanted.
jonasschnelli commented at 7:37 AM on January 13, 2015: contributor

@zander in general we like to keep the platform dependent stuff (#ifdef) on a minimum. And the problem with mixed colors could occur on other platforms.
jonasschnelli commented at 7:53 AM on January 13, 2015: contributor

Tested on Win,OSX,Ubuntu (change only visible on Windows, see below).

Tested ACK. But it brought me to a new issue: #5653
laanwj added the label GUI on Jan 13, 2015
zander commented at 9:57 AM on January 13, 2015: none

And the problem with mixed colors could occur on other platforms.

The "problem" is debatable. Having the same icon in all places is quite important for a stable UI (consistency). Anywhere I look having monochrome icons that follow the text color just isn't the default, and the menu in the latest screenshot looks broken. Its completely inconsistent with all the other apps because they all use color icons. Its also inconsistent with itself because the "Show / Hide" for instance is not actually the logo you see in the taskbar, its got a different color.

Bottom line, I have zero apps on either Windows (8.1) nor on KDE that have monochrome icons in their systray context menu. So this just doesn't look consistent in any way at all.
jonasschnelli commented at 10:19 AM on January 13, 2015: contributor

@zander The discussion over the monochrome icons has already been held. But feel free to contribute over issue and new pull requests. This pull requests focus on #5625.
laanwj commented at 8:04 AM on January 14, 2015: member

ACK.
Use text-color icons for system tray Send/Receive menu entries 301cd2a3a9
in src/qt/bitcoingui.h:None in 6ddd105e61 outdated
```
  89 | @@ -90,12 +90,14 @@ class BitcoinGUI : public QMainWindow
  90 |      QAction *historyAction;
  91 |      QAction *quitAction;
  92 |      QAction *sendCoinsAction;
  93 | +    QAction *sendCoinsMenuAction;
```
laanwj commented at 3:52 PM on January 14, 2015:

These extra fields neeed to be zeroed in the constructor

luke-jr commented at 5:02 PM on January 14, 2015:

createActions, which is part of the constructor, initialises them. Regardless, I added the (unnecessary) zeroing as requested.

laanwj commented at 5:07 PM on January 14, 2015:

Yes, it's mostly to satisfy static checkers, as well as a belt-and-suspenders measure in general. Thanks.
luke-jr force-pushed on Jan 14, 2015
laanwj merged this on Jan 14, 2015
laanwj closed this on Jan 14, 2015
laanwj referenced this in commit e0cd2f5523 on Jan 14, 2015
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5625 [QT] Win: icon coloring issue tray-menu