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

Reduce gap between candles #909

Closed
4 tasks done
barnabee opened this issue Aug 15, 2023 · 10 comments · Fixed by #913, vegaprotocol/frontend-monorepo#4753, #917 or vegaprotocol/frontend-monorepo#5023
Closed
4 tasks done
Assignees
Labels

Comments

@barnabee
Copy link
Member

barnabee commented Aug 15, 2023

Story

As a trader
I want the gaps between candles to be a as small as possible
So that I can see more candles for a given interval

Acceptance Criteria

  • There should be a min gap when zoomed out
  • There should be a max gap when zoomed in
  • The gap should scale between these based on zoom level
  • Ensure that the studies underneath the main candles plot align with the candles
@barnabee
Copy link
Member Author

These screenshots are all default views at the same resolution:

Screenshot 2023-08-24 at 13 01 23 Screenshot 2023-08-24 at 13 03 11 Screenshot 2023-08-24 at 13 03 38 Screenshot 2023-08-24 at 13 06 41

@JonRay15
Copy link

I would like to also point out I dont like the new colours...

@Osthumus
Copy link

Screenshot 2023-08-24 at 13 01 23

The volume section is way bigger than necessary and not aesthetic. Also the colour saturation seems to be too high (not sure if that's a bad thing).

@barnabee
Copy link
Member Author

barnabee commented Aug 29, 2023

I was making some screenshots today— I think it'd be great for the initial chart setup to perhaps look something like this (all supported already, just setting zoom and sizes and what's enabled), thoughts?

Screenshot 2023-08-29 at 16 19 15

@Osthumus
Copy link

Yes, looks much cleaner. Current size of indicator is too chunky and overwhelming.
User can change it anyways if they wanted bigger size.

@barnabee
Copy link
Member Author

See these examples for the candle/bar spacing issue in Pennant. In short, candles should be a fixed spacing apart (looks like 1 or 2 pixels?) regardless of zoom, but Pennant grows the gaps as it grows the bars.

Screenshot 2023-08-29 at 23 31 35 Screenshot 2023-08-29 at 23 30 30

@mattrussell36 mattrussell36 transferred this issue from vegaprotocol/frontend-monorepo Sep 5, 2023
@macqbat macqbat self-assigned this Sep 5, 2023
@mattrussell36 mattrussell36 changed the title Improve default chart settings Reduce gap between candles Sep 5, 2023
@macqbat macqbat linked a pull request Sep 6, 2023 that will close this issue
@JonRay15
Copy link

Barney requested to widen the starting gap slightly ... @macqbat can do this with a param.

@JonRay15
Copy link

Re-opening this until we get the full fix in

@JonRay15
Copy link

JonRay15 commented Oct 4, 2023

I think after review with Barney the conclusion is what we actually need is:

  • A max gap when zoomed in
  • A min gap when zoomed out
  • Linear scaling based on zoom in between

Can we achieve this?

@macqbat
Copy link
Contributor

macqbat commented Oct 11, 2023

I think after review with Barney the conclusion is what we actually need is:

  • A max gap when zoomed in
  • A min gap when zoomed out
  • Linear scaling based on zoom in between

Can we achieve this?

I've added new param which determines max gap in pixels (default 2px). Next to the previous parameter, which determines the size of the space in relation to the width of the chart bar, this new one gives us possibility to fully control over dimmension of the gap. This dimmension will be always the smaller of these two. So in high zoomed in chart we will always have max only 2px (or other value given as param) and bars will stop to diverge.
Since everything on candles chart are keep in time units (milisecond), pixels are not really strictly calculated, but converted to miliseconds, hence they could have not always the same dimension like pixels on the page, but difference should be not relevant

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment