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

fix multiple concurrent loading states #1310

Merged
merged 4 commits into from
Jun 25, 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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
All notable changes to `dash` will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## UNRELEASED
### Fixed
- [#1310](https://github.com/plotly/dash/pull/1310) Fix a regression since 1.13.0 preventing more than one loading state from being shown at a time.

## [1.13.3] - 2020-06-19

## [1.13.2] - 2020-06-18
Expand Down
19 changes: 7 additions & 12 deletions dash-renderer/src/observers/loadingMap.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import {
equals,
flatten,
forEach,
isEmpty,
map,
reduce
Expand Down Expand Up @@ -43,26 +42,22 @@ const observer: IStoreObserverDefinition<IStoreState> = {
const nextMap: any = isEmpty(loadingPaths) ?
null :
reduce(
(res, path) => {
(res, {id, property, path}) => {
let target = res;
const idprop = {
id: path.id,
property: path.property
};
const idprop = {id, property};

// Assign all affected props for this path and nested paths
target.__dashprivate__idprops__ = target.__dashprivate__idprops__ || [];
target.__dashprivate__idprops__.push(idprop);

forEach(p => {
target = (target[p] =
target[p] ??
p === 'children' ? [] : {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Turns out ?? has higher precedence than ? in JS 🙈 The only necessary part of this fix is the extra parentheses around this line.

Added the typeof path[i + 1] clause so that the [] vs {} distinction is correct - though it doesn't seem to actually cause problems when the container has the wrong type 🤷

)
path.forEach((p, i) => {
target = (target[p] = target[p] ??
(p === 'children' && typeof path[i + 1] === 'number' ? [] : {})
);

target.__dashprivate__idprops__ = target.__dashprivate__idprops__ || [];
target.__dashprivate__idprops__.push(idprop);
}, path.path);
});

// Assign one affected prop for this path
target.__dashprivate__idprop__ = target.__dashprivate__idprop__ || idprop;
Expand Down
169 changes: 169 additions & 0 deletions tests/integration/renderer/test_loading_states.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
from multiprocessing import Lock

import dash
from dash.dependencies import Input, Output

import dash_core_components as dcc
import dash_html_components as html


def test_rdls001_multi_loading_components(dash_duo):
lock = Lock()

app = dash.Dash(__name__)

app.layout = html.Div(
children=[
html.H3("Edit text input to see loading state"),
dcc.Input(id="input-3", value="Input triggers the loading states"),
dcc.Loading(
className="loading-1",
children=[html.Div(id="loading-output-1")],
type="default",
),
html.Div(
[
dcc.Loading(
className="loading-2",
children=[html.Div([html.Div(id="loading-output-2")])],
type="circle",
),
dcc.Loading(
className="loading-3",
children=dcc.Graph(id="graph"),
type="cube",
),
]
),
],
)

@app.callback(
[
Output("graph", "figure"),
Output("loading-output-1", "children"),
Output("loading-output-2", "children"),
],
[Input("input-3", "value")],
)
def input_triggers_nested(value):
with lock:
return dict(data=[dict(y=[1, 4, 2, 3])]), value, value

def wait_for_all_spinners():
dash_duo.find_element(".loading-1 .dash-spinner.dash-default-spinner")
dash_duo.find_element(".loading-2 .dash-spinner.dash-sk-circle")
dash_duo.find_element(".loading-3 .dash-spinner.dash-cube-container")

def wait_for_no_spinners():
dash_duo.wait_for_no_elements(".dash-spinner")

with lock:
dash_duo.start_server(app)
wait_for_all_spinners()

wait_for_no_spinners()

with lock:
dash_duo.find_element("#input-3").send_keys("X")
wait_for_all_spinners()

wait_for_no_spinners()


def test_rdls002_chained_loading_states(dash_duo):
lock1, lock2, lock34 = Lock(), Lock(), Lock()
app = dash.Dash(__name__)

def loading_wrapped_div(_id, color):
return html.Div(
dcc.Loading(
html.Div(
id=_id,
style={"width": 200, "height": 200, "backgroundColor": color},
),
className=_id,
),
style={"display": "inline-block"},
)

app.layout = html.Div(
[
html.Button(id="button", children="Start", n_clicks=0),
loading_wrapped_div("output-1", "hotpink"),
loading_wrapped_div("output-2", "rebeccapurple"),
loading_wrapped_div("output-3", "green"),
loading_wrapped_div("output-4", "#FF851B"),
]
)

@app.callback(Output("output-1", "children"), [Input("button", "n_clicks")])
def update_output_1(n_clicks):
with lock1:
return "Output 1: {}".format(n_clicks)

@app.callback(Output("output-2", "children"), [Input("output-1", "children")])
def update_output_2(children):
with lock2:
return "Output 2: {}".format(children)

@app.callback(
[Output("output-3", "children"), Output("output-4", "children")],
[Input("output-2", "children")],
)
def update_output_34(children):
with lock34:
return "Output 3: {}".format(children), "Output 4: {}".format(children)

dash_duo.start_server(app)

def find_spinners(*nums):
if not nums:
dash_duo.wait_for_no_elements(".dash-spinner")
return

for n in nums:
dash_duo.find_element(".output-{} .dash-spinner".format(n))

assert len(dash_duo.find_elements(".dash-spinner")) == len(nums)

def find_text(spec):
templates = [
"Output 1: {}",
"Output 2: Output 1: {}",
"Output 3: Output 2: Output 1: {}",
"Output 4: Output 2: Output 1: {}",
]
for n, v in spec.items():
dash_duo.wait_for_text_to_equal(
"#output-{}".format(n), templates[n - 1].format(v)
)

find_text({1: 0, 2: 0, 3: 0, 4: 0})
find_spinners()

btn = dash_duo.find_element("#button")
# Can't use lock context managers here, because we want to acquire the
# second lock before releasing the first
lock1.acquire()
btn.click()

find_spinners(1)
find_text({2: 0, 3: 0, 4: 0})

lock2.acquire()
lock1.release()

find_spinners(2)
find_text({1: 1, 3: 0, 4: 0})

lock34.acquire()
lock2.release()

find_spinners(3, 4)
find_text({1: 1, 2: 1})

lock34.release()

find_spinners()
find_text({1: 1, 2: 1, 3: 1, 4: 1})