[Bug] Fix theme flickering and 404 page (#865)

mckinsey · Nov 13, 2024 · 0245684 · 0245684
1 parent 285f566
commit 0245684
Show file tree

Hide file tree

Showing 8 changed files with 90 additions and 58 deletions.
diff --git a/vizro-core/changelog.d/20241112_120233_huong_li_nguyen_fix_404_page.md b/vizro-core/changelog.d/20241112_120233_huong_li_nguyen_fix_404_page.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
+
+- Fix 404 error page and page flickering on refresh ([#865](https://github.com/mckinsey/vizro/pull/865)).
+
+<!--
+### 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/_vizro.py b/vizro-core/src/vizro/_vizro.py
@@ -90,10 +90,6 @@ def build(self, dashboard: Dashboard):
             self: Vizro app
 
         """
-        # Note Dash.index uses self.dash.title instead of self.dash.app.config.title.
-        if dashboard.title:
-            self.dash.title = dashboard.title
-
         # Set global chart template to vizro_light or vizro_dark.
         # The choice between these is generally meaningless because chart colors in the two are identical, and
         # everything else gets overridden in the clientside theme selector callback.
@@ -108,9 +104,16 @@ def build(self, dashboard: Dashboard):
 
         # Note that model instantiation and pre_build are independent of Dash.
         self._pre_build()
-
         self.dash.layout = dashboard.build()
 
+        # Add data-bs-theme attribute that is always present, even for pages without theme selector,
+        # i.e. the Dash "Loading..." screen.
+        bootstrap_theme = dashboard.theme.removeprefix("vizro_")
+        self.dash.index_string = self.dash.index_string.replace("<html>", f"<html data-bs-theme='{bootstrap_theme}'>")
+
+        # Note Dash.index uses self.dash.title instead of self.dash.app.config.title.
+        if dashboard.title:
+            self.dash.title = dashboard.title
         return self
 
     def run(self, *args, **kwargs):  # if type annotated, mkdocstring stops seeing the class

diff --git a/vizro-core/src/vizro/models/_dashboard.py b/vizro-core/src/vizro/models/_dashboard.py
@@ -302,14 +302,24 @@ def _make_page_layout(self, page: Page, **kwargs):
         page_layout.id = page.id
         return page_layout
 
-    @staticmethod
-    def _make_page_404_layout():
+    def _make_page_404_layout(self):
         # The svg file is available through the _dash-component-suites/vizro route, as used in Dash's
         # _relative_url_path, but that feels too private to access directly. Hence read the file in directly rather
         # than referring to its path.
         error_404_svg = base64.b64encode((VIZRO_ASSETS_PATH / "images/error_404.svg").read_bytes()).decode("utf-8")
         return html.Div(
             [
+                # Theme switch is added such that the 404 page has the same theme as the user-selected one.
+                html.Div(
+                    children=dmc.Switch(
+                        id="theme_selector",
+                        checked=self.theme == "vizro_light",
+                        persistence=True,
+                        persistence_type="session",
+                        className="toggle-switch",
+                    ),
+                    id="settings",
+                ),
                 html.Img(src=f"data:image/svg+xml;base64,{error_404_svg}"),
                 html.Div(
                     [

diff --git a/vizro-core/src/vizro/static/css/layout.css b/vizro-core/src/vizro/static/css/layout.css
@@ -87,11 +87,10 @@
 
 .page-error-container {
   align-items: center;
-  background: var(--surfaces-bg-03);
   display: flex;
   flex-direction: column;
   height: 100vh;
-  padding: 64px;
+  justify-content: center;
   width: 100vw;
 }
 
@@ -242,3 +241,8 @@
     zoom: 80%;
   }
 }
+
+/* Hide Loading... text on loading page */
+._dash-loading {
+  display: none;
+}
diff --git a/vizro-core/src/vizro/static/css/loading.css b/vizro-core/src/vizro/static/css/loading.css
diff --git a/vizro-core/src/vizro/static/css/vizro-bootstrap.min.css b/vizro-core/src/vizro/static/css/vizro-bootstrap.min.css
diff --git a/vizro-core/src/vizro/static/images/error_404.svg b/vizro-core/src/vizro/static/images/error_404.svg
diff --git a/vizro-core/tests/unit/vizro/models/test_dashboard.py b/vizro-core/tests/unit/vizro/models/test_dashboard.py
@@ -2,6 +2,7 @@
 
 import dash
 import dash_bootstrap_components as dbc
+import dash_mantine_components as dmc
 import plotly.io as pio
 import pytest
 from asserts import assert_component_equal
@@ -239,10 +240,20 @@ def test_valid_logo_combinations(self, page_1, tmp_path, logo_files):
         Vizro(assets_folder=tmp_path)
         vm.Dashboard(pages=[page_1]).pre_build()
 
-    def test_make_page_404_layout(self, vizro_app):
+    def test_make_page_404_layout(self, page_1, vizro_app):
         # vizro_app fixture is needed to avoid mocking out get_relative_path.
         expected = html.Div(
             [
+                html.Div(
+                    children=dmc.Switch(
+                        id="theme_selector",
+                        checked=False,
+                        persistence=True,
+                        persistence_type="session",
+                        className="toggle-switch",
+                    ),
+                    id="settings",
+                ),
                 html.Img(),
                 html.Div(
                     [
@@ -263,7 +274,7 @@ def test_make_page_404_layout(self, vizro_app):
 
         # Strip out src since it's too long to be worth comparing and just comes directly
         # from reading a file.
-        assert_component_equal(vm.Dashboard._make_page_404_layout(), expected, keys_to_strip={"src"})
+        assert_component_equal(vm.Dashboard(pages=[page_1])._make_page_404_layout(), expected, keys_to_strip={"src"})
 
 
 class TestDashboardBuild: