search

chartjs / chartjs-plugin-deferred

Chart.js plugin to defer initial chart updates
https://chartjs-plugin-deferred.netlify.app/
MIT License
110 stars 21 forks source link

Ticking issue in IE - in response to #8 #10

Closed fitzy90 closed 6 years ago

fitzy90 commented 6 years ago

This seems to correct the IE ticking issue #8. If its OK to commit that would be great

simonbrunel commented 6 years ago

Thanks @fitzy90, however this change silents the issue instead of fixing it, which can potentially hide real errors later. We need to figure out why stub is undefined (or null) in some cases, maybe event.target is not correct or maybe IE doesn't accept expando?

I'm not able to reproduce this issue in IE11, what version are you using to reproduce this error? can you log the content of event and node when stub is undefined?

fitzy90 commented 6 years ago

Hey

I have managed to replicate for you, see the test case link. I haven’t looked at it for very long so I can’t really comment.

If you use IE11 to navigate here you should see the issue in the developer tools

http://chart.tech-solutions.co.uk/

Let me know if can help any further

Thanks

Jack

From: Simon Brunel [mailto:notifications@github.com] Sent: 02 March 2018 13:28 To: chartjs/chartjs-plugin-deferred Cc: Jack Fitzgerald; Mention Subject: Re: [chartjs/chartjs-plugin-deferred] Ticking issue in IE - in response to #8 (#10)

Thanks @fitzy90https://github.com/fitzy90, however this change silents the issue instead of fixing it, which can potentially hide real errors later. We need to figure out why stub is undefined (or null) in some cases, maybe event.target is not correct or maybe IE doesn't accept expando?

I'm not able to reproduce this issue in IE11, what version are you using to reproduce this error? can you log the content of event and node when stub is undefined?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/chartjs/chartjs-plugin-deferred/pull/10#issuecomment-369920441, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AfISlDDDT7ttW-l55qHiB7NmVF50EenRks5taUjKgaJpZM4SZ4Cv.

simonbrunel commented 6 years ago

Thanks @fitzy90, I can reproduce it! I think the problem is that the scroll event is not correctly removed (at this line), certainly because Chart.helpers.removeEvent doesn't call the associated "remove" method matching the call at line 104

simonbrunel commented 6 years ago

@fitzy90 I pushed a fix for this issue in master (ee8c7a9ee18558304ed9d065f8354db709b964c9), can you confirm that it works correctly? (samples)

fitzy90 commented 6 years ago

@simonbrunel I can confirm that works perfectly! Thanks for your help on this!

simonbrunel commented 6 years ago

Thanks @fitzy90, I'm closing this PR and will release the fix very soon.