From b7dc6bfa35b12d307399cef9b0d14a70a5a1a6ce Mon Sep 17 00:00:00 2001 From: kurkle Date: Wed, 15 Dec 2021 20:49:23 +0200 Subject: [PATCH 1/2] Fix setActiveElements behavior after a mouse event --- src/core/core.controller.js | 11 ++++++--- src/plugins/plugin.tooltip.js | 6 +++++ test/specs/core.controller.tests.js | 36 +++++++++++++++++++++++++++++ test/specs/plugin.tooltip.tests.js | 36 +++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 3 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 7b4df4137bf..6b0bd2f71da 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -504,9 +504,12 @@ class Chart { this._layers.sort(compare2Level('z', '_idx')); - // Replay last event from before update - if (this._lastEvent) { - this._eventHandler(this._lastEvent, true); + // Replay last event from before update, or set hover styles on active elements + const {_active, _lastEvent} = this; + if (_lastEvent) { + this._eventHandler(_lastEvent, true); + } else if (_active.length) { + this._updateHoverStyles(_active, _active, true); } this.render(); @@ -1086,6 +1089,8 @@ class Chart { if (changed) { this._active = active; + // Make sure we don't use the previous mouse event to override the active elements in update. + this._lastEvent = null; this._updateHoverStyles(active, lastActive); } } diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index 751569a8b7c..e35bc9d168b 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -1004,6 +1004,7 @@ export class Tooltip extends Element { if (changed || positionChanged) { this._active = active; this._eventPosition = eventPosition; + this._ignoreOneReplay = true; this.update(true); } } @@ -1016,6 +1017,11 @@ export class Tooltip extends Element { * @returns {boolean} true if the tooltip changed */ handleEvent(e, replay, inChartArea = true) { + if (replay && this._ignoreOneReplay) { + return false; + } + this._ignoreOneReplay = false; + const options = this.options; const lastActive = this._active || []; const active = this._getActiveElements(e, lastActive, replay, inChartArea); diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index b0197b152ab..6c4af91b7c8 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -2287,6 +2287,42 @@ describe('Chart', function() { }); }); + it('should not replace the user set active elements by event replay', async function() { + var chart = acquireChart({ + type: 'line', + data: { + labels: [1, 2, 3], + datasets: [{ + data: [1, 2, 3], + borderColor: 'red', + hoverBorderColor: 'blue', + }] + } + }); + + const meta = chart.getDatasetMeta(0); + const point0 = meta.data[0]; + const point1 = meta.data[1]; + + let props = meta.data[0].getProps(['borderColor']); + expect(props.options.borderColor).toEqual('red'); + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: point0.x, y: point0.y}); + expect(chart.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point0}]); + expect(point0.options.borderColor).toEqual('blue'); + expect(point1.options.borderColor).toEqual('red'); + + chart.setActiveElements([{datasetIndex: 0, index: 1}]); + expect(chart.getActiveElements()).toEqual([{datasetIndex: 0, index: 1, element: point1}]); + expect(point0.options.borderColor).toEqual('red'); + expect(point1.options.borderColor).toEqual('blue'); + + chart.update(); + expect(chart.getActiveElements()).toEqual([{datasetIndex: 0, index: 1, element: point1}]); + expect(point0.options.borderColor).toEqual('red'); + expect(point1.options.borderColor).toEqual('blue'); + }); + describe('platform', function() { it('should use the platform constructor provided in config', function() { const chart = acquireChart({ diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js index dedd7db88db..8644b34c794 100644 --- a/test/specs/plugin.tooltip.tests.js +++ b/test/specs/plugin.tooltip.tests.js @@ -1556,6 +1556,42 @@ describe('Plugin.Tooltip', function() { expect(chart.tooltip.getActiveElements()[0].element).toBe(meta.data[0]); }); + it('should not replace the user set active elements by event replay', async function() { + var chart = window.acquireChart({ + type: 'line', + data: { + datasets: [{ + label: 'Dataset 1', + data: [10, 20, 30], + pointHoverBorderColor: 'rgb(255, 0, 0)', + pointHoverBackgroundColor: 'rgb(0, 255, 0)' + }], + labels: ['Point 1', 'Point 2', 'Point 3'] + }, + options: { + events: ['pointerdown', 'pointerup'] + } + }); + + const meta = chart.getDatasetMeta(0); + const point0 = meta.data[0]; + const point1 = meta.data[1]; + + await jasmine.triggerMouseEvent(chart, 'pointerdown', {x: point0.x, y: point0.y}); + expect(chart.tooltip.opacity).toBe(1); + expect(chart.tooltip.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point0}]); + + chart.tooltip.setActiveElements([{datasetIndex: 0, index: 1}]); + chart.update(); + expect(chart.tooltip.opacity).toBe(1); + expect(chart.tooltip.getActiveElements()).toEqual([{datasetIndex: 0, index: 1, element: point1}]); + + chart.tooltip.setActiveElements([]); + chart.update(); + expect(chart.tooltip.opacity).toBe(0); + expect(chart.tooltip.getActiveElements().length).toBe(0); + }); + it('should not change the active elements on events outside chartArea, except for mouseout', async function() { var chart = acquireChart({ type: 'line', From 67163696d3ecf995034941045d5b2d89f572cfd1 Mon Sep 17 00:00:00 2001 From: kurkle Date: Wed, 15 Dec 2021 21:56:45 +0200 Subject: [PATCH 2/2] Better variable name --- src/plugins/plugin.tooltip.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index e35bc9d168b..76db6bef5fa 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -1004,7 +1004,7 @@ export class Tooltip extends Element { if (changed || positionChanged) { this._active = active; this._eventPosition = eventPosition; - this._ignoreOneReplay = true; + this._ignoreReplayEvents = true; this.update(true); } } @@ -1017,10 +1017,10 @@ export class Tooltip extends Element { * @returns {boolean} true if the tooltip changed */ handleEvent(e, replay, inChartArea = true) { - if (replay && this._ignoreOneReplay) { + if (replay && this._ignoreReplayEvents) { return false; } - this._ignoreOneReplay = false; + this._ignoreReplayEvents = false; const options = this.options; const lastActive = this._active || [];