launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05137
[Merge] lp:~rvb/launchpad/longpoll-icons into lp:launchpad
Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/longpoll-icons into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/launchpad/longpoll-icons/+merge/77672
This branch adds the new longpolling icons to the LP tree. It also creates the methods to use them on the MP page. The method themselves are not used yet because we need some FF protected usage of this before we figure out how to best use these icons.
This branch refactors the updating attribute of the widget into the set_status_updating method and adds two methods: set_status_longpollerror and set_status_longpolling.
= Tests =
lib/lp/code/javascript/tests/test_branchmergeproposal.updater.html
--
https://code.launchpad.net/~rvb/launchpad/longpoll-icons/+merge/77672
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/longpoll-icons into lp:launchpad.
=== added file 'lib/canonical/launchpad/images/longpoll_error.png'
Binary files lib/canonical/launchpad/images/longpoll_error.png 1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/longpoll_error.png 2011-09-30 09:21:27 +0000 differ
=== added file 'lib/canonical/launchpad/images/longpoll_loading.gif'
Binary files lib/canonical/launchpad/images/longpoll_loading.gif 1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/longpoll_loading.gif 2011-09-30 09:21:27 +0000 differ
=== modified file 'lib/lp/code/javascript/branchmergeproposal.updater.js'
--- lib/lp/code/javascript/branchmergeproposal.updater.js 2011-09-27 14:37:10 +0000
+++ lib/lp/code/javascript/branchmergeproposal.updater.js 2011-09-30 09:21:27 +0000
@@ -46,27 +46,6 @@
},
/**
- * Manages whether or not this MP is 'updating' (i.e. the content is
- * about to be updated).
- *
- * @attribute updating
- */
- updating: {
- setter: function(value) {
- if (value) {
- this._setup_diff_container();
- this.get(
- 'srcNode').one(
- 'h2').append(
- '<img src="/@@/spinner" />');
- }
- else {
- this.get('srcNode').all('h2 img').remove();
- }
- }
- },
-
- /**
* The HTML code for the diff.
*
* @attribute diff
@@ -106,6 +85,64 @@
// Create our own instance of the LP client.
this.set("lp_client", new Y.lp.client.Launchpad());
}
+ var self = this;
+ },
+
+ /*
+ * Set the proper icon to indicate the diff is updating.
+ *
+ * @method set_status_updating
+ */
+ set_status_updating: function() {
+ this.cleanup_status();
+ this._set_status('spinner', 'Update in progress.');
+ },
+
+ /*
+ * Set the proper icon to indicate the diff will be updated when the
+ * new version is available.
+ *
+ * @method set_status_longpolling
+ */
+ set_status_longpolling: function() {
+ this.cleanup_status();
+ this._set_status(
+ 'longpoll_loading',
+ 'The diff will be updated as soon as a new version is available.');
+ },
+
+ /*
+ * Set the proper icon to indicate that the diff update is broken.
+ *
+ * @method set_status_longpollerror
+ */
+ set_status_longpollerror: function() {
+ this.cleanup_status();
+ this._set_status(
+ 'longpoll_error',
+ 'Diff update error, please reload to see the changes.');
+ },
+
+ /*
+ * Add a status image to the diff title.
+ *
+ * @method _set_status
+ */
+ _set_status: function(image_name, title) {
+ var image = Y.Node.create('<img />')
+ .set('src', '/@@/' + image_name)
+ .set('title', title);
+ this.get('srcNode').one('h2').append(image);
+ },
+
+ /*
+ * Remove the status image to the diff title.
+ *
+ * @method cleanup_status
+ */
+ cleanup_status: function() {
+ this._setup_diff_container();
+ this.get('srcNode').all('h2 img').remove();
},
/*
@@ -122,7 +159,7 @@
var review_diff = Y.Node.create('<div />')
.set('id', 'review-diff')
.append(Y.Node.create('<h2 />')
- .set("text", "Preview Diff"))
+ .set("text", "Preview Diff "))
.append(Y.Node.create('<div />')
.addClass("diff-content"));
this.get('srcNode').append(review_diff);
@@ -149,10 +186,10 @@
Y.lp.anim.red_flash({node: node}).run();
},
start: function() {
- self.set('updating', true);
+ self.set_status_updating();
},
end: function() {
- self.set('updating', false);
+ self.cleanup_status();
}
}
};
=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js 2011-09-27 14:37:10 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.updater.js 2011-09-30 09:21:27 +0000
@@ -52,7 +52,7 @@
this.updater._setup_diff_container();
Y.Assert.isFalse(this.updater.get('pending'));
Y.Assert.areEqual(
- "Preview Diff",
+ "Preview Diff ",
this.updater.get(
'srcNode').one('#review-diff h2').get('text'));
Y.Assert.areEqual(
@@ -61,19 +61,33 @@
'srcNode').one('.diff-content').get('text'));
},
- test_set_updating_true: function() {
- this.updater.set('updating', true);
- Y.Assert.areEqual(
- 'Preview Diff<img src="/@@/spinner">',
- Y.one('h2').get('innerHTML'));
- },
-
- test_set_updating_false: function() {
+ test_set_status_updating: function() {
+ this.updater.set_status_updating();
+ Y.Assert.areEqual(
+ '/@@/spinner',
+ Y.one('h2').one('img').getAttribute('src'));
+ },
+
+ test_set_status_longpolling: function() {
+ this.updater.set_status_longpolling();
+ Y.Assert.areEqual(
+ '/@@/longpoll_loading',
+ Y.one('h2').one('img').getAttribute('src'));
+ },
+
+ test_set_status_longpollerror: function() {
+ this.updater.set_status_longpollerror();
+ Y.Assert.areEqual(
+ '/@@/longpoll_error',
+ Y.one('h2').one('img').getAttribute('src'));
+ },
+
+ test_cleanup_status: function() {
this.updater._setup_diff_container();
- this.updater.set('updating', true);
- this.updater.set('updating', false);
+ this.updater.set_status_updating();
+ this.updater.cleanup_status();
Y.Assert.areEqual(
- 'Preview Diff',
+ 'Preview Diff ',
Y.one('h2').get('innerHTML'));
},
=== modified file 'lib/lp/code/templates/branchmergeproposal-index.pt'
--- lib/lp/code/templates/branchmergeproposal-index.pt 2011-09-27 14:37:10 +0000
+++ lib/lp/code/templates/branchmergeproposal-index.pt 2011-09-30 09:21:27 +0000
@@ -191,7 +191,7 @@
</div>
</div>
<div id="review-diff" tal:condition="view/preview_diff">
- <h2>Preview Diff</h2>
+ <h2>Preview Diff </h2>
<div class="diff-content">
<div tal:replace="structure context/@@++diff"/>
</div>