-
Notifications
You must be signed in to change notification settings - Fork 29
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
Fix multiple charts facade and small google fix #1065
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, left some code format NITs. Maybe if you have time we can also consider a test.
@Soare-Robert-Daniel Tested and the editor issue is fixed now, thank you ✔️ Found something strange:
Can be checked here:
|
@irinelenache, it seems that depending on browser loading, the events might be overloaded and ignored. I added a short delay as a fix. |
@Soare-Robert-Daniel Checked again and everything's fine now, thank you 🚀 |
🎉 This PR is included in version 3.10.5 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
The problem: after the first chart started the rendering process, the function
displayChartsOnFrontEnd()
triggered again.Because the charts were empty (they were in the process of rendering), this code removed the rendering status
$( 'div.viz-facade-loaded:not(.visualizer-lazy):not(.visualizer-cw-error):empty' ).removeClass( 'viz-facade-loaded' );
And then this `$('div.visualizer-front:not(.visualizer-lazy):not(.viz-facade-loaded)') put it again in the loop since it considered that they never started the rendering process.
Practically, we were re-rendering the charts that were not finished.
Thus, the infinite loop was crashing the page.
I simplified the logic. Now, after the page loads, the rendering of the chart is triggered once.
Testing