From 6b60bc64087e130f30e3bc095a3ef9e0c1790fef Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Thu, 30 Jun 2022 17:28:29 -0700 Subject: [PATCH] rustdoc: improve scroll locking in the rustdoc mobile 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. This does not affect the behavior on larger screens. Only small ones, where the sidebar covers up the main content. --- src/librustdoc/html/static/js/main.js | 46 +++++++++++++++++-- .../html/static/js/source-script.js | 20 ++++++-- .../rustdoc-gui/sidebar-mobile-scroll.goml | 31 +++++++++++++ .../sidebar-source-code-display.goml | 11 +++++ 4 files changed, 101 insertions(+), 7 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 6658f07ce0103..045dfe313dfe4 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -348,8 +348,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); } @@ -731,11 +730,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) { @@ -778,9 +816,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 1e9bfa5cc8959..c5bfc00c78b06 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 closeSidebarIfMobile() { if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { @@ -71,18 +71,21 @@ function toggleSidebar() { 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 < window.RUSTDOC_MOBILE_BREAKPOINT) { + if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT && 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/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 fa322574fdec7..e4662a10ed5da 100644 --- a/src/test/rustdoc-gui/sidebar-source-code-display.goml +++ b/src/test/rustdoc-gui/sidebar-source-code-display.goml @@ -233,6 +233,17 @@ 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. +size: (500, 700) +click: "#sidebar-toggle" +wait-for-css: ("#source-sidebar", {"visibility": "visible"}) +assert-window-property: {"pageYOffset": "0"} +size: (900, 900) +assert-window-property: {"pageYOffset": "2519"} +size: (500, 700) +click: "#sidebar-toggle" +wait-for-css: ("#source-sidebar", {"visibility": "hidden"}) + // 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