jenkins-bot has submitted this change and it was merged. Change subject: Generalize the SubGroupListWidget ......................................................................
Generalize the SubGroupListWidget Allow for the widget to have a mark-as-read button to its individual groups, as well as change the event listening from 'remove' to 'discard' The problem with 'remove' event is that it is triggered when an item is either intentionally removed from the list *and* when an item is changing its position in the list (move event includes 'remove' and then 'add' event) If we listen to 'remove' events we will get both cases, which is unhelpful. Instead, a new event - 'discard' - was introduced so we are certain it is used with the clear intention of removing the item completely. Change-Id: Ia08720bf4c547fa41edf62331eeb1a45ff4965b7 --- M modules/controller/mw.echo.Controller.js M modules/model/mw.echo.dm.NotificationItem.js M modules/model/mw.echo.dm.NotificationsList.js M modules/styles/mw.echo.ui.CrossWikiNotificationItemWidget.less M modules/styles/mw.echo.ui.SubGroupListWidget.less M modules/ui/mw.echo.ui.SubGroupListWidget.js 6 files changed, 186 insertions(+), 31 deletions(-) Approvals: Catrope: Looks good to me, approved jenkins-bot: Verified diff --git a/modules/controller/mw.echo.Controller.js b/modules/controller/mw.echo.Controller.js index 41f6a06..adbd5ed 100644 --- a/modules/controller/mw.echo.Controller.js +++ b/modules/controller/mw.echo.Controller.js @@ -202,7 +202,7 @@ items.push( new mw.echo.dm.NotificationItem( groupItems[ i ].id, $.extend( notifData, { source: group, - bundle: true, + bundled: true, foreign: true } ) ) ); @@ -293,7 +293,7 @@ sourceModel = xwikiModel.getList().getGroupBySource( modelSource ); notifs = sourceModel.findByIds( itemIds ); - sourceModel.removeItems( notifs ); + sourceModel.discardItems( notifs ); return this.api.markItemsRead( itemIds, modelSource, true ) .then( this.refreshUnreadCount.bind( this ) ); diff --git a/modules/model/mw.echo.dm.NotificationItem.js b/modules/model/mw.echo.dm.NotificationItem.js index 9cfa7d9..06ab66e 100644 --- a/modules/model/mw.echo.dm.NotificationItem.js +++ b/modules/model/mw.echo.dm.NotificationItem.js @@ -23,6 +23,7 @@ * @cfg {string} [timestamp] Notification timestamp in Mediawiki timestamp format * @cfg {string} [primaryUrl] Notification primary link in raw url format * @cfg {boolean} [foreign=false] This notification is from a foreign source + * @cfg {boolean} [bundled=false] This notification is part of a bundle * @cfg {string} [source] The source this notification is coming from, if it is foreign * @cfg {Object[]} [secondaryUrls] An array of objects defining the secondary URLs * for this notification. The secondary URLs are expected to have this structure: @@ -58,6 +59,7 @@ this.category = config.category || ''; this.type = config.type || 'message'; this.foreign = !!config.foreign; + this.bundled = !!config.bundled; this.source = config.source || ''; this.iconType = config.iconType; this.iconURL = config.iconURL; @@ -158,6 +160,15 @@ }; /** + * Check whether this notification item is part of a bundle + * + * @return {boolean} Notification item is part of a bundle + */ + mw.echo.dm.NotificationItem.prototype.isBundled = function () { + return this.bundled; + }; + + /** * Set this notification item as foreign * * @param {boolean} isForeign Notification item is foreign diff --git a/modules/model/mw.echo.dm.NotificationsList.js b/modules/model/mw.echo.dm.NotificationsList.js index faa0f1f..3fcc293 100644 --- a/modules/model/mw.echo.dm.NotificationsList.js +++ b/modules/model/mw.echo.dm.NotificationsList.js @@ -72,13 +72,29 @@ /** * Set the items in this list * - * @param {mw.echo.dm.NotificationItem} items Items to insert into the list + * @param {mw.echo.dm.NotificationItem[]} items Items to insert into the list * @fires update */ mw.echo.dm.NotificationsList.prototype.setItems = function ( items ) { this.clearItems(); this.addItems( items ); this.emit( 'update', this.getItems() ); + }; + + /** + * Discard items from the list. + * + * This is a more precise operation than 'removeItems' because when + * the list is resorting the position of a single item, it removes + * the item and reinserts it, which makes the 'remove' event unhelpful + * to differentiate between actually discarding items, and only + * temporarily moving them. + * + * @param {mw.echo.dm.NotificationItem[]} items Items to insert into the list + */ + mw.echo.dm.NotificationsList.prototype.discardItems = function ( items ) { + this.removeItems( items ); + this.emit( 'discard', items ); }; /** @@ -176,4 +192,5 @@ mw.echo.dm.NotificationsList.prototype.isGroup = function () { return false; }; + } )( mediaWiki ); diff --git a/modules/styles/mw.echo.ui.CrossWikiNotificationItemWidget.less b/modules/styles/mw.echo.ui.CrossWikiNotificationItemWidget.less index 5d48b5e..f979a17 100644 --- a/modules/styles/mw.echo.ui.CrossWikiNotificationItemWidget.less +++ b/modules/styles/mw.echo.ui.CrossWikiNotificationItemWidget.less @@ -37,4 +37,15 @@ border-bottom: 1px #dddddd solid; margin-bottom: 0.4em; } + + .mw-echo-ui-subGroupListWidget-header { + margin-bottom: @bundle-group-padding; + + &-row-title { + // Override OOUI's line height for labels + line-height: 1em !important; + font-weight: bold; + color: #666666; + } + } } diff --git a/modules/styles/mw.echo.ui.SubGroupListWidget.less b/modules/styles/mw.echo.ui.SubGroupListWidget.less index bfa6eaa..95035eb 100644 --- a/modules/styles/mw.echo.ui.SubGroupListWidget.less +++ b/modules/styles/mw.echo.ui.SubGroupListWidget.less @@ -6,12 +6,25 @@ padding-top: @bundle-group-padding; } - &-title { - // Override OOUI's line height for labels - line-height: 1em !important; - font-weight: bold; - color: #666666; - margin-bottom: @bundle-group-padding; + &-header { + display: table; + width: 100%; + + &-row { + display: table-row; + + &-title { + display: table-cell; + width: 100%; + vertical-align: bottom; + } + + &-markAllReadButton { + display: table-cell; + text-align: right; + padding-bottom: 0.5em; + } + } } .mw-echo-ui-sortedListWidget { diff --git a/modules/ui/mw.echo.ui.SubGroupListWidget.js b/modules/ui/mw.echo.ui.SubGroupListWidget.js index 74364bf..c823fba 100644 --- a/modules/ui/mw.echo.ui.SubGroupListWidget.js +++ b/modules/ui/mw.echo.ui.SubGroupListWidget.js @@ -8,11 +8,14 @@ * @param {mw.echo.dm.SortedList} listModel Notifications list model for this source * @param {Object} config Configuration object * @cfg {boolean} [showTitle=false] Show the title of this group + * @cfg {boolean} [showMarkAllRead=false] Show a mark all read button for this group * @cfg {jQuery} [$overlay] A jQuery element functioning as an overlay * for popups. */ mw.echo.ui.SubGroupListWidget = function MwEchoUiSubGroupListWidget( controller, listModel, config ) { - var sourceURL; + var sourceURL, + $header = $( '<div>' ) + .addClass( 'mw-echo-ui-subGroupListWidget-header' ); config = config || {}; @@ -23,6 +26,7 @@ mw.echo.ui.SubGroupListWidget.parent.call( this, $.extend( { data: this.getSource() }, config ) ); this.showTitle = !!config.showTitle; + this.showMarkAllRead = !!config.showMarkAllRead; this.$overlay = config.$overlay || this.$element; this.listWidget = new mw.echo.ui.SortedListWidget( @@ -45,28 +49,60 @@ sourceURL = this.model.getSourceURL() ? this.model.getSourceURL().replace( '$1', 'Special:Notifications' ) : null; - this.title = new OO.ui.ButtonWidget( { - framed: false, - classes: [ 'mw-echo-ui-subGroupListWidget-title' ], - href: sourceURL - } ); + if ( sourceURL ) { + this.title = new OO.ui.ButtonWidget( { + framed: false, + classes: [ 'mw-echo-ui-subGroupListWidget-header-row-title' ], + href: sourceURL + } ); + } else { + this.title = new OO.ui.LabelWidget( { + classes: [ 'mw-echo-ui-subGroupListWidget-header-row-title' ] + } ); + } + if ( this.model.getTitle() ) { this.title.setLabel( this.model.getTitle() ); } this.title.toggle( this.showTitle ); - // Events - this.model.connect( this, { - // We really only need to listen to 'remove' item here - // There is no other update event worthwhile in this list. - remove: 'onModelRemoveItem', - update: 'onModelUpdate' // Adding all items + // Mark all as read button + this.markAllReadButton = new OO.ui.ButtonWidget( { + framed: true, + label: mw.msg( 'echo-mark-all-as-read' ), + classes: [ 'mw-echo-ui-subGroupListWidget-header-row-markAllReadButton' ] } ); + // Events + this.model.connect( this, { + // Cross-wiki items can be discarded when marked as read. + // We need to differentiate this explicit action from the + // action of 'remove' because 'remove' is also used when + // an item is resorted by OO.SortedEmitterWidget before + // it is re-added again + discard: 'onModelDiscardItems', + // Update all items + update: 'resetItemsFromModel' + } ); + this.markAllReadButton.connect( this, { click: 'onMarkAllReadButtonClick' } ); + // We must aggregate on item update, so we know when and if all + // items are read and can hide/show the 'mark all read' button + this.model.aggregate( { update: 'itemUpdate' } ); + this.model.connect( this, { itemUpdate: 'toggleMarkAllReadButton' } ); + + // Initialize + this.toggleMarkAllReadButton(); this.$element .addClass( 'mw-echo-ui-subGroupListWidget' ) .append( - this.title.$element, + $header.append( + $( '<div>' ) + .addClass( 'mw-echo-ui-subGroupListWidget-header-row' ) + .append( + this.title.$element, + this.markAllReadButton.$element + ) + ), this.listWidget.$element ); }; @@ -78,13 +114,45 @@ /* Methods */ /** - * Respond to model update event - * - * @param {mw.echo.dm.NotificationItem[]} items Item models that are added + * Toggle the visibility of the mark all read button for this group + * based on whether there are unread notifications */ - mw.echo.ui.SubGroupListWidget.prototype.onModelUpdate = function ( items ) { + mw.echo.ui.SubGroupListWidget.prototype.toggleMarkAllReadButton = function () { + this.markAllReadButton.toggle( this.hasUnread() ); + }; + + /** + * Respond to 'mark all as read' button click + */ + mw.echo.ui.SubGroupListWidget.prototype.onMarkAllReadButtonClick = function () { + this.controller.markEntireListModelRead( this.model.getSource() ); + }; + + /** + * Check whether this sub group list has any unread notifications + * + * @return {boolean} Sub group has unread notifications + */ + mw.echo.ui.SubGroupListWidget.prototype.hasUnread = function () { + var isUnread = function ( item ) { + return !item.isRead(); + }, + items = this.model.getItems(); + + return items.some( isUnread ); + }; + + /** + * Reset the items and rebuild them according to the model. + * + * @param {mw.echo.dm.NotificationItem[]} [items] Item models that are added. + * If this is empty, the widget will request all the items from the model. + */ + mw.echo.ui.SubGroupListWidget.prototype.resetItemsFromModel = function ( items ) { var i, itemWidgets = []; + + items = items || this.model.getItems(); for ( i = 0; i < items.length; i++ ) { itemWidgets.push( @@ -93,7 +161,7 @@ items[ i ], { $overlay: this.$overlay, - bundle: true + bundle: items[ i ].isBundled() } ) ); @@ -106,13 +174,19 @@ }; /** - * Respond to mode remove event. This may happen when an item + * Respond to model remove event. This may happen when an item * is marked as read. * - * @param {mw.echo.dm.NotificationItem} item Notification item model + * @param {mw.echo.dm.NotificationItem[]} items Notification item models */ - mw.echo.ui.SubGroupListWidget.prototype.onModelRemoveItem = function ( item ) { - this.listWidget.removeItems( [ this.listWidget.getItemFromId( item.getId() ) ] ); + mw.echo.ui.SubGroupListWidget.prototype.onModelDiscardItems = function ( items ) { + var i, + itemWidgets = []; + + for ( i = 0; i < items.length; i++ ) { + itemWidgets.push( this.listWidget.getItemFromId( items[ i ].getId() ) ); + } + this.listWidget.removeItems( itemWidgets ); }; /** @@ -158,6 +232,35 @@ }; /** + * Get an array of IDs of all of the items in this group + * + * @return {number[]} Array of item IDs + */ + mw.echo.ui.SubGroupListWidget.prototype.getAllItemIDs = function () { + return this.model.getAllItemIds(); + }; + + /** + * Get an array of IDs of all of the items in this group that + * correspond to a specific type + * + * @param {string} type Item type + * @return {number[]} Array of item IDs + */ + mw.echo.ui.SubGroupListWidget.prototype.getAllItemIDsByType = function ( type ) { + return this.model.getAllItemIdsByType( type ); + }; + + /** + * Check whether this group is foreign + * + * @return {boolean} This group is foreign + */ + mw.echo.ui.SubGroupListWidget.prototype.isForeign = function () { + return this.model.isForeign(); + }; + + /** * Get the group id, which is represented by its source. * This is meant for sorting callbacks that fallback on * sorting by IDs. -- To view, visit https://gerrit.wikimedia.org/r/289738 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ia08720bf4c547fa41edf62331eeb1a45ff4965b7 Gerrit-PatchSet: 5 Gerrit-Project: mediawiki/extensions/Echo Gerrit-Branch: master Gerrit-Owner: Mooeypoo <mor...@gmail.com> Gerrit-Reviewer: Catrope <roan.katt...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits