jonny
/
Vector-Local
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Follow-up: Refactor button/icon class helpers, make portlet classes consistent"

This commit is contained in:
jenkins-bot 2022-08-30 02:35:50 +00:00 committed by Gerrit Code Review
parent 0d957abf77 3881820483
commit be7797e012
4 changed files with 240 additions and 157 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

115
includes/Hooks.php
View File

@ -168,9 +168,9 @@ class Hooks implements
// Promote watch link from actions to views and add an icon
if ( $key !== null ) {
self::appendClassToListItem(
$content_navigation['actions'][$key],
'icon'
self::appendClassToItem(
$content_navigation['actions'][$key]['class'],
[ 'icon' ]
);
$content_navigation['views'][$key] = $content_navigation['actions'][$key];
unset( $content_navigation['actions'][$key] );
@ -178,36 +178,30 @@ class Hooks implements
}
/**
* Updates class list on list item
* Adds class to a property
*
* @param array &$item to update for use in makeListItem
* @param array $classes to add to the item
* @param bool $applyToLink (optional) and defaults to false.
* If set will modify `link-class` instead of `class`
* @param array &$item to update
* @param array|string $classes to add to the item
*/
private static function addListItemClass( &$item, $classes, $applyToLink = false ) {
$property = $applyToLink ? 'link-class' : 'class';
$existingClass = $item[$property] ?? [];
private static function appendClassToItem( &$item, $classes ) {
$existingClasses = $item;
if ( is_array( $existingClass ) ) {
$item[$property] = array_merge( $existingClass, $classes );
} elseif ( is_string( $existingClass ) ) {
// treat as string
$item[$property] = array_merge( [ $existingClass ], $classes );
if ( is_array( $existingClasses ) ) {
// Treat as array
$newArrayClasses = is_array( $classes ) ? $classes : [ trim( $classes ) ];
$item = array_merge( $existingClasses, $newArrayClasses );
} elseif ( is_string( $existingClasses ) ) {
// Treat as string
$newStrClasses = is_string( $classes ) ? trim( $classes ) : implode( ' ', $classes );
$item .= ' ' . $newStrClasses;
} else {
$item[$property] = $classes;
// Treat as whatever $classes is
$item = $classes;
}
}
/**
* Updates the class on an existing item taking into account whether
* a class exists there already.
*
* @param array &$item
* @param string $newClass
*/
private static function appendClassToListItem( &$item, $newClass ) {
self::addListItemClass( $item, [ $newClass ] );
if ( is_string( $item ) ) {
$item = trim( $item );
}
}
/**
@ -227,12 +221,12 @@ class Hooks implements
if ( isset( $content_navigation['user-page']['tmpuserpage'] ) ) {
$content_navigation['user-page']['tmpuserpage']['collapsible'] = true;
$content_navigation['user-page']['tmpuserpage'] =
self::updateMenuItem( $content_navigation['user-page']['tmpuserpage'] );
self::updateMenuItemData( $content_navigation['user-page']['tmpuserpage'] );
}
if ( isset( $content_navigation['user-menu']['tmpuserpage'] ) ) {
$content_navigation['user-menu']['tmpuserpage']['collapsible'] = true;
$content_navigation['user-menu']['tmpuserpage'] =
self::updateMenuItem( $content_navigation['user-menu']['tmpuserpage'] );
self::updateMenuItemData( $content_navigation['user-menu']['tmpuserpage'] );
}
} elseif ( $isRegistered ) {
// Remove user page from personal menu dropdown for logged in use
@ -360,10 +354,7 @@ class Hooks implements
*/
private static function makeMenuItemCollapsible( array &$item, string $prefix = 'user-links-' ) {
$COLLAPSE_MENU_ITEM_CLASS = $prefix . 'collapsible-item';
self::appendClassToListItem(
$item,
$COLLAPSE_MENU_ITEM_CLASS
);
self::appendClassToItem( $item[ 'class' ], $COLLAPSE_MENU_ITEM_CLASS );
}
/**
@ -379,14 +370,15 @@ class Hooks implements
}
/**
* Updates template data for Vector menu items.
* Update template data to include classes and html that handle buttons, icons, and collapsible 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
* @internal for use inside Vector skin.
* @param array $item data to update
* @param string $buttonClassProp property to append button classes
* @param string $iconHtmlProp property to set icon HTML
* @return array $item Updated data
*/
public static function updateMenuItem( $item, $isLinkData = false ) {
private static function updateItemData( $item, $buttonClassProp, $iconHtmlProp ) {
$hasButton = $item['button'] ?? false;
$hideText = $item['text-hidden'] ?? false;
$isCollapsible = $item['collapsible'] ?? false;
@ -395,11 +387,12 @@ class Hooks implements
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 );
self::appendClassToItem( $item[ $buttonClassProp ], [ 'mw-ui-button', 'mw-ui-quiet' ] );
}
if ( $icon ) {
if ( $hideText ) {
@ -410,14 +403,50 @@ class Hooks implements
// We should seek to remove all these instances.
'mw-ui-icon-wikimedia-' . $icon
];
self::addListItemClass( $item, $iconElementClasses, !$isLinkData );
self::appendClassToItem( $item[ $buttonClassProp ], $iconElementClasses );
} else {
$item['link-html'] = self::makeIcon( $icon );
$item[ $iconHtmlProp ] = self::makeIcon( $icon );
}
}
return $item;
}
/**
* Updates template data for Vector dropdown menus.
*
* @param array $item Menu data to update
* @return array $item Updated menu data
*/
public static function updateDropdownMenuData( $item ) {
$buttonClassProp = 'heading-class';
$iconHtmlProp = 'html-vector-heading-icon';
return self::updateItemData( $item, $buttonClassProp, $iconHtmlProp );
}
/**
* Updates template data for Vector link items.
*
* @param array $item link data to update
* @return array $item Updated link data
*/
public static function updateLinkData( $item ) {
$buttonClassProp = 'class';
$iconHtmlProp = 'link-html';
return self::updateItemData( $item, $buttonClassProp, $iconHtmlProp );
}
/**
* Updates template data for Vector menu items.
*
* @param array $item menu item data to update
* @return array $item Updated menu item data
*/
public static function updateMenuItemData( $item ) {
$buttonClassProp = 'link-class';
$iconHtmlProp = 'link-html';
return self::updateItemData( $item, $buttonClassProp, $iconHtmlProp );
}
/**
* Updates user interface preferences for modern Vector to upgrade icon/button menu items.
*
@ -426,7 +455,7 @@ class Hooks implements
*/
private static function updateMenuItems( &$content_navigation, $menu ) {
foreach ( $content_navigation[$menu] as $key => $item ) {
$content_navigation[$menu][$key] = self::updateMenuItem( $item );
$content_navigation[$menu][$key] = self::updateMenuItemData( $item );
}
}

148
includes/SkinVector.php
View File

@ -126,9 +126,7 @@ abstract class SkinVector extends SkinMustache {
];
private const SEARCH_SHOW_THUMBNAIL_CLASS = 'vector-search-box-show-thumbnail';
private const SEARCH_AUTO_EXPAND_WIDTH_CLASS = 'vector-search-box-auto-expand-width';
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';
/**
* T243281: Code used to track clicks to opt-out link.
@ -142,17 +140,6 @@ abstract class SkinVector extends SkinMustache {
*/
private const OPT_OUT_LINK_TRACKING_CODE = 'vctw1';
/**
* @param string $icon the name of the icon without wikimedia- prefix.
* @return string
*/
private function iconClass( $icon ) {
if ( $icon ) {
return 'mw-ui-icon-wikimedia-' . $icon;
}
return '';
}
abstract protected function isLegacy(): bool;
/**
@ -249,7 +236,7 @@ abstract class SkinVector extends SkinMustache {
'icon' => $isDropdownItem ? $createAccountData['icon'] : null,
'button' => !$isDropdownItem,
] );
$createAccountData = Hooks::updateMenuItem( $createAccountData, true );
$createAccountData = Hooks::updateLinkData( $createAccountData );
return $this->makeLink( 'create-account', $createAccountData );
}
@ -265,7 +252,7 @@ abstract class SkinVector extends SkinMustache {
$loginLinkData = array_merge( $this->buildLoginData( $returnto, $useCombinedLoginLink ), [
'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-login' ],
] );
$loginLinkData = Hooks::updateMenuItem( $loginLinkData, true );
$loginLinkData = Hooks::updateLinkData( $loginLinkData );
$templateData = [
'htmlCreateAccount' => $this->getCreateAccountHTML( $returnto, true ),
'htmlLogin' => $this->makeLink( 'login', $loginLinkData ),
@ -296,7 +283,7 @@ abstract class SkinVector extends SkinMustache {
$logoutLinkData = array_merge( $this->buildLogoutLinkData(), [
'class' => [ 'vector-menu-content-item', 'vector-menu-content-item-logout' ],
] );
$logoutLinkData = Hooks::updateMenuItem( $logoutLinkData, true );
$logoutLinkData = Hooks::updateLinkData( $logoutLinkData );
$templateParser = $this->getTemplateParser();
return $templateParser->processTemplate( 'UserLinks__logout', [
@ -379,11 +366,9 @@ abstract class SkinVector extends SkinMustache {
'html-items' => '',
'html-vector-menu-checkbox-attributes' => 'tabindex="-1"',
'html-vector-menu-heading-attributes' => 'tabindex="-1"',
'heading-class' => implode( ' ', [
self::CLASS_QUIET_BUTTON,
self::CLASS_ICON_BUTTON,
$this->iconClass( 'listBullet' )
] ),
'button' => true,
'text-hidden' => true,
'icon' => 'listBullet'
] );
// Show sticky ULS if the ULS extension is enabled and the ULS in header is not hidden
@ -728,9 +713,9 @@ abstract class SkinVector extends SkinMustache {
$this->getULSLabel( 'vector-language-button-aria-label', $numLanguages ),
// ext.uls.interface attaches click handler to this selector.
'checkbox-class' => ' mw-interlanguage-selector ',
'html-vector-heading-icon' => Hooks::makeIcon( 'wikimedia-language-progressive' ),
'heading-class' => self::CLASS_QUIET_BUTTON . ' ' . self::CLASS_PROGRESSIVE .
' mw-portlet-lang-heading-' . strval( $numLanguages ),
'icon' => 'language-progressive',
'button' => true,
'heading-class' => self::CLASS_PROGRESSIVE . ' mw-portlet-lang-heading-' . strval( $numLanguages ),
];
// Adds class to hide language button
@ -743,13 +728,43 @@ abstract class SkinVector extends SkinMustache {
}
/**
* helper for applying Vector menu classes to portlets
* Creates portlet data for the user menu dropdown
*
* @param array $portletData
* @return array
*/
private function getUserMenuPortletData( $portletData ) {
// Add target class to apply different icon to personal menu dropdown for logged in users.
$portletData['class'] .= ' vector-user-menu';
$portletData['class'] .= $this->loggedin ?
' vector-user-menu-logged-in' :
' vector-user-menu-logged-out';
if ( $this->getUser()->isTemp() ) {
$icon = 'userAnonymous';
} elseif ( $this->loggedin ) {
$icon = 'userAvatar';
} else {
$icon = 'ellipsis';
// T287494 We use tooltip messages to provide title attributes on hover over certain menu icons.
// For modern Vector, the "tooltip-p-personal" key is set to "User menu" which is appropriate for
// the user icon (dropdown indicator for user links menu) for logged-in users.
// This overrides the tooltip for the user links menu icon which is an ellipsis for anonymous users.
$portletData['html-tooltip'] = Linker::tooltip( 'vector-anon-user-menu-title' );
}
$portletData['icon'] = $icon;
$portletData['button'] = true;
$portletData['text-hidden'] = true;
return $portletData;
}
/**
* Helper for applying Vector menu classes to portlets
*
* @param array $portletData returned by SkinMustache to decorate
* @param int $type representing one of the menu types (see MENU_TYPE_* constants)
* @return array modified version of portletData input
*/
private function decoratePortletClass(
private function updatePortletClasses(
array $portletData,
int $type = self::MENU_TYPE_DEFAULT
) {
@ -762,47 +777,17 @@ abstract class SkinVector extends SkinMustache {
if ( $this->isLegacy() ) {
$extraClasses[self::MENU_TYPE_TABS] .= ' vector-menu-tabs-legacy';
}
$portletData['class'] .= ' ' . $extraClasses[$type];
if ( !isset( $portletData['heading-class'] ) ) {
$portletData['heading-class'] = '';
}
// Add target class to apply different icon to personal menu dropdown for logged in users.
if ( $portletData['id'] === 'p-personal' ) {
if ( $this->isLegacy() ) {
$portletData['class'] .= ' vector-user-menu-legacy';
} else {
$portletData['class'] .= ' vector-user-menu';
$portletData['class'] .= $this->loggedin ?
' vector-user-menu-logged-in' :
' vector-user-menu-logged-out';
$portletData['heading-class'] .= ' ' . self::CLASS_QUIET_BUTTON . ' ' .
self::CLASS_ICON_BUTTON . ' ';
if ( $this->getUser()->isTemp() ) {
$icon = 'userAnonymous';
} elseif ( $this->loggedin ) {
$icon = 'userAvatar';
} else {
$icon = 'ellipsis';
}
$portletData['heading-class'] .= $this->iconClass( $icon );
}
}
switch ( $portletData['id'] ) {
case 'p-variants':
case 'p-cactions':
$portletData['class'] .= ' vector-menu-dropdown-noicon';
break;
case 'p-vector-user-menu-overflow':
$portletData['class'] .= ' vector-user-menu-overflow';
break;
default:
break;
if ( $type === self::MENU_TYPE_DROPDOWN ) {
$portletData = Hooks::updateDropdownMenuData( $portletData );
}
if ( $portletData['id'] === 'p-lang' && $this->isLanguagesInContent() ) {
$portletData = array_merge( $portletData, $this->getULSPortletData() );
}
$class = $portletData['class'];
$portletData['class'] = trim( "$class $extraClasses[$type]" );
$portletData['class'] = trim( $portletData['class'] );
$portletData['heading-class'] = trim( $portletData['heading-class'] );
return $portletData;
}
@ -873,10 +858,24 @@ abstract class SkinVector extends SkinMustache {
break;
}
$portletData = $this->decoratePortletClass(
$portletData,
$type
);
if ( $key === 'data-languages' && $this->isLanguagesInContent() ) {
$portletData = array_merge( $portletData, $this->getULSPortletData() );
}
if ( $key === 'data-user-menu' && !$this->isLegacy() ) {
$portletData = $this->getUserMenuPortletData( $portletData );
}
if ( $key === 'data-vector-user-menu-overflow' ) {
$portletData['class'] .= ' vector-user-menu-overflow';
}
if ( $key === 'data-personal' && $this->isLegacy() ) {
// Set tooltip to empty string for the personal menu for both logged-in and logged-out users
// to avoid showing the tooltip for legacy version.
$portletData['html-tooltip'] = '';
$portletData['class'] .= ' vector-user-menu-legacy';
}
// Special casing for Variant to change label to selected.
// Hopefully we can revisit and possibly remove this code when the language switcher is moved.
@ -891,19 +890,10 @@ abstract class SkinVector extends SkinMustache {
$portletData['aria-label'] = $this->msg( 'vector-language-variant-switcher-label' );
}
// T287494 We use tooltip messages to provide title attributes on hover over certain menu icons. For modern
// Vector, the "tooltip-p-personal" key is set to "User menu" which is appropriate for the user icon (dropdown
// indicator for user links menu) for logged-in users. This overrides the tooltip for the user links menu icon
// which is an ellipsis for anonymous users.
if ( $key === 'data-user-menu' && !$this->isLegacy() && !$this->loggedin ) {
$portletData['html-tooltip'] = Linker::tooltip( 'vector-anon-user-menu-title' );
}
// Set tooltip to empty string for the personal menu for both logged-in and logged-out users to avoid showing
// the tooltip for legacy version.
if ( $key === 'data-personal' && $this->isLegacy() ) {
$portletData['html-tooltip'] = '';
}
$portletData = $this->updatePortletClasses(
$portletData,
$type
);
return $portletData + [
'is-dropdown' => $type === self::MENU_TYPE_DROPDOWN,

4
tests/phpunit/integration/SkinVectorTest.php
View File

@ -271,11 +271,11 @@ class SkinVectorTest extends MediaWikiIntegrationTestCase {
$namespaces['class']
);
$this->assertSame(
'mw-portlet mw-portlet-variants vector-menu-dropdown-noicon vector-menu-dropdown',
'mw-portlet mw-portlet-variants vector-menu-dropdown',
$variants['class']
);
$this->assertSame(
'mw-portlet mw-portlet-cactions vector-menu-dropdown-noicon vector-menu-dropdown',
'mw-portlet mw-portlet-cactions vector-menu-dropdown',
$actions['class']
);
$this->assertSame(

130
tests/phpunit/integration/VectorHooksTest.php
View File

@ -400,12 +400,12 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
$skin->getContext()->setTitle( Title::newFromText( 'Foo' ) );
$contentNavWatch = [
'actions' => [
'watch' => [ 'class' => 'watch' ],
'watch' => [ 'class' => [ 'watch' ] ],
]
];
$contentNavUnWatch = [
'actions' => [
'move' => [ 'class' => 'move' ],
'move' => [ 'class' => [ 'move' ] ],
'unwatch' => [],
],
];
@ -413,16 +413,16 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
Hooks::onSkinTemplateNavigation( $skin, $contentNavUnWatch );
Hooks::onSkinTemplateNavigation( $skin, $contentNavWatch );
$this->assertTrue(
in_array( 'icon', $contentNavWatch['views']['watch']['class'] ) !== false,
$this->assertContains(
'icon', $contentNavWatch['views']['watch']['class'],
'Watch list items require an "icon" class'
);
$this->assertTrue(
in_array( 'icon', $contentNavUnWatch['views']['unwatch']['class'] ) !== false,
$this->assertContains(
'icon', $contentNavUnWatch['views']['unwatch']['class'],
'Unwatch list items require an "icon" class'
);
$this->assertFalse(
strpos( $contentNavUnWatch['actions']['move']['class'], 'icon' ) !== false,
$this->assertNotContains(
'icon', $contentNavUnWatch['actions']['move']['class'],
'List item other than watch or unwatch should not have an "icon" class'
);
}
@ -585,18 +585,79 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
);
}
public function provideUpdateMenuItem() {
public function provideAppendClassToItem() {
return [
// Add array class to array
[
[],
[ 'array1', 'array2' ],
[ 'array1', 'array2' ],
],
// Add string class to array
[
[],
'array1 array2',
[ 'array1 array2' ],
],
// Add array class to string
[
'',
[ 'array1', 'array2' ],
'array1 array2',
],
// Add string class to string
[
'',
'array1 array2',
'array1 array2',
],
// Add string class to undefined
[
null,
'array1 array2',
'array1 array2',
],
// Add array class to undefined
[
null,
[ 'array1', 'array2' ],
[ 'array1', 'array2' ],
],
];
}
/**
* @covers ::appendClassToItem
* @dataProvider provideAppendClassToItem
*/
public function testAppendClassToItem(
$item,
$classes,
$expected
) {
$appendClassToItem = new ReflectionMethod(
Hooks::class,
'appendClassToItem'
);
$appendClassToItem->setAccessible( true );
$appendClassToItem->invokeArgs( null, [ &$item, $classes ] );
$this->assertEquals( $expected, $item );
}
public function provideUpdateItemData() {
return [
// Removes extra attributes
[
[ 'class' => [], 'icon' => '', 'button' => false, 'text-hidden' => false, 'collapsible' => false ],
false,
'link-class',
'link-html',
[ 'class' => [] ],
],
// Adds link-html
// Adds icon html
[
[ 'class' => [], 'icon' => 'userpage' ],
false,
'link-class',
'link-html',
[
'class' => [],
'link-html' =>
@ -606,19 +667,22 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
// Adds collapsible class
[
[ 'class' => [], 'collapsible' => true ],
false,
'link-class',
'link-html',
[ 'class' => [ 'user-links-collapsible-item' ] ],
],
// Adds button classes
[
[ 'class' => [], 'button' => true ],
false,
'link-class',
'link-html',
[ 'class' => [], 'link-class' => [ 'mw-ui-button', 'mw-ui-quiet' ] ],
],
// Adds text hidden classes
[
[ 'class' => [], 'text-hidden' => true, 'icon' => 'userpage' ],
false,
'link-class',
'link-html',
[ 'class' => [], 'link-class' => [
'mw-ui-icon',
'mw-ui-icon-element',
@ -626,37 +690,37 @@ class VectorHooksTest extends MediaWikiIntegrationTestCase {
'mw-ui-icon-wikimedia-userpage'
] ],
],
// Adds classes for link data
// Adds button and icon classes
[
[ 'class' => [], 'button' => true, 'text-hidden' => true, 'icon' => 'userpage' ],
true,
[ 'class' => [], 'button' => true, 'icon' => 'userpage' ],
'class',
'link-html',
[ 'class' => [
'mw-ui-button',
'mw-ui-quiet',
'mw-ui-icon',
'mw-ui-icon-element',
'mw-ui-icon-userpage',
'mw-ui-icon-wikimedia-userpage'
] ],
'mw-ui-quiet'
], 'link-html' =>
'<span class="mw-ui-icon mw-ui-icon-userpage mw-ui-icon-wikimedia-userpage"></span>'
],
]
];
}
/**
* @covers ::updateMenuItem
* @dataProvider provideUpdateMenuItem
* @covers ::updateItemData
* @dataProvider provideUpdateItemData
*/
public function testUpdateMenuItem(
array $menuItem,
bool $isLinkData,
public function testUpdateItemData(
array $item,
string $buttonClassProp,
string $iconHtmlProp,
array $expected
) {
$updateMenuItem = new ReflectionMethod(
$updateItemData = new ReflectionMethod(
Hooks::class,
'updateMenuItem'
'updateItemData'
);
$updateMenuItem->setAccessible( true );
$data = $updateMenuItem->invokeArgs( null, [ $menuItem, $isLinkData ] );
$updateItemData->setAccessible( true );
$data = $updateItemData->invokeArgs( null, [ $item, $buttonClassProp, $iconHtmlProp ] );
$this->assertEquals( $expected, $data );
}
}