-
Notifications
You must be signed in to change notification settings - Fork 94
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
Highcharts prototype overrides can leak between charts #115
Comments
Good spot, that looks like a bug to me. Are you using React JSX Highcharts version 3? |
yes, from package.json: |
By any chance are you overriding I've found that in my examples I'm overriding the reset method for the SynchronisedCharts example, however that seems to be bleeding through to other examples such as the SplineWithPlotBands example. I had no idea Webpack would build the code in that way! |
hehe yes i am, in a sync charts as you do :) I'll try to remove and see if it goes away. |
yes that is it! |
Wondering if I should make i.e. export default withHighcharts(MyComponent, Highcharts, {
'Pointer.prototype.reset': () => {},
'Point.prototype.highlight': function (func, event) {
this.onMouseOver(); // Show the hover marker
this.series.chart.tooltip.refresh(this); // Show the tooltip
this.series.chart.xAxis[0].drawCrosshair(event, this); // Show the crosshair
}
}); We can apply the overrides when the Worth noting, in the second override function |
IMHO that looks clean. |
It overrides class prototype methods, so it should bleed all instances created. About the HOC: Isn't there any other way to have that functionality than modifying the prototype? Events? Tooltip formatter code? |
Highcharts instantiates chart.pointer on first render. The intance methods can be modified in some "after first render" event hook. That way the global Pointer prototype is not polluted. Something like this (does not work, just an example): <Chart onRedraw={ chart.pointer = myPointer //TODO run only once } /> |
Yeah good point
Ah yes, also correct. I was thinking about this too.
Potentially we could use the getChart = function (chart) {
chart.pointer.reset = () => {};
// Not sure if this works
Highcharts.addEvent(Highcharts.Point, 'afterInit', function () {
const point = this;
// Adding highlight fn to every point instance, not very efficient
point.highlight = function (event) {
this.onMouseOver(); // Show the hover marker
this.series.chart.tooltip.refresh(this); // Show the tooltip
this.series.chart.xAxis[0].drawCrosshair(event, this); // Show the crosshair
};
}
}
render () {
return (
<HighchartChart callback={this.getChart}>
// omitted
<HighchartChart />
)
} Although that requires a bit more |
I've encountered a similar bug, testing my application in IE11. I have a chart with multiple series with a tooltip that shows correctly on hover, and also disappears correctly when I leave the chart. In my case I have an onclick event tied to the charts' points, which when triggered has the unintended effect of creating a static tooltip which does not disappear. This can be repeated multiple times. It does not happen in chrome or FF. Some info on the used versions:
|
@ErnstWevers would you be able to give a simplified reproducible example? Its difficult to debug without knowing what you're doing in the the click handler etc. |
@whawker I worked on it yesterday, but I am having some issues replicating the behavior outside our application. I have narrowed it down but there are still a number of options. A little recap: since I'm working in 2.3.0 the onClick prop has to be passed as an click event option. This works with a logger in the application but once I connect it to the actual functionality, i.e. make a fetch call to grab the raw data the graph represents, it shows the erroneous behavior. I haven't been able to replicate the error outside of the original application. So for now the tooltip is out, I will keep you informed if I manage to produce a reproduction. |
I have replicated the problem here. It seems to be a side effect of using Preact instead of React. |
@ErnstWevers that is a strange way to pass plot options, the usual React JSX Highcharts way is just to pass This seems to resolve the issue. Highcharts.setOptions({
lang: {
noData: 'Geen berichten gevonden voor deze periode',
resetZoom: 'Herstel zoom'
}
});
const plotOptions = onClickAction => ({
series: {
cursor: 'pointer',
point: {
events: {
click() {
console.log('I got clicked! My category is: ', this.category);
onClickAction();
}
}
}
}
});
const Chart = ({ title = '', onClickAction = () => {} }) => {
const plotOpts = plotOptions(onClickAction);
return (
<div className="app">
<HighchartsChart plotOptions={plotOpts}>
<Title>{title}</Title>
<Legend layout="vertical" align="right" verticalAlign="middle" />
<XAxis>
<XAxis.Title>X</XAxis.Title>
</XAxis>
<YAxis id="axis">
<LineSeries name="testSeries1" data={[1, 2, 3, 2, 3, 4, 3, 4]} />
<LineSeries name="testSeries2" data={[2, 3, 4, 3, 4, 5, 4, 5]} />
<LineSeries name="testSeries2" data={[4, 3, 3, 2, 2, 3, 2, 1]} />
</YAxis>
<Tooltip shared />
</HighchartsChart>
</div>
);
}; |
When i hover on a curve the tooltip shows as expected.
But when I leave the chart though, the tooltip stays there. Is it normal?
The text was updated successfully, but these errors were encountered: