launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05331
[Merge] lp:~deryck/launchpad/buglists-orderby-widget into lp:launchpad
Deryck Hodge has proposed merging lp:~deryck/launchpad/buglists-orderby-widget into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~deryck/launchpad/buglists-orderby-widget/+merge/80563
This branch creates a new OrderByBar widget for use in the CustomBugListings feature. The goal of the widget is to provide for buttons that can be clicked allowing search results to be resorted dynamically.
There is an HTML prototype where you can see the bar in action here:
http://people.canonical.com/~deryck/new-buglistings/new-buglistings.html
The LEP for the feature is here:
https://dev.launchpad.net/LEP/CustomBugListings
This branch adds the widget and test coverage. As the comment in the file says:
A menu bar for quickly reordering a list of items on a page.
This widget is used for managing the bar, it's buttons, and
the state of the ordering.
The widget doesn't actually change the page state based on
requested ordering. It fires an event to signal that a sort
order update is required.
So this is not integrated into Launchpad at all. The work here is to cut over from my HTML prototype to a widget that can be generally reusable for this sort of interaction. The next branch from me will bring this into bug search results page and integrate it with Aaron's work to get client-side rendering of bug lists.
There are two things left undone that I'd like to save for the follow-up integration work branch.
* Adding CSS for the widget
* Remove use of "asc" and "desc" class names in HTML
The latter is a holdover from how I tracked the sort order state in an HTML prototype and this could all be handled by the widget now, rather than via CSS classes.
--
https://code.launchpad.net/~deryck/launchpad/buglists-orderby-widget/+merge/80563
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/buglists-orderby-widget into lp:launchpad.
=== added directory 'lib/lp/app/javascript/ordering'
=== added file 'lib/lp/app/javascript/ordering/ordering.js'
--- lib/lp/app/javascript/ordering/ordering.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/ordering/ordering.js 2011-10-27 13:16:26 +0000
@@ -0,0 +1,275 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI().add('lp.ordering', function(Y) {
+ /**
+ * A menu bar for quickly reordering a list of items on a page.
+ * This widget is used for managing the bar, it's buttons, and
+ * the state of the ordering.
+ *
+ * The widget doesn't actually change the page state based on
+ * requested ordering. It fires an event to signal that a sort
+ * order update is required.
+ *
+ * External code should listen for orderbybar:sort to know when
+ * to update the page.
+ *
+ * @module lp.ordering
+ */
+ function OrderByBar() {
+ OrderByBar.superclass.constructor.apply(this, arguments);
+ }
+
+ OrderByBar.NAME = 'orderbybar';
+
+ OrderByBar.ATTRS = {
+
+ /**
+ * A list of [key, name] pairs where key is the sorting key
+ * as used in a URL, i.e. ?orderby=importance, and name is
+ * the display name for the button of the menu bar.
+ *
+ * We could probably get away without a default here, since
+ * this will always be changed. But having a default allows
+ * the widget to show something if you create an OrderByBar
+ * without any config.
+ *
+ * @attribute sort_keys
+ * @type Array
+ */
+ sort_keys: {
+ value: [
+ ['bugnumber', 'Bug number'],
+ ['bugtitle', 'Bug title'],
+ ['status', 'Status'],
+ ['importance', 'Importance'],
+ ['bug-heat-icons', 'Bug heat'],
+ ['package', 'Package name'],
+ ['milestone', 'Milestone'],
+ ['assignee', 'Assignee'],
+ ['bug-age', 'Bug age']
+ ]
+ },
+
+
+ /**
+ * The active sort key. This should coorespond to the current
+ * view of the data on the web page.
+ *
+ * Again, defaults will likely be overwritten with any use.
+ *
+ * @attribute active
+ * @type String
+ * @default importance
+ */
+ active: {
+ value: 'importance'
+ },
+
+ /**
+ * The current sort order, either desc or asc.
+ *
+ * @attribute sort_order
+ * @type String
+ * @default 'asc'
+ */
+ sort_order: {
+ value: 'asc',
+ validator: function(value) {
+ // Fail big here, so call sites know what went wrong.
+ if (value === 'asc' || value === 'desc') {
+ return true;
+ } else {
+ throw Error('sort_order must be either "asc" or "desc"');
+ }
+ }
+ },
+
+ /**
+ * The constructed sort key passed to a URL. Used internally
+ * to track the sort key and ordering, i.e. "-importance" vs.
+ * "importance".
+ *
+ * @attribute sort_clause
+ * @type String
+ * @default null
+ *
+ */
+ sort_clause: {
+ value: null
+ },
+
+ /**
+ * Cache for the created li nodes.
+ *
+ * This allows the widget to refer back to the nodes
+ * without having to look them up again via DOM trafersal.
+ *
+ * @attribute li_nodes
+ * @type Array
+ * @default null
+ */
+ li_nodes: {
+ value: null
+ }
+ };
+
+ OrderByBar.LI_TEMPLATE = [
+ '<li id="{li_id}">{li_label}',
+ '<span class="sort-arr"></span></li>'].join('');
+
+ Y.extend(OrderByBar, Y.Widget, {
+
+ /**
+ * Method used by the widget to fire custom event to
+ * signal a new sorting is required.
+ *
+ * @method _fireSortEvent
+ */
+ _fireSortEvent: function() {
+ var prefix = '';
+ if (this.get('sort_order') === 'desc') {
+ prefix = '-';
+ }
+ var sort_clause = prefix + this.get('active');
+ this.set('sort_clause', sort_clause);
+ var event_name = this.constructor.NAME + ':sort';
+ Y.fire(event_name, sort_clause);
+ },
+
+ /**
+ * Method used to update the arrows used in the display.
+ * This will also update the widget's attributes to match
+ * the new state.
+ *
+ * @method _updateSortArrows
+ */
+ _updateSortArrows: function(
+ 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 is_active_sort_button = false;
+ if (clicked_node_sort_key === preclick_sort_key) {
+ is_active_sort_button = true;
+ }
+ 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);
+ arrow_span.addClass('asc');
+ arrow_span.removeClass('desc');
+ this.set('sort_order', 'asc');
+ } else {
+ arrow_span.set('innerHTML', up_arrow);
+ arrow_span.addClass('desc');
+ arrow_span.removeClass('asc');
+ this.set('sort_order', 'desc');
+ }
+ } else {
+ // We have a different sort order clicked and need to
+ // remove arrow from recently active sort button as
+ // well as add an arrow to a new button.
+ var old_active_sort_key = '#sort-' + preclick_sort_key;
+ 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', '');
+ 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);
+ }
+ },
+
+ /**
+ * Handle the click of one of the li nodes.
+ *
+ * @method _handleClick
+ */
+ _handleClick: function(clicked_node) {
+ // Reverse from the node's ID to the sort key, i.e.
+ // "sort-foo" gives us "foo" as the sort key.
+ var clicked_node_sort_key = clicked_node.get('id').replace(
+ 'sort-', '');
+ // Get a reference to what was active before click and update
+ // the "active" widget state.
+ var preclick_sort_key = this.get('active');
+ this.set('active', clicked_node_sort_key);
+ // Update display and fire events.
+ this._updateSortArrows(
+ clicked_node, clicked_node_sort_key, preclick_sort_key);
+ this._fireSortEvent();
+ },
+
+
+ /**
+ * Create the bar, the li nodes used for buttons, and
+ * append to the page via the provided srcNode.
+ *
+ * @method renderUI
+ */
+ renderUI: function() {
+ var orderby_ul = Y.Node.create('<ul></ul>');
+ var keys = this.get('sort_keys');
+ var len = keys.length;
+ var li_nodes = [];
+ var i,
+ id,
+ label,
+ li_html,
+ li_node,
+ sort_order;
+ for (i=0; i<len; i++) {
+ id = keys[i][0];
+ label = keys[i][1];
+ li_html = Y.Lang.substitute(
+ this.constructor.LI_TEMPLATE,
+ {li_id: 'sort-' + id, li_label: label});
+ li_node = Y.Node.create(li_html);
+ if (this.get('active') === id) {
+ li_node.addClass('active-sort');
+ 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', '↓');
+ } else if (sort_order === 'desc') {
+ li_node.one('span').set('innerHTML', '↑');
+ }
+ }
+ orderby_ul.appendChild(li_node);
+ li_nodes.push(li_node);
+ }
+ this.set('li_nodes', li_nodes);
+ this.get('srcNode').appendChild(orderby_ul);
+ },
+
+ /**
+ * Add click listeners to the li nodes used as buttons.
+ *
+ * @method bindUI
+ */
+ bindUI: function() {
+ var li_nodes = this.get('li_nodes');
+ var len = li_nodes.length;
+ var that = this;
+ var i,
+ li_node;
+ for (i=0; i<len; i++) {
+ li_node = li_nodes[i];
+ li_node.on('click', function(e) {
+ that._handleClick(this);
+ });
+ }
+ }
+ });
+
+ var ordering = Y.namespace('lp.ordering');
+ ordering.OrderByBar = OrderByBar;
+
+}, '0.1', {'requires': ['widget']});
=== added directory 'lib/lp/app/javascript/ordering/tests'
=== added file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.html'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.html 2011-10-27 13:16:26 +0000
@@ -0,0 +1,27 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+ "http://www.w3.org/TR/html4/strict.dtd";>
+<html>
+ <head>
+ <title>Order By Widget Tests</title>
+
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../../canonical/launchpad/icing/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+ <script type="text/javascript"
+ src="../../../../app/javascript/testing/testrunner.js"></script>
+
+ <!-- The module under test -->
+ <script type="text/javascript" src="../ordering.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="test_orderby_widget.js"></script>
+
+</head>
+<body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.orderbybar.test</li>
+ </ul>
+</body>
+</html>
=== added file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.js'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.js 2011-10-27 13:16:26 +0000
@@ -0,0 +1,264 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.orderbybar.test', function(Y) {
+
+var basic_test = Y.namespace('lp.orderbybar.test');
+
+var suite = new Y.Test.Suite('OrderByBar Tests');
+
+var Assert = Y.Assert;
+var ArrayAssert = Y.ArrayAssert;
+
+suite.add(new Y.Test.Case({
+
+ name: 'orderbybar_widget_tests',
+
+ _should: {
+ error: {
+ test_sort_order_validator:
+ new Error('sort_order must be either "asc" or "desc"')
+ }
+ },
+
+ /**
+ * Unpack a list of key, name pairs into individual lists.
+ *
+ * [[Foo, 'Foo Item'], ['Bar', 'Bar item']] becomes
+ * ['Foo', 'Bar'] and ['Foo Item', 'Bar Item'].
+ */
+ getIdsAndNames: function(keys) {
+ var ids = [];
+ var names = [];
+ var len = keys.length;
+ var i;
+ for (i=0; i<len; i++) {
+ ids.push(keys[i][0]);
+ names.push(keys[i][1]);
+ }
+ return [ids, names];
+ },
+
+ /*
+ * Helper function to create the srcNode on the page. Widgets
+ * will append to the body tag if srcNode is not supplied.
+ */
+ makeSrcNode: function(id) {
+ // Calling the widget's destroy method will clean this up.
+ var parent_node = Y.Node.create('<div></div>');
+ parent_node.set('id', id);
+ Y.one('body').appendChild(parent_node);
+ },
+
+ test_default_sort_keys: function() {
+ // The default sort keys shoudl exist in a newly created widget.
+ var orderby = new Y.lp.ordering.OrderByBar();
+ var expected_sort_keys = [
+ ['bugnumber', 'Bug number'],
+ ['bugtitle', 'Bug title'],
+ ['status', 'Status'],
+ ['importance', 'Importance'],
+ ['bug-heat-icons', 'Bug heat'],
+ ['package', 'Package name'],
+ ['milestone', 'Milestone'],
+ ['assignee', 'Assignee'],
+ ['bug-age', 'Bug age']
+ ];
+ var expected = this.getIdsAndNames(expected_sort_keys);
+ var actual = this.getIdsAndNames(orderby.get('sort_keys'));
+ ArrayAssert.itemsAreSame(expected[0], actual[0]);
+ ArrayAssert.itemsAreSame(expected[1], actual[1]);
+ },
+
+ test_user_supplied_sort_keys: function() {
+ // Call sites can supply their own sort keys to a widget.
+ var user_supplied_sort_keys = [
+ ['foo', 'Foo item'],
+ ['bar', 'Bar item'],
+ ['baz', 'Baz item']
+ ];
+ var orderby = new Y.lp.ordering.OrderByBar({
+ sort_keys: user_supplied_sort_keys});
+ var expected = this.getIdsAndNames(user_supplied_sort_keys);
+ var actual = this.getIdsAndNames(orderby.get('sort_keys'));
+ ArrayAssert.itemsAreSame(expected[0], actual[0]);
+ ArrayAssert.itemsAreSame(expected[1], actual[1]);
+ },
+
+ test_rendered_items_html: function() {
+ // We should be able to get a node from the DOM via an ID
+ // created from sort keys, and the name should be used as
+ // a button display name in HTML.
+ var test_sort_keys = [
+ ['foo', 'Foo item'],
+ ['bar', 'Bar item']
+ ];
+ this.makeSrcNode('test-div');
+ var orderby = new Y.lp.ordering.OrderByBar({
+ sort_keys: test_sort_keys,
+ srcNode: Y.one('#test-div'),
+ active: 'foo'
+ });
+ orderby.render();
+ var foo_node = Y.one('#sort-foo');
+ Assert.isNotNull(foo_node);
+ Assert.areEqual(foo_node.get('firstChild').get('text'), 'Foo item');
+ var bar_node = Y.one('#sort-bar');
+ Assert.isNotNull(bar_node);
+ Assert.areEqual(bar_node.get('firstChild').get('text'), 'Bar item');
+ orderby.destroy();
+ },
+
+ test_render_active_sort_default: function() {
+ // Confirm that there is a default active sort class applied.
+ this.makeSrcNode('test-div');
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div')
+ });
+ orderby.render();
+ var li_node = Y.one('#sort-importance');
+ Assert.isTrue(li_node.hasClass('active-sort'));
+ orderby.destroy();
+ },
+
+ test_render_active_sort_user_supplied: function() {
+ // The active sort class is also set when "active"
+ // is supplied via config.
+ this.makeSrcNode('test-div');
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ active: 'status'
+ });
+ orderby.render();
+ var li_node = Y.one('#sort-status');
+ Assert.isTrue(li_node.hasClass('active-sort'));
+ orderby.destroy();
+ },
+
+ test_active_sort_arrow_display_asc: function() {
+ // Buttons using "asc" order get a down arrow added to the li.
+ this.makeSrcNode('test-div');
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ sort_order: 'asc'
+ });
+ orderby.render();
+ var arrow_span = Y.one('.active-sort span');
+ var expected_text = Y.Node.create('↓').get('text');
+ Assert.areEqual(expected_text, arrow_span.get('innerHTML'));
+ orderby.destroy();
+ },
+
+ test_active_sort_arrow_display_desc: function() {
+ // Buttons using "desc" order get an up arrow added to the li.
+ this.makeSrcNode('test-div');
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ sort_order: 'desc'
+ });
+ orderby.render();
+ var arrow_span = Y.one('.active-sort span');
+ var expected_text = Y.Node.create('↑').get('text');
+ Assert.areEqual(expected_text, arrow_span.get('innerHTML'));
+ orderby.destroy();
+ },
+
+ test_sort_order_validator: function() {
+ // This should fail when using a sort order
+ // other than "asc" or "desc".
+ var orderby = new Y.lp.ordering.OrderByBar({
+ sort_order: 'foobar'
+ });
+ orderby.render();
+ },
+
+ test_click_current_sort_arrow_changes: function() {
+ // Clicking the currently sorted on button should change
+ // the arrow and widget state to show a sort change should
+ // happen.
+ this.makeSrcNode('test-div');
+ var test_sort_keys = [
+ ['foo', 'Foo item'],
+ ['bar', 'Bar item']
+ ];
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ sort_keys: test_sort_keys,
+ active: 'foo',
+ sort_order: 'asc'
+ });
+ orderby.render();
+ var foo_node = Y.one('#sort-foo');
+ var expected_starting_text = Y.Node.create('↓').get('text');
+ var expected_ending_text = Y.Node.create('↑').get('text');
+ Assert.areEqual(
+ expected_starting_text, foo_node.one('span').get('innerHTML'));
+ Assert.isTrue(foo_node.one('span').hasClass('asc'));
+ foo_node.simulate('click');
+ Assert.areEqual(
+ expected_ending_text, foo_node.one('span').get('innerHTML'));
+ Assert.isTrue(foo_node.one('span').hasClass('desc'));
+ orderby.destroy();
+ },
+
+ test_click_different_sort_arrows_change: function() {
+ // Clicking a button other than the currently sorted on button
+ // should change the arrow and widget state to show a sort
+ // change should happen.
+ this.makeSrcNode('test-div');
+ var test_sort_keys = [
+ ['foo', 'Foo item'],
+ ['bar', 'Bar item']
+ ];
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ sort_keys: test_sort_keys,
+ active: 'foo',
+ sort_order: 'asc'
+ });
+ orderby.render();
+ var bar_node = Y.one('#sort-bar');
+ bar_node.simulate('click');
+ var expected_arrow = Y.Node.create('↓').get('text');
+ Assert.areEqual(
+ expected_arrow, bar_node.one('span').get('innerHTML'));
+ Assert.isTrue(bar_node.one('span').hasClass('asc'));
+ // Ensure the original button doesn't have sort classes.
+ Assert.isFalse(Y.one('#sort-foo').one('span').hasClass('asc'));
+ Assert.isFalse(Y.one('#sort-foo').one('span').hasClass('desc'));
+ orderby.destroy();
+ },
+
+ test_sort_event_fires_with_data: function() {
+ // A custom sort event fires from the widget to signal a
+ // sort order change should happen in the page. The
+ // callback receives the objects sort_clause for use in
+ // a URL.
+ this.makeSrcNode('test-div');
+ var test_sort_keys = [
+ ['foo', 'Foo item'],
+ ['bar', 'Bar item']
+ ];
+ var orderby = new Y.lp.ordering.OrderByBar({
+ srcNode: Y.one('#test-div'),
+ sort_keys: test_sort_keys,
+ active: 'foo',
+ sort_order: 'asc'
+ });
+ orderby.render();
+ var foo_node = Y.one('#sort-foo');
+ var event_fired = false;
+ Y.on('orderbybar:sort', function(e) {
+ event_fired = true;
+ // Confirm that we get the sort statement we expect, too.
+ Assert.areEqual('-foo', e);
+ });
+ foo_node.simulate('click');
+ Assert.isTrue(event_fired);
+ orderby.destroy();
+ }
+
+}));
+
+basic_test.suite = suite;
+
+}, '0.1', {'requires': ['test', 'node-event-simulate', 'lp.ordering']});
