Display tooltip for only one item per dataset at a time

[CrokinoleMaster]

I have a problem with the tooltips.

Sometimes when using mode x tooltip option, you want there to be only one item per dataset. Currently you can set pointHitRadius low. While this would work most of the time, sometimes it will not return anything because of the different distances between data points. With an option like the one in this PR, you can have a larger pointHitRadius but always get only one tooltip item returned per dataset.

[drivkin]

I have the same problem

[benmccann]

Is there ever a case where you would not want this behavior? I'm wondering if it should at least be the default or if it should always happen without an option. @etimberg @simonbrunel thoughts?

Also, this would need documentation and a test

[CrokinoleMaster]

You're right, I think it should be the default behavior. I'll make it default and add documentation and test.

[simonbrunel]

@benmccann good point and I agree: if it's a bug, we should not introduce any option, else, if it's a feature, we should not change the default behavior since it would be a breaking change in projects relying on multiple values per dataset. I don't know if there is actually use cases where we want multiple values per dataset?

@huaruiwu can you please make a fiddle that reproduces your issue?

Thought, isn't the same as #5231? If so, there is already a PR that tries to fix that case: #5332. However it's a different approach because it "limits" the number of values instead of picking only one. If we agree on a bug, I think I prefer this PR (#5578), I would simply make sure to pick the closest one.

@etimberg @MPierre9 thoughts?

[benmccann]

I would call this a bug

Agree on picking the closest point

[etimberg]

Agree this sounds like a bug. I can't think of reasons to have multiple values from the same dataset in the tooltip so returning the nearest works for me. For completeness, the y tooltip mode should get the same update.

[simonbrunel]

Is it always true in case of y which would intersect the spline multiple time? or x with vertical bars?

[CrokinoleMaster]

Just got rid of the option and made it so that it always returns only the closest point per dataset. Should I add the same thing for when mode is set to "y"?

[CrokinoleMaster]

added test

[CrokinoleMaster]

@benmccann Thanks for the review! I think I fixed the two issues you found.

[CrokinoleMaster]

@benmccann missed that one. just removed the Object.values

[CrokinoleMaster]

Thanks!

[CrokinoleMaster]

When is it possible to get this merged?

[benmccann]

@huaruiwu we're just waiting for @simonbrunel 's review before merging

[CrokinoleMaster]

@benmccann done

[CrokinoleMaster]

any chance of getting this merged soon?

[benmccann]

@kurkle can you review this PR? I'm wondering how it relates to #5857

[kurkle]

There are two issues remaining in this PR.

1. If a dataset has multiple points at the exact same distance, then all of those points should be returned.
   • consider a spline or scatter, or y-mode on a line chart
2. y-mode should have the same update.

Note: mode: x will essentially become same as mode: 'nearest', axis: 'x', but limited to hitRadius

For implementation, I would modify getNearestItems to optionally consider hitRadius:

/**
 * Helper function to get the items nearest to the event position considering all visible items in teh chart
 * @param chart {Chart} the chart to look at elements from
 * @param position {Point} the point to be nearest to
 * @param intersect {Boolean} if true, only consider items that intersect the position
 * @param axis {String} 'x','y','xy'
 * @param useHitRadius {Boolean} limit items by hitRadius
 * @return {ChartElement[]} the nearest items
 */
function getNearestItems(chart, position, intersect, axis, useHitRadius) {
	var minDistance = Number.POSITIVE_INFINITY;
	var distanceMetric = getDistanceMetricForAxis(axis);
	var nearestItems = [];

	parseVisibleItems(chart, function(element) {
		if (intersect && !element.inRange(position.x, position.y)) {
			return;
		}

		if (useHitRadius) {
			if ((axis === 'x' && !element.inXRange(position.x)) || (axis === 'y' && !element.inYRange(position.y))) {
				return;
			}
		}

		var center = element.getCenterPoint();
		var distance = distanceMetric(position, center);

		if (distance < minDistance) {
			nearestItems = [element];
			minDistance = distance;
		} else if (distance === minDistance) {
			// Can have multiple items at the same distance in which case we sort by size
			nearestItems.push(element);
		}
	});

	return nearestItems;
}

And use it:

		x: function(chart, e, options) {
			var position = getRelativePosition(e, chart);
			return getNearestItems(chart, position, options.intersect, 'x', true);
		},

Would need to modify all calls to getNearestItems (to supply axis instead of distanceMetric, useHitRadius can be omitted) and also move getDistanceMetricForAxis above getNearestItems

Thoughts, @benmccann @simonbrunel @etimberg (others welcome too)?

[malle-pietje]

Interested to see what the status of this PR is.

I also wanted to add that IMHO the tooltip should always only display data for a single data point (together with data for any corresponding data point from other datasets). I have worked with HighCharts quite a lot and even in very dense charts (365 points on a 580x200 chart) the tooltip always only displays data for a single data point:

Building a chart consuming the same data structure with Chart.js, even with a pointHitRadius value of 1 I always see multiple data points show up in a single tooltip:

Apart from this minor issue I really love the ease of use of Chart.js. Let that be clear 😃

For now, I can minimize the effect by dynamically modifying the pointHitRadius value when zooming in and out.

[benmccann]

@kurkle 's suggestion generally sounds good to me. @huaruiwu I know this PR has been open for a long time. Are you still interested in moving forward and do you think you'd be able to implement the suggested changes?

[benmccann]

Hi @huaruiwu we haven't heard anything about this PR for awhile, so I'm just checking to see if you're still interested in pursuing. If we don't hear anything after awhile we may end up closing this PR, but you can always pursue it again in the future. Thanks!

[CrokinoleMaster]

hi @benmccann, it's been a while since I've used ChartJS. I do want to take a look at this but not sure when I'll get to it. Feel free to close this if you're cleaning things up. I can always open another PR when I decide to come back to it

[benmccann]

Ok, I'll close it for now just to make it easier for us to keep track of which PRs need review. Please feel free to reopen at any time though