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

Move layout creation to Dashboard #142

Merged
merged 13 commits into from
Nov 1, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!--
A new scriv changelog fragment.

Uncomment the section that is right (remove the HTML comment wrapper).
-->

<!--
### Highlights ✨

- A bullet item for the Highlights ✨ 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))

-->
<!--
### Removed

- 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))

-->
<!--
### Added

- A bullet item for the Added 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))

-->
<!--
### Changed

- A bullet item for the Changed 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))

-->
<!--
### Deprecated

- A bullet item for the Deprecated 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))

-->
<!--
### Fixed

- A bullet item for the Fixed 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))

-->
<!--
### Security

- A bullet item for the Security 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))

-->
2 changes: 1 addition & 1 deletion vizro-core/docs/pages/user_guides/custom_components.md
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion vizro-core/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ exclude_lines = [
"if __name__ == .__main__.:",
"if TYPE_CHECKING:"
]
fail_under = 92
fail_under = 91
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to reduce as strangely this fails on the unit tests for 3.11 only: https://github.com/mckinsey/vizro/actions/runs/6718518053/job/18258559080?pr=142

We have a proper ticket to investigate further here:
https://github.com/McK-Internal/vizro-internal/issues/43

Copy link
Contributor

Choose a reason for hiding this comment

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

For transparency:

  • the reason this fails for python 3.11 and not others is not related to this PR: the coverage for different version is different also in previous checks (the reason seems to be the kedro integrations code, which skips some tests depending on the version)
  • this means that this PR decreased the overall coverage just enough to push some tests below the threshold
  • the culprit seems to lines 101-128 in _dashboard.py

Let's investigate this further in another PR, just wanted to clarify for the person that looks at this

Copy link
Contributor

Choose a reason for hiding this comment

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

Kedro 0.18.13 onwards support Python 3.11 so we should be able to remove our special skipping of kedro in that environment now anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

show_missing = true
skip_covered = true

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
]

Expand Down
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/models/_components/card.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/models/_components/form/dropdown.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/models/_components/form/slider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/models/_components/table.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
85 changes: 58 additions & 27 deletions vizro-core/src/vizro/models/_dashboard.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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].

Expand Down Expand Up @@ -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):
Expand All @@ -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"]
huong-li-nguyen marked this conversation as resolved.
Show resolved Hide resolved

# 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(
Expand All @@ -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",
)
2 changes: 1 addition & 1 deletion vizro-core/src/vizro/models/_navigation/_accordion.py
Original file line number Diff line number Diff line change
Expand Up @@ -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():
Expand Down
Loading
Loading