Skip to content

Changing className on container breaks subsequent redraws #916

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

Closed
benjeffery opened this issue Sep 8, 2016 · 3 comments
Closed

Changing className on container breaks subsequent redraws #916

benjeffery opened this issue Sep 8, 2016 · 3 comments

Comments

@benjeffery
Copy link
Contributor

benjeffery commented Sep 8, 2016

Hi,
Had a quick search and can't see any mention of this.
See fiddle here: https://jsfiddle.net/b1jLzqs1/2/
The plot should redraw three times - but only does so twice.
If one removes the line that modifies the container's className between redraws, then the redraw is successful.
Initial report: benjeffery/react-plotlyjs#10
Thanks!

@etpinard
Copy link
Contributor

etpinard commented Sep 8, 2016

plotly.js pre-1.17.0 failed silently on redraw when the input graph div classes didn't include js-plotly-plot - see Lib.isPlotDiv

If you're looking for a better handling of this situation, I think this is now fixed via #907 which is part of the 1.17.0 release - where Plotly.redraw now throws an error when Lib.isPlotDiv(gd) is false.

Can I ask why you would want to change the contaienr.className before a redraw?

@benjeffery
Copy link
Contributor Author

benjeffery commented Sep 8, 2016

Can I ask why you would want to change the container.className before a redraw?

The original reporter wanted to style the div when loading data, this can easily be done with an enclosing div, so I don't think there are any critical use cases for this.

I'm happy enough for this to be an error - the silent failure was the problem, so it's great that 1.17.0 includes this. I will add code to react-plotlyjs to detect class changes and give a friendly error also.

Many thanks for the response, feel free to close if you agree.

@etpinard
Copy link
Contributor

etpinard commented Sep 8, 2016

I'm happy enough for this to be an error

Great. I'll close this issue.

But, I'm open to suggestions about making plotly.js a little less intrusive (e.g. with className) if you have any.

@etpinard etpinard closed this as completed Sep 8, 2016
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

No branches or pull requests

2 participants