Render add discussion button in PHP not JS

The talk page JavaScript progressively enhances an
existing button in the page.

Remove the frontend logic and rely entirely on whether
the button is in the page or not.

Additional change:
* The browser tests incorrectly suggest a user needs
5 edits to be able to use the talk feature. This is not
true. They just need to be logged in. Update that logic.

Bug: T167728
Change-Id: Iacedea30bdd0775b3d785db5b143abafd7a18b39
This commit is contained in:
jdlrobson 2017-09-28 15:06:39 -05:00 committed by Jdlrobson
parent 55f1192323
commit b93b7eda7c
4 changed files with 53 additions and 44 deletions

View File

@ -991,14 +991,24 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin {
* 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
* @param bool $addSection (optional) when added the talk button will render
* as an add topic button. Defaults to false.
* @return array
*/
protected function getTalkButton( $talkTitle, $talkButton ) {
protected function getTalkButton( $talkTitle, $talkButton, $addSection = false ) {
if ( $addSection ) {
$params = [ 'action' => 'edit', 'section' => 'new' ];
$className = 'talk continue add';
} else {
$params = [];
$className = 'talk';
}
return [
'attributes' => [
'href' => $talkTitle->getLinkURL(),
'href' => $talkTitle->getLinkURL( $params ),
'data-title' => $talkTitle->getFullText(),
'class' => 'talk',
'class' => $className,
],
'label' => $talkButton['text'],
];
@ -1037,10 +1047,16 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin {
// FIXME [core]: This seems unnecessary..
$subjectId = $title->getNamespaceKey( '' );
$talkId = $subjectId === 'main' ? 'talk' : "{$subjectId}_talk";
if ( isset( $namespaces[$talkId] ) && !$title->isTalkPage() ) {
if ( isset( $namespaces[$talkId] ) ) {
$talkButton = $namespaces[$talkId];
$talkTitle = $title->getTalkPage();
$buttons['talk'] = $this->getTalkButton( $talkTitle, $talkButton );
if ( $title->isTalkPage() ) {
$talkButton['text'] = wfMessage( 'minerva-talk-add-topic' );
$buttons['talk'] = $this->getTalkButton( $title, $talkButton, true );
} else {
$buttons['talk'] = $this->getTalkButton( $talkTitle, $talkButton );
}
}
}
@ -1265,8 +1281,7 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin {
protected function isTalkAllowed() {
$title = $this->getTitle();
return $this->isAllowedPageAction( 'talk' ) &&
!$title->isTalkPage() &&
$title->canHaveTalkPage() &&
( $title->isTalkPage() || $title->canHaveTalkPage() ) &&
$this->getUser()->isLoggedIn();
}

View File

@ -1,15 +1,13 @@
( function ( M, $ ) {
var loader = M.require( 'mobile.startup/rlModuleLoader' ),
LoadingOverlay = M.require( 'mobile.startup/LoadingOverlay' ),
user = M.require( 'mobile.startup/user' ),
Button = M.require( 'mobile.startup/Button' ),
$talk = $( '.talk' ),
// use the plain return value here - T128273
title = $talk.attr( 'data-title' ),
page = M.getCurrentPage(),
overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ),
skin = M.require( 'skins.minerva.scripts/skin' ),
pageTitle, talkTitle;
inTalkNamespace = false,
pageTitle, talkTitle, talkNs, pageNs;
// if there's no title for any reason, don't do anything
if ( !title ) {
@ -25,12 +23,15 @@
// The method to get associated namespaces will change later (maybe), see T487
pageTitle = mw.Title.newFromText( mw.config.get( 'wgPageName' ) );
talkTitle = mw.Title.newFromText( title );
if (
!pageTitle ||
!talkTitle ||
( pageTitle.getMainText() !== talkTitle.getMainText() ) ||
( pageTitle.getNamespaceId() + 1 !== talkTitle.getNamespaceId() )
) {
if ( !pageTitle || !talkTitle || pageTitle.getMainText() !== talkTitle.getMainText() ) {
return;
}
talkNs = talkTitle.getNamespaceId();
pageNs = pageTitle.getNamespaceId();
inTalkNamespace = talkNs === pageNs;
if ( pageNs + 1 !== talkNs && !inTalkNamespace ) {
return;
}
@ -63,24 +64,18 @@
*/
function init() {
$talk.on( 'click', function () {
window.location.hash = '#/talk';
if ( $talk.hasClass( 'add' ) ) {
window.location.hash = '#/talk/new';
} else {
window.location.hash = '#/talk';
}
return false;
} );
}
init();
// add an "add discussion" button to talk pages (only for logged in users)
if (
!user.isAnon() &&
( page.inNamespace( 'talk' ) || page.inNamespace( 'user_talk' ) )
) {
new Button( {
label: mw.msg( 'minerva-talk-add-topic' ),
href: '#/talk/new',
progressive: true
} ).prependTo( '#content #bodyContent' );
if ( inTalkNamespace ) {
// reload the page after the new discussion was added
M.on( 'talk-added-wo-overlay', function () {
var loadingOverlay = new LoadingOverlay();

View File

@ -424,9 +424,6 @@
],
"scripts": [
"resources/skins.minerva.talk/init.js"
],
"messages": [
"minerva-talk-add-topic"
]
},
"skins.minerva.toggling": {

View File

@ -4,40 +4,42 @@ Feature: Talk
Background:
Given I am using the mobile site
@smoke @integration @login
Scenario: Talk doesn't show on talk pages
Given the page "Talk:Selenium talk test" exists
And I am logged in as a user with a > 5 edit count
And I am on the "Talk:Selenium talk test" page
Then there should be no talk button
@login
Scenario: Talk on a page that does exist
Given the page "Talk:Selenium talk test" exists
And I am logged in as a user with a > 5 edit count
And I am logged into the mobile website
And the page "Selenium talk test" exists
When I click the talk button
Then I should see the talk overlay
@login
Scenario: Talk on a page that doesn't exist (bug 64268)
Given I am logged in as a user with a > 5 edit count
Given I am logged into the mobile website
And I am on a page that does not exist
When I click the talk button
Then I should see the talk overlay
@smoke @integration @login
Scenario: Add discussion on talk page possible as logged in user
Scenario: Add discussion for talk page possible as logged in user
Given the page "Talk:Selenium talk test" exists
And I am logged in as a user with a > 5 edit count
And I am logged into the mobile website
And the page "Selenium talk test" exists
When I click the talk button
Then there should be an add discussion button
@smoke @integration @login
Scenario: Add topic button shows on talk pages for logged in users
Given the page "Talk:Selenium talk test" exists
And I am logged into the mobile website
And I am on the "Talk:Selenium UI test" page
And there should be an add discussion button
When I click the talk button
Then there should be an add discussion button
@integration
Scenario: A newly created topic appears in the list of topics immediately
Given the page "Talk:Selenium talk test" exists
And I am logged in as a user with a > 5 edit count
And I am logged into the mobile website
And the page "Selenium talk test" exists
When I click the talk button
And no topic is present