-
Notifications
You must be signed in to change notification settings - Fork 622
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
fix: avoid repetitive labels by using tickMinStep #8872
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. The method definitely doesn't cover many cases where you have a format such as .1f for example but parsing that would be difficult so this looks like a good start to cover a lot of cases.
@@ -74,6 +74,9 @@ | |||
"gridScale": "y", | |||
"grid": true, | |||
"tickCount": {"signal": "ceil(width/40)"}, | |||
"tickMinStep": { | |||
"signal": "datetime(2001, 1, 1, 0, 0, 0, 0) - datetime(2001, 0, 1, 0, 0, 0, 0)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we could precompute this so that we don't need a signal here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer using signal given it's not that expensive to calculate once, but it's gonna be more readable than some magic constant that presents duration timestamps.
Co-authored-by: GitHub Actions Bot <vega-actions-bot@users.noreply.github.com>
Fix #8867 - Avoid repetitive labels by using tickMinStep
(replacing #8868 to use tickMinStep as @domoritz suggested)