search

benjeffery / react-plotlyjs

ReactJS / PlotlyJS integration. Draw plotly.js graphs in your react app.
MIT License
111 stars 29 forks source link

Fix layout bugs caused by plotly cruft #4

Closed eXeC64 closed 8 years ago

eXeC64 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

I've submitted an upstream issue for this: https://github.com/plotly/plotly.js/issues/513

eXeC64 commented 8 years ago

Good call. I didn't think of that.