From 1209b388e9af7293c6f72c62bb2cf4ad3f5c000a Mon Sep 17 00:00:00 2001
From: Nicholas Ray <nray@wikimedia.org>
Date: Tue, 28 Sep 2021 16:43:15 -0600
Subject: [PATCH] Add scroll padding to the root element when the sticky header
 is enabled

When the sticky header is visible, it has a global impact on the
scrolling UX. For example, it can undesirably overlap elements when the
user clicks on a jump link and when the user tabs through elements in
reverse order. Therefore, we need to add scroll padding to the root
element when the sticky header is enabled (when the feature flag is on
and at higher resolutions)

Known limitations:

* Scroll padding is supported by all the latest modern browsers except
for Safari [1]. This was considered an acceptable tradeoff with the
caveat that this decision may be revisited in the future as we learn
more about user interaction with the sticky header.

[1] https://caniuse.com/mdn-css_properties_scroll-padding-top

Bug: T290518
Change-Id: Ie5eb01d7eafd18ce740be620dfb5c8849386af6e
---
 includes/SkinVector.php                       | 21 ++++++++++++++++++
 .../components/StickyHeader.less              | 22 +++++++++++++++++++
 2 files changed, 43 insertions(+)

diff --git a/includes/SkinVector.php b/includes/SkinVector.php
index a4b040095..d27df7035 100644
--- a/includes/SkinVector.php
+++ b/includes/SkinVector.php
@@ -66,6 +66,7 @@ class SkinVector extends SkinMustache {
 		'class' => 'sticky-header-icon'
 	];
 	private const SEARCH_EXPANDING_CLASS = 'vector-search-box-show-thumbnail';
+	private const STICKY_HEADER_ENABLED_CLASS = 'vector-sticky-header-enabled';
 
 	/**
 	 * T243281: Code used to track clicks to opt-out link.
@@ -323,6 +324,26 @@ class SkinVector extends SkinMustache {
 		return parent::generateHTML();
 	}
 
+	/**
+	 * @inheritDoc
+	 */
+	public function getHtmlElementAttributes() {
+		$original = parent::getHtmlElementAttributes();
+
+		if ( VectorServices::getFeatureManager()->isFeatureEnabled( Constants::FEATURE_STICKY_HEADER ) ) {
+			// T290518: Add scroll padding to root element when the sticky header is
+			// enabled. This class needs to be server rendered instead of added from
+			// JS in order to correctly handle situations where the sticky header
+			// isn't visible yet but we still need scroll padding applied (e.g. when
+			// the user navigates to a page with a hash fragment in the URI). For this
+			// reason, we can't rely on the `vector-sticky-header-visible` class as it
+			// is added too late.
+			$original['class'] = implode( ' ', [ $original['class'] ?? '', self::STICKY_HEADER_ENABLED_CLASS ] );
+		}
+
+		return $original;
+	}
+
 	/**
 	 * Generate data needed to generate the sticky header.
 	 * Lack of i18n is intentional and will be done as part of follow up work.
diff --git a/resources/skins.vector.styles/components/StickyHeader.less b/resources/skins.vector.styles/components/StickyHeader.less
index 5d3668182..a586d2b35 100644
--- a/resources/skins.vector.styles/components/StickyHeader.less
+++ b/resources/skins.vector.styles/components/StickyHeader.less
@@ -1,8 +1,14 @@
 @import '../../common/variables.less';
 @import 'mediawiki.mixins.less';
 
+// Set an explicit height. This is needed for scroll padding and for other
+// sticky elements on the page. Setting the height in relative units enables
+// the header's height to adapt to the browser's font size setting.
+@height-sticky-header: unit( 60px / @font-size-browser, em );
+
 .vector-sticky-header {
 	width: 100%;
+	height: @height-sticky-header;
 	position: fixed;
 	top: 0;
 	left: 0;
@@ -122,3 +128,19 @@
 .client-nojs .vector-sticky-header {
 	display: none;
 }
+
+// T290518: When the sticky header is enabled (feature flag is on, js is
+// enabled, and viewport is at higher resolutions), add scroll padding to the
+// root element. This is needed so that the sticky header does not overlap the
+// top of an element when the URI has a hash fragment (e.g. when the user clicks
+// a jump link) and when the user tabs through elements in reverse order.
+//
+// Please note that this class must be independent of the
+// .vector-sticky-header-visible class to correctly handle situations where the
+// sticky header isn't visible yet but we still need scroll padding applied
+// (e.g. when the user navigates to a page with a hash fragment in the URI).
+@media ( min-width: @width-breakpoint-tablet ) {
+	.client-js.vector-sticky-header-enabled {
+		scroll-padding-top: @height-sticky-header;
+	}
+}