Use newly available Skin::getPortletData method to get mw-portlet class

Kept as simple as possible for now. The new class is added but no classes
are removed. This will be done in a follow up.

Bug: T256897
Bug: T253938
Change-Id: Ib31a9d8f2ac14e63b63e82abd4a9aa1fcb956f45

includes/SkinVector.php

@@ -34,14 +34,6 @@ use Vector\VectorServices;
  * @internal
  */
 class SkinVector extends SkinMustache {
-
-	/** @var array of alternate message keys for menu labels */
-	private const MENU_LABEL_KEYS = [
-		'cactions' => 'vector-more-actions',
-		'tb' => 'toolbox',
-		'personal' => 'personaltools',
-		'lang' => 'otherlanguages',
-	];
 	/** @var int */
 	private const MENU_TYPE_DEFAULT = 0;
 	/** @var int */
@@ -344,8 +336,7 @@ class SkinVector extends SkinMustache {
 
 	/**
 	 * @param string $label to be used to derive the id and human readable label of the menu
-	 *  If the key has an entry in the constant MENU_LABEL_KEYS then that message will be used for the
+	 *  Note certain keys are special cased for historic reasons in core.
-	 *  human readable text instead.
 	 * @param array $urls to convert to list items stored as string in html-items key
 	 * @param int $type of menu (optional) - a plain list (MENU_TYPE_DEFAULT),
 	 *   a tab (MENU_TYPE_TABS) or a dropdown (MENU_TYPE_DROPDOWN)
@@ -359,7 +350,7 @@ class SkinVector extends SkinMustache {
 		int $type = self::MENU_TYPE_DEFAULT,
 		bool $setLabelToSelected = false
 	) : array {
-		$skin = $this->getSkin();
+		$portletData = $this->getPortletData( $label, $urls );
 		$extraClasses = [
 			self::MENU_TYPE_DROPDOWN => 'vector-menu vector-menu-dropdown vectorMenu',
 			self::MENU_TYPE_TABS => 'vector-menu vector-menu-tabs vectorTabs',
@@ -374,24 +365,15 @@ class SkinVector extends SkinMustache {
 		];
 		$isPortal = $type === self::MENU_TYPE_PORTAL;
 
-		// For some menu items, there is no language key corresponding with its menu key.
+		$props = $portletData + [
-		// These inconsitencies are captured in MENU_LABEL_KEYS
-		$msgObj = $skin->msg( self::MENU_LABEL_KEYS[ $label ] ?? $label );
-		$props = [
-			'id' => "p-$label",
 			'label-id' => "p-{$label}-label",
-			// If no message exists fallback to plain text (T252727)
-			'label' => $msgObj->exists() ? $msgObj->text() : $label,
 			'list-classes' => $listClasses[$type] ?? 'vector-menu-content-list',
-			'html-items' => '',
 			'is-dropdown' => $type === self::MENU_TYPE_DROPDOWN,
-			'html-tooltip' => Linker::tooltip( 'p-' . $label ),
 		];
 
+		// Special casing for Variant to change label to selected.
+		// Hopefully we can revisit and possibly remove this code when the language switcher is moved.
 		foreach ( $urls as $key => $item ) {
-			$props['html-items'] .= $this->getSkin()->makeListItem( $key, $item );
-			// Check the class of the item for a `selected` class and if so, propagate the items
-			// label to the main label.
 			if ( $setLabelToSelected ) {
 				if ( isset( $item['class'] ) && stripos( $item['class'], 'selected' ) !== false ) {
 					$props['label'] = $item['text'];
@@ -399,27 +381,8 @@ class SkinVector extends SkinMustache {
 			}
 		}
 
-		$afterPortal = '';
-		if ( $isPortal ) {
-			// The BaseTemplate::getAfterPortlet method ran the SkinAfterPortlet
-			// hook and if content is added appends it to the html-after-portal method.
-			// This replicates that historic behaviour.
-			// This code should eventually be upstreamed to SkinMustache in core.
-			// Currently in production this supports the Wikibase 'edit' link.
-			$content = $this->getAfterPortlet( $label );
-			if ( $content !== '' ) {
-				$afterPortal = Html::rawElement(
-					'div',
-					[ 'class' => [ 'after-portlet', 'after-portlet-' . $label ] ],
-					$content
-				);
-			}
-		}
-		$props['html-after-portal'] = $afterPortal;
-
 		// Mark the portal as empty if it has no content
-		$class = ( count( $urls ) == 0 && !$props['html-after-portal'] )
+		$class = $props['class'];
-			? 'vector-menu-empty emptyPortlet' : '';
 		$props['class'] = trim( "$class $extraClasses[$type]" );
 		return $props;
 	}

resources/mediawiki.d.ts

@@ -6,6 +6,19 @@ type MwApiConstructor = new( options?: Object ) => MwApi;
 
 interface MediaWiki {
 	util: {
+		/**
+		 * @param {string} id of portlet
+		 */
+		showPortlet( id: string ): () => void;
+		/**
+		 * @param {string} id of portlet
+		 */
+		hidePortlet( id: string ): () => void;
+		/**
+		 * @param {string} id of portlet
+		 * @return {bool}
+		 */
+		isPortletVisible( id: string ): () => boolean,
 		/**
 		 * Return a wrapper function that is debounced for the given duration.
 		 *

resources/skins.vector.legacy.js/vector.js

@@ -2,8 +2,8 @@
  * Collapsible tabs for Vector
  */
 function init() {
-	// eslint-disable-next-line no-jquery/no-global-selector
+	var cactionsId = 'p-cactions',
-	var $cactions = $( '#p-cactions' ),
+		$cactions = $( '#' + cactionsId ),
 		// eslint-disable-next-line no-jquery/no-global-selector
 		$tabContainer = $( '#p-views ul' ),
 		initialCactionsWidth = function () {
@@ -27,9 +27,8 @@ function init() {
 		.on( 'beforeTabCollapse', function () {
 			var expandedWidth;
 			// If the dropdown was hidden, show it
-			// eslint-disable-next-line no-jquery/no-class-state
+			if ( mw.util.isPortletVisible( cactionsId ) ) {
-			if ( $cactions.hasClass( 'emptyPortlet' ) ) {
+				mw.util.showPortlet( cactionsId );
-				$cactions.removeClass( 'emptyPortlet vector-menu-empty' );
 				// Now that it is visible, force-render it virtually
 				// to get its expanded width, then shrink it 1px before we
 				// yield from JS (which means the expansion won't be visible).
@@ -46,8 +45,8 @@ function init() {
 			if ( $cactions.find( 'li' ).length === 1 ) {
 				// eslint-disable-next-line no-jquery/no-animate
 				$cactions.find( 'h3' ).animate( { width: '1px' }, 'normal', function () {
-					$( this ).attr( 'style', '' )
+					$( this ).attr( 'style', '' );
-						.parent().addClass( 'emptyPortlet vector-menu-empty' );
+					mw.util.hidePortlet( cactionsId );
 				} );
 			}
 		} )
@@ -85,8 +84,7 @@ function init() {
 				}
 
 				// Always collapse if the "More" button is already shown.
-				// eslint-disable-next-line no-jquery/no-class-state
+				if ( mw.util.isPortletVisible( cactionsId ) ) {
-				if ( !$cactions.hasClass( 'emptyPortlet' ) ) {
 					return true;
 				}
 

resources/skins.vector.styles/Menu.less

@@ -1,18 +1,6 @@
 @import '../../variables.less';
 @import 'mediawiki.mixins.less';
 
-/* Hide empty portlets */
-// This class has special magical powers. It can be removed by
-// mw.util.addPortletLink
-// in core:resources/src/mediawiki.util/util.js
-// When I93fb6c96df9f238d1c0281cb66512b135ca2afc2 has been merged
-// and is deployed this line of CSS should be removed or replaced
-// with the class ".vector-menu-empty"
-// See T253912.
-.emptyPortlet {
-	display: none;
-}
-
 /* Personal */
 .vector-menu {
 	// Hidden by default, but displayed by certain menus

tests/phpunit/integration/SkinVectorTest.php

@@ -76,15 +76,18 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase {
 
 		$this->assertSame(
 			[
+				// Provided by core
 				'id' => 'p-views',
-				'label-id' => 'p-views-label',
+				'class' => 'mw-portlet mw-portlet-views emptyPortlet vector-menu vector-menu-tabs vectorTabs',
-				'label' => $context->msg( 'views' )->text(),
-				'list-classes' => 'vector-menu-content-list',
-				'html-items' => '',
-				'is-dropdown' => false,
 				'html-tooltip' => '',
+				'html-items' => '',
 				'html-after-portal' => '',
-				'class' => 'vector-menu-empty emptyPortlet vector-menu vector-menu-tabs vectorTabs',
+				'label' => $context->msg( 'views' )->text(),
+
+				// provided by VECTOR
+				'label-id' => 'p-views-label',
+				'list-classes' => 'vector-menu-content-list',
+				'is-dropdown' => false,
 			],
 			$views
 		);
@@ -92,19 +95,19 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase {
 		$variants = $props['data-variants'];
 		$actions = $props['data-page-actions-more'];
 		$this->assertSame(
-			'vector-menu-empty emptyPortlet vector-menu vector-menu-tabs vectorTabs',
+			'mw-portlet mw-portlet-namespaces emptyPortlet vector-menu vector-menu-tabs vectorTabs',
 			$namespaces['class']
 		);
 		$this->assertSame(
-			'vector-menu-empty emptyPortlet vector-menu vector-menu-dropdown vectorMenu',
+			'mw-portlet mw-portlet-variants emptyPortlet vector-menu vector-menu-dropdown vectorMenu',
 			$variants['class']
 		);
 		$this->assertSame(
-			'vector-menu-empty emptyPortlet vector-menu vector-menu-dropdown vectorMenu',
+			'mw-portlet mw-portlet-cactions emptyPortlet vector-menu vector-menu-dropdown vectorMenu',
 			$actions['class']
 		);
 		$this->assertSame(
-			'vector-menu-empty emptyPortlet vector-menu',
+			'mw-portlet mw-portlet-personal emptyPortlet vector-menu',
 			$props['data-personal-menu']['class']
 		);
 	}