Extract SkinOptions to separate class

SkinOptions array was used to determine which options are available
for current session. Once we started extracting things from
SkinMinerva class, we found out that lots of things depend on
SkinOptions.

For example MainMenu/PageActionsMenu depend on skinsOptions var.
We could pass $skin object as dependency to a menu builder, but
this would cause a circural dependency (Skin depends on menu builder,
menu builder depends on skin) which is an anti-pattern.
In order to avoid such situations lets prepare first, and extract
the SkinOptions to a separate class, register it as a service
so different parts of Skin Minerva can freely use a single instance
of SkinOptions object.

Bug: T216152
Bug: T221012
Change-Id: Icd5da546e1bfaf8d9bfe86dab3b659a88eae19e4
This commit is contained in:
Piotr Miazga 2019-04-10 23:43:50 +02:00 committed by Pmiazga
parent 13153394a3
commit 258e635ae5
8 changed files with 217 additions and 142 deletions

View File

@ -19,6 +19,7 @@
*/
use MediaWiki\MediaWikiServices;
use MediaWiki\Minerva\SkinOptions;
/**
* Hook handlers for Minerva skin.
@ -191,33 +192,34 @@ class MinervaHooks {
) {
// setSkinOptions is not available
if ( $skin instanceof SkinMinerva ) {
$services = \MediaWiki\MediaWikiServices::getInstance();
$services = MediaWikiServices::getInstance();
$featureManager = $services
->getService( 'MobileFrontend.FeaturesManager' );
$userMode = $services->getService( 'MobileFrontend.AMC.UserMode' );
$skinOptions = $services->getService( 'Minerva.SkinOptions' );
$isBeta = $mobileContext->isBetaGroupMember();
$skin->setSkinOptions( [
SkinMinerva::OPTION_AMC => $userMode->isEnabled(),
SkinMinerva::OPTIONS_TALK_AT_TOP => $featureManager->isFeatureAvailableForCurrentUser(
$skinOptions->setMultiple( [
SkinOptions::OPTION_AMC => $userMode->isEnabled(),
SkinOptions::OPTIONS_TALK_AT_TOP => $featureManager->isFeatureAvailableForCurrentUser(
'MinervaTalkAtTop'
),
SkinMinerva::OPTIONS_MOBILE_BETA
SkinOptions::OPTIONS_MOBILE_BETA
=> $isBeta,
SkinMinerva::OPTION_CATEGORIES
SkinOptions::OPTION_CATEGORIES
=> $featureManager->isFeatureAvailableInContext( 'MinervaShowCategoriesButton',
$mobileContext ),
SkinMinerva::OPTION_BACK_TO_TOP
SkinOptions::OPTION_BACK_TO_TOP
=> $featureManager->isFeatureAvailableInContext( 'MinervaEnableBackToTop', $mobileContext ),
SkinMinerva::OPTION_PAGE_ISSUES
SkinOptions::OPTION_PAGE_ISSUES
=> $featureManager->isFeatureAvailableInContext(
'MinervaPageIssuesNewTreatment', $mobileContext
),
SkinMinerva::OPTION_SHARE_BUTTON
SkinOptions::OPTION_SHARE_BUTTON
=> $featureManager->isFeatureAvailableInContext( 'MinervaShareButton', $mobileContext ),
SkinMinerva::OPTION_TOGGLING => true,
SkinMinerva::OPTION_MOBILE_OPTIONS => true,
SkinMinerva::OPTIONS_HISTORY_PAGE_ACTIONS => $featureManager->isFeatureAvailableForCurrentUser(
SkinOptions::OPTION_TOGGLING => true,
SkinOptions::OPTION_MOBILE_OPTIONS => true,
SkinOptions::OPTIONS_HISTORY_PAGE_ACTIONS => $featureManager->isFeatureAvailableForCurrentUser(
'MinervaHistoryInPageActions'
),
] );

View File

@ -1,6 +1,7 @@
<?php
use MediaWiki\MediaWikiServices;
use MediaWiki\Minerva\SkinOptions;
use MediaWiki\Minerva\SkinUserPageHelper;
return [
@ -9,5 +10,8 @@ return [
},
'Minerva.SkinUserPageHelper' => function ( MediaWikiServices $services ) {
return new SkinUserPageHelper( RequestContext::getMain()->getTitle() );
},
'Minerva.SkinOptions' => function ( MediaWikiServices $services ) {
return new SkinOptions();
}
];

102
includes/SkinOptions.php Normal file
View File

@ -0,0 +1,102 @@
<?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
*/
namespace MediaWiki\Minerva;
/**
* A wrapper for all available Skin options.
*/
final class SkinOptions {
/** Set of keys for available skin options. See $skinOptions. */
const OPTION_MOBILE_OPTIONS = 'mobileOptionsLink';
const OPTION_AMC = 'amc';
const OPTION_CATEGORIES = 'categories';
const OPTION_BACK_TO_TOP = 'backToTop';
const OPTION_PAGE_ISSUES = 'pageIssues';
const OPTION_SHARE_BUTTON = 'shareButton';
const OPTION_TOGGLING = 'toggling';
const OPTIONS_MOBILE_BETA = 'beta';
const OPTIONS_TALK_AT_TOP = 'talkAtTop';
const OPTIONS_HISTORY_PAGE_ACTIONS = 'historyInPageActions';
/** @var array skin specific options */
private $skinOptions = [
// Defaults to true for desktop mode.
self::OPTION_AMC => true,
self::OPTIONS_MOBILE_BETA => false,
/**
* Whether the main menu should include a link to
* Special:Preferences of Special:MobileOptions
*/
self::OPTION_MOBILE_OPTIONS => false,
/** Whether a categories button should appear at the bottom of the skin. */
self::OPTION_CATEGORIES => false,
/** Whether a back to top button appears at the bottom of the view page */
self::OPTION_BACK_TO_TOP => false,
/** Whether a share button should appear in icons section */
self::OPTION_SHARE_BUTTON => false,
/** Whether sections can be collapsed (requires MobileFrontend and MobileFormatter) */
self::OPTION_TOGGLING => false,
self::OPTION_PAGE_ISSUES => false,
self::OPTIONS_TALK_AT_TOP => false,
self::OPTIONS_HISTORY_PAGE_ACTIONS => false,
];
/**
* override an existing option or options with new values
* @param array $options
*/
public function setMultiple( array $options ) {
$this->skinOptions = array_merge( $this->skinOptions, $options );
}
/**
* Return whether a skin option is truthy. Should be one of self:OPTION_* flags
* @param string $key
* @return bool
*/
public function get( $key ) {
if ( !array_key_exists( $key, $this->skinOptions ) ) {
throw new \OutOfBoundsException( "SkinOption $key doesn't exist" );
}
return $this->skinOptions[$key];
}
/**
* Get all skin options
* @return array
*/
public function getAll() {
return $this->skinOptions;
}
/**
* Return whether any of the skin options have been set
* @return bool
*/
public function hasSkinOptions() {
foreach ( $this->skinOptions as $key => $val ) {
if ( $val ) {
return true;
}
}
return false;
}
}

View File

@ -17,6 +17,8 @@
*
* @file
*/
use MediaWiki\MediaWikiServices;
use MediaWiki\Minerva\SkinOptions;
/**
* Extended Template class of BaseTemplate for mobile devices
@ -221,8 +223,9 @@ class MinervaTemplate extends BaseTemplate {
* @todo replace with template engines
*/
protected function render( $data ) {
$skinOptions = MediaWikiServices::getInstance()->getService( 'Minerva.SkinOptions' );
$templateParser = new TemplateParser( __DIR__ );
$skin = $this->getSkin();
$internalBanner = $data[ 'internalBanner' ];
$preBodyHtml = isset( $data['prebodyhtml'] ) ? $data['prebodyhtml'] : '';
$hasHeadingHolder = $internalBanner || $preBodyHtml || isset( $data['page_actions'] );
@ -264,8 +267,8 @@ class MinervaTemplate extends BaseTemplate {
'contenthtml' => $this->getContentHtml( $data ),
'secondaryactionshtml' => $this->getSecondaryActionsHtml(),
'footer' => $this->getFooterTemplateData( $data ),
'isBeta' => $skin->getSkinOption( SkinMinerva::OPTIONS_MOBILE_BETA ),
'tabs' => $hasTalkTabs && $skin->getSkinOption( SkinMinerva::OPTIONS_TALK_AT_TOP ) ? [
'isBeta' => $skinOptions->get( SkinOptions::OPTIONS_MOBILE_BETA ),
'tabs' => $hasTalkTabs && $skinOptions->get( SkinOptions::OPTIONS_TALK_AT_TOP ) ? [
'items' => array_values( $data['content_navigation']['namespaces'] ),
] : false,
];

View File

@ -19,6 +19,7 @@
*/
use MediaWiki\Minerva\MenuBuilder;
use MediaWiki\Minerva\SkinOptions;
use MediaWiki\MediaWikiServices;
/**
@ -27,17 +28,6 @@ use MediaWiki\MediaWikiServices;
* @ingroup Skins
*/
class SkinMinerva extends SkinTemplate {
/** Set of keys for available skin options. See $skinOptions. */
const OPTION_MOBILE_OPTIONS = 'mobileOptionsLink';
const OPTION_AMC = 'amc';
const OPTION_CATEGORIES = 'categories';
const OPTION_BACK_TO_TOP = 'backToTop';
const OPTION_PAGE_ISSUES = 'pageIssues';
const OPTION_SHARE_BUTTON = 'shareButton';
const OPTION_TOGGLING = 'toggling';
const OPTIONS_MOBILE_BETA = 'beta';
const OPTIONS_TALK_AT_TOP = 'talkAtTop';
const OPTIONS_HISTORY_PAGE_ACTIONS = 'historyInPageActions';
/** @const LEAD_SECTION_NUMBER integer which corresponds to the lead section
in editing mode */
@ -51,6 +41,17 @@ class SkinMinerva extends SkinTemplate {
/** @var bool Whether the page is also available in other languages or variants */
protected $doesPageHaveLanguages = false;
/** @var SkinOptions */
private $skinOptions;
/**
* Initialize Minerva Skin
*/
public function __construct() {
parent::__construct( 'minerva' );
$this->skinOptions = MediaWikiServices::getInstance()->getService( 'Minerva.SkinOptions' );
}
/**
* Returns the site name for the footer, either as a text or <img> tag
* @return string
@ -89,59 +90,6 @@ class SkinMinerva extends SkinTemplate {
return $sitename;
}
/** @var array skin specific options */
protected $skinOptions = [
// Defaults to true for desktop mode.
self::OPTION_AMC => true,
self::OPTIONS_MOBILE_BETA => false,
/**
* Whether the main menu should include a link to
* Special:Preferences of Special:MobileOptions
*/
self::OPTION_MOBILE_OPTIONS => false,
/** Whether a categories button should appear at the bottom of the skin. */
self::OPTION_CATEGORIES => false,
/** Whether a back to top button appears at the bottom of the view page */
self::OPTION_BACK_TO_TOP => false,
/** Whether a share button should appear in icons section */
self::OPTION_SHARE_BUTTON => false,
/** Whether sections can be collapsed (requires MobileFrontend and MobileFormatter) */
self::OPTION_TOGGLING => false,
self::OPTION_PAGE_ISSUES => false,
self::OPTIONS_TALK_AT_TOP => false,
self::OPTIONS_HISTORY_PAGE_ACTIONS => false,
];
/**
* override an existing option or options with new values
* @param array $options
*/
public function setSkinOptions( $options ) {
$this->skinOptions = array_merge( $this->skinOptions, $options );
}
/**
* Return whether a skin option is truthy
* @param string $key
* @return bool
*/
public function getSkinOption( $key ) {
return $this->skinOptions[$key];
}
/**
* Return whether any of the skin options have been set
* @return bool
*/
public function hasSkinOptions() {
foreach ( $this->skinOptions as $key => $val ) {
if ( $val ) {
return true;
}
}
return false;
}
/**
* initialize various variables and generate the template
* @return QuickTemplate
@ -198,7 +146,7 @@ class SkinMinerva extends SkinTemplate {
// If it's a talk page, add a link to the main namespace page
// In AMC we do not need to do this as there is an easy way back to the article page
// via the talk/article tabs.
if ( $title->isTalkPage() && !$this->getSkinOption( self::OPTION_AMC ) ) {
if ( $title->isTalkPage() && !$this->skinOptions->get( SkinOptions::OPTION_AMC ) ) {
// if it's a talk page for which we have a special message, use it
switch ( $title->getNamespace() ) {
case NS_USER_TALK:
@ -259,7 +207,7 @@ class SkinMinerva extends SkinTemplate {
}
if ( $action === 'history' && $title->exists() ) {
return $this->getSkinOption( self::OPTIONS_HISTORY_PAGE_ACTIONS );
return $this->skinOptions->get( SkinOptions::OPTIONS_HISTORY_PAGE_ACTIONS );
}
if (
@ -313,7 +261,8 @@ class SkinMinerva extends SkinTemplate {
*/
public function getPageClasses( $title ) {
$className = parent::getPageClasses( $title );
$className .= ' ' . ( $this->getSkinOption( self::OPTIONS_MOBILE_BETA ) ? 'beta' : 'stable' );
$className .= ' ' . ( $this->skinOptions->get( SkinOptions::OPTIONS_MOBILE_BETA )
? 'beta' : 'stable' );
if ( $title->isMainPage() ) {
$className .= ' page-Main_Page ';
@ -325,7 +274,7 @@ class SkinMinerva extends SkinTemplate {
// The new treatment should only apply to the main namespace
if (
$title->getNamespace() === NS_MAIN &&
$this->getSkinOption( self::OPTION_PAGE_ISSUES )
$this->skinOptions->get( SkinOptions::OPTION_PAGE_ISSUES )
) {
$className .= ' issues-group-B';
}
@ -346,7 +295,7 @@ class SkinMinerva extends SkinTemplate {
* @return bool
*/
private function hasCategoryLinks() {
if ( !$this->getSkinOption( self::OPTION_CATEGORIES ) ) {
if ( !$this->skinOptions->get( SkinOptions::OPTION_CATEGORIES ) ) {
return false;
}
$categoryLinks = $this->getOutput()->getCategoryLinks();
@ -538,7 +487,7 @@ class SkinMinerva extends SkinTemplate {
$returnToTitle = $this->getTitle()->getPrefixedText();
// Links specifically for mobile mode
if ( $this->getSkinOption( self::OPTION_MOBILE_OPTIONS ) ) {
if ( $this->skinOptions->get( SkinOptions::OPTION_MOBILE_OPTIONS ) ) {
// Settings link
$menu->insert( 'settings' )
->addComponent(
@ -1084,7 +1033,7 @@ class SkinMinerva extends SkinTemplate {
// in stable it will link to the wikitext talk page
$title = $this->getTitle();
$subjectPage = $title->getSubjectPage();
$talkAtBottom = !$this->getSkinOption( self::OPTIONS_TALK_AT_TOP ) ||
$talkAtBottom = !$this->skinOptions->get( SkinOptions::OPTIONS_TALK_AT_TOP ) ||
$subjectPage->isMainPage();
$namespaces = $tpl->data['content_navigation']['namespaces'];
if ( !$this->getUserPageHelper()->isUserPage()
@ -1306,12 +1255,8 @@ class SkinMinerva extends SkinTemplate {
* @return array
*/
public function getSkinConfigVariables() {
$title = $this->getTitle();
$user = $this->getUser();
$out = $this->getOutput();
$vars = [
'wgMinervaFeatures' => $this->skinOptions,
'wgMinervaFeatures' => $this->skinOptions->getAll(),
'wgMinervaDownloadNamespaces' => $this->getConfig()->get( 'MinervaDownloadNamespaces' ),
'wgMinervaMenuData' => $this->getMenuData(),
];
@ -1374,10 +1319,10 @@ class SkinMinerva extends SkinTemplate {
$modules[] = 'skins.minerva.talk';
}
if ( $this->hasSkinOptions() ) {
if ( $this->skinOptions->hasSkinOptions() ) {
$modules[] = 'skins.minerva.options';
}
if ( $this->getSkinOption( self::OPTION_SHARE_BUTTON ) ) {
if ( $this->skinOptions->get( SkinOptions::OPTION_SHARE_BUTTON ) ) {
$modules[] = 'skins.minerva.share';
}
return $modules;
@ -1407,7 +1352,7 @@ class SkinMinerva extends SkinTemplate {
]
);
if ( $this->getSkinOption( self::OPTION_TOGGLING ) ) {
if ( $this->skinOptions->get( SkinOptions::OPTION_TOGGLING ) ) {
// Extension can unload "toggling" modules via the hook
$modules['toggling'] = [ 'skins.minerva.toggling' ];
}
@ -1428,7 +1373,7 @@ class SkinMinerva extends SkinTemplate {
*/
public function addToBodyAttributes( $out, &$bodyAttrs ) {
$classes = $out->getProperty( 'bodyClassName' );
if ( $this->getSkinOption( self::OPTION_AMC ) ) {
if ( $this->skinOptions->get( SkinOptions::OPTION_AMC ) ) {
$classes .= ' minerva--amc-enabled';
} else {
$classes .= ' minerva--amc-disabled';
@ -1463,7 +1408,7 @@ class SkinMinerva extends SkinTemplate {
$styles[] = 'skins.minerva.icons.loggedin';
}
if ( $this->getSkinOption( self::OPTION_AMC ) ) {
if ( $this->skinOptions->get( SkinOptions::OPTION_AMC ) ) {
$styles[] = 'skins.minerva.amc.styles';
}

View File

@ -76,6 +76,7 @@
"MediaWiki\\Minerva\\MenuBuilder": "includes/skins/MenuBuilder.php",
"MediaWiki\\Minerva\\MenuEntry": "includes/skins/MenuEntry.php",
"MediaWiki\\Minerva\\ResourceLoaderLessVarFileModule": "includes/ResourceLoaderLessVarFileModule.php",
"MediaWiki\\Minerva\\SkinOptions": "includes/SkinOptions.php",
"MediaWiki\\Minerva\\SkinUserPageHelper": "includes/skins/SkinUserPageHelper.php"
},
"ConfigRegistry": {

View File

@ -0,0 +1,50 @@
<?php
namespace Tests\MediaWiki\Minerva;
use MediaWiki\Minerva\SkinOptions;
use MediaWikiTestCase;
/**
* Class SkinMinervaTest
* @package Tests\MediaWiki\Minerva
* @group MinervaNeue
* @coversDefaultClass \MediaWiki\Minerva\SkinOptions
*/
class SkinOptionsTest extends MediaWikiTestCase {
/**
* @covers ::get
* @covers ::getAll
* @covers ::setMultiple
*/
public function testSettersAndGetters() {
$options = new SkinOptions();
$defaultValue = $options->get( SkinOptions::OPTION_AMC );
$options->setMultiple( [ SkinOptions::OPTION_AMC => !$defaultValue ] );
$allOptions = $options->getAll();
$this->assertEquals( !$defaultValue, $options->get( SkinOptions::OPTION_AMC ) );
$this->assertArrayHasKey( SkinOptions::OPTION_AMC, $allOptions );
$this->assertEquals( !$defaultValue, $allOptions[ SkinOptions::OPTION_AMC ] );
}
/**
* @covers ::hasSkinOptions
*/
public function testHasSkinOptions() {
$options = new SkinOptions();
// set OPTION_AMC to true just in case someone decides to set everything to false
// sometime in the future.
$options->setMultiple( [ SkinOptions::OPTION_AMC => true ] );
$this->assertTrue( $options->hasSkinOptions() );
$options->setMultiple( [ SkinOptions::OPTION_BACK_TO_TOP => true ] );
$this->assertTrue( $options->hasSkinOptions() );
$options->setMultiple( [
SkinOptions::OPTION_AMC => false,
SkinOptions::OPTION_BACK_TO_TOP => false
] );
$this->assertFalse( $options->hasSkinOptions() );
}
}

View File

@ -2,6 +2,7 @@
namespace Tests\MediaWiki\Minerva;
use MediaWiki\Minerva\SkinOptions;
use MediaWikiTestCase;
use MinervaUI;
use MWTimestamp;
@ -41,43 +42,14 @@ class EchoNotifUser {
class SkinMinervaTest extends MediaWikiTestCase {
const OPTIONS_MODULE = 'skins.minerva.options';
/**
* @covers ::addToBodyAttributes
*/
public function testAddToBodyAttributes() {
// The `class` attribute gets set to the "bodyClassName" property by
// default.
$this->assertContains(
'no-js',
$this->addToBodyAttributes( 'no-js', false )
);
$classes = $this->addToBodyAttributes( 'no-js', true );
$this->assertContains( 'no-js', $classes );
}
private function addToBodyAttributes(
$bodyClassName
) {
$context = new RequestContext();
$context->setTitle( Title::newFromText( 'Test' ) );
$outputPage = $context->getOutput();
$outputPage->setProperty( 'bodyClassName', $bodyClassName );
$bodyAttrs = [ 'class' => '' ];
$skin = new SkinMinerva();
$skin->setContext( $context );
$skin->addToBodyAttributes( $outputPage, $bodyAttrs );
return explode( ' ', $bodyAttrs[ 'class' ] );
private function overrideSkinOptions( $options ) {
$mockOptions = new SkinOptions();
$mockOptions->setMultiple( $options );
$this->setService( 'Minerva.SkinOptions', $mockOptions );
}
/**
* @covers ::setContext
* @covers ::setSkinOptions
* @covers ::hasCategoryLinks
*/
public function testHasCategoryLinksWhenOptionIsOff() {
@ -87,14 +59,13 @@ class SkinMinervaTest extends MediaWikiTestCase {
$outputPage->expects( $this->never() )
->method( 'getCategoryLinks' );
$this->overrideSkinOptions( [ SkinOptions::OPTION_CATEGORIES => false ] );
$context = new RequestContext();
$context->setTitle( Title::newFromText( 'Test' ) );
$context->setOutput( $outputPage );
$skin = new SkinMinerva();
$skin->setContext( $context );
$skin->setSkinOptions( [ SkinMinerva::OPTION_CATEGORIES => false ] );
$skin = TestingAccessWrapper::newFromObject( $skin );
$this->assertEquals( $skin->hasCategoryLinks(), false );
@ -105,7 +76,6 @@ class SkinMinervaTest extends MediaWikiTestCase {
* @param array $categoryLinks
* @param bool $expected
* @covers ::setContext
* @covers ::setSkinOptions
* @covers ::hasCategoryLinks
*/
public function testHasCategoryLinks( array $categoryLinks, $expected ) {
@ -116,13 +86,14 @@ class SkinMinervaTest extends MediaWikiTestCase {
->method( 'getCategoryLinks' )
->will( $this->returnValue( $categoryLinks ) );
$this->overrideSkinOptions( [ SkinOptions::OPTION_CATEGORIES => true ] );
$context = new RequestContext();
$context->setTitle( Title::newFromText( 'Test' ) );
$context->setOutput( $outputPage );
$skin = new SkinMinerva();
$skin->setContext( $context );
$skin->setSkinOptions( [ SkinMinerva::OPTION_CATEGORIES => true ] );
$skin = TestingAccessWrapper::newFromObject( $skin );
@ -169,21 +140,18 @@ class SkinMinervaTest extends MediaWikiTestCase {
* @param string $moduleName Module name that is being tested
* @param bool $expected Whether the module is expected to be returned by the function being tested
*/
public function testGetContextSpecificModules( $backToTopValue,
$moduleName, $expected
) {
$skin = new SkinMinerva();
$skin->setSkinOptions( [
SkinMinerva::OPTION_AMC => false,
public function testGetContextSpecificModules( $backToTopValue, $moduleName, $expected ) {
$this->overrideSkinOptions( [
SkinOptions::OPTION_AMC => false,
'backToTop' => $backToTopValue,
] );
$skin = new SkinMinerva();
$title = Title::newFromText( 'Test' );
$testContext = RequestContext::getMain();
$testContext->setTitle( $title );
$skin->setContext( $testContext );
$skin->setSkinOptions( [
'backToTop' => $backToTopValue,
] );
if ( $expected ) {
$this->assertContains( $moduleName, $skin->getContextSpecificModules() );