From 894dcb5fb670fd95301fd5dcbeac9fbf916ae3b9 Mon Sep 17 00:00:00 2001 From: kurkle Date: Wed, 8 Dec 2021 09:51:48 +0200 Subject: [PATCH 1/3] Limit active element changes to chartArea --- src/core/core.controller.js | 59 ++++++++++++++++++++--------- src/plugins/plugin.tooltip.js | 49 +++++++++++++++++------- test/specs/core.controller.tests.js | 33 ++++++++++++++++ test/specs/plugin.tooltip.tests.js | 32 ++++++++++++++++ types/index.esm.d.ts | 6 ++- 5 files changed, 147 insertions(+), 32 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 01fcd0975ca..2e025e71e8e 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -1109,14 +1109,19 @@ class Chart { * @private */ _eventHandler(e, replay) { - const args = {event: e, replay, cancelable: true}; + const args = { + event: e, + replay, + cancelable: true, + inChartArea: _isPointInArea(e, this.chartArea, this._minPadding) + }; const eventFilter = (plugin) => (plugin.options.events || this.options.events).includes(e.native.type); if (this.notifyPlugins('beforeEvent', args, eventFilter) === false) { return; } - const changed = this._handleEvent(e, replay); + const changed = this._handleEvent(e, replay, args.inChartArea); args.cancelable = false; this.notifyPlugins('afterEvent', args, eventFilter); @@ -1132,12 +1137,12 @@ class Chart { * Handle an event * @param {ChartEvent} e the event to handle * @param {boolean} [replay] - true if the event was replayed by `update` + * @param {boolean} [inChartArea] - true if the event is inside chartArea * @return {boolean} true if the chart needs to re-render * @private */ - _handleEvent(e, replay) { + _handleEvent(e, replay, inChartArea) { const {_active: lastActive = [], options} = this; - const hoverOptions = options.hover; // If the event is replayed from `update`, we should evaluate with the final positions. // @@ -1153,21 +1158,19 @@ class Chart { // This is done so we do not have to evaluate the active elements each animation frame // - it would be expensive. const useFinalPosition = replay; - - let active = []; - let changed = false; + const active = this._getActiveElements(e, lastActive, inChartArea, useFinalPosition); let lastEvent = null; - // Find Active Elements for hover and tooltips - if (e.type !== 'mouseout') { - active = this.getElementsAtEventForMode(e, hoverOptions.mode, hoverOptions, useFinalPosition); - lastEvent = e.type === 'click' ? this._lastEvent : e; - } - // Set _lastEvent to null while we are processing the event handlers. - // This prevents recursion if the handler calls chart.update() - this._lastEvent = null; + if (inChartArea) { + if (e.type !== 'mouseout') { + // This event should be replayed in subsequent update + lastEvent = e.type === 'click' ? this._lastEvent : e; + } + + // Set _lastEvent to null while we are processing the event handlers. + // This prevents recursion if the handler calls chart.update() + this._lastEvent = null; - if (_isPointInArea(e, this.chartArea, this._minPadding)) { // Invoke onHover hook callCallback(options.onHover, [e, active, this], this); @@ -1176,7 +1179,7 @@ class Chart { } } - changed = !_elementsEqual(active, lastActive); + const changed = !_elementsEqual(active, lastActive); if (changed || replay) { this._active = active; this._updateHoverStyles(active, lastActive, replay); @@ -1186,6 +1189,28 @@ class Chart { return changed; } + + /** + * @param {ChartEvent} e - The event + * @param {import('../../types/index.esm').ActiveElement[]} lastActive - Previously active elements + * @param {boolean} inChartArea - Is the envent inside chartArea + * @param {boolean} useFinalPosition - Should the evaluation be done with current or final (after animation) element positions + * @returns {import('../../types/index.esm').ActiveElement[]} - The active elements + * @pravate + */ + _getActiveElements(e, lastActive, inChartArea, useFinalPosition) { + if (e.type === 'mouseout') { + return []; + } + + if (!inChartArea) { + // Let user control the active elements outside chartArea. Eg. using Legend. + return lastActive; + } + + const hoverOptions = this.options.hover; + return this.getElementsAtEventForMode(e, hoverOptions.mode, hoverOptions, useFinalPosition); + } } // @ts-ignore diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index 6d91c7ac8f9..367e5cd1b66 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -1012,21 +1012,13 @@ export class Tooltip extends Element { * Handle an event * @param {ChartEvent} e - The event to handle * @param {boolean} [replay] - This is a replayed event (from update) + * @param {boolean} [inChartArea] - The event is indide chartArea * @returns {boolean} true if the tooltip changed */ - handleEvent(e, replay) { + handleEvent(e, replay, inChartArea = true) { const options = this.options; const lastActive = this._active || []; - let changed = false; - let active = []; - - // Find Active Elements for tooltips - if (e.type !== 'mouseout') { - active = this.chart.getElementsAtEventForMode(e, options.mode, options, replay); - if (options.reverse) { - active.reverse(); - } - } + const active = this._getActiveElements(e, lastActive, replay, inChartArea); // When there are multiple items shown, but the tooltip position is nearest mode // an update may need to be made because our position may have changed even though @@ -1034,7 +1026,7 @@ export class Tooltip extends Element { const positionChanged = this._positionChanged(active, e); // Remember Last Actives - changed = replay || !_elementsEqual(active, lastActive) || positionChanged; + const changed = replay || !_elementsEqual(active, lastActive) || positionChanged; // Only handle target event on tooltip change if (changed) { @@ -1053,6 +1045,37 @@ export class Tooltip extends Element { return changed; } + /** + * Helper for determining the active elements for event + * @param {ChartEvent} e - The event to handle + * @param {Element[]} lastActive - Previously active elements + * @param {boolean} [replay] - This is a replayed event (from update) + * @param {boolean} [inChartArea] - The event is indide chartArea + * @returns {Element[]} - Active elements + * @private + */ + _getActiveElements(e, lastActive, replay, inChartArea) { + const options = this.options; + + if (e.type === 'mouseout') { + return []; + } + + if (!inChartArea) { + // Let user control the active elements outside chartArea. Eg. using Legend. + return lastActive; + } + + // Find Active Elements for tooltips + const active = this.chart.getElementsAtEventForMode(e, options.mode, options, replay); + + if (options.reverse) { + active.reverse(); + } + + return active; + } + /** * Determine if the active elements + event combination changes the * tooltip position @@ -1117,7 +1140,7 @@ export default { if (chart.tooltip) { // If the event is replayed from `update`, we should evaluate with the final positions. const useFinalPosition = args.replay; - if (chart.tooltip.handleEvent(args.event, useFinalPosition)) { + if (chart.tooltip.handleEvent(args.event, useFinalPosition, args.inChartArea)) { // notify chart about the change, so it will render args.changed = true; } diff --git a/test/specs/core.controller.tests.js b/test/specs/core.controller.tests.js index 12d0df3016a..b0197b152ab 100644 --- a/test/specs/core.controller.tests.js +++ b/test/specs/core.controller.tests.js @@ -361,6 +361,39 @@ describe('Chart', function() { await jasmine.triggerMouseEvent(chart, 'mousemove', point); expect(chart.getActiveElements()).toEqual([]); }); + + it('should not change the active elements when outside chartArea, except for mouseout', async function() { + var chart = acquireChart({ + type: 'line', + data: { + labels: ['A', 'B', 'C', 'D'], + datasets: [{ + data: [10, 20, 30, 100], + hoverRadius: 0 + }], + }, + options: { + scales: { + x: {display: false}, + y: {display: false} + }, + layout: { + padding: 5 + } + } + }); + + var point = chart.getDatasetMeta(0).data[0]; + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: point.x, y: point.y}); + expect(chart.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point}]); + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: 1, y: 1}); + expect(chart.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point}]); + + await jasmine.triggerMouseEvent(chart, 'mouseout', {x: 1, y: 1}); + expect(chart.tooltip.getActiveElements()).toEqual([]); + }); }); describe('when merging scale options', function() { diff --git a/test/specs/plugin.tooltip.tests.js b/test/specs/plugin.tooltip.tests.js index d181c870ca1..dedd7db88db 100644 --- a/test/specs/plugin.tooltip.tests.js +++ b/test/specs/plugin.tooltip.tests.js @@ -1556,6 +1556,38 @@ describe('Plugin.Tooltip', function() { expect(chart.tooltip.getActiveElements()[0].element).toBe(meta.data[0]); }); + it('should not change the active elements on events outside chartArea, except for mouseout', async function() { + var chart = acquireChart({ + type: 'line', + data: { + labels: ['A', 'B', 'C', 'D'], + datasets: [{ + data: [10, 20, 30, 100] + }], + }, + options: { + scales: { + x: {display: false}, + y: {display: false} + }, + layout: { + padding: 5 + } + } + }); + + var point = chart.getDatasetMeta(0).data[0]; + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: point.x, y: point.y}); + expect(chart.tooltip.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point}]); + + await jasmine.triggerMouseEvent(chart, 'mousemove', {x: 1, y: 1}); + expect(chart.tooltip.getActiveElements()).toEqual([{datasetIndex: 0, index: 0, element: point}]); + + await jasmine.triggerMouseEvent(chart, 'mouseout', {x: 1, y: 1}); + expect(chart.tooltip.getActiveElements()).toEqual([]); + }); + it('should update active elements when datasets are removed and added', async function() { var dataset = { label: 'Dataset 1', diff --git a/types/index.esm.d.ts b/types/index.esm.d.ts index 9534d523c4b..7d7721a7d4d 100644 --- a/types/index.esm.d.ts +++ b/types/index.esm.d.ts @@ -1031,9 +1031,10 @@ export interface Plugin exte * @param {object} args - The call arguments. * @param {ChartEvent} args.event - The event object. * @param {boolean} args.replay - True if this event is replayed from `Chart.update` + * @param {boolean} args.inChartArea - The event position is inside chartArea * @param {object} options - The plugin options. */ - beforeEvent?(chart: Chart, args: { event: ChartEvent, replay: boolean, cancelable: true }, options: O): boolean | void; + beforeEvent?(chart: Chart, args: { event: ChartEvent, replay: boolean, cancelable: true, inChartArea: boolean }, options: O): boolean | void; /** * @desc Called after the `event` has been consumed. Note that this hook * will not be called if the `event` has been previously discarded. @@ -1041,10 +1042,11 @@ export interface Plugin exte * @param {object} args - The call arguments. * @param {ChartEvent} args.event - The event object. * @param {boolean} args.replay - True if this event is replayed from `Chart.update` + * @param {boolean} args.inChartArea - The event position is inside chartArea * @param {boolean} [args.changed] - Set to true if the plugin needs a render. Should only be changed to true, because this args object is passed through all plugins. * @param {object} options - The plugin options. */ - afterEvent?(chart: Chart, args: { event: ChartEvent, replay: boolean, changed?: boolean, cancelable: false }, options: O): void; + afterEvent?(chart: Chart, args: { event: ChartEvent, replay: boolean, changed?: boolean, cancelable: false, inChartArea: boolean }, options: O): void; /** * @desc Called after the chart as been resized. * @param {Chart} chart - The chart instance. From f6508d2a596fead3d44f127340981869ef480650 Mon Sep 17 00:00:00 2001 From: kurkle Date: Wed, 8 Dec 2021 10:24:54 +0200 Subject: [PATCH 2/3] CC, remove duplicate ChartEvent interface --- src/core/core.controller.js | 10 ++++++---- src/core/core.interaction.js | 2 +- src/core/core.plugins.js | 2 +- src/helpers/helpers.core.js | 9 +++++++++ src/platform/platform.base.js | 11 ----------- src/plugins/plugin.legend.js | 2 +- src/plugins/plugin.tooltip.js | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 2e025e71e8e..5a10ac50d32 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -7,14 +7,14 @@ import PluginService from './core.plugins'; import registry from './core.registry'; import Config, {determineAxis, getIndexAxis} from './core.config'; import {retinaScale, _isDomSupported} from '../helpers/helpers.dom'; -import {each, callback as callCallback, uid, valueOrDefault, _elementsEqual, isNullOrUndef, setsEqual, defined, isFunction} from '../helpers/helpers.core'; +import {each, callback as callCallback, uid, valueOrDefault, _elementsEqual, isNullOrUndef, setsEqual, defined, isFunction, _isClickEvent} from '../helpers/helpers.core'; import {clearCanvas, clipArea, createContext, unclipArea, _isPointInArea} from '../helpers'; // @ts-ignore import {version} from '../../package.json'; import {debounce} from '../helpers/helpers.extras'; /** - * @typedef { import("../platform/platform.base").ChartEvent } ChartEvent + * @typedef { import('../../types/index.esm').ChartEvent } ChartEvent */ const KNOWN_POSITIONS = ['top', 'bottom', 'left', 'right', 'chartArea']; @@ -1162,9 +1162,11 @@ class Chart { let lastEvent = null; if (inChartArea) { + const isClick = _isClickEvent(e); + if (e.type !== 'mouseout') { // This event should be replayed in subsequent update - lastEvent = e.type === 'click' ? this._lastEvent : e; + lastEvent = isClick ? this._lastEvent : e; } // Set _lastEvent to null while we are processing the event handlers. @@ -1174,7 +1176,7 @@ class Chart { // Invoke onHover hook callCallback(options.onHover, [e, active, this], this); - if (e.type === 'mouseup' || e.type === 'click' || e.type === 'contextmenu') { + if (isClick) { callCallback(options.onClick, [e, active, this], this); } } diff --git a/src/core/core.interaction.js b/src/core/core.interaction.js index 0a350209798..662703d7f7b 100644 --- a/src/core/core.interaction.js +++ b/src/core/core.interaction.js @@ -5,7 +5,7 @@ import {_angleBetween, getAngleFromPoint} from '../helpers/helpers.math'; /** * @typedef { import("./core.controller").default } Chart - * @typedef { import("../platform/platform.base").ChartEvent } ChartEvent + * @typedef { import("../../types/index.esm").ChartEvent } ChartEvent * @typedef {{axis?: string, intersect?: boolean}} InteractionOptions * @typedef {{datasetIndex: number, index: number, element: import("./core.element").default}} InteractionItem */ diff --git a/src/core/core.plugins.js b/src/core/core.plugins.js index 27a07bb0c91..05d1e5c84fb 100644 --- a/src/core/core.plugins.js +++ b/src/core/core.plugins.js @@ -3,7 +3,7 @@ import {callback as callCallback, isNullOrUndef, valueOrDefault} from '../helper /** * @typedef { import("./core.controller").default } Chart - * @typedef { import("../platform/platform.base").ChartEvent } ChartEvent + * @typedef { import("../../types/index.esm").ChartEvent } ChartEvent * @typedef { import("../plugins/plugin.tooltip").default } Tooltip */ diff --git a/src/helpers/helpers.core.js b/src/helpers/helpers.core.js index 0a5fcea1e6f..2663c8b834b 100644 --- a/src/helpers/helpers.core.js +++ b/src/helpers/helpers.core.js @@ -340,3 +340,12 @@ export const setsEqual = (a, b) => { return true; }; + +/** + * @param {import('../../types/index.esm').ChartEvent} e - The event + * @returns {boolean} + * @private + */ +export function _isClickEvent(e) { + return e.type === 'mouseup' || e.type === 'click' || e.type === 'contextmenu'; +} diff --git a/src/platform/platform.base.js b/src/platform/platform.base.js index c29a5945a1c..1da169e9b36 100644 --- a/src/platform/platform.base.js +++ b/src/platform/platform.base.js @@ -81,14 +81,3 @@ export default class BasePlatform { // no-op } } - -/** - * @interface ChartEvent - * @typedef {object} ChartEvent - * @prop {string} type - The event type name, possible values are: - * 'contextmenu', 'mouseenter', 'mousedown', 'mousemove', 'mouseup', 'mouseout', - * 'click', 'dblclick', 'keydown', 'keypress', 'keyup' and 'resize' - * @prop {*} native - The original native event (null for emulated events, e.g. 'resize') - * @prop {number} x - The mouse x position, relative to the canvas (null for incompatible events) - * @prop {number} y - The mouse y position, relative to the canvas (null for incompatible events) - */ diff --git a/src/plugins/plugin.legend.js b/src/plugins/plugin.legend.js index d0bcb82761c..7388bc714f1 100644 --- a/src/plugins/plugin.legend.js +++ b/src/plugins/plugin.legend.js @@ -10,7 +10,7 @@ import { import {_toLeftRightCenter, _alignStartEnd, _textX} from '../helpers/helpers.extras'; import {toTRBLCorners} from '../helpers/helpers.options'; /** - * @typedef { import("../platform/platform.base").ChartEvent } ChartEvent + * @typedef { import("../../types/index.esm").ChartEvent } ChartEvent */ const getBoxSize = (labelOpts, fontSize) => { diff --git a/src/plugins/plugin.tooltip.js b/src/plugins/plugin.tooltip.js index 367e5cd1b66..751569a8b7c 100644 --- a/src/plugins/plugin.tooltip.js +++ b/src/plugins/plugin.tooltip.js @@ -9,7 +9,7 @@ import {createContext, drawPoint} from '../helpers'; /** * @typedef { import("../platform/platform.base").Chart } Chart - * @typedef { import("../platform/platform.base").ChartEvent } ChartEvent + * @typedef { import("../../types/index.esm").ChartEvent } ChartEvent * @typedef { import("../../types/index.esm").ActiveElement } ActiveElement */ From e7e944a979ecb60014893837697d666b8a471537 Mon Sep 17 00:00:00 2001 From: kurkle Date: Wed, 8 Dec 2021 10:42:36 +0200 Subject: [PATCH 3/3] CC2 --- src/core/core.controller.js | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/core/core.controller.js b/src/core/core.controller.js index 5a10ac50d32..7b4df4137bf 100644 --- a/src/core/core.controller.js +++ b/src/core/core.controller.js @@ -83,6 +83,22 @@ function moveNumericKeys(obj, start, move) { } } +/** + * @param {ChartEvent} e + * @param {ChartEvent|null} lastEvent + * @param {boolean} inChartArea + * @param {boolean} isClick + * @returns {ChartEvent|null} + */ +function determineLastEvent(e, lastEvent, inChartArea, isClick) { + if (!inChartArea || e.type === 'mouseout') { + return null; + } + if (isClick) { + return lastEvent; + } + return e; +} class Chart { @@ -1159,16 +1175,10 @@ class Chart { // - it would be expensive. const useFinalPosition = replay; const active = this._getActiveElements(e, lastActive, inChartArea, useFinalPosition); - let lastEvent = null; + const isClick = _isClickEvent(e); + const lastEvent = determineLastEvent(e, this._lastEvent, inChartArea, isClick); if (inChartArea) { - const isClick = _isClickEvent(e); - - if (e.type !== 'mouseout') { - // This event should be replayed in subsequent update - lastEvent = isClick ? this._lastEvent : e; - } - // Set _lastEvent to null while we are processing the event handlers. // This prevents recursion if the handler calls chart.update() this._lastEvent = null;