Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Clone figure.layout so plotly.js doesn't mutate it #279

Merged
merged 7 commits into from
Sep 7, 2018

Conversation

valentijnnieman
Copy link
Contributor

This fixes the bug where Graphs would resize when only supplying one of either width or height in figure.layout, especially visible in a Tab component.

@chriddyp
Copy link
Member

chriddyp commented Aug 27, 2018

  • Can we create a test case for this? You could run through the example that you created and take screenshots on every click. If you rebase, then we should see the test fail before the commit and then fixed after the commit.
  • Can we create a new issue about cloning data and include some of the notes from our internal discussion about cloning data arrays?

@valentijnnieman
Copy link
Contributor Author

@chriddyp Created issue #287 and wrote a test.

self.snapshot("Tabs with Graph - initial (graph should not resize)")

tab_two.click()
time.sleep(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of sleeping, could we try adding an element with an ID inside tab-2-example and then doing a self.wait_for_element_by_css_selector('#tab-2-element')? In principle it should be more stable.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although maybe let's try to start using the official selenium API rather than my wait_for_element_by_css_selector api, which might be buggy. more context here: #288

@chriddyp
Copy link
Member

thanks! 💃

@rmarren1
Copy link
Contributor

rmarren1 commented Sep 4, 2018

Can you re-base? You would just need to change PlotMethod to Plotly.react

@radekwlsk
Copy link

radekwlsk commented Sep 5, 2018

Change introduced in 0.28.1 breaks my candlestick dcc.Graph.

Changed

candlestick and OHLC charts are now plotted using the Plotly.react method instead of the Plotly.newPlot method.

On 0.28.0 it works correctly.

My dcc.Graph has initially empty figure and is in a hidden html.Div, once button of input form is pressed the callbacks generating data, and returning it to output which is figure of a dcc.Graph fires together with callback that sets hidden=False of the html.Div containing the graph.
On 0.28.1 it does not resize correctly to page width set in CSS, the figure only has height=900 set.

Is that what those changes fix?

@chriddyp
Copy link
Member

chriddyp commented Sep 5, 2018

@radekwlsk - Can you create a small, reproducable example?

@rmarren1
Copy link
Contributor

rmarren1 commented Sep 6, 2018

@radekwlsk This is a known issue: https://community.plot.ly/t/plotly-resize-bug-using-html-details-hidden-div-and-width-100/11201/6

I think that the temporary patch on candlestick we had between 0.22.1 and 0.28.1 fixed this on candlestick, but lead to other bugs.

I think that as a work-around, you can display an empty Div (no children) when you should have hidden=True and then, rather than setting the figure prop of your Graph with a callback, you can set the children prop of your Div with a newly constructed Graph. This way, it will re-size.

@valentijnnieman valentijnnieman merged commit 5aa3962 into master Sep 7, 2018
@valentijnnieman valentijnnieman deleted the graph_resize_fix branch September 7, 2018 16:36
@rmarren1
Copy link
Contributor

rmarren1 commented Sep 7, 2018

Can this be closed? #256

@valentijnnieman
Copy link
Contributor Author

@rmarren1 Yep.

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

Successfully merging this pull request may close these issues.

4 participants