Mooeypoo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/249503
Change subject: Implement SortedList in Echo notifications
......................................................................
Implement SortedList in Echo notifications
This is especially important for combined notifications and
notification lists from different sources; the model list should
be sorted to reflect items by timestamp and unread status.
Note: The dm.List and dm.SortedList now mirror the structures
OO.EmitterList and OO.SortedEmitterList that are awaiting to be
added for oojs in Ib94e4e4a49 and I3fd569691549 respectively.
Once those are available, the dm.List and dm.SortedList can
be removed, and the model can mixin OO.SortedEmitterList instead.
Change-Id: I97e1ecbe5dccc478be527a94f037500f78f74b14
---
M Resources.php
M modules/ooui/mw.echo.ui.NotificationsWidget.js
M modules/viewmodel/mw.echo.dm.List.js
M modules/viewmodel/mw.echo.dm.NotificationsModel.js
A modules/viewmodel/mw.echo.dm.SortedList.js
5 files changed, 277 insertions(+), 99 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/03/249503/1
diff --git a/Resources.php b/Resources.php
index 131c1bd..c19323c 100644
--- a/Resources.php
+++ b/Resources.php
@@ -90,6 +90,7 @@
'viewmodel/mw.echo.dm.AbstractAPIHandler.js',
'viewmodel/mw.echo.dm.APIHandler.js',
'viewmodel/mw.echo.dm.List.js',
+ 'viewmodel/mw.echo.dm.SortedList.js',
'viewmodel/mw.echo.dm.NotificationList.js',
'viewmodel/mw.echo.dm.NotificationsModel.js',
),
diff --git a/modules/ooui/mw.echo.ui.NotificationsWidget.js
b/modules/ooui/mw.echo.ui.NotificationsWidget.js
index 435c57f..b711bdf 100644
--- a/modules/ooui/mw.echo.ui.NotificationsWidget.js
+++ b/modules/ooui/mw.echo.ui.NotificationsWidget.js
@@ -45,32 +45,23 @@
/**
* Respond to model add event
*
- * @param {mw.echo.dm.NotificationItem[]} Added notification items
+ * @param {mw.echo.dm.NotificationItem} Added notification item
*/
- mw.echo.ui.NotificationsWidget.prototype.onModelNotificationAdd =
function ( notificationItems, index ) {
- var i, len, widget,
- $elements = $(),
- optionWidgets = [];
-
- for ( i = 0, len = notificationItems.length; i < len; i++ ) {
- widget = new mw.echo.ui.NotificationOptionWidget(
- notificationItems[ i ],
+ mw.echo.ui.NotificationsWidget.prototype.onModelNotificationAdd =
function ( notificationItem, index ) {
+ var widget = new mw.echo.ui.NotificationOptionWidget(
+ notificationItem,
{
markReadWhenSeen: this.markReadWhenSeen
}
);
- optionWidgets.push( widget );
- // Collect the elements for the hook firing
- $elements = $elements.add( widget.$element );
- }
// Fire hook for gadgets to update the option list
- mw.hook( 'ext.echo.overlay.beforeShowingOverlay' ).fire(
$elements );
+ mw.hook( 'ext.echo.overlay.beforeShowingOverlay' ).fire(
widget.$element );
// Remove dummy option
this.removeItems( [ this.loadingOptionWidget ] );
- this.addItems( optionWidgets, index );
+ this.addItems( [ widget ], index );
};
/**
diff --git a/modules/viewmodel/mw.echo.dm.List.js
b/modules/viewmodel/mw.echo.dm.List.js
index 89d91f3..819025a 100644
--- a/modules/viewmodel/mw.echo.dm.List.js
+++ b/modules/viewmodel/mw.echo.dm.List.js
@@ -22,18 +22,31 @@
/* Events */
/**
- * @event add Items have been added
- * @param {mw.echo.dm.NotificationItem[]} items Added items
+ * Item has been added
+ *
+ * @event add
+ * @param {OO.EventEmitter} item Added item
* @param {number} index Index items were added at
*/
/**
- * @event remove Items have been removed
- * @param {mw.echo.dm.NotificationItem[]} items Removed items
+ * Item has been moved to a new index
+ *
+ * @event move
+ * @param {OO.EventEmitter} item Moved item
+ * @param {number} index Index item was moved to
*/
/**
- * @event clear All items have been removed
+ * Item has been removed
+ *
+ * @event remove
+ * @param {OO.EventEmitter} item Removed item
+ * @param {number} index Index the item was removed from
+ */
+
+ /**
+ * @event clear The list has been cleared of items
*/
/* Methods */
@@ -100,7 +113,7 @@
* @throws {Error} An error is thrown if aggregation already exists.
*/
mw.echo.dm.List.prototype.aggregate = function ( events ) {
- var i, len, item, add, remove, itemEvent, groupEvent;
+ var i, item, add, remove, itemEvent, groupEvent;
for ( itemEvent in events ) {
groupEvent = events[ itemEvent ];
@@ -112,7 +125,7 @@
throw new Error( 'Duplicate item event
aggregation for ' + itemEvent );
}
// Remove event aggregation from existing items
- for ( i = 0, len = this.items.length; i < len;
i++ ) {
+ for ( i = 0; i < this.items.length; i++ ) {
item = this.items[ i ];
if ( item.connect && item.disconnect ) {
remove = {};
@@ -129,7 +142,7 @@
// Make future items aggregate event
this.aggregateItemEvents[ itemEvent ] =
groupEvent;
// Add event aggregation to existing items
- for ( i = 0, len = this.items.length; i < len;
i++ ) {
+ for ( i = 0; i < this.items.length; i++ ) {
item = this.items[ i ];
if ( item.connect && item.disconnect ) {
add = {};
@@ -141,62 +154,118 @@
}
};
+
/**
- * Add items
+ * Add items.
*
- * @param {mw.echo.dm.NotificationItem[]} items Items to add
- * @param {number} index Index to add items at
+ * @param {mw.echo.dm.NotificationItem|mw.echo.dm.NotificationItem[]}
items Item to add or
+ * an array of items to add
+ * @param {number} [index] Index to add items at. If no index is
+ * given, or if the index that is given is invalid, the item
+ * will be added at the end of the list.
* @chainable
* @fires add
+ * @fires move
*/
mw.echo.dm.List.prototype.addItems = function ( items, index ) {
- var i, len, item, event, events, currentIndex, existingItem, at;
+ var i;
+
+ if ( !Array.isArray( items ) ) {
+ items = [ items ];
+ }
if ( items.length === 0 ) {
return this;
}
- // Support adding existing items at new locations
- for ( i = 0, len = items.length; i < len; i++ ) {
- item = items[ i ];
- existingItem = this.getItemById( item.getId() );
-
- // Check if item exists then remove it first,
effectively "moving" it
- currentIndex = this.items.indexOf( existingItem );
- if ( currentIndex >= 0 ) {
- this.removeItems( [ existingItem ] );
- // Adjust index to compensate for removal
- if ( currentIndex < index ) {
- index--;
- }
+ index = this.normalizeIndex( index );
+ for ( i = 0; i < items.length; i++ ) {
+ if ( this.items.indexOf( items[ i ] ) !== -1 ) {
+ // Move item to new index
+ index = this.moveItem( items[ i ], index );
+ this.emit( 'move', items[ i ], index );
+ } else {
+ // insert item at index
+ index = this.insertItem( items[ i ], index );
+ this.emit( 'add', items[ i ], index );
}
-
- // Add the item
- if ( item.connect && item.disconnect &&
!$.isEmptyObject( this.aggregateItemEvents ) ) {
- events = {};
- for ( event in this.aggregateItemEvents ) {
- events[ event ] = [ 'emit',
this.aggregateItemEvents[ event ], item ];
- }
- item.connect( this, events );
- }
-
- // Add by reference
- this.itemsById[ item.getId() ] = items[ i ];
+ index++;
}
-
- if ( index === undefined || index < 0 || index >=
this.items.length ) {
- at = this.items.length;
- this.items.push.apply( this.items, items );
- } else if ( index === 0 ) {
- at = 0;
- this.items.unshift.apply( this.items, items );
- } else {
- at = index;
- this.items.splice.apply( this.items, [ index, 0
].concat( items ) );
- }
- this.emit( 'add', items, at );
return this;
+ };
+
+ /**
+ * Move an item from its current position to a new index.
+ *
+ * @param {mw.echo.dm.NotificationItem} item Items to add
+ * @param {number} newIndex Index to move the item to
+ * @private
+ * @return {number} The index the item was moved to
+ */
+ mw.echo.dm.List.prototype.moveItem = function ( item, newIndex ) {
+ var existingIndex = this.items.indexOf( item );
+
+ newIndex = this.normalizeIndex( newIndex );
+
+ if ( existingIndex === -1 ) {
+ return this;
+ }
+
+ // Remove the item from the current index
+ this.items.splice( existingIndex, 1 );
+
+ // Adjust new index after removal
+ newIndex--;
+
+ // Move the item to the new index
+ this.items.splice( newIndex, 0, item );
+
+ return newIndex;
+ };
+
+ /**
+ * Normalize requested index to fit into the array.
+ *
+ * @private
+ * @param {number} index Requested index
+ * @return {number} Normalized index
+ */
+ mw.echo.dm.List.prototype.normalizeIndex = function ( index ) {
+ return ( index === undefined || index < 0 || index >=
this.items.length ) ?
+ this.items.length :
+ index;
+ };
+
+ /**
+ * Utility method to insert an item into the list, and
+ * connect it to aggregate events.
+ *
+ * Don't call this directly unless you know what you're doing.
+ * Use #addItems instead.
+ *
+ * @param {mw.echo.dm.NotificationItem} item Items to add
+ * @param {number} index Index to add items at
+ * @private
+ * @return {number} The index the item was added at
+ */
+ mw.echo.dm.List.prototype.insertItem = function ( item, index ) {
+ var events, event;
+
+ // Add the item to event aggregation
+ if ( item.connect && item.disconnect ) {
+ events = {};
+ for ( event in this.aggregateItemEvents ) {
+ events[ event ] = [ 'emit',
this.aggregateItemEvents[ event ], item ];
+ }
+ item.connect( this, events );
+ }
+
+ index = this.normalizeIndex( index );
+
+ // Insert into items array
+ this.items.splice( index, 0, item );
+ return index;
};
/**
@@ -207,33 +276,29 @@
* @fires remove
*/
mw.echo.dm.List.prototype.removeItems = function ( items ) {
- var i, len, item, index, remove, itemEvent,
- removed = [];
+ var i, item, index;
+
+ if ( !Array.isArray( items ) ) {
+ items = [ items ];
+ }
if ( items.length === 0 ) {
return this;
}
// Remove specific items
- for ( i = 0, len = items.length; i < len; i++ ) {
+ for ( i = 0; i < items.length; i++ ) {
item = items[ i ];
index = this.items.indexOf( item );
if ( index !== -1 ) {
- if (
- item.connect && item.disconnect &&
!$.isEmptyObject( this.aggregateItemEvents )
- ) {
- remove = {};
- if (
Object.prototype.hasOwnProperty.call( this.aggregateItemEvents, itemEvent ) ) {
- remove[ itemEvent ] = [ 'emit',
this.aggregateItemEvents[ itemEvent ], item ];
- }
- item.disconnect( this, remove );
+ if ( item.connect && item.disconnect ) {
+ // Disconnect all listeners from the
item
+ item.disconnect( this );
}
this.items.splice( index, 1 );
- // Remove reference by Id
- delete this.itemsById[ item.getId() ];
+ this.emit( 'remove', item, index );
}
}
- this.emit( 'remove', removed );
return this;
};
@@ -244,24 +309,16 @@
* @fires clear
*/
mw.echo.dm.List.prototype.clearItems = function () {
- var i, len, item, remove, itemEvent;
+ var i, item,
+ items = this.items.splice( 0, this.items.length );
// Remove all items
- for ( i = 0, len = this.items.length; i < len; i++ ) {
- item = this.items[ i ];
- if (
- item.connect && item.disconnect &&
!$.isEmptyObject( this.aggregateItemEvents )
- ) {
- remove = {};
- if ( Object.prototype.hasOwnProperty.call(
this.aggregateItemEvents, itemEvent ) ) {
- remove[ itemEvent ] = [ 'emit',
this.aggregateItemEvents[ itemEvent ], item ];
- }
- item.disconnect( this, remove );
+ for ( i = 0; i < items.length; i++ ) {
+ item = items[ i ];
+ if ( item.connect && item.disconnect ) {
+ item.disconnect( this );
}
}
-
- this.items = [];
- this.itemsById = {};
this.emit( 'clear' );
diff --git a/modules/viewmodel/mw.echo.dm.NotificationsModel.js
b/modules/viewmodel/mw.echo.dm.NotificationsModel.js
index 53046a2..aaf84d4 100644
--- a/modules/viewmodel/mw.echo.dm.NotificationsModel.js
+++ b/modules/viewmodel/mw.echo.dm.NotificationsModel.js
@@ -20,7 +20,7 @@
OO.EventEmitter.call( this );
// Mixin constructor
- mw.echo.dm.List.call( this );
+ mw.echo.dm.SortedList.call( this );
this.type = config.type || 'alert';
@@ -42,13 +42,23 @@
itemSeen: 'onItemSeen',
itemRead: 'onItemRead'
} );
+
+ this.setSortingCallback( function ( a, b ) {
+ if ( !a.isRead() && b.isRead() ) {
+ return -1; // Unread items are always above
read items
+ } else if ( a.isRead() && !b.isRead() ) {
+ return 1;
+ } else {
+ return a.getTimestamp() - b.getTimestamp();
+ }
+ } );
};
/* Initialization */
OO.initClass( mw.echo.dm.NotificationsModel );
OO.mixinClass( mw.echo.dm.NotificationsModel, OO.EventEmitter );
- OO.mixinClass( mw.echo.dm.NotificationsModel, mw.echo.dm.List );
+ OO.mixinClass( mw.echo.dm.NotificationsModel, mw.echo.dm.SortedList );
/* Events */
@@ -341,9 +351,8 @@
* Update the unread and unseen tracking lists when we add items
*
* @param {mw.echo.dm.NotificationItem[]} items Items to add
- * @param {number} index Index to add items at
*/
- mw.echo.dm.NotificationsModel.prototype.addItems = function ( items,
index ) {
+ mw.echo.dm.NotificationsModel.prototype.addItems = function ( items ) {
var i, len;
for ( i = 0, len = items.length; i < len; i++ ) {
@@ -356,7 +365,7 @@
}
// Parent
- mw.echo.dm.List.prototype.addItems.call( this, items, index );
+ mw.echo.dm.SortedList.prototype.addItems.call( this, items );
};
/**
@@ -374,7 +383,7 @@
}
// Parent
- mw.echo.dm.List.prototype.removeItems.call( this, items );
+ mw.echo.dm.SortedList.prototype.removeItems.call( this, items );
};
/**
@@ -385,7 +394,7 @@
this.unseenNotifications.clearItems();
// Parent
- mw.echo.dm.List.prototype.clearItems.call( this );
+ mw.echo.dm.SortedList.prototype.clearItems.call( this );
};
/**
diff --git a/modules/viewmodel/mw.echo.dm.SortedList.js
b/modules/viewmodel/mw.echo.dm.SortedList.js
new file mode 100644
index 0000000..6496e4f
--- /dev/null
+++ b/modules/viewmodel/mw.echo.dm.SortedList.js
@@ -0,0 +1,120 @@
+( function ( mw, oo ) {
+ /**
+ * @class mw.echo.dm.SortedList
+ * Contains and a sorted mw.echo.dm.List
+ *
+ * @constructor
+ */
+ mw.echo.dm.SortedList = function OoSortedEmitterList() {
+ // Mixin constructors
+ mw.echo.dm.List.call( this );
+
+ this.setSortingCallback( function ( a, b ) {
+ return a.getTimestamp() - b.getTimestamp();
+ } );
+ };
+
+ oo.mixinClass( mw.echo.dm.SortedList, mw.echo.dm.List );
+
+ /**
+ * Set the sorting callback for this sorted list.
+ *
+ * @param {Function} sortingCallback Sorting callback
+ */
+ mw.echo.dm.SortedList.prototype.setSortingCallback = function (
sortingCallback ) {
+ this.sortingCallback = sortingCallback;
+ };
+
+ /**
+ * Add items to the sorted list.
+ *
+ * @param {OO.EventEmitter|OO.EventEmitter[]} items Item to add or
+ * an array of items to add
+ */
+ mw.echo.dm.SortedList.prototype.addItems = function ( items ) {
+ var index, i;
+
+ if ( !Array.isArray( items ) ) {
+ items = [ items ];
+ }
+
+ if ( items.length === 0 ) {
+ return this;
+ }
+
+ // Call parent mixin
+ for ( i = 0; i < items.length; i++ ) {
+ // Find the insertion index for this item
+ index = this.findInsertionIndex( items[ i ] );
+ // Make sure the item does not yet exit. If it does
+ // exist it is already in the correct position in
+ // the list
+ if ( this.items[ index ] !== items[ i ] ) {
+ // insert item at index
+ index = this.insertItem( items[ i ], index );
+ this.emit( 'add', items[ i ], index );
+ }
+ }
+ };
+
+ /**
+ * Normalize requested index to fit into the array.
+ * In the case of a sorted list, the index
+ *
+ * @param {OO.EventEmitter} item Items to insert
+ * @return {number} The index the item should be inserted into
+ */
+ mw.echo.dm.SortedList.prototype.findInsertionIndex = function ( item ) {
+ var list = this;
+
+ return this.binarySearchIndex(
+ this.items,
+ // Fake a this.sortingCallback.bind( null, item ) call
here
+ // otherwise this doesn't pass tests in phantomJS
+ function ( otherItem ) {
+ return list.sortingCallback( item, otherItem );
+ },
+ true
+ );
+
+ };
+
+ /**
+ * Use binary search to locate an element in a sorted array.
+ *
+ * searchFunc is given an element from the array. `searchFunc(elem)`
must return a number
+ * above 0 if the element we're searching for is to the right of (has a
higher index than) elem,
+ * below 0 if it is to the left of elem, or zero if it's equal to elem.
+ *
+ * To search for a specific value with a comparator function (a
`function cmp(a,b)` that returns
+ * above 0 if `a > b`, below 0 if `a < b`, and 0 if `a == b`), you can
use
+ * `searchFunc = cmp.bind( null, value )`.
+ *
+ * @param {Array} arr Array to search in
+ * @param {Function} searchFunc Search function
+ * @param {boolean} [forInsertion] If not found, return index where val
could be inserted
+ * @return {number|null} Index where val was found, or null if not found
+ */
+ mw.echo.dm.SortedList.prototype.binarySearchIndex = function ( arr,
searchFunc, forInsertion ) {
+ // TODO: Replace this with OO.binarySearch
+ // See https://gerrit.wikimedia.org/r/#/c/246813/
+ var mid, cmpResult,
+ left = 0,
+ right = arr.length;
+
+ while ( left < right ) {
+ // Equivalent to Math.floor( ( left + right ) / 2 ) but
much faster
+ /*jshint bitwise:false */
+ mid = ( left + right ) >> 1;
+ cmpResult = searchFunc( arr[ mid ] );
+ if ( cmpResult < 0 ) {
+ right = mid;
+ } else if ( cmpResult > 0 ) {
+ left = mid + 1;
+ } else {
+ return mid;
+ }
+ }
+ return forInsertion ? right : null;
+ };
+}( mediaWiki, OO ) );
--
To view, visit https://gerrit.wikimedia.org/r/249503
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I97e1ecbe5dccc478be527a94f037500f78f74b14
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
