Refactor SkinVector to use Hooks::updateMenuItems code to process menu data

SkinVector and Hooks both had code to add classes and handle Vector specific template data. This patch simplifies the way we handle menu data to always use Hooks:updateMenuItem. This has an additional side effect of removing instances of mw-ui-icon-before.

Bug: T306628
Change-Id: I73514a0eada4d92705b70e7c2ebd91092fc12544
This commit is contained in:
bwang 2022-04-14 11:27:31 -05:00 committed by Jdlrobson
parent f83e53d8a4
commit 4ab798441e
5 changed files with 170 additions and 119 deletions

View File

@ -207,24 +207,6 @@ class Hooks implements
self::addListItemClass( $item, [ $newClass ] );
}
/**
* Adds an icon to the list item of a menu.
*
* @param array &$item
* @param string $icon_name
*/
private static function addIconToListItem( &$item, $icon_name ) {
// Set the default menu icon classes.
$menu_icon_classes = [ 'mw-ui-icon', 'mw-ui-icon-before',
// Some extensions declare icons without the wikimedia- prefix. e.g. Echo
'mw-ui-icon-' . $icon_name,
// FIXME: Some icon names are prefixed with `wikimedia-`.
// We should seek to remove all these instances.
'mw-ui-icon-wikimedia-' . $icon_name
];
self::addListItemClass( $item, $menu_icon_classes, true );
}
/**
* Updates personal navigation menu (user links) dropdown for modern Vector:
* - Adds icons
@ -232,8 +214,6 @@ class Hooks implements
*
* @param SkinTemplate $sk
* @param array &$content_navigation
* @suppress PhanTypeArraySuspiciousNullable False positives
* @suppress PhanTypePossiblyInvalidDimOffset False positives
*/
private static function updateUserLinksDropdownItems( $sk, &$content_navigation ) {
// For logged-in users in modern Vector, rearrange some links in the personal toolbar.
@ -242,39 +222,37 @@ class Hooks implements
$isRegistered = $user->isRegistered();
if ( $isTemp ) {
if ( isset( $content_navigation['user-page']['tmpuserpage'] ) ) {
self::makeMenuItemCollapsible( $content_navigation['user-page']['tmpuserpage'] );
$content_navigation['user-page']['tmpuserpage']['collapsible'] = true;
$content_navigation['user-page']['tmpuserpage'] =
self::updateMenuItem( $content_navigation['user-page']['tmpuserpage'] );
}
if ( isset( $content_navigation['user-menu']['tmpuserpage'] ) ) {
self::makeMenuItemCollapsible( $content_navigation['user-menu']['tmpuserpage'] );
$content_navigation['user-menu']['tmpuserpage']['collapsible'] = true;
$content_navigation['user-menu']['tmpuserpage'] =
self::updateMenuItem( $content_navigation['user-menu']['tmpuserpage'] );
}
} elseif ( $isRegistered ) {
// Remove user page from personal menu dropdown for logged in use
self::makeMenuItemCollapsible(
$content_navigation['user-menu']['userpage']
);
$content_navigation['user-menu']['userpage']['collapsible'] = true;
// watchlist may be disabled if $wgGroupPermissions['*']['viewmywatchlist'] = false;
// See [[phab:T299671]]
if ( isset( $content_navigation['user-menu']['watchlist'] ) ) {
self::makeMenuItemCollapsible(
$content_navigation['user-menu']['watchlist']
);
$content_navigation['user-menu']['watchlist']['collapsible'] = true;
}
// Remove logout link from user-menu and recreate it in SkinVector,
unset( $content_navigation['user-menu']['logout'] );
}
if ( $isRegistered ) {
// Don't show icons for anon menu items (besides login and create account).
// Prefix user link items with associated icon.
$user_menu = $content_navigation['user-menu'];
// Don't show icons for anon menu items (besides login and create account).
// Loop through each menu to check/append its link classes.
foreach ( $user_menu as $menu_key => $menu_value ) {
$icon_name = $menu_value['icon'] ?? '';
self::addIconToListItem( $content_navigation['user-menu'][$menu_key], $icon_name );
}
self::updateMenuItems( $content_navigation, 'user-menu' );
} else {
// Remove "Not logged in" from personal menu dropdown for anon users.
unset( $content_navigation['user-menu']['anonuserpage'] );
}
if ( !$isRegistered || $isTemp ) {
// "Create account" link is handled manually by Vector
unset( $content_navigation['user-menu']['createaccount'] );
@ -291,6 +269,8 @@ class Hooks implements
* including 'notification', 'user-interface-preferences', 'user-page', 'vector-user-menu-overflow'
*
* @param array &$content_navigation
* @suppress PhanTypeInvalidDimOffset False positives
* @suppress PhanTypeMismatchArgumentInternal False positives
*/
private static function updateUserLinksOverflowItems( &$content_navigation ) {
// Upgrade preferences, notifications, and watchlist to icon buttons
@ -376,7 +356,47 @@ class Hooks implements
*/
public static function makeIcon( $name ) {
// Html::makeLink will pass this through rawElement
return '<span class="mw-ui-icon mw-ui-icon-' . $name . '"></span>';
return '<span class="mw-ui-icon mw-ui-icon-' . $name . ' mw-ui-icon-wikimedia-' . $name . '"></span>';
}
/**
* Updates template data for Vector menu items.
*
* @param array $item Menu data to update
* @param bool $isLinkData false if data is for li element (i.e. makeListItem()),
* true if for link element (i.e. makeLink())
* @return array $item Updated menu data
*/
public static function updateMenuItem( $item, $isLinkData = false ) {
$hasButton = $item['button'] ?? false;
$hideText = $item['text-hidden'] ?? false;
$isCollapsible = $item['collapsible'] ?? false;
$icon = $item['icon'] ?? '';
unset( $item['button'] );
unset( $item['icon'] );
unset( $item['text-hidden'] );
unset( $item['collapsible'] );
if ( $isCollapsible ) {
self::makeMenuItemCollapsible( $item );
}
if ( $hasButton ) {
self::addListItemClass( $item, [ 'mw-ui-button', 'mw-ui-quiet' ], !$isLinkData );
}
if ( $icon ) {
if ( $hideText ) {
$iconElementClasses = [ 'mw-ui-icon', 'mw-ui-icon-element',
// Some extensions declare icons without the wikimedia- prefix. e.g. Echo
'mw-ui-icon-' . $icon,
// FIXME: Some icon names are prefixed with `wikimedia-`.
// We should seek to remove all these instances.
'mw-ui-icon-wikimedia-' . $icon
];
self::addListItemClass( $item, $iconElementClasses, !$isLinkData );
} else {
$item['link-html'] = self::makeIcon( $icon );
}
}
return $item;
}
/**
@ -387,37 +407,7 @@ class Hooks implements
*/
private static function updateMenuItems( &$content_navigation, $menu ) {
foreach ( $content_navigation[$menu] as $key => $item ) {
$hasButton = $item['button'] ?? false;
$hideText = $item['text-hidden'] ?? false;
$isCollapsible = $item['collapsible'] ?? false;
$icon = $item['icon'] ?? '';
unset( $item['button'] );
unset( $item['icon'] );
unset( $item['text-hidden'] );
unset( $item['collapsible'] );
if ( $isCollapsible ) {
self::makeMenuItemCollapsible( $item );
}
if ( $hasButton ) {
self::addListItemClass( $item, [ 'mw-ui-button', 'mw-ui-quiet' ], true );
}
if ( $icon ) {
if ( $hideText ) {
$iconElementClasses = [ 'mw-ui-icon', 'mw-ui-icon-element',
// Some extensions declare icons without the wikimedia- prefix. e.g. Echo
'mw-ui-icon-' . $icon,
// FIXME: Some icon names are prefixed with `wikimedia-`.
// We should seek to remove all these instances.
'mw-ui-icon-wikimedia-' . $icon
];
self::addListItemClass( $item, $iconElementClasses, true );
} else {
$item['link-html'] = self::makeIcon( $icon );
}
}
$content_navigation[$menu][$key] = $item;
$content_navigation[$menu][$key] = self::updateMenuItem( $item );
}
}

View File

@ -121,7 +121,6 @@ abstract class SkinVector extends SkinMustache {
private const CLASS_QUIET_BUTTON = 'mw-ui-button mw-ui-quiet';
private const CLASS_PROGRESSIVE = 'mw-ui-progressive';
private const CLASS_ICON_BUTTON = 'mw-ui-icon mw-ui-icon-element';
private const CLASS_ICON_LABEL = 'mw-ui-icon mw-ui-icon-before';
/**
* T243281: Code used to track clicks to opt-out link.
@ -227,26 +226,21 @@ abstract class SkinVector extends SkinMustache {
/**
* Returns HTML for the create account link inside the anon user links
* @param string[] $returnto array of query strings used to build the login link
* @param string[] $class array of CSS classes to add.
* @param bool $includeIcon Set true to include icon CSS classes.
* @param bool $isDropdownItem Set true for create account link inside the user menu dropdown
* which includes icon classes and is not styled like a button
* @return string
*/
private function getCreateAccountHTML( $returnto, $class, $includeIcon ) {
private function getCreateAccountHTML( $returnto, $isDropdownItem ) {
$createAccountData = $this->buildCreateAccountData( $returnto );
$createAccountData['single-id'] = 'pt-createaccount';
if ( $includeIcon ) {
$class = array_merge(
$class,
[
self::CLASS_ICON_LABEL,
$this->iconClass( $createAccountData[ 'icon' ] ?? '' )
]
);
}
$createAccountData['class'] = $class;
unset( $createAccountData['icon'] );
$createAccountData = array_merge( $createAccountData, [
'class' => $isDropdownItem ? [
'vector-menu-content-item',
] : '',
'collapsible' => true,
'icon' => $isDropdownItem ? $createAccountData['icon'] : null,
'button' => !$isDropdownItem,
] );
$createAccountData = Hooks::updateMenuItem( $createAccountData, true );
return $this->makeLink( 'create-account', $createAccountData );
}
@ -267,24 +261,15 @@ abstract class SkinVector extends SkinMustache {
* @return string
*/
private function getAnonMenuBeforePortletHTML( $returnto, $useCombinedLoginLink, $isTempUser ) {
$loginData = $this->buildLoginData( $returnto, $useCombinedLoginLink );
$loginData['class'] = [
'vector-menu-content-item',
'vector-menu-content-item-login',
self::CLASS_ICON_LABEL,
$this->iconClass( $loginData[ 'icon' ] ?? '' )
];
unset( $loginData['icon'] );
$templateData = [
'htmlCreateAccount' => $this->getCreateAccountHTML( $returnto, [
'user-links-collapsible-item',
'vector-menu-content-item',
], true ),
'htmlLogin' => $this->makeLink( 'login', $loginData ),
];
$templateParser = $this->getTemplateParser();
$loginLinkData = array_merge( $this->buildLoginData( $returnto, $useCombinedLoginLink ), [
'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-login' ],
] );
$loginLinkData = Hooks::updateMenuItem( $loginLinkData, true );
$templateData = [
'htmlCreateAccount' => $this->getCreateAccountHTML( $returnto, true ),
'htmlLogin' => $this->makeLink( 'login', $loginLinkData ),
];
if ( $isTempUser ) {
$templateName = 'UserLinks__templogin';
@ -308,16 +293,12 @@ abstract class SkinVector extends SkinMustache {
* @return string
*/
private function getLogoutHTML() {
$logoutLinkData = $this->buildLogoutLinkData();
$templateParser = $this->getTemplateParser();
$logoutLinkData['class'] = [
'vector-menu-content-item',
'vector-menu-content-item-logout',
self::CLASS_ICON_LABEL,
$this->iconClass( $logoutLinkData[ 'icon' ] ?? '' )
];
unset( $logoutLinkData['icon'] );
$logoutLinkData = array_merge( $this->buildLogoutLinkData(), [
'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-logout' ],
] );
$logoutLinkData = Hooks::updateMenuItem( $logoutLinkData, true );
$templateParser = $this->getTemplateParser();
return $templateParser->processTemplate( 'UserLinks__logout', [
'htmlLogout' => $this->makeLink( 'logout', $logoutLinkData )
] );
@ -334,9 +315,7 @@ abstract class SkinVector extends SkinMustache {
$isTempUser = $user->isTemp();
$returnto = $this->getReturnToParam();
$useCombinedLoginLink = $this->useCombinedLoginLink();
$htmlCreateAccount = $this->getCreateAccountHTML( $returnto, [
self::CLASS_QUIET_BUTTON
], false );
$htmlCreateAccount = $this->getCreateAccountHTML( $returnto, false );
$templateParser = $this->getTemplateParser();
// See T288428#7303233. The following conditional checks whether config is disabling account creation for

View File

@ -85,7 +85,7 @@
white-space: nowrap;
cursor: pointer;
span {
span:not( .mw-ui-icon ) {
font-size: @font-size-tabs;
}
}

View File

@ -117,7 +117,8 @@
color: @color-base;
text-decoration: none;
span {
// Update menu item text styles
span:not( .mw-ui-icon ) {
font-size: @font-size-user-links;
.text-overflow( @visible: false );
// Overrides .mw-ui-icon's `line-height: 0` property so that the text is

View File

@ -484,6 +484,8 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
'updateUserLinksDropdownItems'
);
$updateUserLinksDropdownItems->setAccessible( true );
// Anon users
$skin = new SkinVector22( [ 'name' => 'vector-2022' ] );
$contentAnon = [
'user-menu' => [
@ -516,14 +518,18 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
$this->assertContains( 'user-links-collapsible-item', $contentRegistered['user-menu']['userpage']['class'],
'User page link in user links dropdown requires collapsible class'
);
$this->assertContains( 'mw-ui-icon-before', $contentRegistered['user-menu']['userpage']['link-class'],
'User page link in user links dropdown requires before icon classes'
$this->assertEquals(
'<span class="mw-ui-icon mw-ui-icon-userpage mw-ui-icon-wikimedia-userpage"></span>',
$contentRegistered['user-menu']['userpage']['link-html'],
'User page link in user links dropdown has link-html'
);
$this->assertContains( 'user-links-collapsible-item', $contentRegistered['user-menu']['watchlist']['class'],
'Watchlist link in user links dropdown requires collapsible class'
);
$this->assertContains( 'mw-ui-icon-before', $contentRegistered['user-menu']['watchlist']['link-class'],
'Watchlist link in user links dropdown requires before icon classes'
$this->assertEquals(
'<span class="mw-ui-icon mw-ui-icon-watchlist mw-ui-icon-wikimedia-watchlist"></span>',
$contentRegistered['user-menu']['watchlist']['link-html'],
'Watchlist link in user links dropdown has link-html'
);
$this->assertFalse( isset( $contentRegistered['user-menu']['logout'] ),
'Logout link in user links dropdown is not set'
@ -595,4 +601,79 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
'Watchlist link in user links has unique id'
);
}
public function provideUpdateMenuItem() {
return [
// Removes extra attributes
[
[ 'class' => [], 'icon' => '', 'button' => false, 'text-hidden' => false, 'collapsible' => false ],
false,
[ 'class' => [] ],
],
// Adds link-html
[
[ 'class' => [], 'icon' => 'userpage' ],
false,
[
'class' => [],
'link-html' =>
'<span class="mw-ui-icon mw-ui-icon-userpage mw-ui-icon-wikimedia-userpage"></span>'
],
],
// Adds collapsible class
[
[ 'class' => [], 'collapsible' => true ],
false,
[ 'class' => [ 'user-links-collapsible-item' ] ],
],
// Adds button classes
[
[ 'class' => [], 'button' => true ],
false,
[ 'class' => [], 'link-class' => [ 'mw-ui-button', 'mw-ui-quiet' ] ],
],
// Adds text hidden classes
[
[ 'class' => [], 'text-hidden' => true, 'icon' => 'userpage' ],
false,
[ 'class' => [], 'link-class' => [
'mw-ui-icon',
'mw-ui-icon-element',
'mw-ui-icon-userpage',
'mw-ui-icon-wikimedia-userpage'
] ],
],
// Adds classes for link data
[
[ 'class' => [], 'button' => true, 'text-hidden' => true, 'icon' => 'userpage' ],
true,
[ 'class' => [
'mw-ui-button',
'mw-ui-quiet',
'mw-ui-icon',
'mw-ui-icon-element',
'mw-ui-icon-userpage',
'mw-ui-icon-wikimedia-userpage'
] ],
]
];
}
/**
* @covers ::updateMenuItem
* @dataProvider provideUpdateMenuItem
*/
public function testUpdateMenuItem(
array $menuItem,
bool $isLinkData,
array $expected
) {
$updateMenuItem = new ReflectionMethod(
Hooks::class,
'updateMenuItem'
);
$updateMenuItem->setAccessible( true );
$data = $updateMenuItem->invokeArgs( null, [ $menuItem, $isLinkData ] );
$this->assertEquals( $expected, $data );
}
}