From a566d16e697c895bcd719ab187c57f8c90daa54c Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Fri, 20 May 2016 20:17:28 +0200 Subject: [PATCH 1/4] Refactor scale base pixel and point calculation. --- src/controllers/controller.bar.js | 71 ++++------------------------ src/controllers/controller.bubble.js | 24 +--------- src/controllers/controller.line.js | 34 ++++--------- src/controllers/controller.radar.js | 15 +----- src/core/core.scale.js | 12 +++++ src/scales/scale.radialLinear.js | 13 +++++ 6 files changed, 46 insertions(+), 123 deletions(-) diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index fc7f1eb2b..7a6915cf6 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -84,18 +84,7 @@ module.exports = function(Chart) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); - - var yScalePoint; - - if (yScale.min < 0 && yScale.max < 0) { - // all less than 0. use the top - yScalePoint = yScale.getPixelForValue(yScale.max); - } else if (yScale.min > 0 && yScale.max > 0) { - yScalePoint = yScale.getPixelForValue(yScale.min); - } else { - yScalePoint = yScale.getPixelForValue(0); - } - + var scaleBase = yScale.getBasePixel(); var rectangleElementOptions = this.chart.options.elements.rectangle; var custom = rectangle.custom || {}; var dataset = this.getDataset(); @@ -112,14 +101,14 @@ module.exports = function(Chart) { // Desired view properties _model: { x: this.calculateBarX(index, this.index), - y: reset ? yScalePoint : this.calculateBarY(index, this.index), + y: reset ? scaleBase : this.calculateBarY(index, this.index), // Tooltip label: this.chart.data.labels[index], datasetLabel: dataset.label, // Appearance - base: reset ? yScalePoint : this.calculateBarBase(this.index, index), + base: reset ? scaleBase : this.calculateBarBase(this.index, index), width: this.calculateBarWidth(index), backgroundColor: custom.backgroundColor ? custom.backgroundColor : helpers.getValueAtIndexOrDefault(dataset.backgroundColor, index, rectangleElementOptions.backgroundColor), borderSkipped: custom.borderSkipped ? custom.borderSkipped : rectangleElementOptions.borderSkipped, @@ -132,9 +121,7 @@ module.exports = function(Chart) { calculateBarBase: function(datasetIndex, index) { var meta = this.getMeta(); - var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); - var base = 0; if (yScale.options.stacked) { @@ -163,24 +150,12 @@ module.exports = function(Chart) { return yScale.getPixelForValue(base); } - base = yScale.getPixelForValue(yScale.min); - - if (yScale.beginAtZero || ((yScale.min <= 0 && yScale.max >= 0) || (yScale.min >= 0 && yScale.max <= 0))) { - base = yScale.getPixelForValue(0, 0); - //base += yScale.options.gridLines.lineWidth; - } else if (yScale.min < 0 && yScale.max < 0) { - // All values are negative. Use the top as the base - base = yScale.getPixelForValue(yScale.max); - } - - return base; - + return yScale.getBasePixel(); }, getRuler: function(index) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); - var yScale = this.getScaleForId(meta.yAxisID); var datasetCount = this.getBarCount(); var tickWidth; @@ -237,7 +212,6 @@ module.exports = function(Chart) { calculateBarX: function(index, datasetIndex) { var meta = this.getMeta(); - var yScale = this.getScaleForId(meta.yAxisID); var xScale = this.getScaleForId(meta.xAxisID); var barIndex = this.getBarIndex(datasetIndex); @@ -259,9 +233,7 @@ module.exports = function(Chart) { calculateBarY: function(index, datasetIndex) { var meta = this.getMeta(); - var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); - var value = this.getDataset().data[index]; if (yScale.options.stacked) { @@ -387,21 +359,11 @@ module.exports = function(Chart) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); - - var xScalePoint; - - if (xScale.min < 0 && xScale.max < 0) { - // all less than 0. use the right - xScalePoint = xScale.getPixelForValue(xScale.max); - } else if (xScale.min > 0 && xScale.max > 0) { - xScalePoint = xScale.getPixelForValue(xScale.min); - } else { - xScalePoint = xScale.getPixelForValue(0); - } - + var scaleBase = xScale.getBasePixel(); var custom = rectangle.custom || {}; var dataset = this.getDataset(); var rectangleElementOptions = this.chart.options.elements.rectangle; + helpers.extend(rectangle, { // Utility _chart: this.chart.chart, @@ -412,7 +374,7 @@ module.exports = function(Chart) { // Desired view properties _model: { - x: reset ? xScalePoint : this.calculateBarX(index, this.index), + x: reset ? scaleBase : this.calculateBarX(index, this.index), y: this.calculateBarY(index, this.index), // Tooltip @@ -420,7 +382,7 @@ module.exports = function(Chart) { datasetLabel: dataset.label, // Appearance - base: reset ? xScalePoint : this.calculateBarBase(this.index, index), + base: reset ? scaleBase : this.calculateBarBase(this.index, index), height: this.calculateBarHeight(index), backgroundColor: custom.backgroundColor ? custom.backgroundColor : helpers.getValueAtIndexOrDefault(dataset.backgroundColor, index, rectangleElementOptions.backgroundColor), borderSkipped: custom.borderSkipped ? custom.borderSkipped : rectangleElementOptions.borderSkipped, @@ -506,8 +468,6 @@ module.exports = function(Chart) { calculateBarBase: function (datasetIndex, index) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); - var yScale = this.getScaleForId(meta.yAxisID); - var base = 0; if (xScale.options.stacked) { @@ -535,21 +495,11 @@ module.exports = function(Chart) { return xScale.getPixelForValue(base); } - base = xScale.getPixelForValue(xScale.min); - - if (xScale.beginAtZero || ((xScale.min <= 0 && xScale.max >= 0) || (xScale.min >= 0 && xScale.max <= 0))) { - base = xScale.getPixelForValue(0, 0); - } else if (xScale.min < 0 && xScale.max < 0) { - // All values are negative. Use the right as the base - base = xScale.getPixelForValue(xScale.max); - } - - return base; + return xScale.getBasePixel(); }, getRuler: function (index) { var meta = this.getMeta(); - var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); var datasetCount = this.getBarCount(); @@ -592,8 +542,6 @@ module.exports = function(Chart) { calculateBarX: function (index, datasetIndex) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); - var yScale = this.getScaleForId(meta.yAxisID); - var value = this.getDataset().data[index]; if (xScale.options.stacked) { @@ -626,7 +574,6 @@ module.exports = function(Chart) { calculateBarY: function (index, datasetIndex) { var meta = this.getMeta(); var yScale = this.getScaleForId(meta.yAxisID); - var xScale = this.getScaleForId(meta.xAxisID); var barIndex = this.getBarIndex(datasetIndex); var ruler = this.getRuler(index); diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index 581287bd5..1faa10f65 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -64,17 +64,6 @@ module.exports = function(Chart) { update: function update(reset) { var meta = this.getMeta(); var points = meta.data; - var yScale = this.getScaleForId(meta.yAxisID); - var xScale = this.getScaleForId(meta.xAxisID); - var scaleBase; - - if (yScale.min < 0 && yScale.max < 0) { - scaleBase = yScale.getPixelForValue(yScale.max); - } else if (yScale.min > 0 && yScale.max > 0) { - scaleBase = yScale.getPixelForValue(yScale.min); - } else { - scaleBase = yScale.getPixelForValue(0); - } // Update Points helpers.each(points, function(point, index) { @@ -85,23 +74,14 @@ module.exports = function(Chart) { updateElement: function(point, index, reset) { var meta = this.getMeta(); - var yScale = this.getScaleForId(meta.yAxisID); var xScale = this.getScaleForId(meta.xAxisID); - var scaleBase; + var yScale = this.getScaleForId(meta.yAxisID); var custom = point.custom || {}; var dataset = this.getDataset(); var data = dataset.data[index]; var pointElementOptions = this.chart.options.elements.point; - if (yScale.min < 0 && yScale.max < 0) { - scaleBase = yScale.getPixelForValue(yScale.max); - } else if (yScale.min > 0 && yScale.max > 0) { - scaleBase = yScale.getPixelForValue(yScale.min); - } else { - scaleBase = yScale.getPixelForValue(0); - } - helpers.extend(point, { // Utility _chart: this.chart.chart, @@ -113,7 +93,7 @@ module.exports = function(Chart) { // Desired view properties _model: { x: reset ? xScale.getPixelForDecimal(0.5) : xScale.getPixelForValue(data, index, this.index, this.chart.isCombo), - y: reset ? scaleBase : yScale.getPixelForValue(data, index, this.index), + y: reset ? yScale.getBasePixel() : yScale.getPixelForValue(data, index, this.index), // Appearance radius: reset ? 0 : custom.radius ? custom.radius : this.getRadius(data), backgroundColor: custom.backgroundColor ? custom.backgroundColor : helpers.getValueAtIndexOrDefault(dataset.backgroundColor, index, pointElementOptions.backgroundColor), diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 080760068..39d0fca28 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -72,17 +72,8 @@ module.exports = function(Chart) { var points = meta.data || []; var options = me.chart.options; var lineElementOptions = options.elements.line; - var yScale = me.getScaleForId(meta.yAxisID); - var xScale = me.getScaleForId(meta.xAxisID); - var scaleBase, i, ilen, dataset, custom; - - if (yScale.min < 0 && yScale.max < 0) { - scaleBase = yScale.getPixelForValue(yScale.max); - } else if (yScale.min > 0 && yScale.max > 0) { - scaleBase = yScale.getPixelForValue(yScale.min); - } else { - scaleBase = yScale.getPixelForValue(0); - } + var scale = me.getScaleForId(meta.yAxisID); + var i, ilen, dataset, custom; // Update Line if (options.showLines) { @@ -95,7 +86,7 @@ module.exports = function(Chart) { } // Utility - line._scale = yScale; + line._scale = scale; line._datasetIndex = me.index; // Data line._children = points; @@ -112,9 +103,9 @@ module.exports = function(Chart) { borderJoinStyle: custom.borderJoinStyle ? custom.borderJoinStyle : (dataset.borderJoinStyle || lineElementOptions.borderJoinStyle), fill: custom.fill ? custom.fill : (dataset.fill !== undefined ? dataset.fill : lineElementOptions.fill), // Scale - scaleTop: yScale.top, - scaleBottom: yScale.bottom, - scaleZero: scaleBase + scaleTop: scale.top, + scaleBottom: scale.bottom, + scaleZero: scale.getBasePixel() }; line.pivot(); @@ -188,15 +179,7 @@ module.exports = function(Chart) { var yScale = me.getScaleForId(meta.yAxisID); var xScale = me.getScaleForId(meta.xAxisID); var pointOptions = me.chart.options.elements.point; - var scaleBase, x, y; - - if (yScale.min < 0 && yScale.max < 0) { - scaleBase = yScale.getPixelForValue(yScale.max); - } else if (yScale.min > 0 && yScale.max > 0) { - scaleBase = yScale.getPixelForValue(yScale.min); - } else { - scaleBase = yScale.getPixelForValue(0); - } + var x, y; // Compatibility: If the properties are defined with only the old name, use those values if ((dataset.radius !== undefined) && (dataset.pointRadius === undefined)) { @@ -207,7 +190,7 @@ module.exports = function(Chart) { } x = xScale.getPixelForValue(value, index, datasetIndex, me.chart.isCombo); - y = reset ? scaleBase : me.calculatePointY(value, index, datasetIndex, me.chart.isCombo); + y = reset ? yScale.getBasePixel() : me.calculatePointY(value, index, datasetIndex, me.chart.isCombo); // Utility point._chart = me.chart.chart; @@ -237,7 +220,6 @@ module.exports = function(Chart) { var me = this; var chart = me.chart; var meta = me.getMeta(); - var xScale = me.getScaleForId(meta.xAxisID); var yScale = me.getScaleForId(meta.yAxisID); var sumPos = 0; var sumNeg = 0; diff --git a/src/controllers/controller.radar.js b/src/controllers/controller.radar.js index f4a3f1e4b..4e1a5c48d 100644 --- a/src/controllers/controller.radar.js +++ b/src/controllers/controller.radar.js @@ -65,21 +65,10 @@ module.exports = function(Chart) { var custom = line.custom || {}; var dataset = this.getDataset(); var lineElementOptions = this.chart.options.elements.line; - var scale = this.chart.scale; - var scaleBase; - - if (scale.min < 0 && scale.max < 0) { - scaleBase = scale.getPointPositionForValue(0, scale.max); - } else if (scale.min > 0 && scale.max > 0) { - scaleBase = scale.getPointPositionForValue(0, scale.min); - } else { - scaleBase = scale.getPointPositionForValue(0, 0); - } // Compatibility: If the properties are defined with only the old name, use those values - if ((dataset.tension !== undefined) && (dataset.lineTension === undefined)) - { + if ((dataset.tension !== undefined) && (dataset.lineTension === undefined)) { dataset.lineTension = dataset.tension; } @@ -104,7 +93,7 @@ module.exports = function(Chart) { // Scale scaleTop: scale.top, scaleBottom: scale.bottom, - scaleZero: scaleBase + scaleZero: scale.getBasePosition() } }); diff --git a/src/core/core.scale.js b/src/core/core.scale.js index a158be624..cf6afb0f3 100644 --- a/src/core/core.scale.js +++ b/src/core/core.scale.js @@ -451,6 +451,18 @@ module.exports = function(Chart) { } }, + getBasePixel: function() { + var me = this; + var min = me.min; + var max = me.max; + + return me.getPixelForValue( + me.beginAtZero? 0: + min < 0 && max < 0? max : + min > 0 && max > 0? min : + 0); + }, + // Actualy draw the scale on the canvas // @param {rectangle} chartArea : the area of the chart to draw full grid lines on draw: function(chartArea) { diff --git a/src/scales/scale.radialLinear.js b/src/scales/scale.radialLinear.js index 5d46f26ee..017c0c747 100644 --- a/src/scales/scale.radialLinear.js +++ b/src/scales/scale.radialLinear.js @@ -321,6 +321,19 @@ module.exports = function(Chart) { getPointPositionForValue: function(index, value) { return this.getPointPosition(index, this.getDistanceFromCenterForValue(value)); }, + + getBasePosition: function() { + var me = this; + var min = me.min; + var max = me.max; + + return me.getPointPositionForValue(0, + me.beginAtZero? 0: + min < 0 && max < 0? max : + min > 0 && max > 0? min : + 0); + }, + draw: function() { if (this.options.display) { var ctx = this.ctx; From 7108f78d2ff0f9d957add630259ae45f924bb614 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Fri, 20 May 2016 23:42:24 +0200 Subject: [PATCH 2/4] Refactor addElements and addElementAndReset Data controllers should now rarely implement addElements and addElementAndReset but instead should define dataElementType (and optionally datasetElementType). Also remove some dead code (e.g. numBars, colorForNewElement, etc.). --- src/chart.js | 22 +++++---- src/controllers/controller.bar.js | 38 +++------------ src/controllers/controller.bubble.js | 35 +------------- src/controllers/controller.doughnut.js | 36 ++------------ src/controllers/controller.line.js | 31 ++---------- src/controllers/controller.polarArea.js | 30 ++---------- src/controllers/controller.radar.js | 39 +++------------ src/core/core.datasetController.js | 64 +++++++++++++++++++++++-- 8 files changed, 93 insertions(+), 202 deletions(-) diff --git a/src/chart.js b/src/chart.js index 6c2d8c51b..58061c2ab 100644 --- a/src/chart.js +++ b/src/chart.js @@ -13,12 +13,10 @@ require('./core/core.scaleService')(Chart); require('./core/core.title')(Chart); require('./core/core.tooltip')(Chart); -require('./controllers/controller.bar')(Chart); -require('./controllers/controller.bubble')(Chart); -require('./controllers/controller.doughnut')(Chart); -require('./controllers/controller.line')(Chart); -require('./controllers/controller.polarArea')(Chart); -require('./controllers/controller.radar')(Chart); +require('./elements/element.arc')(Chart); +require('./elements/element.line')(Chart); +require('./elements/element.point')(Chart); +require('./elements/element.rectangle')(Chart); require('./scales/scale.category')(Chart); require('./scales/scale.linear')(Chart); @@ -26,10 +24,14 @@ require('./scales/scale.logarithmic')(Chart); require('./scales/scale.radialLinear')(Chart); require('./scales/scale.time')(Chart); -require('./elements/element.arc')(Chart); -require('./elements/element.line')(Chart); -require('./elements/element.point')(Chart); -require('./elements/element.rectangle')(Chart); +// Controllers must be loaded after elements +// See Chart.core.datasetController.dataElementType +require('./controllers/controller.bar')(Chart); +require('./controllers/controller.bubble')(Chart); +require('./controllers/controller.doughnut')(Chart); +require('./controllers/controller.line')(Chart); +require('./controllers/controller.polarArea')(Chart); +require('./controllers/controller.radar')(Chart); require('./charts/Chart.Bar')(Chart); require('./charts/Chart.Bubble')(Chart); diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index 7a6915cf6..11cb32447 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -29,12 +29,16 @@ module.exports = function(Chart) { }; Chart.controllers.bar = Chart.DatasetController.extend({ + + dataElementType: Chart.elements.Rectangle, + initialize: function(chart, datasetIndex) { Chart.DatasetController.prototype.initialize.call(this, chart, datasetIndex); // Use this to indicate that this is a bar dataset. this.getMeta().bar = true; }, + // Get the number of datasets that display bars. We use this to correctly calculate the bar width getBarCount: function getBarCount() { var barCount = 0; @@ -47,40 +51,13 @@ module.exports = function(Chart) { return barCount; }, - addElements: function() { - var meta = this.getMeta(); - helpers.each(this.getDataset().data, function(value, index) { - meta.data[index] = meta.data[index] || new Chart.elements.Rectangle({ - _chart: this.chart.chart, - _datasetIndex: this.index, - _index: index - }); - }, this); - }, - - addElementAndReset: function(index) { - var rectangle = new Chart.elements.Rectangle({ - _chart: this.chart.chart, - _datasetIndex: this.index, - _index: index - }); - - var numBars = this.getBarCount(); - - // Add to the points array and reset it - this.getMeta().data.splice(index, 0, rectangle); - this.updateElement(rectangle, index, true, numBars); - }, - update: function update(reset) { - var numBars = this.getBarCount(); - helpers.each(this.getMeta().data, function(rectangle, index) { - this.updateElement(rectangle, index, reset, numBars); + this.updateElement(rectangle, index, reset); }, this); }, - updateElement: function updateElement(rectangle, index, reset, numBars) { + updateElement: function updateElement(rectangle, index, reset) { var meta = this.getMeta(); var xScale = this.getScaleForId(meta.xAxisID); var yScale = this.getScaleForId(meta.yAxisID); @@ -91,13 +68,11 @@ module.exports = function(Chart) { helpers.extend(rectangle, { // Utility - _chart: this.chart.chart, _xScale: xScale, _yScale: yScale, _datasetIndex: this.index, _index: index, - // Desired view properties _model: { x: this.calculateBarX(index, this.index), @@ -366,7 +341,6 @@ module.exports = function(Chart) { helpers.extend(rectangle, { // Utility - _chart: this.chart.chart, _xScale: xScale, _yScale: yScale, _datasetIndex: this.index, diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index 1faa10f65..d0f949101 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -37,29 +37,9 @@ module.exports = function(Chart) { } }; - Chart.controllers.bubble = Chart.DatasetController.extend({ - addElements: function() { - var meta = this.getMeta(); - helpers.each(this.getDataset().data, function(value, index) { - meta.data[index] = meta.data[index] || new Chart.elements.Point({ - _chart: this.chart.chart, - _datasetIndex: this.index, - _index: index - }); - }, this); - }, - addElementAndReset: function(index) { - var point = new Chart.elements.Point({ - _chart: this.chart.chart, - _datasetIndex: this.index, - _index: index - }); - // Add to the points array and reset it - this.getMeta().data.splice(index, 0, point); - this.updateElement(point, index, true); - }, + dataElementType: Chart.elements.Point, update: function update(reset) { var meta = this.getMeta(); @@ -69,7 +49,6 @@ module.exports = function(Chart) { helpers.each(points, function(point, index) { this.updateElement(point, index, reset); }, this); - }, updateElement: function(point, index, reset) { @@ -84,7 +63,6 @@ module.exports = function(Chart) { helpers.extend(point, { // Utility - _chart: this.chart.chart, _xScale: xScale, _yScale: yScale, _datasetIndex: this.index, @@ -115,17 +93,6 @@ module.exports = function(Chart) { return value.r || this.chart.options.elements.point.radius; }, - draw: function(ease) { - var easingDecimal = ease || 1; - - // Transition and Draw the Points - helpers.each(this.getMeta().data, function(point, index) { - point.transition(easingDecimal); - point.draw(); - }); - - }, - setHoverStyle: function(point) { // Point var dataset = this.chart.data.datasets[point._datasetIndex]; diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index 00152fb00..9f7cd2f2c 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -113,40 +113,11 @@ module.exports = function(Chart) { Chart.controllers.doughnut = Chart.controllers.pie = Chart.DatasetController.extend({ - // no scales for doughnut + + dataElementType: Chart.elements.Arc, + linkScales: helpers.noop, - addElements: function() { - var _this = this; - var meta = this.getMeta(), - data = meta.data; - helpers.each(_this.getDataset().data, function(value, index) { - data[index] = data[index] || new Chart.elements.Arc({ - _chart: _this.chart.chart, - _datasetIndex: _this.index, - _index: index - }); - }); - }, - - addElementAndReset: function(index, colorForNewElement) { - var _this = this; - var arc = new Chart.elements.Arc({ - _chart: _this.chart.chart, - _datasetIndex: _this.index, - _index: index - }), - ds = _this.getDataset(); - - if (colorForNewElement && helpers.isArray(ds.backgroundColor)) { - ds.backgroundColor.splice(index, 0, colorForNewElement); - } - - // Add to the points array and reset it - _this.getMeta().data.splice(index, 0, arc); - _this.updateElement(arc, index, true); - }, - // Get index of the dataset in relation to the visible datasets. This allows determining the inner and outer radius correctly getRingIndex: function getRingIndex(datasetIndex) { var ringIndex = 0; @@ -232,7 +203,6 @@ module.exports = function(Chart) { helpers.extend(arc, { // Utility - _chart: chart.chart, _datasetIndex: _this.index, _index: index, diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 39d0fca28..4c692d5ab 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -24,40 +24,16 @@ module.exports = function(Chart) { }; Chart.controllers.line = Chart.DatasetController.extend({ - addElements: function() { - var me = this; - var meta = me.getMeta(); - var data = me.getDataset().data || []; - var value, i, ilen; - meta.dataset = meta.dataset || new Chart.elements.Line({ - _chart: me.chart.chart, - _datasetIndex: me.index, - _points: meta.data - }); + datasetElementType: Chart.elements.Line, - for (i=0, ilen=data.length; i Date: Sat, 21 May 2016 22:53:58 +0200 Subject: [PATCH 3/4] Refactor controller scale methods Rewrite these two methods to reduce code duplication. Note that options.scale is not anymore mapped to 'radialScale' ID but to 'scale' ID (see ensureScalesHaveIDs), since this ID is not referenced anywhere in the code base. --- src/core/core.controller.js | 119 ++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 65 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 439cce27d..aa60acc0b 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -98,82 +98,71 @@ module.exports = function(Chart) { return this; }, + ensureScalesHaveIDs: function ensureScalesHaveIDs() { - var defaultXAxisID = 'x-axis-'; - var defaultYAxisID = 'y-axis-'; + var options = this.options; + var scalesOptions = options.scales || {}; + var scaleOptions = options.scale; - if (this.options.scales) { - if (this.options.scales.xAxes && this.options.scales.xAxes.length) { - helpers.each(this.options.scales.xAxes, function(xAxisOptions, index) { - xAxisOptions.id = xAxisOptions.id || (defaultXAxisID + index); - }); - } + helpers.each(scalesOptions.xAxes, function(xAxisOptions, index) { + xAxisOptions.id = xAxisOptions.id || ('x-axis-' + index); + }); - if (this.options.scales.yAxes && this.options.scales.yAxes.length) { - // Build the y axes - helpers.each(this.options.scales.yAxes, function(yAxisOptions, index) { - yAxisOptions.id = yAxisOptions.id || (defaultYAxisID + index); - }); - } + helpers.each(scalesOptions.yAxes, function(yAxisOptions, index) { + yAxisOptions.id = yAxisOptions.id || ('y-axis-' + index); + }); + + if (scaleOptions) { + scaleOptions.id = scaleOptions.id || 'scale'; } }, + + /** + * Builds a map of scale ID to scale object for future lookup. + */ buildScales: function buildScales() { - // Map of scale ID to scale object so we can lookup later - this.scales = {}; + var me = this; + var options = me.options; + var scales = me.scales = {}; + var items = []; - // Build the x axes - if (this.options.scales) { - if (this.options.scales.xAxes && this.options.scales.xAxes.length) { - helpers.each(this.options.scales.xAxes, function(xAxisOptions, index) { - var xType = helpers.getValueOrDefault(xAxisOptions.type, 'category'); - var ScaleClass = Chart.scaleService.getScaleConstructor(xType); - if (ScaleClass) { - var scale = new ScaleClass({ - ctx: this.chart.ctx, - options: xAxisOptions, - chart: this, - id: xAxisOptions.id - }); - - this.scales[scale.id] = scale; - } - }, this); - } - - if (this.options.scales.yAxes && this.options.scales.yAxes.length) { - // Build the y axes - helpers.each(this.options.scales.yAxes, function(yAxisOptions, index) { - var yType = helpers.getValueOrDefault(yAxisOptions.type, 'linear'); - var ScaleClass = Chart.scaleService.getScaleConstructor(yType); - if (ScaleClass) { - var scale = new ScaleClass({ - ctx: this.chart.ctx, - options: yAxisOptions, - chart: this, - id: yAxisOptions.id - }); - - this.scales[scale.id] = scale; - } - }, this); - } + if (options.scales) { + items = items.concat( + (options.scales.xAxes || []).map(function(xAxisOptions) { + return { options: xAxisOptions, dtype: 'category' }; }), + (options.scales.yAxes || []).map(function(yAxisOptions) { + return { options: yAxisOptions, dtype: 'linear' }; })); } - if (this.options.scale) { - // Build radial axes - var ScaleClass = Chart.scaleService.getScaleConstructor(this.options.scale.type); - if (ScaleClass) { - var scale = new ScaleClass({ - ctx: this.chart.ctx, - options: this.options.scale, - chart: this - }); - this.scale = scale; - - this.scales.radialScale = scale; - } + if (options.scale) { + items.push({ options: options.scale, dtype: 'radialLinear', isDefault: true }); } + helpers.each(items, function(item, index) { + var scaleOptions = item.options; + var scaleType = helpers.getValueOrDefault(scaleOptions.type, item.dtype); + var scaleClass = Chart.scaleService.getScaleConstructor(scaleType); + if (!scaleClass) { + return; + } + + var scale = new scaleClass({ + id: scaleOptions.id, + options: scaleOptions, + ctx: me.chart.ctx, + chart: me + }); + + scales[scale.id] = scale; + + // TODO(SB): I think we should be able to remove this custom case (options.scale) + // and consider it as a regular scale part of the "scales"" map only! This would + // make the logic easier and remove some useless? custom code. + if (item.isDefault) { + me.scale = scale; + } + }); + Chart.scaleService.addScalesToLayout(this); }, From 7f71990a40d12c019b5ba341afd42d93e20ebc42 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Sun, 22 May 2016 00:21:59 +0200 Subject: [PATCH 4/4] Decomplexify Chart.core.controller.eventHandler Refactor redundant code, use local variables and introduce a new helper to compare arrays (Chart.helpers.arrayEquals). --- src/core/core.controller.js | 170 ++++++++++++++++-------------------- src/core/core.helpers.js | 24 +++++ 2 files changed, 99 insertions(+), 95 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index aa60acc0b..36edf9e7a 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -388,6 +388,20 @@ module.exports = function(Chart) { return elementsArray; }, + getElementsAtEventForMode: function(e, mode) { + var me = this; + switch (mode) { + case 'single': + return me.getElementAtEvent(e); + case 'label': + return me.getElementsAtEvent(e); + case 'dataset': + return me.getDatasetAtEvent(e); + default: + return e; + } + }, + getDatasetAtEvent: function(e) { var elementsArray = this.getElementAtEvent(e); @@ -484,140 +498,106 @@ module.exports = function(Chart) { this.eventHandler(evt); }); }, + + updateHoverStyle: function(elements, mode, enabled) { + var method = enabled? 'setHoverStyle' : 'removeHoverStyle'; + var element, i, ilen; + + switch (mode) { + case 'single': + elements = [ elements[0] ]; + break; + case 'label': + case 'dataset': + // elements = elements; + break; + default: + // unsupported mode + return; + } + + for (i=0, ilen=elements.length; i