From 633c48bddfd7de25ea752ce89252c04ae39294fd Mon Sep 17 00:00:00 2001
From: Jan Drewniak <jan.drewniak@gmail.com>
Date: Wed, 11 Sep 2019 14:19:47 +0200
Subject: [PATCH] Utilize the mw-ui-icon-flush-left/right classes to align
 icons

Affected icons:
- hamburger menu
- "search|notifications|user" menu in page header
- notification "circle" icon
- page actions menu
- section edit icon

Bug: T229440
Change-Id: I5587855d0d9ecf2fac20ce16845e6749c26ab7c2
---
 includes/menu/Main/MainMenuDirector.php              |  4 +++-
 includes/menu/PageActions/PageActionsDirector.php    |  2 +-
 includes/menu/PageActions/ToolbarBuilder.php         |  2 +-
 includes/skins/SkinMinerva.php                       |  4 +++-
 includes/skins/minerva.mustache                      |  2 +-
 resources/skins.minerva.base.styles/footer.less      |  1 +
 resources/skins.minerva.base.styles/pageactions.less |  6 ++++++
 .../skins.minerva.base.styles/print/header.less      |  1 +
 resources/skins.minerva.base.styles/ui.less          | 10 +---------
 resources/skins.minerva.base.styles/userMenu.less    | 12 +++++++-----
 resources/skins.minerva.loggedin.styles/styles.less  |  1 +
 skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less  |  5 +++++
 skinStyles/mobile.startup/Overlay.less               |  4 ++++
 13 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/includes/menu/Main/MainMenuDirector.php b/includes/menu/Main/MainMenuDirector.php
index ed8dc2a47..214bbc99c 100644
--- a/includes/menu/Main/MainMenuDirector.php
+++ b/includes/menu/Main/MainMenuDirector.php
@@ -113,7 +113,9 @@ final class MainMenuDirector {
 		return Html::element( 'a', [
 				'title' => $title,
 				'href' => $url,
-				'class' => MinervaUI::iconClass( 'mainmenu', 'element', 'main-menu-button' ),
+				'class' => MinervaUI::iconClass(
+					'mainmenu', 'element', 'main-menu-button mw-ui-icon-flush-left'
+				),
 				'data-event-name' => 'ui.mainmenu',
 				'id' => 'mw-mf-main-menu-button',
 			], $tooltip );
diff --git a/includes/menu/PageActions/PageActionsDirector.php b/includes/menu/PageActions/PageActionsDirector.php
index 44fe73c2e..874750e3a 100644
--- a/includes/menu/PageActions/PageActionsDirector.php
+++ b/includes/menu/PageActions/PageActionsDirector.php
@@ -80,7 +80,7 @@ final class PageActionsDirector {
 				'item-id' => 'page-actions-overflow',
 				'checkboxID' => 'page-actions-overflow-checkbox',
 				'toggleID' => 'page-actions-overflow-toggle',
-				'toggleClass' => MinervaUI::iconClass( 'page-actions-overflow' ),
+				'toggleClass' => MinervaUI::iconClass( 'page-actions-overflow', 'element' ),
 				'listClass' => 'page-actions-overflow-list toggle-list__list--drop-down',
 				'text' => $this->messageLocalizer->msg( 'minerva-page-actions-overflow' ),
 				'analyticsEventName' => 'ui.overflowmenu',
diff --git a/includes/menu/PageActions/ToolbarBuilder.php b/includes/menu/PageActions/ToolbarBuilder.php
index 00b1c0fcd..b336eb90c 100644
--- a/includes/menu/PageActions/ToolbarBuilder.php
+++ b/includes/menu/PageActions/ToolbarBuilder.php
@@ -115,7 +115,7 @@ class ToolbarBuilder {
 				$this->title,
 				$this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->title ),
 				$this->messageLocalizer,
-				MinervaUI::iconClass( 'language-switcher', 'element', '' ) )
+				MinervaUI::iconClass( 'language-switcher', 'element' ) )
 			);
 		}
 
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 1c5e3bf97..8444fac51 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -240,7 +240,9 @@ class SkinMinerva extends SkinTemplate {
 				'data-section' => $section,
 				// Note visibility of the edit section link button is controlled by .edit-page in ui.less so
 				// we default to enabled even though this may not be true.
-				'class' => MinervaUI::iconClass( 'edit-enabled', 'element', 'edit-page' ),
+				'class' => MinervaUI::iconClass(
+					'edit-enabled', 'element', 'edit-page mw-ui-icon-flush-right'
+				),
 			], $message );
 			$html .= Html::closeElement( 'span' );
 			return $html;
diff --git a/includes/skins/minerva.mustache b/includes/skins/minerva.mustache
index 1e6e002f0..dd5442cec 100644
--- a/includes/skins/minerva.mustache
+++ b/includes/skins/minerva.mustache
@@ -21,7 +21,7 @@
 				<nav class="minerva-user-navigation" aria-labelledby="minerva-user-menu-toggle"> {{! See ToggleList's toggleID. }}
 					<div>{{{searchButton}}}</div>
 					{{^isAnon}}
-							<div class="minerva-user-notifications">
+							<div class="mw-ui-icon-element minerva-user-notifications">
 								{{#userNotificationsData}}{{>userNotifications}}{{/userNotificationsData}}
 							</div>
 					{{/isAnon}}
diff --git a/resources/skins.minerva.base.styles/footer.less b/resources/skins.minerva.base.styles/footer.less
index f0c118849..e2aa0e9d0 100644
--- a/resources/skins.minerva.base.styles/footer.less
+++ b/resources/skins.minerva.base.styles/footer.less
@@ -23,6 +23,7 @@ footer {
 		display: block;
 		color: @lastModifiedBarTextColor;
 		transition: background-color 0.2s ease, color 0.2s ease;
+		padding: 0 @contentPadding;
 	}
 
 	.last-modifier-tagline {
diff --git a/resources/skins.minerva.base.styles/pageactions.less b/resources/skins.minerva.base.styles/pageactions.less
index 92ab47863..223515218 100644
--- a/resources/skins.minerva.base.styles/pageactions.less
+++ b/resources/skins.minerva.base.styles/pageactions.less
@@ -55,6 +55,12 @@
 .page-actions-menu__list-item:first-child {
 	flex-grow: 1;
 	justify-content: flex-start;
+	margin-left: @contentPadding/-2;
+}
+
+// position the last icon with the edge of the content container
+.page-actions-menu__list-item:last-child {
+	margin-right: @contentPadding/-2;
 }
 
 .page-actions-overflow-list {
diff --git a/resources/skins.minerva.base.styles/print/header.less b/resources/skins.minerva.base.styles/print/header.less
index 06362c298..4fa583212 100644
--- a/resources/skins.minerva.base.styles/print/header.less
+++ b/resources/skins.minerva.base.styles/print/header.less
@@ -8,6 +8,7 @@
 	border-top: 0;
 	display: block;
 	height: auto;
+	padding: 0 @contentPadding;
 
 	.branding-box {
 		padding-left: 0 !important;
diff --git a/resources/skins.minerva.base.styles/ui.less b/resources/skins.minerva.base.styles/ui.less
index 3f4b7fca4..c1c60a03d 100644
--- a/resources/skins.minerva.base.styles/ui.less
+++ b/resources/skins.minerva.base.styles/ui.less
@@ -44,6 +44,7 @@ main {
 
 .header-container {
 	border-bottom: 1px solid @colorGray12;
+	padding: 0 @contentPadding;
 
 	&.header-chrome {
 		background-color: @chromeColor;
@@ -80,7 +81,6 @@ main {
 	margin-top: @headerMarginTop;
 
 	> div {
-		width: @siteHeaderHeight;
 		position: relative;
 		vertical-align: middle;
 		display: table-cell;
@@ -204,14 +204,6 @@ input.search {
 	.section-heading {
 		border-bottom: 1px solid @colorGray14;
 		margin-bottom: @headingMargin;
-
-		.indicator {
-			margin-left: -@iconGutterWidth;
-			// FIXME: Can be removed when https://gerrit.wikimedia.org/r/535835 is merged
-			font-size: initial;
-			// FIXME: Can be removed when https://gerrit.wikimedia.org/r/535835 is merged
-			vertical-align: initial;
-		}
 	}
 
 	.mw-parser-output > h1,
diff --git a/resources/skins.minerva.base.styles/userMenu.less b/resources/skins.minerva.base.styles/userMenu.less
index 3bc3b52b4..9b40b050e 100644
--- a/resources/skins.minerva.base.styles/userMenu.less
+++ b/resources/skins.minerva.base.styles/userMenu.less
@@ -14,9 +14,11 @@
 	justify-content: flex-end;
 	// Show the list relative the button.
 	position: relative;
-
-	.toggle-list {
-		width: @menuButtonIconSize;
-		display: inline-block;
-	}
+}
+
+// Targets the last direct descendant of the navigation menu
+// (i.e. the last icon, or button, or div, etc).
+// and align with the content container.
+.minerva-user-navigation > *:last-child {
+	margin-right: @contentPadding/-2;
 }
diff --git a/resources/skins.minerva.loggedin.styles/styles.less b/resources/skins.minerva.loggedin.styles/styles.less
index 76761678e..900eefa4f 100644
--- a/resources/skins.minerva.loggedin.styles/styles.less
+++ b/resources/skins.minerva.loggedin.styles/styles.less
@@ -17,6 +17,7 @@
 .notification-count {
 	@circleSize: 24px;
 	@borderSize: 2px;
+	display: inline-block;
 	margin: auto;
 	height: @circleSize;
 	background: @notificationBackgroundRead;
diff --git a/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less b/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
index c7438c9a1..12bc23dad 100644
--- a/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
+++ b/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
@@ -80,6 +80,11 @@
 	}
 }
 
+/**
+ * When aligning a series of similar icons (e.g. list items)
+ * prefer using a `:first-child` or `:last-child` selector.
+ * The flush-left/right classes are appropriate when that's unavailable.
+ */
 .mw-ui-icon-flush-left {
 	margin-left: -@icon-padding-md;
 }
diff --git a/skinStyles/mobile.startup/Overlay.less b/skinStyles/mobile.startup/Overlay.less
index 141834633..fa8b5a1ba 100644
--- a/skinStyles/mobile.startup/Overlay.less
+++ b/skinStyles/mobile.startup/Overlay.less
@@ -17,6 +17,10 @@
 	}
 }
 
+.overlay-header-container {
+	padding: 0;
+}
+
 .overlay-header {
 	height: @overlayHeaderHeight;