jenkins-bot has submitted this change and it was merged.
Change subject: Fix fade-in/out animation in sorting
......................................................................
Fix fade-in/out animation in sorting
The fade in/out animation is asynchronous. This means that if we are
sorting multiple items one after the other, by the time the item faded
out, it will be reinserted back into the wrong position, breaking the
sorting.
This also broke the promise of OO.SortedEmitterList whereby all its items
are always in order.
The way to fix this was to force a better synchronization with the item
order while we hide and show the item in its new place. To do that,
a new widget is created as a fake clone of the old one, in the original
position of the old one. The original item is then reinserted (while hidden)
to the proper location -- preserving order. The fake clone is then faded
out, and the real item is then faded in.
For this to work properly, the cloned item had to preserve some of the
original item's information, like timestamp, foreigness and id. However,
since both the real item and the fake new clone have the same details,
the clone fakes its ID by adding a fraction to it - promising that the
fallback in case of equal timestamps (which happens on the real and
cloned items) will still resolve with some decision about the placement
of the items rather than (falsely but understandably) decide they are
both the same.
Since this whole animation is somewhat of a hack, the list now has a
configuration parameter to turn the animation on.
The animation is on in the popups, but off in the special page.
Bug: T141419
Change-Id: Ic7c35e5ddefc51bf7fde497eab36414b4dddcd9e
---
M Resources.php
M modules/controller/mw.echo.Controller.js
M modules/model/mw.echo.dm.NotificationsList.js
M modules/ui/mw.echo.ui.BundleNotificationItemWidget.js
A modules/ui/mw.echo.ui.ClonedNotificationItemWidget.js
M modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
M modules/ui/mw.echo.ui.DatedNotificationsWidget.js
M modules/ui/mw.echo.ui.DatedSubGroupListWidget.js
M modules/ui/mw.echo.ui.NotificationBadgeWidget.js
M modules/ui/mw.echo.ui.NotificationItemWidget.js
M modules/ui/mw.echo.ui.NotificationsInboxWidget.js
M modules/ui/mw.echo.ui.NotificationsListWidget.js
M modules/ui/mw.echo.ui.SortedListWidget.js
M modules/ui/mw.echo.ui.SubGroupListWidget.js
14 files changed, 211 insertions(+), 44 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/Resources.php b/Resources.php
index a34d0a4..646a7f1 100644
--- a/Resources.php
+++ b/Resources.php
@@ -75,6 +75,7 @@
'ui/mw.echo.ui.SingleNotificationItemWidget.js',
'ui/mw.echo.ui.CrossWikiNotificationItemWidget.js',
'ui/mw.echo.ui.BundleNotificationItemWidget.js',
+ 'ui/mw.echo.ui.ClonedNotificationItemWidget.js',
'ui/mw.echo.ui.ActionMenuPopupWidget.js',
'ui/mw.echo.ui.MenuItemWidget.js',
diff --git a/modules/controller/mw.echo.Controller.js
b/modules/controller/mw.echo.Controller.js
index 8fc96f2..6c427d1 100644
--- a/modules/controller/mw.echo.Controller.js
+++ b/modules/controller/mw.echo.Controller.js
@@ -179,9 +179,21 @@
name: symbolicName,
source: currentSource,
title: date,
- timestamp: date
- } );
+ timestamp: date,
+ sortingCallback: function ( a,
b ) {
+ // Reverse sorting. In
the special page we want the
+ // items sorted only by
timestamp, regardless of
+ // read/unread state
+ if ( b.getTimestamp() <
a.getTimestamp() ) {
+ return -1;
+ } else if (
b.getTimestamp() > a.getTimestamp() ) {
+ return 1;
+ }
+ // Fallback on IDs
+ return b.getId() -
a.getId();
+ }
+ } );
models[ symbolicName ].setItems(
dateItems[ date ] );
}
diff --git a/modules/model/mw.echo.dm.NotificationsList.js
b/modules/model/mw.echo.dm.NotificationsList.js
index 8460c5f..32e787c 100644
--- a/modules/model/mw.echo.dm.NotificationsList.js
+++ b/modules/model/mw.echo.dm.NotificationsList.js
@@ -10,6 +10,8 @@
*
* @constructor
* @param {Object} config Configuration options
+ * @cfg {Function} [sortingCallback] A function defining the sorting
order
+ * of items in this list.
* @cfg {string} [title] An optional title for this notifications list
* @cfg {string} [name='local'] Symbolic name for this list
* @cfg {string} [source='local'] Symbolic name for the source of this
list.
@@ -33,7 +35,7 @@
this.fallbackTimestamp = config.timestamp || 0;
// Sorting callback
- this.setSortingCallback( function ( a, b ) {
+ this.setSortingCallback( config.sortingCallback || function (
a, b ) {
if ( !a.isRead() && b.isRead() ) {
return -1; // Unread items are always above
read items
} else if ( a.isRead() && !b.isRead() ) {
diff --git a/modules/ui/mw.echo.ui.BundleNotificationItemWidget.js
b/modules/ui/mw.echo.ui.BundleNotificationItemWidget.js
index 342c7e4..9cc0fe7 100644
--- a/modules/ui/mw.echo.ui.BundleNotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.BundleNotificationItemWidget.js
@@ -11,6 +11,7 @@
* @param {mw.echo.Controller} controller Echo notifications controller
* @param {mw.echo.dm.BundleNotificationItem} model Notification group
model
* @param {Object} [config] Configuration object
+ * @cfg {boolean} [animateSorting=false] Animate the sorting of items
* @cfg {jQuery} [$overlay] A jQuery element functioning as an overlay
* for popups.
*/
@@ -42,7 +43,8 @@
{
classes: [
'mw-echo-ui-bundleNotificationItemWidget-group' ],
timestamp: this.getTimestamp(),
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animated: !!config.animateSorting
}
);
diff --git a/modules/ui/mw.echo.ui.ClonedNotificationItemWidget.js
b/modules/ui/mw.echo.ui.ClonedNotificationItemWidget.js
new file mode 100644
index 0000000..ec1c7b8
--- /dev/null
+++ b/modules/ui/mw.echo.ui.ClonedNotificationItemWidget.js
@@ -0,0 +1,81 @@
+( function ( mw ) {
+ /*global moment:false */
+ /**
+ * A wrapper widget for a fake, cloned notification. This is used
+ * for the fade in/out effects while reordering.
+ *
+ * @class
+ * @extends OO.ui.Widget
+ *
+ * @constructor
+ * @param {jQuery} $element A clone of an
mw.echo.ui.NotificationItemWidget's $element
+ * @param {Object} [config] Configuration options
+ * @cfg {string} [timestamp] The timestamp for this cloned widget, in
UTC and ISO 8601 format
+ * @cfg {boolean} [read=false] The read state for this cloned widget
+ * @cfg {boolean} [foreign=false] The foreignness state of this cloned
widget
+ * @cfg {number} [id] The id for this cloned widget
+ */
+ mw.echo.ui.ClonedNotificationItemWidget = function
MwEchoUiClonedNotificationItemWidget( $element, config ) {
+ config = config || {};
+
+ // Parent
+ mw.echo.ui.ClonedNotificationItemWidget.parent.call( this,
config );
+
+ this.$element = $element;
+ this.timestamp = config.timestamp || moment.utc().format(
'YYYY-MM-DD[T]HH:mm:ss[Z]' );
+ this.read = !!config.read;
+ this.foreign = config.foreign === undefined ? true :
config.foreign;
+ this.id = config.id;
+
+ this.$element.addClass(
'mw-echo-ui-clonedNotificationItemWidget' );
+ };
+
+ /* Initialization */
+ OO.inheritClass( mw.echo.ui.ClonedNotificationItemWidget, OO.ui.Widget
);
+
+ /**
+ * Get the widget's timestamp
+ *
+ * @return {string} Timestamp in UTC, ISO 8601 format
+ */
+ mw.echo.ui.ClonedNotificationItemWidget.prototype.getTimestamp =
function () {
+ return this.timestamp;
+ };
+
+ /**
+ * Get the widget's read state
+ *
+ * @return {boolean} Widget is read
+ */
+ mw.echo.ui.ClonedNotificationItemWidget.prototype.isRead = function () {
+ return this.read;
+ };
+
+ /**
+ * Get the widget's id
+ *
+ * @return {number} Widget id
+ */
+ mw.echo.ui.ClonedNotificationItemWidget.prototype.getId = function () {
+ return this.id;
+ };
+
+ /**
+ * The foreign state of this widget
+ *
+ * @return {boolean} This item widget is foreign
+ */
+ mw.echo.ui.ClonedNotificationItemWidget.prototype.isForeign = function
() {
+ return this.foreign;
+ };
+
+ /**
+ * This widget is fake by definition.
+ *
+ * @return {boolean} true
+ */
+ mw.echo.ui.ClonedNotificationItemWidget.prototype.isFake = function () {
+ return true;
+ };
+
+} )( mediaWiki );
diff --git a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
index 5226231..e8b950c 100644
--- a/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.CrossWikiNotificationItemWidget.js
@@ -16,6 +16,7 @@
* @param {mw.echo.Controller} controller Echo notifications controller
* @param {mw.echo.dm.CrossWikiNotificationItem} model Notification
group model
* @param {Object} [config] Configuration object
+ * @cfg {boolean} [animateSorting=false] Animate the sorting of items
* @cfg {jQuery} [$overlay] A jQuery element functioning as an overlay
* for popups.
*/
@@ -52,7 +53,8 @@
{
classes: [
'mw-echo-ui-crossWikiNotificationItemWidget-group' ],
timestamp: this.getTimestamp(),
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animated: config.animateSorting
}
);
diff --git a/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
b/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
index 9b2face..60297e4 100644
--- a/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
+++ b/modules/ui/mw.echo.ui.DatedNotificationsWidget.js
@@ -10,6 +10,7 @@
* @param {mw.echo.Controller} controller Echo controller
* @param {mw.echo.dm.ModelManager} modelManager Model manager
* @param {Object} [config] Configuration object
+ * @cfg {boolean} [animateSorting=false] Animate the sorting of items
* @cfg {jQuery} [$overlay] An overlay for the popup menus
*/
mw.echo.ui.DatedNotificationsWidget = function
MwEchoUiDatedNotificationsListWidget( controller, modelManager, config ) {
@@ -25,6 +26,7 @@
this.models = {};
this.$overlay = config.$overlay || this.$element;
+ this.animateSorting = !!config.animateSorting;
this.listWidget = new mw.echo.ui.SortedListWidget(
// Sorting callback
@@ -39,7 +41,8 @@
// Config
{
classes: [
'mw-echo-ui-datedNotificationsWidget-group' ],
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animated: false
}
);
@@ -105,7 +108,8 @@
{
showTitle: true,
showMarkAllRead: true,
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animated: this.animateSorting
}
);
this.attachModel( model, models[ model ] );
diff --git a/modules/ui/mw.echo.ui.DatedSubGroupListWidget.js
b/modules/ui/mw.echo.ui.DatedSubGroupListWidget.js
index 191dbfe..24116fa 100644
--- a/modules/ui/mw.echo.ui.DatedSubGroupListWidget.js
+++ b/modules/ui/mw.echo.ui.DatedSubGroupListWidget.js
@@ -25,7 +25,22 @@
config = config || {};
// Parent constructor
- mw.echo.ui.DatedSubGroupListWidget.parent.call( this,
controller, listModel, config );
+ mw.echo.ui.DatedSubGroupListWidget.parent.call( this,
controller, listModel, $.extend( {
+ // Since this widget is defined as a dated list, we sort
+ // its items according to timestamp without
consideration
+ // of read state or foreignness.
+ sortingCallback: function ( a, b ) {
+ // Reverse sorting
+ if ( b.getTimestamp() < a.getTimestamp() ) {
+ return -1;
+ } else if ( b.getTimestamp() > a.getTimestamp()
) {
+ return 1;
+ }
+
+ // Fallback on IDs
+ return b.getId() - a.getId();
+ }
+ }, config ) );
momentTimestamp = moment.utc( this.model.getTimestamp() );
diff = now.diff( momentTimestamp, 'weeks' );
diff --git a/modules/ui/mw.echo.ui.NotificationBadgeWidget.js
b/modules/ui/mw.echo.ui.NotificationBadgeWidget.js
index 1dcaad0..8ac722d 100644
--- a/modules/ui/mw.echo.ui.NotificationBadgeWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationBadgeWidget.js
@@ -82,7 +82,8 @@
this.manager,
{
type: this.types,
- $overlay: this.$menuOverlay
+ $overlay: this.$menuOverlay,
+ animated: true
}
);
diff --git a/modules/ui/mw.echo.ui.NotificationItemWidget.js
b/modules/ui/mw.echo.ui.NotificationItemWidget.js
index f5bbabf..b1ad36f 100644
--- a/modules/ui/mw.echo.ui.NotificationItemWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationItemWidget.js
@@ -365,4 +365,12 @@
this.$element.removeClass(
'mw-echo-ui-notificationItemWidget-initiallyUnseen' );
};
+ /**
+ * Declares whether this widget is a cloned fake.
+ *
+ * @return {boolean} false
+ */
+ mw.echo.ui.NotificationItemWidget.prototype.isFake = function () {
+ return false;
+ };
} )( mediaWiki, jQuery );
diff --git a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js
b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js
index f3d8089..20dd730 100644
--- a/modules/ui/mw.echo.ui.NotificationsInboxWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationsInboxWidget.js
@@ -39,7 +39,8 @@
this.controller,
this.manager,
{
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animateSorting: false
}
);
this.setPendingElement( this.datedListWidget.$element );
diff --git a/modules/ui/mw.echo.ui.NotificationsListWidget.js
b/modules/ui/mw.echo.ui.NotificationsListWidget.js
index d29ce54..14fd093 100644
--- a/modules/ui/mw.echo.ui.NotificationsListWidget.js
+++ b/modules/ui/mw.echo.ui.NotificationsListWidget.js
@@ -17,7 +17,32 @@
mw.echo.ui.NotificationsListWidget = function
MwEchoUiNotificationsListWidget( controller, manager, config ) {
config = config || {};
// Parent constructor
- mw.echo.ui.NotificationsListWidget.parent.call( this, config );
+ mw.echo.ui.NotificationsListWidget.parent.call(
+ this,
+ // Sorting callback
+ 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 if ( !a.isForeign() && b.isForeign() ) {
+ return -1;
+ } else if ( a.isForeign() && !b.isForeign() ) {
+ return 1;
+ }
+
+ // Reverse sorting
+ if ( b.getTimestamp() < a.getTimestamp() ) {
+ return -1;
+ } else if ( b.getTimestamp() > a.getTimestamp()
) {
+ return 1;
+ }
+
+ // Fallback on IDs
+ return b.getId() - a.getId();
+ },
+ config
+ );
// Initialize models
this.controller = controller;
@@ -30,29 +55,6 @@
// Dummy 'loading' option widget
this.loadingOptionWidget = new
mw.echo.ui.PlaceholderItemWidget();
-
- // Define the sorting order
- 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 if ( !a.isForeign() && b.isForeign() ) {
- return -1;
- } else if ( a.isForeign() && !b.isForeign() ) {
- return 1;
- }
-
- // Reverse sorting
- if ( b.getTimestamp() < a.getTimestamp() ) {
- return -1;
- } else if ( b.getTimestamp() > a.getTimestamp() ) {
- return 1;
- }
-
- // Fallback on IDs
- return b.getId() - a.getId();
- } );
this.resetLoadingOption();
@@ -120,7 +122,8 @@
this.controller,
model,
{
- $overlay: this.$overlay
+ $overlay: this.$overlay,
+ animateSorting:
this.animated
}
) );
} else {
@@ -130,7 +133,8 @@
model,
{
$overlay: this.$overlay,
- bundle: false
+ bundle: false,
+ animateSorting:
this.animated
}
) );
}
diff --git a/modules/ui/mw.echo.ui.SortedListWidget.js
b/modules/ui/mw.echo.ui.SortedListWidget.js
index 26841e7..fd35165 100644
--- a/modules/ui/mw.echo.ui.SortedListWidget.js
+++ b/modules/ui/mw.echo.ui.SortedListWidget.js
@@ -16,6 +16,7 @@
* for popups.
* @cfg {number} [timestamp=0] A fallback timestamp for the list,
usually representing
* the timestamp of the latest item.
+ * @cfg {boolean} [animated=false] Animate the sorting of items
*/
mw.echo.ui.SortedListWidget = function MwEchoUiSortedListWidget(
sortingCallback, config ) {
config = config || {};
@@ -29,6 +30,8 @@
this.$group = null;
this.$overlay = config.$overlay;
this.timestamp = config.timestamp || 0;
+
+ this.animated = !!config.animated;
// Initialization
this.setGroupElement( config.$group || this.$element );
@@ -48,15 +51,42 @@
* @inheritdoc
*/
mw.echo.ui.SortedListWidget.prototype.onItemSortChange = function (
item ) {
- var widget = this;
+ var fakeWidget,
+ widget = this;
- item.$element.fadeOut( 400, function () {
- widget.removeItems( item );
+ if ( this.animated ) {
+ // Create a fake widget with cloned contents
+ fakeWidget = new
mw.echo.ui.ClonedNotificationItemWidget(
+ item.$element.clone( true ),
+ {
+ id: item.getId() + '.42',
+ read: !item.isRead(),
+ foreign: item.isForeign(),
+ timestamp: item.getTimestamp()
+ }
+ );
+ // remove real item from item list, without touching
the DOM
+ this.removeItems( item );
+
+ // insert real item, hidden
item.$element.hide();
- widget.addItems( item );
- item.$element.fadeIn( 400 );
- } );
+ this.addItems( item );
+
+ // insert fake
+ this.addItems( fakeWidget );
+
+ // fade out fake
+ fakeWidget.$element.fadeOut( 400, function () {
+ // remove fake
+ widget.removeItems( fakeWidget );
+ // fade-in real item
+ item.$element.fadeIn( 400 );
+ } );
+ } else {
+ // Mixin method
+ OO.SortedEmitterList.prototype.onItemSortChange.call(
this, item );
+ }
};
/**
* Set the group element.
diff --git a/modules/ui/mw.echo.ui.SubGroupListWidget.js
b/modules/ui/mw.echo.ui.SubGroupListWidget.js
index 04bd8fb..9b1563e 100644
--- a/modules/ui/mw.echo.ui.SubGroupListWidget.js
+++ b/modules/ui/mw.echo.ui.SubGroupListWidget.js
@@ -9,6 +9,7 @@
* @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 {boolean} [animateSorting=false] Animate the sorting of items
* @cfg {jQuery} [$overlay] A jQuery element functioning as an overlay
* for popups.
*/
@@ -31,7 +32,7 @@
this.listWidget = new mw.echo.ui.SortedListWidget(
// Sorting callback
- function ( a, b ) {
+ config.sortingCallback || function ( a, b ) {
// Reverse sorting
if ( b.getTimestamp() < a.getTimestamp() ) {
return -1;
@@ -43,7 +44,10 @@
return b.getId() - a.getId();
},
// Config
- { $overlay: this.$overlay }
+ {
+ $overlay: this.$overlay,
+ animated: config.animateSorting
+ }
);
sourceURL = this.model.getSourceURL() ?
--
To view, visit https://gerrit.wikimedia.org/r/301906
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic7c35e5ddefc51bf7fde497eab36414b4dddcd9e
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Mooeypoo <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
