From 3359036444307cea6d85f9c5c0a947f35ad20f5e Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 21:09:35 +0100 Subject: [PATCH 01/11] Move all page-layout related code to dashboard --- vizro-core/src/vizro/models/_dashboard.py | 38 +++++++- vizro-core/src/vizro/models/_page.py | 106 ++++------------------ 2 files changed, 54 insertions(+), 90 deletions(-) diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 6f42700dd..6d0ae4033 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -1,10 +1,11 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING, List, Literal, Optional +from typing import TYPE_CHECKING, List, Literal, Optional, cast import dash import dash_bootstrap_components as dbc +import dash_daq as daq import plotly.io as pio from dash import ClientsideFunction, Input, Output, clientside_callback, html from pydantic import Field, validator @@ -94,7 +95,7 @@ def pre_build(self): # 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=page.build) + dash.register_page(module=page.id, name=page.title, path=path, order=order, layout=self._make_page_layout(page)) self._create_error_page_404() @_log_call @@ -114,6 +115,39 @@ def build(self): fluid=True, ) + def _make_page_layout(self, page): + # Left-side + dashboard_title = ( + html.Div(children=[html.H2(self.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer") + if self.title + else None + ) + nav_panel = cast(Navigation, self.navigation).build(active_page_id=page.id) + controls_content = [control.build() for control in page.controls] + control_panel = ( + html.Div(children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer") + if controls_content + else None + ) + + # Right-side + page_title = html.H2(children=page.title, id="page_title") + theme_switch = daq.BooleanSwitch(id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True) + header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer") + component_container = page.build() + + # Arrangement + left_side_elements = [dashboard_title, nav_panel, control_panel] + left_side = ( + html.Div(children=left_side_elements, className="left_side", id="left_side_outer") + if any(left_side_elements) + else None + ) + right_side_elements = [header, component_container] + right_side = html.Div(children=right_side_elements, className="right_side", id="right_side_outer") + + return html.Div([left_side, right_side], id="page_container") + @staticmethod def _update_theme(): clientside_callback( diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 6e57d89ad..0642e2454 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -1,18 +1,15 @@ from __future__ import annotations -from typing import List, Optional, cast +from typing import List, Optional -import dash_bootstrap_components as dbc -import dash_daq as daq from dash import Input, Output, Patch, callback, dcc, html from pydantic import Field, root_validator, validator import vizro._themes as themes from vizro._constants import ON_PAGE_LOAD_ACTION_PREFIX from vizro.actions import _on_page_load -from vizro.managers import model_manager from vizro.managers._model_manager import DuplicateIDError -from vizro.models import Action, Dashboard, Graph, Layout, Navigation, VizroBaseModel +from vizro.models import Action, Graph, Layout, VizroBaseModel from vizro.models._action._actions_chain import ActionsChain, Trigger from vizro.models._models_utils import _log_call, get_unique_grid_component_ids @@ -117,7 +114,6 @@ def pre_build(self): @_log_call def build(self): self._update_graph_theme() - controls_content = [control.build() for control in self.controls] components_content = [ html.Div( component.build(), @@ -130,7 +126,7 @@ def build(self): self.components, self.layout.component_grid_lines # type: ignore[union-attr] ) ] - return self._make_page_layout(controls_content, components_content) + return self._create_component_container(components_content) def _update_graph_theme(self): outputs = [ @@ -150,90 +146,24 @@ def update_graph_theme(theme_selector_on: bool): patched_figure["layout"]["template"] = themes.dark if theme_selector_on else themes.light return [patched_figure] * len(outputs) - @staticmethod - def _create_theme_switch(): - _, dashboard = next(model_manager._items_with_type(Dashboard)) - theme_switch = daq.BooleanSwitch( - id="theme_selector", on=True if dashboard.theme == "vizro_dark" else False, persistence=True - ) - return theme_switch - - @staticmethod - def _create_control_panel(controls_content): - control_panel = html.Div( - children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer" - ) - return control_panel if controls_content else None - - def _create_nav_panel(self): - _, dashboard = next(model_manager._items_with_type(Dashboard)) - return cast(Navigation, dashboard.navigation).build(active_page_id=self.id) - def _create_component_container(self, components_content): component_container = html.Div( - children=html.Div( - components_content, - style={ - "gridRowGap": self.layout.row_gap, # type: ignore[union-attr] - "gridColumnGap": self.layout.col_gap, # type: ignore[union-attr] - "gridTemplateColumns": f"repeat({len(self.layout.grid[0])}," # type: ignore[union-attr] - f"minmax({self.layout.col_min_width}, 1fr))", - "gridTemplateRows": f"repeat({len(self.layout.grid)}," # type: ignore[union-attr] - f"minmax({self.layout.row_min_height}, 1fr))", - }, - className="component_container_grid", - ), + children=[ + html.Div( + components_content, + style={ + "gridRowGap": self.layout.row_gap, # type: ignore[union-attr] + "gridColumnGap": self.layout.col_gap, # type: ignore[union-attr] + "gridTemplateColumns": f"repeat({len(self.layout.grid[0])}," # type: ignore[union-attr] + f"minmax({self.layout.col_min_width}, 1fr))", + "gridTemplateRows": f"repeat({len(self.layout.grid)}," # type: ignore[union-attr] + f"minmax({self.layout.row_min_height}, 1fr))", + }, + className="component_container_grid", + ), + dcc.Store(id=f"{ON_PAGE_LOAD_ACTION_PREFIX}_trigger_{self.id}"), + ], className="component_container", id="component_container_outer", ) return component_container - - @staticmethod - def _arrange_containers(page_title, theme_switch, nav_panel, control_panel, component_container): - """Defines div container arrangement on page. - - To change arrangement, one has to change the order in the header, left_side and/or right_side_elements. - """ - _, dashboard = next(model_manager._items_with_type(Dashboard)) - dashboard_title = ( - html.Div( - children=[html.H2(dashboard.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer" - ) - if dashboard.title - else None - ) - - header_elements = [page_title, theme_switch] - left_side_elements = [dashboard_title, nav_panel, control_panel] - header = html.Div(children=header_elements, className="header", id="header_outer") - left_side = ( - html.Div(children=left_side_elements, className="left_side", id="left_side_outer") - if any(left_side_elements) - else None - ) - right_side_elements = [header, component_container] - right_side = html.Div(children=right_side_elements, className="right_side", id="right_side_outer") - return left_side, right_side - - def _make_page_layout(self, controls_content, components_content): - # Create dashboard containers/elements - page_title = html.H2(children=self.title) - theme_switch = self._create_theme_switch() - nav_panel = self._create_nav_panel() - control_panel = self._create_control_panel(controls_content) - component_container = self._create_component_container(components_content) - - # Arrange dashboard containers - left_side, right_side = self._arrange_containers( - page_title=page_title, - theme_switch=theme_switch, - nav_panel=nav_panel, - control_panel=control_panel, - component_container=component_container, - ) - - return dbc.Container( - id=self.id, - children=[dcc.Store(id=f"{ON_PAGE_LOAD_ACTION_PREFIX}_trigger_{self.id}"), left_side, right_side], - className="page_container", - ) From 2dff7e771c13d257edc91bd14d3aefd6d653a4ba Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 21:19:15 +0100 Subject: [PATCH 02/11] Fix late binding --- vizro-core/src/vizro/models/_dashboard.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 6d0ae4033..a2437c9f6 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -1,6 +1,7 @@ from __future__ import annotations import logging +from functools import partial from typing import TYPE_CHECKING, List, Literal, Optional, cast import dash @@ -95,8 +96,10 @@ def pre_build(self): # 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=self._make_page_layout(page)) - self._create_error_page_404() + dash.register_page( + module=page.id, name=page.title, path=path, order=order, layout=partial(self._make_page_layout, page) + ) + dash.register_page(module=MODULE_PAGE_404, layout=create_layout_page_404()) @_log_call def build(self): @@ -132,21 +135,22 @@ def _make_page_layout(self, page): # Right-side page_title = html.H2(children=page.title, id="page_title") - theme_switch = daq.BooleanSwitch(id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True) + theme_switch = daq.BooleanSwitch( + id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True + ) header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer") component_container = page.build() # Arrangement left_side_elements = [dashboard_title, nav_panel, control_panel] + right_side_elements = [header, component_container] left_side = ( html.Div(children=left_side_elements, className="left_side", id="left_side_outer") if any(left_side_elements) else None ) - right_side_elements = [header, component_container] right_side = html.Div(children=right_side_elements, className="right_side", id="right_side_outer") - - return html.Div([left_side, right_side], id="page_container") + return html.Div([left_side, right_side], className="page_container", id="page_container_outer") @staticmethod def _update_theme(): @@ -155,7 +159,3 @@ def _update_theme(): Output("dashboard_container_outer", "className"), Input("theme_selector", "on"), ) - - @staticmethod - def _create_error_page_404(): - return dash.register_page(module=MODULE_PAGE_404, layout=create_layout_page_404()) From 97e8ca757396427a0daa3c48d8c60d3e6bae7dff Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 21:40:19 +0100 Subject: [PATCH 03/11] Add comments --- vizro-core/src/vizro/models/_dashboard.py | 31 ++++++++++++----------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index a2437c9f6..de3e08f53 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -119,37 +119,38 @@ def build(self): ) def _make_page_layout(self, page): - # Left-side + # Identical across pages dashboard_title = ( html.Div(children=[html.H2(self.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer") if self.title - else None + else html.Div(className="hidden") ) - nav_panel = cast(Navigation, self.navigation).build(active_page_id=page.id) + theme_switch = daq.BooleanSwitch( + id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True + ) + + # Shared across pages but slightly differ in content + page_title = html.H2(children=page.title, id="page_title") + navigation = cast(Navigation, self.navigation).build(active_page_id=page.id) + + # Different across pages controls_content = [control.build() for control in page.controls] control_panel = ( html.Div(children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer") if controls_content - else None + else html.Div(className="hidden") ) - - # Right-side - page_title = html.H2(children=page.title, id="page_title") - theme_switch = daq.BooleanSwitch( - id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True - ) - header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer") component_container = page.build() # Arrangement - left_side_elements = [dashboard_title, nav_panel, control_panel] - right_side_elements = [header, component_container] + header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer") + left_side_elements = [dashboard_title, navigation, control_panel] left_side = ( html.Div(children=left_side_elements, className="left_side", id="left_side_outer") if any(left_side_elements) - else None + else html.Div(className="hidden") ) - right_side = html.Div(children=right_side_elements, className="right_side", id="right_side_outer") + right_side = html.Div(children=[header, component_container], className="right_side", id="right_side_outer") return html.Div([left_side, right_side], className="page_container", id="page_container_outer") @staticmethod From c6cbb88619feed1293e24a3212f49b6964b52384 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 22:03:09 +0100 Subject: [PATCH 04/11] Move controls container back to page and unpack in dashboard --- vizro-core/src/vizro/models/_dashboard.py | 14 +++++--------- vizro-core/src/vizro/models/_page.py | 9 ++++++++- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index de3e08f53..839d3d3bb 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -123,7 +123,7 @@ def _make_page_layout(self, page): dashboard_title = ( html.Div(children=[html.H2(self.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer") if self.title - else html.Div(className="hidden") + else html.Div(className="hidden", id="dashboard_title_outer") ) theme_switch = daq.BooleanSwitch( id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True @@ -134,13 +134,9 @@ def _make_page_layout(self, page): navigation = cast(Navigation, self.navigation).build(active_page_id=page.id) # Different across pages - controls_content = [control.build() for control in page.controls] - control_panel = ( - html.Div(children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer") - if controls_content - else html.Div(className="hidden") - ) - component_container = page.build() + page_content = page.build() + control_panel = page_content["control_panel_outer"] + component_container = page_content["component_container_outer"] # Arrangement header = html.Div(children=[page_title, theme_switch], className="header", id="header_outer") @@ -148,7 +144,7 @@ def _make_page_layout(self, page): left_side = ( html.Div(children=left_side_elements, className="left_side", id="left_side_outer") if any(left_side_elements) - else html.Div(className="hidden") + else html.Div(className="hidden", id="left_side_outer") ) right_side = html.Div(children=[header, component_container], className="right_side", id="right_side_outer") return html.Div([left_side, right_side], className="page_container", id="page_container_outer") diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 0642e2454..88ca01774 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -114,6 +114,12 @@ def pre_build(self): @_log_call def build(self): self._update_graph_theme() + controls_content = [control.build() for control in self.controls] + control_panel = ( + html.Div(children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer") + if controls_content + else html.Div(className="hidden", id="control_panel_outer") + ) components_content = [ html.Div( component.build(), @@ -126,7 +132,8 @@ def build(self): self.components, self.layout.component_grid_lines # type: ignore[union-attr] ) ] - return self._create_component_container(components_content) + components_container = self._create_component_container(components_content) + return html.Div([control_panel, components_container]) def _update_graph_theme(self): outputs = [ From c5d7440e9a00df0bb25c0b82e1842fe5b37e78b0 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 22:24:33 +0100 Subject: [PATCH 05/11] Update dashboard unit tests --- vizro-core/tests/unit/vizro/models/test_dashboard.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py index f64b84268..3df95c6fa 100644 --- a/vizro-core/tests/unit/vizro/models/test_dashboard.py +++ b/vizro-core/tests/unit/vizro/models/test_dashboard.py @@ -1,5 +1,6 @@ import json from collections import OrderedDict +from functools import partial import dash import dash_bootstrap_components as dbc @@ -29,7 +30,7 @@ def dashboard_container(): @pytest.fixture() -def mock_page_registry(page1, page2): +def mock_page_registry(dashboard, page1, page2): return OrderedDict( { "Page 1": { @@ -44,12 +45,12 @@ def mock_page_registry(page1, page2): "description": "", "order": 0, "supplied_order": 0, - "supplied_layout": page1.build, + "supplied_layout": partial(dashboard._make_page_layout, page1), "supplied_image": None, "image": None, "image_url": None, "redirect_from": None, - "layout": page1.build, + "layout": partial(dashboard._make_page_layout, page1), "relative_path": "/", }, "Page 2": { @@ -64,12 +65,12 @@ def mock_page_registry(page1, page2): "description": "", "order": 1, "supplied_order": 1, - "supplied_layout": page2.build, + "supplied_layout": partial(dashboard._make_page_layout, page2), "supplied_image": None, "image": None, "image_url": None, "redirect_from": None, - "layout": page2.build, + "layout": partial(dashboard._make_page_layout, page2), "relative_path": "/page-2", }, "not_found_404": { From b3f16cda9fff469040b6d3f5e83f7f1fc1f12d5e Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 22:25:11 +0100 Subject: [PATCH 06/11] Add changelog --- ...ong_li_nguyen_move_page_layout_creation.md | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 vizro-core/changelog.d/20231031_222504_huong_li_nguyen_move_page_layout_creation.md diff --git a/vizro-core/changelog.d/20231031_222504_huong_li_nguyen_move_page_layout_creation.md b/vizro-core/changelog.d/20231031_222504_huong_li_nguyen_move_page_layout_creation.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-core/changelog.d/20231031_222504_huong_li_nguyen_move_page_layout_creation.md @@ -0,0 +1,48 @@ + + + + + + + + + From 64b764edb891136b43808169fa5db4daf77a47af Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Tue, 31 Oct 2023 22:36:13 +0100 Subject: [PATCH 07/11] Update lint-vizro-ai.yml --- .github/workflows/lint-vizro-ai.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/lint-vizro-ai.yml b/.github/workflows/lint-vizro-ai.yml index 95ac9e1a2..de1a6be25 100644 --- a/.github/workflows/lint-vizro-ai.yml +++ b/.github/workflows/lint-vizro-ai.yml @@ -53,7 +53,6 @@ jobs: - name: Find added changelog fragments id: added-files run: | - pwd if ${{ github.event_name == 'pull_request' }}; then echo "added_files=$(git diff --name-only --diff-filter=A -r HEAD^1 HEAD -- changelog.d/*.md | xargs)" >> $GITHUB_OUTPUT else @@ -64,9 +63,9 @@ jobs: run: | if [ -z "${{ steps.added-files.outputs.added_files }}" ]; then - echo "No changelog fragment .md file within changelog.d was detected. Run 'hatch run docs:changelog' to create such a fragment."; + echo "No changelog fragment .md file within changelog.d was detected. Run 'hatch run changelog:add' to create such a fragment."; echo "If your PR contains changes that should be mentioned in the CHANGELOG in the next release, please uncomment the relevant section in your created fragment and describe the changes to the user." - echo "If your changes are not relevant for the CHANGELOG, please save and commit the file as." + echo "If your changes are not relevant for the CHANGELOG, please save and commit the file as is." exit 1 else echo "${{ steps.added-files.outputs.added_files }} was added - ready to go!"; From a0d16fe8b521a2c49ae145bc9b2ae91d3edf42e3 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Wed, 1 Nov 2023 11:14:56 +0100 Subject: [PATCH 08/11] PR comments --- vizro-core/src/vizro/models/_dashboard.py | 48 +++++++++---------- .../tests/unit/vizro/models/test_dashboard.py | 10 ++-- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 839d3d3bb..1cd899304 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -27,28 +27,6 @@ def update_theme(on: bool): return "vizro_dark" if on else "vizro_light" -def create_layout_page_404(): - return html.Div( - [ - html.Img(src=STATIC_URL_PREFIX + "/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", - ) - - class Dashboard(VizroBaseModel): """Vizro Dashboard to be used within [`Vizro`][vizro._vizro.Vizro.build]. @@ -99,7 +77,7 @@ def pre_build(self): dash.register_page( module=page.id, name=page.title, path=path, order=order, layout=partial(self._make_page_layout, page) ) - dash.register_page(module=MODULE_PAGE_404, layout=create_layout_page_404()) + dash.register_page(module=MODULE_PAGE_404, layout=self._make_page_404_layout()) @_log_call def build(self): @@ -118,7 +96,7 @@ def build(self): fluid=True, ) - def _make_page_layout(self, page): + def _make_page_layout(self, page: Page): # Identical across pages dashboard_title = ( html.Div(children=[html.H2(self.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer") @@ -156,3 +134,25 @@ def _update_theme(): Output("dashboard_container_outer", "className"), Input("theme_selector", "on"), ) + + @staticmethod + def _make_page_404_layout(): + return html.Div( + [ + html.Img(src=STATIC_URL_PREFIX + "/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", + ) diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py index 3df95c6fa..49ea53cdf 100644 --- a/vizro-core/tests/unit/vizro/models/test_dashboard.py +++ b/vizro-core/tests/unit/vizro/models/test_dashboard.py @@ -12,7 +12,7 @@ import vizro import vizro.models as vm from vizro.actions._action_loop._action_loop import ActionLoop -from vizro.models._dashboard import create_layout_page_404, update_theme +from vizro.models._dashboard import update_theme @pytest.fixture() @@ -85,12 +85,12 @@ def mock_page_registry(dashboard, page1, page2): "description": "", "order": None, "supplied_order": None, - "supplied_layout": create_layout_page_404(), + "supplied_layout": dashboard._make_page_404_layout(), "supplied_image": None, "image": None, "image_url": None, "redirect_from": None, - "layout": create_layout_page_404(), + "layout": dashboard._make_page_404_layout(), "relative_path": "/not-found-404", }, } @@ -142,8 +142,8 @@ def test_dashboard_page_registry(self, dashboard, 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): - result = create_layout_page_404() + def test_create_layout_page_404(self, dashboard): + result = dashboard._make_page_404_layout() result_image = result.children[0] result_div = result.children[1] From 75c97068846e5b319b177e808f0e5b75f10d45fa Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Wed, 1 Nov 2023 11:25:34 +0100 Subject: [PATCH 09/11] Revert changes --- .github/workflows/lint-vizro-ai.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/lint-vizro-ai.yml b/.github/workflows/lint-vizro-ai.yml index de1a6be25..95ac9e1a2 100644 --- a/.github/workflows/lint-vizro-ai.yml +++ b/.github/workflows/lint-vizro-ai.yml @@ -53,6 +53,7 @@ jobs: - name: Find added changelog fragments id: added-files run: | + pwd if ${{ github.event_name == 'pull_request' }}; then echo "added_files=$(git diff --name-only --diff-filter=A -r HEAD^1 HEAD -- changelog.d/*.md | xargs)" >> $GITHUB_OUTPUT else @@ -63,9 +64,9 @@ jobs: run: | if [ -z "${{ steps.added-files.outputs.added_files }}" ]; then - echo "No changelog fragment .md file within changelog.d was detected. Run 'hatch run changelog:add' to create such a fragment."; + echo "No changelog fragment .md file within changelog.d was detected. Run 'hatch run docs:changelog' to create such a fragment."; echo "If your PR contains changes that should be mentioned in the CHANGELOG in the next release, please uncomment the relevant section in your created fragment and describe the changes to the user." - echo "If your changes are not relevant for the CHANGELOG, please save and commit the file as is." + echo "If your changes are not relevant for the CHANGELOG, please save and commit the file as." exit 1 else echo "${{ steps.added-files.outputs.added_files }} was added - ready to go!"; From 12081a1d4650a7d7f7636763ff35ad7d68713d23 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Wed, 1 Nov 2023 12:19:18 +0100 Subject: [PATCH 10/11] Reduce test coverage --- vizro-core/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vizro-core/pyproject.toml b/vizro-core/pyproject.toml index 265614172..1ae13214c 100644 --- a/vizro-core/pyproject.toml +++ b/vizro-core/pyproject.toml @@ -56,7 +56,7 @@ exclude_lines = [ "if __name__ == .__main__.:", "if TYPE_CHECKING:" ] -fail_under = 92 +fail_under = 91 show_missing = true skip_covered = true From f8fb5d7b2c544fa5c5791a30b68edfd87aee90f0 Mon Sep 17 00:00:00 2001 From: huong-li-nguyen Date: Wed, 1 Nov 2023 17:22:33 +0100 Subject: [PATCH 11/11] Consolidate approach of hidden containers --- vizro-core/docs/pages/user_guides/custom_components.md | 2 +- .../vizro/actions/_action_loop/_get_action_loop_components.py | 2 +- vizro-core/src/vizro/models/_components/card.py | 2 +- vizro-core/src/vizro/models/_components/form/_user_input.py | 2 +- vizro-core/src/vizro/models/_components/form/checklist.py | 2 +- vizro-core/src/vizro/models/_components/form/dropdown.py | 2 +- vizro-core/src/vizro/models/_components/form/radio_items.py | 2 +- vizro-core/src/vizro/models/_components/form/range_slider.py | 2 +- vizro-core/src/vizro/models/_components/form/slider.py | 2 +- vizro-core/src/vizro/models/_components/table.py | 2 +- vizro-core/src/vizro/models/_dashboard.py | 4 ++-- vizro-core/src/vizro/models/_navigation/_accordion.py | 2 +- vizro-core/src/vizro/models/_page.py | 2 +- vizro-core/src/vizro/static/css/layout.css | 4 ---- .../actions/_action_loop/test_get_action_loop_components.py | 2 +- .../unit/vizro/models/_components/form/test_range_slider.py | 2 +- vizro-core/tests/unit/vizro/models/_components/test_table.py | 2 +- .../tests/unit/vizro/models/_navigation/test_accordion.py | 2 +- 18 files changed, 18 insertions(+), 22 deletions(-) diff --git a/vizro-core/docs/pages/user_guides/custom_components.md b/vizro-core/docs/pages/user_guides/custom_components.md index dc3fdc805..bc61f9e13 100644 --- a/vizro-core/docs/pages/user_guides/custom_components.md +++ b/vizro-core/docs/pages/user_guides/custom_components.md @@ -125,7 +125,7 @@ vm.Parameter.add_type("selector", TooltipNonCrossRangeSlider) return html.Div( [ - html.P(self.title, id="range_slider_title") if self.title else None, + html.P(self.title, id="range_slider_title") if self.title else html.Div(hidden=True), html.Div( [ dcc.RangeSlider( diff --git a/vizro-core/src/vizro/actions/_action_loop/_get_action_loop_components.py b/vizro-core/src/vizro/actions/_action_loop/_get_action_loop_components.py index 70b877c30..cb69b6210 100644 --- a/vizro-core/src/vizro/actions/_action_loop/_get_action_loop_components.py +++ b/vizro-core/src/vizro/actions/_action_loop/_get_action_loop_components.py @@ -24,7 +24,7 @@ def _get_action_loop_components() -> html.Div: components = [ dcc.Store(id="action_finished"), dcc.Store(id="remaining_actions", data=[]), - html.Div(id="cycle_breaker_div", style={"display": "hidden"}), + html.Div(id="cycle_breaker_div", hidden=True), dcc.Store(id="cycle_breaker_empty_output_store"), ] diff --git a/vizro-core/src/vizro/models/_components/card.py b/vizro-core/src/vizro/models/_components/card.py index 3933406b6..01e219854 100644 --- a/vizro-core/src/vizro/models/_components/card.py +++ b/vizro-core/src/vizro/models/_components/card.py @@ -39,7 +39,7 @@ def build(self): className="button_container", ) if self.href - else None + else html.Div(hidden=True) ) card_container = "nav_card_container" if self.href else "card_container" diff --git a/vizro-core/src/vizro/models/_components/form/_user_input.py b/vizro-core/src/vizro/models/_components/form/_user_input.py index edcfeff0e..cfe29a275 100644 --- a/vizro-core/src/vizro/models/_components/form/_user_input.py +++ b/vizro-core/src/vizro/models/_components/form/_user_input.py @@ -36,7 +36,7 @@ class UserInput(VizroBaseModel): def build(self): return html.Div( [ - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), dbc.Input( id=self.id, placeholder=self.placeholder, diff --git a/vizro-core/src/vizro/models/_components/form/checklist.py b/vizro-core/src/vizro/models/_components/form/checklist.py index 8fec2870e..b8f0daea6 100644 --- a/vizro-core/src/vizro/models/_components/form/checklist.py +++ b/vizro-core/src/vizro/models/_components/form/checklist.py @@ -41,7 +41,7 @@ def build(self): return html.Div( [ - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), dcc.Checklist( id=self.id, options=full_options, diff --git a/vizro-core/src/vizro/models/_components/form/dropdown.py b/vizro-core/src/vizro/models/_components/form/dropdown.py index 5db1a375d..9d01c79a5 100755 --- a/vizro-core/src/vizro/models/_components/form/dropdown.py +++ b/vizro-core/src/vizro/models/_components/form/dropdown.py @@ -53,7 +53,7 @@ def build(self): full_options, default_value = get_options_and_default(options=self.options, multi=self.multi) return html.Div( [ - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), dcc.Dropdown( id=self.id, options=full_options, diff --git a/vizro-core/src/vizro/models/_components/form/radio_items.py b/vizro-core/src/vizro/models/_components/form/radio_items.py index 57e460206..782f15d21 100644 --- a/vizro-core/src/vizro/models/_components/form/radio_items.py +++ b/vizro-core/src/vizro/models/_components/form/radio_items.py @@ -42,7 +42,7 @@ def build(self): return html.Div( [ - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), dcc.RadioItems( id=self.id, options=full_options, diff --git a/vizro-core/src/vizro/models/_components/form/range_slider.py b/vizro-core/src/vizro/models/_components/form/range_slider.py index 64326c711..4ee181c8d 100644 --- a/vizro-core/src/vizro/models/_components/form/range_slider.py +++ b/vizro-core/src/vizro/models/_components/form/range_slider.py @@ -87,7 +87,7 @@ def build(self): "max": self.max, }, ), - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), html.Div( [ dcc.RangeSlider( diff --git a/vizro-core/src/vizro/models/_components/form/slider.py b/vizro-core/src/vizro/models/_components/form/slider.py index 5e6607f7e..4a44e898f 100644 --- a/vizro-core/src/vizro/models/_components/form/slider.py +++ b/vizro-core/src/vizro/models/_components/form/slider.py @@ -81,7 +81,7 @@ def build(self): "max": self.max, }, ), - html.P(self.title) if self.title else None, + html.P(self.title) if self.title else html.Div(hidden=True), html.Div( [ dcc.Slider( diff --git a/vizro-core/src/vizro/models/_components/table.py b/vizro-core/src/vizro/models/_components/table.py index 4dd0a33a3..f1e98718e 100644 --- a/vizro-core/src/vizro/models/_components/table.py +++ b/vizro-core/src/vizro/models/_components/table.py @@ -56,7 +56,7 @@ def build(self): return dcc.Loading( html.Div( [ - html.H3(self.title, className="table-title") if self.title else None, + html.H3(self.title, className="table-title") if self.title else html.Div(hidden=True), html.Div(dash_table.DataTable(), id=self.id), ], className="table-container", diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 1cd899304..cf6fbab63 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -101,7 +101,7 @@ def _make_page_layout(self, page: Page): dashboard_title = ( html.Div(children=[html.H2(self.title), html.Hr()], className="dashboard_title", id="dashboard_title_outer") if self.title - else html.Div(className="hidden", id="dashboard_title_outer") + else html.Div(hidden=True, id="dashboard_title_outer") ) theme_switch = daq.BooleanSwitch( id="theme_selector", on=True if self.theme == "vizro_dark" else False, persistence=True @@ -122,7 +122,7 @@ def _make_page_layout(self, page: Page): left_side = ( html.Div(children=left_side_elements, className="left_side", id="left_side_outer") if any(left_side_elements) - else html.Div(className="hidden", id="left_side_outer") + else html.Div(hidden=True, id="left_side_outer") ) right_side = html.Div(children=[header, component_container], className="right_side", id="right_side_outer") return html.Div([left_side, right_side], className="page_container", id="page_container_outer") diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index 6937e6b2b..be00ff619 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -36,7 +36,7 @@ def coerce_pages_type(cls, pages): def build(self, *, active_page_id=None): # Hide navigation panel if there is only one page if len(list(itertools.chain(*self.pages.values()))) == 1: - return html.Div(className="hidden") + return html.Div(hidden=True) accordion_items = [] for page_group, page_members in self.pages.items(): diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index 88ca01774..e8e0a8245 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -118,7 +118,7 @@ def build(self): control_panel = ( html.Div(children=[*controls_content, html.Hr()], className="control_panel", id="control_panel_outer") if controls_content - else html.Div(className="hidden", id="control_panel_outer") + else html.Div(hidden=True, id="control_panel_outer") ) components_content = [ html.Div( diff --git a/vizro-core/src/vizro/static/css/layout.css b/vizro-core/src/vizro/static/css/layout.css index 5418b05a3..26eb979f0 100644 --- a/vizro-core/src/vizro/static/css/layout.css +++ b/vizro-core/src/vizro/static/css/layout.css @@ -92,10 +92,6 @@ width: 100%; } -.hidden { - display: none; -} - .loading-container { height: 100%; width: 100%; diff --git a/vizro-core/tests/unit/vizro/actions/_action_loop/test_get_action_loop_components.py b/vizro-core/tests/unit/vizro/actions/_action_loop/test_get_action_loop_components.py index dcabb030d..35546c5a5 100644 --- a/vizro-core/tests/unit/vizro/actions/_action_loop/test_get_action_loop_components.py +++ b/vizro-core/tests/unit/vizro/actions/_action_loop/test_get_action_loop_components.py @@ -19,7 +19,7 @@ def fundamental_components(): return [ dcc.Store(id="action_finished"), dcc.Store(id="remaining_actions", data=[]), - html.Div(id="cycle_breaker_div", style={"display": "hidden"}), + html.Div(id="cycle_breaker_div", hidden=True), dcc.Store(id="cycle_breaker_empty_output_store"), ] diff --git a/vizro-core/tests/unit/vizro/models/_components/form/test_range_slider.py b/vizro-core/tests/unit/vizro/models/_components/form/test_range_slider.py index a5a74407a..dba28f7b6 100644 --- a/vizro-core/tests/unit/vizro/models/_components/form/test_range_slider.py +++ b/vizro-core/tests/unit/vizro/models/_components/form/test_range_slider.py @@ -21,7 +21,7 @@ def expected_range_slider_default(): "max": None, }, ), - None, + html.Div(hidden=True), html.Div( [ dcc.RangeSlider( diff --git a/vizro-core/tests/unit/vizro/models/_components/test_table.py b/vizro-core/tests/unit/vizro/models/_components/test_table.py index ca9eedc1b..157f42bfe 100644 --- a/vizro-core/tests/unit/vizro/models/_components/test_table.py +++ b/vizro-core/tests/unit/vizro/models/_components/test_table.py @@ -28,7 +28,7 @@ def expected_table(): return dcc.Loading( html.Div( [ - None, + html.Div(hidden=True), html.Div(dash_table.DataTable(), id="text_table"), ], className="table-container", diff --git a/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py b/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py index 17d483450..c96a782cf 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/test_accordion.py @@ -62,5 +62,5 @@ def test_accordion_build_pages_as_dict(self, pages_as_dict, accordion_from_pages def test_single_page_and_hidden_div(self): accordion = Accordion(pages=["Page 1"]).build() result = json.loads(json.dumps(accordion, cls=plotly.utils.PlotlyJSONEncoder)) - expected = json.loads(json.dumps(html.Div(className="hidden"), cls=plotly.utils.PlotlyJSONEncoder)) + expected = json.loads(json.dumps(html.Div(hidden=True), cls=plotly.utils.PlotlyJSONEncoder)) assert result == expected