From 12b49a70ad07065e631535a30cb78cc5f4afc795 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Wed, 18 Oct 2017 11:28:37 -0700 Subject: [PATCH] Notification icon should not show NaN for persian Wikipedia Bug: T172755 Change-Id: I2884c8daed3fe0e0d05c746ec6319956b7426957 --- includes/skins/SkinMinerva.php | 4 +++- includes/skins/minerva.mustache | 2 +- includes/skins/secondaryButton.mustache | 4 ++-- .../NotificationBadge.js | 18 +++++++++++---- skin.json | 3 +++ tests/phpunit/skins/SkinMinervaTest.php | 9 +++++++- .../test_NotificationBadge.js | 23 ++++++++++++++++--- 7 files changed, 50 insertions(+), 13 deletions(-) diff --git a/includes/skins/SkinMinerva.php b/includes/skins/SkinMinerva.php index 584f6f1..52242d1 100644 --- a/includes/skins/SkinMinerva.php +++ b/includes/skins/SkinMinerva.php @@ -365,6 +365,7 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { // Set user button to empty string by default $tpl->set( 'secondaryButtonData', '' ); $notificationsTitle = ''; + $count = 0; $countLabel = ''; $isZero = true; $hasUnseen = false; @@ -411,7 +412,8 @@ class SkinMinerva extends SkinTemplate implements ICustomizableSkin { 'notificationIconClass' => MinervaUI::iconClass( 'notifications' ), 'title' => $notificationsMsg, 'url' => $url, - 'notificationCount' => $countLabel, + 'notificationCountRaw' => $count, + 'notificationCountString' => $countLabel, 'isNotificationCountZero' => $isZero, 'hasNotifications' => $hasUnseen, 'hasUnseenNotifications' => $hasUnseen diff --git a/includes/skins/minerva.mustache b/includes/skins/minerva.mustache index e43504d..4d0b1c1 100644 --- a/includes/skins/minerva.mustache +++ b/includes/skins/minerva.mustache @@ -32,4 +32,4 @@ {{>footer}} - + diff --git a/includes/skins/secondaryButton.mustache b/includes/skins/secondaryButton.mustache index 59bd982..fda9035 100644 --- a/includes/skins/secondaryButton.mustache +++ b/includes/skins/secondaryButton.mustache @@ -10,8 +10,8 @@ class="notification-count user-button {{#hasUnseenNotifications}}notification-unseen{{/hasUnseenNotifications}} {{#isNotificationCountZero}}zero{{/isNotificationCountZero}}">
- - {{notificationCount}} + + {{notificationCountString}}
diff --git a/resources/skins.minerva.notifications.badge/NotificationBadge.js b/resources/skins.minerva.notifications.badge/NotificationBadge.js index f6a9e84..87293c4 100644 --- a/resources/skins.minerva.notifications.badge/NotificationBadge.js +++ b/resources/skins.minerva.notifications.badge/NotificationBadge.js @@ -14,6 +14,7 @@ */ function NotificationBadge( options ) { var $el, + count = 0, el = options.el; if ( el ) { @@ -22,11 +23,12 @@ options.hasNotifications = options.hasUnseenNotifications; options.title = $el.find( 'a' ).attr( 'title' ); options.url = $el.find( 'a' ).attr( 'href' ); - options.notificationCount = parseInt( $el.find( 'span' ).text(), 10 ); + count = Number( $el.find( 'span' ).data( 'notification-count' ) ); } View.call( this, options ); this.url = this.$el.find( 'a' ).attr( 'href' ); this._bindOverlayManager(); + this.setCount( count ); } OO.mfExtend( NotificationBadge, View, { @@ -35,14 +37,14 @@ * @cfg {String} defaults.notificationIconClass e.g. mw-ui-icon for icon * @cfg {String} defaults.loadingIconHtml for spinner * @cfg {Boolean} defaults.hasUnseenNotifications whether the user has unseen notifications - * @cfg {Number} defaults.notificationCount number of unread notifications - */ + * @cfg {Number} defaults.notificationCountRaw number of unread notifications + */ defaults: { notificationIconClass: notificationIcon.getClassName(), loadingIconHtml: icons.spinner().toHtmlString(), hasNotifications: false, hasUnseenNotifications: false, - notificationCount: 0 + notificationCountRaw: 0 }, isBorderBox: false, /** @@ -126,7 +128,13 @@ * @param {Number} count */ setCount: function ( count ) { - this.options.notificationCount = count; + if ( count > 100 ) { + count = 100; + } + this.options.notificationCountRaw = count; + this.options.notificationCountString = mw.message( 'echo-badge-count', + mw.language.convertNumber( count ) + ).text(); this.options.isNotificationCountZero = count === 0; this.render(); }, diff --git a/skin.json b/skin.json index 73ab2c5..e51d3cb 100644 --- a/skin.json +++ b/skin.json @@ -325,6 +325,9 @@ ] }, "skins.minerva.notifications.badge": { + "messages": [ + "echo-badge-count" + ], "dependencies": [ "mediawiki.router", "mobile.startup" diff --git a/tests/phpunit/skins/SkinMinervaTest.php b/tests/phpunit/skins/SkinMinervaTest.php index 9ce2d91..ebe38cf 100644 --- a/tests/phpunit/skins/SkinMinervaTest.php +++ b/tests/phpunit/skins/SkinMinervaTest.php @@ -284,6 +284,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, $notificationsMsg, $notificationsTitle, + $count, $countLabel, $isZero, $hasUnseen @@ -294,7 +295,8 @@ class SkinMinervaTest extends MediaWikiTestCase { 'url' => SpecialPage::getTitleFor( $notificationsTitle ) ->getLocalURL( [ 'returnto' => $title->getPrefixedText() ] ), - 'notificationCount' => $countLabel, + 'notificationCountRaw' => $count, + 'notificationCountString' => $countLabel, 'isNotificationCountZero' => $isZero, 'hasNotifications' => $hasUnseen, 'hasUnseenNotifications' => $hasUnseen @@ -319,6 +321,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, 'Show my notifications', 'Notifications', + 110, '99+', false, true @@ -331,6 +334,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, 'Show my notifications', 'Notifications', + 3, '3', false, false @@ -343,6 +347,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, 'Show my notifications', 'Notifications', + 5, '5', false, false @@ -353,6 +358,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, 'Show my notifications', 'Notifications', + 0, '0', true, false @@ -379,6 +385,7 @@ class SkinMinervaTest extends MediaWikiTestCase { $title, 'You have new messages on your talk page', 'Mytalk', + 0, '', true, false diff --git a/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js b/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js index 9731762..ae17500 100644 --- a/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js +++ b/tests/qunit/skins.minerva.notifications.badge/test_NotificationBadge.js @@ -15,7 +15,7 @@ overlayManager: this.OverlayManager, hasNotifications: true, hasUnseenNotifications: true, - notificationCount: 5 + notificationCountRawRaw: 5 } ); initialClassExpectationsMet = badge.$el.find( '.mw-ui-icon' ).length === 0 && badge.$el.find( '.zero' ).length === 0; @@ -23,11 +23,28 @@ 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' ); + badge.setCount( 105 ); + assert.ok( badge.options.notificationCountRawRaw, 100, + 'Number is capped to 100.' ); + } ); + + QUnit.test( '#setCount (Eastern Arabic numerals)', function ( assert ) { + var badge = new NotificationBadge( { + overlayManager: this.OverlayManager, + el: $( '
۲
' ) + } ); + assert.ok( badge.options.notificationCountRawRaw, 2, + 'Number is parsed from Eastern Arabic numerals' ); + assert.ok( badge.options.notificationCountRawRawString, '۲', + 'Number will be rendered in Eastern Arabic numerals' ); + badge.setCount( 5 ); + assert.ok( badge.options.notificationCountRawRawString, '۵', + 'Number will be rendered in Eastern Arabic numerals' ); } ); QUnit.test( '#render [hasUnseenNotifications]', function ( assert ) { var badge = new NotificationBadge( { - notificationCount: 0, + notificationCountRawRaw: 0, overlayManager: this.OverlayManager, hasNotifications: false, hasUnseenNotifications: false @@ -37,7 +54,7 @@ QUnit.test( '#markAsSeen', function ( assert ) { var badge = new NotificationBadge( { - notificationCount: 2, + notificationCountRawRaw: 2, overlayManager: this.OverlayManager, hasNotifications: true, hasUnseenNotifications: true