pablojim / highcharts-ng

AngularJS directive for Highcharts
MIT License
1.73k stars 463 forks source link

Checking for existence of ctrl and chart before attempting reflow. #614

Closed natedsaint closed 6 years ago

natedsaint commented 6 years ago

Fixes an issue where rapidly replacing the chart config can lead to this being called before the chart is ready and throwing an error. Also, given that this is in a timeout and therefore the method is called after the initiating render cycle, it makes sense to make sure there's something to act upon.

image

NOTE: we came upon this issue because we realized that simply swapping the config rapidly actually caused a series of bugs if the user still had tooltips visible, as they could not be destroyed correctly and rebuilt. Our fix was to first set the config to null and then set it to the new config on the next draw cycle to avoid this, but it resulted in this issue. I think the correct fix might be addressing that concern as well: I can provide a repro path for it if needed, but this was a relatively easy workaround and also seems logical given the use of a timeout.

ngehlert commented 6 years ago

@natedsaint with the latest version there is support for $onChanges so this problem should not occur anymore. can you test if you still encounter your problem?

natedsaint commented 6 years ago

Sorry for the delay, I had to refactor some things to make use of it. It looks good now! Thanks for the update.