Remove user links feature flag

Update/remove config, constants, hooks, templates, styles, logic, tests, stories to check legacy vs modern Vector where applicable instead of the decommissioned user links feature flag.

Bug: T288852
Change-Id: I5c5831091a10711838a8a2877c782df4996d4596
This commit is contained in:
Clare Ming 2021-08-24 15:34:59 -06:00
parent 30cdc5894e
commit 1efe0a4203
12 changed files with 82 additions and 430 deletions

View File

@ -166,26 +166,6 @@ final class Constants {
*/
public const REQUIREMENT_USE_WVUI_SEARCH = 'VectorUseWvuiSearch';
/**
* @var string
*/
public const QUERY_PARAM_CONSOLIDATE_USER_LINKS = 'vectoruserlinks';
/**
* @var string
*/
public const CONFIG_CONSOLIDATE_USER_LINKS = 'VectorConsolidateUserLinks';
/**
* @var string
*/
public const REQUIREMENT_CONSOLIDATE_USER_LINKS = 'ConsolidateUserLinks';
/**
* @var string
*/
public const FEATURE_CONSOLIDATE_USER_LINKS = 'ConsolidateUserLinks';
/**
* @var string
*/

View File

@ -159,9 +159,9 @@ class Hooks {
}
/**
* Updates personal navigation menu (user links) depending on the consolidated user links feature flag
* When on, user page, create account and login links are removed from the dropdown to be handled separately
* When off, the custom "user-page" bucket is removed to preserve existing behavior
* Updates personal navigation menu (user links) for modern Vector wherein user page, create account and login links
* are removed from the dropdown to be handled separately. In legacy Vector, the custom "user-page" bucket is
* removed to preserve existing behavior.
*
* @param SkinTemplate $sk
* @param array &$content_navigation
@ -169,66 +169,54 @@ class Hooks {
private static function updateUserLinksItems( $sk, &$content_navigation ) {
$COLLAPSE_MENU_ITEM_CLASS = 'user-links-collapsible-item';
// If the consolidate user links feature is enabled, rearrange some links in the personal toolbar.
if ( VectorServices::getFeatureManager()->isFeatureEnabled(
Constants::FEATURE_CONSOLIDATE_USER_LINKS )
) {
if ( $sk->loggedin ) {
// Remove user page from personal menu dropdown for logged in users at higher resolutions.
self::appendClassToListItem(
$content_navigation['user-menu']['userpage'],
$COLLAPSE_MENU_ITEM_CLASS
);
// Remove logout link from user-menu and recreate it in SkinVector,
unset( $content_navigation['user-menu']['logout'] );
} else {
// Remove "Not logged in" from personal menu dropdown for anon users.
unset( $content_navigation['user-menu']['anonuserpage'] );
// "Create account" link is handled manually by Vector
unset( $content_navigation['user-menu']['createaccount'] );
// "Login" link is handled manually by Vector
unset( $content_navigation['user-menu']['login'] );
// Remove duplicate "Login" link added by SkinTemplate::buildPersonalUrls if group read permissions
// are set to false.
unset( $content_navigation['user-menu']['login-private'] );
}
// ULS and user page links are hidden at lower resolutions.
if ( $content_navigation['user-interface-preferences'] ) {
self::appendClassToListItem(
$content_navigation['user-interface-preferences']['uls'],
$COLLAPSE_MENU_ITEM_CLASS
);
}
if ( $content_navigation['user-page'] ) {
self::appendClassToListItem(
$content_navigation['user-page']['userpage'],
$COLLAPSE_MENU_ITEM_CLASS
);
// Style the user page link as mw-ui-button.
self::addListItemClass(
$content_navigation['user-page']['userpage'],
[ 'mw-ui-button', 'mw-ui-quiet' ],
true
);
}
// Don't show icons for anon menu items (besides login and create
// account).
if ( $sk->loggedin ) {
// Prefix user link items with associated icon.
$user_menu = $content_navigation['user-menu'];
// 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 );
}
// For logged-in users in modern Vector, rearrange some links in the personal toolbar.
if ( $sk->loggedin ) {
// Remove user page from personal menu dropdown for logged in users at higher resolutions.
self::appendClassToListItem(
$content_navigation['user-menu']['userpage'],
$COLLAPSE_MENU_ITEM_CLASS
);
// Remove logout link from user-menu and recreate it in SkinVector,
unset( $content_navigation['user-menu']['logout'] );
// 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'];
// 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 );
}
} else {
// Remove user page from personal toolbar since it will be inside the personal menu for logged in
// users when the feature flag is disabled.
unset( $content_navigation['user-page'] );
// Remove "Not logged in" from personal menu dropdown for anon users.
unset( $content_navigation['user-menu']['anonuserpage'] );
// "Create account" link is handled manually by Vector
unset( $content_navigation['user-menu']['createaccount'] );
// "Login" link is handled manually by Vector
unset( $content_navigation['user-menu']['login'] );
// Remove duplicate "Login" link added by SkinTemplate::buildPersonalUrls if group read permissions
// are set to false.
unset( $content_navigation['user-menu']['login-private'] );
}
// ULS and user page links are hidden at lower resolutions.
if ( $content_navigation['user-interface-preferences'] ) {
self::appendClassToListItem(
$content_navigation['user-interface-preferences']['uls'],
$COLLAPSE_MENU_ITEM_CLASS
);
}
if ( $content_navigation['user-page'] ) {
self::appendClassToListItem(
$content_navigation['user-page']['userpage'],
$COLLAPSE_MENU_ITEM_CLASS
);
// Style the user page link as mw-ui-button.
self::addListItemClass(
$content_navigation['user-page']['userpage'],
[ 'mw-ui-button', 'mw-ui-quiet' ],
true
);
}
}
@ -253,11 +241,15 @@ class Hooks {
self::updateActionsMenu( $content_navigation );
}
if (
!self::isSkinVersionLegacy() &&
isset( $content_navigation['user-menu'] )
) {
self::updateUserLinksItems( $sk, $content_navigation );
if ( isset( $content_navigation['user-menu'] ) ) {
if ( self::isSkinVersionLegacy() ) {
// Remove user page from personal toolbar since it will be inside the personal menu for logged-in
// users in legacy Vector.
unset( $content_navigation['user-page'] );
} else {
// For modern Vector, rearrange some links in the personal toolbar.
self::updateUserLinksItems( $sk, $content_navigation );
}
}
}
}
@ -389,14 +381,6 @@ class Hooks {
$bodyAttrs['class'] .= ' skin-vector-search-vue';
}
if (
VectorServices::getFeatureManager()->isFeatureEnabled(
Constants::FEATURE_CONSOLIDATE_USER_LINKS
)
) {
$bodyAttrs['class'] .= ' skin-vector-consolidated-user-links';
}
if (
VectorServices::getFeatureManager()->isFeatureEnabled(
Constants::FEATURE_STICKY_HEADER

View File

@ -152,30 +152,6 @@ return [
]
);
// Feature: Consolidate personal menu links
// ================================
$featureManager->registerRequirement(
new OverridableConfigRequirement(
$services->getMainConfig(),
$context->getUser(),
$context->getRequest(),
null,
Constants::CONFIG_CONSOLIDATE_USER_LINKS,
Constants::REQUIREMENT_CONSOLIDATE_USER_LINKS,
Constants::QUERY_PARAM_CONSOLIDATE_USER_LINKS,
null
)
);
$featureManager->registerFeature(
Constants::FEATURE_CONSOLIDATE_USER_LINKS,
[
Constants::REQUIREMENT_FULLY_INITIALISED,
Constants::REQUIREMENT_LATEST_SKIN_VERSION,
Constants::REQUIREMENT_CONSOLIDATE_USER_LINKS
]
);
// Feature: Sticky header
// ================================
$featureManager->registerRequirement(

View File

@ -86,10 +86,7 @@ class SkinVector extends SkinMustache {
$options['scripts'] = [ 'skins.vector.legacy.js' ];
$options['styles'] = [ 'skins.vector.styles.legacy' ];
$options['template'] = 'skin-legacy';
} else {
if ( $this->shouldConsolidateUserLinks() ) {
$options['link'] = [ 'text-wrapper' => [ 'tag' => 'span' ] ];
}
unset( $options['link'] );
}
$options['templateDirectory'] = __DIR__ . '/templates';
@ -151,18 +148,6 @@ class SkinVector extends SkinMustache {
( $this->isLanguagesInHeader() && empty( $this->getLanguagesCached() ) );
}
/**
* If in modern Vector and the config is set to consolidate user links, enable user link consolidation.
* @return bool
*/
private function shouldConsolidateUserLinks() {
$featureManager = VectorServices::getFeatureManager();
return !$this->isLegacy() &&
$featureManager->isFeatureEnabled(
Constants::FEATURE_CONSOLIDATE_USER_LINKS
);
}
/**
* Returns HTML for the create account button inside the anon user links
* @param string[] $returnto array of query strings used to build the login link
@ -340,8 +325,6 @@ class SkinVector extends SkinMustache {
// Conditionally used values must use null to indicate absence (not false or '').
$commonSkinData = array_merge( $parentData, [
'is-consolidated-user-links' => $this->shouldConsolidateUserLinks(),
'is-article' => (bool)$out->isArticle(),
'is-anon' => $this->getUser()->isAnon(),
@ -371,8 +354,7 @@ class SkinVector extends SkinMustache {
];
}
$shouldConsolidateUserLinks = $this->shouldConsolidateUserLinks();
if ( $shouldConsolidateUserLinks ) {
if ( !$this->isLegacy() ) {
$commonSkinData['data-vector-user-links'] = $this->getUserLinksTemplateData(
$commonSkinData['data-portlets'],
$commonSkinData['is-anon'],
@ -382,7 +364,7 @@ class SkinVector extends SkinMustache {
$commonSkinData['data-search-box'] = $this->getSearchData(
$commonSkinData['data-search-box'],
$shouldConsolidateUserLinks
!$this->isLegacy()
);
return $commonSkinData;
@ -392,13 +374,13 @@ class SkinVector extends SkinMustache {
* Annotates search box with Vector-specific information
*
* @param array $searchBoxData
* @param bool $shouldConsolidateUserLinks
* @param bool $isCollapsible
* @return array modified version of $searchBoxData
*/
private function getSearchData( array $searchBoxData, bool $shouldConsolidateUserLinks ) {
private function getSearchData( array $searchBoxData, bool $isCollapsible ) {
$searchClass = 'vector-search-box';
if ( $shouldConsolidateUserLinks ) {
if ( $isCollapsible ) {
$searchClass .= ' vector-search-box-collapses';
}
@ -408,7 +390,7 @@ class SkinVector extends SkinMustache {
// Annotate search box with a component class.
$searchBoxData['class'] = $searchClass;
$searchBoxData['is-collapsible'] = $shouldConsolidateUserLinks;
$searchBoxData['is-collapsible'] = $isCollapsible;
// At lower resolutions the search input is hidden search and only the submit button is shown.
// It should behave like a form submit link (e.g. submit the form with no input value).
@ -546,7 +528,9 @@ class SkinVector extends SkinMustache {
$portletData['heading-class'] = 'vector-menu-heading';
// Add target class to apply different icon to personal menu dropdown for logged in users.
if ( $portletData['id'] === 'p-personal' ) {
if ( $this->shouldConsolidateUserLinks() ) {
if ( $this->isLegacy() ) {
$portletData['class'] .= ' vector-user-menu-legacy';
} else {
$portletData['class'] .= ' vector-user-menu';
$portletData['class'] .= $this->loggedin ?
' vector-user-menu-logged-in' :
@ -555,8 +539,6 @@ class SkinVector extends SkinMustache {
$portletData['heading-class'] .= $this->loggedin ?
' mw-ui-icon-wikimedia-userAvatar' :
' mw-ui-icon-wikimedia-ellipsis';
} else {
$portletData['class'] .= ' vector-user-menu-legacy';
}
}
if ( $portletData['id'] === 'p-lang' && $this->isLanguagesInHeader() ) {
@ -577,8 +559,6 @@ class SkinVector extends SkinMustache {
): array {
switch ( $label ) {
case 'user-menu':
$type = $this->shouldConsolidateUserLinks() ? self::MENU_TYPE_DROPDOWN : self::MENU_TYPE_DEFAULT;
break;
case 'actions':
case 'variants':
$type = self::MENU_TYPE_DROPDOWN;

View File

@ -12,12 +12,5 @@
</label>
{{>Logo}}
{{#data-search-box}}{{>SearchBox}}{{/data-search-box}}
{{^is-consolidated-user-links}}
{{#data-portlets}}
{{#data-personal}}{{>Menu}}{{/data-personal}}
{{/data-portlets}}
{{/is-consolidated-user-links}}
{{#is-consolidated-user-links}}
{{#data-vector-user-links}}{{>UserLinks}}{{/data-vector-user-links}}
{{/is-consolidated-user-links}}
{{#data-vector-user-links}}{{>UserLinks}}{{/data-vector-user-links}}
</header>

View File

@ -13,15 +13,3 @@
margin: 0;
}
}
// FIXME: Move these rules to the #p-personal li selector in
// skins.vector.styles.legacy/layouts/screen.less when modern Vector no longer
// uses the legacy user menu and only uses the new consolidated user links
// menu.
.vector-user-menu-legacy li {
margin-left: 0.75em;
// `padding-top` instead of `margin-top` necessary for
// anonymous user icon position below
padding-top: 0.5em;
line-height: @line-height-nav-personal;
}

View File

@ -33,6 +33,11 @@ body {
li {
float: left;
margin-left: 0.75em;
// `padding-top` instead of `margin-top` necessary for
// anonymous user icon position below
padding-top: 0.5em;
line-height: @line-height-nav-personal;
}
}

View File

@ -104,20 +104,6 @@
unit( 500px / @font-size-browser, em ) -
( 2 * @padding-horizontal-page-container );
// 31.25em - 3.75em = 27.5em @ 16
@min-width-supported-consolidated-links: unit( 340px / @font-size-browser, em );
// Width used to determine when to break the personal tools onto a separate line
// below the search box.
@width-comfortable:
( 2 * @padding-horizontal-page-container ) +
@size-sidebar-button +
@margin-horizontal-sidebar-button-icon +
@min-width-logo +
( 2 * @margin-horizontal-search ) +
@max-width-search +
@min-width-personal-tools;
// 3.75em + 2.75em + 0.75em + 11.25em + 5.71428571em + 35.71428571em + 18.75em = 78.67857142em @ 16
@border-color-sidebar: @background-color-secondary--modern;
@background-color-secondary--modern: #f8f9fa;
@background-color-page-container: @background-color-base;
@ -162,10 +148,6 @@ body {
position: relative;
z-index: @z-index-header;
.skin-vector-consolidated-user-links & {
flex-wrap: nowrap;
}
.skin-vector-sticky-header & {
position: sticky;
top: 0;
@ -180,10 +162,6 @@ body {
z-index: @z-index-menu;
flex-grow: 1;
.skin-vector-consolidated-user-links & {
margin: 0;
}
// #searchform is only a direct child of #p-search before wvui-loads. After
// wvui loads, `.wvui-typeahead-search` becomes the direct child and is the
// element where these styles should apply.
@ -321,11 +299,6 @@ body {
// For devices too small, they should be more useable with horizontal scrolling.
// e.g. Portrait on an iPad
min-width: @min-width-supported;
// When consolidated user links is enabled, there is more space for compression.
.skin-vector-consolidated-user-links & {
min-width: @min-width-supported-consolidated-links;
}
}
.skin--responsive .mw-page-container {
@ -425,26 +398,12 @@ body {
}
}
// At low resolutions the search must be pushed to the right of the screen
// We use @width-comfortable to determine this threshold as we know it's not possible for
// personal tools to be on the same line at this resolution.
// FIXME: Remove these styles and the @width-comfortable breakpoint after user links has become the default
@media ( max-width: @width-comfortable ) {
body:not( .skin-vector-consolidated-user-links ) {
#p-search > div > #searchform,
#p-search .wvui-typeahead-search {
margin-left: auto;
}
}
}
@media ( min-width: @width-breakpoint-desktop ) {
.mw-header {
flex-wrap: wrap;
}
#p-search,
.skin-vector-consolidated-user-links #p-search {
#p-search {
margin: 0 @margin-end-search 0 @margin-horizontal-search;
// Support: IE 8, Firefox 18-, Chrome 19-, Safari 5.1-, Opera 19-, Android 4.4.4-.
width: 13.2em;

View File

@ -15,8 +15,6 @@
@import './components/VueEnhancedSearchBox.less';
@import './components/Sidebar.less';
@import './components/LanguageButton.less';
// This import can be removed when $wgVectorConsolidateUserLinks feature flag is removed.
@import '../skins.vector.styles.legacy/components/UserLinks.less';
@import './components/UserLinks.less';
}

View File

@ -51,7 +51,12 @@
"sitesubtitle",
"sitetitle",
"tagline"
]
],
"link": {
"text-wrapper": {
"tag": "span"
}
}
}
]
}
@ -318,13 +323,6 @@
"value": false,
"description": "@var boolean Enables or disables the language in header treatment A/B test. See https://phabricator.wikimedia.org/T280825 and associated tasks for additional detail."
},
"VectorConsolidateUserLinks": {
"value": {
"logged_in": false,
"logged_out": false
},
"description": "@var array Moves user links into a consolidated dropdown menu with the exception of several top level items including echo, user page and create account links."
},
"VectorStickyHeader": {
"value": {
"logged_in": false,

View File

@ -12,7 +12,5 @@ export const legacySimpleSearch = () => `
`;
export const simpleSearch = () => `
<div class="skin-vector-consolidated-user-links">
${mustache.render( searchBoxTemplate, searchBoxData )}
</div>
${mustache.render( searchBoxTemplate, searchBoxData )}
`;

View File

@ -350,211 +350,4 @@ class OverridableConfigRequirementTest extends \MediaWikiUnitTestCase {
$this->assertSame( $expected, $requirement->isMet(), $msg );
}
public function providerUserLinksTreatmentRequirement() {
return [
[
// Is consolidate user links enabled (backward compatibility with boolean values)
false,
// note 0 = anon user
0,
// `vectoruserlinks` query param
null,
false,
'If nothing enabled (old config), nobody gets new treatment'
],
[
// Is consolidate user links enabled
[
'logged_in' => false,
'logged_out' => false,
],
// note 0 = anon user
0,
// `vectoruserlinks` query param
null,
false,
'If nothing enabled (new config), nobody gets new treatment'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
true,
// note 0 = anon user
0,
// `vectoruserlinks` query param
null,
true,
'Anon users should get new treatment if enabled (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => true,
'logged_out' => true,
],
// note 0 = anon user
0,
// `vectoruserlinks` query param
null,
true,
'Anon users should get new treatment if enabled (new config)'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
true,
// Logged in user
2,
// `vectoruserlinks` query param
null,
true,
'Logged in users should get new treatment if enabled (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => true,
'logged_out' => true,
],
// Logged in user
2,
// `vectoruserlinks` query param
null,
true,
'Logged in users should get new treatment if enabled (new config)'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
false,
// note 0 = anon user
0,
// `vectoruserlinks` query param
"1",
true,
'Anon users get new treatment when query param set to "1" regardless of config (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => false,
'logged_out' => false,
],
// note 0 = anon user
0,
// `vectoruserlinks` query param
"1",
true,
'Anon users get new treatment when query param set to "1" regardless of config (new config)'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
true,
// note 0 = anon user
0,
// `vectoruserlinks` query param
"0",
false,
'Anon user get old treatment when query param set to "0" regardless of config (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => true,
'logged_out' => true,
],
// note 0 = anon user
0,
// `vectoruserlinks` query param
"0",
false,
'Anon user get old treatment when query param set to "0" regardless of config (new config)'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
false,
// Logged in user
2,
// `vectoruserlinks` query param
"1",
true,
'Logged in users get new treatment when query param set to "1" regardless of config (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => false,
'logged_out' => false,
],
// Logged in user
2,
// `vectoruserlinks` query param
"1",
true,
'Logged in users get new treatment when query param set to "1" regardless of config (new config)'
],
[
// Is consolidate user links enabled (backward compatibility with boolean values)
true,
// Logged in user
2,
// `vectoruserlinks` query param
"0",
false,
'Logged in user get old treatment when query param set to "0" regardless of config (old config)'
],
[
// Is consolidate user links enabled
[
'logged_in' => true,
'logged_out' => true,
],
// Logged in user
2,
// `vectoruserlinks` query param
"0",
false,
'Logged in user get old treatment when query param set to "0" regardless of config (new config)'
],
];
}
/**
* @covers ::isMet
* @dataProvider providerUserLinksTreatmentRequirement
* @param bool $configValue
* @param int $userId
* @param string|null $queryParam
* @param bool $expected
* @param string $msg
*/
public function testUserLinksTreatmentRequirement(
$configValue,
$userId,
$queryParam,
$expected,
$msg
) {
$config = new HashConfig( [
Constants::CONFIG_CONSOLIDATE_USER_LINKS => $configValue,
] );
$user = $this->createMock( User::class );
$user->method( 'isRegistered' )->willReturn( $userId !== 0 );
$user->method( 'getID' )->willReturn( $userId );
$request = $this->createMock( WebRequest::class );
$request->method( 'getCheck' )->willReturn( $queryParam !== null );
$request->method( 'getBool' )->willReturn( (bool)$queryParam );
$requirement = new OverridableConfigRequirement(
$config,
$user,
$request,
null,
'VectorConsolidateUserLinks',
'ConsolidateUserLinks',
'vectoruserlinks',
null
);
$this->assertSame( $expected, $requirement->isMet(), $msg );
}
}