launchpad-reviewers team mailing list archive

[Huw Wilkins <huw.wilkins@xxxxxxxxxxxxx>]

Huw Wilkins has proposed merging lp:~huwshimi/launchpad/order-arrows-894535 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #894535 in Launchpad itself: "Bug listing arrows appear to be backwards"
  https://bugs.launchpad.net/launchpad/+bug/894535

For more details, see:
https://code.launchpad.net/~huwshimi/launchpad/order-arrows-894535/+merge/84432

The ordering arrows on bug listings used to point in the opposite to how the content was displayed on the page.

This branch changes the direction of the arrows and adds a new icon image (https://launchpadlibrarian.net/86655366/order_arrows.png).
-- 
https://code.launchpad.net/~huwshimi/launchpad/order-arrows-894535/+merge/84432
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~huwshimi/launchpad/order-arrows-894535 into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/sprite.css.in'
--- lib/canonical/launchpad/icing/sprite.css.in	2011-11-03 20:22:52 +0000
+++ lib/canonical/launchpad/icing/sprite.css.in	2011-12-05 04:39:05 +0000
@@ -516,3 +516,11 @@
     background-image: url(/@@/notification-close.png); /* sprite-ref: icon-sprites */
     background-repeat: no-repeat;
     }
+.order-ascending {
+    background-image: url(/@@/order-ascending.png); /* sprite-ref: icon-sprites */
+    background-repeat: no-repeat;
+    }
+.order-descending {
+    background-image: url(/@@/order-descending.png); /* sprite-ref: icon-sprites */
+    background-repeat: no-repeat;
+    }

=== added file 'lib/canonical/launchpad/images/order-ascending.png'
Binary files lib/canonical/launchpad/images/order-ascending.png	1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/order-ascending.png	2011-12-05 04:39:05 +0000 differ
=== added file 'lib/canonical/launchpad/images/order-descending.png'
Binary files lib/canonical/launchpad/images/order-descending.png	1970-01-01 00:00:00 +0000 and lib/canonical/launchpad/images/order-descending.png	2011-12-05 04:39:05 +0000 differ
=== modified file 'lib/lp/app/javascript/ordering/assets/ordering-core.css'
--- lib/lp/app/javascript/ordering/assets/ordering-core.css	2011-11-02 19:06:42 +0000
+++ lib/lp/app/javascript/ordering/assets/ordering-core.css	2011-12-05 04:39:05 +0000
@@ -25,6 +25,11 @@
     background: #B8B8B8;
     }
 
+.yui3-orderbybar .sort-arr .sprite {
+    vertical-align: middle;
+    margin-left: 5px;
+    }
+
 .yui3-orderbybar .config-widget {
     float: right;
     border-left: 1px solid #EEE;

=== modified file 'lib/lp/app/javascript/ordering/ordering.js'
--- lib/lp/app/javascript/ordering/ordering.js	2011-11-03 16:42:21 +0000
+++ lib/lp/app/javascript/ordering/ordering.js	2011-12-05 04:39:05 +0000
@@ -216,8 +216,8 @@
             clicked_node, clicked_node_sort_key, preclick_sort_key) {
             // References to the span holding the arrow and the arrow HTML.
             var arrow_span = clicked_node.one('span');
-            var up_arrow = Y.Node.create('↑').get('text');
-            var down_arrow = Y.Node.create('↓').get('text');
+            var ascending_arrow = Y.Node.create('<span class="sprite order-ascending"></span>');
+            var descending_arrow = Y.Node.create('<span class="sprite order-descending"></span>');
 
             var is_active_sort_button = false;
             if (clicked_node_sort_key === preclick_sort_key) {
@@ -226,14 +226,13 @@
             if (is_active_sort_button) {
                 // Handle the case where the button clicked is the current
                 // active sort order.  We change sort directions for it.
-                var current_arrow = arrow_span.get('innerHTML');
-                if (current_arrow === up_arrow) {
-                    arrow_span.set('innerHTML', down_arrow);
+                if (arrow_span.hasClass('desc')) {
+                    arrow_span.setContent(ascending_arrow);
                     arrow_span.addClass('asc');
                     arrow_span.removeClass('desc');
                     this.set('sort_order', 'asc');
                 } else {
-                    arrow_span.set('innerHTML', up_arrow);
+                    arrow_span.setContent(descending_arrow);
                     arrow_span.addClass('desc');
                     arrow_span.removeClass('asc');
                     this.set('sort_order', 'desc');
@@ -246,13 +245,13 @@
                 var old_active_li = this.get('contentBox').one(
                     old_active_sort_key);
                 var old_arrow_span = old_active_li.one('span');
-                old_arrow_span.set('innerHTML', '');
+                old_arrow_span.setContent('');
                 var pre_click_sort_order = this.get('sort_order');
                 old_arrow_span.removeClass(pre_click_sort_order);
                 // Update current li span arrow and set new sort order.
                 arrow_span.addClass('asc');
                 this.set('sort_order', 'asc');
-                arrow_span.set('innerHTML', down_arrow);
+                arrow_span.setContent(descending_arrow);
             }
         },
 
@@ -307,9 +306,9 @@
                     li_node.one('span').addClass(this.get('sort_order'));
                     sort_order = this.get('sort_order');
                     if (sort_order === 'asc') {
-                        li_node.one('span').set('innerHTML', '&darr;');
+                        li_node.one('span').setContent('<span class="sprite order-ascending"></span>');
                     } else if (sort_order === 'desc') {
-                        li_node.one('span').set('innerHTML', '&uarr;');
+                        li_node.one('span').setContent('<span class="sprite order-descending"></span>');
                     }
                 }
                 orderby_ul.appendChild(li_node);