From 83f22885ae9fe7116f97efd7f7df9b6a719d7e6f Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Fri, 1 Jul 2022 12:13:46 -0700 Subject: [PATCH 1/2] 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 | 11 ++++++++-- src/librustdoc/html/static/js/storage.js | 5 +++++ .../sidebar-source-code-display.goml | 22 +++++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) 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 acb1d8d7b5c8d..586c14c33a3ba 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 = 0; +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"; @@ -66,7 +73,7 @@ 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"; @@ -76,7 +83,7 @@ function toggleSidebar() { child.innerText = "<"; updateLocalStorage("source-sidebar-show", "true"); } else { - if (window.innerWidth < 701) { + if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { // This is to keep the scroll position on mobile. document.body.style.position = ""; document.body.style.top = ""; diff --git a/src/librustdoc/html/static/js/storage.js b/src/librustdoc/html/static/js/storage.js index 1c4c88344888c..0c5389d45e5b7 100644 --- a/src/librustdoc/html/static/js/storage.js +++ b/src/librustdoc/html/static/js/storage.js @@ -9,6 +9,11 @@ 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() { const settingsElement = document.getElementById("default-settings"); if (settingsElement === null) { diff --git a/src/test/rustdoc-gui/sidebar-source-code-display.goml b/src/test/rustdoc-gui/sidebar-source-code-display.goml index c441f84a82135..7728bfd025ff2 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 @@ -152,3 +162,15 @@ 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 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"} From 6e2c49f7eddb4c09e17653760965b69a3dc97fb0 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 2 Jul 2022 10:41:46 -0700 Subject: [PATCH 2/2] rustdoc: add gui test case ensuring source sidebar doesn't spontaneously open --- src/test/rustdoc-gui/sidebar-source-code-display.goml | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/test/rustdoc-gui/sidebar-source-code-display.goml b/src/test/rustdoc-gui/sidebar-source-code-display.goml index 7728bfd025ff2..8d13246a63f80 100644 --- a/src/test/rustdoc-gui/sidebar-source-code-display.goml +++ b/src/test/rustdoc-gui/sidebar-source-code-display.goml @@ -26,6 +26,7 @@ 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 +wait-for-css: (".sidebar", {"width": "300px"}) assert-local-storage: {"rustdoc-source-sidebar-show": "true"} // Now we check the display of the sidebar items. @@ -169,8 +170,16 @@ assert-window-property: {"pageYOffset": "2519"} // 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"}) +wait-for-css: ("#source-sidebar", {"visibility": "visible"}) assert-local-storage: {"rustdoc-source-sidebar-show": "true"} click: ".sidebar a.selected" goto: file://|DOC_PATH|/src/test_docs/lib.rs.html +wait-for-css: ("#source-sidebar", {"visibility": "hidden"}) +assert-local-storage: {"rustdoc-source-sidebar-show": "false"} +// Resize back to desktop size, to check that the sidebar doesn't spontaneously open. +size: (1000, 1000) +wait-for-css: ("#source-sidebar", {"visibility": "hidden"}) assert-local-storage: {"rustdoc-source-sidebar-show": "false"} +click: "#sidebar-toggle" +wait-for-css: ("#source-sidebar", {"visibility": "visible"}) +assert-local-storage: {"rustdoc-source-sidebar-show": "true"}