From 2f9f1e06aafe02548115e55e2cccf6aeb7c82de4 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 22 Jan 2019 07:52:30 -0800 Subject: [PATCH 1/3] Common logic for resolving element options --- src/controllers/controller.line.js | 72 +++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 22 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index e57d5e2ffb2..366234a5c2c 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -47,9 +47,9 @@ module.exports = DatasetController.extend({ var options = me.chart.options; var lineElementOptions = options.elements.line; var scale = me.getScaleForId(meta.yAxisID); - var i, ilen, custom; var dataset = me.getDataset(); var showLine = lineEnabled(dataset, options); + var i, ilen, custom, model; // Update Line if (showLine) { @@ -66,24 +66,15 @@ module.exports = DatasetController.extend({ // Data line._children = points; // Model - line._model = { - // Appearance - // The default behavior of lines is to break at null values, according - // to https://github.com/chartjs/Chart.js/issues/2435#issuecomment-216718158 - // This option gives lines the ability to span gaps - spanGaps: valueOrDefault(dataset.spanGaps, options.spanGaps), - tension: resolve([custom.tension, dataset.lineTension, lineElementOptions.tension]), - backgroundColor: resolve([custom.backgroundColor, dataset.backgroundColor, lineElementOptions.backgroundColor]), - borderWidth: resolve([custom.borderWidth, dataset.borderWidth, lineElementOptions.borderWidth]), - borderColor: resolve([custom.borderColor, dataset.borderColor, lineElementOptions.borderColor]), - borderCapStyle: resolve([custom.borderCapStyle, dataset.borderCapStyle, lineElementOptions.borderCapStyle]), - borderDash: resolve([custom.borderDash, dataset.borderDash, lineElementOptions.borderDash]), - borderDashOffset: resolve([custom.borderDashOffset, dataset.borderDashOffset, lineElementOptions.borderDashOffset]), - borderJoinStyle: resolve([custom.borderJoinStyle, dataset.borderJoinStyle, lineElementOptions.borderJoinStyle]), - fill: resolve([custom.fill, dataset.fill, lineElementOptions.fill]), - steppedLine: resolve([custom.steppedLine, dataset.steppedLine, lineElementOptions.stepped]), - cubicInterpolationMode: resolve([custom.cubicInterpolationMode, dataset.cubicInterpolationMode, lineElementOptions.cubicInterpolationMode]), - }; + line._model = model = me._resolveLineElementOptions(line); + + // Appearance + // The default behavior of lines is to break at null values, according + // to https://github.com/chartjs/Chart.js/issues/2435#issuecomment-216718158 + // This option gives lines the ability to span gaps + model.spanGaps = valueOrDefault(dataset.spanGaps, options.spanGaps); + model.tension = resolve([custom.tension, dataset.lineTension, lineElementOptions.tension]); + model.steppedLine = resolve([custom.steppedLine, dataset.steppedLine, lineElementOptions.stepped]); line.pivot(); } @@ -114,7 +105,7 @@ module.exports = DatasetController.extend({ var xScale = me.getScaleForId(meta.xAxisID); var x, y; - var options = me._resolveElementOptions(point, index); + var options = me._resolvePointElementOptions(point, index); x = xScale.getPixelForValue(typeof value === 'object' ? value : NaN, index, datasetIndex); y = reset ? yScale.getBasePixel() : me.calculatePointY(value, index, datasetIndex); @@ -148,12 +139,12 @@ module.exports = DatasetController.extend({ /** * @private */ - _resolveElementOptions: function(point, index) { + _resolvePointElementOptions: function(element, index) { var me = this; var chart = me.chart; var datasets = chart.data.datasets; var dataset = datasets[me.index]; - var custom = point.custom || {}; + var custom = element.custom || {}; var options = chart.options.elements.point; var values = {}; var i, ilen, key; @@ -194,6 +185,43 @@ module.exports = DatasetController.extend({ return values; }, + /** + * @private + */ + _resolveLineElementOptions: function(element) { + var me = this; + var chart = me.chart; + var datasets = chart.data.datasets; + var dataset = datasets[me.index]; + var custom = element.custom || {}; + var options = chart.options.elements.line; + var values = {}; + var i, ilen, key; + + var ELEMENT_OPTIONS = [ + 'backgroundColor', + 'borderWidth', + 'borderColor', + 'borderCapStyle', + 'borderDash', + 'borderDashOffset', + 'borderJoinStyle', + 'fill', + 'cubicInterpolationMode' + ]; + + for (i = 0, ilen = ELEMENT_OPTIONS.length; i < ilen; ++i) { + key = ELEMENT_OPTIONS[i]; + values[key] = resolve([ + custom[key], + dataset[key], + options[key] + ]); + } + + return values; + }, + calculatePointY: function(value, index, datasetIndex) { var me = this; var chart = me.chart; From 4d1c4b0837c7bb1cbf737e0304a8653fd55a2a24 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Tue, 22 Jan 2019 09:18:52 -0800 Subject: [PATCH 2/3] Address review comments --- src/controllers/controller.line.js | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 366234a5c2c..40ac75a675d 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -66,7 +66,7 @@ module.exports = DatasetController.extend({ // Data line._children = points; // Model - line._model = model = me._resolveLineElementOptions(line); + line._model = model = me._resolveLineOptions(line); // Appearance // The default behavior of lines is to break at null values, according @@ -105,7 +105,7 @@ module.exports = DatasetController.extend({ var xScale = me.getScaleForId(meta.xAxisID); var x, y; - var options = me._resolvePointElementOptions(point, index); + var options = me._resolvePointOptions(point, index); x = xScale.getPixelForValue(typeof value === 'object' ? value : NaN, index, datasetIndex); y = reset ? yScale.getBasePixel() : me.calculatePointY(value, index, datasetIndex); @@ -139,11 +139,10 @@ module.exports = DatasetController.extend({ /** * @private */ - _resolvePointElementOptions: function(element, index) { + _resolvePointOptions: function(element, index) { var me = this; var chart = me.chart; - var datasets = chart.data.datasets; - var dataset = datasets[me.index]; + var dataset = chart.data.datasets[me.index]; var custom = element.custom || {}; var options = chart.options.elements.point; var values = {}; @@ -188,17 +187,16 @@ module.exports = DatasetController.extend({ /** * @private */ - _resolveLineElementOptions: function(element) { + _resolveLineOptions: function(element) { var me = this; var chart = me.chart; - var datasets = chart.data.datasets; - var dataset = datasets[me.index]; + var dataset = chart.data.datasets[me.index]; var custom = element.custom || {}; var options = chart.options.elements.line; var values = {}; var i, ilen, key; - var ELEMENT_OPTIONS = [ + var keys = [ 'backgroundColor', 'borderWidth', 'borderColor', @@ -210,8 +208,8 @@ module.exports = DatasetController.extend({ 'cubicInterpolationMode' ]; - for (i = 0, ilen = ELEMENT_OPTIONS.length; i < ilen; ++i) { - key = ELEMENT_OPTIONS[i]; + for (i = 0, ilen = keys.length; i < ilen; ++i) { + key = keys[i]; values[key] = resolve([ custom[key], dataset[key], From 624f206b762dc1108fe6a89972aa6eef84a6e7b7 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Wed, 23 Jan 2019 07:52:13 -0800 Subject: [PATCH 3/3] Move setting of all line options to new method --- src/controllers/controller.line.js | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 40ac75a675d..5341ac1e6dd 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -44,17 +44,13 @@ module.exports = DatasetController.extend({ var meta = me.getMeta(); var line = meta.dataset; var points = meta.data || []; - var options = me.chart.options; - var lineElementOptions = options.elements.line; var scale = me.getScaleForId(meta.yAxisID); var dataset = me.getDataset(); - var showLine = lineEnabled(dataset, options); - var i, ilen, custom, model; + var showLine = lineEnabled(dataset, me.chart.options); + var i, ilen; // Update Line if (showLine) { - custom = line.custom || {}; - // Compatibility: If the properties are defined with only the old name, use those values if ((dataset.tension !== undefined) && (dataset.lineTension === undefined)) { dataset.lineTension = dataset.tension; @@ -66,15 +62,7 @@ module.exports = DatasetController.extend({ // Data line._children = points; // Model - line._model = model = me._resolveLineOptions(line); - - // Appearance - // The default behavior of lines is to break at null values, according - // to https://github.com/chartjs/Chart.js/issues/2435#issuecomment-216718158 - // This option gives lines the ability to span gaps - model.spanGaps = valueOrDefault(dataset.spanGaps, options.spanGaps); - model.tension = resolve([custom.tension, dataset.lineTension, lineElementOptions.tension]); - model.steppedLine = resolve([custom.steppedLine, dataset.steppedLine, lineElementOptions.stepped]); + line._model = me._resolveLineOptions(line); line.pivot(); } @@ -192,7 +180,8 @@ module.exports = DatasetController.extend({ var chart = me.chart; var dataset = chart.data.datasets[me.index]; var custom = element.custom || {}; - var options = chart.options.elements.line; + var options = chart.options; + var elementOptions = options.elements.line; var values = {}; var i, ilen, key; @@ -213,10 +202,17 @@ module.exports = DatasetController.extend({ values[key] = resolve([ custom[key], dataset[key], - options[key] + elementOptions[key] ]); } + // The default behavior of lines is to break at null values, according + // to https://github.com/chartjs/Chart.js/issues/2435#issuecomment-216718158 + // This option gives lines the ability to span gaps + values.spanGaps = valueOrDefault(dataset.spanGaps, options.spanGaps); + values.tension = resolve([custom.tension, dataset.lineTension, elementOptions.tension]); + values.steppedLine = resolve([custom.steppedLine, dataset.steppedLine, elementOptions.stepped]); + return values; },