Hygiene: IssuesOverlay moved from MobileFrontend to Minerva

The CleanupOverlay is moved to Minerva and renamed the
IssuesOverlay to be consistent with current terminology

The new IssuesOverlay is defined inside the module
skins.minerva.scripts to which it now belongs.

Additional changes:
* various file renames
* overlay-cleanup renames overlay-issues
* cleanuptemplates renamed issues.js
* Add a test stub file to avoid the need to load templates inside
the test environment

After this change, I75f47622d94e504688e04dfb2892540473817053
should be merged to avoid confusion.

Change-Id: I08945a324a6b878abe56efed1e988466085b3018
This commit is contained in:
jdlrobson 2018-08-16 12:00:38 -07:00
parent 6a5b2e284d
commit 40eca4e3f3
13 changed files with 206 additions and 41 deletions

View File

@ -75,24 +75,26 @@ class MinervaHooks {
'mobile.startup',
'skins.minerva.notifications.badge',
'mediawiki.user',
'mediawiki.experiments',
'mobile.issues'
'mediawiki.experiments'
],
'localBasePath' => dirname( __DIR__ ),
'remoteSkinPath' => 'MinervaNeue',
'targets' => [ 'mobile', 'desktop' ],
'scripts' => [
// additional scaffolding (minus initialisation scripts)
'resources/skins.minerva.scripts/pageIssueLogger.js',
'resources/skins.minerva.scripts/pageIssueParser.js',
'tests/qunit/skins.minerva.scripts/stubs.js',
'resources/skins.minerva.scripts/pageIssuesLogger.js',
'resources/skins.minerva.scripts/pageIssuesParser.js',
'resources/skins.minerva.scripts/DownloadIcon.js',
'resources/skins.minerva.scripts/AB.js',
'resources/skins.minerva.scripts/cleanuptemplates.js',
'resources/skins.minerva.scripts/PageIssuesOverlay.js',
'resources/skins.minerva.scripts/pageIssues.js',
// test files
'tests/qunit/skins.minerva.scripts/test_DownloadIcon.js',
'tests/qunit/skins.minerva.scripts/test_pageIssueParser.js',
'tests/qunit/skins.minerva.scripts/test_pageIssuesParser.js',
'tests/qunit/skins.minerva.scripts/test_AB.js',
'tests/qunit/skins.minerva.scripts/test_cleanuptemplates.js',
'tests/qunit/skins.minerva.scripts/test_pageIssues.js',
'tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js'
],
];

View File

@ -6,7 +6,7 @@
isEditable = !isReadOnly && mw.config.get( 'wgIsProbablyEditable' ),
blockInfo = mw.config.get( 'wgMinervaUserBlockInfo', false ),
router = require( 'mediawiki.router' ),
issues = M.require( 'skins.minerva.scripts/cleanuptemplates' ),
issues = M.require( 'skins.minerva.scripts/pageIssues' ),
overlayManager = M.require( 'skins.minerva.scripts/overlayManager' ),
loader = M.require( 'mobile.startup/rlModuleLoader' ),
Icon = M.require( 'mobile.startup/Icon' ),

View File

@ -0,0 +1,94 @@
( function ( M ) {
var Overlay = M.require( 'mobile.startup/Overlay' ),
util = M.require( 'mobile.startup/util' );
/**
* @typedef {Object} PageIssue
* @property {string} icon html associated with Icon component
* @property {string} text html explaining the details of the issue
* @property {string} severity associated with the issue
*/
/**
* Obtain severity associated with a given $target node
* by looking at associated parent node (defined by template)
*
* @param {jQuery.Object} $target
* @return {string} severity as defined in associated PageIssue
*/
function parseSeverity( $target ) {
return $target.parents( '.issue-notice' ).data( 'severity' );
}
/**
* Overlay for displaying page issues
* @class PageIssuesOverlay
* @extends Overlay
*
* @param {Object} options Configuration options
* @param {string} options.headingText
* @param {PageIssue[]} options.issues list of page issues for display
* @fires PageIssuesOverlay#link-edit-click
* @fires PageIssuesOverlay#link-internal-click
*/
function PageIssuesOverlay( options ) {
options.heading = '<strong>' + options.headingText + '</strong>';
Overlay.call( this, options );
}
OO.mfExtend( PageIssuesOverlay, Overlay, {
/**
* @memberof PageIssuesOverlay
* @instance
*/
className: 'overlay overlay-issues',
/**
* @memberof PageIssuesOverlay
* @instance
*/
events: util.extend( {}, Overlay.prototype.events, {
'click a:not(.external):not([href*=edit])': 'onInternalClick',
'click a[href*="edit"]': 'onEditClick'
} ),
/**
* @memberof PageIssuesOverlay
* @instance
*/
templatePartials: util.extend( {}, Overlay.prototype.templatePartials, {
content: mw.template.get( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan' )
} ),
/**
* Event that is triggered when an internal link inside the overlay is clicked
* This event will not be triggered if the link contains the edit keyword, in which
* case onEditClick will be fired
* This is primarily used for instrumenting page issues
* (see https://meta.wikimedia.org/wiki/Schema:PageIssues)
* @param {jQuery.Event} ev
* @memberof PageIssuesOverlay
* @instance
*/
onInternalClick: function ( ev ) {
/**
* @event PageIssuesOverlay#link-internal-click
* @param {string} severity
*/
this.emit( 'link-internal-click', parseSeverity( this.$( ev.target ) ) );
},
/**
* Event that is triggered when an edit link inside the overlay is clicked
* This is primarily used for instrumenting page issues
* (see https://meta.wikimedia.org/wiki/Schema:PageIssues)
* @param {jQuery.Event} ev
* @memberof PageIssuesOverlay
* @instance
*/
onEditClick: function ( ev ) {
/**
* @event PageIssuesOverlay#link-edit-click
* @param {string} severity
*/
this.emit( 'link-edit-click', parseSeverity( this.$( ev.target ) ) );
}
} );
M.define( 'skins.minerva.scripts/PageIssuesOverlay', PageIssuesOverlay );
}( mw.mobileFrontend ) );

View File

@ -0,0 +1,54 @@
@import '../../minerva.less/minerva.variables';
@smallIconSize: 24px;
@largeIconSize: 50px;
.mw-mf-cleanup {
display: block;
margin: 0;
padding: 0;
font-size: 0.8em;
color: @colorGray7;
}
// overlay styles
.overlay-issues {
.cleanup {
> li {
border-bottom: solid 1px @grayLight;
.issue-notice {
padding: @smallIconSize @smallIconSize @smallIconSize 0;
.mw-ui-icon {
float: left;
}
}
small,
.hide-when-compact {
font-size: 0.8em;
}
.hide-when-compact {
display: block;
margin: 8px 0;
}
}
}
.issue-details {
// align the first line to the top of the element
// but line-height should be normal for text that follows (T190469)
> :first-line {
line-height: 1;
}
// This should match the icon width for the overlay close icon
// which is derived from these two variables:
padding-left: @iconSize + ( 2 * @iconGutterWidth );
// assume date
small i {
color: @colorGray7;
}
}
}

View File

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

View File

@ -3,7 +3,7 @@
toast = M.require( 'mobile.startup/toast' ),
time = M.require( 'mobile.startup/time' ),
skin = M.require( 'mobile.init/skin' ),
issues = M.require( 'skins.minerva.scripts/cleanuptemplates' ),
issues = M.require( 'skins.minerva.scripts/pageIssues' ),
DownloadIcon = M.require( 'skins.minerva.scripts/DownloadIcon' ),
browser = M.require( 'mobile.startup/Browser' ).getSingleton(),
loader = M.require( 'mobile.startup/rlModuleLoader' ),

View File

@ -11,9 +11,9 @@
CURRENT_NS = config.get( 'wgNamespaceNumber' ),
allPageIssuesSeverity,
Icon = M.require( 'mobile.startup/Icon' ),
pageIssueLogger = M.require( 'skins.minerva.scripts/pageIssueLogger' ),
pageIssueParser = M.require( 'skins.minerva.scripts/pageIssueParser' ),
CleanupOverlay = M.require( 'mobile.issues/CleanupOverlay' ),
pageIssuesLogger = M.require( 'skins.minerva.scripts/pageIssuesLogger' ),
pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' ),
PageIssuesOverlay = M.require( 'skins.minerva.scripts/PageIssuesOverlay' ),
// setup ab test
abTest = new AB( {
testName: 'WME.PageIssuesAB',
@ -64,7 +64,7 @@
}
} );
pageIssue = pageIssueParser.parse( $box.get( 0 ) );
pageIssue = pageIssuesParser.parse( $box.get( 0 ) );
return {
severity: pageIssue.severity,
@ -138,12 +138,12 @@
allIssues[section] = issues;
if ( $metadata.length && inline ) {
severity = pageIssueParser.maxSeverity(
severity = pageIssuesParser.maxSeverity(
issues.map( function ( issue ) { return issue.severity; } )
);
new Icon( {
glyphPrefix: 'minerva',
name: pageIssueParser.iconName( $metadata.get( 0 ), severity )
name: pageIssuesParser.iconName( $metadata.get( 0 ), severity )
} ).prependTo( $metadata.find( '.mbox-text' ) );
$learnMore = $( '<span>' )
.addClass( 'ambox-learn-more' )
@ -156,8 +156,8 @@
$learnMore.appendTo( $metadata.find( '.mbox-text' ) );
}
$metadata.click( function () {
var pageIssue = pageIssueParser.parse( this );
pageIssueLogger.log( {
var pageIssue = pageIssuesParser.parse( this );
pageIssuesLogger.log( {
action: 'issueClicked',
issuesSeverity: [ pageIssue.severity ]
} );
@ -169,7 +169,7 @@
// In group B, we link to all issues no matter where the banner is.
$link.attr( 'href', '#/issues/' + KEYWORD_ALL_SECTIONS );
$link.click( function () {
pageIssueLogger.log( {
pageIssuesLogger.log( {
action: 'issueClicked',
// empty array is passed for 'old' treatment.
issuesSeverity: []
@ -267,9 +267,9 @@
if ( isLoggingRequired( getIssues( KEYWORD_ALL_SECTIONS ) ) ) {
// Enable logging.
pageIssueLogger.subscribe(
pageIssuesLogger.subscribe(
newTreatmentEnabled,
pageIssueLogger.newPageIssueSchemaData(
pageIssuesLogger.newPageIssueSchemaData(
newTreatmentEnabled,
CURRENT_NS,
getIssues( KEYWORD_ALL_SECTIONS ).map( formatPageIssuesSeverity )
@ -279,7 +279,7 @@
// Setup the overlay route.
overlayManager.add( new RegExp( '^/issues/(\\d+|' + KEYWORD_ALL_SECTIONS + ')$' ), function ( section ) {
var overlay = new CleanupOverlay( {
var overlay = new PageIssuesOverlay( {
issues: getIssues( section ),
// 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
@ -288,19 +288,19 @@
} );
// Tracking overlay close event.
overlay.on( 'Overlay-exit', function () {
pageIssueLogger.log( {
pageIssuesLogger.log( {
action: 'modalClose',
issuesSeverity: getIssues( section ).map( formatPageIssuesSeverity )
} );
} );
overlay.on( 'link-edit-click', function ( severity ) {
pageIssueLogger.log( {
pageIssuesLogger.log( {
action: 'modalEditClicked',
issuesSeverity: [ severity ]
} );
} );
overlay.on( 'link-internal-click', function ( severity ) {
pageIssueLogger.log( {
pageIssuesLogger.log( {
action: 'modalInternalClicked',
issuesSeverity: [ severity ]
} );
@ -309,18 +309,18 @@
} );
// Tracking pageLoaded event (technically, "issues" loaded).
pageIssueLogger.log( {
pageIssuesLogger.log( {
action: 'pageLoaded',
issuesSeverity: allPageIssuesSeverity
} );
}
M.define( 'skins.minerva.scripts/cleanuptemplates', {
M.define( 'skins.minerva.scripts/pageIssues', {
init: initPageIssues,
// The logger requires initialization (subscription). Ideally, the logger would be initialized
// and passed to initPageIssues() by the client. Since it's not, expose a log method and hide
// the subscription call in cleanuptemplates.
log: pageIssueLogger.log,
log: pageIssuesLogger.log,
test: {
createBanner: createBanner
}

View File

@ -89,7 +89,7 @@
mwTrack( EVENT_PAGE_ISSUE_LOG, data );
}
M.define( 'skins.minerva.scripts/pageIssueLogger', {
M.define( 'skins.minerva.scripts/pageIssuesLogger', {
newPageIssueSchemaData: newPageIssueSchemaData,
subscribe: subscribe,
log: log

View File

@ -156,7 +156,7 @@
/**
* @module skins.minerva.scripts/utils
*/
M.define( 'skins.minerva.scripts/pageIssueParser', {
M.define( 'skins.minerva.scripts/pageIssuesParser', {
/**
* Extract an icon for use with the issue.
* @param {JQuery.Object} $box element to extract the icon from

View File

@ -393,7 +393,6 @@
"mediawiki.Title",
"mobile.startup",
"skins.minerva.mainMenu",
"mobile.issues",
"mobile.search.api",
"mobile.search",
"mobile.references",
@ -416,15 +415,20 @@
"mobile-frontend-redirected-from"
],
"styles": [
"resources/skins.minerva.scripts/styles.less"
"resources/skins.minerva.scripts/styles.less",
"resources/skins.minerva.scripts/PageIssuesOverlay.less"
],
"templates": {
"PageIssuesOverlayContent.hogan": "resources/skins.minerva.scripts/PageIssuesOverlayContent.hogan"
},
"scripts": [
"resources/skins.minerva.scripts/preInit.js",
"resources/skins.minerva.scripts/DownloadIcon.js",
"resources/skins.minerva.scripts/pageIssueLogger.js",
"resources/skins.minerva.scripts/pageIssueParser.js",
"resources/skins.minerva.scripts/pageIssuesLogger.js",
"resources/skins.minerva.scripts/pageIssuesParser.js",
"resources/skins.minerva.scripts/AB.js",
"resources/skins.minerva.scripts/cleanuptemplates.js",
"resources/skins.minerva.scripts/PageIssuesOverlay.js",
"resources/skins.minerva.scripts/pageIssues.js",
"resources/skins.minerva.scripts/init.js",
"resources/skins.minerva.scripts/initLogging.js",
"resources/skins.minerva.scripts/mobileRedirect.js",

View File

@ -0,0 +1 @@
mw.template.add( 'skins.minerva.scripts', 'PageIssuesOverlayContent.hogan', '' );

View File

@ -1,5 +1,5 @@
( function ( M ) {
var createBanner = M.require( 'skins.minerva.scripts/cleanuptemplates' ).test.createBanner,
var createBanner = M.require( 'skins.minerva.scripts/pageIssues' ).test.createBanner,
OverlayManager = M.require( 'mobile.startup/OverlayManager' ),
Page = M.require( 'mobile.startup/Page' ),
overlayManager = new OverlayManager( require( 'mediawiki.router' ) ),

View File

@ -1,7 +1,7 @@
( function ( M ) {
var pageIssueParser = M.require( 'skins.minerva.scripts/pageIssueParser' );
var pageIssuesParser = M.require( 'skins.minerva.scripts/pageIssuesParser' );
QUnit.module( 'Minerva pageIssueParser' );
QUnit.module( 'Minerva pageIssuesParser' );
/**
* @param {string} className
@ -34,7 +34,7 @@
test = params[2],
box = newBox( className );
assert.strictEqual(
pageIssueParser.test.parseSeverity( box ),
pageIssuesParser.test.parseSeverity( box ),
expect,
'Result should be the correct severity; case ' + i + ': ' + test + '.'
);
@ -63,7 +63,7 @@
test = params[3],
box = newBox( className );
assert.propEqual(
pageIssueParser.test.parseType( box, severity ),
pageIssuesParser.test.parseType( box, severity ),
expect,
'Result should be the correct icon type; case ' + i + ': ' + test + '.'
);
@ -90,7 +90,7 @@
expect = params[2],
box = newBox( className );
assert.strictEqual(
pageIssueParser.iconName( box, severity ),
pageIssuesParser.iconName( box, severity ),
expect,
'Result should be the correct ResourceLoader icon name; case ' + i + ': ' + severity + '.'
);
@ -112,7 +112,7 @@
expect = params[1];
assert.strictEqual(
pageIssueParser.maxSeverity( severities ),
pageIssuesParser.maxSeverity( severities ),
expect,
'Result should be the highest severity in the array; case ' + i + '.'
);