From 5eabefa08730de9d4786fff507dbdc25ac746fcc Mon Sep 17 00:00:00 2001 From: Li Nguyen <90609403+huong-li-nguyen@users.noreply.github.com> Date: Wed, 1 Nov 2023 17:30:54 +0100 Subject: [PATCH] Move layout creation to `Dashboard` (#142) --- ...ong_li_nguyen_move_page_layout_creation.md | 48 ++++++++ .../pages/user_guides/custom_components.md | 2 +- vizro-core/pyproject.toml | 2 +- .../_get_action_loop_components.py | 2 +- .../src/vizro/models/_components/card.py | 2 +- .../models/_components/form/_user_input.py | 2 +- .../models/_components/form/checklist.py | 2 +- .../vizro/models/_components/form/dropdown.py | 2 +- .../models/_components/form/radio_items.py | 2 +- .../models/_components/form/range_slider.py | 2 +- .../vizro/models/_components/form/slider.py | 2 +- .../src/vizro/models/_components/table.py | 2 +- vizro-core/src/vizro/models/_dashboard.py | 85 +++++++++----- .../vizro/models/_navigation/_accordion.py | 2 +- vizro-core/src/vizro/models/_page.py | 111 ++++-------------- vizro-core/src/vizro/static/css/layout.css | 4 - .../test_get_action_loop_components.py | 2 +- .../_components/form/test_range_slider.py | 2 +- .../vizro/models/_components/test_table.py | 2 +- .../models/_navigation/test_accordion.py | 2 +- .../tests/unit/vizro/models/test_dashboard.py | 21 ++-- 21 files changed, 157 insertions(+), 144 deletions(-) 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 @@ + + + + + + + + + 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/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 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 6f42700dd..cf6fbab63 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -1,10 +1,12 @@ from __future__ import annotations import logging -from typing import TYPE_CHECKING, List, Literal, Optional +from functools import partial +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 @@ -25,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]. @@ -94,8 +74,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=page.build) - 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=self._make_page_404_layout()) @_log_call def build(self): @@ -114,6 +96,37 @@ def build(self): fluid=True, ) + 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") + if self.title + 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 + ) + + # 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 + 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") + 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 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") + @staticmethod def _update_theme(): clientside_callback( @@ -123,5 +136,23 @@ def _update_theme(): ) @staticmethod - def _create_error_page_404(): - return dash.register_page(module=MODULE_PAGE_404, layout=create_layout_page_404()) + 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/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 6e57d89ad..e8e0a8245 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 @@ -118,6 +115,11 @@ def pre_build(self): 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(hidden=True, id="control_panel_outer") + ) components_content = [ html.Div( component.build(), @@ -130,7 +132,8 @@ def build(self): self.components, self.layout.component_grid_lines # type: ignore[union-attr] ) ] - return self._make_page_layout(controls_content, components_content) + components_container = self._create_component_container(components_content) + return html.Div([control_panel, components_container]) def _update_graph_theme(self): outputs = [ @@ -150,90 +153,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", - ) 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 diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py index f64b84268..49ea53cdf 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 @@ -11,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() @@ -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": { @@ -84,12 +85,12 @@ def mock_page_registry(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", }, } @@ -141,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]