[Qt] Rework of overview page #479

Warrows · 2018-01-06T12:37:13Z

Here is a proposal for some modifications on the overview page. Thanks to @Mrs-X work it got some really nice improvements but I believe it still holds a few flaws. So here are my proposed changes:

-First, put the combined balance on top. Users want to know how much they have, and how much they can spend right away. Stop.
-For the same reason, remove emphasis from piv balance and put it to combined.
-Move PIVX/zPIV percentages to the areas labels. (piv one was in front of unlocked which was technically false)
-Change sections names for more consistency (plurals, zPIV instead of zerocoin)
-Move locked to the PIVX balance instead of combined (you can't lock zPIV, can you ?)
-Remove redundant informations like zPIV and PIVX balances in both their own areas and the combined balance area.
-Reverse zPIV balances order to be consistent with the two other sections.
-Have a clear available/total policy for each section.

Here is a small before/after comparison.

I believe all previously available information are still in here.

presstab · 2018-01-06T17:17:55Z

I like the concept here. utACK. Need to confirm that this reflects all balances correctly.

Mrs-X

I like the concept as well, and I guess nobody will really miss the "Unlocked" balance.
I wonder how possible Watched-Balances will look like, but we'll find out soon enough.

ACK 32fdefd

Fuzzbawls

Some style changes noted.

Overall Concept ACK, needs some thorough testing, especially with watch-only cases.

Fuzzbawls · 2018-01-07T03:56:56Z

src/qt/forms/overviewpage.ui

Inline HTML shouldn't be used in strings

Yep, missed this one.
Please remove the HTML code and just increase the font size for this label.
a) translation is easier, and it's b) also easier to change the font properties in the CSS file if needed.

Yeah I don't know where this comes from. Probably a mistake with qt creator. Will remove.

Fuzzbawls · 2018-01-07T03:57:14Z

src/qt/forms/overviewpage.ui

trailing whitespace

Text editor fizzling, will remove.

Fuzzbawls · 2018-01-07T03:58:37Z

src/qt/forms/overviewpage.ui

string should be PIV Balances

Will change back.

Fuzzbawls · 2018-01-07T04:01:28Z

src/qt/forms/overviewpage.ui

should be <string notr="true">0.000 000 00 PIV</string> to prevent sending this to transifex for translation

Fuzzbawls · 2018-01-07T04:02:58Z

src/qt/forms/overviewpage.ui

should keep the notr="true" here

It was not removed. I just moved parts around, it probably went something like line 527 or 712. Will add.

Fuzzbawls · 2018-01-07T04:07:06Z

src/qt/overviewpage.cpp

NIT: unnecessary whitespace in function parameters

About this, I will remove them for the sake of file consistency. But there is a serious question. I personally find this area more readable with this whitespaces. Other areas would benefit from the same treatment in my view. They obviously have no influence on compiled code.
But is there coding style guidelines and where can they be found if so?

Fuzzbawls · 2018-01-07T04:07:14Z

src/qt/overviewpage.cpp

NIT: unnecessary whitespace in function parameters

Fuzzbawls · 2018-01-07T04:07:23Z

src/qt/overviewpage.cpp

NIT: unnecessary whitespace in function parameters

Fuzzbawls · 2018-01-07T04:07:39Z

src/qt/overviewpage.cpp

NIT: unnecessary whitespace in function parameters

Fuzzbawls · 2018-01-07T04:11:12Z

src/qt/forms/overviewpage.ui

should probably add notr="true" to the <string> element here.

I am pretty sure this was not everywhere before. Will add of course.

Warrows · 2018-01-07T11:38:15Z

Thanks to thorough review by @Fuzzbawls I got to make some changes.
I added a few "notr" tags which where absent in the original file.
Fixed one or two editor fizzles.
And I rolled back some coding style changes regarding whitespaces. But I am left wondering if and where we have coding style guidelines.

Mrs-X · 2018-01-07T17:25:45Z

where we have coding style guidelines.

Not complete, but gets you started:
https://github.com/PIVX-Project/PIVX/blob/master/doc/developer-notes.md

veramispotato · 2018-01-08T00:11:52Z

Much cleaner, thanks! In last image,

Suggest auto-hiding the following if they are 0: Pending, Immature, Unconfimed, Locked.

Suggest removing Total and Available if they equal the same number, and only show the number.

Correction: PIVX Balances should be PIV Balance, or suggest changing to PIV
Correction: zPIV Balances should be zPIV Balance, or suggest changing to zPIV

Warrows · 2018-01-08T18:06:37Z

So yeah 7a8ad53 Implements changes suggested by @wholewheatbiscuits
It also hides PIV and zPIV areas when there are none.
I also made sure that watch-only addresses are working as pointed out by @Fuzzbawls
Here are the latest screenshots:
When opening a new (empty) wallet

When using it like a "normal" user (aka all operations are complete)

And finally when you like fancy stuff like masternodes and watch-only addresses

Obviously no configuration needed whatsoever and everything you own is always visible. I am just hiding empty and/or redundant balances.

turnkit · 2018-01-08T18:45:21Z

Warrows - great improvement.
Three concerns:

tpiv - is this for "test network pivs" or "thousand pivs" or "total pivs?" Either way, it's confusing. (And hopefully mPIVs and uPIVs eventually can get clever arbitrary names like milliPIVs, microPIVs, 'Doriens', 'Warrows', 'PIVpennies' or 'PIVbits', etc. so these units show up in more human friendly names.)
I'd suggest the "simple view" just show two numbers: avail. PIVs & avail. zPIVs.
when there is not "detailed view" to be seen, still allow the user a way to twiddle/click an arrowhead to see all the "0.00" entries and their descriptions -- but that by default when they are all zero, that arrow is up/closed / view is not seen.

Warrows · 2018-01-08T20:09:35Z

@turnkit Thanks for your valuable input.

tPIV stands for testnetPIV, no worries. About units, it's an interesting question but not to be solved here. I believe it's a community wide question which will need further discussions.
It might be unclear but there is no "simple view" here. It's one and only one panel which shows only numbers above 0. So if you have no pending operation and no locked up PIV in any way you get what's seen in the second image: Total (which happen to also be available), PIV and zPIV.
I am not sure there is a point in showing all the empty balances. If you are a new user you most likely don't care about them and if you are more a advanced one you probably already know about them and don't want it to cluster your view for nothing.

Anyway you'll see them soon enough if you move funds around. And if they don't show long it's that the network is so fast you don't need to know about them.

turnkit · 2018-01-08T20:18:23Z

Thanks for the positive comments. I would disagree about need to keep the zero balances out of view. By allowing a simple view vs. complete view the users who wish to learn by observation have that opportunity. This especially becomes useful when roll-over help text appears describing what different balances mean. So by allowing a user to "open up" the complete detailed view they are then able to learn before it gets complicated, when there is no frustration over, "why can't I spend all the coin I have?!" when coins that were available stop becoming available, then become available again, only leaving a user to wonder, "what is happening?" In those situations it will be useful for a user to open up the detailed view and look at the metrics that were populated when a coin became immature due to minting, right? If that's not clear, I could probably re-write it so that it make more sense. The basic principle here is that the user should be King/Queen: they should have the ability to get into all the details even when it's not primarily relevant, if they wish to. And primarily they should be able to do so using methods that increase their knowledge and understanding by doing so.

…

On Mon, Jan 8, 2018 at 2:09 PM, Warrows ***@***.***> wrote: @turnkit <https://github.com/turnkit> Thanks for your valuable input. tPIV stands for testnetPIV, no worries. About units, it's an interesting question but not to be solved here. I believe it's a community wide question which will need further discussions. It might be unclear but there is no "simple view" here. It's one and only one panel which shows only numbers above 0. So if you have no pending operation and no locked up PIV in any way you get what's seen in the second image: Total (which happen to also be available), PIV and zPIV. I am not sure there is a point in showing all the empty balances. If you are a new user you most likely don't care about them and if you are more a advanced one you probably already know about them and don't want it to cluster your view for nothing. Anyway you'll see them soon enough if you move funds around. And if they don't show long it's that the network is so fast you don't need to know about them. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#479 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD711yxjfEoQYGlVILqOGT7urMCRiXrRks5tInYCgaJpZM4RVTEW> .

-- David C. Sutherland | (310) 729-6411 (c) | Lindale, Texas

Warrows · 2018-01-08T20:32:31Z

@turnkit I absolutely understand your feeling that the user must be king and I agree with it. The thing is I don't think the average user will open the advanced view if he doesn't need it. And even if he did, there is no way we could stuff enough information in the software. If you want to understand cryptocurrencies you need to go out and educate yourself about them. As much as we would want it, we can't give a full course about blockchain and zPIV minting in-wallet.
And in the case you just want to use the wallet without bothering too much about its internal workings, the fact that the locked coins show up when they are locked, with the much needed tooltip explaining why/how they are locked seems good enough to me.
You say yourself, it becomes useful to read about them when this situations happen (and they will happen, and the balances will be there, no click needed). Before their coins are locked, most users don't want to know what can happen to them. And those who want will learn on our website or youtube.

Warrows · 2018-01-09T14:04:22Z

@presstab Thanks, totally agree. I just followed the existing pattern here, now it's done for all the same looking functions.

Mrs-X · 2018-01-09T19:02:19Z

I'm not 100% sure if completely hiding elements when they are empty is support-wise a good idea.
We can't use that space anyway...

Have you ever tried to guide someone via phone through a problem who sees a different UI?

Warrows · 2018-01-09T20:14:33Z

@Mrs-X I can't say I am 100% sure it's the best idea. But I really like the cleanliness it brings.
I am not sure either about the different UI thing, it's not like hiding a button or putting things elsewhere depending on OS. It's more about putting away empty information. If you try to help someone, you know it's either there or 0.
Maybe let's try to get some support people input on this.
If there is a consensus I can rollback the hiding thing. But I think it does not make much sense to have it for some lines (was existing for pending, and maybe immature in PIV balance) and not others.

turnkit · 2018-01-09T22:37:01Z

I agree. I prefer the idea of two panels or a dashboard with a line between top, minimal info, and bottom, details. My idea is that when the details are zero the section Auto-rolls up, or perhaps just has a green light for “all confirmed/processed/available” whatever. But when coins are unavailable somehow those numbers are highlighted. But still the essential two numbers, available PIV and available zPIV should be set apart and prominent, perhaps in much larger font.

On Tue, Jan 9, 2018 at 1:02 PM Mrs-X ***@***.***> wrote: I'm not 100% sure if completely hiding elements when they are empty is support-wise a good idea. We can't use that space anyway... Have you ever tried to guide someone via phone through a problem who sees a different UI? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#479 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD711w6bKtgLzODZ_6IQLZfj-oCvt4wlks5tI7e-gaJpZM4RVTEW> .

-- Sent from Gmail Mobile

turnkit · 2018-01-09T22:38:32Z

It was very disturbing the first time I saw my balance go down, due to minting and immature coins that were previously mature. So any way the UI can calm the user when the previously available coins disappear to a temporarily unavailable status, it is much better.

On Tue, Jan 9, 2018 at 4:36 PM David Sutherland ***@***.***> wrote: I agree. I prefer the idea of two panels or a dashboard with a line between top, minimal info, and bottom, details. My idea is that when the details are zero the section Auto-rolls up, or perhaps just has a green light for “all confirmed/processed/available” whatever. But when coins are unavailable somehow those numbers are highlighted. But still the essential two numbers, available PIV and available zPIV should be set apart and prominent, perhaps in much larger font. On Tue, Jan 9, 2018 at 1:02 PM Mrs-X ***@***.***> wrote: > I'm not 100% sure if completely hiding elements when they are empty is > support-wise a good idea. > We can't use that space anyway... > > Have you ever tried to guide someone via phone through a problem who sees > a different UI? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#479 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AD711w6bKtgLzODZ_6IQLZfj-oCvt4wlks5tI7e-gaJpZM4RVTEW> > . > -- Sent from Gmail Mobile

-- Sent from Gmail Mobile

Mrs-X · 2018-01-10T05:04:32Z

If there is a consensus I can rollback the hiding thing.

@Warrows What about a new setting "Hide zero balances" to make this optional?
Could be used at other places as well.

Warrows · 2018-01-12T10:59:00Z

@Mrs-X We can add a setting of course. But the point being to have a cleaner overview for newcomers the auto-hiding should be opt-out. Would you agree with that ?
Because in my view it's not a cosmetic change for power users but an effort to make the wallet more accessible.

Mrs-X · 2018-01-13T19:45:52Z

@Warrows I just played around with different options.
I think your first screenshot (only Combined Balance) goes a bit too far, the user, even when he's a beginner, should see that something like PIV and zPIV exist, even when one or both them are zero.

So if a more minimalistic overview should be the default I'd like to see the second screenshot as default one (this one:
https://user-images.githubusercontent.com/835098/34684650-1e1cae60-f4a6-11e7-9769-b4b63fb2864a.png).

If more balances are available they should be automatically visible of course, and users who always want to see everything get a checkbox in the option menu.

veramispotato · 2018-01-13T19:48:41Z

@Mrs-X that's a good point, I didn't think about that. There's going to be edge cases where people think PIV is automatically private, but if they see zPIV next to it on the overview tab, that should make many understand they need zPIV for private transactions. On 1/13/2018 11:45:56 AM, Mrs-X <[email protected]> wrote: @Warrows [https://github.com/warrows] I just played around with different options. I think your first screenshot (only Combined Balance) goes a bit too far, in the user, even when he's a beginner, should see that something like PIV and zPIV exist, even when one or both them are zero. So if a more minimalistic overview should be the default I'd like see the second screenshot (this one: https://user-images.githubusercontent.com/835098/34684650-1e1cae60-f4a6-11e7-9769-b4b63fb2864a.png [https://user-images.githubusercontent.com/835098/34684650-1e1cae60-f4a6-11e7-9769-b4b63fb2864a.png]). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub [#479 (comment)], or mute the thread [https://github.com/notifications/unsubscribe-auth/Aczfkl6Vn0qE89Ks2Dnr09PYsf5pNmF-ks5tKQfygaJpZM4RVTEW].

Warrows · 2018-01-13T22:49:37Z

@Mrs-X You are absolutely right, newcomers must see that there are two separate balances. Maybe hiding everything was a bit extreme.
I implemented the setting as opt-out (checkbox is checked by default, therefore default view is second image) in f751e12

And put back the two main panels as always visible in 778d9f7

Mrs-X

Function wise ACK, but you've messed up the resizing of the screen, see screenshots below.
This needs to be fixed for both the minimal view (if possible, might be tricky to do) and the full/original view.

Before:

After your change:

Warrows · 2018-01-14T23:58:35Z

I think I know exactly what the problem is, I am going to push that tomorrow. Do you want me to rebase and squash some commits by the way ?

Warrows · 2018-01-15T12:43:13Z

@Mrs-X I made the resize work like before. I personally prefer the "fixed size" version. But that's not the point of this PR.

Mrs-X · 2018-01-15T15:23:16Z

I personally prefer the "fixed size" version.

Lots of people specifically asked to have the resizing like this, and it makes sense if we want to make use of bigger fonts to support 4k or 8k displays somewhere in the future.

I'll test later, we should do squashing when everything is ACKed.

Mrs-X · 2018-01-15T16:44:49Z

ACK 👍

When it's squashed and @Fuzzbawls approves his requested changes we'll merge.

Put combined balance on top Removed redundent informations Fixed some inconsistencies Rollback styles changes and errors Hide balances when empty Hide available balances when equal total Fix a few things Add setting to show or hide empty balances in overview Always show PIV/zPIV panels. Hide percentages when balance is 0 Rollback changes in overview page resizing

Mrs-X · 2018-01-15T19:03:40Z

ACK b5f420b

Fuzzbawls · 2018-01-16T06:22:17Z

ACK b5f420b

b5f420b [Refactor] Moved wallet balance computation from header to source file (warrows) 2cd94c7 [UI] Rework of overview page (warrows) Tree-SHA512: 5adecaefe3ca3e60594ecd3b48e65def15ea458a6a1d49b5a38db8f91fd95da0476127a2bd4bfdcfed2c1d6539f01652d8400d4cc30150c2cf68b0f7f21cb903

Sieres · 2018-04-01T16:50:34Z

ACK for the hide zero balances. Balance sections do no collapse like @Warrows examples

Mrs-X approved these changes Jan 6, 2018

View reviewed changes

Fuzzbawls added the GUI label Jan 7, 2018

Fuzzbawls requested changes Jan 7, 2018

View reviewed changes

Warrows force-pushed the ui_fixes branch from 32fdefd to 8224e7f Compare January 7, 2018 11:22

Warrows force-pushed the ui_fixes branch from 8224e7f to 9f4c1cb Compare January 8, 2018 14:10

Warrows mentioned this pull request Jan 8, 2018

[Feature Request] Simplify Balances in Overview UI and add a notifications/announcements section #474

Closed

Mrs-X suggested changes Jan 14, 2018

View reviewed changes

Mrs-X approved these changes Jan 15, 2018

View reviewed changes

Warrows force-pushed the ui_fixes branch from 70381ac to ad62aad Compare January 15, 2018 17:58

Warrows added 2 commits January 15, 2018 19:03

[Refactor] Moved wallet balance computation from header to source file

b5f420b

Warrows force-pushed the ui_fixes branch from ad62aad to b5f420b Compare January 15, 2018 18:10

Fuzzbawls approved these changes Jan 16, 2018

View reviewed changes

Fuzzbawls added this to the 3.1.0 milestone Jan 16, 2018

Fuzzbawls added the Usability label Jan 16, 2018

Fuzzbawls merged commit b5f420b into PIVX-Project:master Jan 16, 2018

Fuzzbawls changed the title ~~[UI] Rework of overview page~~ [Qt] Rework of overview page Apr 30, 2018

Mrs-X mentioned this pull request May 6, 2018

[GUI] Fix wrongly displayed balance on Overview tab #598

Merged

[Qt] Rework of overview page #479

[Qt] Rework of overview page #479

Uh oh!

Conversation

Warrows commented Jan 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

presstab commented Jan 6, 2018

Uh oh!

Mrs-X left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fuzzbawls left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Warrows Jan 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Warrows commented Jan 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mrs-X commented Jan 7, 2018

Uh oh!

veramispotato commented Jan 8, 2018

Uh oh!

Warrows commented Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

turnkit commented Jan 8, 2018

Uh oh!

Warrows commented Jan 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

turnkit commented Jan 8, 2018 via email

Uh oh!

Warrows commented Jan 8, 2018

Uh oh!

Warrows commented Jan 9, 2018

Uh oh!

Mrs-X commented Jan 9, 2018

Uh oh!

Warrows commented Jan 9, 2018

Uh oh!

turnkit commented Jan 9, 2018 via email

Warrows commented Jan 6, 2018 •

edited

Loading

Mrs-X left a comment •

edited

Loading

Warrows Jan 7, 2018 •

edited

Loading

Warrows commented Jan 7, 2018 •

edited

Loading

Warrows commented Jan 8, 2018 •

edited

Loading

Warrows commented Jan 8, 2018 •

edited

Loading

Mrs-X commented Jan 13, 2018 •

edited

Loading

Warrows commented Jan 13, 2018 •

edited

Loading

Mrs-X left a comment •

edited

Loading