jenkins-bot has submitted this change and it was merged. Change subject: Display a message if there are no notifications in Special:Notifications ......................................................................
Display a message if there are no notifications in Special:Notifications The zero results can either be because there are no notifications at all for this user in the local wiki, or because there are no results for the specific filter. Both messages are used for either case. Also, clean up the display of push/pop pending for the inbox widget and hide the label in case the message count is 0 or 1 notifications as it is unhelpful and irrelevant in these cases. Bug: T136586 Bug: T136574 Bug: T129363 Change-Id: I1465f772bb9f5247df645d6612f951e5fd7d38cf --- M Resources.php M i18n/en.json M i18n/qqq.json M modules/api/mw.echo.api.EchoApi.js M modules/controller/mw.echo.Controller.js M modules/model/mw.echo.dm.ModelManager.js M modules/model/mw.echo.dm.NotificationsList.js M modules/model/mw.echo.dm.PaginationModel.js M modules/ui/mw.echo.ui.NotificationsInboxWidget.js M modules/ui/mw.echo.ui.PaginationWidget.js 10 files changed, 192 insertions(+), 30 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/Resources.php b/Resources.php index 9cdbfce..999f6e7 100644 --- a/Resources.php +++ b/Resources.php @@ -296,6 +296,9 @@ 'notification-inbox-filter-read', 'notification-inbox-filter-unread', 'notification-inbox-filter-all', + 'echo-notification-placeholder-filters', + 'echo-specialpage-pagination-numnotifications', + 'echo-specialpage-pagination-range', 'echo-more-info', 'echo-feedback', 'echo-specialpage-section-markread', diff --git a/i18n/en.json b/i18n/en.json index c2a9094..1c5db68 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -97,6 +97,8 @@ "echo-specialpage-section-markread": "Mark group as read", "echo-specialpage-markasread": "Notification: Mark as read", "echo-specialpage-markasread-invalid-id": "Invalid event ID", + "echo-specialpage-pagination-numnotifications": "$1 {{PLURAL:$1|notification|notifications}}", + "echo-specialpage-pagination-range": "$1 - $2", "notificationsmarkread-legend": "Mark notification as read", "echo-anon": "To receive notifications, [$1 create an account] or [$2 log in].", "echo-none": "You have no notifications.", @@ -108,6 +110,7 @@ "echo-api-failure": "Failed to fetch notifications.", "echo-api-failure-cross-wiki": "Access to the remote domain was denied.", "echo-notification-placeholder": "There are no notifications.", + "echo-notification-placeholder-filters": "There are no notifications matching this criteria.", "echo-notification-loginrequired": "You must login to see your notifications.", "echo-notification-popup-loginrequired": "Please log in to view your notifications.", "echo-notification-markasread": "Mark as read", diff --git a/i18n/qqq.json b/i18n/qqq.json index da9a4aa..9b792f1 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -88,6 +88,8 @@ "echo-specialpage-section-markread": "Label for the button to mark notifications in the given section as read in [[Special:Notifications]]", "echo-specialpage-markasread": "Special page title for Special:NotificationsMarkRead for marking specific notification as read.", "echo-specialpage-markasread-invalid-id": "Error message shown to users who try to mark a notification as read with an invalid event ID.", + "echo-specialpage-pagination-numnotifications": "Label noting the number of notifications displayed in the page. This only appears if there is a single page of results.\n\nParameters:\n* $1 - Number of notifications in the page.", + "echo-specialpage-pagination-range": "Label noting the range of the notifications displayed in the page. This only appears if there are multiple pages of results available.\n\nParameters:\n* $1 - Number of the first item.\n* $2 - Number of the last item.", "notificationsmarkread-legend": "Title for the form that marks a notification as read in [[Special:NotificationsMarkAsRead]]", "echo-anon": "Error message shown to users who try to visit [[Special:Notifications]] as an anon.\n\nParameters:\n* $1 - URL of signup page, with returnto pointing to Special:Notifications\n* $2 - URL of login page, with returnto pointing to Special:Notifications", "echo-none": "Message shown to users who have no notifications. Also shown in the overlay.", @@ -99,6 +101,7 @@ "echo-api-failure": "Label for the text that notes an error in retrieving notifications for the Echo popup.", "echo-api-failure-cross-wiki": "Label for the api failure text for a failure to fetch cross-wiki notifications, but the remote server is not granted access.", "echo-notification-placeholder": "Label for the text that appears if there are no notifications in the Echo popup.", + "echo-notification-placeholder-filters": "Label for the text that appears if there are no notifications in the Special:Notifications page with filters selected.", "echo-notification-loginrequired": "Message that displays when an anonymous user attempts to view notifications and gets redirect to the login page.", "echo-notification-popup-loginrequired": "Message that displays when an anonymous user attempts to view notifications in their popup after a session expired.", "echo-notification-markasread": "Label for the button to mark the notification as read.", diff --git a/modules/api/mw.echo.api.EchoApi.js b/modules/api/mw.echo.api.EchoApi.js index c0de083..bd5ad6d 100644 --- a/modules/api/mw.echo.api.EchoApi.js +++ b/modules/api/mw.echo.api.EchoApi.js @@ -189,4 +189,12 @@ return this.network.getApiHandler( source ).getFetchNotificationPromise( type ); }; + /** + * Get the set limit for fetching notifications per request + * + * @return {number} Limit of notifications per request + */ + mw.echo.api.EchoApi.prototype.getLimit = function () { + return this.limit; + }; } )( mediaWiki ); diff --git a/modules/controller/mw.echo.Controller.js b/modules/controller/mw.echo.Controller.js index 7fad2b5..bfd2081 100644 --- a/modules/controller/mw.echo.Controller.js +++ b/modules/controller/mw.echo.Controller.js @@ -76,6 +76,7 @@ filters = this.manager.getFiltersModel(), continueValue = pagination.getPageContinue( page || pagination.getCurrPageIndex() ); + pagination.setItemsPerPage( this.api.getLimit() ); return this.api.fetchNotifications( this.manager.getTypeString(), 'local', @@ -84,7 +85,7 @@ filters.getReadState() ) .then( function ( data ) { - var i, notifData, newNotifData, date, itemModel, symbolicName, + var i, notifData, newNotifData, date, itemModel, symbolicName, count, dateItemIds = {}, dateItems = {}, models = {}; @@ -137,7 +138,13 @@ controller.manager.setNotificationModels( models ); // Update the pagination - controller.manager.getPaginationModel().setNextPageContinue( data.continue ); + count = controller.manager.getAllNotificationCount(); + if ( count < pagination.getItemsPerPage() ) { + pagination.setLastPageItemCount( + controller.manager.getAllNotificationCount() + ); + } + pagination.setNextPageContinue( data.continue ); return dateItemIds; } ); diff --git a/modules/model/mw.echo.dm.ModelManager.js b/modules/model/mw.echo.dm.ModelManager.js index 766200c..9fcae1c 100644 --- a/modules/model/mw.echo.dm.ModelManager.js +++ b/modules/model/mw.echo.dm.ModelManager.js @@ -8,11 +8,12 @@ * cross wiki notification, which acts as an item but itself contains * a list. * - * All models that are managed by the manager must implement the following - * methods: + * All notification models that are managed by the manager must implement the + * following methods: * * isGroup - This should be true for xwiki model and local bundles * * hasUnseen - This should iterate in the model's items and check whether * there are any unseen notifications within them. + * * getCount - Get a total count of available notifications currently in the model * * @class * @mixins OO.EventEmitter @@ -116,6 +117,23 @@ }; /** + * Go over all the notification models and return the total number of + * available notifications. + * + * @return {number} A count of all notifications + */ + mw.echo.dm.ModelManager.prototype.getAllNotificationCount = function () { + var model, + count = 0, + models = this.getAllNotificationModels(); + + for ( model in models ) { + count += models[ model ].getCount(); + } + + return count; + }; + /** * Get a notification model. * * @param {string} modelName Unique model name diff --git a/modules/model/mw.echo.dm.NotificationsList.js b/modules/model/mw.echo.dm.NotificationsList.js index 97f2732..b2b46fe 100644 --- a/modules/model/mw.echo.dm.NotificationsList.js +++ b/modules/model/mw.echo.dm.NotificationsList.js @@ -193,6 +193,15 @@ }; /** + * A general method to get the number of notifications in this list + * + * @return {number} Item count + */ + mw.echo.dm.NotificationsList.prototype.getCount = function () { + return this.getItemCount(); + }; + + /** * Check if there are unseen items in this list * * @return {boolean} There are unseen items in the list diff --git a/modules/model/mw.echo.dm.PaginationModel.js b/modules/model/mw.echo.dm.PaginationModel.js index b13d58a..4d516ff 100644 --- a/modules/model/mw.echo.dm.PaginationModel.js +++ b/modules/model/mw.echo.dm.PaginationModel.js @@ -8,6 +8,9 @@ * @constructor * @param {Object} config Configuration object * @cfg {string} [pageNext] The continue value of the next page + * @cfg {number} [lastPageItemCount] The number of items that are in the + * last page. + * @cfg {number} [itemsPerPage] The number of items per page */ mw.echo.dm.PaginationModel = function MwEchoDmPaginationModel( config ) { config = config || {}; @@ -16,6 +19,8 @@ OO.EventEmitter.call( this ); this.pagesContinue = []; + this.lastPageItemCount = this.lastPageItemCount || 0; + this.itemsPerPage = this.itemsPerPage || 25; // Set initial page this.currPageIndex = 0; @@ -161,4 +166,40 @@ mw.echo.dm.PaginationModel.prototype.hasNextPage = function () { return !!this.pagesContinue[ this.currPageIndex + 1 ]; }; + + /** + * Set the number of items in the last page + * + * @param {number} count Number of items + */ + mw.echo.dm.PaginationModel.prototype.setLastPageItemCount = function ( count ) { + this.lastPageItemCount = count; + }; + + /** + * Get the number of items in the last page + * + * @return {number} Number of items + */ + mw.echo.dm.PaginationModel.prototype.getLastPageItemCount = function () { + return this.lastPageItemCount; + }; + + /** + * Set the number of items per page + * + * @param {number} count Number of items per page + */ + mw.echo.dm.PaginationModel.prototype.setItemsPerPage = function ( count ) { + this.itemsPerPage = count; + }; + + /** + * Get the number of items per page + * + * @return {number} Number of items per page + */ + mw.echo.dm.PaginationModel.prototype.getItemsPerPage = function () { + return this.itemsPerPage; + }; } )( mediaWiki ); diff --git a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js index d191873..93bded3 100644 --- a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js +++ b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js @@ -27,6 +27,11 @@ this.$overlay = config.$overlay || this.$element; this.limit = config.limit || 25; + // A notice or error message widget + this.noticeMessageWidget = new OO.ui.LabelWidget( { + classes: [ 'mw-echo-ui-notificationsInboxWidget-notice' ] + } ); + // Notifications list this.datedListWidget = new mw.echo.ui.DatedNotificationsWidget( this.controller, @@ -85,6 +90,7 @@ .append( this.topPaginationWidget.$element ) ) ), + this.noticeMessageWidget.$element, this.datedListWidget.$element, $( '<div>' ) .addClass( 'mw-echo-ui-notificationsInboxWidget-toolbar-bottom' ) @@ -151,12 +157,64 @@ } this.pushPending(); - this.topPaginationWidget.setDisabled( true ); - this.bottomPaginationWidget.setDisabled( true ); return fetchPromise - .then( this.topPaginationWidget.setDisabled.bind( this.topPaginationWidget, false ) ) - .then( this.bottomPaginationWidget.setDisabled.bind( this.bottomPaginationWidget, false ) ) // Pop pending .always( this.popPending.bind( this ) ); }; + + /** + * Extend the pushPending method to disable UI elements + */ + mw.echo.ui.NotificationsInboxWidget.prototype.pushPending = function () { + this.noticeMessageWidget.toggle( false ); + this.readStateSelectWidget.setDisabled( true ); + this.topPaginationWidget.setDisabled( true ); + this.bottomPaginationWidget.setDisabled( true ); + + // Mixin method + OO.ui.mixin.PendingElement.prototype.pushPending.call( this ); + }; + + /** + * Extend the popPending method to enable UI elements + */ + mw.echo.ui.NotificationsInboxWidget.prototype.popPending = function () { + this.resetMessageLabel(); + this.readStateSelectWidget.setDisabled( false ); + this.topPaginationWidget.setDisabled( false ); + this.bottomPaginationWidget.setDisabled( false ); + + // Mixin method + OO.ui.mixin.PendingElement.prototype.popPending.call( this ); + }; + + /** + * Reset the the text of the error message that displays in place of the list + * in case the list is empty. + */ + mw.echo.ui.NotificationsInboxWidget.prototype.resetMessageLabel = function () { + var label, + count = this.manager.getAllNotificationCount(); + + if ( count === 0 ) { + label = this.manager.getFiltersModel().getReadState() === 'all' ? + mw.msg( 'echo-notification-placeholder' ) : + mw.msg( 'echo-notification-placeholder-filters' ); + + this.noticeMessageWidget.setLabel( label ); + } + + this.displayMessage( count === 0 ); + }; + + /** + * Display the error/notice message instead of the notifications list or vise versa. + * + * @private + * @param {boolean} displayMessage Display error message + */ + mw.echo.ui.NotificationsInboxWidget.prototype.displayMessage = function ( displayMessage ) { + this.noticeMessageWidget.toggle( displayMessage ); + this.datedListWidget.toggle( !displayMessage ); + }; } )( jQuery, mediaWiki ); diff --git a/modules/ui/mw.echo.ui.PaginationWidget.js b/modules/ui/mw.echo.ui.PaginationWidget.js index 47f4501..7fbb15d 100644 --- a/modules/ui/mw.echo.ui.PaginationWidget.js +++ b/modules/ui/mw.echo.ui.PaginationWidget.js @@ -109,24 +109,22 @@ * Update the state - disabled and visibility - of the sub widgets. */ mw.echo.ui.PaginationWidget.prototype.updateWidgetState = function () { - if ( - this.dirSelectWidget && - this.startButton && - this.labelWidget - ) { - this.dirSelectWidget.getItemFromData( 'prev' ) - .setDisabled( this.isDisabled() || !this.model.hasPrevPage() ); - this.dirSelectWidget.getItemFromData( 'next' ) - .setDisabled( this.isDisabled() || !this.model.hasNextPage() ); + this.dirSelectWidget.getItemFromData( 'prev' ) + .setDisabled( this.isDisabled() || !this.model.hasPrevPage() ); + this.dirSelectWidget.getItemFromData( 'next' ) + .setDisabled( this.isDisabled() || !this.model.hasNextPage() ); - this.startButton.toggle( - !this.isDisabled() && - this.model.getCurrPageIndex() >= this.showFirstButtonAfter - ); + this.startButton.toggle( + !this.isDisabled() && + this.model.getCurrPageIndex() >= this.showFirstButtonAfter + ); - this.updateLabel(); - this.labelWidget.toggle( !this.isDisabled() ); - } + // Only show pagination buttons if there's anywhere to go + this.dirSelectWidget.toggle( this.model.hasPrevPage() || this.model.hasNextPage() ); + + // Update label text and visibility + this.updateLabel(); + this.labelWidget.toggle( !this.isDisabled() ); }; /** * Set the 'disabled' state of the widget. @@ -137,7 +135,13 @@ // Parent mw.echo.ui.PaginationWidget.parent.prototype.setDisabled.call( this, disabled ); - this.updateWidgetState(); + if ( + this.dirSelectWidget && + this.startButton && + this.labelWidget + ) { + this.updateWidgetState(); + } return this; }; @@ -147,12 +151,20 @@ * per page, and the number of notifications on the current page. */ mw.echo.ui.PaginationWidget.prototype.updateLabel = function () { - var firstNotifNum = this.model.getCurrPageIndex() * this.itemsPerPage, - lastNotifNum = firstNotifNum + this.itemsPerPage, - label = ( firstNotifNum + 1 ) + ' - ' + lastNotifNum; + var label, + itemsInPage = this.model.hasNextPage() ? + this.itemsPerPage : this.model.getLastPageItemCount(), + firstNotifNum = this.model.getCurrPageIndex() * this.itemsPerPage, + lastNotifNum = firstNotifNum + itemsInPage; - // Display the range + if ( itemsInPage === 0 ) { + label = ''; + } else if ( !this.model.hasPrevPage() && !this.model.hasNextPage() ) { + label = mw.msg( 'echo-specialpage-pagination-numnotifications', itemsInPage ); + } else { + label = mw.msg( 'echo-specialpage-pagination-range', firstNotifNum + 1, lastNotifNum ); + } + this.labelWidget.setLabel( label ); }; - } )( jQuery, mediaWiki ); -- To view, visit https://gerrit.wikimedia.org/r/291851 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I1465f772bb9f5247df645d6612f951e5fd7d38cf Gerrit-PatchSet: 8 Gerrit-Project: mediawiki/extensions/Echo Gerrit-Branch: master Gerrit-Owner: Mooeypoo <mor...@gmail.com> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: Mooeypoo <mor...@gmail.com> Gerrit-Reviewer: Siebrand <siebr...@kitano.nl> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits