Mooeypoo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/298910
Change subject: Relate read-state filter and mark read/unread action
......................................................................
Relate read-state filter and mark read/unread action
When we are viewing a certain read state filter ('read' or 'unread')
the visibility of items should correspond to that state even when
the user marks a specific item as read/unread. That means that the
system should remove these items from view when the action is taken.
In this commit:
* The controller makes the judgment of whether to remove items when
read/unread action is taken, based on whether a filter is set.
* We clean up the terminology of discard - no more 'remove' - to
make sure we have consistency in the code.
* Related: The 'discard' event is now scoped within the hierarchy;
meaning, lists emit 'discard' when an item is removed, grouplist
emits 'discard' when a group is removed, and the manager emits
'discard' when an entire notification model is removed. This
means we can actually have proper hierarchy and organization with
a single event, and not worry about clashing between the intentional
'discard' action and the event 'remove' that is also used while
resorting happens.
* The model manager emits a discard event when a model is removed
so that the general list can listen to the manager and remove an
entire batch of items if needed.
Bug: T136891
Change-Id: I247c618042ef256fadf09922f7b83bd1ad361f64
---
M modules/controller/mw.echo.Controller.js
M modules/model/mw.echo.dm.CrossWikiNotificationItem.js
M modules/model/mw.echo.dm.ModelManager.js
M modules/model/mw.echo.dm.NotificationGroupsList.js
M modules/model/mw.echo.dm.NotificationsList.js
M modules/special/ext.echo.special.js
M modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
M modules/ui/mw.echo.ui.DatedNotificationsWidget.js
M modules/ui/mw.echo.ui.NotificationsListWidget.js
9 files changed, 113 insertions(+), 45 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/10/298910/1
diff --git a/modules/controller/mw.echo.Controller.js
b/modules/controller/mw.echo.Controller.js
index f51091b..63db45b 100644
--- a/modules/controller/mw.echo.Controller.js
+++ b/modules/controller/mw.echo.Controller.js
@@ -466,7 +466,9 @@
* for the set type of this controller, in the given source.
*/
mw.echo.Controller.prototype.markItemsRead = function ( itemIds,
modelName, isRead ) {
- var model = this.manager.getNotificationModel( modelName ),
+ var items,
+ model = this.manager.getNotificationModel( modelName ),
+ readState =
this.manager.getFiltersModel().getReadState(),
allIds = [];
itemIds = Array.isArray( itemIds ) ? itemIds : [ itemIds ];
@@ -474,11 +476,22 @@
// Default to true
isRead = isRead === undefined ? true : isRead;
- model.findByIds( itemIds ).forEach( function ( notification ) {
- allIds = allIds.concat( notification.getAllIds() );
- notification.toggleRead( isRead );
- } );
-
+ items = model.findByIds( itemIds );
+ // If we are only looking at specific read state,
+ // then we need to make sure the items are removed
+ // from the visible list, because they no longer
+ // correspond with the chosen state filter
+ if ( readState === 'read' && !isRead ) {
+ model.discardItems( items );
+ } else if ( readState === 'unread' && isRead ) {
+ model.discardItems( items );
+ } else {
+ // Toggle the read state of the items in the UI
regularly
+ items.forEach( function ( notification ) {
+ allIds = allIds.concat(
notification.getAllIds() );
+ notification.toggleRead( isRead );
+ } );
+ }
this.manager.getUnreadCounter().estimateChange( isRead ?
-allIds.length : allIds.length );
return this.api.markItemsRead( allIds, model.getSource(),
isRead ).then( this.refreshUnreadCount.bind( this ) );
@@ -505,7 +518,7 @@
itemIds = Array.isArray( itemIds ) ? itemIds : [ itemIds ];
- sourceModel = xwikiModel.getList().getGroupBySource( source );
+ sourceModel = xwikiModel.getList().getGroupByName( source );
notifs = sourceModel.findByIds( itemIds );
sourceModel.discardItems( notifs );
diff --git a/modules/model/mw.echo.dm.CrossWikiNotificationItem.js
b/modules/model/mw.echo.dm.CrossWikiNotificationItem.js
index efe131f..b14fa39 100644
--- a/modules/model/mw.echo.dm.CrossWikiNotificationItem.js
+++ b/modules/model/mw.echo.dm.CrossWikiNotificationItem.js
@@ -22,7 +22,7 @@
this.list = new mw.echo.dm.NotificationGroupsList();
- this.list.connect( this, { remove: 'onListRemove' } );
+ this.list.connect( this, { discard: 'onListDiscard' } );
};
OO.inheritClass( mw.echo.dm.CrossWikiNotificationItem,
mw.echo.dm.NotificationItem );
@@ -30,10 +30,10 @@
/* Events */
/**
- * @event removeSource
- * @param {string} name The symbolic name for the source that was
removed
+ * @event discard
+ * @param {string} name The symbolic name for the list model that was
discarded
*
- * Source list has been removed
+ * A sub list has been discarded
*/
/* Methods */
@@ -42,11 +42,10 @@
* Respond to list being removed from the cross-wiki bundle.
*
* @param {mw.echo.dm.NotificationGroupsList} sourceModel The source
model that was removed
- * @fires removeSource
+ * @fires discard
*/
- mw.echo.dm.CrossWikiNotificationItem.prototype.onListRemove = function
( sourceModel ) {
- this.emit( 'removeSource', sourceModel.getName() );
-
+ mw.echo.dm.CrossWikiNotificationItem.prototype.onListDiscard = function
( sourceModel ) {
+ this.emit( 'discard', sourceModel.getName() );
};
/**
@@ -82,7 +81,7 @@
* @return {mw.echo.dm.NotificationGroupsList} Source item
*/
mw.echo.dm.CrossWikiNotificationItem.prototype.getItemBySource =
function ( sourceName ) {
- return this.list.getGroupBySource( sourceName );
+ return this.list.getGroupByName( sourceName );
};
/**
@@ -128,4 +127,8 @@
return true;
};
+ mw.echo.dm.CrossWikiNotificationItem.prototype.isEmpty = function () {
+ return this.getList().isEmpty();
+ };
+
} )( mediaWiki );
diff --git a/modules/model/mw.echo.dm.ModelManager.js
b/modules/model/mw.echo.dm.ModelManager.js
index f119847..8facb76 100644
--- a/modules/model/mw.echo.dm.ModelManager.js
+++ b/modules/model/mw.echo.dm.ModelManager.js
@@ -61,10 +61,10 @@
*/
/**
- * @event remove
- * @param {string} Removed model
+ * @event discard
+ * @param {string} modelId Discard model id
*
- * A model has been removed
+ * A model has been permanently removed
*/
/**
@@ -80,6 +80,20 @@
*/
/* Methods */
+
+ /**
+ * Respond to a notification model discarding items.
+ *
+ * @param {string} modelId Model name
+ */
+ mw.echo.dm.ModelManager.prototype.onModelDiscardItems = function (
modelId ) {
+ var model = this.getNotificationModel( modelId );
+
+ // If the model is empty, remove it
+ if ( model.isEmpty() ) {
+ this.removeNotificationModel( modelId );
+ }
+ };
/**
* Get the notifications
@@ -108,6 +122,7 @@
for ( modelId in modelDefinitions ) {
this.notificationModels[ modelId ] = modelDefinitions[
modelId ];
+ this.notificationModels[ modelId ].connect( this, {
discard: [ 'onModelDiscardItems', modelId ] } );
}
localModel = this.getNotificationModel( 'local' );
@@ -171,7 +186,8 @@
*/
mw.echo.dm.ModelManager.prototype.removeNotificationModel = function (
modelName ) {
delete this.notificationModels[ modelName ];
- this.emit( 'remove', modelName );
+
+ this.emit( 'discard', modelName );
};
/**
diff --git a/modules/model/mw.echo.dm.NotificationGroupsList.js
b/modules/model/mw.echo.dm.NotificationGroupsList.js
index 9cf4090..6fdbeb2 100644
--- a/modules/model/mw.echo.dm.NotificationGroupsList.js
+++ b/modules/model/mw.echo.dm.NotificationGroupsList.js
@@ -30,32 +30,40 @@
}
// Fallback on Source
- return b.getSource() - a.getSource();
+ return b.getName() - a.getName();
} );
this.foreign = !!config.foreign;
this.groups = {};
- this.aggregate( { remove: 'groupRemoveItem' } );
- this.connect( this, { groupRemoveItem: 'onGroupRemoveItem' } );
+ this.aggregate( { discard: 'groupDiscardItem' } );
+ this.connect( this, { groupDiscardItem: 'onGroupDiscardItem' }
);
};
/* Initialization */
OO.inheritClass( mw.echo.dm.NotificationGroupsList,
mw.echo.dm.SortedList );
+ /* Events */
+
+ /**
+ * @event discard
+ *
+ * A group was permanently removed
+ */
+
/* Methods */
/**
- * Handle a remove event from any list.
- * This means that one of the sources has removed an item.
+ * Handle a discard event from any list.
+ * This means that one of the sources has discarded an item.
*
* @param {mw.echo.dm.NotificationsList} groupList List source model
for the item
*/
- mw.echo.dm.NotificationGroupsList.prototype.onGroupRemoveItem =
function ( groupList ) {
+ mw.echo.dm.NotificationGroupsList.prototype.onGroupDiscardItem =
function ( groupList ) {
// Check if the list has anything at all
if ( groupList.isEmpty() ) {
// Remove it
- this.removeItems( [ groupList ] );
+ this.removeGroup( groupList.getName() );
}
};
@@ -108,7 +116,7 @@
* @param {mw.echo.dm.NotificationItem[]} groupItems Items to add to
this group
*/
mw.echo.dm.NotificationGroupsList.prototype.addItemsToGroup = function
( groupSource, groupItems ) {
- var group = this.getGroupBySource( groupSource );
+ var group = this.getGroupByName( groupSource );
if ( group ) {
group.addItems( groupItems );
@@ -118,28 +126,36 @@
* Remove a group from the list. This is an easier to use alias
* to 'removeItems()' method.
*
- * @param {string} groupSource Group source name
+ * Since this is an intentional action, we fire 'discard' event.
+ * The main reason for this is that the event 'remove' is a general
+ * event that is fired both when a user intends on removing an
+ * item and also when an item is temporarily removed to be re-added
+ * for the sake of sorting. To avoid ambiguity, we use 'discard' event.
+ *
+ * @param {string} groupName Group name
+ * @fires discard
*/
- mw.echo.dm.NotificationGroupsList.prototype.removeGroup = function (
groupSource ) {
- var group = this.getGroupBySource( groupSource );
+ mw.echo.dm.NotificationGroupsList.prototype.removeGroup = function (
groupName ) {
+ var group = this.getGroupByName( groupName );
if ( group ) {
this.removeItems( group );
+ this.emit( 'discard', group );
}
};
/**
* Get a group by its source identifier.
*
- * @param {string} groupSource Group source
+ * @param {string} groupName Group name
* @return {mw.echo.dm.NotificationsList|null} Requested group, null if
none was found.
*/
- mw.echo.dm.NotificationGroupsList.prototype.getGroupBySource = function
( groupSource ) {
+ mw.echo.dm.NotificationGroupsList.prototype.getGroupByName = function (
groupName ) {
var i,
items = this.getItems();
for ( i = 0; i < items.length; i++ ) {
- if ( items[ i ].getSource() === groupSource ) {
+ if ( items[ i ].getName() === groupName ) {
return items[ i ];
}
}
diff --git a/modules/model/mw.echo.dm.NotificationsList.js
b/modules/model/mw.echo.dm.NotificationsList.js
index 3034149..2191910 100644
--- a/modules/model/mw.echo.dm.NotificationsList.js
+++ b/modules/model/mw.echo.dm.NotificationsList.js
@@ -79,6 +79,13 @@
* An item in the list has been updated
*/
+ /**
+ * @event discard
+ * @param {mw.echo.dm.NotificationItem} item Item that was discarded
+ *
+ * An item was discarded
+ */
+
/* Methods */
/**
@@ -107,6 +114,10 @@
mw.echo.dm.NotificationsList.prototype.discardItems = function ( items
) {
this.removeItems( items );
this.emit( 'discard', items );
+
+ if ( this.isEmpty() ) {
+ this.emit( 'empty' );
+ }
};
/**
diff --git a/modules/special/ext.echo.special.js
b/modules/special/ext.echo.special.js
index 2374c9d..615719e 100644
--- a/modules/special/ext.echo.special.js
+++ b/modules/special/ext.echo.special.js
@@ -11,10 +11,7 @@
modelManager = new mw.echo.dm.ModelManager(
unreadCounter, { type: [ 'message', 'alert' ] } ),
controller = new mw.echo.Controller(
echoApi,
- modelManager,
- {
- type: [ 'message' ]
- }
+ modelManager
),
specialPageContainer = new
mw.echo.ui.NotificationsInboxWidget(
controller,
diff --git a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
index 918e94d..072a123 100644
--- a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
@@ -82,7 +82,7 @@
this.actionsButtonSelectWidget.addItems( [
this.toggleExpandButton ] );
// Events
- this.model.connect( this, { removeSource: 'onModelRemoveSource'
} );
+ this.model.connect( this, { discard: 'onModelDiscard' } );
this.toggleExpandButton.connect( this, { click: 'expand' } );
this.$content.on( 'click', this.expand.bind( this ) );
@@ -120,11 +120,11 @@
/**
* Respond to model removing source group
*
- * @param {string} source Symbolic name of the source group
+ * @param {string} groupName Symbolic name of the group
*/
-
mw.echo.ui.CrossWikiNotificationItemWidget.prototype.onModelRemoveSource =
function ( source ) {
+ mw.echo.ui.CrossWikiNotificationItemWidget.prototype.onModelDiscard =
function ( groupName ) {
var list = this.getList(),
- group = list.getItemFromId( source );
+ group = list.getItemFromId( groupName );
list.removeItems( [ group ] );
diff --git a/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
b/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
index 0755cad..ea3da89 100644
--- a/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
+++ b/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
@@ -42,7 +42,7 @@
// Events
this.manager.connect( this, {
update: 'populateFromModel',
- removeSource: 'onModelRemoveSource'
+ discard: 'onManagerDiscardModel'
} );
this.$element
@@ -57,6 +57,16 @@
OO.inheritClass( mw.echo.ui.DatedNotificationsWidget, OO.ui.Widget );
OO.mixinClass( mw.echo.ui.DatedNotificationsWidget,
OO.ui.mixin.PendingElement );
+ mw.echo.ui.DatedNotificationsWidget.prototype.onManagerDiscardModel =
function ( modelId ) {
+ var group,
+ model = this.models[ modelId ],
+ list = this.getList();
+
+ if ( model ) {
+ group = list.getItemFromId( model.getName() );
+ list.removeItems( [ group ] );
+ }
+ };
/**
* Respond to model removing source group
*
@@ -94,6 +104,8 @@
$overlay: this.$overlay
}
);
+ this.attachModel( model, models[ model ] );
+
subgroupWidget.resetItemsFromModel();
groupWidgets.push( subgroupWidget );
}
diff --git a/modules/ui/mw.echo.ui.NotificationsListWidget.js
b/modules/ui/mw.echo.ui.NotificationsListWidget.js
index 89df922..f273ec7 100644
--- a/modules/ui/mw.echo.ui.NotificationsListWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationsListWidget.js
@@ -59,7 +59,7 @@
this.manager.connect( this, {
update: 'resetDataFromModel',
- remove: 'onModelManagerRemove'
+ discard: 'onModelManagerDiscard'
} );
this.$element
@@ -72,7 +72,7 @@
/* Methods */
- mw.echo.ui.NotificationsListWidget.prototype.onModelManagerRemove =
function ( modelName ) {
+ mw.echo.ui.NotificationsListWidget.prototype.onModelManagerDiscard =
function ( modelName ) {
var i,
items = this.getItems();
--
To view, visit https://gerrit.wikimedia.org/r/298910
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I247c618042ef256fadf09922f7b83bd1ad361f64
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <mor...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
