Skip to content

Fix layout bugs caused by plotly cruft #4

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

Merged
merged 1 commit into from
May 3, 2016
Merged

Fix layout bugs caused by plotly cruft #4

merged 1 commit into from
May 3, 2016

Conversation

eXeC64
Copy link
Contributor

@eXeC64 eXeC64 commented Apr 27, 2016

Plotly creates some additional cruft in the DOM when rendering its graphs. When the graph has been unmounted, the cruft remains and magically interferes with the rest of my layout, specifically the <AppBar> component from material-ui.

This pull request fixes that issue by removing the cruft automatically when unmounting graphs.

@benjeffery
Copy link
Owner

Thanks for the PR! I wasn't aware of this. One question though - if there is more than one plotly component open, and one closes, is the other adversely affected by removing this node?

BTW Nice surname :D

@eXeC64
Copy link
Contributor Author

eXeC64 commented Apr 30, 2016

Removing the element doesn't appear to adversely affect anything. On my end I've actually moved the removal to be immediately after every draw call, so that the element is removed immediately, and the plots themselves sill work fine.

I've no idea why Plotly inserts this stuff, it seems rather unnecessary.

Re surname: Yes :) Although the trouble I (and presumably you) have with people spelling it as Jeffrey, Jeffries, Geoffreys, etc.

@benjeffery
Copy link
Owner

Seems it is used for testing some browser behaviour, might be worth asking plotlyjs to clean up after itself, but for now I'll just merge this.

@benjeffery benjeffery merged commit 6b4b4be into benjeffery:master May 3, 2016
@benjeffery
Copy link
Owner

BTW - my main project just suffered weird layout issues (in chrome only) due to this cruft. It would have taken ages to debug if you hadn't already pointed it out, so thanks!

@benjeffery
Copy link
Owner

I've submitted an upstream issue for this: plotly/plotly.js#513

@eXeC64
Copy link
Contributor Author

eXeC64 commented May 5, 2016

Good call. I didn't think of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants