From 9d78242fa7f9408531a49e440e89480e3ade75e6 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 30 Jun 2022 17:28:29 -0700 Subject: [PATCH 1/5] Improve scroll locking in the rustdoc sidebars This commit ports the scroll locking behavior from the source sidebar to the regular sidebar, and also fixes some bad behavior where opening a "mobile" sidebar, and growing the viewport so that the "desktop" mode without scroll locking is activated, could potentially leave the page stuck. --- src/librustdoc/html/static/js/main.js | 46 +++++++++++++++++-- .../html/static/js/source-script.js | 22 +++++++-- src/librustdoc/html/static/js/storage.js | 2 + .../rustdoc-gui/sidebar-mobile-scroll.goml | 31 +++++++++++++ .../sidebar-source-code-display.goml | 7 +++ 5 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 src/test/rustdoc-gui/sidebar-mobile-scroll.goml diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index 70dbfd4442540..084cb082dd63a 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -382,8 +382,7 @@ function loadCss(cssFileName) { function onHashChange(ev) { // If we're in mobile mode, we should hide the sidebar in any case. - const sidebar = document.getElementsByClassName("sidebar")[0]; - removeClass(sidebar, "shown"); + hideSidebar(); handleHashes(ev); } @@ -764,11 +763,50 @@ function loadCss(cssFileName) { }); }()); + let oldSidebarScrollPosition = null; + + function showSidebar() { + if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { + // This is to keep the scroll position on mobile. + oldSidebarScrollPosition = window.scrollY; + document.body.style.width = `${document.body.offsetWidth}px`; + document.body.style.position = "fixed"; + document.body.style.top = `-${oldSidebarScrollPosition}px`; + document.querySelector(".mobile-topbar").style.top = `${oldSidebarScrollPosition}px`; + document.querySelector(".mobile-topbar").style.position = "relative"; + } else { + oldSidebarScrollPosition = null; + } + const sidebar = document.getElementsByClassName("sidebar")[0]; + addClass(sidebar, "shown"); + } + function hideSidebar() { + if (oldSidebarScrollPosition !== null) { + // This is to keep the scroll position on mobile. + document.body.style.width = ""; + document.body.style.position = ""; + document.body.style.top = ""; + document.querySelector(".mobile-topbar").style.top = ""; + document.querySelector(".mobile-topbar").style.position = ""; + // The scroll position is lost when resetting the style, hence why we store it in + // `oldSidebarScrollPosition`. + window.scrollTo(0, oldSidebarScrollPosition); + oldSidebarScrollPosition = null; + } const sidebar = document.getElementsByClassName("sidebar")[0]; removeClass(sidebar, "shown"); } + window.addEventListener("resize", () => { + if (window.innerWidth >= window.RUSTDOC_MOBILE_BREAKPOINT && + oldSidebarScrollPosition !== null) { + // If the user opens the sidebar in "mobile" mode, and then grows the browser window, + // we need to switch away from mobile mode and make the main content area scrollable. + hideSidebar(); + } + }); + function handleClick(id, f) { const elem = document.getElementById(id); if (elem) { @@ -811,9 +849,9 @@ function loadCss(cssFileName) { sidebar_menu_toggle.addEventListener("click", () => { const sidebar = document.getElementsByClassName("sidebar")[0]; if (!hasClass(sidebar, "shown")) { - addClass(sidebar, "shown"); + showSidebar(); } else { - removeClass(sidebar, "shown"); + hideSidebar(); } }); } diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index acb1d8d7b5c8d..0f5217dac9705 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -10,7 +10,7 @@ (function() { const rootPath = document.getElementById("rustdoc-vars").attributes["data-root-path"].value; -let oldScrollPosition = 0; +let oldScrollPosition = null; function createDirEntry(elem, parent, fullPath, hasFoundFile) { const name = document.createElement("div"); @@ -66,23 +66,26 @@ function createDirEntry(elem, parent, fullPath, hasFoundFile) { function toggleSidebar() { const child = this.children[0]; if (child.innerText === ">") { - if (window.innerWidth < 701) { + if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { // This is to keep the scroll position on mobile. oldScrollPosition = window.scrollY; document.body.style.position = "fixed"; document.body.style.top = `-${oldScrollPosition}px`; + } else { + oldScrollPosition = null; } addClass(document.documentElement, "source-sidebar-expanded"); child.innerText = "<"; updateLocalStorage("source-sidebar-show", "true"); } else { - if (window.innerWidth < 701) { + if (oldScrollPosition !== null) { // This is to keep the scroll position on mobile. document.body.style.position = ""; document.body.style.top = ""; // The scroll position is lost when resetting the style, hence why we store it in - // `oldScroll`. + // `oldScrollPosition`. window.scrollTo(0, oldScrollPosition); + oldScrollPosition = null; } removeClass(document.documentElement, "source-sidebar-expanded"); child.innerText = ">"; @@ -90,6 +93,17 @@ function toggleSidebar() { } } +window.addEventListener("resize", () => { + if (window.innerWidth >= window.RUSTDOC_MOBILE_BREAKPOINT && oldScrollPosition !== null) { + // If the user opens the sidebar in "mobile" mode, and then grows the browser window, + // we need to switch away from mobile mode and make the main content area scrollable. + document.body.style.position = ""; + document.body.style.top = ""; + window.scrollTo(0, oldScrollPosition); + oldScrollPosition = null; + } +}); + function createSidebarToggle() { const sidebarToggle = document.createElement("div"); sidebarToggle.id = "sidebar-toggle"; diff --git a/src/librustdoc/html/static/js/storage.js b/src/librustdoc/html/static/js/storage.js index 1c4c88344888c..3b0dd282ff78e 100644 --- a/src/librustdoc/html/static/js/storage.js +++ b/src/librustdoc/html/static/js/storage.js @@ -9,6 +9,8 @@ const darkThemes = ["dark", "ayu"]; window.currentTheme = document.getElementById("themeStyle"); window.mainTheme = document.getElementById("mainThemeStyle"); +window.RUSTDOC_MOBILE_BREAKPOINT = 701; + const settingsDataset = (function() { const settingsElement = document.getElementById("default-settings"); if (settingsElement === null) { diff --git a/src/test/rustdoc-gui/sidebar-mobile-scroll.goml b/src/test/rustdoc-gui/sidebar-mobile-scroll.goml new file mode 100644 index 0000000000000..b3bcea253388f --- /dev/null +++ b/src/test/rustdoc-gui/sidebar-mobile-scroll.goml @@ -0,0 +1,31 @@ +// This test ensures that the mobile sidebar preserves scroll position. +goto: file://|DOC_PATH|/test_docs/struct.Foo.html +// Switching to "mobile view" by reducing the width to 600px. +size: (600, 600) +assert-css: (".sidebar", {"display": "block", "left": "-1000px"}) + +// Scroll down. +scroll-to: "//h2[@id='blanket-implementations']" +assert-window-property: {"pageYOffset": "702"} + +// Open the sidebar menu. +click: ".sidebar-menu-toggle" +wait-for-css: (".sidebar", {"left": "0px"}) + +// We are no longer "scrolled". It's important that the user can't +// scroll the body at all, but these test scripts are run only in Chrome, +// and we need to use a more complicated solution to this problem because +// of Mobile Safari... +assert-window-property: {"pageYOffset": "0"} + +// Close the sidebar menu. Make sure the scroll position gets restored. +click: ".sidebar-menu-toggle" +wait-for-css: (".sidebar", {"left": "-1000px"}) +assert-window-property: {"pageYOffset": "702"} + +// Now test that scrollability returns when the browser window is just resized. +click: ".sidebar-menu-toggle" +wait-for-css: (".sidebar", {"left": "0px"}) +assert-window-property: {"pageYOffset": "0"} +size: (900, 900) +assert-window-property: {"pageYOffset": "702"} diff --git a/src/test/rustdoc-gui/sidebar-source-code-display.goml b/src/test/rustdoc-gui/sidebar-source-code-display.goml index c441f84a82135..82565c7cdec60 100644 --- a/src/test/rustdoc-gui/sidebar-source-code-display.goml +++ b/src/test/rustdoc-gui/sidebar-source-code-display.goml @@ -152,3 +152,10 @@ click: "#sidebar-toggle" wait-for-css: (".sidebar", {"width": "0px"}) // The "scrollTop" property should be the same. assert-window-property: {"pageYOffset": "2519"} + +// We now check that the scroll position is restored if the window is resized. +click: "#sidebar-toggle" +wait-for-css: (".sidebar", {"width": "500px"}) +assert-window-property: {"pageYOffset": "0"} +size: (900, 900) +assert-window-property: {"pageYOffset": "2519"} From f7c6061ef10e3a82a1b8c8897bb3f3d2246a3fec Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 30 Jun 2022 18:02:47 -0700 Subject: [PATCH 2/5] Improve click behavior of the source code mobile full-screen "sidebar" On desktop, if you open the source code sidebar, it stays open even when you move from page to page. It used to do the same thing on mobile, but I think that's stupid. Since the file list fills the entire screen on mobile, and you can't really do anything with the currently selected file other than dismiss the "sidebar" to look at it, it's safe to assume that anybody who clicks a file in that list probably wants the list to go away so they can see it. --- src/librustdoc/html/static/css/rustdoc.css | 10 ++++++++ .../html/static/js/source-script.js | 7 ++++++ src/librustdoc/html/static/js/storage.js | 3 +++ .../sidebar-source-code-display.goml | 24 +++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 532b98d9bb9f6..04492da82265d 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1683,6 +1683,11 @@ details.rustdoc-toggle[open] > summary.hideme::after { /* Media Queries */ +/* +WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY; +If you update this line, then you also need to update the line with the same warning +in storage.js plus the media query with (max-width: 700px) +*/ @media (min-width: 701px) { /* In case there is no documentation before a code block, we need to add some margin at the top to prevent an overlay between the "collapse toggle" and the information tooltip. @@ -1703,6 +1708,11 @@ details.rustdoc-toggle[open] > summary.hideme::after { } } +/* +WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY +If you update this line, then you also need to update the line with the same warning +in storage.js plus the media query with (min-width: 701px) +*/ @media (max-width: 700px) { /* When linking to an item with an `id` (for instance, by clicking a link in the sidebar, or visiting a URL with a fragment like `#method.new`, we don't want the item to be obscured diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index 0f5217dac9705..0d8316491bfc9 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -12,6 +12,12 @@ const rootPath = document.getElementById("rustdoc-vars").attributes["data-root-path"].value; let oldScrollPosition = null; +function closeSidebarIfMobile() { + if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { + updateLocalStorage("source-sidebar-show", "false"); + } +} + function createDirEntry(elem, parent, fullPath, hasFoundFile) { const name = document.createElement("div"); name.className = "name"; @@ -48,6 +54,7 @@ function createDirEntry(elem, parent, fullPath, hasFoundFile) { const file = document.createElement("a"); file.innerText = file_text; file.href = rootPath + "src/" + fullPath + file_text + ".html"; + file.addEventListener("click", closeSidebarIfMobile); const w = window.location.href.split("#")[0]; if (!hasFoundFile && w === file.href) { file.className = "selected"; diff --git a/src/librustdoc/html/static/js/storage.js b/src/librustdoc/html/static/js/storage.js index 3b0dd282ff78e..0c5389d45e5b7 100644 --- a/src/librustdoc/html/static/js/storage.js +++ b/src/librustdoc/html/static/js/storage.js @@ -9,6 +9,9 @@ const darkThemes = ["dark", "ayu"]; window.currentTheme = document.getElementById("themeStyle"); window.mainTheme = document.getElementById("mainThemeStyle"); +// WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY +// If you update this line, then you also need to update the two media queries with the same +// warning in rustdoc.css window.RUSTDOC_MOBILE_BREAKPOINT = 701; const settingsDataset = (function() { diff --git a/src/test/rustdoc-gui/sidebar-source-code-display.goml b/src/test/rustdoc-gui/sidebar-source-code-display.goml index 82565c7cdec60..1897a6eb0dde8 100644 --- a/src/test/rustdoc-gui/sidebar-source-code-display.goml +++ b/src/test/rustdoc-gui/sidebar-source-code-display.goml @@ -18,6 +18,16 @@ click: "#sidebar-toggle" // Because of the transition CSS, we check by using `wait-for-css` instead of `assert-css`. wait-for-css: ("#sidebar-toggle", {"visibility": "visible", "opacity": 1}) +// We now check that opening the sidebar and clicking a link will leave it open. +// The behavior here on desktop is different than the behavior on mobile, +// but since the sidebar doesn't fill the entire screen here, it makes sense to have the +// sidebar stay resident. +wait-for-css: (".sidebar", {"width": "300px"}) +assert-local-storage: {"rustdoc-source-sidebar-show": "true"} +click: ".sidebar a.selected" +goto: file://|DOC_PATH|/src/test_docs/lib.rs.html +assert-local-storage: {"rustdoc-source-sidebar-show": "true"} + // Now we check the display of the sidebar items. show-text: true @@ -153,9 +163,23 @@ wait-for-css: (".sidebar", {"width": "0px"}) // The "scrollTop" property should be the same. assert-window-property: {"pageYOffset": "2519"} +// We now check that opening the sidebar and clicking a link will close it. +// The behavior here on mobile is different than the behavior on desktop, +// but common sense dictates that if you have a list of files that fills the entire screen, and +// you click one of them, you probably want to actually see the file's contents, and not just +// make it the current selection. +click: "#sidebar-toggle" +wait-for-css: (".sidebar", {"width": "500px"}) +assert-local-storage: {"rustdoc-source-sidebar-show": "true"} +click: ".sidebar a.selected" +goto: file://|DOC_PATH|/src/test_docs/lib.rs.html +assert-local-storage: {"rustdoc-source-sidebar-show": "false"} + // We now check that the scroll position is restored if the window is resized. +scroll-to: '//*[@id="117"]' click: "#sidebar-toggle" wait-for-css: (".sidebar", {"width": "500px"}) assert-window-property: {"pageYOffset": "0"} size: (900, 900) +wait-for-css: (".sidebar", {"width": "300px"}) assert-window-property: {"pageYOffset": "2519"} From 30731098e7334da79b227520a75a3d383b5af8b1 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 1 Jul 2022 10:33:06 -0700 Subject: [PATCH 3/5] rustdoc: use
tag for the source code sidebar This fixes the extremely poor accessibility of the old system, making it possible to navigate the sidebar by keyboard, and also implicitly gives the sidebar items the correct ARIA roles. --- src/librustdoc/html/static/css/rustdoc.css | 35 ++++------------ src/librustdoc/html/static/css/themes/ayu.css | 3 +- .../html/static/css/themes/dark.css | 3 +- .../html/static/css/themes/light.css | 3 +- .../html/static/js/source-script.js | 29 +++++-------- .../sidebar-source-code-display.goml | 42 +++++++++---------- src/test/rustdoc-gui/source-code-page.goml | 20 ++++----- 7 files changed, 53 insertions(+), 82 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 04492da82265d..16735936fe4bf 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1528,38 +1528,19 @@ kbd { margin-bottom: 1em; } -div.children { - padding-left: 27px; - display: none; -} -div.name { +details.dir-entry > summary { + margin: 0 0 0 13px; + list-style-position: outside; cursor: pointer; - position: relative; - margin-left: 16px; } -div.files > a { - display: block; - padding: 0 3px; -} -div.files > a:hover, div.name:hover { - background-color: #a14b4b; + +details.dir-entry div.folders, details.dir-entry div.files { + padding-left: 27px; } -div.name.expand + .children { + +details.dir-entry a { display: block; } -div.name::before { - content: "\25B6"; - padding-left: 4px; - font-size: 0.625rem; - position: absolute; - left: -16px; - top: 4px; -} -div.name.expand::before { - transform: rotate(90deg); - left: -15px; - top: 2px; -} /* The hideme class is used on summary tags that contain a span with placeholder text shown only when the toggle is closed. For instance, diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index b7d0db1f0020f..a78ac5da7490b 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -630,7 +630,8 @@ kbd { color: #fff; border-bottom-color: #5c6773; } -#source-sidebar div.files > a:hover, div.name:hover { +#source-sidebar div.files > a:hover, details.dir-entry summary:hover, +#source-sidebar div.files > a:focus, details.dir-entry summary:focus { background-color: #14191f; color: #ffb44c; } diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index eb64ef3e7710b..a2abd53fa02bc 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -500,7 +500,8 @@ kbd { #source-sidebar > .title { border-bottom-color: #ccc; } -#source-sidebar div.files > a:hover, div.name:hover { +#source-sidebar div.files > a:hover, details.dir-entry summary:hover, +#source-sidebar div.files > a:focus, details.dir-entry summary:focus { background-color: #444; } #source-sidebar div.files > .selected { diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index 00cdf83589716..c4510ad6ae66b 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -484,7 +484,8 @@ kbd { #source-sidebar > .title { border-bottom-color: #ccc; } -#source-sidebar div.files > a:hover, div.name:hover { +#source-sidebar div.files > a:hover, details.dir-entry summary:hover, +#source-sidebar div.files > a:focus, details.dir-entry summary:focus { background-color: #E0E0E0; } #source-sidebar div.files > .selected { diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index 0d8316491bfc9..5e29c2f04f701 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -19,33 +19,27 @@ function closeSidebarIfMobile() { } function createDirEntry(elem, parent, fullPath, hasFoundFile) { - const name = document.createElement("div"); - name.className = "name"; + const dirEntry = document.createElement("details"); + const summary = document.createElement("summary"); + + dirEntry.className = "dir-entry"; fullPath += elem["name"] + "/"; - name.onclick = ev => { - if (hasClass(ev.target, "expand")) { - removeClass(ev.target, "expand"); - } else { - addClass(ev.target, "expand"); - } - }; - name.innerText = elem["name"]; + summary.innerText = elem["name"]; + dirEntry.appendChild(summary); - const children = document.createElement("div"); - children.className = "children"; const folders = document.createElement("div"); folders.className = "folders"; if (elem.dirs) { for (const dir of elem.dirs) { if (createDirEntry(dir, folders, fullPath, hasFoundFile)) { - addClass(name, "expand"); + dirEntry.open = true; hasFoundFile = true; } } } - children.appendChild(folders); + dirEntry.appendChild(folders); const files = document.createElement("div"); files.className = "files"; @@ -58,15 +52,14 @@ function createDirEntry(elem, parent, fullPath, hasFoundFile) { const w = window.location.href.split("#")[0]; if (!hasFoundFile && w === file.href) { file.className = "selected"; - addClass(name, "expand"); + dirEntry.open = true; hasFoundFile = true; } files.appendChild(file); } } - children.appendChild(files); - parent.appendChild(name); - parent.appendChild(children); + dirEntry.appendChild(files); + parent.appendChild(dirEntry); return hasFoundFile; } diff --git a/src/test/rustdoc-gui/sidebar-source-code-display.goml b/src/test/rustdoc-gui/sidebar-source-code-display.goml index 1897a6eb0dde8..46f34f5a77f51 100644 --- a/src/test/rustdoc-gui/sidebar-source-code-display.goml +++ b/src/test/rustdoc-gui/sidebar-source-code-display.goml @@ -37,29 +37,29 @@ reload: // Waiting for the sidebar to be displayed... wait-for-css: ("#sidebar-toggle", {"visibility": "visible", "opacity": 1}) assert-css: ( - "#source-sidebar .expand + .children a.selected", + "#source-sidebar details[open] > .files a.selected", {"color": "rgb(0, 0, 0)", "background-color": "rgb(255, 255, 255)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files a:not(.selected)", {"color": "rgb(0, 0, 0)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children > .files a:not(.selected)" +move-cursor-to: "#source-sidebar details[open] > .files a:not(.selected)" assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files a:not(.selected)", {"color": "rgb(0, 0, 0)", "background-color": "rgb(224, 224, 224)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(0, 0, 0)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children .folders .name" +move-cursor-to: "#source-sidebar details[open] > .folders > details > summary" assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(0, 0, 0)", "background-color": "rgb(224, 224, 224)"}, ) @@ -69,29 +69,29 @@ reload: // Waiting for the sidebar to be displayed... wait-for-css: ("#sidebar-toggle", {"visibility": "visible", "opacity": 1}) assert-css: ( - "#source-sidebar .expand + .children a.selected", + "#source-sidebar details[open] > .files > a.selected", {"color": "rgb(221, 221, 221)", "background-color": "rgb(51, 51, 51)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files > a:not(.selected)", {"color": "rgb(221, 221, 221)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children > .files a:not(.selected)" +move-cursor-to: "#source-sidebar details[open] > .files a:not(.selected)" assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files a:not(.selected)", {"color": "rgb(221, 221, 221)", "background-color": "rgb(68, 68, 68)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(221, 221, 221)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children .folders .name" +move-cursor-to: "#source-sidebar details[open] > .folders > details > summary" assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(221, 221, 221)", "background-color": "rgb(68, 68, 68)"}, ) @@ -101,29 +101,29 @@ reload: // Waiting for the sidebar to be displayed... wait-for-css: ("#sidebar-toggle", {"visibility": "visible", "opacity": 1}) assert-css: ( - "#source-sidebar .expand + .children a.selected", + "#source-sidebar details[open] > .files a.selected", {"color": "rgb(255, 180, 76)", "background-color": "rgb(20, 25, 31)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files a:not(.selected)", {"color": "rgb(197, 197, 197)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children > .files a:not(.selected)" +move-cursor-to: "#source-sidebar details[open] > .files a:not(.selected)" assert-css: ( - "#source-sidebar .expand + .children > .files a:not(.selected)", + "#source-sidebar details[open] > .files a:not(.selected)", {"color": "rgb(255, 180, 76)", "background-color": "rgb(20, 25, 31)"}, ) // Without hover. assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(197, 197, 197)", "background-color": "rgba(0, 0, 0, 0)"}, ) // With hover. -move-cursor-to: "#source-sidebar .expand + .children .folders .name" +move-cursor-to: "#source-sidebar details[open] > .folders > details > summary" assert-css: ( - "#source-sidebar .expand + .children .folders .name", + "#source-sidebar details[open] > .folders > details > summary", {"color": "rgb(255, 180, 76)", "background-color": "rgb(20, 25, 31)"}, ) diff --git a/src/test/rustdoc-gui/source-code-page.goml b/src/test/rustdoc-gui/source-code-page.goml index b45512601f208..05b0e809bca69 100644 --- a/src/test/rustdoc-gui/source-code-page.goml +++ b/src/test/rustdoc-gui/source-code-page.goml @@ -34,19 +34,13 @@ assert-document-property: ({"URL": "/lib.rs.html"}, ENDS_WITH) click: "#sidebar-toggle" assert: ".source-sidebar-expanded" -// We check that the first entry of the sidebar is collapsed (which, for whatever reason, -// is number 2 and not 1...). -assert-attribute: ("#source-sidebar .name:nth-child(2)", {"class": "name"}) -assert-text: ("#source-sidebar .name:nth-child(2)", "implementors") -// We also check its children are hidden too. -assert-css: ("#source-sidebar .name:nth-child(2) + .children", {"display": "none"}) +// We check that the first entry of the sidebar is collapsed +assert-property: ("#source-sidebar details:first-of-type", {"open": "false"}) +assert-text: ("#source-sidebar details:first-of-type > summary", "implementors") // We now click on it. -click: "#source-sidebar .name:nth-child(2)" -assert-attribute: ("#source-sidebar .name:nth-child(2)", {"class": "name expand"}) -// Checking that its children are displayed as well. -assert-css: ("#source-sidebar .name:nth-child(2) + .children", {"display": "block"}) +click: "#source-sidebar details:first-of-type > summary" +assert-property: ("#source-sidebar details:first-of-type", {"open": "true"}) // And now we collapse it again. -click: "#source-sidebar .name:nth-child(2)" -assert-attribute: ("#source-sidebar .name:nth-child(2)", {"class": "name"}) -assert-css: ("#source-sidebar .name:nth-child(2) + .children", {"display": "none"}) +click: "#source-sidebar details:first-of-type > summary" +assert-property: ("#source-sidebar details:first-of-type", {"open": "false"}) From 6ca314e2a37ccfc938a1ee433cf7ca5d060192dd Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 1 Jul 2022 11:05:38 -0700 Subject: [PATCH 4/5] rustdoc: make source sidebar toggle a real button This fixes tab focus, so that you can open and close the sidebar from keyboard. --- src/librustdoc/html/static/css/rustdoc.css | 19 +++++++++++++++++-- src/librustdoc/html/static/css/themes/ayu.css | 2 +- .../html/static/css/themes/dark.css | 2 +- .../html/static/css/themes/light.css | 2 +- .../html/static/js/source-script.js | 6 +++--- 5 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 16735936fe4bf..28e1d896b52e9 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -1370,7 +1370,6 @@ pre.rust { position: sticky; top: 0; left: 0; - cursor: pointer; font-weight: bold; font-size: 1.25rem; border-bottom: 1px solid; @@ -1391,7 +1390,23 @@ pre.rust { border-bottom: 1px solid; margin-bottom: 6px; } - +#sidebar-toggle > button { + background: none; + color: inherit; + cursor: pointer; + text-align: center; + border: none; + outline: none; + position: absolute; + top: 0; + bottom: 0; + left: 0; + right: 0; + width: 100%; + /* iOS button gradient: https://stackoverflow.com/q/5438567 */ + -webkit-appearance: none; + opacity: 1; +} #settings-menu, #help-button { margin-left: 4px; outline: none; diff --git a/src/librustdoc/html/static/css/themes/ayu.css b/src/librustdoc/html/static/css/themes/ayu.css index a78ac5da7490b..66b20ede2aec2 100644 --- a/src/librustdoc/html/static/css/themes/ayu.css +++ b/src/librustdoc/html/static/css/themes/ayu.css @@ -620,7 +620,7 @@ kbd { #sidebar-toggle { background-color: #14191f; } -#sidebar-toggle:hover { +#sidebar-toggle:hover, #sidebar-toggle > button:focus { background-color: rgba(70, 70, 70, 0.33); } #source-sidebar { diff --git a/src/librustdoc/html/static/css/themes/dark.css b/src/librustdoc/html/static/css/themes/dark.css index a2abd53fa02bc..960fa52231d30 100644 --- a/src/librustdoc/html/static/css/themes/dark.css +++ b/src/librustdoc/html/static/css/themes/dark.css @@ -491,7 +491,7 @@ kbd { #sidebar-toggle { background-color: #565656; } -#sidebar-toggle:hover { +#sidebar-toggle:hover, #sidebar-toggle > button:focus { background-color: #676767; } #source-sidebar { diff --git a/src/librustdoc/html/static/css/themes/light.css b/src/librustdoc/html/static/css/themes/light.css index c4510ad6ae66b..4c5b8213f6240 100644 --- a/src/librustdoc/html/static/css/themes/light.css +++ b/src/librustdoc/html/static/css/themes/light.css @@ -475,7 +475,7 @@ kbd { #sidebar-toggle { background-color: #F5F5F5; } -#sidebar-toggle:hover { +#sidebar-toggle:hover, #sidebar-toggle > button:focus { background-color: #E0E0E0; } #source-sidebar { diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index 5e29c2f04f701..b4bed89490759 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -64,7 +64,7 @@ function createDirEntry(elem, parent, fullPath, hasFoundFile) { } function toggleSidebar() { - const child = this.children[0]; + const child = this.parentNode.children[0]; if (child.innerText === ">") { if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { // This is to keep the scroll position on mobile. @@ -107,15 +107,15 @@ window.addEventListener("resize", () => { function createSidebarToggle() { const sidebarToggle = document.createElement("div"); sidebarToggle.id = "sidebar-toggle"; - sidebarToggle.onclick = toggleSidebar; - const inner = document.createElement("div"); + const inner = document.createElement("button"); if (getCurrentValue("source-sidebar-show") === "true") { inner.innerText = "<"; } else { inner.innerText = ">"; } + inner.onclick = toggleSidebar; sidebarToggle.appendChild(inner); return sidebarToggle; From 559434170769ea56c75d1966fd4090d5ae1416ef Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 1 Jul 2022 11:27:09 -0700 Subject: [PATCH 5/5] rustdoc cleanup: remove unused function --- src/librustdoc/html/static/js/source-script.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index b4bed89490759..031dc92664fec 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -2,7 +2,7 @@ /* global sourcesIndex */ // Local js definitions: -/* global addClass, getCurrentValue, hasClass, onEachLazy, removeClass, browserSupportsHistoryApi */ +/* global addClass, getCurrentValue, onEachLazy, removeClass, browserSupportsHistoryApi */ /* global updateLocalStorage */ "use strict";