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
Change scale.ticks
back to an array of strings
#4573
Change scale.ticks
back to an array of strings
#4573
Conversation
Internal ticks are now stored as objects in the PRIVATE this._ticks member and must not be accessed directly from outside this class. this.ticks is around for a long time and hasn't been marked as private, so we can't change its structure without unexpected breaking changes. If you need to access the scale ticks, use scale.getTicks() instead.
Thanks. I'll take a look at this one sometime in the next few days |
src/core/core.scale.js
Outdated
me.afterBuildTicks(); | ||
|
||
me.beforeTickToLabelConversion(); | ||
me.convertTicksToLabels(); | ||
labels = me.convertTicksToLabels(ticks) || me.ticks; |
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.
is || me.ticks
correct? Should it be || ticks
?
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.
Yes, it's me.ticks
in case of a sub class that convertTicksToLabels
in place and so modify me.ticks
directly without returning it. I know it's pretty confusing, I will add a comment.
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.
Ok, makes sense 👍
src/core/core.scale.js
Outdated
@@ -162,13 +182,36 @@ module.exports = function(Chart) { | |||
|
|||
// Ticks | |||
me.beforeBuildTicks(); | |||
me.buildTicks(); | |||
ticks = me.buildTicks() || []; |
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.
is changing this to return breaking?
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 hope not: if buildTicks
doesn't return anything, that means it's the old implementation that directly store ticks in this.ticks
. It also means this same object will be used by the old implementation of convertTicksToLabels
(the base one still use me.ticks
).
Here, ticks
is supposed to be an array of objects: if nothing is returned, it's re-synchronized with labels
bellow (and so me.ticks
for old impl.).
@etimberg I added some comments to hopefully make it clearer. If not, let me know :) |
Internal ticks are now stored as objects in the PRIVATE this._ticks member and must not be accessed directly from outside this class. this.ticks is around for a long time and hasn't been marked as private, so we can't change its structure without unexpected breaking changes. If you need to access the scale ticks, use scale.getTicks() instead.
Internal ticks are now stored as objects in the PRIVATE this._ticks member and must not be accessed directly from outside this class. this.ticks is around for a long time and hasn't been marked as private, so we can't change its structure without unexpected breaking changes. If you need to access the scale ticks, use scale.getTicks() instead.
Internal ticks are now stored as objects in the PRIVATE
this._ticks
member and must not be accessed directly from outside this class. this.ticks is around for a long time and hasn't been marked as private, so we can't change its structure without unexpected breaking changes. If you need to access the scale ticks, usescale.getTicks()
instead.Fixes #4502 (regression introduced in #4268)