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
=== 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:15:28 +0000
@@ -38,6 +38,7 @@
38 'build/picker/assets/skins/sam/picker.css',38 'build/picker/assets/skins/sam/picker.css',
39 'build/activator/assets/skins/sam/activator.css',39 'build/activator/assets/skins/sam/activator.css',
40 'build/choiceedit/assets/choiceedit-core.css',40 'build/choiceedit/assets/choiceedit-core.css',
41 'build/ordering/assets/ordering-core.css',
41 GALLERY_ACCORDION + 'gallery-accordion-core.css',42 GALLERY_ACCORDION + 'gallery-accordion-core.css',
42 GALLERY_ACCORDION + 'skins/sam/gallery-accordion-skin.css',43 GALLERY_ACCORDION + 'skins/sam/gallery-accordion-skin.css',
43 'build/sprite.css',44 'build/sprite.css',
4445
=== 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:15:28 +0000
@@ -0,0 +1,26 @@
1/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
2
3.yui3-orderbybar {
4 border: 1px solid #EEE;
5 float: left;
6 min-width: 600px;
7 width: 100%;
8 }
9
10.yui3-orderbybar p {
11 float: left;
12 margin: 5px;
13 }
14
15.yui3-orderbybar ul li {
16 float: left;
17 margin: 5px 3px;
18 padding: 0 6px;
19 background: #DDD;
20 border-radius: 10px;
21 cursor: pointer;
22 }
23
24.yui3-orderbybar ul li.active-sort {
25 background: #B8B8B8;
26 }
027
=== 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:15:28 +0000
@@ -149,6 +149,38 @@
149 },149 },
150150
151 /**151 /**
152 * Change the active_sort class from the previously active
153 * node to the currently active node.
154 *
155 * @method _setActiveSortClassName
156 */
157 _setActiveSortClassName: function(preclick_sort_key) {
158 var active = this.get('active');
159 // We do not have to do anything if the button is already active.
160 if (active !== preclick_sort_key) {
161 var li_nodes = this.get('li_nodes');
162 var len = li_nodes.length;
163 var prev_li_id = 'sort-' + preclick_sort_key;
164 var active_li_id = 'sort-' + active;
165 var i,
166 li_node,
167 id;
168 // Loop through the li_nodes we have and remove the
169 // active-sort class from the previously active node
170 // and add the class to the currently active node.
171 for (i=0; i<len; i++) {
172 li_node = li_nodes[i];
173 id = li_node.get('id');
174 if (id === prev_li_id) {
175 li_node.removeClass('active-sort');
176 } else if (id === active_li_id) {
177 li_node.addClass('active-sort');
178 }
179 }
180 }
181 },
182
183 /**
152 * Method used to update the arrows used in the display.184 * Method used to update the arrows used in the display.
153 * This will also update the widget's attributes to match185 * This will also update the widget's attributes to match
154 * the new state.186 * the new state.
@@ -213,6 +245,7 @@
213 // the "active" widget state.245 // the "active" widget state.
214 var preclick_sort_key = this.get('active');246 var preclick_sort_key = this.get('active');
215 this.set('active', clicked_node_sort_key);247 this.set('active', clicked_node_sort_key);
248 this._setActiveSortClassName(preclick_sort_key);
216 // Update display and fire events.249 // Update display and fire events.
217 this._updateSortArrows(250 this._updateSortArrows(
218 clicked_node, clicked_node_sort_key, preclick_sort_key);251 clicked_node, clicked_node_sort_key, preclick_sort_key);
@@ -258,7 +291,10 @@
258 li_nodes.push(li_node);291 li_nodes.push(li_node);
259 }292 }
260 this.set('li_nodes', li_nodes);293 this.set('li_nodes', li_nodes);
261 this.get('srcNode').appendChild(orderby_ul);294 var div = Y.Node.create('<div></div>');
295 div.appendChild('<p>Order by:</p>');
296 div.appendChild(orderby_ul);
297 this.get('srcNode').appendChild(div);
262 },298 },
263299
264 /**300 /**
265301
=== 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:15:28 +0000
@@ -168,6 +168,21 @@
168 Assert.areEqual(expected_text, arrow_span.get('innerHTML'));168 Assert.areEqual(expected_text, arrow_span.get('innerHTML'));
169 },169 },
170170
171 test_active_sort_click_class_change: function() {
172 // Click a node should add the active_sort class
173 // and remove that class from the previously active node.
174 this.makeSrcNode('test-div');
175 this.orderby = new Y.lp.ordering.OrderByBar({
176 srcNode: Y.one('#test-div')
177 });
178 this.orderby.render();
179 var importance_node = Y.one('#sort-importance');
180 Assert.isTrue(importance_node.hasClass('active-sort'));
181 var status_node = Y.one('#sort-status');
182 status_node.simulate('click');
183 Assert.isTrue(status_node.hasClass('active-sort'));
184 },
185
171 test_active_sort_validator: function() {186 test_active_sort_validator: function() {
172 // This should fail because we do not allow187 // This should fail because we do not allow
173 // a "active" value not found in sort_keys.188 // a "active" value not found in sort_keys.