From bb7ed64f457be500a74d53c9d12f1d0a8f7badf6 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Mon, 10 Apr 2023 16:15:51 -0700 Subject: [PATCH] rustdoc: use CSS `overscroll-behavior` instead of JavaScript Fixes the desktop scrolling weirdness mentioned in https://github.com/rust-lang/rust/pull/98775#issuecomment-1182575603 As described in the MDN page for this property: * The current Firefox ESR is 102, and the first Firefox version to support this feature is 59. * The current Chrome version 112, and the first version to support this is 63. * Edge is described as having a minor bug in `none` mode, but we use `contain` mode anyway, so it doesn't matter. * Safari 16, released September 2022, is the last browser to add this feature, and is also the oldest version we officially support. --- src/librustdoc/html/static/css/rustdoc.css | 3 +- src/librustdoc/html/static/js/main.js | 52 ------------------- .../html/static/js/source-script.js | 9 ++-- tests/rustdoc-gui/sidebar-mobile-scroll.goml | 39 ++++---------- .../sidebar-source-code-display.goml | 12 +---- 5 files changed, 19 insertions(+), 96 deletions(-) diff --git a/src/librustdoc/html/static/css/rustdoc.css b/src/librustdoc/html/static/css/rustdoc.css index 9df19352567..6fbb4508662 100644 --- a/src/librustdoc/html/static/css/rustdoc.css +++ b/src/librustdoc/html/static/css/rustdoc.css @@ -384,6 +384,7 @@ img { font-size: 0.875rem; flex: 0 0 200px; overflow-y: scroll; + overscroll-behavior: contain; position: sticky; height: 100vh; top: 0; @@ -1531,7 +1532,7 @@ However, it's not needed with smaller screen width because the doc/code block is /* WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY If you update this line, then you also need to update the line with the same warning -in main.js +in source-script.js */ @media (max-width: 700px) { /* When linking to an item with an `id` (for instance, by clicking a link in the sidebar, diff --git a/src/librustdoc/html/static/js/main.js b/src/librustdoc/html/static/js/main.js index a6655663b82..6f5987e68bf 100644 --- a/src/librustdoc/html/static/js/main.js +++ b/src/librustdoc/html/static/js/main.js @@ -4,11 +4,6 @@ "use strict"; -// WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY -// If you update this line, then you also need to update the media query with the same -// warning in rustdoc.css -window.RUSTDOC_MOBILE_BREAKPOINT = 700; - // Given a basename (e.g. "storage") and an extension (e.g. ".js"), return a URL // for a resource under the root-path, with the resource-suffix. function resourcePath(basename, extension) { @@ -730,65 +725,18 @@ function preLoadCss(cssUrl) { window.rustdoc_add_line_numbers_to_examples(); } - let oldSidebarScrollPosition = null; - - // Scroll locking used both here and in source-script.js - - window.rustdocMobileScrollLock = function() { - const mobile_topbar = document.querySelector(".mobile-topbar"); - 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`; - if (mobile_topbar) { - mobile_topbar.style.top = `${oldSidebarScrollPosition}px`; - mobile_topbar.style.position = "relative"; - } - } else { - oldSidebarScrollPosition = null; - } - }; - - window.rustdocMobileScrollUnlock = function() { - const mobile_topbar = document.querySelector(".mobile-topbar"); - if (oldSidebarScrollPosition !== null) { - // This is to keep the scroll position on mobile. - document.body.style.width = ""; - document.body.style.position = ""; - document.body.style.top = ""; - if (mobile_topbar) { - mobile_topbar.style.top = ""; - 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; - } - }; - function showSidebar() { window.hideAllModals(false); - window.rustdocMobileScrollLock(); const sidebar = document.getElementsByClassName("sidebar")[0]; addClass(sidebar, "shown"); } function hideSidebar() { - window.rustdocMobileScrollUnlock(); 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(); - } if (window.CURRENT_TOOLTIP_ELEMENT) { // As a workaround to the behavior of `contains: layout` used in doc togglers, // tooltip popovers are positioned using javascript. diff --git a/src/librustdoc/html/static/js/source-script.js b/src/librustdoc/html/static/js/source-script.js index 6c0f03b5bb0..9aa75517330 100644 --- a/src/librustdoc/html/static/js/source-script.js +++ b/src/librustdoc/html/static/js/source-script.js @@ -15,8 +15,13 @@ const NAME_OFFSET = 0; const DIRS_OFFSET = 1; const FILES_OFFSET = 2; +// WARNING: RUSTDOC_MOBILE_BREAKPOINT MEDIA QUERY +// If you update this line, then you also need to update the media query with the same +// warning in rustdoc.css +const RUSTDOC_MOBILE_BREAKPOINT = 700; + function closeSidebarIfMobile() { - if (window.innerWidth < window.RUSTDOC_MOBILE_BREAKPOINT) { + if (window.innerWidth < RUSTDOC_MOBILE_BREAKPOINT) { updateLocalStorage("source-sidebar-show", "false"); } } @@ -69,12 +74,10 @@ function createDirEntry(elem, parent, fullPath, hasFoundFile) { function toggleSidebar() { const child = this.parentNode.children[0]; if (child.innerText === ">") { - window.rustdocMobileScrollLock(); addClass(document.documentElement, "source-sidebar-expanded"); child.innerText = "<"; updateLocalStorage("source-sidebar-show", "true"); } else { - window.rustdocMobileScrollUnlock(); removeClass(document.documentElement, "source-sidebar-expanded"); child.innerText = ">"; updateLocalStorage("source-sidebar-show", "false"); diff --git a/tests/rustdoc-gui/sidebar-mobile-scroll.goml b/tests/rustdoc-gui/sidebar-mobile-scroll.goml index 84811437eb2..d58d1d48726 100644 --- a/tests/rustdoc-gui/sidebar-mobile-scroll.goml +++ b/tests/rustdoc-gui/sidebar-mobile-scroll.goml @@ -1,31 +1,12 @@ -// This test ensures that the mobile sidebar preserves scroll position. +// This test ensures that the mobile disables scrolling the page. go-to: "file://" + |DOC_PATH| + "/test_docs/struct.Foo.html" -// Switching to "mobile view" by reducing the width to 600px. -set-window-size: (700, 600) -assert-css: (".sidebar", {"display": "block", "left": "-1000px"}) +set-window-size: (1280, 800) // desktop +assert-css: (".sidebar", {"overscroll-behavior": "contain"}) +set-window-size: (700, 600) // mobile +assert-css: (".sidebar", {"overscroll-behavior": "contain"}) -// Scroll down. -scroll-to: "//h2[@id='blanket-implementations']" -assert-window-property: {"pageYOffset": "622"} - -// 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": "622"} - -// 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"} -set-window-size: (900, 600) -assert-window-property: {"pageYOffset": "622"} +go-to: "file://" + |DOC_PATH| + "/src/test_docs/lib.rs.html" +set-window-size: (1280, 800) // desktop +assert-css: (".sidebar", {"overscroll-behavior": "contain"}) +set-window-size: (700, 600) // mobile +assert-css: (".sidebar", {"overscroll-behavior": "contain"}) diff --git a/tests/rustdoc-gui/sidebar-source-code-display.goml b/tests/rustdoc-gui/sidebar-source-code-display.goml index f34e30b724d..20bf0596f95 100644 --- a/tests/rustdoc-gui/sidebar-source-code-display.goml +++ b/tests/rustdoc-gui/sidebar-source-code-display.goml @@ -183,22 +183,12 @@ wait-for-css: (".sidebar", {"left": "-1000px"}) // The "scrollTop" property should be the same. assert-window-property: {"pageYOffset": "2542"} -// We now check that the scroll position is restored if the window is resized. -set-window-size: (500, 700) -click: "#src-sidebar-toggle" -wait-for-css: ("#source-sidebar", {"visibility": "visible"}) -assert-window-property: {"pageYOffset": "0"} -set-window-size: (900, 900) -assert-window-property: {"pageYOffset": "2542"} -set-window-size: (500, 700) -click: "#src-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 // you click one of them, you probably want to actually see the file's contents, and not just // make it the current selection. +set-window-size: (500, 700) click: "#src-sidebar-toggle" wait-for-css: ("#source-sidebar", {"visibility": "visible"}) assert-local-storage: {"rustdoc-source-sidebar-show": "true"}