Fix More dropdown (html, aria, bootstrap classes) (#1414)

* Fix More dropdown (html, aria, bootstrap classes)

* update dropdowns when user changes light/dark theme

* fix padding

* fix active, hover - colors, transition

* Update src/pydata_sphinx_theme/toctree.py

* lint

* turn off all transitions on .nav-link

* fix test

* make test more dry, readable

src/pydata_sphinx_theme/assets/scripts/pydata-sphinx-theme.js

@@ -38,6 +38,15 @@ function setTheme(mode) {
   document.documentElement.dataset.mode = mode;
   var theme = mode == "auto" ? colorScheme : mode;
   document.documentElement.dataset.theme = theme;
+  // TODO: remove this line after Bootstrap upgrade
+  // v5.3 has a colors mode: https://getbootstrap.com/docs/5.3/customize/color-modes/
+  document.querySelectorAll(".dropdown-menu").forEach((el) => {
+    if (theme === "dark") {
+      el.classList.add("dropdown-menu-dark");
+    } else {
+      el.classList.remove("dropdown-menu-dark");
+    }
+  });
 
   // save mode and theme
   localStorage.setItem("mode", mode);

src/pydata_sphinx_theme/assets/styles/sections/_header.scss

@@ -107,10 +107,21 @@
         border: 1px solid var(--pst-color-border);
         box-shadow: 0 0 0.3rem 0.1rem var(--pst-color-shadow);
         background-color: var(--pst-color-on-background);
-        padding: 0.5rem 1rem;
+        padding: 0.5rem 0;
         margin: 0.5rem 0;
         min-width: 20rem;
 
+        .dropdown-item {
+          // Give the items in the dropdown some breathing room but let the hit
+          // and hover area of the items extend to the edges of the menu
+          padding: 0.25rem 1.5rem;
+
+          // Override Bootstrap
+          &:focus:not(:hover):not(:active) {
+            background-color: inherit;
+          }
+        }
+
         // Hide the menu unless show has been clicked
         &:not(.show) {
           display: none;
@@ -130,6 +141,9 @@
 .nav-link {
   @include link-style-hover;
 
+  // Override Bootstrap
+  transition: none;
+
   &.nav-external:after {
     font: var(--fa-font-solid);
     content: var(--pst-icon-external-link);

src/pydata_sphinx_theme/assets/styles/variables/_bootstrap.scss

@@ -15,3 +15,10 @@ $grid-breakpoints: (
   lg: 960px,
   xl: 1200px,
 );
+
+$dropdown-link-hover-bg: var(--pst-color-surface);
+// --pst-color-surface can also be assigned to the dark variant because it is
+// scoped to different values depending on light/dark theme
+$dropdown-dark-link-hover-bg: var(--pst-color-surface);
+$dropdown-link-active-bg: var(--pst-color-surface);
+$dropdown-dark-link-active-bg: var(--pst-color-surface);

src/pydata_sphinx_theme/toctree.py

@@ -137,18 +137,24 @@ def generate_header_nav_html(n_links_before_dropdown: int = 5) -> str:
         out = "\n".join(links_solo)
 
         # Wrap the final few header items in a "more" dropdown
-        links_dropdown = links_html[n_links_before_dropdown:]
+        links_dropdown = [
+            # 🐲 brittle code, relies on the assumption that the code above
+            # gives each link in the nav a `nav-link` CSS class
+            html.replace("nav-link", "nav-link dropdown-item")
+            for html in links_html[n_links_before_dropdown:]
+        ]
+
         if links_dropdown:
             links_dropdown_html = "\n".join(links_dropdown)
             out += f"""
-            <div class="nav-item dropdown">
-                <button class="btn dropdown-toggle nav-item" type="button" data-bs-toggle="dropdown" aria-haspopup="true" aria-expanded="false">
+            <li class="nav-item dropdown">
+                <button class="btn dropdown-toggle nav-item" type="button" data-bs-toggle="dropdown" aria-expanded="false" aria-controls="pst-header-nav-more-links">
                     More
                 </button>
-                <div class="dropdown-menu">
+                <ul id="pst-header-nav-more-links" class="dropdown-menu">
                     {links_dropdown_html}
-                </div>
-            </div>
+                </ul>
+            </li>
             """
 
         return out

tests/test_build.py

@@ -320,21 +320,17 @@ def test_navbar_header_dropdown(sphinx_build_factory, n_links) -> None:
     sphinx_build = sphinx_build_factory("base", confoverrides=confoverrides).build()
     index_html = sphinx_build.html_tree("index.html")
     navbar = index_html.select("ul.bd-navbar-elements")[0]
+    dropdowns = navbar.select("li.dropdown")
+    standalone_links = navbar.select(".navbar-nav > li.nav-item:not(.dropdown)")
     if n_links == 0:
         # There should be *only* a dropdown and no standalone links
-        assert navbar.select("div.dropdown") and not navbar.select(
-            ".navbar-nav > li.nav-item"
-        )
+        assert len(dropdowns) == 1 and not standalone_links
     if n_links == 4:
-        # There should be at least one standalone link, and a dropdown
-        assert navbar.select(".navbar-nav > li.nav-item") and navbar.select(
-            "div.dropdown"
-        )
+        # There should be `n_links` standalone links, and a dropdown
+        assert len(standalone_links) == n_links and len(dropdowns) == 1
     if n_links == 8:
         # There should be no dropdown and only standalone links
-        assert navbar.select(".navbar-nav > li.nav-item") and not navbar.select(
-            "div.dropdown"
-        )
+        assert standalone_links and not dropdowns
 
 
 def test_sidebars_captions(sphinx_build_factory, file_regression) -> None: