Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit active element changes to chartArea #9970

Merged
merged 3 commits into from Dec 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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