rickshaw
rickshaw copied to clipboard
All tooltips rendered on hover - one is enough
For any chart, when you hover over one data point, a tooltip is actually generated for every data point in the chart - the other ones aren't shown, but this results in potentially hundreds of div elements being added, moved, etc.. resulting in really poor performance, not to mention that this is completely useless.
To fix, in Rickshaw.Graph.HoverDetail render() function, move if (d.active) {
from covering the last two lines of the loop, to the very top of the loop, just before var item = document.createElement('div');
- that was only one element is created and shown.. all that needs.
I'm sure this can be made even more efficient, but this would be a good start.
Actually, you can also stop the loop once you've found the correct series that needs to have the tooltip shown. Something like the following should do:
detail.sort(sortFn).forEach( function(d) {
if (domainMouseY > d.value.y0 && domainMouseY < d.value.y0 + d.value.y && !activeItem) {
d.formattedYValue = (this.yFormatter.constructor == Array) ?
this.yFormatter[detail.indexOf(d)](d.value.y) :
this.yFormatter(d.value.y);
d.graphX = graphX;
d.graphY = graph.y(d.value.y0 + d.value.y);
activeItem = d;
return;
}
}, this );
oh and then, you can do
if (this.visible && activeItem) {
this.render( {
activeItem: activeItem,
domainX: domainX,
formattedXValue: formattedXValue,
mouseX: eventX,
mouseY: eventY
} );
}
and the render function then doesnt need any loops
render: function(args) {
var activeItem = args.activeItem;
var domainX = args.domainX;
var mouseX = args.mouseX;
var mouseY = args.mouseY;
var formattedXValue = args.formattedXValue;
var xLabel = document.createElement('div');
xLabel.className = 'x_label';
xLabel.innerHTML = formattedXValue;
this.element.appendChild(xLabel);
var item = document.createElement('div');
item.className = 'item';
item.innerHTML = this.formatter(activeItem.series, domainX, activeItem.value.y, formattedXValue, activeItem.formattedYValue, activeItem);
item.style.top = this.graph.y(activeItem.value.y0 + activeItem.value.y) + 'px';
this.element.appendChild(item);
var dot = document.createElement('div');
dot.className = 'dot';
dot.style.top = item.style.top;
dot.style.borderColor = activeItem.series.color;
this.element.appendChild(dot);
item.className = 'item active';
dot.className = 'dot active';
this.show();
if (typeof this.onRender == 'function') {
this.onRender(args);
}
},
and my apologies, I'm still not too familiar with github in terms of providing patches.. its kinda complicated when i have quite a few more changes to the file too that are unrelated.. hope this works.
All good points.
Things evolved as they did because we started with a bunch of use cases where it was actually useful to show details for all of the series for a given time at once. Then we hid "inactive" tooltips by default, but the consumer can still show them by overriding some CSS.
But I think that's mostly an edge case. We'll pursue going forward with the optimizations you suggested, and maybe just provide an example subclass that shows all the data for a given time at once if that's what to do.
and my apologies, I'm still not too familiar with github in terms of providing patches.. its kinda complicated when i have quite a few more changes to the file too that are unrelated..
Pull requests are pretty awesome: https://help.github.com/articles/using-pull-requests/