Merge pull request #2893 from ehuss/fix-fold-sub-chapter

Fix heading nav with folded chapters

Author: ehuss

crates/mdbook-html/front-end/css/chrome.css

@@ -571,17 +571,18 @@ html:not(.sidebar-resizing) .sidebar {
     line-height: 2.2em;
 }
 
-.chapter ol {
-    width: 100%;
+.chapter li {
+    color: var(--sidebar-non-existant);
 }
 
-.chapter li {
+/* This is a span wrapping the chapter link and the fold chevron. */
+.chapter-link-wrapper {
+    /* Used to position the chevron to the right, allowing the text to wrap before it. */
     display: flex;
-    color: var(--sidebar-non-existant);
 }
+
 .chapter li a {
-    display: block;
-    padding: 0;
+    /* Remove underlines. */
     text-decoration: none;
     color: var(--sidebar-fg);
 }
@@ -594,21 +595,22 @@ html:not(.sidebar-resizing) .sidebar {
     color: var(--sidebar-active);
 }
 
-.chapter li > a.toggle {
+/* This is the toggle chevron. */
+.chapter-fold-toggle {
     cursor: pointer;
-    display: block;
+    /* Positions the chevron to the side. */
     margin-inline-start: auto;
     padding: 0 10px;
     user-select: none;
     opacity: 0.68;
 }
 
-.chapter li > a.toggle div {
+.chapter-fold-toggle div {
     transition: transform 0.5s;
 }
 
 /* collapse the section */
-.chapter li:not(.expanded) + li > ol {
+.chapter li:not(.expanded) > ol {
     display: none;
 }
 
@@ -617,10 +619,12 @@ html:not(.sidebar-resizing) .sidebar {
     margin-block-start: 0.6em;
 }
 
-.chapter li.expanded > a.toggle div {
+/* When expanded, rotate the chevron to point down. */
+.chapter li.expanded > span > .chapter-fold-toggle div {
     transform: rotate(90deg);
 }
 
+/* Horizontal line in chapter list. */
 .spacer {
     width: 100%;
     height: 3px;
@@ -630,6 +634,7 @@ html:not(.sidebar-resizing) .sidebar {
     background-color: var(--sidebar-spacer);
 }
 
+/* On touch devices, add more vertical spacing to make it easier to tap links. */
 @media (-moz-touch-enabled: 1), (pointer: coarse) {
     .chapter li a { padding: 5px 0; }
     .spacer { margin: 10px 0; }
@@ -741,7 +746,6 @@ html:not(.sidebar-resizing) .sidebar {
     content: '';
     position: absolute;
     left: -16px;
-    top: 0;
     margin-top: 10px;
     width: 8px;
     height: 8px;

crates/mdbook-html/front-end/templates/toc.js.hbs

@@ -29,14 +29,9 @@ class MDBookSidebarScrollbox extends HTMLElement {
                 && current_page.endsWith('/index.html')) {
                 link.classList.add('active');
                 let parent = link.parentElement;
-                if (parent && parent.classList.contains('chapter-item')) {
-                    parent.classList.add('expanded');
-                }
                 while (parent) {
-                    if (parent.tagName === 'LI' && parent.previousElementSibling) {
-                        if (parent.previousElementSibling.classList.contains('chapter-item')) {
-                            parent.previousElementSibling.classList.add('expanded');
-                        }
+                    if (parent.tagName === 'LI' && parent.classList.contains('chapter-item')) {
+                        parent.classList.add('expanded');
                     }
                     parent = parent.parentElement;
                 }
@@ -62,9 +57,9 @@ class MDBookSidebarScrollbox extends HTMLElement {
             }
         }
         // Toggle buttons
-        const sidebarAnchorToggles = document.querySelectorAll('#mdbook-sidebar a.toggle');
+        const sidebarAnchorToggles = document.querySelectorAll('.chapter-fold-toggle');
         function toggleSection(ev) {
-            ev.currentTarget.parentElement.classList.toggle('expanded');
+            ev.currentTarget.parentElement.parentElement.classList.toggle('expanded');
         }
         Array.from(sidebarAnchorToggles).forEach(el => {
             el.addEventListener('click', toggleSection);
@@ -237,17 +232,12 @@ window.customElements.define('mdbook-sidebar-scrollbox', MDBookSidebarScrollbox)
     // be expanded.
     function updateHeaderExpanded(currentA) {
         // Add expanded to all header-item li ancestors.
-        let current = currentA.parentElement.parentElement.parentElement;
-        while (current.tagName === 'LI') {
-            const prevSibling = current.previousElementSibling;
-            if (prevSibling !== null
-                && prevSibling.tagName === 'LI'
-                && prevSibling.classList.contains('header-item')) {
-                prevSibling.classList.add('expanded');
-                current = prevSibling.parentElement.parentElement;
-            } else {
-                break;
+        let current = currentA.parentElement;
+        while (current) {
+            if (current.tagName === 'LI' && current.classList.contains('header-item')) {
+                current.classList.add('expanded');
             }
+            current = current.parentElement;
         }
     }
 
@@ -343,19 +333,6 @@ window.customElements.define('mdbook-sidebar-scrollbox', MDBookSidebarScrollbox)
         if (activeSection === null) {
             return;
         }
-        const activeItem = activeSection.parentElement;
-        const activeList = activeItem.parentElement;
-
-        // Build a tree of headers in the sidebar.
-        const rootLi = document.createElement('li');
-        rootLi.classList.add('header-item');
-        rootLi.classList.add('expanded');
-        const rootOl = document.createElement('ol');
-        rootOl.classList.add('section');
-        rootLi.appendChild(rootOl);
-        const stack = [{ level: 0, ol: rootOl }];
-        // The level where it will start folding deeply nested headers.
-        const foldLevel = 3;
 
         const main = document.getElementsByTagName('main')[0];
         headers = Array.from(main.querySelectorAll('h2, h3, h4, h5, h6'))
@@ -365,57 +342,90 @@ window.customElements.define('mdbook-sidebar-scrollbox', MDBookSidebarScrollbox)
             return;
         }
 
+        // Build a tree of headers in the sidebar.
+
+        const stack = [];
+
+        const firstLevel = parseInt(headers[0].tagName.charAt(1));
+        for (let i = 1; i < firstLevel; i++) {
+            const ol = document.createElement('ol');
+            ol.classList.add('section');
+            if (stack.length > 0) {
+                stack[stack.length - 1].ol.appendChild(ol);
+            }
+            stack.push({level: i + 1, ol: ol});
+        }
+
+        // The level where it will start folding deeply nested headers.
+        const foldLevel = 3;
+
         for (let i = 0; i < headers.length; i++) {
             const header = headers[i];
             const level = parseInt(header.tagName.charAt(1));
+
+            const currentLevel = stack[stack.length - 1].level;
+            if (level > currentLevel) {
+                // Begin nesting to this level.
+                for (let nextLevel = currentLevel + 1; nextLevel <= level; nextLevel++) {
+                    const ol = document.createElement('ol');
+                    ol.classList.add('section');
+                    const last = stack[stack.length - 1];
+                    const lastChild = last.ol.lastChild;
+                    // Handle the case where jumping more than one nesting
+                    // level, which doesn't have a list item to place this new
+                    // list inside of.
+                    if (lastChild) {
+                        lastChild.appendChild(ol);
+                    } else {
+                        last.ol.appendChild(ol);
+                    }
+                    stack.push({level: nextLevel, ol: ol});
+                }
+            } else if (level < currentLevel) {
+                while (stack.length > 1 && stack[stack.length - 1].level >= level) {
+                    stack.pop();
+                }
+            }
+
             const li = document.createElement('li');
             li.classList.add('header-item');
             li.classList.add('expanded');
             if (level < foldLevel) {
                 li.classList.add('expanded');
             }
+            const span = document.createElement('span');
+            span.classList.add('chapter-link-wrapper');
             const a = document.createElement('a');
+            span.appendChild(a);
             a.href = '#' + header.id;
             a.classList.add('header-in-summary');
             a.innerHTML = header.children[0].innerHTML;
             a.addEventListener('click', headerThresholdClick);
-            li.appendChild(a);
             const nextHeader = headers[i + 1];
             if (nextHeader !== undefined) {
                 const nextLevel = parseInt(nextHeader.tagName.charAt(1));
                 if (nextLevel > level && level >= foldLevel) {
-                    const div = document.createElement('div');
-                    div.textContent = '❱';
                     const toggle = document.createElement('a');
-                    toggle.classList.add('toggle');
+                    toggle.classList.add('chapter-fold-toggle');
                     toggle.classList.add('header-toggle');
-                    toggle.appendChild(div);
                     toggle.addEventListener('click', () => {
                         li.classList.toggle('expanded');
                     });
-                    li.appendChild(toggle);
+                    const toggleDiv = document.createElement('div');
+                    toggleDiv.textContent = '❱';
+                    toggle.appendChild(toggleDiv);
+                    span.appendChild(toggle);
                     headerToggles.push(li);
                 }
             }
-
-            // Find the appropriate parent level.
-            while (stack.length > 1 && stack[stack.length - 1].level >= level) {
-                stack.pop();
-            }
+            li.appendChild(span);
 
             const currentParent = stack[stack.length - 1];
             currentParent.ol.appendChild(li);
-
-            // Create new nested ol for potential children.
-            const nestedOl = document.createElement('ol');
-            nestedOl.classList.add('section');
-            const nestedLi = document.createElement('li');
-            nestedLi.appendChild(nestedOl);
-            currentParent.ol.appendChild(nestedLi);
-            stack.push({ level: level, ol: nestedOl });
         }
 
-        activeList.insertBefore(rootLi, activeItem.nextSibling);
+        const activeItemSpan = activeSection.parentElement;
+        activeItemSpan.after(stack[0].ol);
     });
 
     document.addEventListener('DOMContentLoaded', reloadCurrentHeader);

crates/mdbook-html/src/html_handlebars/helpers/toc.rs

@@ -57,39 +57,45 @@ impl HelperDef for RenderToc {
         out.write("<ol class=\"chapter\">")?;
 
         let mut current_level = 1;
+        let mut first = true;
 
         for item in chapters {
-            let (_section, level) = if let Some(s) = item.get("section") {
-                (s.as_str(), s.matches('.').count())
-            } else {
-                ("", 1)
-            };
+            let level = item
+                .get("section")
+                .map(|s| s.matches('.').count())
+                .unwrap_or(1);
 
             // Expand if folding is disabled, or if levels that are larger than this would not
             // be folded.
             let is_expanded = !fold_enable || level - 1 < (fold_level as usize);
 
             match level.cmp(&current_level) {
                 Ordering::Greater => {
-                    while level > current_level {
-                        out.write("<li>")?;
-                        out.write("<ol class=\"section\">")?;
-                        current_level += 1;
-                    }
-                    write_li_open_tag(out, is_expanded, false)?;
+                    // There is an assumption that when descending, it can
+                    // only go one level down at a time. This should be
+                    // enforced by the nature of markdown lists and the
+                    // summary parser.
+                    assert_eq!(level, current_level + 1);
+                    current_level += 1;
+                    out.write("<ol class=\"section\">")?;
+                    write_li_open_tag(out, is_expanded)?;
                 }
                 Ordering::Less => {
                     while level < current_level {
-                        out.write("</ol>")?;
                         out.write("</li>")?;
+                        out.write("</ol>")?;
                         current_level -= 1;
                     }
-                    write_li_open_tag(out, is_expanded, false)?;
+                    write_li_open_tag(out, is_expanded)?;
                 }
                 Ordering::Equal => {
-                    write_li_open_tag(out, is_expanded, !item.contains_key("section"))?;
+                    if !first {
+                        out.write("</li>")?;
+                    }
+                    write_li_open_tag(out, is_expanded)?;
                 }
             }
+            first = false;
 
             // Spacer
             if item.contains_key("spacer") {
@@ -105,6 +111,8 @@ impl HelperDef for RenderToc {
                 continue;
             }
 
+            out.write("<span class=\"chapter-link-wrapper\">")?;
+
             // Link
             let path_exists = match item.get("path") {
                 Some(path) if !path.is_empty() => {
@@ -121,7 +129,7 @@ impl HelperDef for RenderToc {
                     true
                 }
                 _ => {
-                    out.write("<div>")?;
+                    out.write("<span>")?;
                     false
                 }
             };
@@ -142,41 +150,35 @@ impl HelperDef for RenderToc {
             if path_exists {
                 out.write("</a>")?;
             } else {
-                out.write("</div>")?;
+                out.write("</span>")?;
             }
 
             // Render expand/collapse toggle
             if let Some(flag) = item.get("has_sub_items") {
                 let has_sub_items = flag.parse::<bool>().unwrap_or_default();
                 if fold_enable && has_sub_items {
-                    out.write("<a class=\"toggle\"><div>❱</div></a>")?;
+                    // The <div> here is to manage rotating the element when
+                    // the chapter title is long and word-wraps.
+                    out.write("<a class=\"chapter-fold-toggle\"><div>❱</div></a>")?;
                 }
             }
-            out.write("</li>")?;
+            out.write("</span>")?;
         }
-        while current_level > 1 {
-            out.write("</ol>")?;
+        while current_level > 0 {
             out.write("</li>")?;
+            out.write("</ol>")?;
             current_level -= 1;
         }
 
-        out.write("</ol>")?;
         Ok(())
     }
 }
 
-fn write_li_open_tag(
-    out: &mut dyn Output,
-    is_expanded: bool,
-    is_affix: bool,
-) -> Result<(), std::io::Error> {
+fn write_li_open_tag(out: &mut dyn Output, is_expanded: bool) -> Result<(), std::io::Error> {
     let mut li = String::from("<li class=\"chapter-item ");
     if is_expanded {
         li.push_str("expanded ");
     }
-    if is_affix {
-        li.push_str("affix ");
-    }
     li.push_str("\">");
     out.write(&li)
 }

tests/gui/books/heading-nav-folded/book.toml

@@ -0,0 +1,6 @@
+[book]
+title = "heading-nav-folded"
+
+[output.html.fold]
+enable = true
+level = 0