pageIssuesOverlay is a factory function that returns an Overlay

This experiments with making PageIssuesOverlay an Overlay with
various options.

The appending of children is a little messy and points at a need
to standardise this some way
(see https://phabricator.wikimedia.org/T209647)

TODO:
* Remove the iconString property on PageIssueSummary which is no longer
needed

Bug: T209647
Change-Id: Iadd798a820dca6bbb31edc9a8570b6db7aac237a
This commit is contained in:
jdlrobson 2018-11-23 17:49:47 -08:00
parent e18ebe5311
commit d29eca2bc8
10 changed files with 84 additions and 37 deletions

View file

@ -101,7 +101,9 @@ class MinervaHooks {
'resources/skins.minerva.scripts/pageIssuesParser.js',
'resources/skins.minerva.scripts/DownloadIcon.js',
'resources/skins.minerva.scripts/AB.js',
'resources/skins.minerva.scripts/PageIssuesOverlay.js',
'resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js',
'resources/skins.minerva.scripts/page-issues/overlay/IssueList.js',
'resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js',
'resources/skins.minerva.scripts/pageIssues.js',
// test files
'tests/qunit/skins.minerva.scripts/DownloadIcon.test.js',

View file

@ -1,10 +0,0 @@
<ul class="cleanup">
{{#issues}}
<li>
<div class="issue-notice" data-severity="{{severity}}">
{{{iconString}}}
<div class="issue-details">{{{text}}}</div>
</div>
</li>
{{/issues}}
</ul>

View file

@ -0,0 +1,30 @@
( function ( M ) {
var View = M.require( 'mobile.startup' ).View,
IssueNotice = M.require( 'skins.minerva.scripts/IssueNotice' );
/**
* IssueList
* @class IssueList
* @extends View
*
* @param {IssueSummary} issues
*/
function IssueList( issues ) {
this.issues = issues;
View.call( this, { className: 'cleanup' } );
}
OO.mfExtend( IssueList, View, {
tagName: 'ul',
postRender: function () {
View.prototype.postRender.apply( this, arguments );
this.append(
this.issues.map( function ( issue ) {
return new IssueNotice( issue ).$el;
} )
);
}
} );
M.define( 'skins.minerva.scripts/IssueList', IssueList );
}( mw.mobileFrontend ) );

View file

@ -0,0 +1,3 @@
<div class="issue-notice" data-severity="{{#issue}}{{severity}}{{/issue}}">
<div class="issue-details">{{{text}}}</div>
</div>

View file

@ -0,0 +1,23 @@
( function ( M ) {
var View = M.require( 'mobile.startup' ).View;
/**
* IssueNotice
* @class IssueNotice
* @extends View
*
* @param {IssueSummary} props
*/
function IssueNotice( props ) {
View.call( this, props );
}
OO.mfExtend( IssueNotice, View, {
tagName: 'li',
template: mw.template.get( 'skins.minerva.scripts', 'IssueNotice.hogan' ),
postRender: function () {
View.prototype.postRender.apply( this, arguments );
this.$( '.issue-notice' ).prepend( this.options.issue.icon.$el );
}
} );
M.define( 'skins.minerva.scripts/IssueNotice', IssueNotice );
}( mw.mobileFrontend ) );

View file

@ -1,4 +1,4 @@
@import '../../minerva.less/minerva.variables';
@import '../../../../minerva.less/minerva.variables';
@smallIconSize: 24px;
@largeIconSize: 50px;

View file

@ -1,7 +1,7 @@
( function ( M, mwMsg ) {
var
Overlay = M.require( 'mobile.startup/Overlay' ),
util = M.require( 'mobile.startup/util' ),
IssueList = M.require( 'skins.minerva.scripts/IssueList' ),
KEYWORD_ALL_SECTIONS = 'all',
NS_MAIN = 0,
NS_TALK = 1,
@ -9,15 +9,14 @@
/**
* Overlay for displaying page issues
* @class PageIssuesOverlay
* @extends Overlay
*
* @param {IssueSummary[]} issues list of page issue summaries for display.
* @param {string} section
* @param {number} namespaceID
* @return {Overlay}
*/
function PageIssuesOverlay( issues, section, namespaceID ) {
var
function pageIssuesOverlay( issues, section, namespaceID ) {
var overlay,
// Note only the main namespace is expected to make use of section issues, so the
// heading will always be minerva-meta-data-issues-section-header regardless of
// namespace.
@ -25,22 +24,16 @@
getNamespaceHeadingText( namespaceID ) :
mwMsg( 'minerva-meta-data-issues-section-header' );
Overlay.call( this, {
issues: issues,
overlay = new Overlay( {
className: 'overlay overlay-issues',
heading: '<strong>' + headingText + '</strong>'
} );
}
OO.mfExtend( PageIssuesOverlay, Overlay, {
/**
* @memberof PageIssuesOverlay
* @instance
*/
templatePartials: util.extend( {}, Overlay.prototype.templatePartials, {
content: mw.template.get( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan' )
} )
} );
overlay.$( '.overlay-content' ).append(
new IssueList( issues ).$el
);
return overlay;
}
/**
* Obtain a suitable heading for the issues overlay based on the namespace
@ -60,5 +53,5 @@
}
}
M.define( 'skins.minerva.scripts/PageIssuesOverlay', PageIssuesOverlay );
M.define( 'skins.minerva.scripts/pageIssuesOverlay', pageIssuesOverlay );
}( mw.mobileFrontend, mw.msg ) );

View file

@ -9,7 +9,7 @@
CURRENT_NS = config.get( 'wgNamespaceNumber' ),
features = mw.config.get( 'wgMinervaFeatures', {} ),
pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ),
PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' ),
pageIssuesOverlay = M.require( 'skins.minerva.scripts/pageIssuesOverlay' ),
// When the query string flag is set force on new treatment.
// When wgMinervaPageIssuesNewTreatment is the default this line can be removed.
QUERY_STRING_FLAG = mw.util.getParamValue( 'minerva-issues' ),
@ -267,8 +267,9 @@
// Setup the overlay route.
overlayManager.add( new RegExp( '^/issues/(\\d+|' + KEYWORD_ALL_SECTIONS + ')$' ), function ( section ) {
return new PageIssuesOverlay(
getIssues( section ), section, CURRENT_NS );
return pageIssuesOverlay(
getIssues( section ), section, CURRENT_NS
);
} );
}

View file

@ -419,10 +419,10 @@
],
"styles": [
"resources/skins.minerva.scripts/styles.less",
"resources/skins.minerva.scripts/PageIssuesOverlay.less"
"resources/skins.minerva.scripts/page-issues/overlay/PageIssuesOverlay.less"
],
"templates": {
"PageIssuesOverlayContent.hogan": "resources/skins.minerva.scripts/PageIssuesOverlayContent.hogan"
"IssueNotice.hogan": "resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.hogan"
},
"scripts": [
"resources/skins.minerva.scripts/errorLogging.js",
@ -430,7 +430,9 @@
"resources/skins.minerva.scripts/DownloadIcon.js",
"resources/skins.minerva.scripts/pageIssuesParser.js",
"resources/skins.minerva.scripts/AB.js",
"resources/skins.minerva.scripts/PageIssuesOverlay.js",
"resources/skins.minerva.scripts/page-issues/overlay/IssueNotice.js",
"resources/skins.minerva.scripts/page-issues/overlay/IssueList.js",
"resources/skins.minerva.scripts/page-issues/overlay/pageIssuesOverlay.js",
"resources/skins.minerva.scripts/pageIssues.js",
"resources/skins.minerva.scripts/init.js",
"resources/skins.minerva.scripts/initLogging.js",

View file

@ -1 +1,4 @@
mw.template.add( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan', '' );
// Since tests.minerva.scripts does
// not pull in the entire module skins.minerva.scripts
// we have to stub certain templates to make it appear like its been loaded.
mw.template.add( 'skins.minerva.scripts', 'IssueNotice.hogan', '' );