Merge pull request #1408 from plotly/1402-cb-graph-layout-fix

persist callback graph layout when callbacks fire

Author: alexcjohnson

CHANGELOG.md

@@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 ### Changed
 - [#1376](https://github.com/plotly/dash/pull/1376) Extends the `getTransform` logic in the renderer to handle `persistenceTransforms` for both nested and non-nested persisted props. This was used to to fix [dcc#700](https://github.com/plotly/dash-core-components/issues/700) in conjunction with [dcc#854](https://github.com/plotly/dash-core-components/pull/854) by using persistenceTransforms to strip the time part of the datetime so that datepickers can persist when defined in callbacks.
 
+### Fixed
+- [#1408](https://github.com/plotly/dash/pull/1408) Fixes a bug where the callback graph layout would reset whenever a callback fired, losing user-initiated layout changes ([#1402](https://github.com/plotly/dash/issues/1402)) or creating a new force layout ([#1401](https://github.com/plotly/dash/issues/1401))
+
 ## [1.16.0] - 2020-09-03
 ### Added
 - [#1371](https://github.com/plotly/dash/pull/1371) You can now get [CSP `script-src` hashes](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/script-src) of all added inline scripts by calling `app.csp_hashes()` (both Dash internal inline scripts, and those added with `app.clientside_callback`) .

dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js

@@ -195,6 +195,13 @@ function CallbackGraph() {
         cy.nodes().each(n => {
             positions[n.id()] = n.position();
         });
+
+        // Hack! We're mutating the redux store directly here, rather than
+        // dispatching an action, because we don't want this to trigger a
+        // rerender, we just want the layout to persist when the callback graph
+        // is rerendered - either because there's new profile information to
+        // display or because the graph was closed and reopened. The latter is
+        // the reason we're not using component state to store the layout.
         profile.graphLayout = {
             name: 'preset',
             fit: false,

dash-renderer/src/reducers/profile.js

@@ -21,7 +21,8 @@ const defaultProfile = {
 const defaultState = {
     updated: [],
     resources: {},
-    callbacks: {}
+    callbacks: {},
+    graphLayout: null
 };
 
 const profile = (state = defaultState, action) => {
@@ -36,7 +37,11 @@ const profile = (state = defaultState, action) => {
         const newState = {
             updated: [id],
             resources: state.resources,
-            callbacks: state.callbacks
+            callbacks: state.callbacks,
+            // graphLayout is never passed in via actions, because we don't
+            // want it to trigger a rerender of the callback graph.
+            // See CallbackGraphContainer.react
+            graphLayout: state.graphLayout
         };
 
         newState.callbacks[id] =

tests/integration/devtools/test_devtools_ui.py

@@ -89,7 +89,7 @@ def test_dvui003_callback_graph(dash_duo):
             cbProfiles[k].network.time = 33;
             cbProfiles[k].total = 77;
         });
-    """
+        """
     )
 
     dash_duo.find_element(".dash-debug-menu").click()
@@ -99,3 +99,27 @@ def test_dvui003_callback_graph(dash_duo):
     dash_duo.find_element('canvas[data-id="layer2-node"]')
 
     dash_duo.percy_snapshot("devtools - callback graph", convert_canvases=True)
+
+    pos = dash_duo.driver.execute_script(
+        """
+        const pos = store.getState().profile.graphLayout.positions['new-item.value'];
+        pos.y -= 100;
+        return pos.y;
+        """
+    )
+
+    # hide and redraw the callback graph so we get the new position
+    dash_duo.find_element(".dash-debug-menu__button--callbacks").click()
+
+    # fire callbacks so the profile state is regenerated
+    dash_duo.find_element("#add").click()
+    dash_duo.find_element(".dash-debug-menu__button--callbacks").click()
+    dash_duo.wait_for_text_to_equal("#totals", "0 of 1 items completed - 0%")
+    sleep(2)
+    # the manually moved node is still in its new position
+    assert pos == dash_duo.driver.execute_script(
+        """
+        const pos = store.getState().profile.graphLayout.positions['new-item.value'];
+        return pos.y;
+        """
+    )