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 interactions to chartArea (+/-0.5px) #6943

Merged
merged 4 commits into from
Jan 11, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/getting-started/v3-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Chart.js 3.0 introduces a number of breaking changes. Chart.js 2.0 was released

### Interactions

* `interactions` are now limited to the chart area
* `{mode: 'label'}` was replaced with `{mode: 'index'}`
* `{mode: 'single'}` was replaced with `{mode: 'nearest', intersect: true}`
* `modes['X-axis']` was replaced with `{mode: 'index', intersect: false}`
Expand Down Expand Up @@ -116,6 +117,7 @@ Animation system was completely rewritten in Chart.js v3. Each property can now
* `Element._view`
* `TimeScale._getPixelForOffset`
* `TimeScale.getLabelWidth`
* `Tooltip._lastActive`

### Renamed

Expand Down
3 changes: 2 additions & 1 deletion src/controllers/controller.horizontalBar.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ defaults._set('horizontalBar', {
scales: {
x: {
type: 'linear',
position: 'bottom'
position: 'bottom',
beginAtZero: true
},
y: {
type: 'category',
Expand Down
14 changes: 13 additions & 1 deletion src/core/core.interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import helpers from '../helpers/index';
import {isNumber} from '../helpers/helpers.math';
import {_isPointInArea} from '../helpers/helpers.canvas';

/**
* Helper function to get relative position for an event
Expand Down Expand Up @@ -43,6 +44,8 @@ function evaluateAllVisibleItems(chart, handler) {
/**
* Helper function to check the items at the hovered index on the index scale
* @param {Chart} chart - the chart
* @param {string} axis - the axis mode. x|y|xy
* @param {object} position - the point to be nearest to
* @param {function} handler - the callback to execute for each visible item
* @return whether all scales were of a suitable type
*/
Expand Down Expand Up @@ -91,13 +94,18 @@ function getDistanceMetricForAxis(axis) {

/**
* Helper function to get the items that intersect the event position
* @param {ChartElement[]} items - elements to filter
* @param {Chart} chart - the chart
* @param {object} position - the point to be nearest to
* @param {string} axis - the axis mode. x|y|xy
* @return {ChartElement[]} the nearest items
*/
function getIntersectItems(chart, position, axis) {
const items = [];

if (!_isPointInArea(position, chart.chartArea)) {
return items;
}

const evaluationFunc = function(element, datasetIndex, index) {
if (element.inRange(position.x, position.y)) {
items.push({element, datasetIndex, index});
Expand Down Expand Up @@ -126,6 +134,10 @@ function getNearestItems(chart, position, axis, intersect) {
let minDistance = Number.POSITIVE_INFINITY;
let items = [];

if (!_isPointInArea(position, chart.chartArea)) {
return items;
}

const evaluationFunc = function(element, datasetIndex, index) {
if (intersect && !element.inRange(position.x, position.y)) {
return;
Expand Down
32 changes: 14 additions & 18 deletions src/core/core.tooltip.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ function createTooltipItem(chart, item) {
const {label, value} = chart.getDatasetMeta(datasetIndex).controller._getLabelAndValue(index);

return {
label: label,
value: value,
index: index,
datasetIndex: datasetIndex
label,
value,
index,
datasetIndex
};
}

Expand Down Expand Up @@ -452,7 +452,6 @@ class Tooltip extends Element {
const me = this;
me.opacity = 0;
me._active = [];
me._lastActive = [];
me.initialize();
}

Expand Down Expand Up @@ -962,28 +961,26 @@ class Tooltip extends Element {
* @returns {boolean} true if the tooltip changed
*/
handleEvent(e) {
var me = this;
var options = me.options;
var changed = false;

me._lastActive = me._lastActive || [];
const me = this;
const options = me.options;
const lastActive = me._active || [];
let changed = false;
let active = [];

// Find Active Elements for tooltips
if (e.type === 'mouseout') {
me._active = [];
} else {
me._active = me._chart.getElementsAtEventForMode(e, options.mode, options);
if (e.type !== 'mouseout') {
active = me._chart.getElementsAtEventForMode(e, options.mode, options);
if (options.reverse) {
me._active.reverse();
active.reverse();
}
}

// Remember Last Actives
changed = !helpers._elementsEqual(me._active, me._lastActive);
changed = !helpers._elementsEqual(active, lastActive);

// Only handle target event on tooltip change
if (changed) {
me._lastActive = me._active;
me._active = active;

if (options.enabled || options.custom) {
me._eventPosition = {
Expand All @@ -992,7 +989,6 @@ class Tooltip extends Element {
};

me.update(true);
// me.pivot();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/helpers/helpers.canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export function drawPoint(ctx, style, radius, x, y, rotation) {
* @private
*/
export function _isPointInArea(point, area) {
var epsilon = 1e-6; // 1e-6 is margin in pixels for accumulated error.
const epsilon = 0.5; // margin - to match rounded decimals

return point.x > area.left - epsilon && point.x < area.right + epsilon &&
point.y > area.top - epsilon && point.y < area.bottom + epsilon;
Expand Down
5 changes: 5 additions & 0 deletions test/specs/controller.line.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,11 @@ describe('Chart.controllers.line', function() {
}]
},
options: {
scales: {
x: {
offset: true
}
},
elements: {
point: {
backgroundColor: 'rgb(100, 150, 200)',
Expand Down
4 changes: 2 additions & 2 deletions test/specs/core.controller.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -1130,12 +1130,12 @@ describe('Chart', function() {
var tooltip = chart.tooltip;

expect(chart.lastActive[0].element).toEqual(point);
expect(tooltip._lastActive[0].element).toEqual(point);
expect(tooltip._active[0].element).toEqual(point);

// Update and confirm tooltip is updated
chart.update();
expect(chart.lastActive[0].element).toEqual(point);
expect(tooltip._lastActive[0].element).toEqual(point);
expect(tooltip._active[0].element).toEqual(point);
});

it ('should update the metadata', function() {
Expand Down
24 changes: 12 additions & 12 deletions test/specs/core.interaction.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

var elements = Chart.Interaction.modes.index(chart, evt, {intersect: false}).map(item => item.element);
Expand Down Expand Up @@ -182,8 +182,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

var elements = Chart.Interaction.modes.index(chart, evt, {axis: 'xy', intersect: false}).map(item => item.element);
Expand Down Expand Up @@ -279,8 +279,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

var elements = Chart.Interaction.modes.dataset(chart, evt, {axis: 'x', intersect: false});
Expand All @@ -293,8 +293,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

var elements = Chart.Interaction.modes.dataset(chart, evt, {axis: 'y', intersect: false});
Expand All @@ -307,8 +307,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

var elements = Chart.Interaction.modes.dataset(chart, evt, {intersect: false});
Expand Down Expand Up @@ -348,8 +348,8 @@ describe('Core.Interaction', function() {
type: 'click',
chart: chart,
native: true, // needed otherwise things its a DOM event
x: 0,
y: 0
x: chart.chartArea.left,
y: chart.chartArea.top
};

// Nearest to 0,0 (top left) will be first point of dataset 2
Expand Down
2 changes: 1 addition & 1 deletion test/specs/core.tooltip.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('Core.Tooltip', function() {
bubbles: true,
cancelable: true,
clientX: rect.left + point.x,
clientY: 0
clientY: rect.top + chart.chartArea.top + 5 // +5 to make tests work consistently
});

// Manually trigger rather than having an async test
Expand Down
4 changes: 2 additions & 2 deletions test/specs/helpers.canvas.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ describe('Chart.helpers.canvas', function() {
expect(isPointInArea({x: -1e-12, y: -1e-12}, area)).toBe(true);
expect(isPointInArea({x: 512, y: 256}, area)).toBe(true);
expect(isPointInArea({x: 512 + 1e-12, y: 256 + 1e-12}, area)).toBe(true);
expect(isPointInArea({x: -1e-3, y: 0}, area)).toBe(false);
expect(isPointInArea({x: 0, y: 256 + 1e-3}, area)).toBe(false);
expect(isPointInArea({x: -0.5, y: 0}, area)).toBe(false);
expect(isPointInArea({x: 0, y: 256.5}, area)).toBe(false);
});
});
});