golem
/
VectorGOLEM
9
0
Fork 0
Code Issues 1 Pull Requests Releases Wiki Activity

Remove SearchInHeader requirement/feature 

Styles will be cleaned up further in a follow up.

Bug: T258116
Change-Id: I878239a85ffbecb5e78d73aed5568c56dbd7d659
This commit is contained in:
jdlrobson 2020-09-30 09:02:31 -07:00 committed by Jdlrobson
parent c0bfa47157
commit eda19bd6e4
11 changed files with 5 additions and 378 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
includes/Constants.php
View File

@ -69,21 +69,6 @@ final class Constants {
public const CONFIG_KEY_DEFAULT_SIDEBAR_VISIBLE_FOR_ANONYMOUS_USER =
'VectorDefaultSidebarVisibleForAnonymousUser';
/**
* @var string
*/
public const CONFIG_SEARCH_IN_HEADER = 'VectorIsSearchInHeader';
/**
* @var string
*/
public const CONFIG_SEARCH_IN_HEADER_AB = 'VectorIsSearchInHeaderABTest';
/**
* @var string
*/
public const REQUIREMENT_SEARCH_IN_HEADER = 'VectorIsSearchInHeaderIsEnabled';
/**
* @var string
*/
@ -116,11 +101,6 @@ final class Constants {
*/
public const FEATURE_LATEST_SKIN = 'LatestSkin';
/**
* @var string
*/
public const FEATURE_SEARCH_IN_HEADER = 'TemporarySearchInHeader';
/**
* @var string
*/

96
includes/FeatureManagement/Requirements/SearchInHeaderRequirement.php
View File

@ -1,96 +0,0 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
* @since 1.36
*/
namespace Vector\FeatureManagement\Requirements;
use Config;
use Vector\Constants;
use Vector\FeatureManagement\Requirement;
/**
* Check whether the search should be part of the header or part of
* the tabs (as in the old design).
* The search in header is enabled if:
* - the associated feature flag has been enabled
* - the feature flag for the A/B test is enabled,
* and the user is logged and bucketed,
* in which case 50% of logged in users will see the search in the header
*
* @unstable
*
* @package Vector\FeatureManagement\Requirements
* @internal
*/
final class SearchInHeaderRequirement implements Requirement {
/**
* @var Config
*/
private $config;
/**
* @var \User
*/
private $user;
/**
* This constructor accepts all dependencies needed to determine
* whether search in header is enabled for current user and config.
*
* @param \Config $config
* @param \User $user
*/
public function __construct( \Config $config, \User $user ) {
$this->config = $config;
$this->user = $user;
}
/**
* @inheritDoc
*/
public function getName() : string {
return Constants::REQUIREMENT_SEARCH_IN_HEADER;
}
/**
* If A/B test is enabled check whether the user is logged in and bucketed
* @return bool
*/
private function isBucketed() {
$isABTestEnabled = (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER_AB );
if ( $isABTestEnabled ) {
return $this->user->getId() % 2 === 0;
} else {
// if A/B test is disabled then resort to using CONFIG_SEARCH_IN_HEADER
return (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER );
}
}
/**
* @inheritDoc
* @throws \ConfigException
*/
public function isMet() : bool {
return $this->user->isRegistered() ?
$this->isBucketed() : (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER );
}
}

8
includes/Hooks.php
View File

@ -272,13 +272,7 @@ class Hooks {
//
// See https://codesearch.wmcloud.org/deployed/?q=skin-vector-search- for an up-to-date
// list.
$featureManager = VectorServices::getFeatureManager();
if ( $featureManager->isFeatureEnabled( Constants::FEATURE_SEARCH_IN_HEADER ) ) {
$bodyAttrs['class'] .= ' skin-vector-search-header';
} else {
$bodyAttrs['class'] .= ' skin-vector-search-header-legacy';
}
$bodyAttrs['class'] .= ' skin-vector-search-header';
}
/**

19
includes/ServiceWiring.php
View File

@ -27,7 +27,6 @@ use Vector\Constants;
use Vector\FeatureManagement\FeatureManager;
use Vector\FeatureManagement\Requirements\DynamicConfigRequirement;
use Vector\FeatureManagement\Requirements\LatestSkinVersionRequirement;
use Vector\FeatureManagement\Requirements\SearchInHeaderRequirement;
use Vector\SkinVersionLookup;
return [
@ -67,24 +66,6 @@ return [
]
);
// Feature (temporary): search in header
// ========================================
$featureManager->registerRequirement(
new SearchInHeaderRequirement(
$services->getMainConfig(),
$context->getUser()
)
);
$featureManager->registerFeature(
Constants::FEATURE_SEARCH_IN_HEADER,
// Requirements
[
Constants::REQUIREMENT_FULLY_INITIALISED,
Constants::REQUIREMENT_SEARCH_IN_HEADER
]
);
return $featureManager;
}
];

12
includes/SkinVector.php
View File

@ -117,8 +117,6 @@ class SkinVector extends SkinMustache {
// Conditionally used values must use null to indicate absence (not false or '').
$mainPageHref = Skin::makeMainPageUrl();
$isSearchInHeader = $featureManager->isFeatureEnabled( Constants::FEATURE_SEARCH_IN_HEADER );
$parentData = parent::getTemplateData();
$commonSkinData = array_merge( $parentData, [
@ -131,8 +129,7 @@ class SkinVector extends SkinMustache {
'html-categories' => $skin->getCategories(),
'is-search-in-header' => $isSearchInHeader,
'input-location' => $this->getSearchBoxInputLocation( $isSearchInHeader ),
'input-location' => $this->getSearchBoxInputLocation(),
'main-page-href' => $mainPageHref,
@ -157,18 +154,15 @@ class SkinVector extends SkinMustache {
/**
* Gets the value of the "input-location" parameter for the SearchBox Mustache template.
*
* @param bool $isSearchInHeader
* @return string Either `Constants::SEARCH_BOX_INPUT_LOCATION_DEFAULT` or
* `Constants::SEARCH_BOX_INPUT_LOCATION_MOVED`
*/
private function getSearchBoxInputLocation( bool $isSearchInHeader ) : string {
private function getSearchBoxInputLocation() : string {
if ( $this->isLegacy() ) {
return Constants::SEARCH_BOX_INPUT_LOCATION_DEFAULT;
}
return $isSearchInHeader
? Constants::SEARCH_BOX_INPUT_LOCATION_MOVED
: Constants::SEARCH_BOX_INPUT_LOCATION_DEFAULT;
return Constants::SEARCH_BOX_INPUT_LOCATION_MOVED;
}
/**

7
includes/templates/Header.mustache
View File

@ -9,14 +9,7 @@
tabindex="0">
{{msg-vector-action-toggle-sidebar}}
</label>
{{^is-search-in-header}}
<div class="mw-workspace-container mw-sidebar-container">
{{#data-sidebar}}{{>Sidebar}}{{/data-sidebar}}
</div>
{{/is-search-in-header}}
{{>Logo}}
{{#is-search-in-header}}
{{#data-search-box}}{{>SearchBox}}{{/data-search-box}}
{{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}}
{{/is-search-in-header}}
</header>

8
includes/templates/Navigation.mustache
View File

@ -1,10 +1,7 @@
<div id="mw-navigation">
<h2>{{msg-navigation-heading}}</h2>
<div id="mw-head">
{{^is-search-in-header}}
{{#data-personal-menu}}{{>Menu}}{{/data-personal-menu}}
{{/is-search-in-header}}
<div class="{{^is-search-in-header}}mw-content-container {{/is-search-in-header}}mw-article-toolbar-container">
<div class="mw-article-toolbar-container">
<div id="left-navigation">
{{#data-namespace-tabs}}{{>Menu}}{{/data-namespace-tabs}}
{{#data-variants}}{{>Menu}}{{/data-variants}}
@ -12,9 +9,6 @@
<div id="right-navigation">
{{#data-page-actions}}{{>Menu}}{{/data-page-actions}}
{{#data-page-actions-more}}{{>Menu}}{{/data-page-actions-more}}
{{^is-search-in-header}}
{{#data-search-box}}{{>SearchBox}}{{/data-search-box}}
{{/is-search-in-header}}
</div>
</div>
</div>

19
includes/templates/skin.mustache
View File

@ -31,30 +31,20 @@
object data-footer for footer template partial. see Footer.mustache for documentation.
}}
<div class="mw-page-container">
{{#is-search-in-header}}
<a class="mw-jump-link" href="#content">{{msg-vector-jumptocontent}}</a>
{{/is-search-in-header}}
<div class="mw-page-container-inner">
{{^is-search-in-header}}
<div id="mw-page-base" class="mw-header-placeholder noprint"></div>
{{/is-search-in-header}}
<input
type="checkbox"
id="mw-sidebar-checkbox"
class="mw-checkbox-hack-checkbox"
{{#sidebar-visible}}checked{{/sidebar-visible}}>
{{#is-search-in-header}}
{{>Header}}
{{/is-search-in-header}}
<div class="mw-workspace-container">
{{#is-search-in-header}}
{{#data-sidebar}}{{>Sidebar}}{{/data-sidebar}}
{{>Navigation}}
{{/is-search-in-header}}
<div class="mw-content-container">
{{! `role` is unnecessary but kept to support selectors in any gadgets or user styles. }}
<!-- Please do not use role attribute as CSS selector, it is deprecated. -->
@ -68,10 +58,6 @@
<div id="contentSub"{{{html-user-language-attributes}}}>{{{html-subtitle}}}</div>
<div id="contentSub2">{{{html-undelete-link}}}</div>
{{{html-user-message}}}
{{^is-search-in-header}}
<a class="mw-jump-link" href="#mw-sidebar-button">{{msg-vector-jumptonavigation}}</a>
<a class="mw-jump-link" href="#searchInput">{{msg-vector-jumptosearch}}</a>
{{/is-search-in-header}}
{{{html-body-content}}}
{{{html-categories}}}
</div>
@ -80,11 +66,6 @@
</div> {{! END mw-content-container }}
</div> {{! END mw-workspace-container }}
{{^is-search-in-header}}
{{>Header}}
{{>Navigation}}
{{/is-search-in-header}}
<div class="mw-workspace-container mw-footer-container">
<div class="mw-content-container">
{{#data-footer}}{{>Footer}}{{/data-footer}}

61
resources/skins.vector.styles/layout-default.less
View File

@ -32,20 +32,6 @@ body {
}
}
/* Space for header above content */
.skin-vector-search-header-legacy .mw-header-placeholder {
// Reserve space for the absolute positioned header and tabs.
height: @height-header + @height-tabs;
}
/* Header layout */
.skin-vector-search-header-legacy .mw-header {
position: absolute;
top: 0;
left: 0;
right: 0;
}
.mw-header {
// A min-height is set to account for projects where no icon is set.
min-height: @height-logo-icon;
@ -97,56 +83,20 @@ body {
top: -9999px;
}
/* Tabs */
.skin-vector-search-header-legacy #mw-head {
position: absolute;
top: 0;
right: 0;
width: 100%;
}
/* Navigation Containers */
.mw-article-toolbar-container {
// Clear the floats on #left-navigation and #right-navigation.
.mixin-clearfix();
}
.skin-vector-search-header-legacy #left-navigation {
margin-top: @height-header;
/* When right nav would overlap left nav, it's placed below it
(normal CSS floats behavior). This rule ensures that no empty space
is shown between them due to right nav's margin-top. Page layout
is still broken, but at least the nav overlaps only the page title
instead of half the content. */
margin-bottom: -@height-header;
}
#left-navigation {
float: left;
}
.skin-vector-search-header-legacy #right-navigation {
margin-top: @height-header;
}
#right-navigation {
float: right;
}
.skin-vector-search-header-legacy #p-personal {
position: absolute;
top: @top-personal-tools;
right: 0.75em;
z-index: @z-index-personal;
ul {
// Keep from spilling out into the first column
// For completeness this should also include the width of the logo.
// As a result it is possible for the personal tools to overlap the logo.
padding-left: @width-grid-column-one;
}
}
#p-personal {
li {
// Some extensions e.g. Echo may change the height of list items
@ -162,13 +112,6 @@ body {
margin-right: @margin-horizontal-sidebar-button-icon; // Accidentally the same.
}
.skin-vector-search-header-legacy #mw-panel {
// The sidebar is absolutely positioned inside the header which applies a top
// margin so we need to subtract this top margin in order to get the correct
// sidebar position.
top: @height-header - @margin-top-header;
}
#mw-panel {
position: absolute;
left: 0;
@ -178,7 +121,3 @@ body {
padding-left: @padding-left-sidebar;
z-index: @z-index-sidebar;
}
.skin-vector-search-header-legacy .mw-footer {
margin-top: 0;
}

6
skin.json
View File

@ -202,12 +202,6 @@
"VectorUseCoreSearch": {
"value": true
},
"VectorIsSearchInHeader": {
"value": true
},
"VectorIsSearchInHeaderABTest": {
"value": false
},
"VectorDefaultSidebarVisibleForAuthorisedUser": {
"value": true
},

127
tests/phpunit/integration/FeatureManagement/Requirements/SearchInHeaderRequirementTest.php
View File

@ -1,127 +0,0 @@
<?php
/**
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
* http://www.gnu.org/copyleft/gpl.html
*
* @file
* @since 1.36
*/
use Vector\Constants;
use Vector\FeatureManagement\Requirements\SearchInHeaderRequirement;
/**
* @group Vector
* @coversDefaultClass \Vector\FeatureManagement\Requirements\SearchInHeaderRequirement
*/
class SearchInHeaderRequirementTest extends \MediaWikiTestCase {
public function providerSearchInHeaderRequirement() {
return [
[
// Is enabled for anons
false,
// is A-B test enabled
false,
// note 0 = anon user
0,
false,
'If nothing enabled nobody gets search in header'
],
[
// Is enabled for anons
true,
// is A-B test enabled
false,
// note 0 = anon user
0,
true,
'All anons should get search in header if enable but when A/B test disabled'
],
[
// Is enabled for anons
true,
// is A-B test enabled
false,
0,
true,
'All even logged in users should get search in header when A/B test disabled'
],
[
// Is enabled for anons
true,
// is A-B test enabled
false,
1,
true,
'All odd logged in users should get search in header when A/B test disabled'
],
[
// Is enabled for anons
true,
// is A-B test enabled
true,
// note 0 = anon user
0,
true,
'All anons get search in header even when A/B enabled'
],
[
// Is enabled for anons
true,
// is A-B test enabled
true,
2,
true,
'Bucketed users get search in header when A/B test enabled'
],
[
// Is enabled for anons
true,
// is A-B test enabled
true,
1,
false,
'Non-Bucketed users do not get search in header when A/B test enabled'
],
];
}
/**
* @covers ::isMet
* @dataProvider providerSearchInHeaderRequirement
* @param bool $searchInHeaderConfigValue
* @param bool $abValue
* @param int $userId
* @param bool $expected
* @param string $msg
*/
public function testSearchInHeaderRequirement(
$searchInHeaderConfigValue, $abValue, $userId, $expected, $msg
) {
$config = new HashConfig( [
Constants::CONFIG_SEARCH_IN_HEADER => $searchInHeaderConfigValue,
Constants::CONFIG_SEARCH_IN_HEADER_AB => $abValue,
] );
$user = $this->getTestUser()->getUser();
$user->setId( $userId );
$requirement = new SearchInHeaderRequirement(
$config, $user
);
$this->assertSame( $requirement->isMet(), $expected, $msg );
}
}