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.

Sign up for GitHub

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

Configure document-title during updates #1315

Merged
merged 17 commits into from
Jul 9, 2020
+58 −2
Merged

Configure document-title during updates #1315

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
6525ca2
Add update_title parameter
stlehmann Jun 13, 2020
3061cba
Save update_title to config
stlehmann Jun 28, 2020
eb427b0
Pass update_title to frontend
stlehmann Jun 28, 2020
28630d9
Use update_title in DocumentTitle component
stlehmann Jun 28, 2020
2f7cdc4
Do not change title if update_title is null
stlehmann Jun 28, 2020
899b20e
Merge branch 'dev' into update-title
stlehmann Jun 28, 2020
f5f97cc
Add changelog entry
stlehmann Jun 28, 2020
1db09c9
Put changelog entry in Unreleased section
stlehmann Jun 29, 2020
8046178
Simplify update_title state assignment
stlehmann Jun 29, 2020
745ba77
Fix config PropType
stlehmann Jun 29, 2020
57f9b37
Update CHANGELOG.md
stlehmann Jun 29, 2020
740b913
Make config PropType more explicit
stlehmann Jun 29, 2020
71ee5ec
Add tests for update_title in test_loading_states.py
stlehmann Jun 30, 2020
bb2a6f7
Fix test function names
stlehmann Jun 30, 2020
571d6ce
Elaborate test sequence
stlehmann Jul 1, 2020
58b1472
Move test out of callback
stlehmann Jul 2, 2020
087774d
Merge branch 'dev' into update-title
alexcjohnson Jul 8, 2020
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
View file
Open in desktop
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
### Added
- [#1315](https://github.com/plotly/dash/pull/1315) Add `update_title` parameter to set or disable the "Updating...." document title during updates. Closes [#856](https://github.com/plotly/dash/issues/856) and [#732](https://github.com/plotly/dash/issues/732)

## [1.13.4] - 2020-06-25
### 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.
Expand Down
8 changes: 6 additions & 2 deletions dash-renderer/src/components/core/DocumentTitle.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@ import PropTypes from 'prop-types';
class DocumentTitle extends Component {
constructor(props) {
super(props);
const {update_title} = props.config;
this.state = {
initialTitle: document.title,
update_title,
};
}

UNSAFE_componentWillReceiveProps(props) {
if (props.isLoading) {
document.title = 'Updating...';
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Marc-Andre-Rivet as we were discussing the other day - the renderer no longer holds this default value at all. That means when the R and Julia implementations update to include this functionality in their renderer, they will need to add this option to the back end at the same time or the default behavior will change to no title changes ever.

if (this.state.update_title && props.isLoading) {
document.title = this.state.update_title;
} else {
document.title = this.state.initialTitle;
}
Expand All @@ -29,8 +31,10 @@ class DocumentTitle extends Component {

DocumentTitle.propTypes = {
isLoading: PropTypes.bool.isRequired,
config: PropTypes.shape({update_title: PropTypes.string}),
};

export default connect(state => ({
isLoading: state.isLoading,
config: state.config,
}))(DocumentTitle);
3 changes: 3 additions & 0 deletions dash/dash.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ def __init__(
prevent_initial_callbacks=False,
show_undo_redo=False,
plugins=None,
update_title="Updating...",
**obsolete
):
_validate.check_obsolete(obsolete)
Expand Down Expand Up @@ -299,6 +300,7 @@ def __init__(
),
prevent_initial_callbacks=prevent_initial_callbacks,
show_undo_redo=show_undo_redo,
update_title=update_title,
)
self.config.set_read_only(
[
Expand Down Expand Up @@ -506,6 +508,7 @@ def _config(self):
"props_check": self._dev_tools.props_check,
"show_undo_redo": self.config.show_undo_redo,
"suppress_callback_exceptions": self.config.suppress_callback_exceptions,
"update_title": self.config.update_title,
}
if self._dev_tools.hot_reload:
config["hot_reload"] = {
Expand Down
108 changes: 108 additions & 0 deletions tests/integration/renderer/test_loading_states.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,3 +167,111 @@ def find_text(spec):

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


def test_update_title_default(dash_duo):
stlehmann marked this conversation as resolved.
Show resolved Hide resolved
lock = Lock()

app = dash.Dash(__name__)

app.layout = html.Div(
children=[
html.H3("Press button see document title updating"),
html.Div(id="output"),
html.Button("Update", id="button", n_clicks=0),
]
)

@app.callback(
Output("output", "children"),
[Input("button", "n_clicks")]
)
def update(n):
with lock:
return n

with lock:
dash_duo.start_server(app)
dash_duo.find_element("#button").click()
assert dash_duo.driver.title == "Updating..."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very nice tests! One thing though: your callback will execute on page load, right? So holding the lock while starting the server means we're testing the title during the initialization sequence. That's a good thing to test, but I think in at least one of these tests (and maybe all of them) we should test:

  • during startup, we see the correct updating title
  • when the callback is done we see the regular title
  • when the callback is triggered interactively we see the updating title again

Also rather than a simple assert, for robustness I think it'd be better to do a wait.until, in case things don't happen quite in the expected order:

from dash.testing.wait import until

until(lambda: dash_duo.driver.title == "Updating...", timeout=1)

And one last thing, if you're feeling adventurous: These test cases are nearly identical so they could be parametrized, something like:

import pytest

@pytest.mark.parametrize(
    "kwargs,page_updating_title",
    [
        ({}, "Updating..."),
        ({"updating_title": None}, "Dash"),
        ({"updating_title": ""}, "Dash"),
        ({"updating_title": "Hello World"}, "Hello World")
    ]
)
def test_rdls003_update_title_default(dash_duo, updating_title, page_updating_title)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to implement your suggestions. I omitted an explicit test for the correct update title on startup because the callback should be called on startup afaik. I hope this is OK. The parametrize decorator is a great thing. Didn't know that until now, thanks :) Hope I did it all right with the locks. I'm not that sure about it.



def test_update_title_None(dash_duo):
lock = Lock()

app = dash.Dash(__name__, update_title=None)

app.layout = html.Div(
children=[
html.H3("Press button see document title updating"),
html.Div(id="output"),
html.Button("Update", id="button", n_clicks=0),
]
)

@app.callback(
Output("output", "children"),
[Input("button", "n_clicks")]
)
def update(n):
with lock:
return n

with lock:
dash_duo.start_server(app)
dash_duo.find_element("#button").click()
assert dash_duo.driver.title == "Dash"


def test_update_title_empty(dash_duo):
lock = Lock()

app = dash.Dash(__name__, update_title="")

app.layout = html.Div(
children=[
html.H3("Press button see document title updating"),
html.Div(id="output"),
html.Button("Update", id="button", n_clicks=0),
]
)

@app.callback(
Output("output", "children"),
[Input("button", "n_clicks")]
)
def update(n):
with lock:
return n

with lock:
dash_duo.start_server(app)
dash_duo.find_element("#button").click()
assert dash_duo.driver.title == "Dash"


def test_update_title_custom(dash_duo):
lock = Lock()

app = dash.Dash(__name__, update_title="Hello World")

app.layout = html.Div(
children=[
html.H3("Press button see document title updating"),
html.Div(id="output"),
html.Button("Update", id="button", n_clicks=0),
]
)

@app.callback(
Output("output", "children"),
[Input("button", "n_clicks")]
)
def update(n):
with lock:
return n

with lock:
dash_duo.start_server(app)
dash_duo.find_element("#button").click()
assert dash_duo.driver.title == "Hello World"