Improve Tooltip and Hover Interaction #3400
Conversation
…on.modes in preparation for adding new modes
…dd new intersect, nearest and nearestIntersect modes
…g with an `intersect` boolean. If the boolean is true, we only base our decisions on the things we intersect with. If it is false, we use the nearest item. Implemented the use of the boolean for dataset, index/label, and nearest modes.
…ad of just the intersect boolean for more flexibility
|
💯 👍 thanks @etimberg |
| @@ -212,7 +212,8 @@ Name | Type | Default | Description | |||
| --- | --- | --- | --- | |||
| enabled | Boolean | true | Are tooltips enabled | |||
| custom | Function | null | See [section](#advanced-usage-external-tooltips) below | |||
| mode | String | 'single' | Sets which elements appear in the tooltip. Acceptable options are `'single'`, `'label'` or `'x-axis'`. <br> <br>`single` highlights the closest element. <br> <br>`label` highlights elements in all datasets at the same `X` value. <br> <br>`'x-axis'` also highlights elements in all datasets at the same `X` value, but activates when hovering anywhere within the vertical slice of the x-axis representing that `X` value. | |||
| mode | String | 'single' | Sets which elements appear in the tooltip. See [Interaction Modes](#interaction-modes) for details | |||
There was a problem hiding this comment.
If single is deprecated, we should replace the default value by its equivalent, which I think is {mode: 'nearest', intersect: true}, right?
| @@ -288,8 +289,28 @@ Name | Type | Default | Description | |||
| --- | --- | --- | --- | |||
| mode | String | 'single' | Sets which elements hover. Acceptable options are `'single'`, `'label'`, `'x-axis'`, or `'dataset'`. <br> <br>`single` highlights the closest element. <br> <br>`label` highlights elements in all datasets at the same `X` value. <br> <br>`'x-axis'` also highlights elements in all datasets at the same `X` value, but activates when hovering anywhere within the vertical slice of the x-axis representing that `X` value. <br> <br>`dataset` highlights the closest dataset. | |||
There was a problem hiding this comment.
I think this option also uses the new modes.
| Mode | Behaviour | ||
| --- | --- | ||
| point | Finds all of the items that intersect the point | ||
| nearest | Gets the item that is nearest to the point. The nearest item is determined based on the distance to the center of the chart item (point, bar). If 2 or more items are at the same size, the one with the smallest area is used. If `intersect` is true, this is only triggered when the mouse is above an item the graph. This is very useful for combo charts where points are hidden behind bars. |
There was a problem hiding this comment.
... at the same size, ... should be ... at the same distance, ... (or position)
... is above an item the graph. doesn't sound right :)
| --- | --- | ||
| point | Finds all of the items that intersect the point | ||
| nearest | Gets the item that is nearest to the point. The nearest item is determined based on the distance to the center of the chart item (point, bar). If 2 or more items are at the same size, the one with the smallest area is used. If `intersect` is true, this is only triggered when the mouse is above an item the graph. This is very useful for combo charts where points are hidden behind bars. | ||
| single (deprecated) | Finds the first item that intersects the point and returns it. |
There was a problem hiding this comment.
Maybe we could also add something like:
Behaves like
'nearest'mode withintersect = true
| nearest | Gets the item that is nearest to the point. The nearest item is determined based on the distance to the center of the chart item (point, bar). If 2 or more items are at the same size, the one with the smallest area is used. If `intersect` is true, this is only triggered when the mouse is above an item the graph. This is very useful for combo charts where points are hidden behind bars. | ||
| single (deprecated) | Finds the first item that intersects the point and returns it. | ||
| label | Finds item at the same index. If the `intersect` setting is true, the first intersecting item is used to determine the index in the data. If `intersect` false the nearest item is used to determine the index. | ||
| index | Same as `'label'` mode but with a more descriptive name |
There was a problem hiding this comment.
Should we deprecate label as well? And then have the detailed description on index, and label (deprecated) above.
| */ | ||
| point: function(chartInstance, e) { | ||
| var eventPosition = helpers.getRelativePosition(e, chartInstance.chart); | ||
| var elementsArray = getIntersectItems(chartInstance, eventPosition); |
There was a problem hiding this comment.
return getIntersectItems(chartInstance, eventPosition);| @@ -31,6 +31,7 @@ module.exports = function() { | |||
| hover: { | |||
| onHover: null, | |||
| mode: 'single', | |||
There was a problem hiding this comment.
Should we use nearest as default?
| @@ -8,6 +8,7 @@ module.exports = function(Chart) { | |||
| enabled: true, | |||
| custom: null, | |||
| mode: 'single', | |||
There was a problem hiding this comment.
Should we use nearest as default?
| }, | ||
| inLabelRange: function(mouseX, mouseY) { | ||
| var me = this; | ||
| var inRange = false; |
There was a problem hiding this comment.
if (!me._view) {
return false;
}
var inRange = false;
var bounds = getBarBounds(me);
...| (mouseX >= vm.x - vm.width / 2 && mouseX <= vm.x + vm.width / 2) && (mouseY >= vm.base && mouseY <= vm.y)) : | ||
| false; | ||
| var center; | ||
| if (isVertical(this)) { |
There was a problem hiding this comment.
var x ,y;
if (isVertical(this)) {
x = ...
y = ...
} else {
x = ...
y = ...
}
return {x: x, y: y};
|
Awesome work, this will make chart interactions much better :) My main concern is about interactions performance, I don't think we should rely on any callback based iterations (see this comment). Adding a minor cosmetic feedback, which is really a personal preference: when there is no confusion possible, I prefer short variable names which make code easier to read. For example, I would not use |
|
Sounds good. I will make these changes. 😄 |
|
@simonbrunel I updated per your code review comments. I agree that we should avoid Edit: I also updated all of the samples to use the new modes. (Samples in general need some updates to have consistent colours, etc but that can be a different PR) |
|
Agree, |
| * @param position {Point} the point to be nearest to | ||
| * @return {ChartElement[]} the nearest items | ||
| */ | ||
| function getNearestItemsFromChart(chart, position) { |
There was a problem hiding this comment.
Maybe we can also refactor these two similar getNearestItems* methods by adding intersect as parameter:
function getNearestItems(chart, position, intersect) {
// ...
parseVisibleItems(chart, function(element) {
if (intersect && !element.inRange(position.x, position.y)) {
return;
}
var distance = Math.round(helpers.distanceBetweenPoints(position, element.getCenterPoint()));
// ...
}| */ | ||
| dataset: function(chart, e, options) { | ||
| var position = helpers.getRelativePosition(e, chart.chart); | ||
| var items = options.intersect ? getIntersectItems(chart, position) : getNearestItemsFromChart(chart, position); |
There was a problem hiding this comment.
var items = options.intersect ? getIntersectItems(chart, position) : getNearestItems(chart, position, false);|
|
||
| function indexMode(chart, e, options) { | ||
| var position = helpers.getRelativePosition(e, chart.chart); | ||
| var items = options.intersect ? getIntersectItems(chart, position) : getNearestItemsFromChart(chart, position); |
There was a problem hiding this comment.
var items = options.intersect ? getIntersectItems(chart, position) : getNearestItems(chart, position, false);
| */ | ||
| nearest: function(chart, e, options) { | ||
| var position = helpers.getRelativePosition(e, chart.chart); | ||
| var nearestItems = options.intersect ? getNearestItems(getIntersectItems(chart, position), position) : getNearestItemsFromChart(chart, position); |
There was a problem hiding this comment.
var items = getNearestItems(chart, position, options.intersect);
|
Sure, i will do that refactoring. |
etimberg
force-pushed
the
v2.4-tooltips
branch
from
34890e4
to
7ef7bef
Compare
|
@simonbrunel I made the last changes and squashed the refactoring commits together. |
|
Are we good to merge this? |
Refactored interaction modes to use lookup functions in Chart.Interaction.modes and added new modes for 'point', 'index', 'nearest', 'x', and 'y'
Significant improvements to the tooltip and hover modes
New Configuration
In addition to the
hover.modeandtooltips.modesettings,hover.intersectandtooltips.intersecthave been added. The intersect settings are used to configure certain modes so that when items are shown can be controlledNew Modes
'label'modeintersectset to true works well for points that are hidden behindPoint Mode
Nearest Mode, Intersect On
Nearest Mode, No Intersect
Deprecated Modes
The
'single'and'x-axis'modes are deprecated.Single mode has been replaced by
'point'which returns all of the items that intersect with the cursorThe X axis mode has been replaced by the
indexmode coupled with settingintersectto true.