fix(sidebar): highlight an active group and keep every row's highlight full width

assets/js/critical/sidebar-active.js

@@ -16,6 +16,11 @@
 //
 // Ancestor matching is a property of the link's href, so it needs no opt-in marker in the
 // markup: every link in the nav is a candidate. Do not reintroduce one.
+//
+// Exactly one entry is marked, whether it is a leaf or a group title. The groups above it
+// are expanded but not highlighted: they are the trail to the current page, not the current
+// page, and marking them would leave a reader unable to tell which of the four rows lit up
+// is the one they are on.
 (function () {
   'use strict'
 
@@ -64,14 +69,21 @@
     var link = match.link
     if (!link) return
 
+    // A group title is not the row it heads: the row is the `.sidebar-item-group` around it,
+    // which also spans the collapse chevron beside the title. Marking only the anchor leaves
+    // nothing for the stylesheet to give an active group the treatment an active leaf gets,
+    // because a leaf *is* its own row. So the class goes on both - the anchor for the text,
+    // the row for its surface - while `aria-current` stays on the link it describes.
+    var group = link.closest('.sidebar-item-group')
+
     nav.classList.add('sidebar-no-transition')
     link.classList.add('active')
+    if (group) group.classList.add('active')
     link.setAttribute('aria-current', match.exact ? 'page' : 'true')
 
     // Expand the collapse trail: the matched group's own collapse (when the link is a group
     // title) plus every collapse ancestor within this nav instance.
     var own = null
-    var group = link.closest('.sidebar-item-group')
     if (group && group.parentElement) {
       own = group.parentElement.querySelector(':scope > .collapse')
     }

assets/scss/components/_sidebar.scss

@@ -15,6 +15,41 @@ $sidebar-collapse-clipping-transition-delayed: white-space 0s $sidebar-collapse-
 // capped by the room instead (see `.sidebar-item-label`).
 $sidebar-label-max-width: 200px !default;
 
+// The unit a nested sidebar row is indented by.
+//
+// The indent used to sit on the list that held the row - `ps-3`, or a wider class when the
+// parent group led with an icon - which moved the row's box along with its text. A row's
+// hover and active surface is its box, so a child's highlight started one indent to the
+// right of its parent's while both ended at the sidebar's edge, and the deeper the row the
+// narrower its highlight. The indent is now spent as padding on the row itself: the text
+// lands exactly where it did, and every box stays the full width of the sidebar.
+//
+// That trades a nesting list for arithmetic, because padding on a row does not accumulate
+// the way padding on nested lists does - so `assets/sidebar.html` sums the depth as it
+// recurses and publishes the total on each child list as a step count, which the table at
+// the foot of this file turns back into a length.
+//
+// A step count rather than the length itself because the markup is rendered through
+// `partialCached` and a Content-Security-Policy site cannot carry an inline `style`. And the
+// count rides a *class* rather than a data attribute, which the enumeration below exists to
+// decode: PurgeCSS runs against `hugo_stats.json`, and Hugo records only tags, classes and
+// ids there - an attribute selector is invisible to it and the whole table is dropped from
+// the production stylesheet, leaving every row at the `0rem` fallback and the nesting flat.
+// A consuming site can list an attribute under PurgeCSS's `dynamicAttributes`, but a theme
+// cannot require every consumer to have done so, and the `ps-3` this replaces was a class
+// for the same reason.
+//
+// Two steps is the plain nesting indent, the width of the `ps-3` it replaces. Three is what
+// a group with a leading icon gives its children so they line up with its label rather than
+// its icon: `fa-fw` (1.25rem) plus the icon's `me-1` gutter (0.25rem). Retuning the step
+// therefore rescales the plain indent and the icon column together, and only the plain one
+// is free to move - the other has to match the icon it hangs under.
+$sidebar-nav-indent-step: 0.5rem !default;
+
+// How deep the step table goes. Each level costs one selector, and a menu deeper than this
+// stops indenting further rather than losing the alignment it has.
+$sidebar-nav-indent-max: 16 !default;
+
 // scss-docs-start sidebar
 .sidebar {
     top: var(--navbar-offset);
@@ -96,6 +131,11 @@ html.sidebar-pre-collapsed .sidebar-collapsible .sidebar-brand {
     display: block;
     padding: 0.1875rem 0.5rem !important;
 
+    // The nesting indent, paid by the row so its box stays full width - see
+    // `$sidebar-nav-indent-step`. Restated after the shorthand above, which would
+    // otherwise reset it; `!important` only because that shorthand carries one.
+    padding-left: calc(0.5rem + var(--sidebar-indent, 0rem)) !important;
+
     &.active {
         color: $primary;
     }
@@ -112,12 +152,26 @@ html.sidebar-pre-collapsed .sidebar-collapsible .sidebar-brand {
 
     border-radius: #{$theme-border-radius};
 
+    // A group is the current page whenever its own title is what the reader navigated to,
+    // and `critical/sidebar-active.js` marks it here rather than only on the title anchor:
+    // the row's surface is this element - it spans the chevron too, which the anchor does
+    // not - so an active group otherwise could not be given the same treatment as an active
+    // leaf. The anchor is restated because `> div > a` below sets a colour of its own, which
+    // an inherited one would lose to.
+    &.active {
+        color: $primary;
+
+        > div > a {
+            color: $primary;
+        }
+    }
+
     &:hover,
     &:focus {
         color: $primary;
         background-color: tint-color($primary, 90%);
     }
-    
+
     > div > a {
         display: block;
         width: 100%;
@@ -132,8 +186,11 @@ html.sidebar-pre-collapsed .sidebar-collapsible .sidebar-brand {
     }
 }
 
+// A group header sits on the wrapper's padding rather than its own, so the nesting indent
+// lands here - which is what keeps `.sidebar-item-group` itself, the row's hover and active
+// surface, at the full width of the sidebar.
 .sidebar-item-group > div {
-    padding: 0.1875rem 0 0.1875rem 0.5rem !important;
+    padding: 0.1875rem 0 0.1875rem calc(0.5rem + var(--sidebar-indent, 0rem)) !important;
 }
 
 // Hanging indent for rows that lead with an icon.
@@ -169,7 +226,7 @@ $sidebar-item-icon-column: calc(1.25em + #{$spacer * 0.25});
     // `padding-left` needs `!important` only because the base `.sidebar-item`
     // padding above carries one.
     .sidebar-item:not(.d-flex):has(> svg:first-child, > i:first-child) {
-        padding-left: calc(0.5rem + #{$sidebar-item-icon-column}) !important;
+        padding-left: calc(0.5rem + var(--sidebar-indent, 0rem) + #{$sidebar-item-icon-column}) !important;
         text-indent: calc(-1 * #{$sidebar-item-icon-column});
     }
 
@@ -204,7 +261,7 @@ $sidebar-item-icon-column: calc(1.25em + #{$spacer * 0.25});
 // `!important` for the same reason the block above needs it: the base
 // `.sidebar-item` padding carries one.
 .sidebar-item.sidebar-item-icon-indent {
-    padding-left: calc(0.5rem + #{$sidebar-item-icon-column}) !important;
+    padding-left: calc(0.5rem + var(--sidebar-indent, 0rem) + #{$sidebar-item-icon-column}) !important;
 }
 
 // Group headers again sit on the wrapper's padding, so they reserve the column
@@ -263,6 +320,10 @@ $sidebar-item-icon-column: calc(1.25em + #{$spacer * 0.25});
 
             &.active {
                 color: $primary-text-emphasis-dark !important;
+
+                > div > a {
+                    color: $primary-text-emphasis-dark !important;
+                }
             }
 
             &:hover,
@@ -697,21 +758,20 @@ html.sidebar-pre-collapsed .sidebar-collapsible .sidebar-footer .sidebar-seconda
     transform: translateX(-50%);
 }
 
-// Align a group's children with its label rather than its icon.
+// The step table: turn the depth `assets/sidebar.html` counted back into a length.
 //
-// A leading icon pushes the parent's own label right by the icon's width plus
-// its `me-1` gutter. `ps-3` is narrower than that, so a child list using it
-// sits slightly to the *left* of the label it belongs to and the hierarchy
-// reads as broken. Groups with an icon get this class from the template in
-// place of `ps-3`; groups without one are untouched.
+// A list publishes the indent its rows are to spend, and the rows read it through this
+// custom property (see `$sidebar-nav-indent-step` for why the renderer counts steps rather
+// than emitting the length itself). The property inherits, so a row picks up the value of
+// the nearest list above it and a deeper list simply overrides it - a group header, which
+// belongs to the list its *parent* opened, is indented by that list rather than by its own.
 //
-// The template sets it rather than CSS deriving it with `:has()`. The collapse
-// trail is expanded client side, and a selector that must be re-resolved as a
-// subtree goes from display:none to visible can paint the old indent first and
-// correct it a frame later, which reads as the sidebar shifting on load.
-.sidebar-nav-icon-indent {
-    // Matches `fa-fw` (--fa-width: 1.25em) plus the icon's `me-1` gutter.
-    padding-left: calc(1.25rem + 0.25rem);
+// Scoped under `.sidebar` so the class means nothing outside a sidebar, and setting only a
+// custom property so it cannot compete with the rules that spend it.
+@for $level from 1 through $sidebar-nav-indent-max {
+    .sidebar .sidebar-nav-indent-#{$level} {
+        --sidebar-indent: #{$level * $sidebar-nav-indent-step};
+    }
 }
 
 // scss-docs-end sidebar

layouts/_partials/assets/sidebar.html

@@ -147,10 +147,24 @@
     {{- $pre := $group.pre -}}
     {{- $iconColumn := .iconColumn | default false -}}
     {{- $filename := .filename -}}
+    {{- $indent := .indent | default 0 -}}
 
     {{- /* Resolved for this group's own children, who are a list of their own */ -}}
     {{- $childIconColumn := partial "inline/sidebar/has-icons.html" $group.pages -}}
 
+    {{- /* How far this group's children hang under its label, counted in indent steps of
+           `$sidebar-nav-indent-step` (see components/_sidebar.scss). Two steps is the plain
+           nesting indent; three is that plus the column a leading icon opens, because the
+           group's own label starts an icon-width further right and its children line up with
+           the label rather than the icon.
+
+           Accumulated here rather than left to the lists to nest, because the indent is spent
+           on the rows and not on the list around them: a row that carries its own indent as
+           padding keeps its box the full width of the sidebar at every depth, so a child's
+           hover and active surface is as wide as its parent's. Nested list padding moved the
+           box instead, and the highlight narrowed one step per level. */ -}}
+    {{- $childIndent := add $indent (cond $pre 3 2) -}}
+
     {{- $doc_slug := partial "utilities/URLJoin.html" (dict "base" $baseURL "path" ($group.title | urlize)) -}}
     {{- $href := or $group.link $doc_slug -}}
     {{- /* Handle absolute paths from hierarchical menu (starting with /) */ -}}
@@ -204,13 +218,14 @@
             </button>
         </div>
         <div class="collapse" id="sidebar-collapse-{{ $index }}-{{ $level }}">
-            {{/* When this group has a leading icon its own label sits an icon-width
-                 further right, so the child list is indented to match. Emitted here
-                 rather than derived in CSS: the collapse trail is expanded client
-                 side, and a selector that has to be re-resolved as the subtree goes
-                 from display:none to visible can land its first paint on the wrong
-                 value. A class in the cached markup is correct from the start. */}}
-            <ul class="btn-toggle-nav list-unstyled fw-normal {{ if eq $level 0}} pb-1 {{ end }}{{ if $pre }}sidebar-nav-icon-indent{{ else }}ps-3{{ end }}">
+            {{/* The list publishes the indent its rows are to spend; the rows read it as a
+                 custom property and pad themselves by it. Emitted here rather than derived
+                 in CSS: the collapse trail is expanded client side, and a selector that has
+                 to be re-resolved as the subtree goes from display:none to visible can land
+                 its first paint on the wrong value. A class in the cached markup is correct
+                 from the start - and is a class rather than a data attribute so PurgeCSS can
+                 see it, see `$sidebar-nav-indent-step`. */}}
+            <ul class="btn-toggle-nav list-unstyled fw-normal{{ if eq $level 0}} pb-1{{ end }} sidebar-nav-indent-{{ $childIndent }}">
                 {{- range $childIndex, $item := $group.pages -}}
                     {{- if $item.pages -}}
                         {{/* Compose a unique index path through the recursion so
@@ -226,6 +241,7 @@
                             "pre"        $item.pre
                             "iconColumn" $childIconColumn
                             "filename"   $filename
+                            "indent"     $childIndent
                             )
                         }}
                     {{- else -}}

tests/templates/layouts/index.html

@@ -388,3 +388,94 @@
 {{- end }}
 
 SIDEBAR ICON COLUMN FAILURES: {{ $sbFail }}
+
+{{- /*
+    Assertions for the nesting indent in assets/sidebar.html.
+
+    A nested row's indent used to sit on the list that holds it, which also moved the row's
+    box - so its hover and active surface started at the indent while its right edge stayed
+    at the sidebar's, and the highlight of a child read as narrower than its parent's. The
+    indent now travels as a step count on the child list and is spent as padding on the rows
+    themselves, which leaves every row box full width whatever its depth.
+
+    The count is what these cases pin, since it is the only place the depth arithmetic lives:
+    a group adds two steps for its children, or three when it carries a leading icon and its
+    own label therefore starts an icon-width further right. Nesting accumulates, so a case's
+    `want` reads as the sequence of lists the renderer opens, outermost first.
+
+    It has to be a class, and these cases hold that too by matching one. PurgeCSS runs against
+    `hugo_stats.json`, where Hugo records only tags, classes and ids - so the same count on a
+    data attribute is invisible to it, the step table is dropped from the production
+    stylesheet, and every row silently falls back to no indent at all. Only a minified build
+    shows it, which is why the shape is pinned here rather than left to the eye.
+*/ -}}
+{{- $inFail := 0 -}}
+{{- $inPage := site.GetPage "/blog/without-exact" -}}
+{{- $inCases := slice
+  (dict "case" "a flat menu opens no nested list and needs no indent"
+        "menu" `
+- title: Home
+  link: /
+- title: With Exact
+  link: /blog/with-exact/`
+        "want" slice)
+  (dict "case" "a group indents its children by two steps"
+        "menu" `
+- title: Blog
+  link: /blog/
+  pages:
+    - title: With Exact
+      link: /blog/with-exact/`
+        "want" (slice "2"))
+  (dict "case" "a group with an icon indents its children past the icon column"
+        "menu" `
+- title: Blog
+  link: /blog/
+  pre: fas house
+  pages:
+    - title: With Exact
+      link: /blog/with-exact/`
+        "want" (slice "3"))
+  (dict "case" "nesting accumulates the indent"
+        "menu" `
+- title: Blog
+  link: /blog/
+  pages:
+    - title: With Exact
+      link: /blog/with-exact/
+      pages:
+        - title: Toc Override
+          link: /blog/toc-override/`
+        "want" (slice "2" "4"))
+  (dict "case" "an icon group nested under a plain one adds its own column"
+        "menu" `
+- title: Blog
+  link: /blog/
+  pages:
+    - title: With Exact
+      link: /blog/with-exact/
+      pre: fas house
+      pages:
+        - title: Toc Override
+          link: /blog/toc-override/`
+        "want" (slice "2" "5"))
+-}}
+{{- range $inCases -}}
+{{- $menu := .menu | transform.Unmarshal -}}
+{{- $html := partial "assets/sidebar.html" (dict "page" $inPage "menu" $menu) -}}
+{{- $got := slice -}}
+{{- range (findRE `class="[^"]*\bsidebar-nav-indent-[0-9]+\b[^"]*"` $html) -}}
+    {{- $got = $got | append (index (findRE `sidebar-nav-indent-([0-9]+)` . 1) 0 | strings.TrimPrefix "sidebar-nav-indent-") -}}
+{{- end -}}
+{{- if eq (delimit $got "|") (delimit .want "|") }}
+PASS {{ .case }}
+{{- else }}
+FAIL {{ .case }}
+     want={{ delimit .want "|" }}
+     got ={{ delimit $got "|" }}
+{{- $inFail = add $inFail 1 -}}
+{{- errorf "sidebar indent mismatch: case=%q want=%q got=%q" .case (delimit .want "|") (delimit $got "|") -}}
+{{- end -}}
+{{- end }}
+
+SIDEBAR INDENT FAILURES: {{ $inFail }}