From da3aa68f38b9db25156c3b32c56098f9ca70f4d8 Mon Sep 17 00:00:00 2001 From: Ben McCann Date: Mon, 25 Jun 2018 23:56:53 -0700 Subject: [PATCH] Restore original styles when removing hover (#5570) Refactor `updateElement` and `removeHoverStyle` and fix tests. --- karma.conf.js | 1 - src/controllers/controller.bar.js | 23 --------------- src/controllers/controller.bubble.js | 20 ++++--------- src/controllers/controller.doughnut.js | 12 ++++---- src/controllers/controller.line.js | 37 +++++++++--------------- src/controllers/controller.polarArea.js | 13 +++++---- src/controllers/controller.radar.js | 20 +++++-------- src/core/core.datasetController.js | 19 ++++++------ test/specs/controller.bar.tests.js | 26 ++++++++++++++++- test/specs/controller.doughnut.tests.js | 8 +++++ test/specs/controller.line.tests.js | 4 +++ test/specs/controller.polarArea.tests.js | 9 ++++++ test/specs/controller.radar.tests.js | 3 ++ 13 files changed, 99 insertions(+), 96 deletions(-) diff --git a/karma.conf.js b/karma.conf.js index 5601cbd7212..c41b23e0e03 100644 --- a/karma.conf.js +++ b/karma.conf.js @@ -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 }; diff --git a/src/controllers/controller.bar.js b/src/controllers/controller.bar.js index ff2b56ae5a0..cadae3fdb6c 100644 --- a/src/controllers/controller.bar.js +++ b/src/controllers/controller.bar.js @@ -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({ diff --git a/src/controllers/controller.bubble.js b/src/controllers/controller.bubble.js index 65c6fae01b8..b808b366b1f 100644 --- a/src/controllers/controller.bubble.js +++ b/src/controllers/controller.bubble.js @@ -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 */ diff --git a/src/controllers/controller.doughnut.js b/src/controllers/controller.doughnut.js index f7b36e989a9..eb759fe60b8 100644 --- a/src/controllers/controller.doughnut.js +++ b/src/controllers/controller.doughnut.js @@ -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) { @@ -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(); diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 7aacf2d23e2..2d1856e01e1 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -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); - } }); }; diff --git a/src/controllers/controller.polarArea.js b/src/controllers/controller.polarArea.js index 9ac7af0c3ac..663a9534d55 100644 --- a/src/controllers/controller.polarArea.js +++ b/src/controllers/controller.polarArea.js @@ -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() { diff --git a/src/controllers/controller.radar.js b/src/controllers/controller.radar.js index 5de4e4ede0a..10d71cdacb5 100644 --- a/src/controllers/controller.radar.js +++ b/src/controllers/controller.radar.js @@ -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); - } }); }; diff --git a/src/core/core.datasetController.js b/src/core/core.datasetController.js index ee6158c35b1..4928f98913e 100644 --- a/src/core/core.datasetController.js +++ b/src/core/core.datasetController.js @@ -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) { @@ -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); diff --git a/test/specs/controller.bar.tests.js b/test/specs/controller.bar.tests.js index 096b2e8e116..7957b6ab5db 100644 --- a/test/specs/controller.bar.tests.js +++ b/test/specs/controller.bar.tests.js @@ -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)'); @@ -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)'); @@ -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)'); diff --git a/test/specs/controller.doughnut.tests.js b/test/specs/controller.doughnut.tests.js index a4d6526c912..b1969e2817a 100644 --- a/test/specs/controller.doughnut.tests.js +++ b/test/specs/controller.doughnut.tests.js @@ -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)'); @@ -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)'); @@ -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)'); @@ -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)'); diff --git a/test/specs/controller.line.tests.js b/test/specs/controller.line.tests.js index 31c0763a8fe..b03162ea9d4 100644 --- a/test/specs/controller.line.tests.js +++ b/test/specs/controller.line.tests.js @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/test/specs/controller.polarArea.tests.js b/test/specs/controller.polarArea.tests.js index 380c1b46e8a..11e0be0b7d3 100644 --- a/test/specs/controller.polarArea.tests.js +++ b/test/specs/controller.polarArea.tests.js @@ -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); @@ -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); @@ -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); diff --git a/test/specs/controller.radar.tests.js b/test/specs/controller.radar.tests.js index df1001420c4..8e3b8d49130 100644 --- a/test/specs/controller.radar.tests.js +++ b/test/specs/controller.radar.tests.js @@ -410,6 +410,7 @@ describe('Chart.controllers.radar', 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); @@ -422,6 +423,7 @@ describe('Chart.controllers.radar', 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); @@ -436,6 +438,7 @@ describe('Chart.controllers.radar', 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);