[6] Market Depth Redesign

[gibbsfromncis]

This issue was a part of #1484 and was moved to separate issue

Suggestions for use design from GDEX:

[gibbsfromncis]

@wmbutler do you want to use different Chart library or just redesign this chart?

If redesign only I suppose we won't get much changes - only colors and tooltip. Or these changes exactly what you want to get in result of this task?

[MarkoPaasila]

I would like logarithmic horizontal scale. Otherwise orders get cramped on the left side and look very deep, and the right side gets stretched and looks thin.

It would also be nice if you could zoom in or out

[wmbutler]

Are we suggesting that we rebuild this from scratch or use some existing software? Is it realistic to have this completed in July? I view this as a pretty low priority unless you are jus tplanning to style the existing market depth chart.

[gibbsfromncis]

@wmbutler It was a part of Exchange redesign (I was assigned to task about Trading View restyle and idk why Market Depth restyling was included too). I'm not sure there is any reason restyling it without replacing it for more nice library. Lets unassign myself and put it for later. What do you think?

[startailcoon]

I think we have more present needs at the moment, as the current market depth chart works. As it needs some hands-on I think it would be better to find a nice library later on that does this in a really nice manner, and that would take some implementation time.

Maybe adding this to a task later, as I still think we should replace it at some point.

[gibbsfromncis]

@startailcoon agreed.

[kapeer42]

Can I claim this? Won't be able to deliver for 180919 though.

[gibbsfromncis]

@kapeer42 please make sure you have all information needed before claim this

[wmbutler]

@kapeer42 important for you to work closely with @startailcoon and @gibbsfromncis for UX with ANT.

[kapeer42]

I destroyed my computer by accident, couldn't finish the work for today, my bad. Could not comment before today too, sorry for the short notice :( Will resume work as soon as I recover (a couple of days tops probably).

[startailcoon]

Should we remove your assignment on this @kapeer42 , or will you have it ready for next sprint of 2018-10-17?

[kapeer42]

@startailcoon It will be ready

[kapeer42]

Work in progress:

Using the Victory library

[wmbutler]

Looking pretty awesome. One suggestion:

• [ ] Decrease the banner size by 50%

I think you can release this with that change in place.

[svk31]

I'm not a fan of adding yet another plotting library without also removing Highcharts. If we replace Highcharts with Victory in the depth chart, we should also add issues to do the same for the three remaining components that use Highcharts:

• AccountTreeMap (might need to just be removed entirely)
• BlockTimeChart
• TransactionChart

[kapeer42]

Still work in progress (banner size not addressed yet)

Zoom demo:

@wmbutler The ANT buttons seems kinda out of place IMO. Should I mimic the initial issue buttons style?

@MarkoPaasila I think that rendering a logarithmic price axis while keeping the chart centered on the mid market price + handling zoom levels won't be a trivial thing, if this is still required I think we should open a new issue for it.

[wmbutler]

• @kapeeer everything will be ANT eventually so you may as well keep them as is. @gibbsfromncis any thoughts here?
• related to @svk31 is there a reason you didn't just use highcharts? It would be nice to standardize on one library. Would it make more sense to rewrite the others or to move this on to highcharts. I'm assuming there were limitations with highcharts that made this difficulty to implement.

[kapeer42]

@wmbutler highcharts requires a paid license if I'm not mistaken

[wmbutler]

Looks like we'd qualify as a not for profit.

[kapeer42]

Oh well, my mistake, should have read more :(

I guess I will scrap the victory implementation and just update the current highcharts one.

Also there was these comments @startailcoon

I think it would be better to find a nice library later on

and

@gibbsfromncis

I'm not sure there is any reason restyling it without replacing it for more nice library

[wmbutler]

@kapeer42 What would you like to do? Happy to add the hours to improve it. Let us know.

[svk31]

I'm fine with moving everything to victory since you've already made good progress on the depth chart, the other components are non-essential anyway imo and apart from the tree map quite easy to reimplement.

I'd be interested to know how big it is though compared to highcharts.

[kapeer42]

@svk31 Done on master branch:

Bundle:

victory + new depth chart code adds roughly 300kB to the production bundle's javascript.

It's roughly 600kB when comparing the whole bundle (including .map, etc..).

These two estimates are done while keeping react-highcharts and the old depth chart code.

Full bundle details, if you want to compare individual files (for example exchange.js increased by 8k): Without victory: https://ptpb.pw/godI With it: https://ptpb.pw/fLSB

cost-of-modules report:

┌─────────────────────────────┬──────────────┬─────────┐
│ name                        │ children     │ size    │
├─────────────────────────────┼──────────────┼─────────┤
│ bitshares-ui-style-guide    │ 365          │ 193.24M │
├─────────────────────────────┼──────────────┼─────────┤
│ victory                     │ 131          │ 124.57M │
├─────────────────────────────┼──────────────┼─────────┤
│ bitsharesjs                 │ 43           │ 59.00M  │
├─────────────────────────────┼──────────────┼─────────┤
│ react-highcharts            │ 1            │ 28.38M  │
├─────────────────────────────┼──────────────┼─────────┤
│ intl                        │ 0            │ 21.20M  │
├─────────────────────────────┼──────────────┼─────────┤
│ react-datepicker2           │ 19           │ 14.27M  │
├─────────────────────────────┼──────────────┼─────────┤
│ react-intl                  │ 6            │ 14.01M  │
├─────────────────────────────┼──────────────┼─────────┤
│ steem-js-api                │ 16           │ 7.44M   │
├─────────────────────────────┼──────────────┼─────────┤
│ bitshares-report            │ 10           │ 5.61M   │
├─────────────────────────────┼──────────────┼─────────┤
│ react-foundation-apps       │ 8            │ 4.88M   │
├─────────────────────────────┼──────────────┼─────────┤

[kapeer42]

@svk31 @wmbutler I don't feel comfortable making a decision over a library integration but since no answer and sprint ends tomorrow. I'll finish the victory implementation and port the existing charts.

[startailcoon]

The preview you post looks good imho @kapeer42 I'll need some review by @svk31 to sign this of.

cc @wmbutler

[wmbutler]

@kapeer42 ?

[wmbutler]

@kapeer42 can you submit a PR?

[startailcoon]

Submitted PR needs review

[startailcoon]

@sschiessl-bcp I can't see that we an open PR for this issue anymore, was the PR rejected?

[sschiessl-bcp]

@sschiessl-bcp I can't see that we an open PR for this issue anymore, was the PR rejected?

Not that I know, can you see if you can find the old PR? Or possibly the branch in kapeers repo for it?

[startailcoon]

Open for claims

[OpenLedgerApp]

@startailcoon, do you want that depth chart will be redesigned with victory library and all charts written with react-highcharts library will be ported to victory?

[sschiessl-bcp]

I'd rather check first if kapeer still has this, and also wait for @gibbsfromncis regarding new UX before tackling this.