From 4efd72a87995d6567a7bcdacec2c68622ca36d8f Mon Sep 17 00:00:00 2001 From: Piotr Miazga Date: Thu, 1 Aug 2019 18:18:18 +0200 Subject: [PATCH] Hygiene: Remove OPTIONS_ and OPTION_ prefixes from SKinOptions Additionally, renamed couple options for better readability. Bug: T221012 Change-Id: Ia347a60d469fba8f35afa7c70aa806a46271dccd --- includes/MinervaHooks.php | 24 ++++----- includes/ServiceWiring.php | 8 +-- includes/SkinOptions.php | 54 +++++++++---------- includes/menu/PageActions/ToolbarBuilder.php | 2 +- .../permissions/MinervaPagePermissions.php | 6 +-- includes/skins/MinervaTemplate.php | 6 +-- includes/skins/SkinMinerva.php | 28 +++++----- tests/phpunit/skins/SkinMinervaTest.php | 6 +-- tests/phpunit/unit/SkinOptionsTest.php | 20 +++---- 9 files changed, 77 insertions(+), 77 deletions(-) diff --git a/includes/MinervaHooks.php b/includes/MinervaHooks.php index ae528c5..ece6f9c 100644 --- a/includes/MinervaHooks.php +++ b/includes/MinervaHooks.php @@ -239,29 +239,29 @@ class MinervaHooks { $isBeta = $mobileContext->isBetaGroupMember(); $skinOptions->setMultiple( [ - SkinOptions::OPTION_AMC => $userMode->isEnabled(), - SkinOptions::OPTIONS_TALK_AT_TOP => $featureManager->isFeatureAvailableForCurrentUser( + SkinOptions::AMC_MODE => $userMode->isEnabled(), + SkinOptions::TALK_AT_TOP => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaTalkAtTop' ), - SkinOptions::OPTIONS_MOBILE_BETA + SkinOptions::BETA_MODE => $isBeta, - SkinOptions::OPTION_CATEGORIES + SkinOptions::CATEGORIES => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaShowCategoriesButton' ), - SkinOptions::OPTION_BACK_TO_TOP + SkinOptions::BACK_TO_TOP => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaEnableBackToTop' ), - SkinOptions::OPTION_PAGE_ISSUES + SkinOptions::PAGE_ISSUES => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaPageIssuesNewTreatment' ), - SkinOptions::OPTION_SHARE_BUTTON + SkinOptions::SHARE_BUTTON => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaShareButton' ), - SkinOptions::OPTION_TOGGLING => true, - SkinOptions::OPTION_MOBILE_OPTIONS => true, - SkinOptions::OPTIONS_HISTORY_PAGE_ACTIONS => $featureManager->isFeatureAvailableForCurrentUser( + SkinOptions::TOGGLING => true, + SkinOptions::MOBILE_OPTIONS => true, + SkinOptions::HISTORY_IN_PAGE_ACTIONS => $featureManager->isFeatureAvailableForCurrentUser( 'MinervaHistoryInPageActions' ), - SkinOptions::OPTION_OVERFLOW_SUBMENU => $featureManager->isFeatureAvailableForCurrentUser( + SkinOptions::TOOLBAR_SUBMENU => $featureManager->isFeatureAvailableForCurrentUser( self::FEATURE_OVERFLOW_PAGE_ACTIONS ), - SkinOptions::OPTION_TABS_ON_SPECIALS => false, + SkinOptions::TABS_ON_SPECIALS => false, ] ); Hooks::run( 'SkinMinervaOptionsInit', [ $skin, $skinOptions ] ); } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 08d69d2..c9d7649 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -42,7 +42,7 @@ return [ $definitions = $services->getService( 'Minerva.Menu.Definitions' ); $context = RequestContext::getMain(); - $builder = $options->get( SkinOptions::OPTION_AMC ) ? + $builder = $options->get( SkinOptions::AMC_MODE ) ? new AdvancedUserMenuBuilder( $context, $context->getUser(), @@ -60,9 +60,9 @@ return [ /** @var SkinOptions $options */ $options = $services->getService( 'Minerva.SkinOptions' ); $definitions = $services->getService( 'Minerva.Menu.Definitions' ); - $showMobileOptions = $options->get( SkinOptions::OPTION_MOBILE_OPTIONS ); + $showMobileOptions = $options->get( SkinOptions::MOBILE_OPTIONS ); $user = $context->getUser(); - $builder = $options->get( SkinOptions::OPTION_AMC ) ? + $builder = $options->get( SkinOptions::AMC_MODE ) ? new MainMenu\AdvancedBuilder( $showMobileOptions, $user, $definitions ) : new MainMenu\DefaultBuilder( $showMobileOptions, $user, $definitions ); @@ -91,7 +91,7 @@ return [ $userPageHelper, $languagesHelper ); - if ( $skinOptions->get( SkinOptions::OPTION_OVERFLOW_SUBMENU ) ) { + if ( $skinOptions->get( SkinOptions::TOOLBAR_SUBMENU ) ) { $overflowBuilder = $userPageHelper->isUserPage() ? new PageActionsMenu\UserNamespaceOverflowBuilder( $title, diff --git a/includes/SkinOptions.php b/includes/SkinOptions.php index 780736c..bcbb45f 100644 --- a/includes/SkinOptions.php +++ b/includes/SkinOptions.php @@ -23,44 +23,44 @@ 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'; - const OPTION_OVERFLOW_SUBMENU = 'overflowSubmenu'; - const OPTION_TABS_ON_SPECIALS = 'tabsOnSpecials'; - /** @var array skin specific options */ + const MOBILE_OPTIONS = 'mobileOptionsLink'; + const AMC_MODE = 'amc'; + const CATEGORIES = 'categories'; + const BACK_TO_TOP = 'backToTop'; + const PAGE_ISSUES = 'pageIssues'; + const SHARE_BUTTON = 'shareButton'; + const TOGGLING = 'toggling'; + const BETA_MODE = 'beta'; + const TALK_AT_TOP = 'talkAtTop'; + const HISTORY_IN_PAGE_ACTIONS = 'historyInPageActions'; + const TOOLBAR_SUBMENU = 'overflowSubmenu'; + const TABS_ON_SPECIALS = 'tabsOnSpecials'; + + /** @var array skin specific options, initialized with default values */ private $skinOptions = [ // Defaults to true for desktop mode. - self::OPTION_AMC => true, - self::OPTIONS_MOBILE_BETA => false, + self::AMC_MODE => true, + self::BETA_MODE => false, /** * Whether the main menu should include a link to * Special:Preferences of Special:MobileOptions */ - self::OPTION_MOBILE_OPTIONS => false, + self::MOBILE_OPTIONS => false, /** Whether a categories button should appear at the bottom of the skin. */ - self::OPTION_CATEGORIES => false, + self::CATEGORIES => false, /** Whether a back to top button appears at the bottom of the view page */ - self::OPTION_BACK_TO_TOP => false, + self::BACK_TO_TOP => false, /** Whether a share button should appear in icons section */ - self::OPTION_SHARE_BUTTON => false, + self::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, - self::OPTION_OVERFLOW_SUBMENU => false, + self::TOGGLING => false, + self::PAGE_ISSUES => false, + self::TALK_AT_TOP => false, + self::HISTORY_IN_PAGE_ACTIONS => false, + self::TOOLBAR_SUBMENU => false, /** Whether to show tabs on special pages */ - self::OPTION_TABS_ON_SPECIALS => false, + self::TABS_ON_SPECIALS => false, ]; /** @@ -77,7 +77,7 @@ final class SkinOptions { } /** - * Return whether a skin option is truthy. Should be one of self:OPTION_* flags + * Return whether a skin option is truthy. Should be one of self:* constants * @param string $key * @return bool */ diff --git a/includes/menu/PageActions/ToolbarBuilder.php b/includes/menu/PageActions/ToolbarBuilder.php index 55d5206..8123bd3 100644 --- a/includes/menu/PageActions/ToolbarBuilder.php +++ b/includes/menu/PageActions/ToolbarBuilder.php @@ -115,7 +115,7 @@ class ToolbarBuilder { public function getGroup(): Group { $group = new Group(); $permissions = $this->permissions; - $userPageWithOveflowMode = $this->skinOptions->get( SkinOptions::OPTION_OVERFLOW_SUBMENU ) && + $userPageWithOveflowMode = $this->skinOptions->get( SkinOptions::TOOLBAR_SUBMENU ) && $this->userPageHelper->isUserPage(); if ( !$userPageWithOveflowMode && $permissions->isAllowed( diff --git a/includes/permissions/MinervaPagePermissions.php b/includes/permissions/MinervaPagePermissions.php index a279d51..b5bbced 100644 --- a/includes/permissions/MinervaPagePermissions.php +++ b/includes/permissions/MinervaPagePermissions.php @@ -123,11 +123,11 @@ final class MinervaPagePermissions implements IMinervaPagePermissions { } if ( $action === self::HISTORY && $this->title->exists() ) { - return $this->skinOptions->get( SkinOptions::OPTIONS_HISTORY_PAGE_ACTIONS ); + return $this->skinOptions->get( SkinOptions::HISTORY_IN_PAGE_ACTIONS ); } - if ( $action === SkinOptions::OPTION_OVERFLOW_SUBMENU ) { - return $this->skinOptions->get( SkinOptions::OPTION_OVERFLOW_SUBMENU ); + if ( $action === SkinOptions::TOOLBAR_SUBMENU ) { + return $this->skinOptions->get( SkinOptions::TOOLBAR_SUBMENU ); } if ( !in_array( $action, $this->config->get( 'MinervaPageActions' ) ) ) { diff --git a/includes/skins/MinervaTemplate.php b/includes/skins/MinervaTemplate.php index af6feaa..d1f0f44 100644 --- a/includes/skins/MinervaTemplate.php +++ b/includes/skins/MinervaTemplate.php @@ -274,9 +274,9 @@ class MinervaTemplate extends BaseTemplate { 'secondaryactionshtml' => $this->getSecondaryActionsHtml(), 'dataAfterContent' => $this->get( 'dataAfterContent' ), 'footer' => $this->getFooterTemplateData( $data ), - 'isBeta' => $skinOptions->get( SkinOptions::OPTIONS_MOBILE_BETA ), + 'isBeta' => $skinOptions->get( SkinOptions::BETA_MODE ), 'tabs' => $this->showTalkTabs( $hasPageActions, $skinOptions ) && - $skinOptions->get( SkinOptions::OPTIONS_TALK_AT_TOP ) ? [ + $skinOptions->get( SkinOptions::TALK_AT_TOP ) ? [ 'items' => array_values( $data['content_navigation']['namespaces'] ), ] : false, ]; @@ -306,7 +306,7 @@ class MinervaTemplate extends BaseTemplate { private function showTalkTabs( $hasPageActions, SkinOptions $skinOptions ) { $hasTalkTabs = $hasPageActions && !$this->isMainPageTalk; if ( !$hasTalkTabs && $this->isSpecialPage && - $skinOptions->get( SkinOptions::OPTION_TABS_ON_SPECIALS ) ) { + $skinOptions->get( SkinOptions::TABS_ON_SPECIALS ) ) { $hasTalkTabs = true; } return $hasTalkTabs; diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 037a4ba..b68c7b6 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -195,7 +195,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->skinOptions->get( SkinOptions::OPTION_AMC ) ) { + if ( $title->isTalkPage() && !$this->skinOptions->get( SkinOptions::AMC_MODE ) ) { // if it's a talk page for which we have a special message, use it switch ( $title->getNamespace() ) { case NS_USER_TALK: @@ -252,7 +252,7 @@ class SkinMinerva extends SkinTemplate { */ public function getPageClasses( $title ) { $className = parent::getPageClasses( $title ); - $className .= ' ' . ( $this->skinOptions->get( SkinOptions::OPTIONS_MOBILE_BETA ) + $className .= ' ' . ( $this->skinOptions->get( SkinOptions::BETA_MODE ) ? 'beta' : 'stable' ); if ( $title->isMainPage() ) { @@ -265,7 +265,7 @@ class SkinMinerva extends SkinTemplate { // The new treatment should only apply to the main namespace if ( $title->getNamespace() === NS_MAIN && - $this->skinOptions->get( SkinOptions::OPTION_PAGE_ISSUES ) + $this->skinOptions->get( SkinOptions::PAGE_ISSUES ) ) { $className .= ' issues-group-B'; } @@ -277,7 +277,7 @@ class SkinMinerva extends SkinTemplate { * @return bool */ private function hasCategoryLinks() { - if ( !$this->skinOptions->get( SkinOptions::OPTION_CATEGORIES ) ) { + if ( !$this->skinOptions->get( SkinOptions::CATEGORIES ) ) { return false; } $categoryLinks = $this->getOutput()->getCategoryLinks(); @@ -581,7 +581,7 @@ class SkinMinerva extends SkinTemplate { if ( $this->getUserPageHelper()->isUserPage() && // when overflow menu is visible, we don't need to build secondary options // as most of options are visible on Toolbar/Overflow menu - !$this->skinOptions->get( SkinOptions::OPTION_OVERFLOW_SUBMENU ) ) { + !$this->skinOptions->get( SkinOptions::TOOLBAR_SUBMENU ) ) { $pageUser = $this->getUserPageHelper()->getPageUser(); $talkPage = $pageUser->getTalkPage(); $data = [ @@ -731,7 +731,7 @@ class SkinMinerva extends SkinTemplate { // in stable it will link to the wikitext talk page $title = $this->getTitle(); $subjectPage = $title->getSubjectPage(); - $talkAtBottom = !$this->skinOptions->get( SkinOptions::OPTIONS_TALK_AT_TOP ) || + $talkAtBottom = !$this->skinOptions->get( SkinOptions::TALK_AT_TOP ) || $subjectPage->isMainPage(); $namespaces = $tpl->data['content_navigation']['namespaces']; if ( !$this->getUserPageHelper()->isUserPage() @@ -881,7 +881,7 @@ class SkinMinerva extends SkinTemplate { ] ); - if ( $this->skinOptions->get( SkinOptions::OPTION_TOGGLING ) ) { + if ( $this->skinOptions->get( SkinOptions::TOGGLING ) ) { // Extension can unload "toggling" modules via the hook $modules['toggling'] = [ 'skins.minerva.toggling' ]; } @@ -902,7 +902,7 @@ class SkinMinerva extends SkinTemplate { */ public function addToBodyAttributes( $out, &$bodyAttrs ) { $classes = $out->getProperty( 'bodyClassName' ); - if ( $this->skinOptions->get( SkinOptions::OPTION_AMC ) ) { + if ( $this->skinOptions->get( SkinOptions::AMC_MODE ) ) { $classes .= ' minerva--amc-enabled'; } else { $classes .= ' minerva--amc-disabled'; @@ -939,11 +939,11 @@ class SkinMinerva extends SkinTemplate { } $keys = [ - SkinOptions::OPTION_AMC, - SkinOptions::OPTIONS_TALK_AT_TOP, - SkinOptions::OPTIONS_HISTORY_PAGE_ACTIONS, - SkinOptions::OPTION_OVERFLOW_SUBMENU, - SkinOptions::OPTION_TABS_ON_SPECIALS + SkinOptions::AMC_MODE, + SkinOptions::TALK_AT_TOP, + SkinOptions::HISTORY_IN_PAGE_ACTIONS, + SkinOptions::TOOLBAR_SUBMENU, + SkinOptions::TABS_ON_SPECIALS ]; $includeAMCStyles = array_reduce( $keys, function ( $val, $key ) { return $val || $this->skinOptions->get( $key ); @@ -952,7 +952,7 @@ class SkinMinerva extends SkinTemplate { $styles[] = 'skins.minerva.amc.styles'; $styles[] = 'wikimedia.ui'; } - if ( $this->skinOptions->get( SkinOptions::OPTION_AMC ) ) { + if ( $this->skinOptions->get( SkinOptions::AMC_MODE ) ) { // ToolbarBuilder is reusing the Contributions icon in toolbar @see T224735 $styles[] = 'skins.minerva.mainMenu.icons'; } diff --git a/tests/phpunit/skins/SkinMinervaTest.php b/tests/phpunit/skins/SkinMinervaTest.php index 1f29955..19f4d6c 100644 --- a/tests/phpunit/skins/SkinMinervaTest.php +++ b/tests/phpunit/skins/SkinMinervaTest.php @@ -60,7 +60,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $outputPage->expects( $this->never() ) ->method( 'getCategoryLinks' ); - $this->overrideSkinOptions( [ SkinOptions::OPTION_CATEGORIES => false ] ); + $this->overrideSkinOptions( [ SkinOptions::CATEGORIES => false ] ); $context = new RequestContext(); $context->setTitle( Title::newFromText( 'Test' ) ); $context->setOutput( $outputPage ); @@ -87,7 +87,7 @@ class SkinMinervaTest extends MediaWikiTestCase { ->method( 'getCategoryLinks' ) ->will( $this->returnValue( $categoryLinks ) ); - $this->overrideSkinOptions( [ SkinOptions::OPTION_CATEGORIES => true ] ); + $this->overrideSkinOptions( [ SkinOptions::CATEGORIES => true ] ); $context = new RequestContext(); $context->setTitle( Title::newFromText( 'Test' ) ); @@ -143,7 +143,7 @@ class SkinMinervaTest extends MediaWikiTestCase { */ public function testGetContextSpecificModules( $backToTopValue, $moduleName, $expected ) { $this->overrideSkinOptions( [ - SkinOptions::OPTION_AMC => false, + SkinOptions::AMC_MODE => false, 'backToTop' => $backToTopValue, ] ); diff --git a/tests/phpunit/unit/SkinOptionsTest.php b/tests/phpunit/unit/SkinOptionsTest.php index c29cd84..50d00f3 100644 --- a/tests/phpunit/unit/SkinOptionsTest.php +++ b/tests/phpunit/unit/SkinOptionsTest.php @@ -19,14 +19,14 @@ class SkinOptionsTest extends \MediaWikiUnitTestCase { */ public function testSettersAndGetters() { $options = new SkinOptions(); - $defaultValue = $options->get( SkinOptions::OPTION_AMC ); - $options->setMultiple( [ SkinOptions::OPTION_AMC => !$defaultValue ] ); + $defaultValue = $options->get( SkinOptions::AMC_MODE ); + $options->setMultiple( [ SkinOptions::AMC_MODE => !$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 ] ); + $this->assertEquals( !$defaultValue, $options->get( SkinOptions::AMC_MODE ) ); + $this->assertArrayHasKey( SkinOptions::AMC_MODE, $allOptions ); + $this->assertEquals( !$defaultValue, $allOptions[ SkinOptions::AMC_MODE ] ); } /** @@ -34,15 +34,15 @@ class SkinOptionsTest extends \MediaWikiUnitTestCase { */ public function testHasSkinOptions() { $options = new SkinOptions(); - // set OPTION_AMC to true just in case someone decides to set everything to false + // set AMC_MODE to true just in case someone decides to set everything to false // sometime in the future. - $options->setMultiple( [ SkinOptions::OPTION_AMC => true ] ); + $options->setMultiple( [ SkinOptions::AMC_MODE => true ] ); $this->assertTrue( $options->hasSkinOptions() ); - $options->setMultiple( [ SkinOptions::OPTION_BACK_TO_TOP => true ] ); + $options->setMultiple( [ SkinOptions::BACK_TO_TOP => true ] ); $this->assertTrue( $options->hasSkinOptions() ); $options->setMultiple( [ - SkinOptions::OPTION_AMC => false, - SkinOptions::OPTION_BACK_TO_TOP => false + SkinOptions::AMC_MODE => false, + SkinOptions::BACK_TO_TOP => false ] ); $this->assertFalse( $options->hasSkinOptions() ); }