From 08d133817b1c6454334db165f87373d2ed760b61 Mon Sep 17 00:00:00 2001 From: Ben McCann <322311+benmccann@users.noreply.github.com> Date: Sun, 27 Oct 2019 13:35:15 -0700 Subject: [PATCH] Remove xLabel and yLabel from tooltip and chart and scales from element (#6606) * Element should not have reference to chart * Remove scales from element * Remove deprecated xLabel and yLabel --- docs/configuration/tooltip.md | 8 ---- src/controllers/controller.bar.js | 3 -- src/controllers/controller.bubble.js | 4 +- src/controllers/controller.line.js | 9 +---- src/controllers/controller.polarArea.js | 3 +- src/controllers/controller.radar.js | 3 -- src/controllers/controller.scatter.js | 2 +- src/core/core.controller.js | 1 - src/core/core.datasetController.js | 4 +- src/core/core.tooltip.js | 18 +++------ src/elements/element.arc.js | 2 +- src/elements/element.line.js | 2 +- src/elements/element.point.js | 2 +- src/elements/element.rectangle.js | 2 +- src/plugins/plugin.filler.js | 7 ++-- test/specs/controller.bar.tests.js | 2 - test/specs/controller.line.tests.js | 2 - test/specs/core.tooltip.tests.js | 10 +---- test/specs/element.line.tests.js | 52 +++++++------------------ test/specs/element.point.tests.js | 8 +--- 20 files changed, 37 insertions(+), 107 deletions(-) diff --git a/docs/configuration/tooltip.md b/docs/configuration/tooltip.md index 213f5c4b8..8ef57bbd6 100644 --- a/docs/configuration/tooltip.md +++ b/docs/configuration/tooltip.md @@ -183,14 +183,6 @@ The tooltip items passed to the tooltip callbacks implement the following interf // Value for the tooltip value: string, - // X Value of the tooltip - // (deprecated) use `value` or `label` instead - xLabel: number | string, - - // Y value of the tooltip - // (deprecated) use `value` or `label` instead - yLabel: number | string, - // Index of the dataset the item comes from datasetIndex: number, diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index fa59c9465..398cdd9cd 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -177,12 +177,9 @@ module.exports = DatasetController.extend({ updateElement: function(rectangle, index, reset) { var me = this; - var meta = me.getMeta(); var dataset = me.getDataset(); var options = me._resolveDataElementOptions(index); - rectangle._xScale = me.getScaleForId(meta.xAxisID); - rectangle._yScale = me.getScaleForId(meta.yAxisID); rectangle._datasetIndex = me.index; rectangle._index = index; rectangle._model = { diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index 35a92992a..5549097b7 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -35,7 +35,7 @@ defaults._set('bubble', { label: function(item, data) { var datasetLabel = data.datasets[item.datasetIndex].label || ''; var dataPoint = data.datasets[item.datasetIndex].data[item.index]; - return datasetLabel + ': (' + item.xLabel + ', ' + item.yLabel + ', ' + dataPoint.r + ')'; + return datasetLabel + ': (' + item.label + ', ' + item.value + ', ' + dataPoint.r + ')'; } } } @@ -92,8 +92,6 @@ module.exports = DatasetController.extend({ var x = reset ? xScale.getPixelForDecimal(0.5) : xScale.getPixelForValue(typeof data === 'object' ? data : NaN, index, dsIndex); var y = reset ? yScale.getBasePixel() : yScale.getPixelForValue(data, index, dsIndex); - point._xScale = xScale; - point._yScale = yScale; point._options = options; point._datasetIndex = dsIndex; point._index = index; diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index f8f415ef9..de689941e 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -88,7 +88,6 @@ module.exports = DatasetController.extend({ } // Utility - line._scale = me._yScale; line._datasetIndex = me.index; // Data line._children = points; @@ -119,19 +118,15 @@ module.exports = DatasetController.extend({ var dataset = me.getDataset(); var datasetIndex = me.index; var value = dataset.data[index]; - var xScale = me._xScale; - var yScale = me._yScale; var lineModel = meta.dataset._model; var x, y; var options = me._resolveDataElementOptions(index); - x = xScale.getPixelForValue(typeof value === 'object' ? value : NaN, index, datasetIndex); - y = reset ? yScale.getBasePixel() : me.calculatePointY(value, index, datasetIndex); + x = me._xScale.getPixelForValue(typeof value === 'object' ? value : NaN, index, datasetIndex); + y = reset ? me._yScale.getBasePixel() : me.calculatePointY(value, index, datasetIndex); // Utility - point._xScale = xScale; - point._yScale = yScale; point._options = options; point._datasetIndex = datasetIndex; point._index = index; diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 9178860b6..b73a938bb 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -98,7 +98,7 @@ defaults._set('polarArea', { return ''; }, label: function(item, data) { - return data.labels[item.index] + ': ' + item.yLabel; + return data.labels[item.index] + ': ' + item.value; } } } @@ -207,7 +207,6 @@ module.exports = DatasetController.extend({ // Utility _datasetIndex: me.index, _index: index, - _scale: scale, // Desired view properties _model: { diff --git a/src/controllers/controller.radar.js b/src/controllers/controller.radar.js index d2a0d9551..a690c75d0 100644 --- a/src/controllers/controller.radar.js +++ b/src/controllers/controller.radar.js @@ -76,7 +76,6 @@ module.exports = DatasetController.extend({ var meta = me.getMeta(); var line = meta.dataset; var points = meta.data || []; - var scale = me.chart.scale; var config = me._config; var i, ilen; @@ -86,7 +85,6 @@ module.exports = DatasetController.extend({ } // Utility - line._scale = scale; line._datasetIndex = me.index; // Data line._children = points; @@ -121,7 +119,6 @@ module.exports = DatasetController.extend({ var y = reset ? scale.yCenter : pointPosition.y; // Utility - point._scale = scale; point._options = options; point._datasetIndex = me.index; point._index = index; diff --git a/src/controllers/controller.scatter.js b/src/controllers/controller.scatter.js index 177e56320..1b2061b4a 100644 --- a/src/controllers/controller.scatter.js +++ b/src/controllers/controller.scatter.js @@ -27,7 +27,7 @@ defaults._set('scatter', { return ''; // doesn't make sense for scatter since data are formatted as a point }, label: function(item) { - return '(' + item.xLabel + ', ' + item.yLabel + ')'; + return '(' + item.label + ', ' + item.value + ')'; } } } diff --git a/src/core/core.controller.js b/src/core/core.controller.js index f56b04901..849d31d35 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -939,7 +939,6 @@ helpers.extend(Chart.prototype, /** @lends Chart */ { var me = this; me.tooltip = new Tooltip({ _chart: me, - _chartInstance: me, // deprecated, backward compatibility _data: me.data, _options: me.options.tooltips }, me); diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index d8adf9fd5..d885c53c5 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -209,7 +209,7 @@ helpers.extend(DatasetController.prototype, { var me = this; var type = me.datasetElementType; return type && new type({ - _chart: me.chart, + _ctx: me.chart.ctx, _datasetIndex: me.index }); }, @@ -218,7 +218,7 @@ helpers.extend(DatasetController.prototype, { var me = this; var type = me.dataElementType; return type && new type({ - _chart: me.chart, + _ctx: me.chart.ctx, _datasetIndex: me.index, _index: index }); diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js index f42ede285..d4a892e2e 100644 --- a/src/core/core.tooltip.js +++ b/src/core/core.tooltip.js @@ -49,8 +49,6 @@ defaults._set('global', { var item = tooltipItems[0]; if (item.label) { title = item.label; - } else if (item.xLabel) { - title = item.xLabel; } else if (labelCount > 0 && item.index < labelCount) { title = labels[item.index]; } @@ -73,8 +71,6 @@ defaults._set('global', { } if (!helpers.isNullOrUndef(tooltipItem.value)) { label += tooltipItem.value; - } else { - label += tooltipItem.yLabel; } return label; }, @@ -208,18 +204,14 @@ function splitNewlines(str) { * @param element - the chart element (point, arc, bar) to create the tooltip item for * @return new tooltip item */ -function createTooltipItem(element) { - var xScale = element._xScale; - var yScale = element._yScale || element._scale; // handle radar || polarArea charts - var index = element._index; +function createTooltipItem(chart, element) { var datasetIndex = element._datasetIndex; - var controller = element._chart.getDatasetMeta(datasetIndex).controller; + var index = element._index; + var controller = chart.getDatasetMeta(datasetIndex).controller; var indexScale = controller._getIndexScale(); var valueScale = controller._getValueScale(); return { - xLabel: xScale ? xScale.getLabelForIndex(index, datasetIndex) : '', - yLabel: yScale ? yScale.getLabelForIndex(index, datasetIndex) : '', label: indexScale ? '' + indexScale.getLabelForIndex(index, datasetIndex) : '', value: valueScale ? '' + valueScale.getLabelForIndex(index, datasetIndex) : '', index: index, @@ -359,7 +351,7 @@ function getTooltipSize(tooltip, model) { function determineAlignment(tooltip, size) { var model = tooltip._model; var chart = tooltip._chart; - var chartArea = tooltip._chart.chartArea; + var chartArea = chart.chartArea; var xAlign = 'center'; var yAlign = 'center'; @@ -612,7 +604,7 @@ var exports = Element.extend({ var tooltipItems = []; for (i = 0, len = active.length; i < len; ++i) { - tooltipItems.push(createTooltipItem(active[i])); + tooltipItems.push(createTooltipItem(me._chart, active[i])); } // If the user provided a filter function, use it to modify the tooltip items diff --git a/src/elements/element.arc.js b/src/elements/element.arc.js index 3dab9ad62..84b5e6335 100644 --- a/src/elements/element.arc.js +++ b/src/elements/element.arc.js @@ -160,7 +160,7 @@ module.exports = Element.extend({ }, draw: function() { - var ctx = this._chart.ctx; + var ctx = this._ctx; var vm = this._view; var pixelMargin = (vm.borderAlign === 'inner') ? 0.33 : 0; var arc = { diff --git a/src/elements/element.line.js b/src/elements/element.line.js index 782772bfb..98bb48c40 100644 --- a/src/elements/element.line.js +++ b/src/elements/element.line.js @@ -31,7 +31,7 @@ module.exports = Element.extend({ draw: function() { var me = this; var vm = me._view; - var ctx = me._chart.ctx; + var ctx = me._ctx; var spanGaps = vm.spanGaps; var points = me._children.slice(); // clone array var globalDefaults = defaults.global; diff --git a/src/elements/element.point.js b/src/elements/element.point.js index 9e1456a2d..dc020c86a 100644 --- a/src/elements/element.point.js +++ b/src/elements/element.point.js @@ -69,7 +69,7 @@ module.exports = Element.extend({ draw: function(chartArea) { var vm = this._view; - var ctx = this._chart.ctx; + var ctx = this._ctx; var pointStyle = vm.pointStyle; var rotation = vm.rotation; var radius = vm.radius; diff --git a/src/elements/element.rectangle.js b/src/elements/element.rectangle.js index d1ef58c13..254d77514 100644 --- a/src/elements/element.rectangle.js +++ b/src/elements/element.rectangle.js @@ -134,7 +134,7 @@ module.exports = Element.extend({ _type: 'rectangle', draw: function() { - var ctx = this._chart.ctx; + var ctx = this._ctx; var vm = this._view; var rects = boundingRects(vm); var outer = rects.outer; diff --git a/src/plugins/plugin.filler.js b/src/plugins/plugin.filler.js index 8fed6d905..00000e4fd 100644 --- a/src/plugins/plugin.filler.js +++ b/src/plugins/plugin.filler.js @@ -104,7 +104,7 @@ function decodeFill(el, index, count) { function computeLinearBoundary(source) { var model = source.el._model || {}; - var scale = source.el._scale || {}; + var scale = source.scale || {}; var fill = source.fill; var target = null; var horizontal; @@ -145,7 +145,7 @@ function computeLinearBoundary(source) { } function computeCircularBoundary(source) { - var scale = source.el._scale; + var scale = source.scale; var options = scale.options; var length = scale.chart.data.labels.length; var fill = source.fill; @@ -174,7 +174,7 @@ function computeCircularBoundary(source) { } function computeBoundary(source) { - var scale = source.el._scale || {}; + var scale = source.scale || {}; if (scale.getPointPositionForValue) { return computeCircularBoundary(source); @@ -333,6 +333,7 @@ module.exports = { visible: chart.isDatasetVisible(i), fill: decodeFill(el, i, count), chart: chart, + scale: meta.controller.getScaleForId(meta.yAxisID) || chart.scale, el: el }; } diff --git a/test/specs/controller.bar.tests.js b/test/specs/controller.bar.tests.js index dffb668ba..8d864af21 100644 --- a/test/specs/controller.bar.tests.js +++ b/test/specs/controller.bar.tests.js @@ -741,8 +741,6 @@ describe('Chart.controllers.bar', function() { ].forEach(function(expected, i) { expect(meta.data[i]._datasetIndex).toBe(1); expect(meta.data[i]._index).toBe(i); - expect(meta.data[i]._xScale).toBe(chart.scales.firstXScaleID); - expect(meta.data[i]._yScale).toBe(chart.scales.firstYScaleID); expect(meta.data[i]._model.x).toBeCloseToPixel(expected.x); expect(meta.data[i]._model.y).toBeCloseToPixel(expected.y); expect(meta.data[i]._model.base).toBeCloseToPixel(1024); diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index ac17061e2..6d3e861f8 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -217,8 +217,6 @@ describe('Chart.controllers.line', function() { ].forEach(function(expected, i) { expect(meta.data[i]._datasetIndex).toBe(0); expect(meta.data[i]._index).toBe(i); - expect(meta.data[i]._xScale).toBe(chart.scales.firstXScaleID); - expect(meta.data[i]._yScale).toBe(chart.scales.firstYScaleID); expect(meta.data[i]._model.x).toBeCloseToPixel(expected.x); expect(meta.data[i]._model.y).toBeCloseToPixel(expected.y); expect(meta.data[i]._model).toEqual(jasmine.objectContaining({ diff --git a/test/specs/core.tooltip.tests.js b/test/specs/core.tooltip.tests.js index 89898677a..737b50c78 100755 --- a/test/specs/core.tooltip.tests.js +++ b/test/specs/core.tooltip.tests.js @@ -15,8 +15,8 @@ describe('Core.Tooltip', function() { var tooltipItem = { index: 1, datasetIndex: 0, - xLabel: 'Point 2', - yLabel: '20' + label: 'Point 2', + value: '20' }; var label = Chart.defaults.global.tooltips.callbacks.label(tooltipItem, data); @@ -907,12 +907,6 @@ describe('Core.Tooltip', function() { expect(tooltipItem.index).toBe(pointIndex); expect(tooltipItem.datasetIndex).toBe(datasetIndex); - var indexLabel = type !== 'horizontalBar' ? 'xLabel' : 'yLabel'; - expect(typeof tooltipItem[indexLabel]).toBe('string'); - expect(tooltipItem[indexLabel]).toBe(chart.data.labels[pointIndex]); - var valueLabel = type !== 'horizontalBar' ? 'yLabel' : 'xLabel'; - expect(typeof tooltipItem[valueLabel]).toBe('number'); - expect(tooltipItem[valueLabel]).toBe(chart.data.datasets[datasetIndex].data[pointIndex]); expect(typeof tooltipItem.label).toBe('string'); expect(tooltipItem.label).toBe(chart.data.labels[pointIndex]); expect(typeof tooltipItem.value).toBe('string'); diff --git a/test/specs/element.line.tests.js b/test/specs/element.line.tests.js index 5b423ce0d..e509bbfc9 100644 --- a/test/specs/element.line.tests.js +++ b/test/specs/element.line.tests.js @@ -65,9 +65,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -183,9 +181,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -306,9 +302,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -435,9 +429,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -572,9 +564,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -695,9 +685,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -819,9 +807,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -932,9 +918,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -1049,9 +1033,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -1154,9 +1136,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -1267,9 +1247,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -1381,9 +1359,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { @@ -1494,9 +1470,7 @@ describe('Chart.elements.Line', function() { var line = new Chart.elements.Line({ _datasetindex: 2, - _chart: { - ctx: mockContext, - }, + _ctx: mockContext, _children: points, // Need to provide some settings _view: { diff --git a/test/specs/element.point.tests.js b/test/specs/element.point.tests.js index 887250acd..f6700e347 100644 --- a/test/specs/element.point.tests.js +++ b/test/specs/element.point.tests.js @@ -99,9 +99,7 @@ describe('Chart.elements.Point', function() { var point = new Chart.elements.Point({ _datasetIndex: 2, _index: 1, - _chart: { - ctx: mockContext, - } + _ctx: mockContext }); // Attach a view object as if we were the controller @@ -147,9 +145,7 @@ describe('Chart.elements.Point', function() { var point = new Chart.elements.Point({ _datasetIndex: 2, _index: 1, - _chart: { - ctx: mockContext, - } + _ctx: mockContext }); // Attach a view object as if we were the controller -- 2.47.2