Fix flickering graphs (#166)

mckinsey · Nov 15, 2023 · 0a73928 · 0a73928
1 parent 2f5e1e3
commit 0a73928
Showing 8 changed files with 87 additions and 76 deletions.
diff --git 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-core/changelog.d/20231113_122927_antony.milne_update_theme.md b/vizro-core/changelog.d/20231113_122927_antony.milne_update_theme.md
@@ -0,0 +1,47 @@
+<!--
+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
+
+- Remove graph flickering on page load with Vizro light theme ([#166](https://github.com/mckinsey/vizro/pull/166))
+
+<!--
+### 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))
+
+-->
diff --git 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
@@ -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
@@ -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/_page.py b/vizro-core/src/vizro/models/_page.py
@@ -136,18 +136,23 @@ 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
             if isinstance(component, Graph)
         ]
         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/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/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