From ffda1975e32120693a6667e92477582bae6ae24b Mon Sep 17 00:00:00 2001 From: Nicholas Ray Date: Fri, 3 Feb 2023 19:01:56 -0700 Subject: [PATCH] Make page tools sticky * Adds sticky behavior to pinned page tools * Moves scroll indicator styles into a mixin shared by TOC and page tools * Replaces the 10px magic number in the TOC used to calculate the bottom padding with the @padding-vertical-dropdown-menu variable. * Increases the pinned TOC max-height per T319315 * Corrects spacing between bottom of sticky header and top of TOC after discussion with designer * Causes 43 visual changes in Pixel that include intentional changes and subpixel rendering changes associated with the `contain: paint` rule. Bug: T318169 Bug: T319315 Change-Id: Ica0c4e0de1825d37d8136b589a9bf5decc96855e --- resources/common/mixins.less | 15 ++++++++++ resources/common/variables.less | 17 +++++++---- .../components/PageTools.less | 26 ++++++++++++++++- .../components/StickyHeader.less | 6 ++++ .../components/TableOfContents.less | 2 +- .../components/TableOfContentsPinned.less | 29 ++++++++----------- 6 files changed, 71 insertions(+), 24 deletions(-) diff --git a/resources/common/mixins.less b/resources/common/mixins.less index 195feb126..f386c22b8 100644 --- a/resources/common/mixins.less +++ b/resources/common/mixins.less @@ -53,3 +53,18 @@ color: @color-link-selected; } } + +// Scroll indicator used by TOC and page tools. +.mixin-vector-scroll-indicator() { + content: ''; + display: block; + position: absolute; + bottom: 0; + left: 0; + right: 0; + height: @height-scroll-indicator; + background: linear-gradient( rgba( 255, 255, 255, 0 ), @background-color-page-container ); + background-repeat: no-repeat; + background-position: -12px; // T311436 Hacky way to prevent the fade from covering the scrollbar + pointer-events: none; // Make the link below the fade clickable +} diff --git a/resources/common/variables.less b/resources/common/variables.less index 0fc19226e..82c7803f0 100644 --- a/resources/common/variables.less +++ b/resources/common/variables.less @@ -125,7 +125,8 @@ // Tabs @font-size-tabs: unit( 13 / @font-size-browser, em ); // Equals `0.8125em`. @padding-horizontal-tabs: 8px; -@padding-vertical-tabs: 18px 0 7px 0; +@padding-top-tabs: 18px; +@padding-vertical-tabs: @padding-top-tabs 0 7px 0; // Dropdowns @height-dropdown-menu-item: unit( 32 / @font-size-browser, rem ); @@ -139,13 +140,19 @@ @selector-main-menu-open: ~'#mw-sidebar-checkbox:checked'; @selector-main-menu-closed: ~'#mw-sidebar-checkbox:not( :checked )'; +// Scroll Indicator +@height-scroll-indicator: 30px; +// The amount of space that should be between the bottom of the viewport and the +// bottom of the scroll indicator. +@max-height-bottom-spacing-scroll-indicator: 16px; + // TOC -@height-toc-scroll-indicator: 30px; -@scrollbar-offset-toc-scroll-indicator: 12px; @spacing-subsection-toggle: 22px; // @size-toc-subsection-toggle-icon @ 12 @size-toc-subsection-toggle-icon: 1.834em; -// Calculated with: 1.5em (TOC top padding) + @margin-top-pinned-toc = 0.5em (.mw-body padding) + 2.34 (height of #firstHeading) -@margin-top-pinned-toc: @padding-top-content + (@font-size-heading-1 * @line-height-heading) - 1.5em; +// Calculated with: @margin-top-pinned-toc = 0.5em (.mw-body padding) + 2.34 (height of #firstHeading) +@margin-top-pinned-toc: @padding-top-content + (@font-size-heading-1 * @line-height-heading); +// The design spec specifies 30px of space from the top of the viewport to the top of the pinned TOC (excluding top padding). +@top-sticky-toc: 30px - @padding-top-tabs; // Search @max-width-search: unit( 500px / @font-size-browser / @font-size-base, em ); // 35.71428571em @ 16 & 0.875em T270202 diff --git a/resources/skins.vector.styles/components/PageTools.less b/resources/skins.vector.styles/components/PageTools.less index 30f709233..c4a3f479d 100644 --- a/resources/skins.vector.styles/components/PageTools.less +++ b/resources/skins.vector.styles/components/PageTools.less @@ -1,3 +1,6 @@ +@import '../../common/variables.less'; +@import '../../common/mixins.less'; + // Ensure there is only 1 page tools landmark at anytime .vector-page-tools-landmark { .vector-feature-page-tools-pinned-enabled .vector-page-toolbar-container &, @@ -6,11 +9,32 @@ } } +.vector-column-end { + // T327460: Prevent subpixel rendering issues associated with the text and Chrome. + contain: paint; + + .vector-page-tools-landmark { + // stylelint-disable-next-line plugin/no-unsupported-browser-features + position: sticky; + top: 0; + } +} + .vector-page-tools { #vector-page-tools-pinned-container & { width: 140px; // Match spacing of TOC and main menu - padding-left: @padding-horizontal-dropdown-menu-item; + margin-left: @padding-horizontal-dropdown-menu-item; + max-height: ~'calc( 100vh - @{max-height-bottom-spacing-scroll-indicator} )'; + overflow-y: auto; + // Add extra padding for the fade scrollable indicator. + padding-bottom: @height-scroll-indicator - @padding-vertical-dropdown-menu-item; + box-sizing: border-box; + + // Add fade indicator. + &:after { + .mixin-vector-scroll-indicator(); + } } } diff --git a/resources/skins.vector.styles/components/StickyHeader.less b/resources/skins.vector.styles/components/StickyHeader.less index 0cddaf8c4..8fd2fa05f 100644 --- a/resources/skins.vector.styles/components/StickyHeader.less +++ b/resources/skins.vector.styles/components/StickyHeader.less @@ -165,10 +165,16 @@ // https://www.mediawiki.org/wiki/Reading/Web/Desktop_Improvements/Features/Sticky_Header // - `.charts-stickyhead th` makes chart and table headers appear below the sticky header. .vector-toc-pinned #vector-toc-pinned-container, + &.vector-feature-page-tools-pinned-enabled .vector-page-tools-landmark, .mw-sticky-header-element, .charts-stickyhead th { /* stylelint-disable-next-line declaration-no-important */ top: @height-sticky-header !important; } + + #vector-toc-pinned-container .vector-toc, + #vector-page-tools-pinned-container .vector-page-tools { + max-height: ~'calc( 100vh - @{height-sticky-header} - @{max-height-bottom-spacing-scroll-indicator} )'; + } } } diff --git a/resources/skins.vector.styles/components/TableOfContents.less b/resources/skins.vector.styles/components/TableOfContents.less index b658e7ff3..edb7fdb39 100644 --- a/resources/skins.vector.styles/components/TableOfContents.less +++ b/resources/skins.vector.styles/components/TableOfContents.less @@ -8,7 +8,7 @@ background-color: @background-color-page-container; .vector-feature-page-tools-disabled & { - padding: 20px 12px 20px 27px; + padding: @padding-top-tabs 12px 20px 27px; .vector-toc-pinnable-header { // Override default pinnable header styles diff --git a/resources/skins.vector.styles/components/TableOfContentsPinned.less b/resources/skins.vector.styles/components/TableOfContentsPinned.less index 81ca46eee..28edcbb9e 100644 --- a/resources/skins.vector.styles/components/TableOfContentsPinned.less +++ b/resources/skins.vector.styles/components/TableOfContentsPinned.less @@ -1,4 +1,5 @@ @import '../../common/variables.less'; +@import '../../common/mixins.less'; .mw-table-of-contents-container { // Needed for Grid-based layout @@ -36,19 +37,23 @@ #vector-toc-pinned-container { // stylelint-disable-next-line plugin/no-unsupported-browser-features position: sticky; - top: 0; + // The design spec specifies 30px of space from the top of the viewport to the top of the pinned TOC (excluding top padding). + top: @top-sticky-toc; + + .vector-toc { + max-height: ~'calc( 100vh - @{top-sticky-toc} - @{max-height-bottom-spacing-scroll-indicator} )'; + } .vector-toc-pinned & { // Default spacing separating the sidebar TOC from the main menu or viewport. - // Need to use padding in order for the spacing to apply when sticky - padding-top: 1.5em; + margin-top: 1.5em; } // FIXME: .vector-feature-page-tools-disabled.vector-toc-pinned can be removed when cached HTML no longer an issue. .vector-feature-page-tools-disabled .vector-toc-pinned @{selector-main-menu-closed} ~ .mw-table-of-contents-container &, .vector-feature-page-tools-disabled.vector-toc-pinned @{selector-main-menu-closed} ~ .mw-table-of-contents-container & { - // Needed to align TOC with bottom of title, 1.5em padding + 1.5em margin = 3em - margin-top: 1.5em; + // Needed to align TOC with bottom of title. + margin-top: @margin-top-pinned-toc; } // FIXME: .vector-feature-page-tools-enabled.vector-toc-pinned can be removed when cached HTML no longer an issue. @@ -62,23 +67,13 @@ padding-left: @spacing-subsection-toggle; padding-right: @padding-horizontal-dropdown-menu-item; // Add extra padding for the fade scrollable indicator, which is 30px tall - padding-bottom: 20px; + padding-bottom: @height-scroll-indicator - @padding-vertical-dropdown-menu-item; } // T302076: Add fade scrollable indicator when TOC is in sidebar // Avoid showing indicator when the TOC is floating, or collapsed in the page title/sticky header .vector-toc-pinned & .vector-toc:after { - content: ''; - display: block; - position: absolute; - bottom: 0; - left: 0; - right: 0; - height: @height-toc-scroll-indicator; - background: linear-gradient( rgba( 255, 255, 255, 0 ), @background-color-page-container ); - background-repeat: no-repeat; - background-position: -@scrollbar-offset-toc-scroll-indicator; // T311436 Hacky way to prevent the fade from covering the scrollbar - pointer-events: none; // Make the link below the fade clickable + .mixin-vector-scroll-indicator(); } } }