From 4def8ed659c94fffe71a5d55f3445849bceb5c6c Mon Sep 17 00:00:00 2001 From: Antony Milne Date: Tue, 19 Dec 2023 00:36:00 +0000 Subject: [PATCH 1/9] Improve dashboard title --- ...02_antony.milne_improve_dashboard_title.md | 48 +++++ vizro-core/examples/default/app.py | 1 + vizro-core/src/vizro/models/_dashboard.py | 8 +- vizro-core/tests/unit/vizro/conftest.py | 8 - .../unit/vizro/models/_navigation/conftest.py | 9 + .../tests/unit/vizro/models/test_dashboard.py | 182 ++++++++---------- 6 files changed, 147 insertions(+), 109 deletions(-) create mode 100644 vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md diff --git a/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md b/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md @@ -0,0 +1,48 @@ + + + + + + + + + diff --git a/vizro-core/examples/default/app.py b/vizro-core/examples/default/app.py index 4957f1e00..1af7cefb5 100644 --- a/vizro-core/examples/default/app.py +++ b/vizro-core/examples/default/app.py @@ -545,6 +545,7 @@ def create_home_page(): dashboard = vm.Dashboard( + title="My dashboard", pages=[ create_home_page(), create_variable_analysis(), diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 6c2bc0c9c..23de7d239 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -67,9 +67,13 @@ def pre_build(self): # For now the homepage (path /) corresponds to self.pages[0]. # Note redirect_from=["/"] doesn't work and so the / route must be defined separately. for order, page in enumerate(self.pages): - path = page.path if order else "/" dash.register_page( - module=page.id, name=page.title, path=path, order=order, layout=partial(self._make_page_layout, page) + module=page.id, + name=page.title, + title=f"{self.title}: {page.title}" if self.title else page.title, + path=page.path if order else "/", + order=order, + layout=partial(self._make_page_layout, page), ) dash.register_page(module=MODULE_PAGE_404, layout=self._make_page_404_layout()) diff --git a/vizro-core/tests/unit/vizro/conftest.py b/vizro-core/tests/unit/vizro/conftest.py index 4d6914e6b..240f540c6 100644 --- a/vizro-core/tests/unit/vizro/conftest.py +++ b/vizro-core/tests/unit/vizro/conftest.py @@ -60,11 +60,3 @@ def vizro_app(): app instantiation.pages. """ return Vizro() - - -@pytest.fixture() -def prebuilt_two_page_dashboard(vizro_app, page_1, page_2): - """Minimal two page dashboard, used mainly for testing navigation.""" - dashboard = vm.Dashboard(pages=[page_1, page_2]) - dashboard.pre_build() - return dashboard diff --git a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py index 200ada86e..108b9b634 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py @@ -2,6 +2,8 @@ import pytest +import vizro.models as vm + @pytest.fixture() def pages_as_list(): @@ -11,3 +13,10 @@ def pages_as_list(): @pytest.fixture def pages_as_dict(): return {"Group": ["Page 1", "Page 2"]} + + +@pytest.fixture() +def prebuilt_two_page_dashboard(vizro_app, page_1, page_2): + dashboard = vm.Dashboard(pages=[page_1, page_2]) + dashboard.pre_build() + return dashboard diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py index 39f499528..8339822b5 100644 --- a/vizro-core/tests/unit/vizro/models/test_dashboard.py +++ b/vizro-core/tests/unit/vizro/models/test_dashboard.py @@ -1,3 +1,4 @@ +import functools import json from collections import OrderedDict from functools import partial @@ -8,6 +9,8 @@ import pytest from dash import html +from asserts import assert_component_equal + try: from pydantic.v1 import ValidationError except ImportError: # pragma: no cov @@ -18,88 +21,6 @@ from vizro.actions._action_loop._action_loop import ActionLoop -@pytest.fixture() -def dashboard_container(): - return dbc.Container( - id="dashboard_container_outer", - children=[ - html.Div(vizro.__version__, id="vizro_version", hidden=True), - ActionLoop._create_app_callbacks(), - dash.page_container, - ], - className="vizro_dark", - fluid=True, - ) - - -@pytest.fixture() -def mock_page_registry(prebuilt_two_page_dashboard, page_1, page_2): - return OrderedDict( - { - "Page 1": { - "module": "Page 1", - "supplied_path": "/", - "path_template": None, - "path": "/", - "supplied_name": "Page 1", - "name": "Page 1", - "supplied_title": None, - "title": "Page 1", - "description": "", - "order": 0, - "supplied_order": 0, - "supplied_layout": partial(prebuilt_two_page_dashboard._make_page_layout, page_1), - "supplied_image": None, - "image": None, - "image_url": None, - "redirect_from": None, - "layout": partial(prebuilt_two_page_dashboard._make_page_layout, page_1), - "relative_path": "/", - }, - "Page 2": { - "module": "Page 2", - "supplied_path": "/page-2", - "path_template": None, - "path": "/page-2", - "supplied_name": "Page 2", - "name": "Page 2", - "supplied_title": None, - "title": "Page 2", - "description": "", - "order": 1, - "supplied_order": 1, - "supplied_layout": partial(prebuilt_two_page_dashboard._make_page_layout, page_2), - "supplied_image": None, - "image": None, - "image_url": None, - "redirect_from": None, - "layout": partial(prebuilt_two_page_dashboard._make_page_layout, page_2), - "relative_path": "/page-2", - }, - "not_found_404": { - "module": "not_found_404", - "supplied_path": None, - "path_template": None, - "path": "/not-found-404", - "supplied_name": None, - "name": "Not found 404", - "supplied_title": None, - "title": "Not found 404", - "description": "", - "order": None, - "supplied_order": None, - "supplied_layout": prebuilt_two_page_dashboard._make_page_404_layout(), - "supplied_image": None, - "image": None, - "image_url": None, - "redirect_from": None, - "layout": prebuilt_two_page_dashboard._make_page_404_layout(), - "relative_path": "/not-found-404", - }, - } - ) - - class TestDashboardInstantiation: """Tests model instantiation and the validators run at that time.""" @@ -149,27 +70,90 @@ def test_field_invalid_theme_input_type(self, page_1): class TestDashboardPreBuild: """Tests dashboard pre_build method.""" - def test_dashboard_page_registry(self, prebuilt_two_page_dashboard, mock_page_registry): - result = dash.page_registry - expected = mock_page_registry - # Str conversion required as comparison of OrderedDict values result in False otherwise - assert str(result.items()) == str(expected.items()) - - def test_create_layout_page_404(self, prebuilt_two_page_dashboard, mocker): - mocker.patch("vizro.models._dashboard.get_relative_path") - result = prebuilt_two_page_dashboard._make_page_404_layout() - result_image = result.children[0] - result_div = result.children[1] - - assert isinstance(result, html.Div) - assert isinstance(result_image, html.Img) - assert isinstance(result_div, html.Div) + def test_page_registry(self, vizro_app, page_1, page_2, mocker): + mock_register_page = mocker.patch("dash.register_page", autospec=True) + mock_make_page_404_layout = mocker.patch( + "vizro.models._dashboard.Dashboard._make_page_404_layout" + ) # Checking the actual dash components is done in test_make_page_404_layout. + vm.Dashboard(pages=[page_1, page_2]).pre_build() + + mock_register_page.assert_any_call( + module=page_1.id, + name="Page 1", + title="Page 1", + path="/", + order=0, + layout=mocker.ANY, # partial call is tricky to mock out so we ignore it. + ) + mock_register_page.assert_any_call( + module=page_2.id, + name="Page 2", + title="Page 2", + path="/page-2", + order=1, + layout=mocker.ANY, # partial call is tricky to mock out so we ignore it. + ) + mock_register_page.assert_any_call( + module="not_found_404", + layout=mock_make_page_404_layout(), + ) + assert mock_register_page.call_count == 3 + + def test_page_registry_with_title(self, vizro_app, page_1, mocker): + mock_register_page = mocker.patch("dash.register_page", autospec=True) + vm.Dashboard(pages=[page_1], title="My dashboard").pre_build() + + mock_register_page.assert_any_call( + module=page_1.id, + name="Page 1", + title="My dashboard: Page 1", + path="/", + order=0, + layout=mocker.ANY, # partial call is tricky to mock out so we ignore it. + ) + + def test_make_page_404_layout(self, vizro_app): + # vizro_app fixture is needed to avoid mocking out get_relative_path. + expected = html.Div( + [ + html.Img(src=f"/vizro/images/errors/error_404.svg"), + html.Div( + [ + html.Div( + [ + html.H3("This page could not be found.", className="heading-3-600"), + html.P("Make sure the URL you entered is correct."), + ], + className="error_text_container", + ), + dbc.Button("Take me home", href="/", className="button_primary"), + ], + className="error_content_container", + ), + ], + className="page_error_container", + ) + + assert_component_equal(vm.Dashboard._make_page_404_layout(), expected, {}) class TestDashboardBuild: """Tests dashboard build method.""" - def test_dashboard_build(self, dashboard_container, prebuilt_two_page_dashboard): - result = json.loads(json.dumps(prebuilt_two_page_dashboard.build(), cls=plotly.utils.PlotlyJSONEncoder)) + def test_dashboard_build(self, vizro_app, page_1, page_2): + dashboard = vm.Dashboard(pages=[page_1, page_2]) + dashboard.pre_build() + + dashboard_container = dbc.Container( + id="dashboard_container_outer", + children=[ + html.Div(vizro.__version__, id="vizro_version", hidden=True), + ActionLoop._create_app_callbacks(), + dash.page_container, + ], + className="vizro_dark", + fluid=True, + ) + result = json.loads(json.dumps(dashboard.build(), cls=plotly.utils.PlotlyJSONEncoder)) expected = json.loads(json.dumps(dashboard_container, cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected From cfc3cf7bdca29742d3a172d03aeca57af0472bbe Mon Sep 17 00:00:00 2001 From: Antony Milne Date: Tue, 19 Dec 2023 00:40:56 +0000 Subject: [PATCH 2/9] Lint and changelog --- ...0231218_204302_antony.milne_improve_dashboard_title.md | 7 +++---- vizro-core/tests/unit/vizro/models/test_dashboard.py | 8 ++------ 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md b/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md index f1f65e73c..402bb77fb 100644 --- a/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md +++ b/vizro-core/changelog.d/20231218_204302_antony.milne_improve_dashboard_title.md @@ -16,16 +16,15 @@ Uncomment the section that is right (remove the HTML comment wrapper). - A bullet item for the Removed category with a link to the relevant PR at the end of your entry, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1)) --> -