Skip to content

Commit

Permalink
Restore original styles when removing hover (chartjs#5570)
Browse files Browse the repository at this point in the history 
Refactor `updateElement` and `removeHoverStyle` and fix tests.
  • Loading branch information
@benmccann @simonbrunel
benmccann authored and simonbrunel committed Jun 26, 2018
1 parent cbe465f commit e49b268
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 96 deletions.
1 change: 0 additions & 1 deletion karma.conf.js
View file
Expand Up @@ -19,7 +19,6 @@ module.exports = function(karma) {
// These settings deal with browser disconnects. We had seen test flakiness from Firefox
// [Firefox 56.0.0 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
// https://github.com/jasmine/jasmine/issues/1327#issuecomment-332939551
browserNoActivityTimeout: 60000,
browserDisconnectTolerance: 3
};

Expand Down
23 changes: 0 additions & 23 deletions src/controllers/controller.bar.js
View file
Expand Up @@ -461,29 +461,6 @@ module.exports = function(Chart) {

helpers.canvas.unclipArea(chart.ctx);
},

setHoverStyle: function(rectangle) {
var dataset = this.chart.data.datasets[rectangle._datasetIndex];
var index = rectangle._index;
var custom = rectangle.custom || {};
var model = rectangle._model;

model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : helpers.valueAtIndexOrDefault(dataset.hoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : helpers.valueAtIndexOrDefault(dataset.hoverBorderColor, index, helpers.getHoverColor(model.borderColor));
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : helpers.valueAtIndexOrDefault(dataset.hoverBorderWidth, index, model.borderWidth);
},

removeHoverStyle: function(rectangle) {
var dataset = this.chart.data.datasets[rectangle._datasetIndex];
var index = rectangle._index;
var custom = rectangle.custom || {};
var model = rectangle._model;
var rectangleElementOptions = this.chart.options.elements.rectangle;

model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : helpers.valueAtIndexOrDefault(dataset.backgroundColor, index, rectangleElementOptions.backgroundColor);
model.borderColor = custom.borderColor ? custom.borderColor : helpers.valueAtIndexOrDefault(dataset.borderColor, index, rectangleElementOptions.borderColor);
model.borderWidth = custom.borderWidth ? custom.borderWidth : helpers.valueAtIndexOrDefault(dataset.borderWidth, index, rectangleElementOptions.borderWidth);
}
});

Chart.controllers.horizontalBar = Chart.controllers.bar.extend({
Expand Down
20 changes: 6 additions & 14 deletions src/controllers/controller.bubble.js
View file
Expand Up @@ -102,26 +102,18 @@ module.exports = function(Chart) {
setHoverStyle: function(point) {
var model = point._model;
var options = point._options;

point.$previousStyle = {
backgroundColor: model.backgroundColor,
borderColor: model.borderColor,
borderWidth: model.borderWidth,
radius: model.radius
};
model.backgroundColor = helpers.valueOrDefault(options.hoverBackgroundColor, helpers.getHoverColor(options.backgroundColor));
model.borderColor = helpers.valueOrDefault(options.hoverBorderColor, helpers.getHoverColor(options.borderColor));
model.borderWidth = helpers.valueOrDefault(options.hoverBorderWidth, options.borderWidth);
model.radius = options.radius + options.hoverRadius;
},

/**
* @protected
*/
removeHoverStyle: function(point) {
var model = point._model;
var options = point._options;

model.backgroundColor = options.backgroundColor;
model.borderColor = options.borderColor;
model.borderWidth = options.borderWidth;
model.radius = options.radius;
},

/**
* @private
*/
Expand Down
12 changes: 7 additions & 5 deletions src/controllers/controller.doughnut.js
View file
Expand Up @@ -229,8 +229,14 @@ module.exports = function(Chart) {
});

var model = arc._model;

// Resets the visual styles
this.removeHoverStyle(arc);
var custom = arc.custom || {};
var valueOrDefault = helpers.valueAtIndexOrDefault;
var elementOpts = this.chart.options.elements.arc;
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);

// Set correct angles if not resetting
if (!reset || !animationOpts.animateRotate) {
Expand All @@ -246,10 +252,6 @@ module.exports = function(Chart) {
arc.pivot();
},

removeHoverStyle: function(arc) {
Chart.DatasetController.prototype.removeHoverStyle.call(this, arc, this.chart.options.elements.arc);
},

calculateTotal: function() {
var dataset = this.getDataset();
var meta = this.getMeta();
Expand Down
37 changes: 13 additions & 24 deletions src/controllers/controller.line.js
View file
Expand Up @@ -307,35 +307,24 @@ module.exports = function(Chart) {
}
},

setHoverStyle: function(point) {
setHoverStyle: function(element) {
// Point
var dataset = this.chart.data.datasets[point._datasetIndex];
var index = point._index;
var custom = point.custom || {};
var model = point._model;
var dataset = this.chart.data.datasets[element._datasetIndex];
var index = element._index;
var custom = element.custom || {};
var model = element._model;

element.$previousStyle = {
backgroundColor: model.backgroundColor,
borderColor: model.borderColor,
borderWidth: model.borderWidth,
radius: model.radius
};

model.radius = custom.hoverRadius || helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
model.backgroundColor = custom.hoverBackgroundColor || helpers.valueAtIndexOrDefault(dataset.pointHoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
model.borderColor = custom.hoverBorderColor || helpers.valueAtIndexOrDefault(dataset.pointHoverBorderColor, index, helpers.getHoverColor(model.borderColor));
model.borderWidth = custom.hoverBorderWidth || helpers.valueAtIndexOrDefault(dataset.pointHoverBorderWidth, index, model.borderWidth);
model.radius = custom.hoverRadius || helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
},

removeHoverStyle: function(point) {
var me = this;
var dataset = me.chart.data.datasets[point._datasetIndex];
var index = point._index;
var custom = point.custom || {};
var model = point._model;

// Compatibility: If the properties are defined with only the old name, use those values
if ((dataset.radius !== undefined) && (dataset.pointRadius === undefined)) {
dataset.pointRadius = dataset.radius;
}

model.radius = custom.radius || helpers.valueAtIndexOrDefault(dataset.pointRadius, index, me.chart.options.elements.point.radius);
model.backgroundColor = me.getPointBackgroundColor(point, index);
model.borderColor = me.getPointBorderColor(point, index);
model.borderWidth = me.getPointBorderWidth(point, index);
}
});
};
13 changes: 8 additions & 5 deletions src/controllers/controller.polarArea.js
View file
Expand Up @@ -199,13 +199,16 @@ module.exports = function(Chart) {
});

// Apply border and fill style
me.removeHoverStyle(arc);
var elementOpts = this.chart.options.elements.arc;
var custom = arc.custom || {};
var valueOrDefault = helpers.valueAtIndexOrDefault;
var model = arc._model;

arc.pivot();
},
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);

removeHoverStyle: function(arc) {
Chart.DatasetController.prototype.removeHoverStyle.call(this, arc, this.chart.options.elements.arc);
arc.pivot();
},

countVisibleElements: function() {
Expand Down
20 changes: 7 additions & 13 deletions src/controllers/controller.radar.js
View file
Expand Up @@ -146,23 +146,17 @@ module.exports = function(Chart) {
var index = point._index;
var model = point._model;

point.$previousStyle = {
backgroundColor: model.backgroundColor,
borderColor: model.borderColor,
borderWidth: model.borderWidth,
radius: model.radius
};

model.radius = custom.hoverRadius ? custom.hoverRadius : helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : helpers.valueAtIndexOrDefault(dataset.pointHoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : helpers.valueAtIndexOrDefault(dataset.pointHoverBorderColor, index, helpers.getHoverColor(model.borderColor));
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : helpers.valueAtIndexOrDefault(dataset.pointHoverBorderWidth, index, model.borderWidth);
},

removeHoverStyle: function(point) {
var dataset = this.chart.data.datasets[point._datasetIndex];
var custom = point.custom || {};
var index = point._index;
var model = point._model;
var pointElementOptions = this.chart.options.elements.point;

model.radius = custom.radius ? custom.radius : helpers.valueAtIndexOrDefault(dataset.pointRadius, index, pointElementOptions.radius);
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : helpers.valueAtIndexOrDefault(dataset.pointBackgroundColor, index, pointElementOptions.backgroundColor);
model.borderColor = custom.borderColor ? custom.borderColor : helpers.valueAtIndexOrDefault(dataset.pointBorderColor, index, pointElementOptions.borderColor);
model.borderWidth = custom.borderWidth ? custom.borderWidth : helpers.valueAtIndexOrDefault(dataset.pointBorderWidth, index, pointElementOptions.borderWidth);
}
});
};
19 changes: 9 additions & 10 deletions src/core/core.datasetController.js
View file
Expand Up @@ -238,16 +238,9 @@ module.exports = function(Chart) {
}
},

removeHoverStyle: function(element, elementOpts) {
var dataset = this.chart.data.datasets[element._datasetIndex];
var index = element._index;
var custom = element.custom || {};
var valueOrDefault = helpers.valueAtIndexOrDefault;
var model = element._model;

model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);
removeHoverStyle: function(element) {
helpers.merge(element._model, element.$previousStyle || {});
delete element.$previousStyle;
},

setHoverStyle: function(element) {
Expand All @@ -258,6 +251,12 @@ module.exports = function(Chart) {
var getHoverColor = helpers.getHoverColor;
var model = element._model;

element.$previousStyle = {
backgroundColor: model.backgroundColor,
borderColor: model.borderColor,
borderWidth: model.borderWidth
};

model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : valueOrDefault(dataset.hoverBackgroundColor, index, getHoverColor(model.backgroundColor));
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : valueOrDefault(dataset.hoverBorderColor, index, getHoverColor(model.borderColor));
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : valueOrDefault(dataset.hoverBorderWidth, index, model.borderWidth);
Expand Down
26 changes: 25 additions & 1 deletion test/specs/controller.bar.tests.js
View file
Expand Up @@ -1372,13 +1372,21 @@ describe('Chart.controllers.bar', function() {

var meta = chart.getDatasetMeta(1);
var bar = meta.data[0];
var helpers = window.Chart.helpers;

// Change default
chart.options.elements.rectangle.backgroundColor = 'rgb(128, 128, 128)';
chart.options.elements.rectangle.borderColor = 'rgb(15, 15, 15)';
chart.options.elements.rectangle.borderWidth = 3.14;

// Remove to defaults
chart.update();
expect(bar._model.backgroundColor).toBe('rgb(128, 128, 128)');
expect(bar._model.borderColor).toBe('rgb(15, 15, 15)');
expect(bar._model.borderWidth).toBe(3.14);
meta.controller.setHoverStyle(bar);
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(128, 128, 128)'));
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(15, 15, 15)'));
expect(bar._model.borderWidth).toBe(3.14);
meta.controller.removeHoverStyle(bar);
expect(bar._model.backgroundColor).toBe('rgb(128, 128, 128)');
expect(bar._model.borderColor).toBe('rgb(15, 15, 15)');
Expand All @@ -1389,6 +1397,14 @@ describe('Chart.controllers.bar', function() {
chart.data.datasets[1].borderColor = ['rgb(9, 9, 9)', 'rgb(0, 0, 0)'];
chart.data.datasets[1].borderWidth = [2.5, 5];

chart.update();
expect(bar._model.backgroundColor).toBe('rgb(255, 255, 255)');
expect(bar._model.borderColor).toBe('rgb(9, 9, 9)');
expect(bar._model.borderWidth).toBe(2.5);
meta.controller.setHoverStyle(bar);
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(255, 255, 255)'));
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(9, 9, 9)'));
expect(bar._model.borderWidth).toBe(2.5);
meta.controller.removeHoverStyle(bar);
expect(bar._model.backgroundColor).toBe('rgb(255, 255, 255)');
expect(bar._model.borderColor).toBe('rgb(9, 9, 9)');
Expand All @@ -1401,6 +1417,14 @@ describe('Chart.controllers.bar', function() {
borderWidth: 1.5
};

chart.update();
expect(bar._model.backgroundColor).toBe('rgb(255, 0, 0)');
expect(bar._model.borderColor).toBe('rgb(0, 255, 0)');
expect(bar._model.borderWidth).toBe(1.5);
meta.controller.setHoverStyle(bar);
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(255, 0, 0)'));
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(0, 255, 0)'));
expect(bar._model.borderWidth).toBe(1.5);
meta.controller.removeHoverStyle(bar);
expect(bar._model.backgroundColor).toBe('rgb(255, 0, 0)');
expect(bar._model.borderColor).toBe('rgb(0, 255, 0)');
Expand Down
8 changes: 8 additions & 0 deletions test/specs/controller.doughnut.tests.js
View file
Expand Up @@ -355,6 +355,8 @@ describe('Chart.controllers.doughnut', function() {
var meta = chart.getDatasetMeta(0);
var arc = meta.data[0];

chart.update();
meta.controller.setHoverStyle(arc);
meta.controller.removeHoverStyle(arc);
expect(arc._model.backgroundColor).toBe('rgb(255, 0, 0)');
expect(arc._model.borderColor).toBe('rgb(0, 0, 255)');
Expand All @@ -365,6 +367,8 @@ describe('Chart.controllers.doughnut', function() {
chart.data.datasets[0].borderColor = 'rgb(18, 18, 18)';
chart.data.datasets[0].borderWidth = 1.56;

chart.update();
meta.controller.setHoverStyle(arc);
meta.controller.removeHoverStyle(arc);
expect(arc._model.backgroundColor).toBe('rgb(9, 9, 9)');
expect(arc._model.borderColor).toBe('rgb(18, 18, 18)');
Expand All @@ -375,6 +379,8 @@ describe('Chart.controllers.doughnut', function() {
chart.data.datasets[0].borderColor = ['rgb(18, 18, 18)'];
chart.data.datasets[0].borderWidth = [0.1, 1.56];

chart.update();
meta.controller.setHoverStyle(arc);
meta.controller.removeHoverStyle(arc);
expect(arc._model.backgroundColor).toBe('rgb(255, 255, 255)');
expect(arc._model.borderColor).toBe('rgb(18, 18, 18)');
Expand All @@ -387,6 +393,8 @@ describe('Chart.controllers.doughnut', function() {
borderWidth: 3.14159,
};

chart.update();
meta.controller.setHoverStyle(arc);
meta.controller.removeHoverStyle(arc);
expect(arc._model.backgroundColor).toBe('rgb(7, 7, 7)');
expect(arc._model.borderColor).toBe('rgb(17, 17, 17)');
Expand Down
4 changes: 4 additions & 0 deletions test/specs/controller.line.tests.js
View file
Expand Up @@ -703,6 +703,7 @@ describe('Chart.controllers.line', function() {
chart.options.elements.point.radius = 1.01;

meta.controller.removeHoverStyle(point);
chart.update();
expect(point._model.backgroundColor).toBe('rgb(45, 46, 47)');
expect(point._model.borderColor).toBe('rgb(50, 51, 52)');
expect(point._model.borderWidth).toBe(10.1);
Expand All @@ -715,6 +716,7 @@ describe('Chart.controllers.line', function() {
chart.data.datasets[0].pointBorderWidth = 2.1;

meta.controller.removeHoverStyle(point);
chart.update();
expect(point._model.backgroundColor).toBe('rgb(77, 79, 81)');
expect(point._model.borderColor).toBe('rgb(123, 125, 127)');
expect(point._model.borderWidth).toBe(2.1);
Expand All @@ -726,6 +728,7 @@ describe('Chart.controllers.line', function() {
chart.data.datasets[0].radius = 20;

meta.controller.removeHoverStyle(point);
chart.update();
expect(point._model.backgroundColor).toBe('rgb(77, 79, 81)');
expect(point._model.borderColor).toBe('rgb(123, 125, 127)');
expect(point._model.borderWidth).toBe(2.1);
Expand All @@ -740,6 +743,7 @@ describe('Chart.controllers.line', function() {
};

meta.controller.removeHoverStyle(point);
chart.update();
expect(point._model.backgroundColor).toBe('rgb(0, 0, 0)');
expect(point._model.borderColor).toBe('rgb(10, 10, 10)');
expect(point._model.borderWidth).toBe(5.5);
Expand Down
9 changes: 9 additions & 0 deletions test/specs/controller.polarArea.tests.js
View file
Expand Up @@ -332,7 +332,14 @@ describe('Chart.controllers.polarArea', function() {
chart.options.elements.arc.borderColor = 'rgb(50, 51, 52)';
chart.options.elements.arc.borderWidth = 10.1;

meta.controller.setHoverStyle(arc);
chart.update();
expect(arc._model.backgroundColor).toBe('rgb(45, 46, 47)');
expect(arc._model.borderColor).toBe('rgb(50, 51, 52)');
expect(arc._model.borderWidth).toBe(10.1);

meta.controller.removeHoverStyle(arc);
chart.update();
expect(arc._model.backgroundColor).toBe('rgb(45, 46, 47)');
expect(arc._model.borderColor).toBe('rgb(50, 51, 52)');
expect(arc._model.borderWidth).toBe(10.1);
Expand All @@ -343,6 +350,7 @@ describe('Chart.controllers.polarArea', function() {
chart.data.datasets[0].borderWidth = 2.1;

meta.controller.removeHoverStyle(arc);
chart.update();
expect(arc._model.backgroundColor).toBe('rgb(77, 79, 81)');
expect(arc._model.borderColor).toBe('rgb(123, 125, 127)');
expect(arc._model.borderWidth).toBe(2.1);
Expand All @@ -355,6 +363,7 @@ describe('Chart.controllers.polarArea', function() {
};

meta.controller.removeHoverStyle(arc);
chart.update();
expect(arc._model.backgroundColor).toBe('rgb(0, 0, 0)');
expect(arc._model.borderColor).toBe('rgb(10, 10, 10)');
expect(arc._model.borderWidth).toBe(5.5);
Expand Down

0 comments on commit e49b268

Please sign in to comment.