Skip to content

Commit

Permalink
Limit active element changes to chartArea (#9970)
Browse files Browse the repository at this point in the history
* Limit active element changes to chartArea

* CC, remove duplicate ChartEvent interface

* CC2
  • Loading branch information
kurkle committed Dec 8, 2021
1 parent d2d5f49 commit ba6b446
Show file tree
Hide file tree
Showing 10 changed files with 176 additions and 51 deletions.
79 changes: 58 additions & 21 deletions src/core/core.controller.js
Expand Up @@ -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'];
Expand Down Expand Up @@ -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 {

Expand Down Expand Up @@ -1109,14 +1125,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);
Expand All @@ -1132,12 +1153,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.
//
Expand All @@ -1153,30 +1174,24 @@ 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;
const active = this._getActiveElements(e, lastActive, inChartArea, useFinalPosition);
const isClick = _isClickEvent(e);
const lastEvent = determineLastEvent(e, this._lastEvent, inChartArea, isClick);

let active = [];
let changed = false;
let lastEvent = null;
if (inChartArea) {
// Set _lastEvent to null while we are processing the event handlers.
// This prevents recursion if the handler calls chart.update()
this._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 (_isPointInArea(e, this.chartArea, this._minPadding)) {
// 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);
}
}

changed = !_elementsEqual(active, lastActive);
const changed = !_elementsEqual(active, lastActive);
if (changed || replay) {
this._active = active;
this._updateHoverStyles(active, lastActive, replay);
Expand All @@ -1186,6 +1201,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
Expand Down
2 changes: 1 addition & 1 deletion src/core/core.interaction.js
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion src/core/core.plugins.js
Expand Up @@ -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
*/

Expand Down
9 changes: 9 additions & 0 deletions src/helpers/helpers.core.js
Expand Up @@ -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';
}
11 changes: 0 additions & 11 deletions src/platform/platform.base.js
Expand Up @@ -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)
*/
2 changes: 1 addition & 1 deletion src/plugins/plugin.legend.js
Expand Up @@ -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) => {
Expand Down
51 changes: 37 additions & 14 deletions src/plugins/plugin.tooltip.js
Expand Up @@ -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
*/

Expand Down Expand Up @@ -1012,29 +1012,21 @@ 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
// the items are the same as before.
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) {
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
33 changes: 33 additions & 0 deletions test/specs/core.controller.tests.js
Expand Up @@ -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() {
Expand Down
32 changes: 32 additions & 0 deletions test/specs/plugin.tooltip.tests.js
Expand Up @@ -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',
Expand Down
6 changes: 4 additions & 2 deletions types/index.esm.d.ts
Expand Up @@ -1031,20 +1031,22 @@ export interface Plugin<TType extends ChartType = ChartType, O = AnyObject> 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.
* @param {Chart} chart - The chart instance.
* @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.
Expand Down

0 comments on commit ba6b446

Please sign in to comment.