jenkins-bot has submitted this change and it was merged.
Change subject: Promote "switch-language" page action to stable
......................................................................
Promote "switch-language" page action to stable
Changes:
* Extract SkinMinerva#createSwitchLanguagePageAction from
SkinMinervaBeta#preparePageActions and remove
SkinMinervaBeta#preparePageActions
* Feature flag the new page action using $wgMinervaUsePageActionBarV2,
which is disabled by default
* Update the default value and documentation of $wgMinervaPageActions
* Merge language and language_beta browser tests, updating their
terminology, i.e. "alternative language button" -> "switch-language
page action"
Bug: T140260
Change-Id: Ie11a32bb0a3203a105b9951c69aba3213d219335
---
M README.md
M extension.json
M includes/skins/SkinMinerva.php
M includes/skins/SkinMinervaBeta.php
R resources/skins.minerva.icons.images/languageSwitcher.svg
M tests/browser/LocalSettings.php
M tests/browser/features/language.feature
D tests/browser/features/language_beta.feature
M tests/browser/features/step_definitions/language_icon_steps.rb
M tests/browser/features/support/pages/article_page.rb
M tests/phpunit/skins/SkinMinervaPageActionsTest.php
11 files changed, 146 insertions(+), 125 deletions(-)
Approvals:
Jhernandez: Looks good to me, approved
jenkins-bot: Verified
diff --git a/README.md b/README.md
index 513cb7b..497d82f 100644
--- a/README.md
+++ b/README.md
@@ -352,12 +352,18 @@
```
-#### $wgMFPageActions
+#### $wgMFPageActions (deprecated)
-Controls, which page action show and which not. Allowed: `edit`, `watch`
+See `$wgMinervaPageActions`.
+
+#### $wgMinervaPageActions
+
+
+Controls which page actions, if any, are displayed. Allowed: `edit`, `watch`,
`talk`, and
+`switch-language`.
* Type: `Array`
-* Default: `['edit', 'watch']`
+* Default: `['edit', 'talk', 'watch', 'switch-language']`
#### $wgMFQueryPropModules
@@ -604,14 +610,6 @@
* Type: `Boolean`
* Default: `true`
-
-#### $wgMinervaPageActions
-
-Controls, which page action show and which not. Allowed: `edit`, `talk`,
-`upload`, `watch`
-
-* Type: `Array`
-* Default: `['edit', 'talk', 'upload', 'watch']`
#### $wgMFNamespacesWithoutCollapsibleSections
diff --git a/extension.json b/extension.json
index 4bc0150..bc16987 100644
--- a/extension.json
+++ b/extension.json
@@ -193,15 +193,8 @@
"mainmenu":
"resources/skins.minerva.icons.images/hamburger.svg",
"edit":
"resources/skins.minerva.icons.images/editLocked.svg",
"edit-enabled":
"resources/skins.minerva.icons.images/edit.svg",
- "magnifying-glass-white":
"resources/skins.minerva.icons.images/magnifying-glass-white.svg"
- }
- },
- "skins.minerva.icons.beta.images": {
- "class": "ResourceLoaderImageModule",
- "prefix": "mw-ui",
- "selector": ".mw-ui-icon-{name}:before",
- "images": {
- "language-switcher":
"resources/skins.minerva.icons.beta.images/languageSwitcher.svg"
+ "magnifying-glass-white":
"resources/skins.minerva.icons.images/magnifying-glass-white.svg",
+ "language-switcher":
"resources/skins.minerva.icons.images/languageSwitcher.svg"
}
},
"mobile.overlay.images": {
@@ -2014,7 +2007,8 @@
"MinervaPageActions": [
"edit",
"talk",
- "watch"
+ "watch",
+ "switch-language"
],
"MFNamespacesWithoutCollapsibleSections": [
6,
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index a15d99b..4b210b8 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -133,17 +133,22 @@
* <li>the user is on the main page</li>
* </ul>
*
- * Furthermore, the "edit" page action isn't allowed if the content of
the page doesn't support
- * direct editing via the API.
+ * The "edit" page action is allowed if the content of the page
supports direct editing via the
+ * API.
+ *
+ * The "switch-language" is allowed if
<code>$wgMinervaUsePageActionBarV2</code> is truthy and
+ * there are interlanguage links on the page, or
<code>$wgMinervaAlwaysShowLanguageButton</code>
+ * is truthy.
*
* @param string $action
* @return boolean
*/
protected function isAllowedPageAction( $action ) {
$title = $this->getTitle();
+ $config = $this->getMFConfig();
if (
- ! in_array( $action, $this->getMFConfig()->get(
'MinervaPageActions' ) )
+ ! in_array( $action, $config->get( 'MinervaPageActions'
) )
|| $title->isMainPage()
|| ( $this->isUserPage && !$title->exists() )
) {
@@ -155,6 +160,11 @@
return $contentHandler->supportsDirectEditing() &&
$contentHandler->supportsDirectApiEditing();
+ }
+
+ if ( $action === 'switch-language' ) {
+ return $config->get( 'MinervaUsePageActionBarV2' ) &&
+ ( $this->doesPageHaveLanguages || $config->get(
'MinervaAlwaysShowLanguageButton' ) );
}
return true;
@@ -895,6 +905,10 @@
$menu['watch'] = $this->createWatchPageAction( $actions
);
}
+ if ( $this->isAllowedPageAction( 'switch-language' ) ) {
+ $menu['switch-language'] =
$this->createSwitchLanguageAction();
+ }
+
$tpl->set( 'page_actions', $menu );
}
@@ -902,7 +916,7 @@
* Creates the "edit" page action: the well-known pencil icon that,
when tapped, will open an
* editor with the lead section loaded.
*
- * @return array A map compatible with BaseTemplat#makeListItem
+ * @return array A map compatible with BaseTemplate#makeListItem
*/
protected function createEditPageAction() {
$noJsEdit = MobileContext::singleton()->getMFConfig()->get(
'MFAllowNonJavaScriptEditing' );
@@ -927,7 +941,7 @@
* add the page to or remove the page from the user's watchlist; or, if
the user is logged out,
* will direct the user's UA to Special:Login.
*
- * @return array A map compatible with BaseTemplat#makeListItem
+ * @return array A map compatible with BaseTemplate#makeListItem
*/
protected function createWatchPageAction( $actions ) {
$baseResult = [
@@ -956,6 +970,33 @@
}
/**
+ * Creates the the "switch-language" action: the icon that, when
tapped, opens the language
+ * switcher.
+ *
+ * @return array A map compatible with BaseTemplate#makeListItem
+ */
+ protected function createSwitchLanguageAction() {
+ $languageSwitcherLinks = [];
+ $languageSwitcherClasses = 'language-selector';
+
+ if ( $this->doesPageHaveLanguages ) {
+
$languageSwitcherLinks['mobile-frontend-language-article-heading'] = [
+ 'href' => SpecialPage::getTitleFor(
'MobileLanguages', $this->getTitle() )->getLocalURL()
+ ];
+ } else {
+ $languageSwitcherClasses .= ' disabled';
+ }
+
+ return [
+ 'text' => '',
+ 'itemtitle' => $this->msg(
'mobile-frontend-language-article-heading' ),
+ 'class' => MobileUI::iconClass( 'language-switcher',
'element', $languageSwitcherClasses ),
+ 'links' => $languageSwitcherLinks,
+ 'is_js_only' => false
+ ];
+ }
+
+ /**
* Checks to see if the current page is (probably) editable.
*
* This is the same check that sets wgIsProbablyEditable later in the
page output
diff --git a/includes/skins/SkinMinervaBeta.php
b/includes/skins/SkinMinervaBeta.php
index f1bcde4..812ea1d 100644
--- a/includes/skins/SkinMinervaBeta.php
+++ b/includes/skins/SkinMinervaBeta.php
@@ -15,46 +15,12 @@
protected $shouldSecondaryActionsIncludeLanguageBtn = true;
/**
- * Do not set page actions on the user page that hasn't been created
yet.
- * Also add the language switcher action.
+ * The "switch-language" is always allowed in MFBeta.
*
* @inheritdoc
- * @param BaseTemplate $tpl
*/
- protected function preparePageActions( BaseTemplate $tpl ) {
- $setPageActions = true;
-
- if ( $this->isUserPage ) {
- if ( !$this->getTitle()->exists() ) {
- $setPageActions = false;
- }
- }
- if ( $setPageActions ) {
- parent::preparePageActions( $tpl );
- $menu = $tpl->data[ 'page_actions' ];
-
- $languageSwitcherLinks = [];
- $languageSwitcherClasses = 'disabled';
- if ( $this->doesPageHaveLanguages ) {
-
$languageSwitcherLinks['mobile-frontend-language-article-heading'] = [
- 'href' => SpecialPage::getTitleFor(
'MobileLanguages', $this->getTitle() )->getLocalURL()
- ];
- $languageSwitcherClasses = '';
- }
- $languageSwitcherClasses .= ' language-selector';
- if ( $this->getMFConfig()->get(
'MinervaAlwaysShowLanguageButton' ) ||
- $this->doesPageHaveLanguages ) {
- $menu['language-switcher'] = [ 'text' => '',
- 'itemtitle' => $this->msg(
'mobile-frontend-language-article-heading' ),
- 'class' => MobileUI::iconClass(
'language-switcher', 'element', $languageSwitcherClasses ),
- 'links' => $languageSwitcherLinks,
- 'is_js_only' => false
- ];
- $tpl->set( 'page_actions', $menu );
- }
- } else {
- $tpl->set( 'page_actions', [] );
- }
+ protected function isAllowedPageAction( $action ) {
+ return $action === 'switch-language' ? true :
parent::isAllowedPageAction( $action );
}
/**
@@ -128,7 +94,6 @@
$styles[] = 'skins.minerva.mainPage.beta.styles';
}
$styles[] = 'skins.minerva.content.styles.beta';
- $styles[] = 'skins.minerva.icons.beta.images';
return $styles;
}
diff --git a/resources/skins.minerva.icons.beta.images/languageSwitcher.svg
b/resources/skins.minerva.icons.images/languageSwitcher.svg
similarity index 100%
rename from resources/skins.minerva.icons.beta.images/languageSwitcher.svg
rename to resources/skins.minerva.icons.images/languageSwitcher.svg
diff --git a/tests/browser/LocalSettings.php b/tests/browser/LocalSettings.php
index 26f9757..e18d7b4 100644
--- a/tests/browser/LocalSettings.php
+++ b/tests/browser/LocalSettings.php
@@ -24,6 +24,7 @@
// needed for testing whether the language button is displayed and disabled
$wgMinervaAlwaysShowLanguageButton = true;
+$wgMinervaUsePageActionBarV2 = true;
// For those who have wikibase installed.
$wgMFUseWikibase = true;
diff --git a/tests/browser/features/language.feature
b/tests/browser/features/language.feature
index 20efc18..9a548a9 100644
--- a/tests/browser/features/language.feature
+++ b/tests/browser/features/language.feature
@@ -3,26 +3,58 @@
Background:
Given I am using the mobile site
- And I go to a page that has languages
- When I click the language button
- And I see the language overlay
@smoke @integration
+ Scenario: Language button in beta
+ Given I go to a page that has languages
+ Then I should see the switch-language page action
+
+ @smoke @integration
+ Scenario: Language button in beta (on a page that doesn't have languages)
+ Given I go to a page that does not have languages
+ Then I should see the disabled switch-language page action
+
+ Scenario: Tapping icon opens language overlay
+ Given I go to a page that has languages
+ When I click the switch-language page action
+ Then I should see the language overlay
+
+ Scenario: Tapping icon does not open language overlay (on a page that
doesn't have languages)
+ Given I go to a page that does not have languages
+ When I click the switch-language page action
+ Then I should not see the languages overlay
+
+ Scenario: Tapping the disabled icon shows a toast
+ Given I go to a page that does not have languages
+ When I click the switch-language page action
+ Then I should see a toast with message about page not being available in
other languages
+
Scenario: Closing language overlay (overlay button)
- When I click the language overlay close button
+ Given I go to a page that has languages
+ When I click the switch-language page action
+ And I see the language overlay
+ And I click the language overlay close button
Then I should not see the languages overlay
Scenario: Closing language overlay (browser button)
- When I click the browser back button
+ Given I go to a page that has languages
+ When I click the switch-language page action
+ And I see the language overlay
+ And I click the browser back button
Then I should not see the languages overlay
Scenario: Checking that there are no suggested language links
+ Given I go to a page that has languages
+ When I click the switch-language page action
+ And I see the language overlay
Then I should not see a suggested language link
Then I should see a non-suggested language link
@smoke
Scenario: Checking that the suggested language link has been created
- When I click on a language from the list of all languages
- And I click the browser back button
- And I see the language overlay
+ Given I go to a page that has languages
+ And I click the switch-language page action
+ And I click on a language from the list of all languages
+ And I click the browser back button
+ And I see the language overlay
Then I should see a suggested language link
diff --git a/tests/browser/features/language_beta.feature
b/tests/browser/features/language_beta.feature
deleted file mode 100644
index fcbaac4..0000000
--- a/tests/browser/features/language_beta.feature
+++ /dev/null
@@ -1,45 +0,0 @@
-@chrome @en.m.wikipedia.beta.wmflabs.org @firefox @test2.m.wikipedia.org
-Feature: Language selection via language switcher button in beta
-
- Background:
- Given I am using the mobile site
- And I am in beta mode
-
- @smoke @integration
- Scenario: Language button in beta
- Given I go to a page that has languages
- Then I should see the alternative language switcher button
-
- @smoke @integration
- Scenario: Language button in beta (on a page that doesn't have languages)
- Given I go to a page that does not have languages
- Then I should see the disabled alternative language switcher button
-
- Scenario: Tapping icon opens language overlay
- Given I go to a page that has languages
- When I click the alternative language button
- Then I should see the language overlay
-
- Scenario: Tapping icon does not open language overlay (on a page that
doesn't have languages)
- Given I go to a page that does not have languages
- When I click the alternative language button
- Then I should not see the languages overlay
-
- Scenario: Tapping the disabled icon shows a toast
- Given I go to a page that does not have languages
- When I click the alternative language button
- Then I should see a toast with message about page not being available in
other languages
-
- Scenario: Closing language overlay (overlay button)
- Given I go to a page that has languages
- When I click the alternative language button
- And I see the language overlay
- And I click the language overlay close button
- Then I should not see the languages overlay
-
- Scenario: Closing language overlay (browser button)
- Given I go to a page that has languages
- When I click the alternative language button
- And I see the language overlay
- And I click the browser back button
- Then I should not see the languages overlay
diff --git a/tests/browser/features/step_definitions/language_icon_steps.rb
b/tests/browser/features/step_definitions/language_icon_steps.rb
index dc246ab..0b0f609 100644
--- a/tests/browser/features/step_definitions/language_icon_steps.rb
+++ b/tests/browser/features/step_definitions/language_icon_steps.rb
@@ -1,11 +1,11 @@
-When /^I click the alternative language button$/ do
- on(ArticlePage).alternative_language_button_element.when_present.click
+When /^I click the switch-language page action$/ do
+ on(ArticlePage).switch_language_page_action_element.when_present.click
end
-Then(/^I should see the disabled alternative language switcher button$/) do
- expect(on(ArticlePage).disabled_alternative_language_button_element).to
be_visible
+Then(/^I should see the disabled switch-language page action$/) do
+ expect(on(ArticlePage).disabled_switch_langage_page_action_element).to
be_visible
end
-Then(/^I should see the alternative language switcher button$/) do
- expect(on(ArticlePage).alternative_language_button_element).to be_visible
+Then(/^I should see the switch-language page action$/) do
+ expect(on(ArticlePage).switch_language_page_action_element).to be_visible
end
diff --git a/tests/browser/features/support/pages/article_page.rb
b/tests/browser/features/support/pages/article_page.rb
index 500fbbd..dbf127d 100644
--- a/tests/browser/features/support/pages/article_page.rb
+++ b/tests/browser/features/support/pages/article_page.rb
@@ -144,8 +144,8 @@
# secondary menu
## languages
a(:language_button, css: '#page-secondary-actions .language-selector')
- a(:alternative_language_button, css: '#page-actions .language-selector')
- a(:disabled_alternative_language_button, css: '#page-actions
.language-selector.disabled')
+ a(:switch_language_page_action, css: '#page-actions .language-selector')
+ a(:disabled_switch_langage_page_action, css: '#page-actions
.language-selector.disabled')
# Can't use generic overlay class as this will match with the LoadingOverlay
that shows before loading the language overlay
div(:overlay_languages, css: '.language-overlay')
a(:non_suggested_language_link, css: '.all-languages a', index: 0)
diff --git a/tests/phpunit/skins/SkinMinervaPageActionsTest.php
b/tests/phpunit/skins/SkinMinervaPageActionsTest.php
index cb8f0fe..7f47501 100644
--- a/tests/phpunit/skins/SkinMinervaPageActionsTest.php
+++ b/tests/phpunit/skins/SkinMinervaPageActionsTest.php
@@ -10,6 +10,10 @@
public function setContentHandler( ContentHandler $contentHandler ) {
$this->contentHandler = $contentHandler;
}
+
+ public function setDoesPageHaveLanguages( $doesPageHaveLanguages ) {
+ $this->doesPageHaveLanguages = $doesPageHaveLanguages;
+ }
}
/**
@@ -119,4 +123,35 @@
$this->assertTrue( $skin->isAllowedPageAction( 'talk' ) );
}
+
+ public static function switchLanguagePageActionProvider() {
+ return [
+ [ true, false, true, true ],
+ [ false, true, true, true ],
+ [ true, false, false, false ],
+ ];
+ }
+
+ /**
+ * The "switch-language" page action is allowed when: v2 of the page
action bar is enabled and
+ * if the page has interlanguage links or if the
<code>$wgMinervaAlwaysShowLanguageButton</code>
+ * configuration variable is set to truthy.
+ *
+ * @dataProvider switchLanguagePageActionProvider
+ * @covers SkinMinerva::isAllowedPageAction
+ */
+ public function test_switch_language_page_action(
+ $doesPageHaveLanguages,
+ $minervaAlwaysShowLanguageButton,
+ $minervaUsePageActionBarV2,
+ $expected
+ ) {
+ $this->skin->setDoesPageHaveLanguages( $doesPageHaveLanguages );
+ $this->setMwGlobals( [
+ 'wgMinervaAlwaysShowLanguageButton' =>
$minervaAlwaysShowLanguageButton,
+ 'wgMinervaUsePageActionBarV2' =>
$minervaUsePageActionBarV2,
+ ] );
+
+ $this->assertEquals( $expected,
$this->skin->isAllowedPageAction( 'switch-language' ) );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/301351
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie11a32bb0a3203a105b9951c69aba3213d219335
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Phuedx <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Jhernandez <[email protected]>
Gerrit-Reviewer: Jhobs <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
