vegaprotocol / frontend-monorepo

Toolkit for building apps that interact with Vega
https://vega.xyz
MIT License
21 stars 14 forks source link

Set default zoom on Console charts #4727

Closed JonRay15 closed 8 months ago

JonRay15 commented 8 months ago

Barney wants the default zoom and study size to look something like this

Image

barnabee commented 8 months ago

See further examples (and the original text of the ticket) here: https://github.com/vegaprotocol/pennant/issues/909

bglownia commented 8 months ago

I was looking for option to control zoom from outside of CandlestickChart but I could not find any option. @johnwalley please can you advise

barnabee commented 8 months ago

It's not clear to me that we have addressed the default zoom level (candle size) in this PR. Can someone confirm that this will result in a chart with the required candle size regardless of the browser window and chart area width, or that doing so is covered elsewhere?

macqbat commented 8 months ago

Yeah, this needs refactoring and revisiting. I shouldn't set this in "ready for review" state. Number of displayed candles basically plays his role as a zoom trigger, but should also depend on the size of the window. I'll take a look at it again.

barnabee commented 8 months ago

Thanks @macqbat!

We really want to get Pennant / the charts in Console working so that they remember the width (in pixels or whatever) of the candles not the number displayed. You can see that this already happens on resizing the window, it just needs to happen correctly when opening or reopening Console, and we need to make sure the default on first use gives the desired candle width regardless of what size the window opens at.

macqbat commented 8 months ago

Hm, I'm not even able to say it's possible to base everything on bar graph width in pennant library. We have width of bars and x axis as well kept and handled in miliseconds - time unit. D3 library under the hood is doing the same. Reversing this logic, that now the bar graph will be constant and the canvas will adapt seems to me to be not so easy task. :-(

barnabee commented 8 months ago

Hmm– how does it work when the canvas is resized? The bars stay the same size and the axis extents are expanded in that case, so it must somehow know how to do that rather than zoom the bars?

Osthumus commented 8 months ago

Hm, I'm not even able to say it's possible to base everything on bar graph width in pennant library. We have width of bars and x axis as well kept and handled in miliseconds - time unit. D3 library under the hood is doing the same. Reversing this logic, that now the bar graph will be constant and the canvas will adapt seems to me to be not so easy task. :-(

How about a workaround : just set a equation showing relation between the number of desired candle bars to be shown initially and the size of the screen/ trading screen size. Then just work out the number of candles ro be shown based on that equation.

For example: X = y × c

Where: X= number of candle bars to be shown y = the size of the screen (length × height or pixels or whatever) c = some constant you find by hit or trial or experimenting

So the console finds the initial number of bars to be shown based on the equation and then user can change as desired without need of this equation.

Please disregard if it doesn't make sense (most probably it doesn't )

JonRay15 commented 8 months ago

I think that is broadly what @macqbat has implemented in this PR. He's defined a "CANDLES_TO_WIDTH_FACTOR" ... which he's currently set to 0.15 to try to do this.

That change is currently merged and I suggest we review it in testing and see if it achieves what we want it to. If not we can think about further refactoring.

barnabee commented 8 months ago

The default view in testing with the current setting seems to totally remove the gap between candles in at least some cases. Not sure if that's indicative of an issue with the approach or if we just need to refine the setting?

It's also worth checking whether this factor works against the width of the whole chart component or the plot area (i.e. excluding padding and axis labels, etc.). If it's the former that might explain why it doesn't work well in all cases, and perhaps changing it to use the latter would help…

Screenshot 2023-09-12 at 12 03 39