Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

persist callback graph layout when callbacks fire #1408

Merged
merged 5 commits into from
Sep 15, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`) .
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions dash-renderer/src/reducers/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ const defaultProfile = {
const defaultState = {
updated: [],
resources: {},
callbacks: {}
callbacks: {},
graphLayout: null
};

const profile = (state = defaultState, action) => {
Expand All @@ -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] =
Expand Down
26 changes: 25 additions & 1 deletion tests/integration/devtools/test_devtools_ui.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Worked fine locally but failed on CI before this commit, because the "Add" button was hidden by the callback graph. The bug still applies if the callbacks are fired while the graph is hidden, so that's what the test now does... but this brings up that it would be nice at some point to be able to shrink and/or move the callback graph so you can still interact with whichever parts of the app you want to while it's open.

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;
"""
)