jenkins-bot has submitted this change and it was merged.
Change subject: Hygiene: More icon abstraction
......................................................................
Hygiene: More icon abstraction
Make more things dependent on Icon.js
Change-Id: Ie247030bd7d936b6d3a146fff15a0e6d28435216
---
M javascripts/Icon.js
M javascripts/application.js
M javascripts/modules/editor/editor.js
M javascripts/modules/toggling/toggle.js
M javascripts/modules/uploads/LeadPhotoUploaderButton.js
M javascripts/modules/uploads/PhotoUploaderButton.js
M javascripts/modules/watchstar/Watchstar.js
M less/icons.less
M less/modules/watchstar.less
M tests/qunit/modules/test_PageList.js
M tests/qunit/modules/watchstar/test_Watchstar.js
11 files changed, 77 insertions(+), 30 deletions(-)
Approvals:
Phuedx: Looks good to me, approved
jenkins-bot: Verified
diff --git a/javascripts/Icon.js b/javascripts/Icon.js
index b971e83..64fbab6 100644
--- a/javascripts/Icon.js
+++ b/javascripts/Icon.js
@@ -16,6 +16,22 @@
name: '',
modifier: ''
},
+ /**
+ * Return the full class name that is required for the icon to
render
+ * @method
+ * @return {string}
+ */
+ getClassName: function() {
+ return this.$el.children( 0 ).attr( 'class' );
+ },
+ /**
+ * Return the class that relates to the icon glyph
+ * @method
+ * @return {string}
+ */
+ getGlyphClassName: function() {
+ return this.options.base + '-' + this.options.name;
+ },
initialize: function( options ) {
if ( options.hasText ) {
options.modifier = 'icon-text';
diff --git a/javascripts/application.js b/javascripts/application.js
index 58eb71d..ec99cec 100644
--- a/javascripts/application.js
+++ b/javascripts/application.js
@@ -7,6 +7,8 @@
( function( M, $ ) {
var Router = M.require( 'Router' ),
OverlayManager = M.require( 'OverlayManager' ),
+ Icon = M.require( 'Icon' ),
+ watchIcon = new Icon( { name: 'watched' } ),
qs = window.location.search.split( '?' )[1],
PageApi = M.require( 'PageApi' ),
pageApi = new PageApi(),
@@ -392,7 +394,7 @@
},
lead: getLeadSection().html(),
isMainPage: mw.config.get( 'wgIsMainPage' ),
- isWatched: $( '#ca-watch' ).hasClass( 'watched' ),
+ isWatched: $( '#ca-watch' ).hasClass(
watchIcon.getGlyphClassName() ),
sections: pageApi.getSectionsFromHTML( $( '#content' )
),
id: mw.config.get( 'wgArticleId' )
} );
diff --git a/javascripts/modules/editor/editor.js
b/javascripts/modules/editor/editor.js
index 0dafde8..b7c8e4f 100644
--- a/javascripts/modules/editor/editor.js
+++ b/javascripts/modules/editor/editor.js
@@ -1,6 +1,11 @@
( function( M, $ ) {
var
+ Icon = M.require( 'Icon' ),
+ disabledEditIcon = new Icon( { name: 'edit' } ),
+ enabledEditIcon = new Icon( { name: 'edit-enabled' } ),
+ enabledClass = enabledEditIcon.getGlyphClassName(),
+ disabledClass = disabledEditIcon.getGlyphClassName(),
user = M.require( 'user' ),
popup = M.require( 'toast' ),
// FIXME: Disable on IE < 10 for time being
@@ -146,7 +151,7 @@
return result;
} );
- $( '#ca-edit' ).addClass( 'enabled' );
+ $( '#ca-edit' ).addClass( enabledClass ).removeClass(
disabledClass );
// Make sure we never create two edit links by accident
if ( $( '#ca-edit .edit-page' ).length === 0 ) {
@@ -187,9 +192,10 @@
} else {
M.getCurrentPage().isEditable( user ).done( function(
isEditable ) {
if ( isEditable ) {
- $( '#ca-edit' ).addClass( 'enabled'
).on( 'tap', function() {
- drawer.render().show();
- });
+ $( '#ca-edit' ).addClass( enabledClass
).removeClass( disabledClass ).
+ on( 'tap', function() {
+ drawer.render().show();
+ } );
} else {
showSorryToast(
'mobile-frontend-editor-disabled' );
}
diff --git a/javascripts/modules/toggling/toggle.js
b/javascripts/modules/toggling/toggle.js
index 6a7b9f9..0854c63 100644
--- a/javascripts/modules/toggling/toggle.js
+++ b/javascripts/modules/toggling/toggle.js
@@ -1,5 +1,10 @@
( function( M, $ ) {
- var currentPageTitle = M.getCurrentPage().title;
+ var currentPageTitle = M.getCurrentPage().title,
+ Icon = M.require( 'Icon' ),
+ iconUp = new Icon( { name: 'arrow-up', hasText: true } ),
+ iconDown = new Icon( { name: 'arrow-down', hasText: true,
additionalClassNames: 'icon-15px' } ),
+ classOpen = iconUp.getGlyphClassName(),
+ classClosed = iconDown.getGlyphClassName();
function getExpandedSections() {
var expandedSections = $.parseJSON(
@@ -92,9 +97,9 @@
$heading.toggleClass( 'open-block' );
if ( $heading.hasClass( 'open-block' ) ) {
- $heading.addClass( 'icon-arrow-up' ).removeClass(
'icon-arrow-down' );
+ $heading.addClass( classOpen ).removeClass( classClosed
);
} else {
- $heading.removeClass( 'icon-arrow-up' ).addClass(
'icon-arrow-down' );
+ $heading.removeClass( classOpen ).addClass( classClosed
);
}
$heading.next()
.toggleClass( 'open-block' )
@@ -153,13 +158,14 @@
function enable( $page ) {
var tagName, $headings, expandSections,
$firstHeading,
+ iconClass = iconDown.getClassName(),
collapseSectionsByDefault = mw.config.get(
'wgMFCollapseSectionsByDefault' );
$page = $page || $( '#content' );
$( 'html' ).removeClass( 'stub' );
$firstHeading = $page.find( 'h1,h2,h3,h4,h5,h6' ).eq(0);
tagName = $firstHeading.prop( 'tagName' ) || 'H1';
- $page.find( tagName ).addClass( 'collapsible-heading icon
icon-text icon-15px icon-arrow-down' );
+ $page.find( tagName ).addClass( 'collapsible-heading ' +
iconClass );
$headings = $page.find( '.collapsible-heading' );
$headings.next( 'div' ).addClass( 'collapsible-block' );
diff --git a/javascripts/modules/uploads/LeadPhotoUploaderButton.js
b/javascripts/modules/uploads/LeadPhotoUploaderButton.js
index 2ba3c8f..4189c6d 100644
--- a/javascripts/modules/uploads/LeadPhotoUploaderButton.js
+++ b/javascripts/modules/uploads/LeadPhotoUploaderButton.js
@@ -1,11 +1,13 @@
( function( M ) {
var
PhotoUploaderButton = M.require(
'modules/uploads/PhotoUploaderButton' ),
+ Icon = M.require( 'Icon' ),
+ uploadIcon = new Icon( { name: 'addimage-enabled',
additionalClassNames: 'enabled' } ),
LeadPhotoUploaderButton;
LeadPhotoUploaderButton = PhotoUploaderButton.extend( {
template: M.template.get(
'modules/uploads/LeadPhotoUploaderButton.hogan' ),
- className: 'enabled',
+ className: uploadIcon.getClassName(),
defaults: {
buttonCaption: mw.msg( 'mobile-frontend-photo-upload' ),
diff --git a/javascripts/modules/uploads/PhotoUploaderButton.js
b/javascripts/modules/uploads/PhotoUploaderButton.js
index 01b9a71..4928f12 100644
--- a/javascripts/modules/uploads/PhotoUploaderButton.js
+++ b/javascripts/modules/uploads/PhotoUploaderButton.js
@@ -1,5 +1,8 @@
( function( M, $ ) {
var View = M.require( 'View' ),
+ Icon = M.require( 'Icon' ),
+ photoIcon = new Icon( { name: 'photo', hasText: true,
+ additionalClassNames: 'mw-ui-progressive mw-ui-button
button' } ),
PhotoUploaderButton;
function isSupported() {
@@ -45,7 +48,7 @@
*/
PhotoUploaderButton = View.extend( {
template: M.template.get(
'modules/uploads/PhotoUploaderButton.hogan' ),
- className: 'mw-ui-progressive mw-ui-button button icon-photo
icon icon-text',
+ className: photoIcon.getClassName(),
postRender: function() {
var self = this, $input = this.$( 'input' );
diff --git a/javascripts/modules/watchstar/Watchstar.js
b/javascripts/modules/watchstar/Watchstar.js
index c330159..a8e1286 100644
--- a/javascripts/modules/watchstar/Watchstar.js
+++ b/javascripts/modules/watchstar/Watchstar.js
@@ -2,6 +2,9 @@
var View = M.require( 'View' ), Watchstar,
WatchstarApi = M.require( 'modules/watchstar/WatchstarApi' ),
+ Icon = M.require( 'Icon' ),
+ watchIcon = new Icon( { name: 'watch', additionalClassNames:
'icon-32px watch-this-article' } ),
+ watchedIcon = new Icon( { name: 'watched',
additionalClassNames: 'icon-32px watch-this-article' } ),
toast = M.require( 'toast' ),
user = M.require( 'user' ),
api = new WatchstarApi(),
@@ -17,7 +20,7 @@
page: M.getCurrentPage()
},
tagName: 'div',
- className: 'icon icon-32px watch-this-article',
+ className: watchIcon.getClassName(),
template: M.template.compile( '<a>{{tooltip}}</a>', 'hogan' ),
initialize: function( options ) {
var self = this, _super = View.prototype.initialize,
@@ -48,6 +51,8 @@
},
postRender: function( options ) {
var self = this, callback,
+ unwatchedClass = watchIcon.getGlyphClassName(),
+ watchedClass = watchedIcon.getGlyphClassName(),
checker,
page = options.page,
$el = self.$el;
@@ -92,9 +97,9 @@
// Add watched class if necessary
if ( !user.isAnon() && api.isWatchedPage( page ) ) {
- $el.addClass( 'watched' );
+ $el.addClass( watchedClass ).removeClass(
unwatchedClass );
} else {
- $el.removeClass( 'watched' );
+ $el.addClass( unwatchedClass ).removeClass(
watchedClass );
}
}
} );
diff --git a/less/icons.less b/less/icons.less
index 3b3b49e..1a592c0 100644
--- a/less/icons.less
+++ b/less/icons.less
@@ -126,18 +126,18 @@
// Styleguide 8.3.1.
.icon-edit {
.background-image('images/pagemenu/edit-locked.png');
+}
- // Icon (enabled edit)
- //
- // Renders an enabled edit icon
- //
- // Markup:
- // <div class="icon icon-edit enabled">edit</div>
- //
- // Styleguide 8.3.2.
- &.enabled {
- .background-image('images/pagemenu/edit.png');
- }
+// Icon (enabled edit)
+//
+// Renders an enabled edit icon
+//
+// Markup:
+// <div class="icon icon-edit-enabled">edit</div>
+//
+// Styleguide 8.3.2.
+.icon-edit-enabled {
+ .background-image('images/pagemenu/edit.png');
}
// Icon (talk)
@@ -172,7 +172,9 @@
// <div class="icon watch-this-article watched">watch</div>
//
// Styleguide 8.3.4.
- &.watched {
+ &.watched,
+ // FIXME: Remove above when cache clears
+ &.icon-watched {
.background-image('images/watched.png');
}
}
diff --git a/less/modules/watchstar.less b/less/modules/watchstar.less
index 19179d6..2aa5299 100644
--- a/less/modules/watchstar.less
+++ b/less/modules/watchstar.less
@@ -18,7 +18,7 @@
.watch-this-article {
.transition-transform( .5s );
- &.watched {
+ &.icon-watched {
.transform( rotate(72deg) );
}
}
diff --git a/tests/qunit/modules/test_PageList.js
b/tests/qunit/modules/test_PageList.js
index e264607..2ea359f 100644
--- a/tests/qunit/modules/test_PageList.js
+++ b/tests/qunit/modules/test_PageList.js
@@ -2,6 +2,8 @@
var PageList = M.require( 'modules/PageList' ),
user = M.require( 'user' ),
+ Icon = M.require( 'Icon' ),
+ watchIcon = new Icon( { name: 'watched' } ),
WatchstarApi = M.require( 'modules/watchstar/WatchstarApi' );
QUnit.module( 'MobileFrontend modules/PageList', {
@@ -24,14 +26,14 @@
pageids: [ 30, 50 ]
} ), 'A request to API was made to retrieve the statuses' );
assert.strictEqual( pl.$el.find( '.watch-this-article'
).length, 2, "2 articles have watch stars" );
- assert.strictEqual( pl.$el.find( '.watched' ).length, 1, "1 of
articles is marked as watched" );
+ assert.strictEqual( pl.$el.find( '.' +
watchIcon.getGlyphClassName() ).length, 1, "1 of articles is marked as watched"
);
} );
QUnit.test( 'In watched mode', 3, function( assert ) {
var pl = new PageList( { pages: [ { id: 30 }, { id: 50 }, { id:
60 } ], isWatchList: true } );
assert.ok( this.spy.notCalled, 'Callback avoided' );
assert.strictEqual( pl.$el.find( '.watch-this-article'
).length, 3, "3 articles have watch stars..." );
- assert.strictEqual( pl.$el.find( '.watched' ).length, 3,
"...and all are marked as watched." );
+ assert.strictEqual( pl.$el.find( '.' +
watchIcon.getGlyphClassName() ).length, 3, "...and all are marked as watched."
);
} );
}( jQuery, mw.mobileFrontend ) );
diff --git a/tests/qunit/modules/watchstar/test_Watchstar.js
b/tests/qunit/modules/watchstar/test_Watchstar.js
index f0091f7..705b857 100644
--- a/tests/qunit/modules/watchstar/test_Watchstar.js
+++ b/tests/qunit/modules/watchstar/test_Watchstar.js
@@ -4,6 +4,8 @@
WatchstarApi = M.require( 'modules/watchstar/WatchstarApi' ),
CtaDrawer = M.require( 'CtaDrawer' ),
toast = M.require( 'toast' ),
+ Icon = M.require( 'Icon' ),
+ watchIcon = new Icon( { name: 'watched' } ),
user = M.require( 'user' ),
Page = M.require( 'Page' );
@@ -48,7 +50,8 @@
action: 'watch',
pageids: 42
} ), 'The watch happened' );
- assert.strictEqual( $el.hasClass( 'watched' ), true, "After successful
watch has watched class" );
+ assert.strictEqual( $el.hasClass( watchIcon.getGlyphClassName() ),
+ true, "After successful watch has watched class" );
assert.strictEqual( $( '.toast' ).is( ':visible' ), true, "A toast is
shown" );
} );
--
To view, visit https://gerrit.wikimedia.org/r/167881
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie247030bd7d936b6d3a146fff15a0e6d28435216
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <[email protected]>
Gerrit-Reviewer: Awjrichards <[email protected]>
Gerrit-Reviewer: JGonera <[email protected]>
Gerrit-Reviewer: Jdlrobson <[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
