Merge pull request #2725 from GuillaumeGomez/search-collapsed

Hide the sidebar when collapsed to prevent browser search to find text from it

Author: ehuss

src/front-end/css/chrome.css

@@ -530,7 +530,6 @@ html:not(.sidebar-resizing) .sidebar {
 /* sidebar-hidden */
 #sidebar-toggle-anchor:not(:checked) ~ .sidebar {
     transform: translateX(calc(0px - var(--sidebar-width) - var(--sidebar-resize-indicator-width)));
-    z-index: -1;
 }
 [dir=rtl] #sidebar-toggle-anchor:not(:checked) ~ .sidebar {
     transform: translateX(calc(var(--sidebar-width) + var(--sidebar-resize-indicator-width)));

src/front-end/js/book.js

@@ -519,12 +519,35 @@ aria-label="Show hidden lines"></button>';
     const sidebar = document.getElementById('sidebar');
     const sidebarLinks = document.querySelectorAll('#sidebar a');
     const sidebarToggleButton = document.getElementById('sidebar-toggle');
-    const sidebarToggleAnchor = document.getElementById('sidebar-toggle-anchor');
     const sidebarResizeHandle = document.getElementById('sidebar-resize-handle');
+    const sidebarCheckbox = document.getElementById('sidebar-toggle-anchor');
     let firstContact = null;
 
+
+    /* Because we cannot change the `display` using only CSS after/before the transition, we
+       need JS to do it. We change the display to prevent the browsers search to find text inside
+       the collapsed sidebar. */
+    if (!document.documentElement.classList.contains('sidebar-visible')) {
+        sidebar.style.display = 'none';
+    }
+    sidebar.addEventListener('transitionend', () => {
+        /* We only change the display to "none" if we're collapsing the sidebar. */
+        if (!sidebarCheckbox.checked) {
+            sidebar.style.display = 'none';
+        }
+    });
+    sidebarToggleButton.addEventListener('click', () => {
+        /* To allow the sidebar expansion animation, we first need to put back the display. */
+        if (!sidebarCheckbox.checked) {
+            sidebar.style.display = '';
+            // Workaround for Safari skipping the animation when changing
+            // `display` and a transform in the same event loop. This forces a
+            // reflow after updating the display.
+            sidebar.offsetHeight;
+        }
+    });
+
     function showSidebar() {
-        body.classList.remove('sidebar-hidden');
         body.classList.add('sidebar-visible');
         Array.from(sidebarLinks).forEach(function(link) {
             link.setAttribute('tabIndex', 0);
@@ -540,7 +563,6 @@ aria-label="Show hidden lines"></button>';
 
     function hideSidebar() {
         body.classList.remove('sidebar-visible');
-        body.classList.add('sidebar-hidden');
         Array.from(sidebarLinks).forEach(function(link) {
             link.setAttribute('tabIndex', -1);
         });
@@ -554,8 +576,8 @@ aria-label="Show hidden lines"></button>';
     }
 
     // Toggle sidebar
-    sidebarToggleAnchor.addEventListener('change', function sidebarToggle() {
-        if (sidebarToggleAnchor.checked) {
+    sidebarCheckbox.addEventListener('change', function sidebarToggle() {
+        if (sidebarCheckbox.checked) {
             const current_width = parseInt(
                 document.documentElement.style.getPropertyValue('--sidebar-target-width'), 10);
             if (current_width < 150) {
@@ -579,7 +601,7 @@ aria-label="Show hidden lines"></button>';
         if (pos < 20) {
             hideSidebar();
         } else {
-            if (body.classList.contains('sidebar-hidden')) {
+            if (!body.classList.contains('sidebar-visible')) {
                 showSidebar();
             }
             pos = Math.min(pos, window.innerWidth - 100);
@@ -765,7 +787,7 @@ aria-label="Show hidden lines"></button>';
         let scrollTop = document.scrollingElement.scrollTop;
         let prevScrollTop = scrollTop;
         const minMenuY = -menu.clientHeight - 50;
-        // When the script loads, the page can be at any scroll (e.g. if you reforesh it).
+        // When the script loads, the page can be at any scroll (e.g. if you refresh it).
         menu.style.top = scrollTop + 'px';
         // Same as parseInt(menu.style.top.slice(0, -2), but faster
         let topCache = menu.style.top.slice(0, -2);

src/front-end/templates/index.hbs

@@ -116,10 +116,13 @@
                 sidebar = sidebar || 'visible';
             } else {
                 sidebar = 'hidden';
+                sidebar_toggle.checked = false;
+            }
+            if (sidebar === 'visible') {
+                sidebar_toggle.checked = true;
+            } else {
+                html.classList.remove('sidebar-visible');
             }
-            sidebar_toggle.checked = sidebar === 'visible';
-            html.classList.remove('sidebar-visible');
-            html.classList.add("sidebar-" + sidebar);
         </script>
 
         <nav id="sidebar" class="sidebar" aria-label="Table of contents">

tests/gui/sidebar.goml

@@ -7,42 +7,39 @@ set-window-size: (1100, 600)
 reload:
 
 store-value: (content_indent, 308)
+store-value: (sidebar_storage_value, "mdbook-sidebar")
+store-value: (sidebar_storage_hidden_value, "hidden")
+store-value: (sidebar_storage_displayed_value, "visible")
 
 define-function: (
     "hide-sidebar",
     [],
     block {
-        // The content should be "moved" to the right because of the sidebar.
-        assert-css: ("#sidebar", {"transform": "none"})
         assert-position: ("#page-wrapper", {"x": |content_indent|})
 
         // We now hide the sidebar.
         click: "#sidebar-toggle"
-        wait-for: "body.sidebar-hidden"
-        // `transform` is 0.3s so we need to wait a bit (0.5s) to ensure the animation is done.
-        wait-for: 5000
-        assert-css-false: ("#sidebar", {"transform": "none"})
-        // The page content should now be on the left.
-        assert-position: ("#page-wrapper", {"x": 0})
+        wait-for-css: ("#sidebar", {"display": "none"})
+        assert-local-storage: {|sidebar_storage_value|: |sidebar_storage_hidden_value|}
     },
 )
 
 define-function: (
     "show-sidebar",
     [],
     block {
-        // The page content should be on the left and the sidebar "moved out".
-        assert-css: ("#sidebar", {"transform": "matrix(1, 0, 0, 1, -308, 0)"})
+        assert-css: ("#sidebar", {"display": "none"})
         assert-position: ("#page-wrapper", {"x": 0})
 
         // We expand the sidebar.
         click: "#sidebar-toggle"
-        wait-for: "body.sidebar-visible"
+        wait-for-css-false: ("#sidebar", {"display": "none"})
         // `transform` is 0.3s so we need to wait a bit (0.5s) to ensure the animation is done.
         wait-for: 5000
         assert-css-false: ("#sidebar", {"transform": "matrix(1, 0, 0, 1, -308, 0)"})
         // The page content should be moved to the right.
         assert-position: ("#page-wrapper", {"x": |content_indent|})
+        assert-local-storage: {|sidebar_storage_value|: |sidebar_storage_displayed_value|}
     },
 )
 
@@ -54,3 +51,13 @@ set-window-size: (900, 600)
 reload:
 call-function: ("show-sidebar", {})
 call-function: ("hide-sidebar", {})
+
+// We now test that if the sidebar is considered open and we reload the page, since
+// the width is small, it will still be collapsed.
+set-local-storage: {|sidebar_storage_value|: |sidebar_storage_displayed_value|}
+reload:
+// The stored value shouldn't have changed.
+assert-local-storage: {|sidebar_storage_value|: |sidebar_storage_displayed_value|}
+// But the sidebar should be hidden anyway.
+assert-css: ("#sidebar", {"display": "none"})
+assert-position: ("#page-wrapper", {"x": 0})