launchpad-reviewers team mailing list archive

[Deryck Hodge <deryck.hodge@xxxxxxxxxxxxx>]

Deryck Hodge has proposed merging lp:~deryck/launchpad/orderby-bar-settings-slot into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~deryck/launchpad/orderby-bar-settings-slot/+merge/81148

This branch updates OrderByBar to allow for the addition of a config slot, a div added to the object which other widgets can use to connect themselves to OrderByBar widgets.

When config_slot is true, a rendered OrderByBar will have an empty div at the far right of the bar, with a light gray vertical separator bar.  This will look similar to the prototype found at people.canonical.com/~deryck/new-buglistings/new-buglistings.html except that the settings icon will not be present, just the div to hold that icon.  The settings icon will be handled by a new widget I'm working on, and that widget will accept the node created here as it's srcNode and place the wired up settings icon into the OrderByBar via this node.
-- 
https://code.launchpad.net/~deryck/launchpad/orderby-bar-settings-slot/+merge/81148
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~deryck/launchpad/orderby-bar-settings-slot into lp:launchpad.

=== modified file 'lib/lp/app/javascript/ordering/assets/ordering-core.css'
--- lib/lp/app/javascript/ordering/assets/ordering-core.css	2011-10-28 19:12:16 +0000
+++ lib/lp/app/javascript/ordering/assets/ordering-core.css	2011-11-03 15:48:33 +0000
@@ -24,3 +24,11 @@
 .yui3-orderbybar ul li.active-sort {
     background: #B8B8B8;
     }
+
+.yui3-orderbybar .config-widget {
+    float: right;
+    border-left: 1px solid #EEE;
+    margin: 3px;
+    min-width: 20px;
+    min-height: 20px;
+    }

=== modified file 'lib/lp/app/javascript/ordering/ordering.js'
--- lib/lp/app/javascript/ordering/ordering.js	2011-10-28 18:01:20 +0000
+++ lib/lp/app/javascript/ordering/ordering.js	2011-11-03 15:48:33 +0000
@@ -122,6 +122,31 @@
          */
         li_nodes: {
             value: null
+        },
+
+        /**
+         * Config param to signal whether we need a div added
+         * for config/settings widgets to hook onto.
+         *
+         * @attribute config_slot
+         * @type Boolean
+         * @default false
+         */
+        config_slot: {
+            value: false
+        },
+
+        /**
+         * A reference to the node created if config_slot is true.
+         * This prevents having to do DOM lookup to find the node
+         * again.
+         *
+         * @attribute config_node
+         * @type Y.Node
+         * @default null
+         */
+        config_node: {
+            value: null
         }
     };
 
@@ -294,6 +319,13 @@
             var div = Y.Node.create('<div></div>');
             div.appendChild('<p>Order by:</p>');
             div.appendChild(orderby_ul);
+            // Optionally, add a slot for any config widget to hook onto.
+            if (this.get('config_slot')) {
+                var config_div = Y.Node.create(
+                    '<div class="config-widget"></div>');
+                div.appendChild(config_div);
+                this.set('config_node', config_div);
+            }
             this.get('srcNode').appendChild(div);
         },
 

=== modified file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.html'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.html	2011-10-26 12:39:19 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.html	2011-11-03 15:48:33 +0000
@@ -14,6 +14,7 @@
 
   <!-- The module under test -->
   <script type="text/javascript" src="../ordering.js"></script>
+  <link rel="stylesheet" href="../assets/ordering-core.css" />
 
   <!-- The test suite -->
   <script type="text/javascript" src="test_orderby_widget.js"></script>

=== modified file 'lib/lp/app/javascript/ordering/tests/test_orderby_widget.js'
--- lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2011-10-28 18:01:20 +0000
+++ lib/lp/app/javascript/ordering/tests/test_orderby_widget.js	2011-11-03 15:48:33 +0000
@@ -294,6 +294,32 @@
         });
         foo_node.simulate('click');
         Assert.isTrue(event_fired);
+    },
+
+    test_add_settings_slot: function() {
+        // The widget optionally can add a div for settings/config
+        // widgets to hook onto.
+        this.makeSrcNode('test-div');
+        this.orderby = new Y.lp.ordering.OrderByBar({
+            srcNode: Y.one('#test-div'),
+            config_slot: true
+        });
+        this.orderby.render();
+        var config_slot = Y.one('#test-div').one('.config-widget');
+        Assert.isNotNull(config_slot);
+    },
+
+    test_settings_slot_node_attribute: function() {
+        // The widget keeps a reference to the settings slot
+        // node if config_slot is true.
+        this.makeSrcNode('test-div');
+        this.orderby = new Y.lp.ordering.OrderByBar({
+            srcNode: Y.one('#test-div'),
+            config_slot: true
+        });
+        this.orderby.render();
+        var config_slot = Y.one('#test-div').one('.config-widget');
+        Assert.areEqual(config_slot, this.orderby.get('config_node'));
     }
 }));