Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore original styles when removing hover #5570

Merged
merged 30 commits into from
Jun 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c98185e
Refactor updateElement and removeHoverStyle
loicbourgois Jan 28, 2018
8de5a31
Update style only
loicbourgois Jan 28, 2018
87929a4
typo
loicbourgois Jan 28, 2018
0bf1910
Refactor
loicbourgois Jan 28, 2018
8f664f1
Revert "Refactor"
loicbourgois Jan 29, 2018
bb2c547
Revert "typo"
loicbourgois Jan 29, 2018
9388dd8
Revert "Update style only"
loicbourgois Jan 29, 2018
ac18953
Revert "Refactor updateElement and removeHoverStyle"
loicbourgois Jan 29, 2018
0f96799
Merge branch 'master' of https://github.com/chartjs/Chart.js into bor…
loicbourgois Jan 29, 2018
a9af225
Implement @simonbrunel solution
loicbourgois Jan 29, 2018
dc2a66e
Mix @simbrunel solution with previous implementation
loicbourgois Jan 29, 2018
af1abd2
Update failing test
loicbourgois Jan 31, 2018
2527620
Remove not.toBe()
loicbourgois Feb 1, 2018
b8c5aef
Merge branch 'master' of https://github.com/chartjs/Chart.js into bor…
loicbourgois Feb 11, 2018
4b30773
Refactor
loicbourgois Feb 11, 2018
c8064d6
Better hover style for line
loicbourgois Feb 11, 2018
fcef4b6
Update radar hover style logic
loicbourgois Feb 11, 2018
fad0561
cleanup
loicbourgois Feb 11, 2018
743a123
refactor
loicbourgois Feb 11, 2018
57f3df0
refactor
loicbourgois Feb 11, 2018
07bf637
refactor
loicbourgois Feb 11, 2018
7448c51
refactor
loicbourgois Feb 11, 2018
0d9a74b
Pass tests
loicbourgois Feb 11, 2018
d89c572
Refactor
loicbourgois Feb 11, 2018
2d9a387
refactor
loicbourgois Feb 11, 2018
ea376e6
setup test
loicbourgois Feb 11, 2018
693fb03
Comment seamingly unecessary code
loicbourgois Feb 11, 2018
92cb840
Setup test for doughnut.controller.removeHoverStyle()
loicbourgois Feb 11, 2018
9cd0686
Test commenting seamingly unnecessary code
loicbourgois Feb 11, 2018
0bda0a3
Fix tests
benmccann Jun 16, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion karma.conf.js
Original file line number Diff line number Diff line change
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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this line that I had added in an earlier PR because it's very annoying to use with gulp test --watch. It closes the browser after 10 min which makes debugging really hard. Hopefully browserDisconnectTolerance: 3 will be enough. We could make it browserDisconnectTolerance: 5 or something if we want to add a little extra buffer since we're removing browserNoActivityTimeout

browserDisconnectTolerance: 3
};

Expand Down
23 changes: 0 additions & 23 deletions src/controllers/controller.bar.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -299,35 +299,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
Original file line number Diff line number Diff line change
Expand Up @@ -188,13 +188,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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,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 @@ -341,6 +348,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 @@ -353,6 +361,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
Loading