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

Improve facts and figures #5156

Merged
merged 26 commits into from
Feb 18, 2021

Conversation

chimp1984
Copy link
Contributor

@chimp1984
Copy link
Contributor Author

chimp1984 commented Feb 5, 2021

UPDATE:

  • Added total issuance and total burned. Similar like in current chart but adjusted with historical data from Github.
  • Added interval selector so one can get numbers of year, months, week and day

Screen Shot 2021-02-05 at 18 46 16

Screen Shot 2021-02-05 at 18 44 07

Screen Shot 2021-02-05 at 18 45 15

Screen Shot 2021-02-05 at 18 44 55

@chimp1984
Copy link
Contributor Author

chimp1984 commented Feb 5, 2021

Some notes:

  • To fix the mixed comp requests and reimbursements in the earlier months @MwithM provided static data derived from Github data.
  • To be able to combine the data I needed to map it to the beginning of calender month. Using cycles instead is likely possible but more effort. UPDATE: Added a timeinterval selector.
  • When using the timeline navigation the data in the textfields is updated as well to the filtered time period. It uses the same timeinterval than the chart.
  • To show the time raster (months) in the timeline navigation might be nice. Also to snap to the months selection would be nice.
  • Time timeline navigation would need a bit css magic to get a arrow handle to make it more clear its a naviation element.
  • The attempt to extract the BTC fees from subracting reimbursement from proof of burn did not work well as there are periods where that would be negative. There is quite some delay between those 2 events.
  • Adding volume or price would not work as it would require an additional y-axis and that is not supported by the JavaFx charts and would require quite some extra effort.
  • I am not sure if we should add 2 accumulated series: bsq-trade-fee + proof of burn and comp requests + reimbursement request. That would be then similar to the old version. As the BTC fee part is missing I think it would give an incorrect impression, but might still be useful. UPDATE: Is now added.
  • I would like to get the BTC fees in, but I think that is not trivial as we don't have natively those data. Once we have generated the data from blockchain explorer sources we can have a look if adding historical data would make sense or if it would require API calls to get up-to-date data. UPDATE: I leave that for a future PR.

Any dev who can help me with the css for the navigation timeline? Css is not my forte...

@chimp1984 chimp1984 marked this pull request as ready for review February 6, 2021 00:42
@chimp1984
Copy link
Contributor Author

@ripcurlx @jmacxx Do you have an idea how to add a handle icon to the time navigation?

@chimp1984
Copy link
Contributor Author

Will add a new BSQ price chart and trade volume as well, so converted back to draft.

@chimp1984 chimp1984 marked this pull request as ready for review February 8, 2021 22:52
@chimp1984
Copy link
Contributor Author

Added price and volume charts.

Screen Shot 2021-02-08 at 17 53 48

@chimp1984
Copy link
Contributor Author

Please ignore the codacy complaints.

@cbeams
Copy link
Member

cbeams commented Feb 9, 2021

Looking great!

@cbeams
Copy link
Member

cbeams commented Feb 9, 2021

I just tried this out locally, and it's a great improvement all around. Here are some areas for improvement I noticed:

  • It would be good to tighten up the vertical whitespace a bit. As is, I can just barely fit both charts on my 13" MacBook screen:

image

  • It wasn't immediately obvious to me that the slider bar was actually a slider bar and that I could grab the handles on either side. I don't know what would improve this, just noting it. Great to have it in any case, though.
  • It would be good to be able to mouse over the points on the graph and get an overlay showing the specific date, specific price / volume values, etc as is possible in the existing column chart on the main Market > Trades screen
  • Speaking of that, I'm not sure why we're using line charts here in the DAO > Facts & Figures > Dashboard screen and using a column chart on the Market > Trades screen. Seems unnecessarily inconsistent
  • In both charts, it would be better to use radio buttons instead of the current toggle switches. Toggle switches suggest that it's possible to toggle more than one to the "on" position at a time, but it's not. They are in fact mutually exclusive, more like a typical radio button experience.
  • One weird effect of having these three toggle switches is that you can turn them all off and see an empty chart.
  • On the first chart (the BSQ price chart), it's nice to be able to see the BTC/USD price, but it's weird to see it alone, because it has nothing on its own to do with the BSQ price, and that's what this chart is supposed to be about. It would be better if the BTC/USD price were an overlay on the BSQ price, whether the BSQ price is being charted in BTC or USD terms. So ideally, we would see two radio buttons that allows the user to switch between displaying BSQ/USD price and BSQ/BTC price, and then we would see a single toggle that allows for displaying the BTC/USD price as an overlay.
  • coming back to the point about using column charts consistently, it would be good to have the BSQ price expressed as a column chart, and then to have the optional BTC/USD price expressed as a line chart overlay. This is a common visual approach, and is usually called a "combo chart".
  • coming back to ways to tighten up the vertical space, one of the things that threw me off initially was seeing the same date range duplicated on both charts. I understand now why this is necessary given the way the date slider is implemented, but perhaps we could improve the UX of narrowing down the date and tighten up vertical space by removing the date slider and the second row of dates entirely, in favor of a simpler date range picker experience like the ones shown below:

From https://finance.yahoo.com/quote/BTC-USD/chart?p=BTC-USD
image

From the Apple Stocks app:
image

  • Also, on the DAO > Facts & Figures > BSQ supply screen, there's a typo. "Total issues BSQ" should read "Total issued BSQ":

image

@chimp1984, I realize that most of these points I've brought up here are UI / UX polishing stuff that you'd probably rather not spend time on. Perhaps we could ship these changes more or less as-is, and the @bisq-network/design-maintainers could shop these tweaks around as some relatively low-hanging fruit "good first issue" candidates. Probably worth moving many of them into one ore more separate issues, so that this PR can get merged more quickly.

@cbeams
Copy link
Member

cbeams commented Feb 9, 2021

Another way to save a ton of vertical space and provide a more common financial chart user experience would be to combine the volume and price charts as seen above in the yahoo finance screenshot. This could be done in a more or less sophisticated way, and would probably also be its own dedicated issue, but thought I'd mention it here. The same thinking would apply to unifying the currently split price and volume charts on the Market > Trades screen as well.

@chimp1984
Copy link
Contributor Author

Hi @cbeams
Thanks for the feedback. As you mentioned yourself, I tried to to limit the effort/scope of that work, so my goal was to get the charts more usable for analysing our financial data. To finetune UI will take quite some more time and I will leave that for a dedicated UI dev who has already good experience with charting.

Here are some replies:

It would be good to tighten up the vertical whitespace a bit

Yes will try to shink all a bit and make chart height smaller.

It wasn't immediately obvious to me that the slider bar was actually a slider bar

Yes thats my main pain point as well and I asked if anyone can help here. CSS is not my strength and I I guess it requires some hacks as most of those detail nodes are not accessible via the chart API.

It would be good to be able to mouse over the points on the graph and get an overlay showing the specific date, specific price / volume values

If there are not too many data points (e.g. select month) then there are tooltips. It turns off the symbol nodes in case too many data points get displayed, and so the tooltips.

Speaking of that, I'm not sure why we're using line charts here in the DAO > Facts & Figures > Dashboard screen and using a column chart on the Market > Trades screen. Seems unnecessarily inconsistent

use radio buttons

Agree.

BTC/USD price as an overlay
Would again required multiple y-axis.

Yes agree. I had a quick look to the market charts but they have quite a different implementation and custom bar nodes. If using default bars it requires category series and would have not worked as smooth with the time intervals. Can be all done but would take anothre few days work.

in favor of a simpler date range picker experience

I think the time navigatio is one of the most useful feature and was a main reason why I started on that. I want to be able to zoom into any historical date period.

combine the volume and price charts

I wanted to do that first but the JavaFx charts are very basic and lack support for multiple y-axis, so that would be a bigger effort to extend charts and implement that.

There is one missing feature I will add to that PR: To update the text fields with total burned BSQ, total issues/reimbured... according to the time period selection. I think its useful to be able to see what was total BSQ fees in Aug 2019 for instance.

Additional to your suggestions I think it might be useful to have an exact date picker to select the data range. I had that initially but it was redundant to the time slider and I dropped it then. But it would help to get the numbers more exactly from the text fields (see above open feature). Now one does not see the exact selected time period and need to try to get close to the desired period. I did not find a good solution to not overload the chart with redundancy, but I think there would be a solution to get that feature. But again leave it to other devs...

@chimp1984
Copy link
Contributor Author

chimp1984 commented Feb 10, 2021

Implemented update of the values in the text fields at timeline filter change and added a tooltip for the from-to dates as well as added total volume text fields which are sensitive to timeline filter, so one can get total volume of selected period.

The tooltip for the from-to date is not great from the design. Leave it to designers/UI devs to improve that.

Screen Shot 2021-02-10 at 01 11 52

Screen Shot 2021-02-10 at 01 06 46

Screen Shot 2021-02-10 at 01 41 19

@chimp1984
Copy link
Contributor Author

Added average BSQ price for timeperiod selected in chart.

Screen Shot 2021-02-10 at 22 31 53

It might be good to show also the from-to data in the text field descriptions. I leave that for other devs to fine tune...

One might argue that those fields are getting too much and I partially agree. But at the moment we lack on good tools to get a better understanding of Bisq revenue and I think it can be useful (I just needed it today for the discussion about the BSQ rate for compensation), so I think its better to have a bit of overloaded UI for now and fine tune later to the minimum required once we have a more clear picture.
The charts is also getting a bit slow due lot of data churning going on. Some areas specially the slow access to block time will be fixed with another PR.

@cbeams
Copy link
Member

cbeams commented Feb 15, 2021

After spending more time with them, I'm really liking the new charts. Very useful, and I see now how the time slider is critically important to being able to drill down and get detailed information.

A couple additional, hopefully minor feedback issues:

  • The state of the timeline slider is not persisted between view changes. So if I select a date range in DAO > Facts and Figures > Dashboard on one or both charts, and then switch to a different view, say Market > Trades and then switch back to DAO > Facts and Figures > Dashboard, my timeline slider gets reset to the default range.
  • The BSQ/USD toggle should be labeled USD/BSQ, because what's actually being charted is "the number of USD per 1 BSQ", not the other way around. As it reads right now, it's confusing. I realize this isn't the only place in the UI that we use this reverse labeling, but perhaps we could at least do it correctly here and then change the other uses later.

@chimp1984
Copy link
Contributor Author

chimp1984 commented Feb 16, 2021

Thanks for your feedback!

The state of the timeline slider is not persisted

Unfortunately that is likely not that trivial. I have reset it to avoid some weird issues with the custom series colors, tooltips, etc. Resetting all made all easier, though I agree it would be nice to have it.

The BSQ/USD toggle should be labeled USD/BSQ, because what's actually being charted is "the number of USD per 1 BSQ", not the other way around.

That is what logics say, but financial markets decided to swap that the other way. See the fiat currency pairs like BTC/USD. In fact it was swapped in earlier days but then traders complained that we not follow standards. We failed to estaablish a new standard ;-)

See: https://www.bitstamp.net/ or https://www.marketwatch.com/investing/cryptocurrency/btcusd

@Conza88
Copy link

Conza88 commented Feb 17, 2021

Out of interest, how 'heavy' / 'draining' / 'memory intensive' are these charts?
Big impact, small, marginal?

Forgive my ignorance, all look epic and looking forward to using.

@ripcurlx
Copy link
Contributor

@chimp1984 Although the charts are not 100% final I think they provide already value to the user so I'll review them as is and merge them for our upcoming release if nothing specific pops up. I'll solve the conflict and push it directly to this branch (if git lfs is not stopping me to do so)

@chimp1984
Copy link
Contributor Author

@ripcurlx Merged and rebased on current master

Copy link
Contributor

@ripcurlx ripcurlx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK - Tested it on Mainnet.

I'm not super happy with the usage of the toggle buttons, behaving as radio buttons in one and like a checkbox in the other chart. But as mentioned in the comment already I do think it is already a great additional value for our users so 👍

@ripcurlx ripcurlx merged commit 14ae0c3 into bisq-network:master Feb 18, 2021
@ripcurlx ripcurlx added this to the v1.5.7 milestone Feb 18, 2021
@chimp1984 chimp1984 deleted the improve-facts-and-figures branch March 14, 2021 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants