Fix flickering graphs

vizro-core/changelog.d/20231113_122927_antony.milne_update_theme.md

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

vizro-core/src/vizro/actions/__init__.py

@@ -9,12 +9,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",
-}

vizro-core/src/vizro/actions/_actions_utils.py

@@ -267,7 +267,4 @@ def _get_modified_page_figures(
         outputs[target] = model_manager[target](  # type: ignore[operator]
             data_frame=filtered_data[target], **parameterized_config[target]
         )
-        if hasattr(outputs[target], "update_layout"):
-            outputs[target].update_layout(template="vizro_dark" if ctd_theme["value"] else "vizro_light")
-
     return outputs

vizro-core/src/vizro/models/_components/graph.py

@@ -1,7 +1,8 @@
 import logging
 from typing import List, Literal
 
-from dash import dcc
+from dash import dcc, ctx
+from dash.exceptions import MissingCallbackContextException
 from plotly import graph_objects as go
 from pydantic import Field, PrivateAttr, validator
 
@@ -16,18 +17,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.
 
@@ -56,6 +45,16 @@ def __call__(self, **kwargs):
         # Remove top margin if title is provided
         if fig.layout.title.text is None:
             fig.update_layout(margin_t=24)
+
+        try:
+            # if "theme_selector" in ctx.states # Probably want this here given code should work outside Dashboard
+            # for now
+            fig.update_layout(template="vizro_dark" if ctx.states["theme_selector.on"] else "vizro_light")
+        except MissingCallbackContextException:
+            # Possibly we should enforce that __call__ can only be used within the context of a callback, but it's easy
+            # to just swallow up the error here as it doesn't cause any problems.
+            logger.info("fig.update_layout called outside of callback context.")
+
         return fig
 
     # Convenience wrapper/syntactic sugar.
@@ -68,14 +67,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,

vizro-core/src/vizro/models/_dashboard.py

@@ -23,10 +23,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 +56,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 +72,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 +97,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 +119,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(

vizro-core/src/vizro/models/_page.py

@@ -136,18 +136,25 @@ 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 this as dashboard level like with _update_theme (though possibly pointless because you
+        #  don't see graphs on other pages and then when you change page it always redraws graphs anyway).
+        # 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

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