launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/orderbybar-integration-fixes into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~deryck/launchpad/orderbybar-integration-fixes/+merge/80705

This branch contains my fixes to OrderByBar that were discovered as I began to integrate the widget into the LP page.  This includes:

* Adding CSS for the widget
* Adding a test and fixing the issue that active-sort
  class was not being applied to the node that was clicked

Actually, it's more accurate to say I have a large branch going doing integration work, and this widget stuff could be broken out into this branch to make it easier to get reviewed.
-- 
https://code.launchpad.net/~deryck/launchpad/orderbybar-integration-fixes/+merge/80705
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/orderbybar-integration-fixes into lp:launchpad.

=== modified file 'buildout-templates/bin/combine-css.in'
--- buildout-templates/bin/combine-css.in	2011-08-30 15:06:11 +0000
+++ buildout-templates/bin/combine-css.in	2011-10-28 19:03:24 +0000
@@ -38,6 +38,7 @@
     'build/picker/assets/skins/sam/picker.css',
     'build/activator/assets/skins/sam/activator.css',
     'build/choiceedit/assets/choiceedit-core.css',
+    'build/ordering/assets/ordering-core.css',
     GALLERY_ACCORDION + 'gallery-accordion-core.css',
     GALLERY_ACCORDION + 'skins/sam/gallery-accordion-skin.css',
     'build/sprite.css',

=== added directory 'lib/lp/app/javascript/ordering/assets'
=== added file 'lib/lp/app/javascript/ordering/assets/ordering-core.css'
--- lib/lp/app/javascript/ordering/assets/ordering-core.css	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/ordering/assets/ordering-core.css	2011-10-28 19:03:24 +0000
@@ -0,0 +1,23 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+.yui3-orderbybar {
+    border:1px solid #EEE;
+    float: left;
+    min-width: 600px;
+    width: 100%;
+}
+.yui3-orderbybar p {
+    float: left;
+    margin: 5px;
+}
+.yui3-orderbybar ul li {
+    float: left;
+    margin: 5px 3px;
+    padding: 0 6px;
+    background: #DDD;
+    border-radius: 10px;
+    cursor: pointer;
+}
+.yui3-orderbybar ul li.active-sort {
+    background: #B8B8B8;
+}

=== added directory 'lib/lp/app/javascript/ordering/assets/skins'
=== added directory 'lib/lp/app/javascript/ordering/assets/skins/sam'
=== modified file 'lib/lp/app/javascript/ordering/ordering.js'
--- lib/lp/app/javascript/ordering/ordering.js	2011-10-27 18:38:33 +0000
+++ lib/lp/app/javascript/ordering/ordering.js	2011-10-28 19:03:24 +0000
@@ -149,6 +149,38 @@
         },
 
         /**
+         * Change the active_sort class from the previously active
+         * node to the currently active node.
+         *
+         * @method _setActiveSortClassName
+         */
+        _setActiveSortClassName: function(preclick_sort_key) {
+            var active = this.get('active');
+            // We do not have to do anything if the button is already active.
+            if (active !== preclick_sort_key) {
+                var li_nodes = this.get('li_nodes');
+                var len = li_nodes.length;
+                var prev_li_id = 'sort-' + preclick_sort_key;
+                var active_li_id = 'sort-' + active;
+                var i,
+                    li_node,
+                    id;
+                // Loop through the li_nodes we have and remove the
+                // active-sort class from the previously active node
+                // and add the class to the currently active node.
+                for (i=0; i<len; i++) {
+                    li_node = li_nodes[i];
+                    id = li_node.get('id');
+                    if (id === prev_li_id) {
+                        li_node.removeClass('active-sort');
+                    } else if (id === active_li_id) {
+                        li_node.addClass('active-sort');
+                    }
+                }
+            }
+        },
+
+        /**
          * Method used to update the arrows used in the display.
          * This will also update the widget's attributes to match
          * the new state.
@@ -213,6 +245,7 @@
             // the "active" widget state.
             var preclick_sort_key = this.get('active');
             this.set('active', clicked_node_sort_key);
+            this._setActiveSortClassName(preclick_sort_key);
             // Update display and fire events.
             this._updateSortArrows(
                 clicked_node, clicked_node_sort_key, preclick_sort_key);
@@ -258,7 +291,10 @@
                 li_nodes.push(li_node);
             }
             this.set('li_nodes', li_nodes);
-            this.get('srcNode').appendChild(orderby_ul);
+            var div = Y.Node.create('<div></div>');
+            div.appendChild('<p>Order by:</p>');
+            div.appendChild(orderby_ul);
+            this.get('srcNode').appendChild(div);
         },
 
         /**

=== modified file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.js'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2011-10-27 18:47:11 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2011-10-28 19:03:24 +0000
@@ -168,6 +168,21 @@
         Assert.areEqual(expected_text, arrow_span.get('innerHTML'));
     },
 
+    test_active_sort_click_class_change: function() {
+        // Click a node should add the active_sort class
+        // and remove that class from the previously active node.
+        this.makeSrcNode('test-div');
+        this.orderby = new Y.lp.ordering.OrderByBar({
+            srcNode: Y.one('#test-div')
+        });
+        this.orderby.render();
+        var importance_node = Y.one('#sort-importance');
+        Assert.isTrue(importance_node.hasClass('active-sort'));
+        var status_node = Y.one('#sort-status');
+        status_node.simulate('click');
+        Assert.isTrue(status_node.hasClass('active-sort'));
+    },
+
     test_active_sort_validator: function() {
         // This should fail because we do not allow
         // a "active" value not found in sort_keys.