From 0482abdb63c2f3ec1d87ade1a36c67ea50cb78b9 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Mon, 14 Dec 2020 22:03:03 +0100 Subject: [PATCH 01/15] Dont clone figure.layout --- src/fragments/Graph.react.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index d4cdcf918..5f8a7ce87 100644 --- a/src/fragments/Graph.react.js +++ b/src/fragments/Graph.react.js @@ -151,13 +151,12 @@ class PlotlyGraph extends Component { } const configClone = this.getConfig(config, responsive); - const layoutClone = this.getLayout(figure.layout, responsive); gd.classList.add('dash-graph--pending'); return Plotly.react(gd, { data: figure.data, - layout: layoutClone, + layout: figure.layout, frames: figure.frames, config: configClone, }).then(() => { From ec7b6bc936e7fc2d340e05a723e496a42146c2de Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Tue, 15 Dec 2020 15:56:30 +0100 Subject: [PATCH 02/15] include responsive --- src/fragments/Graph.react.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index 5f8a7ce87..37f890e79 100644 --- a/src/fragments/Graph.react.js +++ b/src/fragments/Graph.react.js @@ -151,12 +151,13 @@ class PlotlyGraph extends Component { } const configClone = this.getConfig(config, responsive); + const layoutClone = this.getLayout(figure.layout, responsive); gd.classList.add('dash-graph--pending'); return Plotly.react(gd, { data: figure.data, - layout: figure.layout, + layout: layoutClone, frames: figure.frames, config: configClone, }).then(() => { @@ -225,8 +226,11 @@ class PlotlyGraph extends Component { if (!layout) { return layout; } - - return mergeDeepRight(layout, this.getLayoutOverride(responsive)); + let override = this.getLayoutOverride(responsive); + for (let key in override) { + layout[key] = override[key]; + } + return layout; // not really a clone } getConfigOverride(responsive) { From fc1d7a261fef7cb2c166e04c6b1b621c6851f316 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 16 Dec 2020 14:40:39 +0100 Subject: [PATCH 03/15] keep track of original values of overridden data --- src/fragments/Graph.react.js | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index 37f890e79..77931be89 100644 --- a/src/fragments/Graph.react.js +++ b/src/fragments/Graph.react.js @@ -226,8 +226,23 @@ class PlotlyGraph extends Component { if (!layout) { return layout; } - let override = this.getLayoutOverride(responsive); - for (let key in override) { + const override = this.getLayoutOverride(responsive); + const prev_layout = this.gd.current.layout; // === this.props.figure.layout + const prev_override_originals = (this.state && this.state.override_originals) || {}; + // Store the original data that we're about to override + const override_originals = {}; + for (const key in override) { + override_originals[key] = layout[key]; + } + this.setState({override_originals: override_originals}); + // Undo the previous override, but only for keys that the user did not change + for (const key in prev_override_originals) { + if (layout[key] === prev_layout[key]) { + layout[key] = prev_override_originals[key]; + } + } + // Apply the current override + for (const key in override) { layout[key] = override[key]; } return layout; // not really a clone From 99b4411e0a7c0b10d6beaa510d32b5e1d4e42994 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 16 Dec 2020 16:37:46 +0100 Subject: [PATCH 04/15] add test for layout shapes --- tests/integration/graph/test_graph_varia.py | 57 +++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index ea26c6178..bf2d08db7 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -2,6 +2,7 @@ import pytest import time import json +import plotly import dash from dash.dependencies import Input, Output, State import dash_html_components as html @@ -658,3 +659,59 @@ 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_shapes_not_lost(dash_dcc): + # See issue #879 + app = dash.Dash(__name__) + + fig = plotly.graph_objects.Figure(data=[]) + fig.update_layout(dragmode="drawrect") + graph = dcc.Graph(id="graph", figure=fig) + graph.config = {"modeBarButtonsToAdd": ["drawrect"]} + + 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) { + let new_figure = {...figure}; + let 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") + output = dash_dcc.wait_for_element("#output") + time.sleep(1) + + # 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") From 196ec857eae57d6ddacb9e0c8654d3c042050d8b Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 16 Dec 2020 16:56:43 +0100 Subject: [PATCH 05/15] fix test (and black) --- tests/integration/graph/test_graph_varia.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index bf2d08db7..02f1bbaa8 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -2,7 +2,6 @@ import pytest import time import json -import plotly import dash from dash.dependencies import Input, Output, State import dash_html_components as html @@ -665,17 +664,15 @@ def test_shapes_not_lost(dash_dcc): # See issue #879 app = dash.Dash(__name__) - fig = plotly.graph_objects.Figure(data=[]) - fig.update_layout(dragmode="drawrect") + fig = {"data": [], "layout": {"dragmode": "drawrect"}} graph = dcc.Graph(id="graph", figure=fig) - graph.config = {"modeBarButtonsToAdd": ["drawrect"]} app.layout = html.Div( [ graph, html.Br(), - html.Button(id='button', children="Clone figure"), - html.Div(id='output', children=""), + html.Button(id="button", children="Clone figure"), + html.Div(id="output", children=""), ] ) From c7338c1a3300cc809989625f42b533d8dc31c0fb Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Wed, 16 Dec 2020 17:12:39 +0100 Subject: [PATCH 06/15] flake --- tests/integration/graph/test_graph_varia.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index 02f1bbaa8..401691c98 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -690,7 +690,6 @@ def test_shapes_not_lost(dash_dcc): dash_dcc.start_server(app) button = dash_dcc.wait_for_element("#button") - output = dash_dcc.wait_for_element("#output") time.sleep(1) # Draw a shape From eb82ae126248e2bf96f58dfdc2641d43a8a6409c Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Wed, 16 Dec 2020 12:03:29 -0500 Subject: [PATCH 07/15] lint fixes for this PR and the import fix --- dash_core_components_base/__init__.py | 2 +- src/fragments/Graph.react.js | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dash_core_components_base/__init__.py b/dash_core_components_base/__init__.py index a236a451c..2a758db3a 100644 --- a/dash_core_components_base/__init__.py +++ b/dash_core_components_base/__init__.py @@ -19,7 +19,7 @@ "named \n'dash.py' in your current directory.", file=_sys.stderr) _sys.exit(1) -from ._imports_ import * # noqa: F401, F403 +from ._imports_ import * # noqa: F401, F403, E402 from ._imports_ import __all__ # noqa: E402 _current_path = _os.path.dirname(_os.path.abspath(__file__)) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index 77931be89..72a8c49bf 100644 --- a/src/fragments/Graph.react.js +++ b/src/fragments/Graph.react.js @@ -227,8 +227,9 @@ class PlotlyGraph extends Component { return layout; } const override = this.getLayoutOverride(responsive); - const prev_layout = this.gd.current.layout; // === this.props.figure.layout - const prev_override_originals = (this.state && this.state.override_originals) || {}; + const prev_layout = this.gd.current.layout; // === this.props.figure.layout + const prev_override_originals = + (this.state && this.state.override_originals) || {}; // Store the original data that we're about to override const override_originals = {}; for (const key in override) { @@ -245,7 +246,7 @@ class PlotlyGraph extends Component { for (const key in override) { layout[key] = override[key]; } - return layout; // not really a clone + return layout; // not really a clone } getConfigOverride(responsive) { From fcdd87d58837b947e00d450668e5ebedfbe199d8 Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Thu, 17 Dec 2020 22:51:02 +0100 Subject: [PATCH 08/15] improve how prev originals are handled --- src/fragments/Graph.react.js | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index 72a8c49bf..8cb1e2dec 100644 --- a/src/fragments/Graph.react.js +++ b/src/fragments/Graph.react.js @@ -227,19 +227,22 @@ class PlotlyGraph extends Component { return layout; } const override = this.getLayoutOverride(responsive); - const prev_layout = this.gd.current.layout; // === this.props.figure.layout - const prev_override_originals = - (this.state && this.state.override_originals) || {}; + const prev_override = (this.state && this.state.override) || {}; + const prev_originals = (this.state && this.state.originals) || {}; // Store the original data that we're about to override - const override_originals = {}; + const originals = {}; for (const key in override) { - override_originals[key] = layout[key]; + 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: override_originals}); + this.setState({override: override, originals: originals}); // Undo the previous override, but only for keys that the user did not change - for (const key in prev_override_originals) { - if (layout[key] === prev_layout[key]) { - layout[key] = prev_override_originals[key]; + for (const key in prev_originals) { + if (layout[key] === prev_override[key]) { + layout[key] = prev_originals[key]; } } // Apply the current override From 05ddff44a872c36d9c95dfda2e093a88f9bf7ecc Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Fri, 18 Dec 2020 13:48:11 +0100 Subject: [PATCH 09/15] add test to verify that original layout params are maintained --- tests/integration/graph/test_graph_varia.py | 61 ++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index 401691c98..ffbd2f946 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -661,7 +661,7 @@ def load_chart(n_clicks): def test_shapes_not_lost(dash_dcc): - # See issue #879 + # See issue #879 and pr #905 app = dash.Dash(__name__) fig = {"data": [], "layout": {"dragmode": "drawrect"}} @@ -711,3 +711,62 @@ def test_shapes_not_lost(dash_dcc): dash_dcc.wait_for_text_to_equal("#output", "1") button.click() dash_dcc.wait_for_text_to_equal("#output", "2") + + +def test_originals_maintained_for_responsive_override(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__) + + fig = {"data": [], "layout": {"autosize": None, "width": 300, "height": 300}} + graph = dcc.Graph(id="graph", figure=fig) + + app.layout = html.Div( + [ + graph, + html.Br(), + html.Button(id="button", children="Clone figure"), + html.Div(id="output", children=""), + ] + ) + + # Each time that the button is pressed we change responsive mode. + # It goes from null (default), to true, false, and back to null. + app.clientside_callback( + """function clone_figure(_, figure) { + let new_figure = {...figure}; + let index = window.internal_state_905 || 0; + window.internal_state_905 = index + 1; + let responsive = [true, false, null, null][index]; + return [new_figure, responsive, figure.layout.autosize + ' ' + figure.layout.width]; + } + """, + [ + Output("graph", "figure"), + Output("graph", "responsive"), + Output("output", "children"), + ], + [Input("button", "n_clicks")], + [State("graph", "figure")], + ) + + dash_dcc.start_server(app) + button = dash_dcc.wait_for_element("#button") + time.sleep(1) + + # Draw a shape + dash_dcc.release() + + # Initial values of autosize and width + dash_dcc.wait_for_text_to_equal("#output", "null 300") + button.click() + # Values for responsive is true + dash_dcc.wait_for_text_to_equal("#output", "true undefined") + button.click() + # Values for responsive is false + dash_dcc.wait_for_text_to_equal("#output", "false 300") + button.click() + # Values for responsive is null + dash_dcc.wait_for_text_to_equal("#output", "null 300") From 9075fc37845385d5c8e32b9b2a0d38be87a7d14c Mon Sep 17 00:00:00 2001 From: Almar Klein Date: Fri, 18 Dec 2020 22:05:15 +0100 Subject: [PATCH 10/15] get rid of sleep --- tests/dash_core_components_page.py | 9 +++++++++ tests/integration/graph/test_graph_varia.py | 12 ++++-------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/tests/dash_core_components_page.py b/tests/dash_core_components_page.py index 909de7a3f..3e33a64f4 100644 --- a/tests/dash_core_components_page.py +++ b/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() diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index ffbd2f946..7ab707cdb 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -690,7 +690,7 @@ def test_shapes_not_lost(dash_dcc): dash_dcc.start_server(app) button = dash_dcc.wait_for_element("#button") - time.sleep(1) + 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) @@ -754,19 +754,15 @@ def test_originals_maintained_for_responsive_override(dash_dcc): dash_dcc.start_server(app) button = dash_dcc.wait_for_element("#button") - time.sleep(1) - - # Draw a shape - dash_dcc.release() # Initial values of autosize and width dash_dcc.wait_for_text_to_equal("#output", "null 300") - button.click() # Values for responsive is true - dash_dcc.wait_for_text_to_equal("#output", "true undefined") button.click() + dash_dcc.wait_for_text_to_equal("#output", "true undefined") # Values for responsive is false - dash_dcc.wait_for_text_to_equal("#output", "false 300") button.click() + dash_dcc.wait_for_text_to_equal("#output", "false 300") # Values for responsive is null + button.click() dash_dcc.wait_for_text_to_equal("#output", "null 300") From 27d93b3d75edf9ced329b0bdc0f3f738d0bba60e Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 14 Jan 2021 21:42:38 -0500 Subject: [PATCH 11/15] standardize state usage in Graph --- src/fragments/Graph.react.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fragments/Graph.react.js b/src/fragments/Graph.react.js index 8cb1e2dec..d4353b8d5 100644 --- a/src/fragments/Graph.react.js +++ b/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) { @@ -227,8 +229,7 @@ class PlotlyGraph extends Component { return layout; } const override = this.getLayoutOverride(responsive); - const prev_override = (this.state && this.state.override) || {}; - const prev_originals = (this.state && this.state.originals) || {}; + 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) { @@ -238,7 +239,7 @@ class PlotlyGraph extends Component { originals[key] = prev_originals[key]; } } - this.setState({override: override, originals: originals}); + 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]) { From 582d28118b00e5f67c9a8e116a22d86a29379256 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Thu, 14 Jan 2021 21:54:09 -0500 Subject: [PATCH 12/15] number the tests in graph_varia --- tests/integration/graph/test_graph_varia.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index 7ab707cdb..02329b7e6 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/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, @@ -660,7 +660,7 @@ def load_chart(n_clicks): assert findAsyncPlotlyJs(scripts) is None -def test_shapes_not_lost(dash_dcc): +def test_grva008_shapes_not_lost(dash_dcc): # See issue #879 and pr #905 app = dash.Dash(__name__) @@ -713,7 +713,7 @@ def test_shapes_not_lost(dash_dcc): dash_dcc.wait_for_text_to_equal("#output", "2") -def test_originals_maintained_for_responsive_override(dash_dcc): +def test_grva009_originals_maintained_for_responsive_override(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. From 271ce56eaa8d93c5fe57a5377f97ae2072773792 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Fri, 15 Jan 2021 03:10:58 -0500 Subject: [PATCH 13/15] tweak new tests - usage and explicit graph height --- tests/integration/graph/test_graph_varia.py | 37 ++++++++++++--------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index 02329b7e6..1dec11a1b 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -665,7 +665,7 @@ def test_grva008_shapes_not_lost(dash_dcc): app = dash.Dash(__name__) fig = {"data": [], "layout": {"dragmode": "drawrect"}} - graph = dcc.Graph(id="graph", figure=fig) + graph = dcc.Graph(id="graph", figure=fig, style={"height": "400px"}) app.layout = html.Div( [ @@ -677,15 +677,17 @@ def test_grva008_shapes_not_lost(dash_dcc): ) app.clientside_callback( - """function clone_figure(_, figure) { + """ + function clone_figure(_, figure) { let new_figure = {...figure}; let shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """, - [Output("graph", "figure"), Output("output", "children")], - [Input("button", "n_clicks")], - [State("graph", "figure")], + Output("graph", "figure"), + Output("output", "children"), + Input("button", "n_clicks"), + State("graph", "figure"), ) dash_dcc.start_server(app) @@ -721,7 +723,7 @@ def test_grva009_originals_maintained_for_responsive_override(dash_dcc): app = dash.Dash(__name__) fig = {"data": [], "layout": {"autosize": None, "width": 300, "height": 300}} - graph = dcc.Graph(id="graph", figure=fig) + graph = dcc.Graph(id="graph", figure=fig, style={"height": "400px"}) app.layout = html.Div( [ @@ -735,21 +737,24 @@ def test_grva009_originals_maintained_for_responsive_override(dash_dcc): # Each time that the button is pressed we change responsive mode. # It goes from null (default), to true, false, and back to null. app.clientside_callback( - """function clone_figure(_, figure) { + """ + function clone_figure(_, figure) { let new_figure = {...figure}; let index = window.internal_state_905 || 0; window.internal_state_905 = index + 1; - let responsive = [true, false, null, null][index]; - return [new_figure, responsive, figure.layout.autosize + ' ' + figure.layout.width]; + let responsive = [true, false, null, null][index]; + return [ + new_figure, + responsive, + figure.layout.autosize + ' ' + figure.layout.width + ]; } """, - [ - Output("graph", "figure"), - Output("graph", "responsive"), - Output("output", "children"), - ], - [Input("button", "n_clicks")], - [State("graph", "figure")], + Output("graph", "figure"), + Output("graph", "responsive"), + Output("output", "children"), + Input("button", "n_clicks"), + State("graph", "figure"), ) dash_dcc.start_server(app) From 6c8490c1d04a8f08d1f991f94f215360dc395db9 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Fri, 15 Jan 2021 04:23:12 -0500 Subject: [PATCH 14/15] more extensive test of graph responsive changes --- tests/integration/graph/test_graph_varia.py | 118 ++++++++++++++------ 1 file changed, 85 insertions(+), 33 deletions(-) diff --git a/tests/integration/graph/test_graph_varia.py b/tests/integration/graph/test_graph_varia.py index 1dec11a1b..a7d6bb647 100644 --- a/tests/integration/graph/test_graph_varia.py +++ b/tests/integration/graph/test_graph_varia.py @@ -679,8 +679,8 @@ def test_grva008_shapes_not_lost(dash_dcc): app.clientside_callback( """ function clone_figure(_, figure) { - let new_figure = {...figure}; - let shapes = new_figure.layout.shapes || []; + const new_figure = {...figure}; + const shapes = new_figure.layout.shapes || []; return [new_figure, shapes.length]; } """, @@ -715,59 +715,111 @@ def test_grva008_shapes_not_lost(dash_dcc): dash_dcc.wait_for_text_to_equal("#output", "2") -def test_grva009_originals_maintained_for_responsive_override(dash_dcc): +@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__) - fig = {"data": [], "layout": {"autosize": None, "width": 300, "height": 300}} - graph = dcc.Graph(id="graph", figure=fig, style={"height": "400px"}) + 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="button", children="Clone figure"), + html.Button(id="edit_figure", children="Edit figure"), + html.Button(id="edit_responsive", children="Edit responsive"), html.Div(id="output", children=""), ] ) - # Each time that the button is pressed we change responsive mode. - # It goes from null (default), to true, false, and back to null. - app.clientside_callback( + 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}; """ - function clone_figure(_, figure) { - let new_figure = {...figure}; - let index = window.internal_state_905 || 0; - window.internal_state_905 = index + 1; - let responsive = [true, false, null, null][index]; - return [ - new_figure, - responsive, - figure.layout.autosize + ' ' + figure.layout.width - ]; + 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("button", "n_clicks"), + Input("edit_figure", "n_clicks"), + Input("edit_responsive", "n_clicks"), State("graph", "figure"), ) dash_dcc.start_server(app) - button = dash_dcc.wait_for_element("#button") + 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]; + """ + ) - # Initial values of autosize and width - dash_dcc.wait_for_text_to_equal("#output", "null 300") - # Values for responsive is true - button.click() - dash_dcc.wait_for_text_to_equal("#output", "true undefined") - # Values for responsive is false - button.click() - dash_dcc.wait_for_text_to_equal("#output", "false 300") - # Values for responsive is null - button.click() - dash_dcc.wait_for_text_to_equal("#output", "null 300") + 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 From 3c3f61d736fce8db5fff3135dcf4472d6825dc3e Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Fri, 15 Jan 2021 08:58:53 -0500 Subject: [PATCH 15/15] changelog for don't clone figure layout --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e31289e30..998577aae 100644 --- a/CHANGELOG.md +++ b/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