[QT] minor UI updates #6110

jonasschnelli commented at 12:21 PM on May 6, 2015: contributor

I cumulate changes to avoid having to many PRs:

I resized the transaction icon in the overview page a little bit and changed the amount of transaction from 3 to 5. Otherwise there is always blank space at the bottom.
To avoid the double frame issue in sendcoins / sendcoinsentry (recipient box) i replace the inner frame with a horizontal line.
Use warning icons instead of "(out of sync)" text in overview page
Don't colorize icons on win/mac

EDIT: added point 4

bildschirmfoto 2015-05-06 um 14 09 01

bildschirmfoto 2015-05-06 um 14 19 37

bildschirmfoto 2015-05-06 um 14 06 32

bildschirmfoto 2015-05-06 um 14 19 50

Screens from current master

Double frame issue: bildschirmfoto 2015-05-06 um 14 09 45

Empty space: bildschirmfoto 2015-05-06 um 14 09 40

jonasschnelli force-pushed on May 6, 2015

[QT] change transaction amount and height in overview page 2a6b844427

jonasschnelli force-pushed on May 6, 2015

jonasschnelli commented at 2:51 PM on May 6, 2015: contributor

Added another change on the top (cumulating changes to avoid another blizzard of PRs):

use warning icons instead of "(out of sync)" text.

Looks like: bildschirmfoto-2015-05-06-um-16 49 26

bildschirmfoto-2015-05-06-um-16 50 25

ddehueck commented at 3:05 PM on May 6, 2015: none

This looks really good. The only suggestions I have would be to make the icon red and the tooltip state that the synchronization process could take awhile if the user is synchronizing for the first time. Thoughts?

jonasschnelli commented at 3:11 PM on May 6, 2015: contributor

@essofluffy: IMO red is the color of errors. Yellow would be appropriate. But since i started with black, flat icons (https://github.com/bitcoin/bitcoin/pull/5219) i would say black is fine and it would avoid rainbow color software.

jonasschnelli force-pushed on May 6, 2015

ddehueck commented at 3:34 PM on May 6, 2015: none

@jonasschnelli That PR had some good discussion so I'll concede on the color of the icon. Black will work just fine.

jonasschnelli commented at 3:37 PM on May 6, 2015: contributor

Added another commit:

use always black icons on Mac and Win

jonasschnelli renamed this:
~~[QT] minor UI updates (amount of transactions in overview and sendcoinsentry frame)~~
[QT] minor UI updates
on May 6, 2015

ghost commented at 11:02 PM on May 6, 2015: none

utAck, I like tooltips for verbiage, but the icon looks really blended in with the text, I'm not sure a user would notice it has additional information if hovered over.

ddehueck commented at 11:28 PM on May 6, 2015: none

@jonasschnelli Did you make these icons?

jonasschnelli commented at 6:42 AM on May 7, 2015: contributor

Did you make these icons? @essofluffy: no, check: https://github.com/jonasschnelli/bitcoin/blob/2015/05/qt_polish_1/doc/assets-attribution.md#typiconsstephen-hutchings

[...] I'm not sure a user would notice it has additional information if hovered over.

I have to agree. Placing the text "(out of sync)" next to the icon would be a option. Other ideas?

ghost commented at 7:00 AM on May 7, 2015: none

I have to agree. Placing the text "(out of sync)" next to the icon would be a option. Other ideas?

Typically the visual cue i look for in a tooltip is a little (?) but i think this tooltip "cue" can really be basically anything.. dashed underlines i've seen before too.

ddehueck commented at 2:45 PM on May 7, 2015: none

Maybe something like, (out of sync, hover for more info)?

sipa commented at 3:05 AM on May 8, 2015: member

Concept ACK

jonasschnelli commented at 8:32 AM on May 8, 2015: contributor

@essofluffy @faizkhan00 : i don't think we should tell the user "hover for more information". It's somehow uncommon. I think the current solution is almost okay. What it misses if a reaction when you click on the new "warning" icon. There ist should open a alert with the out of since information text (same as tooltip). I'll try to add this soon.

ghost commented at 4:06 PM on May 8, 2015: none

@jonasschnelli Hmm, yeah looking over the images I think this is reasonable too, I'm correct in assuming those icons disappear once the blockchain is fully synced up, as well?

ddehueck commented at 4:25 PM on May 8, 2015: none

@faizkhan00 your assumption would be correct.

ddehueck commented at 2:01 PM on May 9, 2015: none

@jonasschnelli Also, if you ever need custom graphics made let me know and I can help you out.

ghost commented at 6:20 PM on May 9, 2015: none

Tested ACK, I really like the tooltips, actually

fanquake commented at 5:12 AM on May 10, 2015: member

As has already been mentioned by faiz and esso, I'm dubious over wether the "out of sync" icon is better than the text. Having an icon, and then having to somewhat convey to the user they need to hover to actually get useful info seems kind of pointless, we might as well just keep the text there.

ACK 1 & 2

ghost commented at 5:24 AM on May 10, 2015: none

@fanquake I was dubious at first too, but after building and seeing it in action I felt convinced the more intuitive experience is to hover-over-then-read versus force-to-read, if only because the icons are a intuitive visual cue that something needs your attention.

ddehueck commented at 7:19 PM on May 10, 2015: none

Tested ACK btw

jonasschnelli commented at 9:45 AM on May 11, 2015: contributor

@fanquake: would you agree with 3) if the icon would be clickable then showing a info-alert with the extended out-of-sync information?

What about 4)? (https://github.com/bitcoin/bitcoin/pull/6110#issuecomment-99516379)

laanwj commented at 3:27 PM on May 11, 2015: member

ACK

fanquake commented at 2:29 AM on May 12, 2015: member

@jonasschnelli ACK 4 as well. If consensus is that 3 is an improvement, then lets get this merged.

luke-jr commented at 4:28 AM on May 12, 2015: member

Concept ACK

Significant issue: point 2 results in less recipients visible in sendcoins dialog.

Nits/preferred changes:

Overview recent txs should grow with window size.
Warning icon maybe ought to be red?

laanwj commented at 9:05 AM on May 12, 2015: member

@luke-jr Icon color has been already discussed above, IMO it's fine like this. It is not a very dangerous warning, we don't want to unnecessarily light up the overview page like a christmas tree.

jonasschnelli commented at 11:23 AM on May 12, 2015: contributor

@luke-jr

Significant issue: point 2 results in less recipients visible in sendcoins dialog.

The new sendcoinsentries are some pixels higher. But it depends on you window height. So i think this is not very urgent.

Overview recent txs should grow with window size.

Agreed. But this would be a different PR

Warning icon maybe ought to be red?

As @laanwj said. See above. IMO red makes only sense for errors.

laanwj added the label GUI on May 12, 2015

luke-jr commented at 5:26 PM on May 12, 2015: member

@jonasschnelli I don't get the point of making sendcoinsentry taller. It doesn't depend on window height - no matter what height is used, less now fits in it.

jonasschnelli commented at 6:09 PM on May 12, 2015: contributor

@luke-jr: Okay. The top/bottom margin could be better. I try to solve it and will see if i get ~the same height.

jonasschnelli force-pushed on May 12, 2015

[QT] remove frame to avoid double-frame situation in sendcoinsentry.ui 51c7c7029e

[QT] use alert icon with tooltip insted of "(out of sync)" text

# Conflicts:
#	src/qt/forms/overviewpage.ui
#	src/qt/overviewpage.cpp

7247d103ff

jonasschnelli force-pushed on May 12, 2015

jonasschnelli commented at 11:16 AM on May 13, 2015: contributor

@luke-jr: I did optimize the sendcoinsentry height. Check the comparison screenshot (open the image url in a new tab/window to get full resolution).

bildschirmfoto-2015-05-13-um-13 07 10

I hope this PR makes it into 0.11!

luke-jr commented at 4:25 PM on May 13, 2015: member

Tested ACK

[QT] don't colorize icons on win and mac ca5f688547

in src/qt/scicon.cpp:None in 093ed953cd outdated

  55 | @@ -51,6 +56,9 @@ QIcon SingleColorIcon(const QString& filename, const QColor& colorbase)
  56 |  
  57 |  QColor SingleColor()
  58 |  {
  59 | +#if defined(WIN32) || defined(MAC_OSX)
  60 | +    return QColor(0,0,0);

laanwj commented at 5:48 PM on May 13, 2015:

I wonder if this will cause code-unreachable warnings, may be better to #else #ifdef the rest of the function, same for SingleColorIcon.

jonasschnelli commented at 6:30 PM on May 13, 2015:

Agreed. It's kinda stupid to try to compile code after return.

Fixed.

jonasschnelli force-pushed on May 13, 2015

laanwj merged this on May 14, 2015

laanwj closed this on May 14, 2015

laanwj referenced this in commit a538126a8c on May 14, 2015

laanwj commented at 7:19 AM on May 15, 2015: member

Posthumous nit: the warning icon doesn't get the color of the other single-color icons, it is black even on Linux

jonasschnelli commented at 7:34 AM on May 15, 2015: contributor

Oh. Right. Will fix this.

luke-jr commented at 4:23 PM on May 15, 2015: member

@laanwj I wouldn't expect it to? Looks like it really ought to be a considered part of the text now. :)

laanwj commented at 7:30 AM on May 16, 2015: member

@luke-jr Fine with me. But then it should color with the text, currently it is always black.

luke-jr commented at 7:36 AM on May 16, 2015: member

Oh, I forgot sometimes text isn't black :)

I wonder if we should just make a font for our single-colour icons at this point? (assuming Qt lets you use Unicode characters for icons... which it might not)

laanwj commented at 7:54 AM on May 16, 2015: member

Using a font for the icons would be a neat and modern solution. Could put the icons in the Private Use Area, and use them accordingly in (rich)text. I also proposed it in the beginning when the single-color-icons were introduced. Technically it could be somewhat more of a challenge, though - I don't know Qt-Custom-Font-Handling specifics. No desperate need though, that's the realm of 'far future would be nice'. E.g. no more custom code would be needed to color the icons.

jonasschnelli commented at 7:09 AM on May 17, 2015: contributor

Most of the "new" icons are available as font (typicon font). But handling a font on three platform is probably much more complex the handling PNG files. A font would be really nice because it's vector based. On the other hand it could introduce antialiasing issues (blurring) different margin/paddings on different systems. IMO a font would probably add more issues then I potentially could solve.

MarcoFalke locked this on Sep 8, 2021