review comments

chartjs · Oct 22, 2019 · a637ebc · a637ebc
1 parent 43ea2b4
commit a637ebc
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 28 deletions.
diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js
@@ -315,24 +315,4 @@ module.exports = DatasetController.extend({
 		model.borderWidth = valueOrDefault(options.hoverBorderWidth, options.borderWidth);
 		model.radius = valueOrDefault(options.hoverRadius, options.radius);
 	},
-
-	/**
-	 * @protected
-	 */
-	setDatasetHoverStyle: function() {
-		var line = this.getMeta().dataset;
-		var model = line._model;
-		var config = this._config;
-		var getHoverColor = helpers.getHoverColor;
-
-		line.$previousStyle = {
-			backgroundColor: model.backgroundColor,
-			borderColor: model.borderColor,
-			borderWidth: model.borderWidth
-		};
-
-		model.backgroundColor = valueOrDefault(config.hoverBackgroundColor, getHoverColor(config.backgroundColor));
-		model.borderColor = valueOrDefault(config.hoverBorderColor, getHoverColor(config.borderColor));
-		model.borderWidth = valueOrDefault(config.hoverBorderWidth, config.borderWidth);
-	},
 });
diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js
@@ -356,29 +356,31 @@ helpers.extend(DatasetController.prototype, {
 	/**
 	 * @private
 	 */
-	_resolveDatasetElementOptions: function(element) {
+	_resolveDatasetElementOptions: function(element, hover) {
 		var me = this;
 		var chart = me.chart;
 		var datasetOpts = me._config;
 		var custom = element.custom || {};
 		var options = chart.options.elements[me.datasetElementType.prototype._type] || {};
 		var elementOptions = me._datasetElementOptions;
 		var values = {};
-		var i, ilen, key;
+		var i, ilen, key, readKey;
 
 		// Scriptable options
 		var context = {
 			chart: chart,
 			dataset: me.getDataset(),
-			datasetIndex: me.index
+			datasetIndex: me.index,
+			hover: hover
 		};
 
 		for (i = 0, ilen = elementOptions.length; i < ilen; ++i) {
 			key = elementOptions[i];
+			readKey = hover ? 'hover' + key.charAt(0).toUpperCase() + key.slice(1) : key;
 			values[key] = resolve([
-				custom[key],
-				datasetOpts[key],
-				options[key]
+				custom[readKey],
+				datasetOpts[readKey],
+				options[readKey]
 			], context);
 		}
 
@@ -469,10 +471,34 @@ helpers.extend(DatasetController.prototype, {
 	},
 
 	removeDatasetHoverStyle: function() {
-		this.removeHoverStyle(this.getMeta().dataset);
+		var element = this.getMeta().dataset;
+
+		if (element) {
+			this.removeHoverStyle(element);
+		}
 	},
 
-	setDatasetHoverStyle: helpers.noop,
+	setDatasetHoverStyle: function() {
+		var element = this.getMeta().dataset;
+		var prev = {};
+		var i, ilen, key, keys, hoverOptions, model;
+
+		if (!element) {
+			return;
+		}
+
+		model = element._model;
+		hoverOptions = this._resolveDatasetElementOptions(element, true);
+
+		keys = Object.keys(hoverOptions);
+		for (i = 0, ilen = keys.length; i < ilen; ++i) {
+			key = keys[i];
+			prev[key] = model[key];
+			model[key] = hoverOptions[key];
+		}
+
+		element.$previousStyle = prev;
+	},
 
 	/**
 	 * @private

diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js
@@ -895,6 +895,34 @@ describe('Chart.controllers.line', function() {
 			expect(point._model.borderWidth).toBe(2);
 			expect(point._model.radius).toBe(3);
 		});
+
+		it ('should handle dataset hover styles defined via dataset properties', function() {
+			var chart = this.chart;
+			var point = chart.getDatasetMeta(0).data[0];
+			var dataset = chart.getDatasetMeta(0).dataset;
+
+			Chart.helpers.merge(chart.data.datasets[0], {
+				backgroundColor: '#AAA',
+				borderColor: '#BBB',
+				borderWidth: 6,
+				hoverBackgroundColor: '#000',
+				hoverBorderColor: '#111',
+				hoverBorderWidth: 12
+			});
+
+			chart.options.hover = {mode: 'dataset'};
+			chart.update();
+
+			jasmine.triggerMouseEvent(chart, 'mousemove', point);
+			expect(dataset._model.backgroundColor).toBe('#000');
+			expect(dataset._model.borderColor).toBe('#111');
+			expect(dataset._model.borderWidth).toBe(12);
+
+			jasmine.triggerMouseEvent(chart, 'mouseout', point);
+			expect(dataset._model.backgroundColor).toBe('#AAA');
+			expect(dataset._model.borderColor).toBe('#BBB');
+			expect(dataset._model.borderWidth).toBe(6);
+		});
 	});
 
 	it('should allow 0 as a point border width', function() {