refactor(core): clean up personal menu implementation

includes/Partials/Header.php

@@ -52,6 +52,7 @@ final class Header extends Partial {
 		$user = $this->skin->getUser();
 
 		// Move the Echo badges out of default list
+		// TODO: Remove notifications since MW 1.36 from buildPersonalUrls
 		if ( isset( $personalTools['notifications-alert'] ) ) {
 			unset( $personalTools['notifications-alert'] );
 		}
@@ -59,11 +60,13 @@ final class Header extends Partial {
 			unset( $personalTools['notifications-notice'] );
 		}
 
+		// TODO: Decorate personal menu for anon users in the future
 		if ( $user->isRegistered() ) {
-			$personalTools = $this->addUserInfoToMenu( $personalTools, $user );
+			$personalTools = $this->decoratePersonalMenu( $personalTools, $user );
 		}
 
 		$personalMenu = $this->skin->getPortletData( 'personal', $personalTools );
+
 		// Hide label for personal tools
 		$personalMenu[ 'label-class' ] = 'screen-reader-text';
 
@@ -78,21 +81,21 @@ final class Header extends Partial {
 	 *
 	 * @return array
 	 */
-	public function getExtratools(): array {
+	public function getNotifications(): array {
 		$personalTools = $this->skin->getPersonalToolsForMakeListItem(
 			$this->skin->buildPersonalUrlsPublic()
 		);
 
 		// Create the Echo badges
-		$extraTools = [];
+		$notifications = [];
 		if ( isset( $personalTools['notifications-alert'] ) ) {
-			$extraTools['notifications-alert'] = $personalTools['notifications-alert'];
+			$notifications['notifications-alert'] = $personalTools['notifications-alert'];
 		}
 		if ( isset( $personalTools['notifications-notice'] ) ) {
-			$extraTools['notifications-notice'] = $personalTools['notifications-notice'];
+			$notifications['notifications-notice'] = $personalTools['notifications-notice'];
 		}
 
-		$html = $this->skin->getPortletData( 'personal-extra', $extraTools );
+		$html = $this->skin->getPortletData( 'notifications', $notifications );
 
 		// Hide label for extra tools
 		$html[ 'label-class' ] = 'screen-reader-text';
@@ -122,71 +125,84 @@ final class Header extends Partial {
 	}
 
 	/**
-	 * Adds user info to the personal menu
+	 * Decorate the personal menu
-	 * Adds all explicit user groups as links to the personal menu
-	 * Links are added right below the user page link
-	 * Wrapped in an <li> element with id 'pt-usergroups'
 	 *
-	 * @param array $originalUrls The original personal tools urls
+	 * @param array $personalTools The original personal tools urls
 	 * @param User $user
 	 *
 	 * @return array
 	 */
-	private function addUserInfoToMenu( $originalUrls, $user ) {
+	private function decoratePersonalMenu( $personalTools, $user ): array {
-		$personalTools = [];
+		$personalMenu = [
+			'userpage' => $personalTools['userpage'] ?? null,
+			'usergroups' => $this->getUserGroupsData( $personalTools, $user ),
+			'usercontris' => $this->getUserContributionsData( $user ),
+		];
 
+		return array_merge( $personalMenu, $personalTools );
+	}
+
+	/**
+	 * Build and return user groups data 
+	 *
+	 * @param array $personalTools The original personal tools urls
+	 * @param User $user
+	 *
+	 * @return array
+	 */
+	private function getUserGroupsData( $personalTools, $user ): array {
 		// This does not return implicit groups
 		$groups = MediaWikiServices::getInstance()->getUserGroupManager()->getUserGroups( $user );
 
-		// Return user edits
+		if ( empty( $groups ) ) {
-		$edits = MediaWikiServices::getInstance()->getUserEditTracker()->getUserEditCount( $user );
+			return null;
+		}
 
 		// Add user group
-		if ( !empty( $groups ) ) {
+		$groupLinks = [];
-			$userPage = array_shift( $originalUrls );
+		$msgName = 'group-%s';
-			$groupLinks = [];
-			$msgName = 'group-%s';
 
-			foreach ( $groups as $group ) {
+		foreach ( $groups as $group ) {
-				$groupPage = Title::newFromText(
+			$groupPage = Title::newFromText(
-					$this->skin->msg( sprintf( $msgName, $group ) )->text(),
+				$this->skin->msg( sprintf( $msgName, $group ) )->text(),
-					NS_PROJECT
+				NS_PROJECT
-				);
+			);
 
-				$groupLinks[$group] = [
+			$groupLinks[$group] = [
-					'msg' => sprintf( $msgName, $group ),
+				'msg' => sprintf( $msgName, $group ),
-					// Nullpointer should not happen
+				// Nullpointer should not happen
-					'href' => $groupPage->getLinkURL(),
+				'href' => $groupPage->getLinkURL(),
-					'tooltiponly' => true,
+				'tooltiponly' => true,
-					'id' => sprintf( $msgName, $group ),
+				'id' => sprintf( $msgName, $group ),
-					// 'exists' => $groupPage->exists() - This will add an additional DB call
+				// 'exists' => $groupPage->exists() - This will add an additional DB call
-				];
-			}
-
-			$userGroups = [
-				'id' => 'pt-usergroups',
-				'links' => $groupLinks
 			];
 		}
 
-		$userContris = [
+		return [
-			'text' => $this->skin->msg( 'usereditcount' )->numParams( sprintf( '%s', number_format( $edits, 0 ) ) ),
+			'id' => 'pt-usergroups',
+			'links' => $groupLinks
+		];
+	}
+
+	/**
+	 * Build and return user contributions data
+	 *
+	 * @param User $user
+	 *
+	 * @return array
+	 */
+	private function getUserContributionsData( $user ): array {
+		// Return user edits
+		$edits = MediaWikiServices::getInstance()->getUserEditTracker()->getUserEditCount( $user );
+
+		if ( empty( $edits ) ) {
+			return null;
+		}
+
+		return [
+			'text' => $this->skin->msg( 'usereditcount' )
+				->numParams( sprintf( '%s', number_format( $edits, 0 ) ) ),
 			'id' => 'pt-usercontris'
 		];
-
-		// The following defines the order of links added
-		if ( isset( $userPage ) ) {
-			$personalTools['userpage'] = $userPage;
-		}
-		if ( isset( $userGroups ) ) {
-			$personalTools['usergroups'] = $userGroups;
-		}
-		$personalTools['usercontris'] = $userContris;
-
-		foreach ( $originalUrls as $key => $url ) {
-			$personalTools[$key] = $url;
-		}
-
-		return $personalTools;
 	}
 }

includes/SkinCitizen.php

@@ -106,7 +106,7 @@ class SkinCitizen extends SkinMustache {
 
 			'data-header' => [
 				'data-drawer' => $drawer->getDrawerTemplateData(),
-				'data-extratools' => $header->getExtraTools(),
+				'data-notifications' => $header->getNotifications(),
 				'data-personal-menu' => $header->buildPersonalMenu(),
 				'data-search-box' => $header->buildSearchProps(),
 				'msg-citizen-jumptotop' => $this->msg( 'citizen-jumptotop' )->text() . ' [home]',

resources/skins.citizen.styles/Header.less

@@ -174,8 +174,8 @@
 	}
 }
 
-// User icon bar
+// Notifications
-#p-personal-extra {
+#p-notifications {
 	ul {
 		display: flex;
 		height: var( --height-header );

templates/Header.mustache

@@ -10,7 +10,7 @@
 			<a class="citizen-header__title citizen-header__title--page" href="#top" title="{{msg-citizen-jumptotop}}"><div>{{{html-title}}}</div></a>
 		</div>
 		<div class="citizen-header__end">
-			{{#data-extratools}}{{>Menu}}{{/data-extratools}}
+			{{#data-notifications}}{{>Menu}}{{/data-notifications}}
 			{{#data-personal-menu}}{{>PersonalMenu}}{{/data-personal-menu}}
 		</div>
 	</div>