From 2ccc975f0ab2c64218467239a6de87c30c691d9d Mon Sep 17 00:00:00 2001
From: AronDemian <AronManning5@gmail.com>
Date: Tue, 23 Jun 2020 04:43:28 +0200
Subject: [PATCH] [modern] Move sidebar checkbox above content, sidebar button
 into header

- Sidebar's checkbox hack CSS selectors adjusted.

Bug: T246420
Change-Id: I1cc1ad4eb1938c4931b1ae881b3878fbd6bb7a39
---
 includes/VectorTemplate.php                |  6 +-
 includes/templates/Sidebar.mustache        | 18 -----
 includes/templates/skin.mustache           | 23 +++++-
 resources/skins.vector.styles/Logo.less    |  2 -
 resources/skins.vector.styles/Sidebar.less | 94 ++++++++++------------
 skin.json                                  |  4 +-
 6 files changed, 69 insertions(+), 78 deletions(-)

diff --git a/includes/VectorTemplate.php b/includes/VectorTemplate.php
index 0a9383eb..0de38789 100644
--- a/includes/VectorTemplate.php
+++ b/includes/VectorTemplate.php
@@ -177,6 +177,9 @@ class VectorTemplate extends BaseTemplate {
 			'main-page-href' => $mainPageHref,
 
 			'data-sidebar' => $this->buildSidebar(),
+			// [todo] fetch user preference when logged in (T246427).
+			'sidebar-visible' => true,
+			'msg-vector-action-toggle-sidebar' => $this->msg( 'vector-action-toggle-sidebar' )->text(),
 		] + $this->getMenuProps();
 
 		// The following logic is unqiue to Vector (not used by legacy Vector) and
@@ -343,9 +346,6 @@ class VectorTemplate extends BaseTemplate {
 			'array-portals-rest' => array_slice( $props, 1 ),
 			'data-portals-first' => $firstPortal,
 			'data-portals-languages' => $languages,
-			'msg-vector-action-toggle-sidebar' => $this->msg( 'vector-action-toggle-sidebar' )->text(),
-			// [todo] fetch user preference when logged in (T246427).
-			'sidebar-visible' => true
 		];
 	}
 
diff --git a/includes/templates/Sidebar.mustache b/includes/templates/Sidebar.mustache
index 1fd80e87..73f254ee 100644
--- a/includes/templates/Sidebar.mustache
+++ b/includes/templates/Sidebar.mustache
@@ -7,26 +7,8 @@
 	MenuDefinition data-portals-first
 	MenuDefinition[] array-portals-rest
 	emphasized-sidebar-action data-emphasized-sidebar-action For displaying an emphasized action in the sidebar.
-	string msg-vector-action-toggle-sidebar The label used by the sidebar button.
-	boolean sidebar-visible For users that want to see the sidebar on initial render, this should be
-	  true.
 }}
 
-<input
-		type="checkbox"
-		id="mw-sidebar-checkbox"
-		class="mw-checkbox-hack-checkbox"
-		role="button"
-		{{#sidebar-visible}}checked{{/sidebar-visible}}
-		aria-labelledby="mw-sidebar-button"
-		aria-controls="mw-panel">
-<label
-	id="mw-sidebar-button"
-	class="mw-checkbox-hack-button mw-ui-icon mw-ui-icon-element"
-	for="mw-sidebar-checkbox"
-	data-event-name="ui.sidebar">
-	{{msg-vector-action-toggle-sidebar}}
-</label>
 <div id="mw-panel" class="mw-sidebar mw-checkbox-hack-target">
 	{{#data-portals-first}}{{>Menu}}{{/data-portals-first}}
 	{{#data-emphasized-sidebar-action}}
diff --git a/includes/templates/skin.mustache b/includes/templates/skin.mustache
index 348a45b4..90deb703 100644
--- a/includes/templates/skin.mustache
+++ b/includes/templates/skin.mustache
@@ -29,6 +29,9 @@
 	MenuDefinition data-page-actions
 	MenuDefinition data-page-actions-more
 	object data-search-box. See SearchBox.mustache for documentation.
+	boolean sidebar-visible For users that want to see the sidebar on initial render, this should be
+	  true.
+	string msg-vector-action-toggle-sidebar The label used by the sidebar button.
 	object data-sidebar. See Sidebar.mustache for documentation.
 	object data-footer for footer template partial. see Footer.mustache for documentation.
 	string html-printtail HTML to render at the end of the page contained necessary script tags for
@@ -38,6 +41,16 @@
 {{{html-headelement}}}
 
 <div id="mw-page-base" class="mw-header-placeholder noprint"></div>
+
+<input
+	type="checkbox"
+	id="mw-sidebar-checkbox"
+	class="mw-checkbox-hack-checkbox"
+	role="button"
+	{{#sidebar-visible}}checked{{/sidebar-visible}}
+	aria-labelledby="mw-sidebar-button"
+	aria-controls="mw-panel">
+
 {{! `role` is unnecessary but kept to support selectors in any gadgets or user styles. }}
 <!-- Please do not use role attribute as CSS selector, it is deprecated. -->
 <main id="content" class="mw-body" role="main">
@@ -72,7 +85,15 @@
 {{{html-dataAfterContent}}}
 
 <header class="mw-header">
-{{>Logo}}
+	<label
+		id="mw-sidebar-button"
+		class="mw-checkbox-hack-button mw-ui-icon mw-ui-icon-element"
+		for="mw-sidebar-checkbox"
+		data-event-name="ui.sidebar">
+		{{msg-vector-action-toggle-sidebar}}
+	</label>
+
+	{{>Logo}}
 </header>
 
 <div id="mw-navigation">
diff --git a/resources/skins.vector.styles/Logo.less b/resources/skins.vector.styles/Logo.less
index 6d35b384..eee96d05 100644
--- a/resources/skins.vector.styles/Logo.less
+++ b/resources/skins.vector.styles/Logo.less
@@ -9,8 +9,6 @@
 	height: 100%;
 	// Center vertically.
 	align-items: center;
-	// Make room for the sidebar menu button.
-	margin-left: @size-sidebar-button;
 }
 
 .mw-logo-icon {
diff --git a/resources/skins.vector.styles/Sidebar.less b/resources/skins.vector.styles/Sidebar.less
index e247187b..88fa2376 100644
--- a/resources/skins.vector.styles/Sidebar.less
+++ b/resources/skins.vector.styles/Sidebar.less
@@ -4,6 +4,8 @@
 @import 'legacy/Sidebar.less';
 @import 'checkboxHack.less';
 
+@margin-horizontal-sidebar-button-icon: 12px;
+
 .mw-sidebar-action {
 	// Align with the portal heading/links
 	// `.portal` + `.portal .body`
@@ -15,65 +17,53 @@
 	font-weight: bold;
 }
 
-// FIXME please add a class, .mw-navigation, and use that here and below instead of this identifier.
-#mw-navigation {
-	.mw-checkbox-hack-checkbox,
-	.mw-checkbox-hack-button {
-		// The icon is only 44px tall but the header is 50px. Offset by the difference from the logo
-		// icon and center with respect to the header.
-		top: @margin-top-header + ( ( @height-logo-icon - @size-sidebar-button ) / 2 );
-		// FIXME: Replace by proper calculation and variable.
-		left: 14px;
+#mw-sidebar-button {
+	// Override minimum dimensions set by mw-ui-icon.mw-ui-icon-element.
+	min-width: @size-sidebar-button;
+	min-height: @size-sidebar-button;
+	width: @size-sidebar-button;
+	height: @size-sidebar-button;
+	border: 1px solid transparent;
+	border-radius: @border-radius-base;
+	margin-left: -@margin-horizontal-sidebar-button-icon;
+	margin-right: @margin-horizontal-sidebar-button-icon; // Accidentally the same.
+
+	&:before {
+		// FIXME: the icon itself is supposed to be 20px. mediawiki.ui uses 24px.
+		// As soon as mediawiki.ui is standardized, remove this override. See T191021.
+		min-width: 20px;
+		min-height: 20px;
+		height: 100%;
+		// Center it horizontally.
+		margin: 0 @margin-horizontal-sidebar-button-icon;
+		// Equals `#555`, closest to `#54595d` on background-color `#fff`.
+		opacity: 0.67;
 	}
 
-	.mw-checkbox-hack-button {
-		position: absolute;
-		z-index: @z-index-sidebar-button;
-		// Override minimum dimensions set by mw-ui-icon.mw-ui-icon-element.
-		min-width: @size-sidebar-button;
-		min-height: @size-sidebar-button;
-		width: @size-sidebar-button;
-		height: @size-sidebar-button;
-		border: 1px solid transparent;
-		border-radius: @border-radius-base;
+	&:hover {
+		background-color: @background-color-frameless--hover;
 
 		&:before {
-			// FIXME: the icon itself is supposed to be 20px. mediawiki.ui uses 24px.
-			// As soon as mediawiki.ui is standardized, remove this override. See T191021.
-			min-width: 20px;
-			min-height: 20px;
-			height: 100%;
-			// Center it horizontally.
-			margin: 0 12px;
-			// Equals `#555`, closest to `#54595d` on background-color `#fff`.
-			opacity: 0.67;
-		}
-
-		&:hover {
-			background-color: @background-color-frameless--hover;
-
-			&:before {
-				opacity: 1;
-			}
+			opacity: 1;
 		}
 	}
+}
 
-	.mw-checkbox-hack-checkbox:focus ~ .mw-checkbox-hack-button {
-		// Next two rules from OOUI, frameless, icon-only button widget.
-		border-color: @color-primary;
-		.box-shadow( inset 0 0 0 1px @color-primary );
-	}
+#mw-sidebar-checkbox:focus ~ .mw-header .mw-checkbox-hack-button {
+	// Next two rules from OOUI, frameless, icon-only button widget.
+	border-color: @color-primary;
+	.box-shadow( inset 0 0 0 1px @color-primary );
+}
 
-	// Use the MediaWiki checkbox hack class from checkboxHack.less. This class exists on the
-	// checkbox input for the menu panel.
-	.mw-checkbox-hack-checkbox:not( :checked ) ~ .mw-sidebar {
-		// Turn off presentation so that screen readers get the same effect as visually hiding.
-		// Visibility and opacity can be animated. If animation is unnecessary, use `display: none`
-		// instead to avoid hidden rendering.
-		visibility: hidden;
-		opacity: 0;
-		.transform( translateX( -100% ) );
-	}
+// Use the MediaWiki checkbox hack class from checkboxHack.less. This class exists on the
+// checkbox input for the menu panel.
+#mw-sidebar-checkbox:not( :checked ) ~ #mw-navigation .mw-sidebar {
+	// Turn off presentation so that screen readers get the same effect as visually hiding.
+	// Visibility and opacity can be animated. If animation is unnecessary, use `display: none`
+	// instead to avoid hidden rendering.
+	visibility: hidden;
+	opacity: 0;
+	.transform( translateX( -100% ) );
 }
 
 // Disable transitions on page load. No-JS users will unfortunately miss out. A similar pattern is
@@ -89,7 +79,7 @@
 	}
 
 	// Enable sidebar button transitions.
-	#mw-navigation .mw-checkbox-hack-button {
+	#mw-sidebar-button {
 		.transition(
 			background-color @transition-duration-base,
 			border-color @transition-duration-base,
diff --git a/skin.json b/skin.json
index ea108fc1..d5bfe34b 100644
--- a/skin.json
+++ b/skin.json
@@ -63,13 +63,13 @@
 			"class": "ResourceLoaderImageModule",
 			"selector": "{name}",
 			"images": {
-				"#mw-navigation .mw-checkbox-hack-button:before": {
+				"#mw-sidebar-button:before": {
 					"file": {
 						"ltr": "resources/skins.vector.icons/chevronHorizontal-ltr.svg",
 						"rtl": "resources/skins.vector.icons/chevronHorizontal-rtl.svg"
 					}
 				},
-				"#mw-navigation .mw-checkbox-hack-checkbox:not( :checked ) ~ .mw-checkbox-hack-button:before": "resources/skins.vector.icons/menu.svg"
+				"#mw-sidebar-checkbox:not( :checked ) ~ .mw-header #mw-sidebar-button:before": "resources/skins.vector.icons/menu.svg"
 			}
 		},
 		"skins.vector.styles.responsive": {