Merge lp:~deryck/launchpad/orderbybar-integration-fixes into lp:launchpad

Proposed by Deryck Hodge
Status: Merged
Approved by: Deryck Hodge
Approved revision: no longer in the source branch.
Merged at revision: 14217
Proposed branch: lp:~deryck/launchpad/orderbybar-integration-fixes
Merge into: lp:launchpad
Diff against target: 134 lines (+79/-1)
4 files modified
buildout-templates/bin/combine-css.in (+1/-0)
lib/lp/app/javascript/ordering/assets/ordering-core.css (+26/-0)
lib/lp/app/javascript/ordering/ordering.js (+37/-1)
lib/lp/app/javascript/ordering/tests/test_orderby_widget.js (+15/-0)
To merge this branch: bzr merge lp:~deryck/launchpad/orderbybar-integration-fixes
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Approve
Review via email: mp+80705@code.launchpad.net

Commit message

[r=bac][no-qa] Add CSS for OrderByBar and fix a bug where active-sort class wasn't being changed.

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

This change looks good Deryck. Thanks for fixing the lint issues.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'buildout-templates/bin/combine-css.in'
2--- buildout-templates/bin/combine-css.in 2011-08-30 15:06:11 +0000
3+++ buildout-templates/bin/combine-css.in 2011-10-28 19:15:28 +0000
4@@ -38,6 +38,7 @@
5 'build/picker/assets/skins/sam/picker.css',
6 'build/activator/assets/skins/sam/activator.css',
7 'build/choiceedit/assets/choiceedit-core.css',
8+ 'build/ordering/assets/ordering-core.css',
9 GALLERY_ACCORDION + 'gallery-accordion-core.css',
10 GALLERY_ACCORDION + 'skins/sam/gallery-accordion-skin.css',
11 'build/sprite.css',
12
13=== added directory 'lib/lp/app/javascript/ordering/assets'
14=== added file 'lib/lp/app/javascript/ordering/assets/ordering-core.css'
15--- lib/lp/app/javascript/ordering/assets/ordering-core.css 1970-01-01 00:00:00 +0000
16+++ lib/lp/app/javascript/ordering/assets/ordering-core.css 2011-10-28 19:15:28 +0000
17@@ -0,0 +1,26 @@
18+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
19+
20+.yui3-orderbybar {
21+ border: 1px solid #EEE;
22+ float: left;
23+ min-width: 600px;
24+ width: 100%;
25+ }
26+
27+.yui3-orderbybar p {
28+ float: left;
29+ margin: 5px;
30+ }
31+
32+.yui3-orderbybar ul li {
33+ float: left;
34+ margin: 5px 3px;
35+ padding: 0 6px;
36+ background: #DDD;
37+ border-radius: 10px;
38+ cursor: pointer;
39+ }
40+
41+.yui3-orderbybar ul li.active-sort {
42+ background: #B8B8B8;
43+ }
44
45=== added directory 'lib/lp/app/javascript/ordering/assets/skins'
46=== added directory 'lib/lp/app/javascript/ordering/assets/skins/sam'
47=== modified file 'lib/lp/app/javascript/ordering/ordering.js'
48--- lib/lp/app/javascript/ordering/ordering.js 2011-10-27 18:38:33 +0000
49+++ lib/lp/app/javascript/ordering/ordering.js 2011-10-28 19:15:28 +0000
50@@ -149,6 +149,38 @@
51 },
52
53 /**
54+ * Change the active_sort class from the previously active
55+ * node to the currently active node.
56+ *
57+ * @method _setActiveSortClassName
58+ */
59+ _setActiveSortClassName: function(preclick_sort_key) {
60+ var active = this.get('active');
61+ // We do not have to do anything if the button is already active.
62+ if (active !== preclick_sort_key) {
63+ var li_nodes = this.get('li_nodes');
64+ var len = li_nodes.length;
65+ var prev_li_id = 'sort-' + preclick_sort_key;
66+ var active_li_id = 'sort-' + active;
67+ var i,
68+ li_node,
69+ id;
70+ // Loop through the li_nodes we have and remove the
71+ // active-sort class from the previously active node
72+ // and add the class to the currently active node.
73+ for (i=0; i<len; i++) {
74+ li_node = li_nodes[i];
75+ id = li_node.get('id');
76+ if (id === prev_li_id) {
77+ li_node.removeClass('active-sort');
78+ } else if (id === active_li_id) {
79+ li_node.addClass('active-sort');
80+ }
81+ }
82+ }
83+ },
84+
85+ /**
86 * Method used to update the arrows used in the display.
87 * This will also update the widget's attributes to match
88 * the new state.
89@@ -213,6 +245,7 @@
90 // the "active" widget state.
91 var preclick_sort_key = this.get('active');
92 this.set('active', clicked_node_sort_key);
93+ this._setActiveSortClassName(preclick_sort_key);
94 // Update display and fire events.
95 this._updateSortArrows(
96 clicked_node, clicked_node_sort_key, preclick_sort_key);
97@@ -258,7 +291,10 @@
98 li_nodes.push(li_node);
99 }
100 this.set('li_nodes', li_nodes);
101- this.get('srcNode').appendChild(orderby_ul);
102+ var div = Y.Node.create('<div></div>');
103+ div.appendChild('<p>Order by:</p>');
104+ div.appendChild(orderby_ul);
105+ this.get('srcNode').appendChild(div);
106 },
107
108 /**
109
110=== modified file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.js'
111--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.js 2011-10-27 18:47:11 +0000
112+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.js 2011-10-28 19:15:28 +0000
113@@ -168,6 +168,21 @@
114 Assert.areEqual(expected_text, arrow_span.get('innerHTML'));
115 },
116
117+ test_active_sort_click_class_change: function() {
118+ // Click a node should add the active_sort class
119+ // and remove that class from the previously active node.
120+ this.makeSrcNode('test-div');
121+ this.orderby = new Y.lp.ordering.OrderByBar({
122+ srcNode: Y.one('#test-div')
123+ });
124+ this.orderby.render();
125+ var importance_node = Y.one('#sort-importance');
126+ Assert.isTrue(importance_node.hasClass('active-sort'));
127+ var status_node = Y.one('#sort-status');
128+ status_node.simulate('click');
129+ Assert.isTrue(status_node.hasClass('active-sort'));
130+ },
131+
132 test_active_sort_validator: function() {
133 // This should fail because we do not allow
134 // a "active" value not found in sort_keys.