jenkins-bot has submitted this change and it was merged.
Change subject: Beta: add an alternative language switcher button to the top of
the page
......................................................................
Beta: add an alternative language switcher button to the top of the page
Bug: T128350
Change-Id: I4d09c645c02a0d137595c6f6124696913b763c44
---
M extension.json
M includes/skins/MinervaTemplate.php
M includes/skins/MinervaTemplateBeta.php
M includes/skins/SkinMinerva.php
M includes/skins/SkinMinervaBeta.php
M minerva.less/minerva.variables.less
M resources/skins.minerva.base.styles/pageactions.less
M resources/skins.minerva.base.styles/ui.less
A resources/skins.minerva.beta.styles/pageactions.less
A resources/skins.minerva.icons.beta.images/languageSwitcher.svg
M resources/skins.minerva.scripts/init.js
M tests/browser/LocalSettings.php
A tests/browser/features/language_beta.feature
M tests/browser/features/step_definitions/common_steps.rb
A tests/browser/features/step_definitions/language_icon_steps.rb
M tests/browser/features/step_definitions/language_steps.rb
M tests/browser/features/support/pages/article_page.rb
17 files changed, 209 insertions(+), 36 deletions(-)
Approvals:
Jdlrobson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/extension.json b/extension.json
index cfdb469..6c8e485 100644
--- a/extension.json
+++ b/extension.json
@@ -130,6 +130,16 @@
"resources/skins.minerva.base.styles/images.less"
]
},
+ "skins.minerva.beta.styles": {
+ "targets": [
+ "mobile",
+ "desktop"
+ ],
+ "position": "top",
+ "styles": [
+
"resources/skins.minerva.beta.styles/pageactions.less"
+ ]
+ },
"skins.minerva.content.styles": {
"targets": [
"mobile",
@@ -192,6 +202,14 @@
"edit-enabled":
"resources/skins.minerva.icons.images/edit.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"
+ }
+ },
"mobile.overlay.images": {
"selectorWithoutVariant": ".mw-ui-icon-{name}:before",
"selectorWithVariant":
".mw-ui-icon-{name}-{variant}:before",
diff --git a/includes/skins/MinervaTemplate.php
b/includes/skins/MinervaTemplate.php
index d24b78b..0e1e0f8 100644
--- a/includes/skins/MinervaTemplate.php
+++ b/includes/skins/MinervaTemplate.php
@@ -16,6 +16,9 @@
/** @var boolean Specify whether the page is main page */
protected $isMainPage;
+ /** @var boolean Whether to insert the page actions before the heading
in HTML */
+ protected $shouldDisplayPageActionsBeforeHeading = true;
+
/**
* Gets the header content for the top chrome.
* @param array $data Data used to build the page
@@ -169,27 +172,7 @@
return array();
}
- $result = $this->data['secondary_actions'];
- $hasLanguages = $this->data['content_navigation']['variants'] ||
- $this->data['language_urls'];
-
- // If languages are available, add a languages link
- if ( $hasLanguages ) {
- $languageUrl = SpecialPage::getTitleFor(
- 'MobileLanguages',
- $this->getSkin()->getTitle()
- )->getLocalURL();
-
- $result['language'] = array(
- 'attributes' => array(
- 'class' => 'languageSelector',
- 'href' => $languageUrl,
- ),
- 'label' => $this->getMsg(
'mobile-frontend-language-article-heading' )->text()
- );
- }
-
- return $result;
+ return $this->data['secondary_actions'];
}
/**
@@ -255,10 +238,15 @@
if ( $internalBanner || $preBodyHtml || isset(
$data['page_actions'] ) ) {
$html .= $preBodyHtml
. Html::openElement( 'div', array( 'class' =>
'pre-content heading-holder' ) );
+ if (
!$this->shouldDisplayPageActionsBeforeHeading ) {
+ $html .= $headingHtml;
+ }
if ( !$this->isSpecialPage ){
$html .= $this->getPageActionsHtml(
$data );
}
- $html .= $headingHtml;
+ if (
$this->shouldDisplayPageActionsBeforeHeading ) {
+ $html .= $headingHtml;
+ }
$html .= $postHeadingHtml;
$html .= $data['subtitle'];
// FIXME: Temporary solution until we have
design
diff --git a/includes/skins/MinervaTemplateBeta.php
b/includes/skins/MinervaTemplateBeta.php
index e7b1565..47d4cac 100644
--- a/includes/skins/MinervaTemplateBeta.php
+++ b/includes/skins/MinervaTemplateBeta.php
@@ -8,6 +8,9 @@
* beta mode via Special:MobileOptions
*/
class MinervaTemplateBeta extends MinervaTemplate {
+ /** @inheritdoc */
+ protected $shouldDisplayPageActionsBeforeHeading = false;
+
/**
* Get attributes to create search input
* @return array Array with attributes for search bar
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index c1ae499..6ebaa05 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -24,6 +24,10 @@
protected $mobileContext;
/** @var bool whether the page is the user's page, i.e. User:Username */
public $isUserPage = false;
+ /** @var boolean Whether the language button should be included in the
secondary actions HTML */
+ protected $shouldSecondaryActionsIncludeLanguageBtn = true;
+ /** @var bool Whether the page is also available in other languages or
variants */
+ protected $doesPageHaveLanguages = false;
/**
* Wrapper for MobileContext::getMFConfig()
@@ -52,6 +56,9 @@
// Generate skin template
$tpl = parent::prepareQuickTemplate();
+
+ $this->doesPageHaveLanguages =
$tpl->data['content_navigation']['variants'] ||
+ $tpl->data['language_urls'];
// Set whether or not the page content should be wrapped in
div.content (for
// example, on a special page)
@@ -735,6 +742,27 @@
}
/**
+ * Returns an array with details for a language button.
+ * @return array
+ */
+ protected function getLanguageButton() {
+ $languageUrl = SpecialPage::getTitleFor(
+ 'MobileLanguages',
+ $this->getSkin()->getTitle()
+ )->getLocalURL();
+
+ return array(
+ 'attributes' => array(
+ 'id' => 'language-switcher',
+ // FIXME: remove class when cache clears
+ 'class' => 'languageSelector',
+ 'href' => $languageUrl,
+ ),
+ 'label' => $this->msg(
'mobile-frontend-language-article-heading' )->text()
+ );
+ }
+
+ /**
* Returns an array with details for a talk button.
* @param Title $talkTitle Title object of the talk page
* @param array $talkButton Array with data of desktop talk button
@@ -775,6 +803,11 @@
$talkTitle = $title->getTalkPage();
$buttons['talk'] = $this->getTalkButton( $talkTitle,
$talkButton );
}
+
+ if ( $this->shouldSecondaryActionsIncludeLanguageBtn &&
$this->doesPageHaveLanguages ) {
+ $buttons['language'] = $this->getLanguageButton();
+ }
+
return $buttons;
}
diff --git a/includes/skins/SkinMinervaBeta.php
b/includes/skins/SkinMinervaBeta.php
index 57fd66f..845482a 100644
--- a/includes/skins/SkinMinervaBeta.php
+++ b/includes/skins/SkinMinervaBeta.php
@@ -11,6 +11,8 @@
public $template = 'MinervaTemplateBeta';
/** @var string $mode Describes 'stability' of the skin - beta, stable
*/
protected $mode = 'beta';
+ /** @inheritdoc */
+ protected $shouldSecondaryActionsIncludeLanguageBtn = false;
/** @inheritdoc **/
protected function getHeaderHtml() {
@@ -30,6 +32,7 @@
/**
* Do not set page actions on the user page that hasn't been created
yet.
+ * Also add the language switcher action.
*
* @inheritdoc
* @param BaseTemplate $tpl
@@ -44,16 +47,31 @@
}
if ( $setPageActions ) {
parent::preparePageActions( $tpl );
+ $menu = $tpl->data[ 'page_actions' ];
+
+ $languageSwitcherLinks = array();
+ $languageSwitcherClasses = 'disabled';
+ if ( $this->doesPageHaveLanguages ) {
+
$languageSwitcherLinks['mobile-frontend-language-article-heading'] = array(
+ 'href' => SpecialPage::getTitleFor(
'MobileLanguages', $this->getTitle() )->getLocalURL()
+ );
+ $languageSwitcherClasses = '';
+ }
+ $menu['language-switcher'] = array( 'id' =>
'language-switcher', 'text' => '',
+ 'itemtitle' => $this->msg(
'mobile-frontend-language-article-heading' ),
+ 'class' => MobileUI::iconClass(
'language-switcher', 'element', $languageSwitcherClasses ),
+ 'links' => $languageSwitcherLinks
+ );
+ $tpl->set( 'page_actions', $menu );
} else {
$tpl->set( 'page_actions', array() );
}
}
/**
- * Do not return secondary actions on the user page
+ * Do not return secondary actions on the user page.
*
- * @param BaseTemplate $tpl
- * @return string[]
+ * @inheritdoc
*/
protected function getSecondaryActions( BaseTemplate $tpl ) {
if ( $this->isUserPage ) {
@@ -128,7 +146,9 @@
if ( $title->isMainPage() ) {
$styles[] = 'skins.minerva.mainPage.beta.styles';
}
+ $styles[] = 'skins.minerva.beta.styles';
$styles[] = 'skins.minerva.content.styles.beta';
+ $styles[] = 'skins.minerva.icons.beta.images';
return $styles;
}
diff --git a/minerva.less/minerva.variables.less
b/minerva.less/minerva.variables.less
index e50d353..12a8de9 100644
--- a/minerva.less/minerva.variables.less
+++ b/minerva.less/minerva.variables.less
@@ -28,6 +28,9 @@
@headerTitleFontSize: 1em;
@headerHeight: 3.35em;
+@titleSectionSpacingTop: 20px;
+@titleSectionSpacingBottom: 25px;
+
@grayDark: #252525;
@grayMediumDark: @colorGray5;
@grayMedium: @colorGray7;
diff --git a/resources/skins.minerva.base.styles/pageactions.less
b/resources/skins.minerva.base.styles/pageactions.less
index 0b53e89..eb72435 100644
--- a/resources/skins.minerva.base.styles/pageactions.less
+++ b/resources/skins.minerva.base.styles/pageactions.less
@@ -3,9 +3,6 @@
@borderBottomColor: #CACACA;
-@titleSectionSpacingTop: 20px;
-@titleSectionSpacingBottom: 25px;
-
// hide menu items when not possible to use
.client-nojs #ca-watch,
.client-nojs #ca-edit,
diff --git a/resources/skins.minerva.base.styles/ui.less
b/resources/skins.minerva.base.styles/ui.less
index dc07f17..96e10db 100644
--- a/resources/skins.minerva.base.styles/ui.less
+++ b/resources/skins.minerva.base.styles/ui.less
@@ -199,6 +199,8 @@
.stable {
// Remove when/if page-secondary-actions are promoted to stable
+ #language-switcher,
+ // FIXME: remove .languageSelector when cache clears
.languageSelector {
margin-top: 1em;
}
diff --git a/resources/skins.minerva.beta.styles/pageactions.less
b/resources/skins.minerva.beta.styles/pageactions.less
new file mode 100644
index 0000000..8d6b840
--- /dev/null
+++ b/resources/skins.minerva.beta.styles/pageactions.less
@@ -0,0 +1,48 @@
+@import "minerva.variables";
+
+.heading-holder {
+ overflow: hidden;
+}
+
+#page-actions {
+ float: none;
+ border-top: 1px solid @colorGray14;
+ border-bottom: 1px solid @colorGray12;
+ // align the left and right icons with the left and right borders
respectively
+ margin: 1em -@iconGutterWidth;
+ padding: 0.5em 0;
+
+ li {
+ display: inline-block;
+ margin-bottom: 0;
+ float: right;
+
+ &:first-child {
+ margin-top: 0;
+ }
+ }
+
+ #language-switcher {
+ float: left;
+
+ &.disabled {
+ cursor: default;
+ opacity: 0.25;
+ }
+ }
+}
+
+@media all and (min-width: @deviceWidthTablet) {
+ .heading-holder {
+ position: relative;
+ }
+
+ #page-actions {
+ border: none;
+ bottom: @titleSectionSpacingBottom;
+ // align the left and right icons with the left and right
borders respectively
+ margin: 0 -@iconGutterWidth;
+ position: absolute;
+ right: 0;
+ }
+}
diff --git a/resources/skins.minerva.icons.beta.images/languageSwitcher.svg
b/resources/skins.minerva.icons.beta.images/languageSwitcher.svg
new file mode 100644
index 0000000..e4a8953
--- /dev/null
+++ b/resources/skins.minerva.icons.beta.images/languageSwitcher.svg
@@ -0,0 +1,12 @@
+<svg xmlns="http://www.w3.org/2000/svg"; width="28" height="21" viewBox="0 0 28
21">
+ <title>
+ uniE021 - translation
+ </title>
+ <g id="Page-1" fill="none" fill-rule="evenodd">
+ <g id="uniE021---translation" fill="#777">
+ <path d="M23.89 16l1.72 5h2.37L22.44 3.94h-3.3L13.34
21h2.37l1.59-5h6.59zm-3.3-10.09l2.77 8.13h-5.54l2.77-8.13z" id="Shape"/>
+ <path d="M8.2 2.63h1.18L8.07 0H5.43l.66 1.31A2.34 2.34 0 0 0 8.2
2.63z" id="Shape"/>
+ <path d="M16.51 4H.15v2h2.51c.79 2.27 1.98 4.24 3.69 6.08-1.85
1.44-4.09 2.23-6.33 3.01l.66 1.97c2.64-.79 5.01-1.83 7.25-3.54 1.19.92 2.77
1.84 4.62 2.36l.66-1.97c-1.45-.39-2.64-1.05-3.69-1.83 2.5-2.5 3.29-4.99
3.42-5.25l.27-.83h2.64l.66-2zm-8.58 6.67C6.61 9.41 5.43 7.77 4.9 6h6.2c-.66
1.77-1.85 3.28-3.17 4.67-2.11-2.15 0 0 0 0z" id="Shape"/>
+ </g>
+ </g>
+</svg>
diff --git a/resources/skins.minerva.scripts/init.js
b/resources/skins.minerva.scripts/init.js
index 844ed43..678b898 100644
--- a/resources/skins.minerva.scripts/init.js
+++ b/resources/skins.minerva.scripts/init.js
@@ -51,14 +51,16 @@
* @ignore
*/
function initButton() {
- var $languageSelector = $( '#page-secondary-actions' ).find(
'.languageSelector' );
+ // FIXME: remove .languageSelector when cache clears
+ var $languageSwitcherBtn = $( '#language-switcher,
.languageSelector' ),
+ languageButtonVersion = context.isBetaGroupMember() ?
'top-of-article' : 'bottom-of-article';
/**
* Log impression when the language button is seen by the user
* @ignore
*/
function logLanguageButtonImpression() {
- if ( mw.viewport.isElementInViewport(
$languageSelector[0] ) ) {
+ if ( mw.viewport.isElementInViewport(
$languageSwitcherBtn[0] ) ) {
M.off( 'scroll', logLanguageButtonImpression );
schemaMobileWebLanguageSwitcher.log( {
@@ -90,7 +92,7 @@
return bucket;
}
- if ( $languageSelector.length ) {
+ if ( $languageSwitcherBtn.length ) {
schemaMobileWebLanguageSwitcher.log( {
event: 'pageLoaded',
beaconCapable: $.isFunction(
navigator.sendBeacon )
@@ -100,13 +102,19 @@
// maybe the button is already visible?
logLanguageButtonImpression();
- $languageSelector.on( 'click', function ( ev ) {
+ $languageSwitcherBtn.on( 'click', function ( ev ) {
var previousTapCount = settings.get(
'mobile-language-button-tap-count' ),
+ $languageLink =
context.isBetaGroupMember() ? $languageSwitcherBtn.find( 'a' ) :
$languageSwitcherBtn,
tapCountBucket;
ev.preventDefault();
- router.navigate( '/languages' );
+ // In beta the icon is still shown even though
there are no languages to show.
+ // Only show the overlay if the page has other
languages.
+ if ( $languageLink.attr( 'href' ) ) {
+ router.navigate( '/languages' );
+ }
+
uiSchema.log( {
name: 'languages'
} );
@@ -127,7 +135,7 @@
settings.save(
'mobile-language-button-tap-count', previousTapCount + 1 );
schemaMobileWebLanguageSwitcher.log( {
event: 'languageButtonTap',
- languageButtonVersion:
'bottom-of-article',
+ languageButtonVersion:
languageButtonVersion,
languageButtonTappedBucket:
tapCountBucket,
primaryLanguageOfUser:
getDeviceLanguage() || 'unknown'
} );
diff --git a/tests/browser/LocalSettings.php b/tests/browser/LocalSettings.php
index 710fe9f..1500529 100644
--- a/tests/browser/LocalSettings.php
+++ b/tests/browser/LocalSettings.php
@@ -4,7 +4,6 @@
// Allow users to edit privacy link.
$wgGroupPermissions['user']['editinterface'] = true;
-
$wgMFIgnoreEventLoggingBucketing = true;
$wgHooks['InterwikiLoadPrefix'][] = function ( $prefix, &$iwdata ) {
if ( $prefix === 'es' ) {
@@ -19,3 +18,5 @@
// nothing to do, continue lookup
return true;
};
+
+$wgMFEnableBeta = true;
diff --git a/tests/browser/features/language_beta.feature
b/tests/browser/features/language_beta.feature
new file mode 100644
index 0000000..96e73ec
--- /dev/null
+++ b/tests/browser/features/language_beta.feature
@@ -0,0 +1,27 @@
+@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
+ And I go to a page that has languages
+
+ @smoke @integration
+ Scenario: Language button in beta
+ Then I should see the alternative language switcher button
+
+ Scenario: Tapping icon opens language overlay
+ When I click the alternative language button
+ Then I should see the language overlay
+
+ Scenario: Closing language overlay (overlay button)
+ 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)
+ 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/common_steps.rb
b/tests/browser/features/step_definitions/common_steps.rb
index c69e771..6710eac 100644
--- a/tests/browser/features/step_definitions/common_steps.rb
+++ b/tests/browser/features/step_definitions/common_steps.rb
@@ -4,6 +4,7 @@
# A domain is explicitly given to avoid a bug in earlier versions of Chrome
domain = page_uri.host == 'localhost' ? nil : page_uri.host
+ # FIXME: remove 'mf_useformat' cookie from here
browser.cookies.add 'mf_useformat', 'true', domain: domain
browser.cookies.add 'optin', 'beta', domain: domain
diff --git a/tests/browser/features/step_definitions/language_icon_steps.rb
b/tests/browser/features/step_definitions/language_icon_steps.rb
new file mode 100644
index 0000000..c6ea3df
--- /dev/null
+++ b/tests/browser/features/step_definitions/language_icon_steps.rb
@@ -0,0 +1,7 @@
+When /^I click the alternative language button$/ do
+ on(ArticlePage).alternative_language_button_element.when_present.click
+end
+
+Then(/^I should see the alternative language switcher button$/) do
+ expect(on(ArticlePage).alternative_language_button_element).to be_visible
+end
diff --git a/tests/browser/features/step_definitions/language_steps.rb
b/tests/browser/features/step_definitions/language_steps.rb
index 12179b9..24be76b 100644
--- a/tests/browser/features/step_definitions/language_steps.rb
+++ b/tests/browser/features/step_definitions/language_steps.rb
@@ -13,3 +13,7 @@
Then(/^I should not see the languages overlay$/) do
expect(on(ArticlePage).overlay_languages_element).not_to be_visible
end
+
+Then(/^I should see the language overlay$/) do
+ expect(on(ArticlePage).overlay_languages_element.when_present).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 e0e59c2..ea66695 100644
--- a/tests/browser/features/support/pages/article_page.rb
+++ b/tests/browser/features/support/pages/article_page.rb
@@ -137,7 +137,8 @@
# secondary menu
## languages
- a(:language_button, css: '.languageSelector')
+ a(:language_button, css: '#page-secondary-actions #language-switcher')
+ a(:alternative_language_button, css: '#page-actions #language-switcher')
# 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')
--
To view, visit https://gerrit.wikimedia.org/r/275881
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4d09c645c02a0d137595c6f6124696913b763c44
Gerrit-PatchSet: 27
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Bmansurov <[email protected]>
Gerrit-Reviewer: Bmansurov <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: Jhobs <[email protected]>
Gerrit-Reviewer: Nirzar <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: VolkerE <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
