Degrade gracefully when JavaScript is disabled

Support for light/dark themes has been implemented using a `data-theme` attribute set on the `<html/>` tag. As this attribute is set using JavaScript, this meant that it was left unset when visitors had JavaScript disabled. This resulted in several important CSS rules not being matched and a “broken feeling” due to wrong colors, and logo or images shown twice. To better support browsers with JavaScript disabled: 1. Add the same CSS rules as for the light theme when the `data-theme` attribute is not set. This creates a safe fallback in every situation. 2. If `default_mode` is set, write its value to the `data-theme` attribute when writing the HTML files. This enables theme users to present their preferred mode to visitors with JavaScript disabled. 3. Use JavaScript to add the search and theme switcher buttons as they require JavaScript to work. This avoid unusable UI elements to be shown to visitors with JavaScript disabled. 4. Use JavaScript to write the logo for the “other theme”, depending on the value of `default_mode`, defaulting to “light” if unset. While this last change might seem redundant considering the other ones, it does make the resulting pages better for search engines and text browsers. Closes: #1145
pydata · Feb 6, 2023 · 51a975c · 51a975c
1 parent d50d3aa
commit 51a975c
Show file tree

Hide file tree

Showing 8 changed files with 162 additions and 78 deletions.
diff --git a/src/pydata_sphinx_theme/assets/styles/variables/_color.scss b/src/pydata_sphinx_theme/assets/styles/variables/_color.scss
@@ -80,52 +80,70 @@ $pst-semantic-colors: (
 
 /*******************************************************************************
 * write the color rules for each theme (light/dark)
-*
-* NOTE: @each {...} is like a for-loop
-* https://sass-lang.com/documentation/at-rules/control/each
-* and #{...} inserts a variable into a CSS selector or property name
-* https://sass-lang.com/documentation/interpolation
 */
-@each $mode in (light, dark) {
-  html[data-theme="#{$mode}"] {
-    @each $name, $value in $pst-semantic-colors {
-      // check if this color is defined differently for light/dark
-      @if type-of($value) == map {
-        $value: map-get($value, $mode);
-      }
+
+/* NOTE:
+ * Mixins enable us to reuse the same definitions for the different modes
+ * https://sass-lang.com/documentation/at-rules/mixin
+ * #{...} inserts a variable into a CSS selector or property name
+ * https://sass-lang.com/documentation/interpolation
+ */
+@mixin theme-colors($mode) {
+  // check if this color is defined differently for light/dark
+  @each $name, $value in $pst-semantic-colors {
+    @if type-of($value) == map {
+      $value: map-get($value, $mode);
+    }
+    & {
       --pst-color-#{$name}: #{$value};
     }
-    // assign the "duplicate" colors (ones that just reference other variables)
+  }
+  // assign the "duplicate" colors (ones that just reference other variables)
+  & {
     --pst-color-link: var(--pst-color-primary);
     --pst-color-link-hover: var(--pst-color-warning);
-    // adapt to light/dark-specific content
-    @if $mode == "light" {
-      .only-dark {
-        display: none !important;
-      }
-    } @else {
-      .only-light {
-        display: none !important;
-      }
-      /* Adjust images in dark mode (unless they have class .only-dark or
-       * .dark-light, in which case assume they're already optimized for dark
-       * mode).
-       */
-      img:not(.only-dark):not(.dark-light) {
-        filter: brightness(0.8) contrast(1.2);
-      }
-      /* Give images a light background in dark mode in case they have
-      *  transparency and black text (unless they have class .only-dark or .dark-light, in
-      *  which case assume they're already optimized for dark mode).
-      */
-      .bd-content img:not(.only-dark):not(.dark-light) {
-        background: rgb(255, 255, 255);
-        border-radius: 0.25rem;
-      }
-      // MathJax SVG outputs should be filled to same color as text.
-      .MathJax_SVG * {
-        fill: var(--pst-color-text-base);
-      }
+  }
+  // adapt to light/dark-specific content
+  @if $mode == "light" {
+    .only-dark {
+      display: none !important;
+    }
+  } @else {
+    .only-light {
+      display: none !important;
+    }
+    /* Adjust images in dark mode (unless they have class .only-dark or
+     * .dark-light, in which case assume they're already optimized for dark
+     * mode).
+     */
+    img:not(.only-dark):not(.dark-light) {
+      filter: brightness(0.8) contrast(1.2);
     }
+    /* Give images a light background in dark mode in case they have
+    *  transparency and black text (unless they have class .only-dark or .dark-light, in
+    *  which case assume they're already optimized for dark mode).
+    */
+    .bd-content img:not(.only-dark):not(.dark-light) {
+      background: rgb(255, 255, 255);
+      border-radius: 0.25rem;
+    }
+    // MathJax SVG outputs should be filled to same color as text.
+    .MathJax_SVG * {
+      fill: var(--pst-color-text-base);
+    }
+  }
+}
+
+/* Defaults to light mode if data-theme is not set */
+html:not([data-theme]) {
+  @include theme-colors("light");
+}
+
+/* NOTE: @each {...} is like a for-loop
+ * https://sass-lang.com/documentation/at-rules/control/each
+ */
+@each $mode in (light, dark) {
+  html[data-theme="#{$mode}"] {
+    @include theme-colors($mode);
   }
 }
diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/navbar-logo.html b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/navbar-logo.html
@@ -13,8 +13,16 @@
   {% set is_logo = "light" in theme_logo["image_relative"] %}
   {% set alt = theme_logo.get("alt_text", "Logo image") %}
   {% if is_logo %}
-    <img src="{{ theme_logo['image_relative']['light'] }}" class="logo__image only-light" alt="{{ alt }}"/>
-    <img src="{{ theme_logo['image_relative']['dark'] }}" class="logo__image only-dark" alt="{{ alt }}"/>
+    {# Theme switching is only available when JavaScript is enabled.
+     # Thus we should add the extra image using JavaScript, defaulting
+     # depending on the value of default_mode; and light if unset.
+     #}
+    {% if default_mode is undefined %}
+      {% set default_mode = "light" %}
+    {% endif %}
+    {% set js_mode = "light" if default_mode == "dark" else "dark" %}
+    <img src="{{ theme_logo['image_relative'][default_mode] }}" class="logo__image only-{{ default_mode }}" alt="{{ alt }}"/>
+    <script>document.write(`<img src="{{ theme_logo['image_relative'][js_mode] }}" class="logo__image only-{{ js_mode }}" alt="{{ alt }}"/>`);</script>
   {% endif %}
   {% if not is_logo or theme_logo.get("text") %}
     <p class="title logo__title">{{ theme_logo.get("text") or docstitle }}</p>

diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/search-button.html b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/search-button.html
@@ -1,4 +1,11 @@
-{# A button that, when clicked, will trigger a search popup overlay #}
-<button class="btn btn-sm navbar-btn search-button search-button__button" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
-  <i class="fa-solid fa-magnifying-glass"></i>
-</button>
+{# A button that, when clicked, will trigger a search popup overlay.
+ #
+ # As this function will only work when JavaScript is enabled, we add it through JavaScript.
+ #}
+<script>
+document.write(`
+  <button class="btn btn-sm navbar-btn search-button search-button__button" title="{{ _('Search') }}" aria-label="{{ _('Search') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
+    <i class="fa-solid fa-magnifying-glass"></i>
+  </button>
+`);
+</script>
diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/theme-switcher.html b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/theme-switcher.html
@@ -1,5 +1,11 @@
-<button class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" title="{{ _('light/dark') }}" aria-label="{{ _('light/dark') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
+{# As the theme switcher will only work when JavaScript is enabled, we add it through JavaScript.
+ #}
+<script>
+document.write(`
+  <button class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" title="{{ _('light/dark') }}" aria-label="{{ _('light/dark') }}" data-bs-placement="bottom" data-bs-toggle="tooltip">
     <span class="theme-switch" data-mode="light"><i class="fa-solid fa-sun"></i></span>
     <span class="theme-switch" data-mode="dark"><i class="fa-solid fa-moon"></i></span>
     <span class="theme-switch" data-mode="auto"><i class="fa-solid fa-circle-half-stroke"></i></span>
-</button>
+  </button>
+`);
+</script>
diff --git a/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html b/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/layout.html
@@ -1,3 +1,9 @@
+{# We redefine <html/> for "basic/layout.html" to add a default `data-theme` attribute when
+ # a default mode has been set. This also improves compatibility when JavaScript is disabled.
+ #}
+{% set html_tag %}
+<html{% if not html5_doctype %} xmlns="http://www.w3.org/1999/xhtml"{% endif %}{% if language is not none %} lang="{{ language }}"{% endif %} {% if default_mode %}data-theme="{{ default_mode }}"{% endif %}>
+{% endset %}
 {%- extends "basic/layout.html" %}
 {%- import "static/webpack-macros.html" as _webpack with context %}
 {# Metadata and asset linking #}

diff --git a/tests/test_build.py b/tests/test_build.py
@@ -201,6 +201,51 @@ def test_logo_two_images(sphinx_build_factory):
     assert "Foo Title" in index_str
 
 
+def test_primary_logo_is_light_when_no_default_mode(sphinx_build_factory):
+    """Test that the primary logo image is light
+    (and secondary, written through JavaScript, is dark)
+    when no default mode is set."""
+    # Ensure no default mode is set
+    confoverrides = {
+        "html_context": {},
+    }
+    sphinx_build = sphinx_build_factory("base", confoverrides=confoverrides).build()
+    index_html = sphinx_build.html_tree("index.html")
+    navbar_brand = index_html.select(".navbar-brand")[0]
+    assert navbar_brand.find("img", class_="only-light") is not None
+    assert navbar_brand.find("script", string=re.compile("only-dark")) is not None
+
+
+def test_primary_logo_is_light_when_default_mode_is_light(sphinx_build_factory):
+    """Test that the primary logo image is light
+    (and secondary, written through JavaScript, is dark)
+    when default mode is set to light."""
+    # Ensure no default mode is set
+    confoverrides = {
+        "html_context": {"default_mode": "light"},
+    }
+    sphinx_build = sphinx_build_factory("base", confoverrides=confoverrides).build()
+    index_html = sphinx_build.html_tree("index.html")
+    navbar_brand = index_html.select(".navbar-brand")[0]
+    assert navbar_brand.find("img", class_="only-light") is not None
+    assert navbar_brand.find("script", string=re.compile("only-dark")) is not None
+
+
+def test_primary_logo_is_dark_when_default_mode_is_dark(sphinx_build_factory):
+    """Test that the primary logo image is dark
+    (and secondary, written through JavaScript, is light)
+    when default mode is set to dark."""
+    # Ensure no default mode is set
+    confoverrides = {
+        "html_context": {"default_mode": "dark"},
+    }
+    sphinx_build = sphinx_build_factory("base", confoverrides=confoverrides).build()
+    index_html = sphinx_build.html_tree("index.html")
+    navbar_brand = index_html.select(".navbar-brand")[0]
+    assert navbar_brand.find("img", class_="only-dark") is not None
+    assert navbar_brand.find("script", string=re.compile("only-light")) is not None
+
+
 def test_logo_missing_image(sphinx_build_factory):
     """Test that a missing image will raise a warning."""
     # Test with a specified title and a dark logo
@@ -683,7 +728,11 @@ def test_theme_switcher(sphinx_build_factory, file_regression):
     """Regression test the theme switcher btn HTML"""
 
     sphinx_build = sphinx_build_factory("base").build()
-    switcher = sphinx_build.html_tree("index.html").select(".theme-switch-button")[0]
+    switcher = (
+        sphinx_build.html_tree("index.html")
+        .find(string=re.compile("theme-switch-button"))
+        .find_parent("script")
+    )
     file_regression.check(
         switcher.prettify(), basename="navbar_theme", extension=".html"
     )

diff --git a/tests/test_build/navbar_theme.html b/tests/test_build/navbar_theme.html
@@ -1,14 +1,9 @@
-<button aria-label="light/dark" class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" data-bs-placement="bottom" data-bs-toggle="tooltip" title="light/dark">
- <span class="theme-switch" data-mode="light">
-  <i class="fa-solid fa-sun">
-  </i>
- </span>
- <span class="theme-switch" data-mode="dark">
-  <i class="fa-solid fa-moon">
-  </i>
- </span>
- <span class="theme-switch" data-mode="auto">
-  <i class="fa-solid fa-circle-half-stroke">
-  </i>
- </span>
-</button>
+<script>
+ document.write(`
+  <button class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" title="light/dark" aria-label="light/dark" data-bs-placement="bottom" data-bs-toggle="tooltip">
+    <span class="theme-switch" data-mode="light"><i class="fa-solid fa-sun"></i></span>
+    <span class="theme-switch" data-mode="dark"><i class="fa-solid fa-moon"></i></span>
+    <span class="theme-switch" data-mode="auto"><i class="fa-solid fa-circle-half-stroke"></i></span>
+  </button>
+`);
+</script>
diff --git a/tests/test_build/sidebar_subpage.html b/tests/test_build/sidebar_subpage.html
@@ -36,20 +36,15 @@
   </div>
   <div class="sidebar-header-items__end">
    <div class="navbar-end-item">
-    <button aria-label="light/dark" class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" data-bs-placement="bottom" data-bs-toggle="tooltip" title="light/dark">
-     <span class="theme-switch" data-mode="light">
-      <i class="fa-solid fa-sun">
-      </i>
-     </span>
-     <span class="theme-switch" data-mode="dark">
-      <i class="fa-solid fa-moon">
-      </i>
-     </span>
-     <span class="theme-switch" data-mode="auto">
-      <i class="fa-solid fa-circle-half-stroke">
-      </i>
-     </span>
-    </button>
+    <script>
+     document.write(`
+  <button class="theme-switch-button btn btn-sm btn-outline-primary navbar-btn rounded-circle" title="light/dark" aria-label="light/dark" data-bs-placement="bottom" data-bs-toggle="tooltip">
+    <span class="theme-switch" data-mode="light"><i class="fa-solid fa-sun"></i></span>
+    <span class="theme-switch" data-mode="dark"><i class="fa-solid fa-moon"></i></span>
+    <span class="theme-switch" data-mode="auto"><i class="fa-solid fa-circle-half-stroke"></i></span>
+  </button>
+`);
+    </script>
    </div>
    <div class="navbar-end-item">
     <ul aria-label="Icon Links" class="navbar-nav" id="navbar-icon-links">