Skip to content

Commit e49b268

Browse files
benmccannsimonbrunel
benmccann
authored and
simonbrunel
committed
Restore original styles when removing hover (chartjs#5570)
Refactor `updateElement` and `removeHoverStyle` and fix tests.
1 parent cbe465f commit e49b268

13 files changed

+99
-96
lines changed

‎karma.conf.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ module.exports = function(karma) {
1919
// These settings deal with browser disconnects. We had seen test flakiness from Firefox
2020
// [Firefox 56.0.0 (Linux 0.0.0)]: Disconnected (1 times), because no message in 10000 ms.
2121
// https://github.com/jasmine/jasmine/issues/1327#issuecomment-332939551
22-
browserNoActivityTimeout: 60000,
2322
browserDisconnectTolerance: 3
2423
};
2524

‎src/controllers/controller.bar.js

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -461,29 +461,6 @@ module.exports = function(Chart) {
461461

462462
helpers.canvas.unclipArea(chart.ctx);
463463
},
464-
465-
setHoverStyle: function(rectangle) {
466-
var dataset = this.chart.data.datasets[rectangle._datasetIndex];
467-
var index = rectangle._index;
468-
var custom = rectangle.custom || {};
469-
var model = rectangle._model;
470-
471-
model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : helpers.valueAtIndexOrDefault(dataset.hoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
472-
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : helpers.valueAtIndexOrDefault(dataset.hoverBorderColor, index, helpers.getHoverColor(model.borderColor));
473-
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : helpers.valueAtIndexOrDefault(dataset.hoverBorderWidth, index, model.borderWidth);
474-
},
475-
476-
removeHoverStyle: function(rectangle) {
477-
var dataset = this.chart.data.datasets[rectangle._datasetIndex];
478-
var index = rectangle._index;
479-
var custom = rectangle.custom || {};
480-
var model = rectangle._model;
481-
var rectangleElementOptions = this.chart.options.elements.rectangle;
482-
483-
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : helpers.valueAtIndexOrDefault(dataset.backgroundColor, index, rectangleElementOptions.backgroundColor);
484-
model.borderColor = custom.borderColor ? custom.borderColor : helpers.valueAtIndexOrDefault(dataset.borderColor, index, rectangleElementOptions.borderColor);
485-
model.borderWidth = custom.borderWidth ? custom.borderWidth : helpers.valueAtIndexOrDefault(dataset.borderWidth, index, rectangleElementOptions.borderWidth);
486-
}
487464
});
488465

489466
Chart.controllers.horizontalBar = Chart.controllers.bar.extend({

‎src/controllers/controller.bubble.js

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -102,26 +102,18 @@ module.exports = function(Chart) {
102102
setHoverStyle: function(point) {
103103
var model = point._model;
104104
var options = point._options;
105-
105+
point.$previousStyle = {
106+
backgroundColor: model.backgroundColor,
107+
borderColor: model.borderColor,
108+
borderWidth: model.borderWidth,
109+
radius: model.radius
110+
};
106111
model.backgroundColor = helpers.valueOrDefault(options.hoverBackgroundColor, helpers.getHoverColor(options.backgroundColor));
107112
model.borderColor = helpers.valueOrDefault(options.hoverBorderColor, helpers.getHoverColor(options.borderColor));
108113
model.borderWidth = helpers.valueOrDefault(options.hoverBorderWidth, options.borderWidth);
109114
model.radius = options.radius + options.hoverRadius;
110115
},
111116

112-
/**
113-
* @protected
114-
*/
115-
removeHoverStyle: function(point) {
116-
var model = point._model;
117-
var options = point._options;
118-
119-
model.backgroundColor = options.backgroundColor;
120-
model.borderColor = options.borderColor;
121-
model.borderWidth = options.borderWidth;
122-
model.radius = options.radius;
123-
},
124-
125117
/**
126118
* @private
127119
*/

‎src/controllers/controller.doughnut.js

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,8 +229,14 @@ module.exports = function(Chart) {
229229
});
230230

231231
var model = arc._model;
232+
232233
// Resets the visual styles
233-
this.removeHoverStyle(arc);
234+
var custom = arc.custom || {};
235+
var valueOrDefault = helpers.valueAtIndexOrDefault;
236+
var elementOpts = this.chart.options.elements.arc;
237+
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
238+
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
239+
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);
234240

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

249-
removeHoverStyle: function(arc) {
250-
Chart.DatasetController.prototype.removeHoverStyle.call(this, arc, this.chart.options.elements.arc);
251-
},
252-
253255
calculateTotal: function() {
254256
var dataset = this.getDataset();
255257
var meta = this.getMeta();

‎src/controllers/controller.line.js

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -307,35 +307,24 @@ module.exports = function(Chart) {
307307
}
308308
},
309309

310-
setHoverStyle: function(point) {
310+
setHoverStyle: function(element) {
311311
// Point
312-
var dataset = this.chart.data.datasets[point._datasetIndex];
313-
var index = point._index;
314-
var custom = point.custom || {};
315-
var model = point._model;
312+
var dataset = this.chart.data.datasets[element._datasetIndex];
313+
var index = element._index;
314+
var custom = element.custom || {};
315+
var model = element._model;
316+
317+
element.$previousStyle = {
318+
backgroundColor: model.backgroundColor,
319+
borderColor: model.borderColor,
320+
borderWidth: model.borderWidth,
321+
radius: model.radius
322+
};
316323

317-
model.radius = custom.hoverRadius || helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
318324
model.backgroundColor = custom.hoverBackgroundColor || helpers.valueAtIndexOrDefault(dataset.pointHoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
319325
model.borderColor = custom.hoverBorderColor || helpers.valueAtIndexOrDefault(dataset.pointHoverBorderColor, index, helpers.getHoverColor(model.borderColor));
320326
model.borderWidth = custom.hoverBorderWidth || helpers.valueAtIndexOrDefault(dataset.pointHoverBorderWidth, index, model.borderWidth);
327+
model.radius = custom.hoverRadius || helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
321328
},
322-
323-
removeHoverStyle: function(point) {
324-
var me = this;
325-
var dataset = me.chart.data.datasets[point._datasetIndex];
326-
var index = point._index;
327-
var custom = point.custom || {};
328-
var model = point._model;
329-
330-
// Compatibility: If the properties are defined with only the old name, use those values
331-
if ((dataset.radius !== undefined) && (dataset.pointRadius === undefined)) {
332-
dataset.pointRadius = dataset.radius;
333-
}
334-
335-
model.radius = custom.radius || helpers.valueAtIndexOrDefault(dataset.pointRadius, index, me.chart.options.elements.point.radius);
336-
model.backgroundColor = me.getPointBackgroundColor(point, index);
337-
model.borderColor = me.getPointBorderColor(point, index);
338-
model.borderWidth = me.getPointBorderWidth(point, index);
339-
}
340329
});
341330
};

‎src/controllers/controller.polarArea.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,13 +199,16 @@ module.exports = function(Chart) {
199199
});
200200

201201
// Apply border and fill style
202-
me.removeHoverStyle(arc);
202+
var elementOpts = this.chart.options.elements.arc;
203+
var custom = arc.custom || {};
204+
var valueOrDefault = helpers.valueAtIndexOrDefault;
205+
var model = arc._model;
203206

204-
arc.pivot();
205-
},
207+
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
208+
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
209+
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);
206210

207-
removeHoverStyle: function(arc) {
208-
Chart.DatasetController.prototype.removeHoverStyle.call(this, arc, this.chart.options.elements.arc);
211+
arc.pivot();
209212
},
210213

211214
countVisibleElements: function() {

‎src/controllers/controller.radar.js

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -146,23 +146,17 @@ module.exports = function(Chart) {
146146
var index = point._index;
147147
var model = point._model;
148148

149+
point.$previousStyle = {
150+
backgroundColor: model.backgroundColor,
151+
borderColor: model.borderColor,
152+
borderWidth: model.borderWidth,
153+
radius: model.radius
154+
};
155+
149156
model.radius = custom.hoverRadius ? custom.hoverRadius : helpers.valueAtIndexOrDefault(dataset.pointHoverRadius, index, this.chart.options.elements.point.hoverRadius);
150157
model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : helpers.valueAtIndexOrDefault(dataset.pointHoverBackgroundColor, index, helpers.getHoverColor(model.backgroundColor));
151158
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : helpers.valueAtIndexOrDefault(dataset.pointHoverBorderColor, index, helpers.getHoverColor(model.borderColor));
152159
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : helpers.valueAtIndexOrDefault(dataset.pointHoverBorderWidth, index, model.borderWidth);
153160
},
154-
155-
removeHoverStyle: function(point) {
156-
var dataset = this.chart.data.datasets[point._datasetIndex];
157-
var custom = point.custom || {};
158-
var index = point._index;
159-
var model = point._model;
160-
var pointElementOptions = this.chart.options.elements.point;
161-
162-
model.radius = custom.radius ? custom.radius : helpers.valueAtIndexOrDefault(dataset.pointRadius, index, pointElementOptions.radius);
163-
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : helpers.valueAtIndexOrDefault(dataset.pointBackgroundColor, index, pointElementOptions.backgroundColor);
164-
model.borderColor = custom.borderColor ? custom.borderColor : helpers.valueAtIndexOrDefault(dataset.pointBorderColor, index, pointElementOptions.borderColor);
165-
model.borderWidth = custom.borderWidth ? custom.borderWidth : helpers.valueAtIndexOrDefault(dataset.pointBorderWidth, index, pointElementOptions.borderWidth);
166-
}
167161
});
168162
};

‎src/core/core.datasetController.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -238,16 +238,9 @@ module.exports = function(Chart) {
238238
}
239239
},
240240

241-
removeHoverStyle: function(element, elementOpts) {
242-
var dataset = this.chart.data.datasets[element._datasetIndex];
243-
var index = element._index;
244-
var custom = element.custom || {};
245-
var valueOrDefault = helpers.valueAtIndexOrDefault;
246-
var model = element._model;
247-
248-
model.backgroundColor = custom.backgroundColor ? custom.backgroundColor : valueOrDefault(dataset.backgroundColor, index, elementOpts.backgroundColor);
249-
model.borderColor = custom.borderColor ? custom.borderColor : valueOrDefault(dataset.borderColor, index, elementOpts.borderColor);
250-
model.borderWidth = custom.borderWidth ? custom.borderWidth : valueOrDefault(dataset.borderWidth, index, elementOpts.borderWidth);
241+
removeHoverStyle: function(element) {
242+
helpers.merge(element._model, element.$previousStyle || {});
243+
delete element.$previousStyle;
251244
},
252245

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

254+
element.$previousStyle = {
255+
backgroundColor: model.backgroundColor,
256+
borderColor: model.borderColor,
257+
borderWidth: model.borderWidth
258+
};
259+
261260
model.backgroundColor = custom.hoverBackgroundColor ? custom.hoverBackgroundColor : valueOrDefault(dataset.hoverBackgroundColor, index, getHoverColor(model.backgroundColor));
262261
model.borderColor = custom.hoverBorderColor ? custom.hoverBorderColor : valueOrDefault(dataset.hoverBorderColor, index, getHoverColor(model.borderColor));
263262
model.borderWidth = custom.hoverBorderWidth ? custom.hoverBorderWidth : valueOrDefault(dataset.hoverBorderWidth, index, model.borderWidth);

‎test/specs/controller.bar.tests.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1372,13 +1372,21 @@ describe('Chart.controllers.bar', function() {
13721372

13731373
var meta = chart.getDatasetMeta(1);
13741374
var bar = meta.data[0];
1375+
var helpers = window.Chart.helpers;
13751376

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

1381-
// Remove to defaults
1382+
chart.update();
1383+
expect(bar._model.backgroundColor).toBe('rgb(128, 128, 128)');
1384+
expect(bar._model.borderColor).toBe('rgb(15, 15, 15)');
1385+
expect(bar._model.borderWidth).toBe(3.14);
1386+
meta.controller.setHoverStyle(bar);
1387+
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(128, 128, 128)'));
1388+
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(15, 15, 15)'));
1389+
expect(bar._model.borderWidth).toBe(3.14);
13821390
meta.controller.removeHoverStyle(bar);
13831391
expect(bar._model.backgroundColor).toBe('rgb(128, 128, 128)');
13841392
expect(bar._model.borderColor).toBe('rgb(15, 15, 15)');
@@ -1389,6 +1397,14 @@ describe('Chart.controllers.bar', function() {
13891397
chart.data.datasets[1].borderColor = ['rgb(9, 9, 9)', 'rgb(0, 0, 0)'];
13901398
chart.data.datasets[1].borderWidth = [2.5, 5];
13911399

1400+
chart.update();
1401+
expect(bar._model.backgroundColor).toBe('rgb(255, 255, 255)');
1402+
expect(bar._model.borderColor).toBe('rgb(9, 9, 9)');
1403+
expect(bar._model.borderWidth).toBe(2.5);
1404+
meta.controller.setHoverStyle(bar);
1405+
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(255, 255, 255)'));
1406+
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(9, 9, 9)'));
1407+
expect(bar._model.borderWidth).toBe(2.5);
13921408
meta.controller.removeHoverStyle(bar);
13931409
expect(bar._model.backgroundColor).toBe('rgb(255, 255, 255)');
13941410
expect(bar._model.borderColor).toBe('rgb(9, 9, 9)');
@@ -1401,6 +1417,14 @@ describe('Chart.controllers.bar', function() {
14011417
borderWidth: 1.5
14021418
};
14031419

1420+
chart.update();
1421+
expect(bar._model.backgroundColor).toBe('rgb(255, 0, 0)');
1422+
expect(bar._model.borderColor).toBe('rgb(0, 255, 0)');
1423+
expect(bar._model.borderWidth).toBe(1.5);
1424+
meta.controller.setHoverStyle(bar);
1425+
expect(bar._model.backgroundColor).toBe(helpers.getHoverColor('rgb(255, 0, 0)'));
1426+
expect(bar._model.borderColor).toBe(helpers.getHoverColor('rgb(0, 255, 0)'));
1427+
expect(bar._model.borderWidth).toBe(1.5);
14041428
meta.controller.removeHoverStyle(bar);
14051429
expect(bar._model.backgroundColor).toBe('rgb(255, 0, 0)');
14061430
expect(bar._model.borderColor).toBe('rgb(0, 255, 0)');

‎test/specs/controller.doughnut.tests.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -355,6 +355,8 @@ describe('Chart.controllers.doughnut', function() {
355355
var meta = chart.getDatasetMeta(0);
356356
var arc = meta.data[0];
357357

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

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

382+
chart.update();
383+
meta.controller.setHoverStyle(arc);
378384
meta.controller.removeHoverStyle(arc);
379385
expect(arc._model.backgroundColor).toBe('rgb(255, 255, 255)');
380386
expect(arc._model.borderColor).toBe('rgb(18, 18, 18)');
@@ -387,6 +393,8 @@ describe('Chart.controllers.doughnut', function() {
387393
borderWidth: 3.14159,
388394
};
389395

396+
chart.update();
397+
meta.controller.setHoverStyle(arc);
390398
meta.controller.removeHoverStyle(arc);
391399
expect(arc._model.backgroundColor).toBe('rgb(7, 7, 7)');
392400
expect(arc._model.borderColor).toBe('rgb(17, 17, 17)');

0 commit comments

Comments
 (0)