From 8a3e24e3467ecf90a738753a7d1a5b5ffd885cd0 Mon Sep 17 00:00:00 2001
From: Jan Drewniak <jan.drewniak@gmail.com>
Date: Mon, 16 Oct 2023 23:31:18 -0400
Subject: [PATCH] Simplify TOC design for pinned TOC on no js small viewports

Bug: T340571
Change-Id: Ie34d7d478b3a73cade9f5328439515e419692eb7
---
 includes/templates/ColumnStart.mustache       |  1 -
 includes/templates/PageTitlebar.mustache      | 12 ------
 .../components/PinnableElement.less           | 21 ---------
 .../skins.vector.styles/layouts/gradeC.less   | 17 --------
 .../skins.vector.styles/layouts/print.less    |  2 -
 .../layouts/toc/unpinned.less                 | 39 ++---------------
 resources/skins.vector.styles/skin.less       |  2 -
 .../components/PinnableElement.less           | 33 +++++++-------
 .../components/TableOfContents.less           |  9 ----
 .../layouts/toc/unpinned.less                 | 43 +++----------------
 10 files changed, 24 insertions(+), 155 deletions(-)
 delete mode 100644 resources/skins.vector.styles/layouts/gradeC.less

diff --git a/includes/templates/ColumnStart.mustache b/includes/templates/ColumnStart.mustache
index 751de721c..baa85f0c8 100644
--- a/includes/templates/ColumnStart.mustache
+++ b/includes/templates/ColumnStart.mustache
@@ -1,7 +1,6 @@
 {{#is-zebra-enabled}}<div class="vector-column-start">{{/is-zebra-enabled}}
 	{{>MainMenuPinned}}
 	{{#data-toc}}
-	<input type="checkbox" id="vector-toc-collapsed-checkbox" class="vector-menu-checkbox">
 	<nav id="mw-panel-toc" role="navigation" aria-label="{{ msg-vector-toc-label }}" data-event-name="ui.sidebar-toc" class="mw-table-of-contents-container vector-toc-landmark vector-sticky-pinned-container">
 		{{! T313060 Additional container div needed to prevent the sticky element from being siblings with the footer }}
 		{{#data-toc-pinnable-container}}
diff --git a/includes/templates/PageTitlebar.mustache b/includes/templates/PageTitlebar.mustache
index b3b45cbcf..3a59c612b 100644
--- a/includes/templates/PageTitlebar.mustache
+++ b/includes/templates/PageTitlebar.mustache
@@ -4,18 +4,6 @@
 }}
 <header class="mw-body-header vector-page-titlebar{{#is-title-blank}} vector-page-titlebar-blank{{/is-title-blank}}">
 	{{#data-toc}}
-	{{! Checkbox hack used by collapsed TOC on narrow viewports for no JS users }}
-	<label
-		id="vector-toc-collapsed-button"
-		class="cdx-button cdx-button--fake-button cdx-button--fake-button--enabled cdx-button--weight-quiet vector-button-flush-left cdx-button--icon-only"
-		for="vector-toc-collapsed-checkbox"
-		role="button"
-		aria-controls="vector-toc"
-		tabindex="0"
-		title="{{msg-vector-toc-menu-tooltip}}">
-		<span class="vector-icon mw-ui-icon-wikimedia-listBullet"></span>
-		<span>{{msg-vector-toc-collapsible-button-label}}</span>
-	</label>
 	{{! TOC is moved into this dropdown from the sidebar in stickyHeader.js. }}
 	<nav role="navigation" aria-label="{{ msg-vector-toc-label }}" class="vector-toc-landmark">
 		{{#data-page-titlebar-toc-dropdown}}{{>Dropdown/Open}}{{/data-page-titlebar-toc-dropdown}}
diff --git a/resources/skins.vector.styles/components/PinnableElement.less b/resources/skins.vector.styles/components/PinnableElement.less
index 94466bcee..8050abd9d 100644
--- a/resources/skins.vector.styles/components/PinnableElement.less
+++ b/resources/skins.vector.styles/components/PinnableElement.less
@@ -8,24 +8,3 @@
 		padding-right: 0;
 	}
 }
-
-/**
- * At lower resolutions, we want to hide the pinned containers when JS is enabled since these
- * elements collapse (become unpinned) at this resolution via PinnableElement.js.
- * Although this is handled in JS, this rule prevents the pinned menu from
- * appearing on pageload, at low resolutions, before the JS kicks in.
- *
- * When JS is disabled, we still want to hide the pinned containers at lower resolutions
- * to fallback to the gradeC, single column layout.
- */
-@media ( max-width: @max-width-tablet ) {
-	// .client-js and .client-nojs selectors needed to increase specificity to override gradeC.less
-	// Prefered to using !important as this still needs to be overridable to handle
-	// the anon, no-js TOC case found in layouts/toc/unpinned.less
-	&.client-js,
-	&.client-nojs {
-		.vector-pinned-container {
-			display: none;
-		}
-	}
-}
diff --git a/resources/skins.vector.styles/layouts/gradeC.less b/resources/skins.vector.styles/layouts/gradeC.less
deleted file mode 100644
index 7eadef29c..000000000
--- a/resources/skins.vector.styles/layouts/gradeC.less
+++ /dev/null
@@ -1,17 +0,0 @@
-//
-// No grid support behaviour
-//
-
-// If there is no display grid support we remove pinnable elements
-// On IE11 this leaves a clean single column grid.
-// To be revisited when table of contents is collapsible in which situation we should be able to
-// force it as sticky.
-.vector-pinned-container {
-	display: none;
-}
-
-@supports ( display: grid ) {
-	.vector-pinned-container {
-		display: block;
-	}
-}
diff --git a/resources/skins.vector.styles/layouts/print.less b/resources/skins.vector.styles/layouts/print.less
index 41b4266cd..fcd9ecf44 100644
--- a/resources/skins.vector.styles/layouts/print.less
+++ b/resources/skins.vector.styles/layouts/print.less
@@ -11,8 +11,6 @@
 #vector-sticky-header,
 #p-lang-btn,
 .vector-menu-checkbox,
-// T339344
-#vector-toc-collapsed-button,
 .vector-limited-width-toggle,
 // Hide all navigation
 nav,
diff --git a/resources/skins.vector.styles/layouts/toc/unpinned.less b/resources/skins.vector.styles/layouts/toc/unpinned.less
index 5bb67df4f..489b68a3b 100644
--- a/resources/skins.vector.styles/layouts/toc/unpinned.less
+++ b/resources/skins.vector.styles/layouts/toc/unpinned.less
@@ -1,10 +1,7 @@
 @import '../../variables.less';
 @import '../../mixins.less';
 
-@selector-nojs-collapsed-toc-open: ~'#vector-toc-collapsed-checkbox:checked';
-
 // TOC dropdown styles
-#vector-toc-collapsed-button,
 .vector-sticky-header-toc,
 .vector-page-titlebar-toc {
 	display: none;
@@ -107,44 +104,16 @@
 	}
 
 	//
-	// No-JS TOC in page titlebar on narrow screens for anon users
+	// Applies simplified TOC styles to No-JS pinned TOC on narrow viewports
 	//
 	@media ( max-width: @max-width-tablet ) {
-		// Make the TOC appear in the page titlebar on small viewports
-		// even when the TOC location in the DOM is actually in the sidebar
-		// This case only happens for no-js anon users.
 		&.vector-feature-toc-pinned-clientpref-1 {
-			#vector-toc-collapsed-button {
-				display: block;
-			}
-
-			#vector-toc-pinned-container {
-				// Override TOC container's sticky positioning
-				position: relative;
-				.mixin-toc-unpinned();
+			.vector-sticky-pinned-container {
+				position: static;
 			}
 
 			.vector-toc {
-				display: none;
-				position: absolute;
-				top: 36px; // TOC button height
-				// FIXME: Don't use a magic number. This used to be tied to the private variable in core
-				// @icon-padding-md so perhaps this needs to make use of the flush classes?
-				left: -12px;
-				margin-top: @padding-top-content; // Account for padding-top from .mw-body
-				// FIXME: Collapsed TOC styles are not consistent with other vector dropdowns
-				border: @border-width-base @border-style-base @border-color-base;
-				z-index: @z-index-dropdown;
-
-				.vector-pinnable-header,
-				.vector-toc-contents {
-					padding-left: @padding-horizontal-dropdown-menu-item;
-				}
-			}
-
-			@{selector-nojs-collapsed-toc-open} ~ .mw-table-of-contents-container .vector-toc {
-				// Hide the TOC when the button is not checked
-				display: block;
+				max-height: 200px;
 			}
 		}
 	}
diff --git a/resources/skins.vector.styles/skin.less b/resources/skins.vector.styles/skin.less
index f7beb4d06..6d54fe1c8 100644
--- a/resources/skins.vector.styles/skin.less
+++ b/resources/skins.vector.styles/skin.less
@@ -57,5 +57,3 @@
 	@import './print.less';
 	@import './layouts/print.less';
 }
-
-@import './layouts/gradeC.less';
diff --git a/resources/skins.vector.zebra.styles/components/PinnableElement.less b/resources/skins.vector.zebra.styles/components/PinnableElement.less
index 57ca274e7..5fa83b8fe 100644
--- a/resources/skins.vector.zebra.styles/components/PinnableElement.less
+++ b/resources/skins.vector.zebra.styles/components/PinnableElement.less
@@ -17,31 +17,28 @@
 	margin-bottom: @grid-column-gap;
 	// Height is viewport height - row gap above and below the container.
 	max-height: ~'calc( 100vh - (@{grid-row-gap} * 2) )';
+	display: none;
+
+	// If there is no display grid support we remove pinnable elements
+	@supports ( display: grid ) {
+		display: block;
+	}
 }
 
-// Applies styles for making a pinned element sticky
 .vector-sticky-pinned-container {
+	// Applies styles for making a pinned element sticky
 	position: sticky;
 	top: @grid-row-gap;
 }
 
-/**
- * At lower resolutions, we want to hide the pinned containers when JS is enabled since these
- * elements collapse (become unpinned) at this resolution via PinnableElement.js.
- * Although this is handled in JS, this rule prevents the pinned menu from
- * appearing on pageload, at low resolutions, before the JS kicks in.
- *
- * When JS is disabled, we still want to hide the pinned containers at lower resolutions
- * to fallback to the gradeC, single column layout.
- */
 @media ( max-width: @max-width-tablet ) {
-	// .client-js and .client-nojs selectors needed to increase specificity to override gradeC.less
-	// Prefered to using !important as this still needs to be overridable to handle
-	// the anon, no-js TOC case found in layouts/toc/unpinned.less
-	&.client-js,
-	&.client-nojs {
-		.vector-pinned-container {
-			display: none;
-		}
+	/**
+ 	 * At lower resolutions, we want to hide the pinned containers when JS is enabled since these
+ 	 * elements collapse (become unpinned) at this resolution via PinnableElement.js.
+ 	 * Although this is handled in JS, this rule prevents the pinned menu from
+ 	 * appearing on pageload, at low resolutions, before the JS kicks in.
+ 	 */
+	&.client-js .vector-pinned-container {
+		display: none;
 	}
 }
diff --git a/resources/skins.vector.zebra.styles/components/TableOfContents.less b/resources/skins.vector.zebra.styles/components/TableOfContents.less
index a17ebd303..6dc061779 100644
--- a/resources/skins.vector.zebra.styles/components/TableOfContents.less
+++ b/resources/skins.vector.zebra.styles/components/TableOfContents.less
@@ -1,12 +1,3 @@
-/**
- * Checkbox hack used by collapsed TOC on narrow viewports for no JS users.
- * Although it's display:none, the :checked state is still affected when clicking
- * the associated label.
- */
-#vector-toc-collapsed-checkbox {
-	display: none;
-}
-
 .vector-toc {
 	.vector-pinnable-header {
 		padding-left: @toc-list-item-padding-start;
diff --git a/resources/skins.vector.zebra.styles/layouts/toc/unpinned.less b/resources/skins.vector.zebra.styles/layouts/toc/unpinned.less
index 15555def1..0ad353a67 100644
--- a/resources/skins.vector.zebra.styles/layouts/toc/unpinned.less
+++ b/resources/skins.vector.zebra.styles/layouts/toc/unpinned.less
@@ -1,16 +1,6 @@
-@selector-nojs-collapsed-toc-open: ~'#vector-toc-collapsed-checkbox:checked';
-
-// TOC dropdown styles
-#vector-toc-collapsed-button,
-.vector-sticky-header-toc,
-.vector-page-titlebar-toc {
-	display: none;
-	margin-right: 8px;
-}
-
 // TOC styles when unpinned
 .mixin-toc-unpinned() {
-	display: block;
+	margin-right: 8px;
 
 	.vector-dropdown-content {
 		// Override default dropdown max width
@@ -94,39 +84,16 @@
 	}
 
 	//
-	// No-JS TOC in page titlebar on narrow screens for anon users
+	// Applies simplified TOC styles to No-JS pinned TOC on narrow viewports
 	//
 	@media ( max-width: @max-width-tablet ) {
-		// Make the TOC appear in the page titlebar on small viewports
-		// even when the TOC location in the DOM is actually in the sidebar
-		// This case only happens for no-js anon users.
 		&.vector-feature-toc-pinned-clientpref-1 {
-			#vector-toc-collapsed-button {
-				display: block;
+			.vector-sticky-pinned-container {
+				position: static;
 			}
 
 			#vector-toc-pinned-container {
-				// Override TOC container's sticky positioning
-				position: relative;
-				.mixin-toc-unpinned();
-			}
-
-			.vector-toc {
-				display: none;
-				position: absolute;
-				top: 36px; // TOC button height
-				// FIXME: Don't use a magic number. This used to be tied to the private variable in core
-				// @icon-padding-md so perhaps this needs to make use of the flush classes?
-				left: -12px;
-				margin-top: @padding-top-content; // Account for padding-top from .mw-body
-				// FIXME: Collapsed TOC styles are not consistent with other vector dropdowns
-				border: @border-width-base @border-style-base @border-color-base;
-				z-index: @z-index-dropdown;
-			}
-
-			@{selector-nojs-collapsed-toc-open} ~ .vector-toc-landmark .vector-toc {
-				// Hide the TOC when the button is not checked
-				display: block;
+				max-height: 200px;
 			}
 		}
 	}