Jdlrobson has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/348795 )
Change subject: Hygiene: Refactor Notification code - add NotificationBadge
class
......................................................................
Hygiene: Refactor Notification code - add NotificationBadge class
The code is very brittle here as the NotificationOverlay
communicates with the NotificationBadge. NotificationOverlay assumes
HTML markup rather than uses reliable methods to communicate changes.
This formalises their relationship.
To render the badge on the client the secondaryButton template is
now shipped to the client.
Tests have been added to cover behaviour.
The module lives in skins.minerva.notifications.badge to avoid pulling
all skins.minerva.notifications modules into the environment which can
lead to side effects due to the unconditional wiring up many of these modules
do to the existing HTML. It is not added to mobile.notifications.overlay to
avoid pulling OOjs UI into the startup JavaScript.
Change-Id: I246ed23164ff3f7ef8ea15896a9b3a0d19c2bb0d
---
M extension.json
M includes/Minerva.hooks.php
M includes/MobileFrontend.hooks.php
M includes/skins/SkinMinerva.php
M includes/skins/secondaryButton.mustache
M resources/mobile.notifications.overlay/NotificationsOverlay.js
A resources/skins.minerva.notifications.badge/NotificationBadge.js
M resources/skins.minerva.notifications/init.js
A tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js
9 files changed, 246 insertions(+), 99 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/95/348795/1
diff --git a/extension.json b/extension.json
index 622fdc2..8490356 100644
--- a/extension.json
+++ b/extension.json
@@ -1634,6 +1634,7 @@
"MobileFrontendHooks::onDiffViewHeader"
],
"ResourceLoaderTestModules": [
+ "MinervaHooks::onResourceLoaderTestModules",
"MobileFrontendHooks::onResourceLoaderTestModules"
],
"GetCacheVaryCookies": [
diff --git a/includes/Minerva.hooks.php b/includes/Minerva.hooks.php
index cacafff..7e4e89e 100644
--- a/includes/Minerva.hooks.php
+++ b/includes/Minerva.hooks.php
@@ -13,6 +13,35 @@
*/
class MinervaHooks {
/**
+ * ResourceLoaderTestModules hook handler
+ * @see
https://www.mediawiki.org/wiki/Manual:Hooks/ResourceLoaderTestModules
+ *
+ * @param array $testModules
+ * @param ResourceLoader $resourceLoader
+ * @return bool
+ */
+ public static function onResourceLoaderTestModules( array &$testModules,
+ ResourceLoader &$resourceLoader
+ ) {
+ $testModule = [
+ 'dependencies' => [
+ 'skins.minerva.notifications.badge'
+ ],
+ 'localBasePath' => dirname( __DIR__ ),
+ 'remoteExtPath' => 'MobileFrontend',
+ 'targets' => [ 'mobile', 'desktop' ],
+ 'scripts' => [
+
'tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js'
+ ],
+ ];
+
+ // Expose templates module
+ $testModules['qunit']["tests.skins.minerva"] = $testModule;
+
+ return true;
+ }
+
+ /**
* Invocation of hook SpecialPageBeforeExecute
*
* We use this hook to ensure that login/account creation pages
diff --git a/includes/MobileFrontend.hooks.php
b/includes/MobileFrontend.hooks.php
index aca4ee2..9fa1bc5 100644
--- a/includes/MobileFrontend.hooks.php
+++ b/includes/MobileFrontend.hooks.php
@@ -1024,12 +1024,23 @@
// add Echo, if it's installed
if ( class_exists( 'MWEchoNotifUser' ) ) {
$resourceLoader->register( [
+ 'skins.minerva.notifications.badge' =>
$resourceBoilerplate + [
+ 'dependencies' => [
+ 'mediawiki.router',
+ 'mobile.startup',
+ ],
+ 'scripts' => [
+
'resources/skins.minerva.notifications.badge/NotificationBadge.js',
+ ],
+ 'templates' => [
+ 'badge.hogan' =>
'includes/skins/secondaryButton.mustache',
+ ],
+ 'targets' => [ 'mobile', 'desktop' ],
+ ],
'skins.minerva.notifications' =>
$resourceBoilerplate + [
'dependencies' => [
- 'mobile.startup',
- 'mediawiki.router',
+
'skins.minerva.notifications.badge',
'skins.minerva.scripts',
- 'mediawiki.ui.anchor'
],
'scripts' => [
'resources/skins.minerva.notifications/init.js',
diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php
index 627ebcc..6b3b367 100644
--- a/includes/skins/SkinMinerva.php
+++ b/includes/skins/SkinMinerva.php
@@ -382,7 +382,7 @@
[ 'returnto' =>
$currentTitle->getPrefixedText() ] );
$tpl->set( 'secondaryButtonData', [
- 'class' => MobileUI::iconClass( 'notifications'
),
+ 'notificationIconClass' => MobileUI::iconClass(
'notifications' ),
'title' => $notificationsMsg,
'url' => $url,
'notificationCount' => $countLabel,
diff --git a/includes/skins/secondaryButton.mustache
b/includes/skins/secondaryButton.mustache
index 205554e..b3d6137 100644
--- a/includes/skins/secondaryButton.mustache
+++ b/includes/skins/secondaryButton.mustache
@@ -1,6 +1,6 @@
{{^hasUnseenNotifications}}
<a href="{{url}}" title="{{title}}"
- class="{{class}} user-button main-header-button" id="secondary-button">
+ class="{{notificationIconClass}} user-button main-header-button"
id="secondary-button">
{{title}}
</a>
{{/hasUnseenNotifications}}
diff --git a/resources/mobile.notifications.overlay/NotificationsOverlay.js
b/resources/mobile.notifications.overlay/NotificationsOverlay.js
index f55db5a..b3eaaa5 100644
--- a/resources/mobile.notifications.overlay/NotificationsOverlay.js
+++ b/resources/mobile.notifications.overlay/NotificationsOverlay.js
@@ -20,7 +20,7 @@
Overlay.apply( this, options );
// Anchor tag that corresponds to a notifications badge
- this.$badge = options.$badge;
+ this.badge = options.badge;
this.$overlay = $( '<div>' )
.addClass( 'notifications-overlay-overlay
position-fixed' );
@@ -32,7 +32,6 @@
mw.echo.config.maxPrioritizedActions = 1;
- this.count = 0;
this.doneLoading = false;
unreadCounter = new mw.echo.dm.UnreadNotificationCounter(
echoApi, 'all', maxNotificationCount );
@@ -90,7 +89,7 @@
wrapperWidget.populate()
.then( this.setDoneLoading.bind( this ) )
.then( this.controller.updateSeenTime.bind(
this.controller ) )
- .then( this.setBadgeSeen.bind( this ) )
+ .then( this.badge.markAsSeen() )
.then( this.checkShowMarkAllRead.bind( this ) );
};
@@ -159,7 +158,7 @@
* @method
*/
onError: function () {
- window.location.href = this.$badge.attr( 'href' );
+ window.location.href = this.badge.getNotificationURL();
},
/**
* Update the unread number on the notifications badge
@@ -168,29 +167,13 @@
* @method
*/
onUnreadCountChange: function ( count ) {
- var $badgeCounter = this.$badge.find(
'.notification-count' );
- this.count =
this.controller.manager.getUnreadCounter().getCappedNotificationCount( count );
+ this.badge.setCount(
+
this.controller.manager.getUnreadCounter().getCappedNotificationCount( count )
+ );
- if ( this.count >= 0 ) {
- $badgeCounter.find( 'span' ).text(
- mw.msg( 'echo-badge-count',
mw.language.convertNumber( this.count ) )
- ).show();
- }
- if ( this.count === 0 ) {
- $badgeCounter.removeClass(
'notification-unseen' );
- }
this.checkShowMarkAllRead();
},
- /**
- * Mark that all the notifications in the badge are seen.
- *
- * @method
- */
- setBadgeSeen: function () {
- this.$badge
- .find( '.notification-count' )
- .removeClass( 'notification-unseen' );
- },
+
/** @inheritdoc */
preRender: function () {
this.options.heading = '<strong>' + mw.msg(
'notifications' ) + '</strong>';
diff --git a/resources/skins.minerva.notifications.badge/NotificationBadge.js
b/resources/skins.minerva.notifications.badge/NotificationBadge.js
new file mode 100644
index 0000000..bec2c28
--- /dev/null
+++ b/resources/skins.minerva.notifications.badge/NotificationBadge.js
@@ -0,0 +1,133 @@
+( function ( M ) {
+ var View = M.require( 'mobile.startup/View' ),
+ Icon = M.require( 'mobile.startup/Icon' ),
+ notificationIcon = new Icon( { name: 'notifications' } ),
+ icons = M.require( 'mobile.startup/icons' );
+
+ /**
+ * A notification button for communicating with an NotificationOverlay
+ * @class NotificationButton
+ * @extends View
+ *
+ * @constructor
+ * @param {Object} options Configuration options
+ */
+ function NotificationBadge( options ) {
+ var $el,
+ el = options.el;
+
+ if ( el ) {
+ $el = $( el );
+ options.hasUnseenNotifications = $el.find(
'.notification-unseen' ).length;
+ options.title = $el.find( 'a' ).attr( 'title' );
+ options.url = $el.find( 'a' ).attr( 'href' );
+ }
+ View.apply( this, arguments );
+ this.url = this.$el.find( 'a' ).attr( 'href' );
+ this._bindOverlayManager();
+ }
+
+ OO.mfExtend( NotificationBadge, View, {
+ /** @inheritdoc */
+ defaults: {
+ notificationIconClass: notificationIcon.getClassName(),
+ isLoading: false,
+ loadingIconHtml: icons.spinner().toHtmlString(),
+ hasUnseenNotifications: false
+ },
+ /**
+ * Loads a ResourceLoader module script. Shows ajax loader
whilst loading.
+ * @method
+ * @private
+ * @param {string} moduleName Name of a module to fetch
+ * @return {jQuery.Deferred}
+ */
+ _loadModuleScript: function ( moduleName ) {
+ var self = this;
+
+ this.$el.html( this.options.loadingIconHtml );
+ return mw.loader.using( moduleName ).done( function () {
+ // trigger a re-render once one to remove
loading icon
+ self.render();
+ } );
+ },
+ /**
+ * Load the notification overlay.
+ * @method
+ * @private
+ * @uses NotificationsOverlay
+ * @return {jQuery.Deferred} with an instance of
NotificationsOverlay
+ */
+ _loadNotificationOverlay: function () {
+ var self = this;
+
+ return this._loadModuleScript(
'mobile.notifications.overlay' ).then( function () {
+ var NotificationsOverlay = M.require(
'mobile.notifications.overlay/NotificationsOverlay' );
+ return new NotificationsOverlay( {
+ badge: self
+ } );
+ } );
+ },
+ /**
+ * Sets up routes in overlay manager and click behaviour for
NotificationBadge
+ * This is not unit tested as it's behaviour is covered by
browser tests.
+ * @private
+ */
+ _bindOverlayManager: function () {
+ var self = this,
+ mainMenu = this.options.mainMenu;
+
+ this.$el.on( 'click', $.proxy( this.onClickBadge, this
) );
+ this.options.overlayManager.add( /^\/notifications$/,
function () {
+ return self._loadNotificationOverlay().done(
function ( overlay ) {
+ mainMenu.openNavigationDrawer(
'secondary' );
+ overlay.on( 'hide', function () {
+
mainMenu.closeNavigationDrawers();
+ $( '#mw-mf-page-center' ).off(
'.secondary' );
+ } );
+
+ $( '#mw-mf-page-center' ).one(
'click.secondary', function () {
+ self.options.router.back();
+ } );
+ } );
+ } );
+ },
+ /** @inheritdoc */
+ template: mw.template.get( 'skins.minerva.notifications.badge',
'badge.hogan' ),
+ /**
+ * Click handler for clicking on the badge
+ * @return {Boolean}
+ */
+ onClickBadge: function () {
+ this.options.router.navigate( '#/notifications' );
+ // Important that we also prevent propagation to avoid
interference with events that may be
+ // binded on #mw-mf-page-center that close overlay
+ return false;
+ },
+ /**
+ * Return the URL for the full non-overlay notification view
+ * @return {String} url
+ */
+ getNotificationURL: function () {
+ return this.options.url;
+ },
+ /**
+ * Update the notification count
+ * @param {Number} count
+ */
+ setCount: function ( count ) {
+ this.options.notificationCount = count;
+ this.options.isNotificationCountZero = count === 0;
+ this.render();
+ },
+ /**
+ * Marks all notifications as seen
+ */
+ markAsSeen: function () {
+ this.options.hasUnseenNotifications = false;
+ this.render();
+ }
+ } );
+
+ M.define( 'skins.minerva.notifications/NotificationBadge',
NotificationBadge );
+}( mw.mobileFrontend ) );
diff --git a/resources/skins.minerva.notifications/init.js
b/resources/skins.minerva.notifications/init.js
index 4b285f1..4cbccb2 100644
--- a/resources/skins.minerva.notifications/init.js
+++ b/resources/skins.minerva.notifications/init.js
@@ -2,81 +2,22 @@
* This code loads the necessary modules for the notifications overlay, not to
be confused
* with the Toast notifications defined by common/toast.js.
*/
-( function ( M, $, mw ) {
+( function ( M, $ ) {
var mainMenu = M.require( 'skins.minerva.scripts/skin' ).getMainMenu(),
- $btn = $( '#secondary-button.user-button' ).parent(),
router = require( 'mediawiki.router' ),
- overlayManager = M.require(
'skins.minerva.scripts/overlayManager' ),
- icons = M.require( 'mobile.startup/icons' );
-
- /**
- * Loads a ResourceLoader module script. Shows ajax loader whilst
loading.
- * @method
- * @ignore
- * FIXME: Upstream to mw.mobileFrontend and reuse elsewhere
- * @param {string} moduleName Name of a module to fetch
- * @return {jQuery.Deferred}
- */
- function loadModuleScript( moduleName ) {
- var d = $.Deferred(),
- $spinner = $( icons.spinner().toHtmlString() );
-
- $btn.hide().after( $spinner );
- mw.loader.using( moduleName, function () {
- // FIXME: Some code uses the loading class while other
code uses the
- // spinner class. Make all code consistent so it's
easier to change.
- $spinner.remove();
- $btn.show();
- d.resolve();
- } );
- return d;
- }
+ NotificationBadge = M.require(
'skins.minerva.notifications/NotificationBadge' ),
+ overlayManager = M.require(
'skins.minerva.scripts/overlayManager' );
// Once the DOM is loaded hijack the notifications button to display an
overlay rather
// than linking to Special:Notifications.
$( function () {
- $btn.on( 'click', function () {
- router.navigate( '#/notifications' );
- // Important that we also prevent propagation to avoid
interference with events that may be
- // binded on #mw-mf-page-center that close overlay
- return false;
- } );
-
- /**
- * Load the notification overlay.
- * @method
- * @ignore
- * @private
- * @uses NotificationsOverlay
- * @return {jQuery.Deferred} with an instance of
NotificationsOverlay
- */
- function loadNotificationOverlay() {
- var result = $.Deferred();
- loadModuleScript( 'mobile.notifications.overlay'
).done( function () {
- var NotificationsOverlay = M.require(
'mobile.notifications.overlay/NotificationsOverlay' );
- result.resolve(
- new NotificationsOverlay( {
- $badge: $btn,
- count: parseInt( $btn.find(
'span' ).text(), 10 )
- } )
- );
- } );
-
- return result;
- }
-
- overlayManager.add( /^\/notifications$/, function () {
- return loadNotificationOverlay().done( function (
overlay ) {
- mainMenu.openNavigationDrawer( 'secondary' );
- overlay.on( 'hide', function () {
- mainMenu.closeNavigationDrawers();
- $( '#mw-mf-page-center' ).off(
'.secondary' );
- } );
-
- $( '#mw-mf-page-center' ).one(
'click.secondary', function () {
- router.back();
- } );
- } );
+ // eslint-disable-next-line no-new
+ new NotificationBadge( {
+ overlayManager: overlayManager,
+ router: router,
+ mainMenu: mainMenu,
+ el: $( '#secondary-button.user-button' ).parent()
} );
} );
-}( mw.mobileFrontend, jQuery, mw ) );
+
+}( mw.mobileFrontend, jQuery ) );
diff --git
a/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js
b/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js
new file mode 100644
index 0000000..742e669
--- /dev/null
+++ b/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js
@@ -0,0 +1,49 @@
+( function ( M ) {
+ var OverlayManager = M.require( 'mobile.startup/OverlayManager' ),
+ NotificationBadge = M.require(
'skins.minerva.notifications/NotificationBadge' );
+
+ QUnit.module( 'Minerva NotificationBadge', {
+ setup: function () {
+ this.router = require( 'mediawiki.router' );
+ this.OverlayManager = new OverlayManager( this.router );
+ }
+ } );
+
+ QUnit.test( '#setCount', 2, function ( assert ) {
+ var initialClassExpectationsMet,
+ badge = new NotificationBadge( {
+ overlayManager: this.OverlayManager,
+ hasUnseenNotifications: true,
+ notificationCount: 5
+ } );
+ initialClassExpectationsMet = badge.$el.find( '.mw-ui-icon'
).length === 0 &&
+ badge.$el.find( '.zero' ).length === 0;
+
+ badge.setCount( 0 );
+ assert.ok( initialClassExpectationsMet, 'No icon and no zero
class' );
+ assert.ok( badge.$el.find( '.zero' ).length === 1, 'A zero
class is present on the badge' );
+ } );
+
+ QUnit.test( '#render [hasUnseenNotifications]', 1, function ( assert ) {
+ var badge = new NotificationBadge( {
+ notificationCount: 0,
+ overlayManager: this.OverlayManager,
+ hasUnseenNotifications: false
+ } );
+ assert.ok( badge.$el.find( '.mw-ui-icon' ).length === 1, 'A
bell icon is visible' );
+ } );
+
+ QUnit.test( '#markAsSeen', 2, function ( assert ) {
+ var badge = new NotificationBadge( {
+ notificationCount: 2,
+ overlayManager: this.OverlayManager,
+ hasUnseenNotifications: true
+ } );
+ // Badge resets counter to zero
+ badge.setCount( 0 );
+ assert.ok( badge.$el.find( '.mw-ui-icon' ).length === 0, 'The
bell icon is not visible' );
+ badge.markAsSeen();
+ assert.ok( badge.$el.find( '.mw-ui-icon' ).length === 1,
+ 'A bell icon is visible only after a call to markAsSeen
is called.' );
+ } );
+}( mw.mobileFrontend ) );
--
To view, visit https://gerrit.wikimedia.org/r/348795
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I246ed23164ff3f7ef8ea15896a9b3a0d19c2bb0d
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
