-
Notifications
You must be signed in to change notification settings - Fork 11.9k
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 scale options update error #4198
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
<!doctype html> | ||
<html> | ||
|
||
<head> | ||
<title>Toggle Scale Type</title> | ||
<script src="../../dist/Chart.bundle.js"></script> | ||
<script src="../utils.js"></script> | ||
<style> | ||
canvas { | ||
-moz-user-select: none; | ||
-webkit-user-select: none; | ||
-ms-user-select: none; | ||
} | ||
</style> | ||
</head> | ||
|
||
<body> | ||
<div style="width:75%;"> | ||
<canvas id="canvas"></canvas> | ||
</div> | ||
<button id="toggleScale">Toggle Scale Type</button> | ||
<script> | ||
var randomScalingFactor = function() { | ||
return Math.ceil(Math.random() * 10.0) * Math.pow(10, Math.ceil(Math.random() * 5)); | ||
}; | ||
|
||
var type = 'linear'; | ||
|
||
var config = { | ||
type: 'line', | ||
data: { | ||
labels: ["January", "February", "March", "April", "May", "June", "July"], | ||
datasets: [{ | ||
label: "My First dataset", | ||
backgroundColor: window.chartColors.red, | ||
borderColor: window.chartColors.red, | ||
fill: false, | ||
data: [ | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor() | ||
], | ||
}, { | ||
label: "My Second dataset", | ||
backgroundColor: window.chartColors.blue, | ||
borderColor: window.chartColors.blue, | ||
fill: false, | ||
data: [ | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor(), | ||
randomScalingFactor() | ||
], | ||
}] | ||
}, | ||
options: { | ||
responsive: true, | ||
title:{ | ||
display: true, | ||
text: 'Chart.js Line Chart - ' + type | ||
}, | ||
scales: { | ||
xAxes: [{ | ||
display: true, | ||
}], | ||
yAxes: [{ | ||
display: true, | ||
type: type | ||
}] | ||
} | ||
} | ||
}; | ||
|
||
window.onload = function() { | ||
var ctx = document.getElementById("canvas").getContext("2d"); | ||
window.myLine = new Chart(ctx, config); | ||
}; | ||
|
||
document.getElementById('toggleScale').addEventListener('click', function() { | ||
type = type === 'linear' ? 'logarithmic' : 'linear'; | ||
window.myLine.options.title.text = 'Chart.js Line Chart - ' + type; | ||
window.myLine.options.scales.yAxes[0] = { | ||
display: true, | ||
type: type | ||
} | ||
|
||
window.myLine.update(); | ||
}); | ||
</script> | ||
</body> | ||
|
||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,17 +45,21 @@ module.exports = function(Chart) { | |
function updateConfig(chart) { | ||
var newOptions = chart.options; | ||
|
||
// Update Scale(s) with options | ||
if (newOptions.scale) { | ||
chart.scale.options = newOptions.scale; | ||
} else if (newOptions.scales) { | ||
newOptions.scales.xAxes.concat(newOptions.scales.yAxes).forEach(function(scaleOptions) { | ||
chart.scales[scaleOptions.id].options = scaleOptions; | ||
}); | ||
} | ||
|
||
helpers.each(chart.scales, function(scale) { | ||
Chart.layoutService.removeBox(chart, scale); | ||
}); | ||
|
||
newOptions = helpers.configMerge( | ||
Chart.defaults.global, | ||
Chart.defaults[chart.config.type], | ||
newOptions); | ||
|
||
chart.options = chart.config.options = newOptions; | ||
chart.ensureScalesHaveIDs(); | ||
chart.buildOrUpdateScales(); | ||
// Tooltip | ||
chart.tooltip._options = newOptions.tooltips; | ||
chart.tooltip.initialize(); | ||
} | ||
|
||
function positionIsHorizontal(position) { | ||
|
@@ -143,7 +147,7 @@ module.exports = function(Chart) { | |
|
||
// Make sure scales have IDs and are built before we build any controllers. | ||
me.ensureScalesHaveIDs(); | ||
me.buildScales(); | ||
me.buildOrUpdateScales(); | ||
me.initToolTip(); | ||
|
||
// After init plugin notification | ||
|
@@ -223,11 +227,15 @@ module.exports = function(Chart) { | |
/** | ||
* Builds a map of scale ID to scale object for future lookup. | ||
*/ | ||
buildScales: function() { | ||
buildOrUpdateScales: function() { | ||
var me = this; | ||
var options = me.options; | ||
var scales = me.scales = {}; | ||
var scales = me.scales || {}; | ||
var items = []; | ||
var updated = Object.keys(scales).reduce(function(obj, id) { | ||
obj[id] = false; | ||
return obj; | ||
}, {}); | ||
|
||
if (options.scales) { | ||
items = items.concat( | ||
|
@@ -251,24 +259,35 @@ module.exports = function(Chart) { | |
|
||
helpers.each(items, function(item) { | ||
var scaleOptions = item.options; | ||
var id = scaleOptions.id; | ||
var scaleType = helpers.valueOrDefault(scaleOptions.type, item.dtype); | ||
var scaleClass = Chart.scaleService.getScaleConstructor(scaleType); | ||
if (!scaleClass) { | ||
return; | ||
} | ||
|
||
if (positionIsHorizontal(scaleOptions.position) !== positionIsHorizontal(item.dposition)) { | ||
scaleOptions.position = item.dposition; | ||
} | ||
|
||
var scale = new scaleClass({ | ||
id: scaleOptions.id, | ||
options: scaleOptions, | ||
ctx: me.ctx, | ||
chart: me | ||
}); | ||
updated[id] = true; | ||
var scale = null; | ||
if (id in scales && scales[id].type === scaleType) { | ||
scale = scales[id]; | ||
scale.options = scaleOptions; | ||
scale.ctx = me.ctx; | ||
scale.chart = me; | ||
} else { | ||
var scaleClass = Chart.scaleService.getScaleConstructor(scaleType); | ||
if (!scaleClass) { | ||
return; | ||
} | ||
scale = new scaleClass({ | ||
id: id, | ||
type: scaleType, | ||
options: scaleOptions, | ||
ctx: me.ctx, | ||
chart: me | ||
}); | ||
scales[scale.id] = scale; | ||
} | ||
|
||
scales[scale.id] = scale; | ||
scale.mergeTicksOptions(); | ||
|
||
// TODO(SB): I think we should be able to remove this custom case (options.scale) | ||
|
@@ -278,6 +297,14 @@ module.exports = function(Chart) { | |
me.scale = scale; | ||
} | ||
}); | ||
// clear up discarded scales | ||
helpers.each(updated, function(hasUpdated, id) { | ||
if (!hasUpdated) { | ||
delete scales[id]; | ||
} | ||
}); | ||
|
||
me.scales = scales; | ||
|
||
Chart.scaleService.addScalesToLayout(this); | ||
}, | ||
|
@@ -301,6 +328,7 @@ module.exports = function(Chart) { | |
|
||
if (meta.controller) { | ||
meta.controller.updateIndex(datasetIndex); | ||
meta.controller.linkScales(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I came across this when I tried to update scales with different I'm not sure whether I missed anything else, please point out if there are. It's a little bit too many configs here : P |
||
} else { | ||
var ControllerClass = Chart.controllers[meta.type]; | ||
if (ControllerClass === 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.
@etimberg last commit here was definitely a mistake. Fixed here.
So now whenever user specify a scale with a new
id
or thetype
is changed, user lose the reference to the old scale.