From 109b24e0ee06dd0052cd06309dfd3a8fbef9d909 Mon Sep 17 00:00:00 2001
From: jdlrobson <jdlrobson@gmail.com>
Date: Fri, 13 Sep 2019 13:29:14 -0700
Subject: [PATCH] Adjust notification icon to be the same size as the other
 icons

Bug: T232011
Change-Id: I0300685d38551bc9ee2c2bd32d6348f32d4df4de
---
 includes/skins/SkinMinerva.php                       |  2 +-
 includes/skins/minerva.mustache                      |  2 +-
 minerva.less/minerva.variables.less                  | 11 +++++++++++
 resources/skins.minerva.base.styles/userMenu.less    |  7 -------
 resources/skins.minerva.loggedin.styles/styles.less  | 12 ++++++------
 resources/skins.minerva.scripts/NotificationBadge.js |  1 +
 skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less  |  9 ---------
 tests/phpunit/skins/SkinMinervaTest.php              |  3 ++-
 8 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 8444fac51..c61c3507e 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -405,7 +405,7 @@ class SkinMinerva extends SkinTemplate {
 
 			$tpl->set( 'userNotificationsData', [
 				'notificationIconClass' => MinervaUI::iconClass( 'bellOutline-base20',
-					'element', '', 'wikimedia' ),
+					'element', 'mw-ui-icon-flush-right', 'wikimedia' ),
 				'title' => $notificationsMsg,
 				'url' => $url,
 				'notificationCountRaw' => $count,
diff --git a/includes/skins/minerva.mustache b/includes/skins/minerva.mustache
index dd5442cec..1e6e002f0 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="mw-ui-icon-element minerva-user-notifications">
+							<div class="minerva-user-notifications">
 								{{#userNotificationsData}}{{>userNotifications}}{{/userNotificationsData}}
 							</div>
 					{{/isAnon}}
diff --git a/minerva.less/minerva.variables.less b/minerva.less/minerva.variables.less
index 7ffa84991..1e70d6db8 100644
--- a/minerva.less/minerva.variables.less
+++ b/minerva.less/minerva.variables.less
@@ -35,6 +35,17 @@
 
 @skinContentBgColor: #fff;
 
+//
+// Icons
+//
+// Small icon - 16px square, with 9px padding (total 34px square).
+@icon-size-sm: unit( 16 / @font-size-browser, em );
+@icon-padding-sm: unit( 9 / @font-size-browser, em );
+
+// Medium icon - 16px square, with 9px padding (total 34px square).
+@icon-size-md: unit( 20 / @font-size-browser, em ); // 20px;
+@icon-padding-md: unit( 10 / @font-size-browser, em ); // 44px;
+
 // Navigation Drawers
 @rightDrawerWidth: 60%;
 @primaryNavBackgroundColor: @colorGray14;
diff --git a/resources/skins.minerva.base.styles/userMenu.less b/resources/skins.minerva.base.styles/userMenu.less
index 9b40b050e..fad3e6fad 100644
--- a/resources/skins.minerva.base.styles/userMenu.less
+++ b/resources/skins.minerva.base.styles/userMenu.less
@@ -15,10 +15,3 @@
 	// Show the list relative the button.
 	position: relative;
 }
-
-// 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 900eefa4f..14f18fbbd 100644
--- a/resources/skins.minerva.loggedin.styles/styles.less
+++ b/resources/skins.minerva.loggedin.styles/styles.less
@@ -15,21 +15,20 @@
 }
 
 .notification-count {
-	@circleSize: 24px;
-	@borderSize: 2px;
 	display: inline-block;
 	margin: auto;
-	height: @circleSize;
 	background: @notificationBackgroundRead;
 	color: @notificationColorRead;
+	// same as mw-ui-icon-flush-right
+	margin-right: -@icon-padding-md;
 	cursor: pointer;
 
 	.circle {
 		border-radius: 50%;
-		border: @borderSize solid @notificationColorRead;
+		border: 2px solid @notificationColorRead;
 		margin: auto;
-		height: @circleSize - @borderSize;
-		width: @circleSize - @borderSize;
+		height: @icon-size-md;
+		width: @icon-size-md;
 
 		/* stylelint-disable declaration-block-no-duplicate-properties */
 		// Center the text number inside the circle
@@ -52,6 +51,7 @@
 
 	&.notification-unseen {
 		color: @notificationColorUnread;
+		padding: @icon-padding-md;
 
 		.circle {
 			background: @notificationBackgroundUnread;
diff --git a/resources/skins.minerva.scripts/NotificationBadge.js b/resources/skins.minerva.scripts/NotificationBadge.js
index f04b9ab79..5832a88a5 100644
--- a/resources/skins.minerva.scripts/NotificationBadge.js
+++ b/resources/skins.minerva.scripts/NotificationBadge.js
@@ -7,6 +7,7 @@
 		Icon = mobile.Icon,
 		notificationIcon = new Icon( {
 			name: 'bellOutline-base20',
+			additionalClassNames: 'mw-ui-icon-flush-right',
 			glyphPrefix: 'wikimedia'
 		} );
 
diff --git a/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less b/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
index 12bc23dad..5669f1003 100644
--- a/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
+++ b/skinStyles/mediawiki.ui.icon/mediawiki.ui.icon.less
@@ -16,15 +16,6 @@
  */
 
 @import '../../minerva.less/minerva.variables.less';
-
-// Small icon - 16px square, with 9px padding (total 34px square).
-@icon-size-sm: unit( 16 / @font-size-browser, em );
-@icon-padding-sm: unit( 9 / @font-size-browser, em );
-
-// Medium icon - 16px square, with 9px padding (total 34px square).
-@icon-size-md: unit( 20 / @font-size-browser, em ); // 20px;
-@icon-padding-md: unit( 10 / @font-size-browser, em ); // 44px;
-
 /**
  * Mixin for a pseudo-element with a background image.
  */
diff --git a/tests/phpunit/skins/SkinMinervaTest.php b/tests/phpunit/skins/SkinMinervaTest.php
index f53fae5ae..94d4aab43 100644
--- a/tests/phpunit/skins/SkinMinervaTest.php
+++ b/tests/phpunit/skins/SkinMinervaTest.php
@@ -272,7 +272,8 @@ class SkinMinervaTest extends MediaWikiTestCase {
 	) {
 		return [
 			'notificationIconClass' =>
-				MinervaUI::iconClass( 'bellOutline-base20', 'element', '', 'wikimedia' ),
+				MinervaUI::iconClass( 'bellOutline-base20', 'element',
+					'mw-ui-icon-flush-right', 'wikimedia' ),
 			'title' => $notificationsMsg,
 			'url' => SpecialPage::getTitleFor( $notificationsTitle )
 				->getLocalURL(