launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03899
[Merge] lp:~danilo/launchpad/bug-772754-other-subscribers-activity into lp:launchpad
Данило Шеган has proposed merging lp:~danilo/launchpad/bug-772754-other-subscribers-activity into lp:launchpad with lp:~danilo/launchpad/bug-772754-other-subscribers-subscribers as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-activity/+merge/64180
= Bug 772754: Other subscribers list, part 4 =
This is part of ongoing work for providing the "other subscribers" list as indicated in mockup https://launchpadlibrarian.net/71552495/all-in-one.png attached to bug 772754 by Gary.
This branch continues on the previous branches to provide methods to indicate either some global activity in the subscribers list (like "Loading subscribers") or some activity on a particular subscriber (eg. "Subscribing", "Unsubscribing", along with spinner).
It is comprehensively tested.
== Tests ==
lp/bugs/javascript/tests/test_subscribers_list.html
== Demo and Q/A ==
N/A
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/bugs/javascript/subscribers_list.js
lib/lp/bugs/javascript/tests/test_subscribers_list.js
--
https://code.launchpad.net/~danilo/launchpad/bug-772754-other-subscribers-activity/+merge/64180
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~danilo/launchpad/bug-772754-other-subscribers-activity into lp:launchpad.
=== modified file 'lib/lp/bugs/javascript/subscribers_list.js'
--- lib/lp/bugs/javascript/subscribers_list.js 2011-06-10 13:31:25 +0000
+++ lib/lp/bugs/javascript/subscribers_list.js 2011-06-10 13:31:29 +0000
@@ -82,6 +82,9 @@
list: 'subscribers-list',
subscriber: 'subscriber',
no_subscribers: 'no-subscribers-indicator',
+ activity: 'global-activity-indicator',
+ activity_text: 'global-activity-text',
+ subscriber_activity: 'subscriber-activity-indicator',
actions: 'subscriber-actions',
unsubscribe: 'unsubscribe-action'
};
@@ -150,12 +153,14 @@
* of no subscribers.
*
* @method resetNoSubscribers
+ * @param force_hide {Boolean} Whether to force hiding of the "no subscribers"
+ * indication.
*/
-SubscribersList.prototype.resetNoSubscribers = function() {
+SubscribersList.prototype.resetNoSubscribers = function(force_hide) {
var has_sections = (
this.container_node.one('.' + CSS_CLASSES.section) !== null);
var no_subs;
- if (has_sections) {
+ if (has_sections || force_hide === true) {
// Make sure the indicator for no subscribers is not there.
no_subs = this.container_node.one('.' + CSS_CLASSES.no_subscribers);
if (no_subs !== null) {
@@ -170,6 +175,106 @@
};
/**
+ * Returns or creates a node for progress indication for the subscribers list.
+ *
+ * If node is not present, it is created and added to the beginning of
+ * subscribers list container node.
+ *
+ * @method _ensureActivityNode
+ * @return {Y.Node} A node with the spinner img node and a span text node.
+ */
+SubscribersList.prototype._ensureActivityNode = function() {
+ var activity_node = this.container_node.one('.' + CSS_CLASSES.activity);
+ if (activity_node === null) {
+ activity_node = Y.Node.create('<div></div>')
+ .addClass(CSS_CLASSES.activity);
+ progress_icon = Y.Node.create('<img></img>')
+ .set('src', '/@@/spinner');
+ activity_node.appendChild(progress_icon);
+ activity_node.appendChild(
+ Y.Node.create('<span></span>')
+ .addClass(CSS_CLASSES.activity_text));
+ this.container_node.prepend(activity_node);
+ }
+ return activity_node;
+};
+
+/**
+ * Sets icon in the activity node to either 'error' or 'spinner' icon.
+ *
+ * @method _setActivityErrorIcon
+ * @param node {Y.Node} An activity node as returned by _ensureActivityNode().
+ * @param error {Boolean} Whether to show an error icon.
+ * Otherwise shows a spinner image.
+ */
+SubscribersList.prototype._setActivityErrorIcon = function(node, error) {
+ var progress_icon = node.one('img');
+ if (error === true) {
+ progress_icon.set('src', '/@@/error');
+ } else {
+ progress_icon.set('src', '/@@/spinner');
+ }
+};
+
+/**
+ * Sets the activity text inside the activity node.
+ *
+ * @method _setActivityText
+ * @param node {Y.Node} An activity node as returned by _ensureActivityNode().
+ * @param text {String} Description of the activity currently in progress.
+ */
+SubscribersList.prototype._setActivityText = function(node, text) {
+ var text_node = node.one('.' + CSS_CLASSES.activity_text);
+ text_node.set('text', ' ' + text);
+};
+
+/**
+ * Indicate some activity for the subscribers list with a progress spinner
+ * and optionally some text.
+ *
+ * @method startActivity
+ * @param text {String} Description of the action to indicate progress of.
+ */
+SubscribersList.prototype.startActivity = function(text) {
+ // We don't ever want "No subscribers" to be shown when loading is in
+ // progress.
+ this.resetNoSubscribers(true);
+
+ var activity_node = this._ensureActivityNode();
+ // Ensure the icon is back to the spinner.
+ this._setActivityErrorIcon(activity_node, false);
+ this._setActivityText(activity_node, text);
+};
+
+/**
+ * Stop any activity indication for the subscribers list and optionally
+ * display an error message.
+ *
+ * @method stopActivity
+ * @param error_text {String} Error message to display. If not a string,
+ * it is considered that the operation was successful and no error
+ * indication is added to the subscribers list.
+ */
+SubscribersList.prototype.stopActivity = function(error_text) {
+ var activity_node = this.container_node.one('.' + CSS_CLASSES.activity);
+ if (Y.Lang.isString(error_text)) {
+ // There is an error message, keep the node visible with
+ // the error message in.
+ activity_node = this._ensureActivityNode(true);
+ this._setActivityErrorIcon(activity_node, true);
+ this._setActivityText(activity_node, error_text);
+ this.resetNoSubscribers(true);
+ } else {
+ // No errors, remove the activity node if present.
+ if (activity_node !== null) {
+ activity_node.remove();
+ }
+ // Restore "No subscribers" indication if needed.
+ this.resetNoSubscribers();
+ }
+};
+
+/**
* Get a CSS class to use for the section of the subscribers' list
* with subscriptions with the level `level`.
*
@@ -542,5 +647,88 @@
this._removeSectionNodeIfEmpty(existing_section);
};
+/**
+ * Indicates some activity for a subscriber in the subscribers list.
+ * Uses a regular Launchpad progress spinner UI.
+ *
+ * If subscriber is not in the list already, it fails with an exception.
+ * If there are any actions available for the subscriber (such as unsubscribe
+ * action), they are hidden.
+ *
+ * @method indicateSubscriberActivity
+ * @param subscriber {Object} Object containing at least `name`
+ * for the subscriber.
+ */
+SubscribersList.prototype.indicateSubscriberActivity = function(subscriber) {
+ var subscriber_node = this._getSubscriberNode(subscriber);
+ var progress_node = subscriber_node.one(
+ '.' + CSS_CLASSES.subscriber_activity);
+
+ // No-op if there is already indication of progress,
+ // and creates a new node with the spinner if there isn't.
+ if (progress_node === null) {
+ var actions_node = subscriber_node.one('.' + CSS_CLASSES.actions);
+ if (actions_node !== null) {
+ actions_node.setStyle('display', 'none');
+ }
+ var progress_icon = Y.Node.create('<img></img>')
+ .set('src', '/@@/spinner');
+
+ progress_node = Y.Node.create('<span></span>')
+ .addClass(CSS_CLASSES.subscriber_activity)
+ .setStyle('float', 'right');
+ progress_node.appendChild(progress_icon);
+ subscriber_node.appendChild(progress_node);
+ }
+};
+
+/**
+ * Stop any indication of activity for a subscriber in the subscribers list.
+ *
+ * If the spinner is present, it removes it. If `success` parameter is
+ * passed in, it determines if success or failure animation will be shown
+ * as well.
+ *
+ * If subscriber is not in the list already, it fails with an exception.
+ * If there are any actions available for the subscriber (such as unsubscribe
+ * action), they are re-displayed if hidden.
+ *
+ * @method stopSubscriberActivity
+ * @param subscriber {Object} Object containing at least `name`
+ * for the subscriber.
+ * @param success {Boolean} Whether to indicate success (`success` == true,
+ * flash green) or failure (false, red). Otherwise, perform no
+ * animation.
+ * @param callback {Function} Function to call if and when success/failure
+ * animation completes.
+ */
+SubscribersList.prototype.stopSubscriberActivity = function(subscriber,
+ success,
+ callback) {
+ var subscriber_node = this._getSubscriberNode(subscriber);
+ var progress_node = subscriber_node.one(
+ '.' + CSS_CLASSES.subscriber_activity);
+ if (progress_node !== null) {
+ // Remove and destroy the node if present.
+ progress_node.remove(true);
+ }
+ // If actions node is present and hidden, show it.
+ var actions_node = subscriber_node.one('.' + CSS_CLASSES.actions);
+ if (actions_node !== null) {
+ actions_node.setStyle('display', 'inline');
+ }
+
+ if (success === true || success === false) {
+ var anim;
+ if (success === true) {
+ anim = Y.lazr.anim.green_flash({ node: subscriber_node });
+ } else {
+ anim = Y.lazr.anim.red_flash({ node: subscriber_node });
+ }
+ anim.on('end', callback);
+ anim.run();
+ }
+};
+
}, "0.1", {"requires": ["node", "lazr.anim", "lp.client", "lp.names"]});
=== modified file 'lib/lp/bugs/javascript/tests/test_subscribers_list.js'
--- lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-10 13:31:25 +0000
+++ lib/lp/bugs/javascript/tests/test_subscribers_list.js 2011-06-10 13:31:29 +0000
@@ -430,6 +430,29 @@
no_subs_nodes.item(0).get('text'));
},
+ test_no_subscribers_force_hide: function() {
+ // When resetNoSubscribers() is called on an empty
+ // SubscribersList but with force_hide parameter set to true,
+ // indication of no subscribers is not added.
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.resetNoSubscribers(true);
+ var no_subs_nodes = this.root.all(
+ '.no-subscribers-indicator');
+ Y.Assert.areEqual(0, no_subs_nodes.size());
+ },
+
+ test_no_subscribers_force_hide_removal: function() {
+ // When resetNoSubscribers() is called on an empty
+ // SubscribersList which already has a no-subscribers
+ // indication shown, it is removed.
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.resetNoSubscribers();
+ subscribers_list.resetNoSubscribers(true);
+ var no_subs_nodes = this.root.all(
+ '.no-subscribers-indicator');
+ Y.Assert.areEqual(0, no_subs_nodes.size());
+ },
+
test_subscribers_no_addition: function() {
// When resetNoSubscribers() is called on a SubscribersList
// with some subscribers, no indication of no subscribers is added.
@@ -474,6 +497,184 @@
/**
+ * Test activity/progress indication for the entire subscribers list.
+ */
+suite.add(new Y.Test.Case({
+ name: 'SubscribersList.startActivity() and stopActivity() test',
+
+ _should: {
+ error: {
+ test_setActivityErrorIcon_error: true,
+ test_setActivityText_error: true
+ }
+ },
+
+ setUp: function() {
+ this.root = Y.Node.create('<div></div>');
+ Y.one('body').appendChild(this.root);
+ },
+
+ tearDown: function() {
+ this.root.remove();
+ },
+
+ test_ensureActivityNode: function() {
+ // With no activity node present, one is created and put
+ // into the subscribers list container node.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = subscribers_list._ensureActivityNode();
+ Y.Assert.isNotNull(node);
+ Y.Assert.isTrue(node.hasClass('global-activity-indicator'));
+ Y.Assert.areSame(
+ subscribers_list.container_node, node.get('parentNode'));
+ },
+
+ test_ensureActivityNode_contents: function() {
+ // Created node contains an img tag with the spinner icon
+ // and a span tag for the text.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = subscribers_list._ensureActivityNode();
+ var icon = node.one('img');
+ Y.Assert.isNotNull(icon);
+ Y.Assert.areEqual('file:///@@/spinner', icon.get('src'));
+ var text = node.one('span');
+ Y.Assert.isNotNull(text);
+ Y.Assert.isTrue(text.hasClass('global-activity-text'));
+ },
+
+ test_ensureActivityNode_existing: function() {
+ // When activity node already exists, it is returned
+ // and no new one is created.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var existing_node = subscribers_list._ensureActivityNode();
+ var new_node = subscribers_list._ensureActivityNode();
+ Y.Assert.areSame(existing_node, new_node);
+ Y.Assert.areEqual(
+ 1,
+ subscribers_list
+ .container_node
+ .all('.global-activity-indicator')
+ .size());
+ },
+
+ test_setActivityErrorIcon_error_icon: function() {
+ // With the activity node passed in, error icon is set
+ // when desired.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = subscribers_list._ensureActivityNode();
+ var icon_node = node.one('img');
+ subscribers_list._setActivityErrorIcon(node, true);
+ Y.Assert.areEqual('file:///@@/error', icon_node.get('src'));
+ },
+
+ test_setActivityErrorIcon_spinner_icon: function() {
+ // With the activity node passed in, spinner icon is restored
+ // when requested (error parameter !== true).
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = subscribers_list._ensureActivityNode();
+ var icon_node = node.one('img');
+ subscribers_list._setActivityErrorIcon(node, false);
+ Y.Assert.areEqual('file:///@@/spinner', icon_node.get('src'));
+ },
+
+ test_setActivityErrorIcon_error: function() {
+ // With non-activity node passed in, it fails.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = Y.Node.create('<div></div>');
+ subscribers_list._setActivityErrorIcon(node, true);
+ },
+
+ test_setActivityText: function() {
+ // With activity node and text passed in, proper
+ // text is set in the activity text node.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = subscribers_list._ensureActivityNode();
+ subscribers_list._setActivityText(node, "Blah");
+ // Single whitespace is prepended to better separate
+ // icon from the text.
+ Y.Assert.areEqual(" Blah", node.one('span').get('text'));
+ },
+
+ test_setActivityText_error: function() {
+ // With non-activity node passed in, it fails.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var node = Y.Node.create('<div></div>');
+ subscribers_list._setActivityText(node, "Blah");
+ },
+
+ test_startActivity: function() {
+ // startActivity adds the spinner icon and sets the appropriate text.
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.startActivity("Blah");
+
+ var node = subscribers_list._ensureActivityNode();
+
+ Y.Assert.areEqual('file:///@@/spinner', node.one('img').get('src'));
+ Y.Assert.areEqual(" Blah", node.one('span').get('text'));
+ },
+
+ test_startActivity_restores_state: function() {
+ // startActivity removes the no-subscribers indicator if present
+ // and restores the activity node icon.
+ var subscribers_list = setUpSubscribersList(this.root);
+ // Add a no-subscribers indication.
+ subscribers_list.resetNoSubscribers();
+ // Create an activity node and set the error icon.
+ var node = subscribers_list._ensureActivityNode();
+ subscribers_list._setActivityErrorIcon(node, true);
+
+ // Call startActivity() and see how it restores everything.
+ subscribers_list.startActivity();
+ Y.Assert.areEqual('file:///@@/spinner', node.one('img').get('src'));
+ Y.Assert.isNull(
+ subscribers_list.container_node.one('.no-subscribers-indicator'));
+ },
+
+ test_stopActivity: function() {
+ // stopActivity without parameters assumes a successful completion
+ // of the activity, so it removes the activity node and restores
+ // no-subscribers indication if needed.
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.startActivity("Blah");
+ subscribers_list.stopActivity();
+
+ var node = subscribers_list.container_node.one(
+ '.global-activity-indicator');
+ Y.Assert.isNull(node);
+ // Indication of no subscribers is restored.
+ Y.Assert.isNotNull(
+ subscribers_list.container_node.one('.no-subscribers-indicator'));
+ },
+
+ test_stopActivity_noop: function() {
+ // stopActivity without parameters assumes a successful completion
+ // of the activity. If no activity was in progress, nothing happens.
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.stopActivity();
+
+ var node = subscribers_list.container_node.one(
+ '.global-activity-indicator');
+ Y.Assert.isNull(node);
+ },
+
+ test_stopActivity_with_error_message: function() {
+ // stopActivity with error message passed in creates an activity
+ // node even if activity was not in progress and sets the error
+ // icon and error text to the passed in message..
+ var subscribers_list = setUpSubscribersList(this.root);
+ subscribers_list.stopActivity("Problem!");
+ var node = subscribers_list._ensureActivityNode();
+ Y.Assert.areEqual('file:///@@/error', node.one('img').get('src'));
+ Y.Assert.areEqual(" Problem!", node.one('span').get('text'));
+
+ // Indication of no subscribers is not added.
+ Y.Assert.isNull(
+ subscribers_list.container_node.one('.no-subscribers-indicator'));
+ }
+}));
+
+
+/**
* Function to get a list of all the sections present in the
* subscribers_list (a SubscribersList object).
*/
@@ -816,7 +1017,6 @@
}));
-
/**
* Test adding of subscribers and relevant helper methods.
*/
@@ -1236,6 +1436,156 @@
}));
+/**
+ * Test showing/stopping indication of activity for a subscriber.
+ */
+suite.add(new Y.Test.Case({
+ name: 'SubscribersList.indicateSubscriberActivity() and ' +
+ 'SubscribersList.stopSubscriberActivity() test',
+
+ setUp: function() {
+ this.root = Y.Node.create('<div></div>');
+ Y.one('body').appendChild(this.root);
+ },
+
+ tearDown: function() {
+ this.root.remove();
+ },
+
+ _should: {
+ error: {
+ test_indicateSubscriberActivity_error:
+ new Error('Subscriber is not present in the subscribers list. ' +
+ 'Please call addSubscriber(subscriber) first.'),
+ test_stopSubscriberActivity_error:
+ new Error('Subscriber is not present in the subscribers list. ' +
+ 'Please call addSubscriber(subscriber) first.')
+ }
+ },
+
+ test_indicateSubscriberActivity_error: function() {
+ // When subscriber is not in the list, fails with an exception.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ subscribers_list.indicateSubscriberActivity(subscriber);
+ },
+
+ test_indicateSubscriberActivity_node: function() {
+ // Creates a node with spinner image in it.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ var node = subscribers_list.addSubscriber(subscriber, 'Details');
+ subscribers_list.indicateSubscriberActivity(subscriber);
+
+ // This is the created node.
+ var progress_node = node.one('.subscriber-activity-indicator');
+ Y.Assert.isNotNull(progress_node);
+ var progress_icon = progress_node.one('img');
+ // We get an absolute URI, instead of the relative one which
+ // the code sets. Since the test runs from the local file system,
+ // that means "file://".
+ Y.Assert.areEqual('file:///@@/spinner', progress_icon.get('src'));
+ },
+
+ test_indicateSubscriberActivity_actions_hidden: function() {
+ // If there are any actions (in an actions node), they are
+ // all hidden.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ var node = subscribers_list.addSubscriber(subscriber, 'Details');
+ var actions_node = subscribers_list._getOrCreateActionsNode(node);
+
+ subscribers_list.indicateSubscriberActivity(subscriber);
+ Y.Assert.areEqual('none', actions_node.getStyle('display'));
+ },
+
+ test_stopSubscriberActivity_error: function() {
+ // When subscriber is not in the list, fails with an exception.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ subscribers_list.stopSubscriberActivity(subscriber);
+ },
+
+ test_stopSubscriberActivity_noop: function() {
+ // When there's no activity in progress, nothing happens.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ var node = subscribers_list.addSubscriber(subscriber, 'Details');
+ subscribers_list.stopSubscriberActivity(subscriber);
+ },
+
+ test_stopSubscriberActivity_spinner_removed: function() {
+ // When there is some activity in progress, spinner is removed.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ var node = subscribers_list.addSubscriber(subscriber, 'Details');
+ // Create the spinner.
+ subscribers_list.indicateSubscriberActivity(subscriber);
+ // And remove it.
+ subscribers_list.stopSubscriberActivity(subscriber);
+ Y.Assert.isNull(node.one('.subscriber-activity-indicator'));
+ },
+
+ test_stopSubscriberActivity_actions_restored: function() {
+ // When there is some activity in progress, spinner is removed.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ var node = subscribers_list.addSubscriber(subscriber, 'Details');
+ var actions_node = subscribers_list._getOrCreateActionsNode(node);
+ // Hide actions.
+ actions_node.setStyle('display', 'none');
+ // And restore actions.
+ subscribers_list.stopSubscriberActivity(subscriber);
+ Y.Assert.areEqual('inline', actions_node.getStyle('display'));
+ },
+
+ test_stopSubscriberActivity_success_callback: function() {
+ // When we are indicating successful/failed operation,
+ // green_flash/red_flash animation is executed and callback
+ // function is called when it ends.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ subscribers_list.addSubscriber(subscriber, 'Details');
+ var callback_called = false;
+ var callback = function() {
+ callback_called = true;
+ };
+
+ subscribers_list.stopSubscriberActivity(
+ subscriber, true, callback);
+ // Callback is not called immediatelly.
+ Y.Assert.isFalse(callback_called);
+ this.wait(function() {
+ // But after waiting for animation to complete,
+ // callback is called.
+ Y.Assert.isTrue(callback_called);
+ }, 1100);
+ },
+
+ test_stopSubscriberActivity_no_callback: function() {
+ // When we pass the callback in, but success is neither
+ // 'true' nor 'false', callback is not called.
+ var subscribers_list = setUpSubscribersList(this.root);
+ var subscriber = { name: 'user' };
+ subscribers_list.addSubscriber(subscriber, 'Details');
+ var callback_called = false;
+ var callback = function() {
+ callback_called = true;
+ };
+
+ subscribers_list.stopSubscriberActivity(
+ subscriber, "no-callback", callback);
+ // Callback is not called.
+ Y.Assert.isFalse(callback_called);
+ this.wait(function() {
+ // Nor is it called after any potential animations complete.
+ Y.Assert.isFalse(callback_called);
+ }, 1100);
+ }
+
+}));
+
+
var handle_complete = function(data) {
status_node = Y.Node.create(
'<p id="complete">Test status: complete</p>');
