revert to default OS theme for progressbar (fix #1071) #1091

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:GUI-BlockDL changing 1 files +6 −9

Diapolo commented at 4:28 PM on April 13, 2012: none

As there were a few readability issues with certain OS themes and our self-defined progressbar style, this reverts the bar to OS defaults.

Fixes #1071.
revert to default OS theme for progressbar / fix small glitches in bitcoingui.cpp e72c6a1830
Diapolo commented at 4:29 PM on April 13, 2012: none

Dunno why, but the default is now even readable on Win7 as the text stays black, no matter if the Window is active or not ... I'm fine with the OS default then!
in src/qt/bitcoingui.cpp:None in e72c6a1830
```
 421 | @@ -425,17 +422,17 @@ void BitcoinGUI::trayIconActivated(QSystemTrayIcon::ActivationReason reason)
 422 |  void BitcoinGUI::toggleHidden()
 423 |  {
 424 |      // activateWindow() (sometimes) helps with keyboard focus on Windows
 425 | -    if(isHidden())
 426 | +    if (isHidden())
```
laanwj commented at 7:00 PM on April 13, 2012:

Why add spaces here? Most if the ifs() are like this, and IMO that's fine.

Diapolo commented at 7:57 PM on April 13, 2012:

There are at least 5 places in where it is if () instead of if(). What do you prefer? I'll use that in the future, if we don't get a new standard coding style, so that you and I use the same as we often work on the same code parts.
laanwj commented at 6:00 AM on April 14, 2012:

I always use this coding style for if/expressions:

if(a && !b && ((c + d) < e)) { } else [if(...)] { }

You can recognize other people's code if it is different :-) So I prefer it that way, but won't reject pull requests on it unless it's an absolute mess otherwise.

My point was that here's no need to change this function in this patch. We like minimal changes for small things, as they make merging (of other patches, less rebases) easier.
jgarzik commented at 3:34 PM on April 15, 2012: contributor

The dominant bitcoin coding style is overwhelmingly "if (", with a space. Two seconds of grep would show that...

Let us continue to be consistent with the rest of the code.
laanwj commented at 3:43 PM on April 15, 2012: member

Maybe in the core. Not in the Qt code.

Whatever the common coding style is, commits shouldn't be cluttered with unrelated changes.
laanwj referenced this in commit 088a13331b on Apr 16, 2012
laanwj merged this on Apr 16, 2012
laanwj closed this on Apr 16, 2012
sipa commented at 8:43 PM on April 17, 2012: member

Why was this merged? This is how it looks now for me: http://bitcoin.sipa.be/bitcoinqt.png
laanwj commented at 8:48 PM on April 17, 2012: member

Sipa: See discussion on #1071. We intend to only override the style in specific cases. In most cases the OS default simply works.
coblee referenced this in commit a9d5d2a174 on Jul 17, 2012
suprnurd referenced this in commit 01990272e5 on Dec 5, 2017
dagurval referenced this in commit fb326382b4 on May 27, 2018
lateminer referenced this in commit a81c2443e2 on Jan 22, 2019
dexX7 referenced this in commit 70b8bdfaed on Mar 25, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Linked (view graph)

#1071 blockchain download status is hard to read