Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[.5][ariesjia] Change Word and Calculation from "BALANCE" to "VALUE" on Top Bar #374

Closed
landry314 opened this issue Sep 8, 2017 · 16 comments
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Milestone

Comments

@landry314
Copy link

landry314 commented Sep 8, 2017

Ok, here is my argument:

  1. Word: Under the Account tab you have "Balances". Balances are just which assets you hold which exclude assets in open orders and debt. The calculation on the top bar called "BALANCE" DOES include open orders and debt in the calculation. Using the same word is confusing to many people. People might think that they are holding BTS if the Preferred Unit is BTS and it says "BALANCE: XXX BTS" on the top bar even though it is an approximate calculation of the equivalent VALUE in BTS. Why not change the word to "VALUE"?

  2. Calculation: In the "b" tab on the upper left, if you scroll down to Accounts and look on the rightmost column you see it is called "TOTAL VALUE". This is a calculation of balances, orders, and debt and it is appropriately referred to as the account value. This calculation SHOULD be the SAME as the calculation on the upper right but it is not because it lacks a bug as discussed in issue Balance: 0 Displayed at Top When All of an Asset is in Open Order #206. So, why not clone the calculation for "TOTAL VALUE" into the top bar to fix issue Balance: 0 Displayed at Top When All of an Asset is in Open Order #206. They are trying to do the same thing while one is failing and the other is succeeding. I think the calculation is trying to sum the values in the the "Equivalent Value" column in ACCOUNT including open orders but it is failing when all values in Balances are Unknown (because they are too small?). Instead of using this calculation, let's use the one that already works.

This is so simple, change the calculation on the top bar to the same one used in b -> Accounts -> Total Value and rename it to VALUE. This is so easy and makes the program cleaner and clearer.

@wmbutler
Copy link
Contributor

Let's use the label "ACCOUNT VALUE" in the top and under the Overview.

screen shot 2017-09-09 at 7 06 56 pm

screen shot 2017-09-09 at 7 07 59 pm

@wmbutler wmbutler changed the title Change Word and Calculation from "BALANCE" to "VALUE" on Top Bar [.5] Change Word and Calculation from "BALANCE" to "VALUE" on Top Bar Sep 10, 2017
@wmbutler wmbutler added the [3] Feature Classification indicating the addition of novel functionality to the design label Sep 10, 2017
@wmbutler wmbutler added this to the 170914 milestone Sep 10, 2017
@ariesjia
Copy link
Contributor

can i take this issue

@ariesjia ariesjia mentioned this issue Sep 10, 2017
@wmbutler
Copy link
Contributor

Yes, did you change the label in both locations as specified?

@wmbutler wmbutler changed the title [.5] Change Word and Calculation from "BALANCE" to "VALUE" on Top Bar [.5][ariesjia] Change Word and Calculation from "BALANCE" to "VALUE" on Top Bar Sep 10, 2017
@ariesjia
Copy link
Contributor

yes ,i change the all the locations

@landry314
Copy link
Author

landry314 commented Sep 11, 2017

What about the calculation? Did you find the formula used in _ b -> Accounts -> Total Value _ and copy it to the calculation used in the top bar? That formula works when the other fails. Test it out by creating an open order for every bit of an asset for every asset. You should see 0 or "Unknown Value" for every asset in _ Account -> Balances -> Equivalent Value _ and the calculation in the top bar should say "0". It ignores the open orders. Just making sure you got my point above!

ariesjia added a commit to ariesjia/bitshares-ui that referenced this issue Sep 12, 2017
@landry314
Copy link
Author

landry314 commented Sep 12, 2017

This commit only fixes the name and not the calculation bug.

@wmbutler
Copy link
Contributor

It would be helpful if you would delete screenshots that are from older versions of software. Posting outdated bugs takes time for us to troubleshoot.

screen shot 2017-09-12 at 8 20 01 am

@landry314
Copy link
Author

landry314 commented Sep 12, 2017

Ok, I deleted it. Thank you for submitting a newer screenshot, @wmbutler. So, now it says "0 BTS" (or whatever the unit is set in settings) instead of "Unknown value" but the bug is the same. We can't see you have an open order there but I assume you do.

svk31 pushed a commit that referenced this issue Sep 13, 2017
* fix issue 374 ,update text Balance to Account value

* #374 update ACCOUNT VALUE to  Account value
@calvinfroedge
Copy link
Contributor

Closing this issue as it has been merged

@Ashaman-
Copy link
Contributor

Should this issue have been closed, given that the calculation bug has not been fixed yet?

@o5j5vg55bv5hv5j5f8799f9
Copy link

o5j5vg55bv5hv5j5f8799f9 commented Sep 14, 2017

Entire point of this was to point out it's an estimate or rough calculation and not what you literally have. Might actually be way off if one of your asset's markets is not liquid.

Equivalent value and value are two different things. First one implies estimated value (one through calculation and second is just value. the top text doesn't communicate that it isn't meeting goal. I'd almost favor

"EST. VALUE: ~" or "EST. TOTAL: ~" or "EST. BALANCE: ~" or "TOTAL VALUE: ~"

but I see issue is closed so I won't follow up on it again.

@landry314
Copy link
Author

landry314 commented Sep 15, 2017

Just installed the new GUI and the ACCOUNT VALUE calculation bug is still there. It says "ACCOUNT VALUE: 0" because all my assets are in open orders while the TOTAL VALUE for my account in the "b" tab on the upper left is properly calculating my account value... the bug is still there... :-(

Why cant we just use the algorithm that works correctly? I really don't understand. This is a glaring bug.

@Ashaman-
Copy link
Contributor

@o5j5vg55bv5hv5j5f8799f9 I agree that the point of it is an estimate, but it should work consistently. I have seen it say 0 estimated value intermittently as well, and it always scares the shit out of me (and I expect it will continue to do so until the Ledger Nano integration is complete)...

@landry314
Copy link
Author

landry314 commented Sep 16, 2017

Intermittently, @Ashaman-? I guess I need to put steps to reproduce with bullet numbers:

Steps to Reproduce:

  1. Create an open order which is maxed out to take every last unit of your asset out of balances.
  2. Repeat for every asset you have
  3. Notice in the ACCOUNT section in the Equivalent Value column you see "0" for every asset
  4. Look at the ACCOUNT VALUE at the top, it says "Account Value: 0"
  5. Click on the "b" tab on the upper left. Find your active account under Accounts.
  6. Look all the way to the right under the column TOTAL VALUE.
  7. Notice that is does NOT say "0" but rather calculates your value correctly.

Why wouldn't we simply clone the correct calculation to the Account Value on the top? Is there any reason?

@svk31
Copy link
Contributor

svk31 commented Sep 16, 2017

Fixed. It's the same component and calculations being used, but in the header there was a wrapper component that returned 0 if there were no current balances.

@landry314
Copy link
Author

You don't know how happy that makes me! :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[3] Feature Classification indicating the addition of novel functionality to the design
Projects
None yet
Development

No branches or pull requests

7 participants