Page MenuHomePhabricator

Minerva menus should use data from core hooks (deprecate MobileMenu hook)
Open, HighPublic

Description

Extensions can use the methods onSkinTemplateNavigation::Universal and onSidebarBeforeOutput to modify menus in all skins except Minerva.

In Minerva, a MobileMenu hook is provided to allow modifications to the menu.

It's proposed that we drop the MobileMenu hook and allow extensions to use those hooks to make modifications instead.

Acceptance criteria

  • Update DefaultMainMenuBuilder and AdvancedUserMenuBuilder to source personal menu items from SkinTemplateNavigation::Universal (user-menu group) https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/753596
  • Update DefaultMainMenuBuilder and AdvancedUserMenuBuilder to source home from SkinTemplateNavigation::Universal (user-menu group)
  • Extension:GrowthExperiments should replace usages of MobileMenu with SkinTemplateNavigation::Universal
  • Hard deprecate MobileMenu hook

Event Timeline

Jdlrobson renamed this task from Minerv menus should use data from core hooks to Minerva menus should use data from core hooks.Sep 22 2021, 3:07 PM
Jdlrobson added a project: MinervaNeue.

Change 722730 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Begin using core menu definitions for sourcing menu data

https://gerrit.wikimedia.org/r/722730

I think it makes sense to work towards deprecating the MobileMenu hook, so that onSkinTemplateNavigation::Universal and onSidebarBeforeOutput can become the standard way for extensions to modify menus. MobileMenu also doesn't seem super well documented, so moving forward with a consistent approach seems good to me!

Change 722729 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/GrowthExperiments@master] Update menu items via hook

https://gerrit.wikimedia.org/r/722729

Change 722729 abandoned by Jdlrobson:

[mediawiki/extensions/GrowthExperiments@master] Update menu items via hook

Reason:

Not working on this right now. Will pick this up again soon. Please let me know if there is any problems with working towards this from the Growth Team side. We don't want to have mobile specific hooks for menu modification on the longer term.

https://gerrit.wikimedia.org/r/722729

Example (To port the menu that is shown to logged in AMC users:)

Screen Shot 2022-01-05 at 8.56.20 AM.png (824×444 px, 86 KB)

diff --git a/includes/Menu/Entries/SingleMenuEntry.php b/includes/Menu/Entries/SingleMenuEntry.php
index 9ab4e0d45..951e068a8 100644
--- a/includes/Menu/Entries/SingleMenuEntry.php
+++ b/includes/Menu/Entries/SingleMenuEntry.php
@@ -44,8 +44,9 @@ class SingleMenuEntry implements IMenuEntry {
 	 * @param string $text Text to show on menu element
 	 * @param string $url URL menu element points to
 	 * @param string $className Additional CSS class names.
+	 * @param string|null $icon icon to use
 	 */
-	public function __construct( $name, $text, $url, $className = '' ) {
+	public function __construct( $name, $text, $url, $className = '', $icon = null ) {
 		$this->name = $name;
 		if ( $className ) {
 			$className .= ' ';
@@ -53,7 +54,7 @@ class SingleMenuEntry implements IMenuEntry {
 		$className .= 'menu__item--' . $name;
 
 		$this->attributes = [
-			'icon' => null,
+			'icon' => $icon,
 			'text' => $text,
 			'href' => $url,
 			'class' => $className
diff --git a/includes/Menu/Main/DefaultMainMenuBuilder.php b/includes/Menu/Main/DefaultMainMenuBuilder.php
index 7bf529762..7eaeb8358 100644
--- a/includes/Menu/Main/DefaultMainMenuBuilder.php
+++ b/includes/Menu/Main/DefaultMainMenuBuilder.php
@@ -76,7 +76,6 @@ final class DefaultMainMenuBuilder implements IMainMenuBuilder {
 	public function getGroups(): array {
 		$donate = $this->showDonateLink ?
 			BuilderUtil::getDonateGroup( $this->definitions ) : null;
-
 		$groups = [
 			BuilderUtil::getDiscoveryTools( $this->definitions ),
 			$this->getPersonalTools( $this->showMobileOptions ),
diff --git a/includes/Menu/User/AdvancedUserMenuBuilder.php b/includes/Menu/User/AdvancedUserMenuBuilder.php
index 8f8089812..0b38bd895 100644
--- a/includes/Menu/User/AdvancedUserMenuBuilder.php
+++ b/includes/Menu/User/AdvancedUserMenuBuilder.php
@@ -67,31 +67,26 @@ final class AdvancedUserMenuBuilder implements IUserMenuBuilder {
 	 */
 	public function getGroup( array $personalTools ): Group {
 		$group = new Group( 'p-personal' );
-		$group->insertEntry( new ProfileMenuEntry( $this->user ) );
-		$talkPage = $this->user->getUserPage()->getTalkPageIfDefined();
-		if ( $talkPage ) {
-			$entry = SingleMenuEntry::create(
-				'userTalk',
-				$this->messageLocalizer->msg( 'mobile-frontend-user-page-talk' )->escaped(),
-				$talkPage->getLocalURL()
-			);
-			$group->insertEntry( $entry );
-		}
-		$sandbox = $personalTools['sandbox']['links'][0] ?? false;
-
-		if ( $sandbox ) {
-			$group->insertEntry( SingleMenuEntry::create(
-				'sandbox',
-				$sandbox['text'],
-				$sandbox['href']
-			) );
-		}
-		$this->definitions->insertWatchlistMenuItem( $group );
-		$this->definitions->insertContributionsMenuItem( $group );
-		if ( $this->user->isRegistered() ) {
-			$this->definitions->insertLogOutMenuItem( $group );
-		} else {
-			$this->definitions->insertLogInMenuItem( $group );
+		unset( $personalTools[ 'preferences' ] );
+		unset( $personalTools[ 'betafeatures' ] );
+		unset( $personalTools[ 'uploads' ] );
+		foreach ( $personalTools as $key => $definition ) {
+			if ( $definition['links'] ) {
+				$link = $personalTools[$key]['links'][0] ?? false;
+				if ( $link ) {
+					$class = $link['class'] ?? '';
+					if ( is_array( $class ) ) {
+						$class = implode( ' ', $class );
+					}
+					$group->insertEntry( SingleMenuEntry::create(
+						$key,
+						$link['text'],
+						$link['href'],
+						$class,
+						$link['icon'] ?? null
+					) );
+				}
+			}
 		}
 		Hooks::run( 'MobileMenu', [ 'user', &$group ] );
 		return $group;
Jdlrobson added a project: patch-welcome.
Jdlrobson renamed this task from Minerva menus should use data from core hooks to Minerva menus should use data from core hooks (deprecate MobileMenu hook).Tue, Jan 11, 11:25 PM

Targetting 1.38 as a stretch goal.
https://gerrit.wikimedia.org/r/c/mediawiki/skins/MinervaNeue/+/753596 will allow Extension:GrowthExperiments to use the SkinTemplateNavigation::Universal hook to make modifications to menu items when operating in mobile mode.

Will coordinate with Growth team if we make progress on our side.

Change 753596 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] [Refactor] Use core definitions for personal menu icons

https://gerrit.wikimedia.org/r/753596

Change 755046 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Main page definition come from MediaWiki:Sidebar

https://gerrit.wikimedia.org/r/755046

Change 755047 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/GrowthExperiments@master] Drop usage of MobileMenu hook for user page

https://gerrit.wikimedia.org/r/755047

Change 755048 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/skins/MinervaNeue@master] Deprecate MobileMenu hook

https://gerrit.wikimedia.org/r/755048

Change 753596 merged by jenkins-bot:

[mediawiki/skins/MinervaNeue@master] [Refactor] Use core definitions for personal menu icons

https://gerrit.wikimedia.org/r/753596

Change 756021 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/core@master] Add icons to navigation sidebar entries

https://gerrit.wikimedia.org/r/756021