From f523cb7678f1bf81e35eedeac5faf20815fb05de Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 14 Sep 2020 21:50:50 -0400 Subject: [PATCH 1/5] persist callback graph layout when callbacks fire --- dash-renderer/src/reducers/profile.js | 6 +++-- .../integration/devtools/test_devtools_ui.py | 27 ++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/dash-renderer/src/reducers/profile.js b/dash-renderer/src/reducers/profile.js index b71a61106d..5b571988ba 100644 --- a/dash-renderer/src/reducers/profile.js +++ b/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,8 @@ const profile = (state = defaultState, action) => { const newState = { updated: [id], resources: state.resources, - callbacks: state.callbacks + callbacks: state.callbacks, + graphLayout: state.graphLayout }; newState.callbacks[id] = diff --git a/tests/integration/devtools/test_devtools_ui.py b/tests/integration/devtools/test_devtools_ui.py index d32facb033..fa5f891eaa 100644 --- a/tests/integration/devtools/test_devtools_ui.py +++ b/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,28 @@ 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() + dash_duo.find_element(".dash-debug-menu__button--callbacks").click() + sleep(2) + + # fire callbacks so the callback graph redraws again + dash_duo.find_element("#add").click() + dash_duo.wait_for_text_to_equal("#totals", "0 of 1 items completed - 0%") + sleep(1) + # 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; + """ + ) From 3a6222c0ee4d8cc01ccc5f163c80b1f11f3ce0bb Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 14 Sep 2020 22:03:38 -0400 Subject: [PATCH 2/5] changelog for callback graph layout fix --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3be5312c0d..d04464edfb 100644 --- a/CHANGELOG.md +++ b/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`) . From 8b3abd206db01d205ca19d7d555ac6eedc9a1806 Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Mon, 14 Sep 2020 22:06:50 -0400 Subject: [PATCH 3/5] fix new callback graph layout test for small CI screen --- tests/integration/devtools/test_devtools_ui.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/integration/devtools/test_devtools_ui.py b/tests/integration/devtools/test_devtools_ui.py index fa5f891eaa..78bdfc67b0 100644 --- a/tests/integration/devtools/test_devtools_ui.py +++ b/tests/integration/devtools/test_devtools_ui.py @@ -110,13 +110,12 @@ def test_dvui003_callback_graph(dash_duo): # hide and redraw the callback graph so we get the new position dash_duo.find_element(".dash-debug-menu__button--callbacks").click() - dash_duo.find_element(".dash-debug-menu__button--callbacks").click() - sleep(2) - # fire callbacks so the callback graph redraws again + # 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(1) + sleep(2) # the manually moved node is still in its new position assert pos == dash_duo.driver.execute_script( """ From 906884bea0a473ae05da4d1655c7fd505f39246a Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 15 Sep 2020 09:17:47 -0400 Subject: [PATCH 4/5] comments on the graphLayout hack --- .../error/CallbackGraph/CallbackGraphContainer.react.js | 7 +++++++ dash-renderer/src/reducers/profile.js | 3 +++ 2 files changed, 10 insertions(+) diff --git a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js b/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js index 3e298154ce..02c80f6711 100644 --- a/dash-renderer/src/components/error/CallbackGraph/CallbackGraphContainer.react.js +++ b/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, diff --git a/dash-renderer/src/reducers/profile.js b/dash-renderer/src/reducers/profile.js index 5b571988ba..28ba2d0ec1 100644 --- a/dash-renderer/src/reducers/profile.js +++ b/dash-renderer/src/reducers/profile.js @@ -38,6 +38,9 @@ const profile = (state = defaultState, action) => { updated: [id], resources: state.resources, 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 }; From fe60ea516dc03954ca78ad750a77608874e768dd Mon Sep 17 00:00:00 2001 From: alexcjohnson Date: Tue, 15 Sep 2020 09:56:23 -0400 Subject: [PATCH 5/5] noise