Proposal only! Fix voronoi tooltips to show all siblings for duplicates #798
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When using voronoi points for tooltips on any chart relying on a scatter chart underneath, collapse points to unique values that D3 accepts (basically, points are considered duplicates with 1e-6 of each other). But instead of hiding the values considered duplicates, show them all in the same toolip. FIXME: this is only implemented for lineChart, still need to amend this commit with changes to the others that use scatter underneath.
|
@liquidpele, @robinfhu, what do you guys think? Perhaps too much mechanism? |
| @@ -54,6 +54,13 @@ nv.models.lineChart = function() { | |||
| y = yAxis.tickFormat()(lines.y()(e.point, e.pointIndex)), | |||
| content = tooltip(e.series.key, x, y, e, chart); | |||
|
|
|||
| if (e.sibs) { | |||
|
One thing I like is that this didn't really add much code, it was more of a refactor. The change should be transparent to end users...most people shouldn't notice any differences. The difference is that there is now a 'siblings' data property available for the tooltips. This should be tested pretty thoroughly too. Nice work. |
| @@ -54,6 +54,13 @@ nv.models.lineChart = function() { | |||
| y = yAxis.tickFormat()(lines.y()(e.point, e.pointIndex)), | |||
| content = tooltip(e.series.key, x, y, e, chart); | |||
|
|
|||
| if (e.sibs) { | |||
| for (var i = 0; i < e.sibs.length; i++) { | |||
There was a problem hiding this comment.
This is a good place to use "for in" instead of having to use the length property, this also makes it able to handle objects if you wanted to switch to that later.
|
So from what I can tell, it looks like the issue this solves is that the tooltips would only return data about a single point if there were multiple points in the same location? |
|
This PR still looks incomplete, is it in progress? |
|
@robinfhu, yes, just behind on a number of other things at work. |
|
@robinfhu, @liquidpele, just getting back to this, probably too late for 1.8.1. |
portante
added a commit
to portante/pbench
that referenced
this pull request
Dec 11, 2015
Instead of just eliminating points, calculate a full set of statistics for each series, and then sort them by their average values, picking the top 20. We stop considering any series for the graph once we find one whose average is less than the threshold. We also stop using the Voronoi objects for tool tips since NVD3 has a bug which causes an exception in D3 (see: novus/nvd3#798). Change-Id: I8f232bdd7f8024706220c9fa87fa460ab61209e8
ndokos
pushed a commit
to ndokos/pbench
that referenced
this pull request
Dec 11, 2015
Instead of just eliminating points, calculate a full set of statistics for each series, and then sort them by their average values, picking the top 20. We stop considering any series for the graph once we find one whose average is less than the threshold. We also stop using the Voronoi objects for tool tips since NVD3 has a bug which causes an exception in D3 (see: novus/nvd3#798). Change-Id: I8f232bdd7f8024706220c9fa87fa460ab61209e8
portante
added a commit
to portante/pbench
that referenced
this pull request
Dec 23, 2015
Instead of just eliminating points, calculate a full set of statistics for each series, and then sort them by their average values, picking the top 20. We stop considering any series for the graph once we find one whose average is less than the threshold. We also stop using the Voronoi objects for tool tips since NVD3 has a bug which causes an exception in D3 (see: novus/nvd3#798). Change-Id: I8f232bdd7f8024706220c9fa87fa460ab61209e8
portante
added a commit
to portante/pbench
that referenced
this pull request
Dec 23, 2015
Instead of just eliminating points, calculate a full set of statistics for each series, and then sort them by their average values, picking the top 20. We stop considering any series for the graph once we find one whose average is less than the threshold. We also stop using the Voronoi objects for tool tips since NVD3 has a bug which causes an exception in D3 (see: novus/nvd3#798). Change-Id: I8f232bdd7f8024706220c9fa87fa460ab61209e8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a proposal for a fix. It solves the voronoi problem of duplicate points by eliminating them, but saving the duplicates as siblings that are also displayed in the tooltip. This should solve problems like #330 once and for all.
For now, we only do that for
lineCharts.Please ignore the two debugging commits, and the base change to
lines.js.