Enforce tooltip item label and value to be strings (chartjs#6030)

Also update the docs for `xLabel` and `yLabel` to also accept a `number`.
janelledement · Feb 10, 2019 · de1b856 · de1b856
1 parent ca22a46
commit de1b856
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 57 deletions.
diff --git a/docs/configuration/tooltip.md b/docs/configuration/tooltip.md
@@ -170,11 +170,11 @@ The tooltip items passed to the tooltip callbacks implement the following interf
 
     // X Value of the tooltip
     // (deprecated) use `value` or `label` instead
-    xLabel: string,
+    xLabel: number | string,
 
     // Y value of the tooltip
     // (deprecated) use `value` or `label` instead
-    yLabel: string,
+    yLabel: number | string,
 
     // Index of the dataset the item comes from
     datasetIndex: number,

diff --git a/src/core/core.tooltip.js b/src/core/core.tooltip.js
@@ -217,8 +217,8 @@ function createTooltipItem(element) {
 	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) : '',
+		label: indexScale ? '' + indexScale.getLabelForIndex(index, datasetIndex) : '',
+		value: valueScale ? '' + valueScale.getLabelForIndex(index, datasetIndex) : '',
 		index: index,
 		datasetIndex: datasetIndex,
 		x: element._model.x,

diff --git a/test/specs/core.tooltip.tests.js b/test/specs/core.tooltip.tests.js
@@ -704,64 +704,73 @@ describe('Core.Tooltip', function() {
 		}));
 	});
 
-	it('Should have dataPoints', function() {
-		var chart = window.acquireChart({
-			type: 'line',
-			data: {
-				datasets: [{
-					label: 'Dataset 1',
-					data: [10, 20, 30],
-					pointHoverBorderColor: 'rgb(255, 0, 0)',
-					pointHoverBackgroundColor: 'rgb(0, 255, 0)'
-				}, {
-					label: 'Dataset 2',
-					data: [40, 40, 40],
-					pointHoverBorderColor: 'rgb(0, 0, 255)',
-					pointHoverBackgroundColor: 'rgb(0, 255, 255)'
-				}],
-				labels: ['Point 1', 'Point 2', 'Point 3']
-			},
-			options: {
-				tooltips: {
-					mode: 'single'
+	['line', 'bar', 'horizontalBar'].forEach(function(type) {
+		it('Should have dataPoints in a ' + type + ' chart', function() {
+			var chart = window.acquireChart({
+				type: type,
+				data: {
+					datasets: [{
+						label: 'Dataset 1',
+						data: [10, 20, 30],
+						pointHoverBorderColor: 'rgb(255, 0, 0)',
+						pointHoverBackgroundColor: 'rgb(0, 255, 0)'
+					}, {
+						label: 'Dataset 2',
+						data: [40, 40, 40],
+						pointHoverBorderColor: 'rgb(0, 0, 255)',
+						pointHoverBackgroundColor: 'rgb(0, 255, 255)'
+					}],
+					labels: ['Point 1', 'Point 2', 'Point 3']
+				},
+				options: {
+					tooltips: {
+						mode: 'single'
+					}
 				}
-			}
-		});
+			});
 
-		// Trigger an event over top of the
-		var pointIndex = 1;
-		var datasetIndex = 0;
-		var meta = chart.getDatasetMeta(datasetIndex);
-		var point = meta.data[pointIndex];
-		var node = chart.canvas;
-		var rect = node.getBoundingClientRect();
-		var evt = new MouseEvent('mousemove', {
-			view: window,
-			bubbles: true,
-			cancelable: true,
-			clientX: rect.left + point._model.x,
-			clientY: rect.top + point._model.y
-		});
+			// Trigger an event over top of the element
+			var pointIndex = 1;
+			var datasetIndex = 0;
+			var meta = chart.getDatasetMeta(datasetIndex);
+			var point = meta.data[pointIndex];
+			var node = chart.canvas;
+			var rect = node.getBoundingClientRect();
+			var evt = new MouseEvent('mousemove', {
+				view: window,
+				bubbles: true,
+				cancelable: true,
+				clientX: Math.round(rect.left + point._model.x),
+				clientY: Math.round(rect.top + point._model.y)
+			});
 
-		// Manually trigger rather than having an async test
-		node.dispatchEvent(evt);
+			// Manually trigger rather than having an async test
+			node.dispatchEvent(evt);
 
-		// Check and see if tooltip was displayed
-		var tooltip = chart.tooltip;
+			// Check and see if tooltip was displayed
+			var tooltip = chart.tooltip;
 
-		expect(tooltip._view instanceof Object).toBe(true);
-		expect(tooltip._view.dataPoints instanceof Array).toBe(true);
-		expect(tooltip._view.dataPoints.length).toEqual(1);
-		expect(tooltip._view.dataPoints[0].index).toEqual(pointIndex);
-		expect(tooltip._view.dataPoints[0].datasetIndex).toEqual(datasetIndex);
-		expect(tooltip._view.dataPoints[0].xLabel).toEqual(
-			chart.data.labels[pointIndex]
-		);
-		expect(tooltip._view.dataPoints[0].yLabel).toEqual(
-			chart.data.datasets[datasetIndex].data[pointIndex]
-		);
-		expect(tooltip._view.dataPoints[0].x).toBeCloseToPixel(point._model.x);
-		expect(tooltip._view.dataPoints[0].y).toBeCloseToPixel(point._model.y);
+			expect(tooltip._view instanceof Object).toBe(true);
+			expect(tooltip._view.dataPoints instanceof Array).toBe(true);
+			expect(tooltip._view.dataPoints.length).toBe(1);
+
+			var tooltipItem = tooltip._view.dataPoints[0];
+
+			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');
+			expect(tooltipItem.value).toBe('' + chart.data.datasets[datasetIndex].data[pointIndex]);
+			expect(tooltipItem.x).toBeCloseToPixel(point._model.x);
+			expect(tooltipItem.y).toBeCloseToPixel(point._model.y);
+		});
 	});
 
 	it('Should not update if active element has not changed', function() {