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

Scales: add axis to convertTicksToLabels callback #4632

Merged
merged 1 commit into from Aug 13, 2017

Conversation

andig
Copy link
Contributor

@andig andig commented Aug 8, 2017

For tick formatting it can be beneficial to have access to the axis. This PR adds the axis as this parameter- apparently this is amongst the few callbacks that didn't have this yet.

Note: all other scale events are passing [this] which is an array of ChartElement. I couldn't figure why the array notation but it might make sense to use the same here instead of the blunt this.

@andig
Copy link
Contributor Author

andig commented Aug 11, 2017

Updated this to be an array for sake of consistency with other callbacks. Is there any chance to get this PR in?

@simonbrunel
Copy link
Member

The other callbacks are triggered with the helpers.callback method, [this] representing the callback arguments, not the scope (actually, these callbacks are not called with this as scope). In case of map, the second argument is the scope and should be this (not [this]).

@andig
Copy link
Contributor Author

andig commented Aug 11, 2017

the second argument is the scope and should be this (not [this]).

Done, reverted to its earlier implementation.

@simonbrunel simonbrunel merged commit 3e2b405 into chartjs:master Aug 13, 2017
@andig andig deleted the add-axis-reference branch August 13, 2017 10:15
@simonbrunel simonbrunel added this to the Version 2.7 milestone Aug 14, 2017
silverspectro pushed a commit to silverspectro/Chart.js that referenced this pull request Aug 21, 2017
yofreke pushed a commit to yofreke/Chart.js that referenced this pull request Dec 30, 2017
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants