diff --git a/pyproject.toml b/pyproject.toml index 5cbb6a28a..1ce3d8ab7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -49,6 +49,7 @@ warn_untyped_fields = true ignore = [ "D104" # undocumented-public-package ] +ignore-init-module-imports = true line-length = 120 select = [ "E", # pycodestyle errors @@ -70,7 +71,7 @@ known-first-party = ["vizro"] # Tests can use magic values, assertions, and relative imports, ignore missing docstrings "**/tests/**/*" = ["PLR2004", "S101", "TID252", "D100", "D101", "D102", "D103"] # Ignore import violations in all __init__.py files -"__init__.py" = ["E402"] +"__init__.py" = ["E402", "F401"] [tool.ruff.pydocstyle] convention = "google" diff --git a/vizro-ai/changelog.d/20231114_161107_antony.milne_release_lint.md b/vizro-ai/changelog.d/20231114_161107_antony.milne_release_lint.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-ai/changelog.d/20231114_161107_antony.milne_release_lint.md @@ -0,0 +1,48 @@ + + + + + + + + + diff --git a/vizro-ai/hatch.toml b/vizro-ai/hatch.toml index a45091ddf..4525a0e3b 100644 --- a/vizro-ai/hatch.toml +++ b/vizro-ai/hatch.toml @@ -6,7 +6,7 @@ python = ["3.9", "3.10", "3.11"] [envs.changelog] dependencies = ["scriv"] detached = true -scripts = {add = "scriv create --add", collect = ["scriv collect --add", "- hatch run lint:lint --files=CHANGELOG.md > /dev/null"]} +scripts = {add = "scriv create --add", collect = ["scriv collect --add", "- hatch run lint:lint --files=CHANGELOG.md"]} [envs.default] dependencies = [ diff --git a/vizro-core/changelog.d/20231113_122927_antony.milne_update_theme.md b/vizro-core/changelog.d/20231113_122927_antony.milne_update_theme.md new file mode 100644 index 000000000..9a3bc5b97 --- /dev/null +++ b/vizro-core/changelog.d/20231113_122927_antony.milne_update_theme.md @@ -0,0 +1,47 @@ + + + + + + + + +### Fixed + +- Remove graph flickering on page load with Vizro light theme ([#166](https://github.com/mckinsey/vizro/pull/166)) + + diff --git a/vizro-core/changelog.d/20231114_130850_nadija_ratkusic_graca_nav_panel_keyline_v2.md b/vizro-core/changelog.d/20231114_130850_nadija_ratkusic_graca_nav_panel_keyline_v2.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-core/changelog.d/20231114_130850_nadija_ratkusic_graca_nav_panel_keyline_v2.md @@ -0,0 +1,48 @@ + + + + + + + + + diff --git a/vizro-core/changelog.d/20231114_155203_antony.milne_release_lint.md b/vizro-core/changelog.d/20231114_155203_antony.milne_release_lint.md new file mode 100644 index 000000000..f1f65e73c --- /dev/null +++ b/vizro-core/changelog.d/20231114_155203_antony.milne_release_lint.md @@ -0,0 +1,48 @@ + + + + + + + + + diff --git a/vizro-core/hatch.toml b/vizro-core/hatch.toml index 25d9b040d..b5b37f96c 100644 --- a/vizro-core/hatch.toml +++ b/vizro-core/hatch.toml @@ -11,7 +11,7 @@ matrix.python.features = [ [envs.changelog] dependencies = ["scriv"] detached = true -scripts = {add = "scriv create --add", collect = ["scriv collect --add", "- hatch run lint:lint --files=CHANGELOG.md > /dev/null"]} +scripts = {add = "scriv create --add", collect = ["scriv collect --add", "- hatch run lint:lint --files=CHANGELOG.md"]} [envs.default] dependencies = [ diff --git a/vizro-core/src/vizro/actions/__init__.py b/vizro-core/src/vizro/actions/__init__.py index f3411d31f..39224154f 100644 --- a/vizro-core/src/vizro/actions/__init__.py +++ b/vizro-core/src/vizro/actions/__init__.py @@ -1,6 +1,3 @@ -# Redundant aliases here to prevent ruff from removing unused imports. -from typing import Any, Callable, Dict - from vizro.actions._filter_action import _filter from vizro.actions._on_page_load_action import _on_page_load from vizro.actions._parameter_action import _parameter @@ -9,12 +6,3 @@ # Please keep alphabetically ordered __all__ = ["export_data", "filter_interaction"] - -# Actions lookup dictionary to facilitate function comparison -action_functions: Dict[Callable[[Any], Dict[str, Any]], str] = { - export_data.__wrapped__: "export_data", - filter_interaction.__wrapped__: "filter_interaction", - _filter.__wrapped__: "filter", - _parameter.__wrapped__: "parameter", - _on_page_load.__wrapped__: "on_page_load", -} diff --git a/vizro-core/src/vizro/models/_components/graph.py b/vizro-core/src/vizro/models/_components/graph.py index cb792615e..220e835bf 100644 --- a/vizro-core/src/vizro/models/_components/graph.py +++ b/vizro-core/src/vizro/models/_components/graph.py @@ -16,18 +16,6 @@ logger = logging.getLogger(__name__) -def create_empty_fig(message: str) -> go.Figure: - """Creates empty go.Figure object with a display message.""" - fig = go.Figure() - fig.add_trace(go.Scatter(x=[None], y=[None], showlegend=False, hoverinfo="none")) - fig.update_layout( - xaxis={"visible": False}, - yaxis={"visible": False}, - annotations=[{"text": message, "showarrow": False, "font": {"size": 16}}], - ) - return fig - - class Graph(VizroBaseModel): """Wrapper for `dcc.Graph` to visualize charts in dashboard. @@ -68,14 +56,21 @@ def __getitem__(self, arg_name: str): @_log_call def build(self): + # The empty figure here is just a placeholder designed to be replaced by the actual figure when the filters + # etc. are applied. It only appears on the screen for a brief instant, but we need to make sure it's + # transparent and has no axes so it doesn't draw anything on the screen which would flicker away when the + # graph callback is executed to make the dcc.Loading icon appear. return dcc.Loading( dcc.Graph( id=self.id, - # We don't do self.__call__() until the Graph is actually built. This ensures that lazy data is not - # loaded until the graph is first shown on the screen. At the moment, we eagerly run page.build() for - # all pages in Dashboard.build in order to register all the callbacks in advance. In future this should - # no longer be the case so that we achieve true lazy loading. - figure=create_empty_fig(""), + figure=go.Figure( + layout={ + "paper_bgcolor": "rgba(0,0,0,0)", + "plot_bgcolor": "rgba(0,0,0,0)", + "xaxis": {"visible": False}, + "yaxis": {"visible": False}, + } + ), config={ "autosizable": True, "frameMargins": 0, diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py index 11d482706..f28b08f22 100644 --- a/vizro-core/src/vizro/models/_dashboard.py +++ b/vizro-core/src/vizro/models/_dashboard.py @@ -7,7 +7,6 @@ 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, get_relative_path, html from pydantic import Field, validator @@ -23,10 +22,6 @@ logger = logging.getLogger(__name__) -def update_theme(on: bool): - return "vizro_dark" if on else "vizro_light" - - class Dashboard(VizroBaseModel): """Vizro Dashboard to be used within [`Vizro`][vizro._vizro.Vizro.build]. @@ -60,13 +55,6 @@ def validate_navigation(cls, navigation, values): return Navigation(pages=[page.id for page in values["pages"]]) return navigation - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - # pio is the backend global state and shouldn't be changed while - # the app is running. This limitation leads to the case that Graphs blink - # on page load if user previously has changed theme_selector. - pio.templates.default = self.theme - @_log_call def pre_build(self): # Setting order here ensures that the pages in dash.page_registry preserves the order of the List[Page]. @@ -83,7 +71,12 @@ def pre_build(self): def build(self): for page in self.pages: page.build() # TODO: ideally remove, but necessary to register slider callbacks - self._update_theme() + + clientside_callback( + ClientsideFunction(namespace="clientside", function_name="update_dashboard_theme"), + Output("dashboard_container_outer", "className"), + Input("theme_selector", "on"), + ) return dbc.Container( id="dashboard_container_outer", @@ -103,9 +96,7 @@ def _make_page_layout(self, page: Page): 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 - ) + theme_switch = daq.BooleanSwitch(id="theme_selector", on=self.theme == "vizro_dark", persistence=True) # Shared across pages but slightly differ in content page_title = html.H2(children=page.title, id="page_title") @@ -127,14 +118,6 @@ def _make_page_layout(self, page: Page): 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( - ClientsideFunction(namespace="clientside", function_name="update_dashboard_theme"), - Output("dashboard_container_outer", "className"), - Input("theme_selector", "on"), - ) - @staticmethod def _make_page_404_layout(): return html.Div( diff --git a/vizro-core/src/vizro/models/_navigation/_accordion.py b/vizro-core/src/vizro/models/_navigation/_accordion.py index be00ff619..5dabc70f1 100644 --- a/vizro-core/src/vizro/models/_navigation/_accordion.py +++ b/vizro-core/src/vizro/models/_navigation/_accordion.py @@ -59,7 +59,6 @@ def build(self, *, active_page_id=None): persistence_type="session", always_open=True, ), - html.Hr(), ], className="nav_panel", id="nav_panel_outer", diff --git a/vizro-core/src/vizro/models/_page.py b/vizro-core/src/vizro/models/_page.py index e8e0a8245..acefed092 100644 --- a/vizro-core/src/vizro/models/_page.py +++ b/vizro-core/src/vizro/models/_page.py @@ -116,7 +116,7 @@ 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") + html.Div(children=[*controls_content], className="control_panel", id="control_panel_outer") if controls_content else html.Div(hidden=True, id="control_panel_outer") ) @@ -136,6 +136,15 @@ def build(self): return html.Div([control_panel, components_container]) def _update_graph_theme(self): + # The obvious way to do this would be to alter pio.templates.default, but this changes global state and so is + # not good. + # Putting graphs as inputs here would be a nice way to trigger the theme change automatically so that we don't + # need the update_layout call in Graph.__call__, but this results in an extra callback and the graph + # flickering. + # TODO: consider making this clientside callback and then possibly we can remove the update_layout in + # Graph.__call__ without any flickering. + # TODO: consider putting the Graph-specific logic here in the Graph model itself (whether clientside or + # serverside) to keep the code here abstract. outputs = [ Output(component.id, "figure", allow_duplicate=True) for component in self.components @@ -143,11 +152,7 @@ def _update_graph_theme(self): ] if outputs: - @callback( - outputs, - Input("theme_selector", "on"), - prevent_initial_call="initial_duplicate", - ) + @callback(outputs, Input("theme_selector", "on"), prevent_initial_call="initial_duplicate") def update_graph_theme(theme_selector_on: bool): patched_figure = Patch() patched_figure["layout"]["template"] = themes.dark if theme_selector_on else themes.light diff --git a/vizro-core/src/vizro/static/css/layout.css b/vizro-core/src/vizro/static/css/layout.css index 26eb979f0..dd3b67d86 100644 --- a/vizro-core/src/vizro/static/css/layout.css +++ b/vizro-core/src/vizro/static/css/layout.css @@ -14,7 +14,7 @@ padding: 40px 32px 0 32px; width: 352px; overflow: auto; - gap: var(--spacing-04); + gap: var(--spacing-08); } .right_side { @@ -52,6 +52,10 @@ flex-direction: column; } +.nav_panel:not(:empty) { + border-bottom: 1px solid var(--border-subtle-alpha-01); +} + .control_panel { align-self: stretch; display: flex; @@ -60,6 +64,10 @@ padding: 4px 0 24px 0; } +.control_panel:not(:empty) { + border-bottom: 1px solid var(--border-subtle-alpha-01); +} + .page_error_container { align-items: center; background: var(--surfaces-bg-03); diff --git a/vizro-core/tests/unit/vizro/models/_components/test_graph.py b/vizro-core/tests/unit/vizro/models/_components/test_graph.py index e45609321..9694febe9 100644 --- a/vizro-core/tests/unit/vizro/models/_components/test_graph.py +++ b/vizro-core/tests/unit/vizro/models/_components/test_graph.py @@ -11,7 +11,6 @@ import vizro.plotly.express as px from vizro.managers import data_manager from vizro.models._action._action import Action -from vizro.models._components.graph import create_empty_fig @pytest.fixture @@ -44,7 +43,14 @@ def expected_graph(): return dcc.Loading( dcc.Graph( id="text_graph", - figure=create_empty_fig(""), + figure=go.Figure( + layout={ + "paper_bgcolor": "rgba(0,0,0,0)", + "plot_bgcolor": "rgba(0,0,0,0)", + "xaxis": {"visible": False}, + "yaxis": {"visible": False}, + } + ), config={ "autosizable": True, "frameMargins": 0, @@ -136,15 +142,8 @@ def test_process_figure_data_frame_df(self, standard_px_chart, gapminder): class TestBuild: - def test_create_empty_fig(self, expected_empty_chart): - result = create_empty_fig("NO DATA") - assert result == expected_empty_chart - def test_graph_build(self, standard_px_chart, expected_graph): - graph = vm.Graph( - id="text_graph", - figure=standard_px_chart, - ) + graph = vm.Graph(id="text_graph", figure=standard_px_chart) result = json.loads(json.dumps(graph.build(), cls=plotly.utils.PlotlyJSONEncoder)) expected = json.loads(json.dumps(expected_graph, cls=plotly.utils.PlotlyJSONEncoder)) diff --git a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py index 655ca768b..ce060e231 100644 --- a/vizro-core/tests/unit/vizro/models/_navigation/conftest.py +++ b/vizro-core/tests/unit/vizro/models/_navigation/conftest.py @@ -52,7 +52,6 @@ def accordion_from_page_as_list(): persistence_type="session", always_open=True, ), - html.Hr(), ], className="nav_panel", id="nav_panel_outer", @@ -90,7 +89,6 @@ def accordion_from_pages_as_dict(): persistence_type="session", always_open=True, ), - html.Hr(), ], className="nav_panel", id="nav_panel_outer", diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py index 43353b85d..8f2937153 100644 --- a/vizro-core/tests/unit/vizro/models/test_dashboard.py +++ b/vizro-core/tests/unit/vizro/models/test_dashboard.py @@ -12,7 +12,6 @@ import vizro import vizro.models as vm from vizro.actions._action_loop._action_loop import ActionLoop -from vizro.models._dashboard import update_theme @pytest.fixture() @@ -163,9 +162,3 @@ def test_dashboard_build(self, dashboard_container, dashboard): 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 - - -@pytest.mark.parametrize("on, expected", [(True, "vizro_dark"), (False, "vizro_light")]) -def test_update_theme(on, expected): - result = update_theme(on) - assert result == expected