bitshares / bitshares-ui

Fully featured Graphical User Interface / Reference Wallet for the BitShares Blockchain
https://wallet.bitshares.org
MIT License
518 stars 570 forks source link

[react-cat][2.5]Portfolio Price (BTS) column invertible #2870

Open froooze opened 5 years ago

froooze commented 5 years ago

Is your feature request related to a problem? Please describe. The Price (BTS) column in the Portfolio tab does not support inverting the price by click, like in Open Orders

Describe the solution you'd like Make column clickable to invert price.

react-cat commented 5 years ago

@startailcoon I'd like to take this task, but it seems to me that it's already done AwesomeScreenshot-BitShares-Account-react-cat-2019-07-13-18-07-70 AwesomeScreenshot-BitShares-Account-react-cat-2019-07-13-18-07-52

froooze commented 5 years ago

@react-cat : I mean the price with switched quote and base, not the order (like in Open Orders).

react-cat commented 5 years ago

@froooze Okay, I got it. I need a little more description to fully understand you.You want to have an ability to select in which currency see the price by clicking on this currency ? Like after clicking on BTC you see a column price(BTS) changed to price(BTC) ?

sschiessl-bcp commented 5 years ago

See here e.g. https://wallet.bitshares.org/#/asset/USD image

sschiessl-bcp commented 5 years ago

Depending on the use case and UX at the panel you can either

a) directly invert without the popup b) show the popup on invert

froooze commented 5 years ago

Version a) like in Open Orders (not grouped)

react-cat commented 5 years ago

@startailcoon I'd like to take this task. I will use existing component (FormattedPrice) and it has few cons. It doesn't support invert by click on price (in case symbols are hidden), only by click on button in popup. Inverting by click on price cell (not row like in open orders) should be written by myself. Also method shouldComponentUpdate should be supplemented with additional conditions and part of project needs to be refactored a little.

Also I'd like to suggest next thing:

before invert: AwesomeScreenshot-BitShares-3-1-190618-1-2019-07-22-16-07-34

after: AwesomeScreenshot-BitShares-Account-react-cat-2019-07-22-16-07-72

As far as component use market directions that are stored in state, that inverted price could stay inverted for very log terms and it might be a little confusing. After a while you might be wondering what prices were inverted and what are not, so I suggest to take one more part of FormattedPrice component logic and add symbols to prices that was inverted. Here how it would be look like:

AwesomeScreenshot-BitShares-3-1-190731-rc1-2019-07-22-16-07-11

Due to all things above, my estimate is 6 hours.

startailcoon commented 5 years ago

Inverting the price on the portfolio column would be very confusing. The price column is based on the settings selected asset, in the example above it's BTS. Inverting just one asset seems illogical.

I have two suggestions for making this possible, and still be logical for users.

  1. We make it possible to switch Quote and Base price calculations. This would make all assets in the portfolio be inverted. Either they are all inverted or not.
  2. We add another column, or add it inside (), to the table that also views the inverted price.

This needs more discussion before we decide to change something.

react-cat commented 5 years ago

@startailcoon

  1. In this case we should also add a button to invert it back to normal. It would be much simpler to invert it either they are all inverted or not and just add some sign to column header whether prices inverted or not.

  2. Is this really so common case that someone needs to see inverted prices that we need to add a column ? Cause in case if most users would find this column useless for them, it would just add some mess to the UI.

On my opinion, inverting any asset price is aimed for some exact asset, so in those rare cases when someone would need to take a look at inverted price, I think he would like to look at some exact asset and so inverting just one asset will be convenient to use.

react-cat commented 5 years ago

@startailcoon Also there is a problem with component that we are supposed to use here. It use settings that shared across the app, so inverting price with help of that component causes an invert in open orders page for the same market. For example if you invert BTS_CNY market and you have an open order to buy\sell for the same market, it will invert it on open orders page too.

So we can end up with your second solution and create a new column where prices would be permanently inverted and it wouldn't cause a bug that described above.

I'm still interested in taking that task, so let me know if that solution fits and I can start

startailcoon commented 5 years ago

@react-cat good catch. It would indeed be problematic to use the same component, didn't think about that either until you wrote about it.

As I mentioned earlier, I think that we could do it with a new column, perhaps even a value within () on the current column, but that could perhaps also be confusing.

My suggestion would be that we add a toggle to enable or disable the view of inverted prices. If you have any other ideas, I'm interested.

I've assigned the issue to you for 1.5.

react-cat commented 5 years ago

@startailcoon It depends on rareness of the case when user need to see inverted price, so in case it is a common case we can add a column with header "Inverted Price". Otherwise, if this case is rare, this would just add some mess to UI\UX. So in second case adding a button that will invert prices or toggle render of inverted price column would be a better approach.

I haven't enough experience in using bitshares as a trader, so choose one of the following:

  1. Permanently render column "Inverted Prices" next to price column
  2. Add a button

    2.1. "Invert Prices" which would invert prices in "Price" column and change header of that column to "Price (asset) inverted".

    2.2. "Show Inverted prices" which would toggle render of a column from the point 1

startailcoon commented 5 years ago

I just realized we have the new column customization on the dashboard that we should use.

We can add the column as an option there. If user toggles the "Inverted Prices" it will be next to the "Price" column.

image

react-cat commented 5 years ago

@startailcoon actually I spend more time than was estimated and it would be great if you raise estimate at least for 1 hour

sschiessl-bcp commented 5 years ago

Is there a tldr; summary for this?

react-cat commented 5 years ago

@sschiessl-bcp you mean a final decision that we come up with ? Or a whole work that I done on that task ?

To not waste time I would answer on first question. We decide to add a new column with inverted prices, which could be customized via new feature of portfolio table, so anyone could hide/show that column depending on his needs. And also decided not to use proposed component cause it would cause a bug on open orders page

sschiessl-bcp commented 5 years ago

@sschiessl-bcp you mean a final decision that we come up with ? Or a whole work that I done on that task ?

To not waste time I would answer on first question. We decide to add a new column with inverted prices, which could be customized via new feature of portfolio table, so anyone could hide/show that column depending on his needs. And also decided not to use proposed component cause it would cause a bug on open orders page

What's the bug in open orders?

sschiessl-bcp commented 5 years ago

Current proposed solution is this image

Does that satisfy your needs @froooze ? I find it borderline confusing because every line has different price without indication

react-cat commented 5 years ago

@sschiessl-bcp There is a problem with component that we was supposed to use here. It use settings that shared across the app, so inverting price with help of that component causes an invert in open orders page for the same market. For example if you invert BTS_CNY market and you have an open order to buy\sell for the same market, it will invert it on open orders page too and vice-versa.

react-cat commented 5 years ago

@froooze Solution with additional column with inverted prices that could be hidden by your wish satisfy your needs ?

react-cat commented 5 years ago

@sschiessl-bcp Since It's been 18 days without @froooze reply, could we close it ?

sschiessl-bcp commented 5 years ago

I'm still not convinced this is needed. Would await more input, will close PR for now.

About payment, please talk to @startailcoon .