Merge pull request plotly#905 from almarklein/figure-state

Dont clone figure.layout

Author: alexcjohnson

packages/dash-core-components/CHANGELOG.md

@@ -4,12 +4,13 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [UNRELEASED]
 ### Fixed
-- [#903](https://github.com/plotly/dash-core-components/pull/903) - part of fixing dash import bug https://github.com/plotly/dash/issues/1143
+- [#905](https://github.com/plotly/dash-core-components/pull/905) Make sure the `figure` prop of `dcc.Graph` receives updates from user interactions in the graph, by using the same `layout` object as provided in the prop rather than cloning it. Fixes [#879](https://github.com/plotly/dash-core-components/issues/879).
+- [#903](https://github.com/plotly/dash-core-components/pull/903) Part of fixing dash import bug https://github.com/plotly/dash/issues/1143
 
 ### Updated
-- [#911](https://github.com/plotly/dash-core-components/pull/911)
+- [#911](https://github.com/plotly/dash-core-components/pull/911), [#906](https://github.com/plotly/dash-core-components/pull/906)
+  - Upgraded Plotly.js to [1.58.4](https://github.com/plotly/plotly.js/releases/tag/v1.58.4)
     - Patch Release [1.58.4](https://github.com/plotly/plotly.js/releases/tag/v1.58.4)
-- [#906](https://github.com/plotly/dash-core-components/pull/906)
     - Patch Release [1.58.3](https://github.com/plotly/plotly.js/releases/tag/v1.58.3)
 
 ## [1.14.1] - 2020-12-09

packages/dash-core-components/src/fragments/Graph.react.js

@@ -131,6 +131,8 @@ class PlotlyGraph extends Component {
         this.getLayoutOverride = this.getLayoutOverride.bind(this);
         this.graphResize = this.graphResize.bind(this);
         this.isResponsive = this.isResponsive.bind(this);
+
+        this.state = {override: {}, originals: {}};
     }
 
     plot(props) {
@@ -226,8 +228,29 @@ class PlotlyGraph extends Component {
         if (!layout) {
             return layout;
         }
-
-        return mergeDeepRight(layout, this.getLayoutOverride(responsive));
+        const override = this.getLayoutOverride(responsive);
+        const {override: prev_override, originals: prev_originals} = this.state;
+        // Store the original data that we're about to override
+        const originals = {};
+        for (const key in override) {
+            if (layout[key] !== prev_override[key]) {
+                originals[key] = layout[key];
+            } else if (prev_originals.hasOwnProperty(key)) {
+                originals[key] = prev_originals[key];
+            }
+        }
+        this.setState({override, originals});
+        // Undo the previous override, but only for keys that the user did not change
+        for (const key in prev_originals) {
+            if (layout[key] === prev_override[key]) {
+                layout[key] = prev_originals[key];
+            }
+        }
+        // Apply the current override
+        for (const key in override) {
+            layout[key] = override[key];
+        }
+        return layout; // not really a clone
     }
 
     getConfigOverride(responsive) {

packages/dash-core-components/tests/dash_core_components_page.py

@@ -118,3 +118,12 @@ def move_to_coord_fractions(self, elem_or_selector, fx, fy):
 
     def release(self):
         ActionChains(self.driver).release().perform()
+
+    def click_and_drag_at_coord_fractions(self, elem_or_selector, fx1, fy1, fx2, fy2):
+        elem = self._get_element(elem_or_selector)
+
+        ActionChains(self.driver).move_to_element_with_offset(
+            elem, elem.size["width"] * fx1, elem.size["height"] * fy1
+        ).click_and_hold().move_to_element_with_offset(
+            elem, elem.size["width"] * fx2, elem.size["height"] * fy2
+        ).release().perform()

packages/dash-core-components/tests/integration/graph/test_graph_varia.py

@@ -25,7 +25,7 @@ def findAsyncPlotlyJs(scripts):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_candlestick(dash_dcc, is_eager):
+def test_grva001_candlestick(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
     app.layout = html.Div(
         [
@@ -75,7 +75,7 @@ def update_graph(n_clicks):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_graphs_with_different_figures(dash_dcc, is_eager):
+def test_grva002_graphs_with_different_figures(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
     app.layout = html.Div(
         [
@@ -160,7 +160,7 @@ def show_relayout_data(data):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_empty_graph(dash_dcc, is_eager):
+def test_grva003_empty_graph(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
 
     app.layout = html.Div(
@@ -193,7 +193,7 @@ def render_content(click, prev_graph):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_graph_prepend_trace(dash_dcc, is_eager):
+def test_grva004_graph_prepend_trace(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
 
     def generate_with_id(id, data=None):
@@ -358,7 +358,7 @@ def display_data(trigger, fig):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_graph_extend_trace(dash_dcc, is_eager):
+def test_grva005_graph_extend_trace(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
 
     def generate_with_id(id, data=None):
@@ -521,7 +521,7 @@ def display_data(trigger, fig):
 
 
 @pytest.mark.parametrize("is_eager", [True, False])
-def test_unmounted_graph_resize(dash_dcc, is_eager):
+def test_grva006_unmounted_graph_resize(dash_dcc, is_eager):
     app = dash.Dash(__name__, eager_loading=is_eager)
 
     app.layout = html.Div(
@@ -619,7 +619,7 @@ def test_unmounted_graph_resize(dash_dcc, is_eager):
     dash_dcc.driver.set_window_size(window_size["width"], window_size["height"])
 
 
-def test_external_plotlyjs_prevents_lazy(dash_dcc):
+def test_grva007_external_plotlyjs_prevents_lazy(dash_dcc):
     app = dash.Dash(
         __name__,
         eager_loading=False,
@@ -658,3 +658,168 @@ def load_chart(n_clicks):
     scripts = dash_dcc.driver.find_elements(By.CSS_SELECTOR, "script")
     assert findSyncPlotlyJs(scripts) is None
     assert findAsyncPlotlyJs(scripts) is None
+
+
+def test_grva008_shapes_not_lost(dash_dcc):
+    # See issue #879 and pr #905
+    app = dash.Dash(__name__)
+
+    fig = {"data": [], "layout": {"dragmode": "drawrect"}}
+    graph = dcc.Graph(id="graph", figure=fig, style={"height": "400px"})
+
+    app.layout = html.Div(
+        [
+            graph,
+            html.Br(),
+            html.Button(id="button", children="Clone figure"),
+            html.Div(id="output", children=""),
+        ]
+    )
+
+    app.clientside_callback(
+        """
+        function clone_figure(_, figure) {
+            const new_figure = {...figure};
+            const shapes = new_figure.layout.shapes || [];
+            return [new_figure, shapes.length];
+        }
+        """,
+        Output("graph", "figure"),
+        Output("output", "children"),
+        Input("button", "n_clicks"),
+        State("graph", "figure"),
+    )
+
+    dash_dcc.start_server(app)
+    button = dash_dcc.wait_for_element("#button")
+    dash_dcc.wait_for_text_to_equal("#output", "0")
+
+    # Draw a shape
+    dash_dcc.click_and_hold_at_coord_fractions("#graph", 0.25, 0.25)
+    dash_dcc.move_to_coord_fractions("#graph", 0.35, 0.75)
+    dash_dcc.release()
+
+    # Click to trigger an update of the output, the shape should survive
+    dash_dcc.wait_for_text_to_equal("#output", "0")
+    button.click()
+    dash_dcc.wait_for_text_to_equal("#output", "1")
+
+    # Draw another shape
+    dash_dcc.click_and_hold_at_coord_fractions("#graph", 0.75, 0.25)
+    dash_dcc.move_to_coord_fractions("#graph", 0.85, 0.75)
+    dash_dcc.release()
+
+    # Click to trigger an update of the output, the shape should survive
+    dash_dcc.wait_for_text_to_equal("#output", "1")
+    button.click()
+    dash_dcc.wait_for_text_to_equal("#output", "2")
+
+
+@pytest.mark.parametrize("mutate_fig", [True, False])
+def test_grva009_originals_maintained_for_responsive_override(mutate_fig, dash_dcc):
+    # In #905 we made changes to prevent shapes from being lost.
+    # This test makes sure that the overrides applied by the `responsive`
+    # prop are "undone" when the `responsive` prop changes.
+
+    app = dash.Dash(__name__)
+
+    graph = dcc.Graph(
+        id="graph",
+        figure={"data": [{"y": [1, 2]}], "layout": {"width": 300, "height": 250}},
+        style={"height": "400px", "width": "500px"},
+    )
+    responsive_size = [500, 400]
+    fixed_size = [300, 250]
+
+    app.layout = html.Div(
+        [
+            graph,
+            html.Br(),
+            html.Button(id="edit_figure", children="Edit figure"),
+            html.Button(id="edit_responsive", children="Edit responsive"),
+            html.Div(id="output", children=""),
+        ]
+    )
+
+    if mutate_fig:
+        # Modify the layout in place (which still has changes made by responsive)
+        change_fig = """
+            figure.layout.title = {text: String(n_fig || 0)};
+            const new_figure = {...figure};
+        """
+    else:
+        # Or create a new one each time
+        change_fig = """
+            const new_figure = {
+                data: [{y: [1, 2]}],
+                layout: {width: 300, height: 250, title: {text: String(n_fig || 0)}}
+            };
+        """
+
+    callback = (
+        """
+        function clone_figure(n_fig, n_resp, figure) {
+        """
+        + change_fig
+        + """
+            let responsive = [true, false, 'auto'][(n_resp || 0) % 3];
+            return [new_figure, responsive, (n_fig || 0) + ' ' + responsive];
+        }
+        """
+    )
+
+    app.clientside_callback(
+        callback,
+        Output("graph", "figure"),
+        Output("graph", "responsive"),
+        Output("output", "children"),
+        Input("edit_figure", "n_clicks"),
+        Input("edit_responsive", "n_clicks"),
+        State("graph", "figure"),
+    )
+
+    dash_dcc.start_server(app)
+    edit_figure = dash_dcc.wait_for_element("#edit_figure")
+    edit_responsive = dash_dcc.wait_for_element("#edit_responsive")
+
+    def graph_dims():
+        return dash_dcc.driver.execute_script(
+            """
+            const layout = document.querySelector('.js-plotly-plot')._fullLayout;
+            return [layout.width, layout.height];
+            """
+        )
+
+    dash_dcc.wait_for_text_to_equal("#output", "0 true")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "0")
+    assert graph_dims() == responsive_size
+
+    edit_figure.click()
+    dash_dcc.wait_for_text_to_equal("#output", "1 true")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "1")
+    assert graph_dims() == responsive_size
+
+    edit_responsive.click()
+    dash_dcc.wait_for_text_to_equal("#output", "1 false")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "1")
+    assert graph_dims() == fixed_size
+
+    edit_figure.click()
+    dash_dcc.wait_for_text_to_equal("#output", "2 false")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "2")
+    assert graph_dims() == fixed_size
+
+    edit_responsive.click()
+    dash_dcc.wait_for_text_to_equal("#output", "2 auto")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "2")
+    assert graph_dims() == fixed_size
+
+    edit_figure.click()
+    dash_dcc.wait_for_text_to_equal("#output", "3 auto")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "3")
+    assert graph_dims() == fixed_size
+
+    edit_responsive.click()
+    dash_dcc.wait_for_text_to_equal("#output", "3 true")
+    dash_dcc.wait_for_text_to_equal(".gtitle", "3")
+    assert graph_dims() == responsive_size