-
Notifications
You must be signed in to change notification settings - Fork 87
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
Fixes plugin on Chart.js 3.5.x / 3.6.x. #88
base: master
Are you sure you want to change the base?
Conversation
Allows X axis scale to use either 'x' or 'xAxes'.
Update for 'xAxes' scale name.
// set axis scale | ||
chart.options.scales.x.min = start; | ||
chart.options.scales.x.max = end; | ||
chart.options.scales[xAxisName].min = start; | ||
chart.options.scales[xAxisName].max = end; |
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.
Consider that if "xAxisName" is "xAxes" then it's an array, not an single object
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.
This is not true, all scales in chart.js V3 are single objects
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.
this is not a flexibale way of solving that you have given your x axis scale a different ID as the default, more flexibel way would be to allow a option field in the plugin options where this can be configured, that defaults to chart.js's default which is x
|
||
if (!chart.config.options.scales.x) { | ||
return | ||
const xScaleOptions = chart.config.options.scales.x || chart.config.options.scales.xAxes; |
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.
Would be better to make this a variable that the user can configure in the options for the plugin so that all names you give to your x axis are supported.
Then if no name has been given you can fall back to chart.js's default x axis scale id which is x
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.
You are, of course, welcome to make any changes you desire.
@@ -167,15 +168,17 @@ export default { | |||
|
|||
afterEvent: function(chart, event) { | |||
|
|||
if (chart.config.options.scales.x.length == 0) { | |||
return | |||
const xScaleOptions = chart.config.options.scales.x || chart.config.options.scales.xAxes; |
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.
Same as above
@@ -278,6 +281,7 @@ export default { | |||
}, | |||
|
|||
resetZoom: function(chart) { | |||
const xAxisName = chart.config.options.scales.x ? 'x' : chart.config.options.scales.xAxes ? 'xAxes' : undefined; |
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.
same as above, just take it from the options and if not specify fall back to default x
|
||
doZoom: function(chart, start, end) { | ||
const xAxisName = chart.config.options.scales.x ? 'x' : chart.config.options.scales.xAxes ? 'xAxes' : undefined; |
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.
same as above
Any update on this? This prevents using your library with chart.js 3.5+, which means it effectively works with 3.4.x only. |
@AbelHeinsbroek any updates? Would love to get this pulled in. |
Hi, any updates on this one? |
I will look into this |
Fixes plugin with Chart.js 3.5.x / 3.6.x.
Allows X axis scale to use either 'x' or 'xAxes'.