launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04485
[Merge] lp:~allenap/launchpad/javascript-utils into lp:launchpad
Gavin Panella has proposed merging lp:~allenap/launchpad/javascript-utils into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/launchpad/javascript-utils/+merge/70531
This adds a new lp.extras JavaScript module that has two functions,
attrgetter and attrselect. The first is very similar to Python's
attrgetter, and attrselect is basically a convenience for mapping
attrgetter over an array.
It also adds a map method to Y.NodeList (and a static version too)
that allows you to... well, map over a NodeList. I've been baffled for
a long time that it doesn't exist. It's just such an obvious thing to
want to do!
I was already using some copy-n-pasted versions of attrselect, so I've
switched those over to using the implementation in lp.extras. Also,
I've changed one place to use NodeList.map, where before it had to do
a_node_list.each(function(node) { an_array.push(node); }) to get
something mappable.
I am receptive to bikeshedding of the name lp.extras.
--
https://code.launchpad.net/~allenap/launchpad/javascript-utils/+merge/70531
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/javascript-utils into lp:launchpad.
=== added directory 'lib/lp/app/javascript/extras'
=== added file 'lib/lp/app/javascript/extras/extras.js'
--- lib/lp/app/javascript/extras/extras.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/extras/extras.js 2011-08-05 09:06:40 +0000
@@ -0,0 +1,78 @@
+/**
+ * Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Things that YUI3 really needs.
+ *
+ * @module lp
+ * @submodule extras
+ */
+
+YUI.add('lp.extras', function(Y) {
+
+Y.log('loading lp.extras');
+
+var namespace = Y.namespace("lp.extras"),
+ NodeList = Y.NodeList;
+
+/**
+ * NodeList is crying out for map.
+ * @static
+ *
+ * @param {Y.NodeList|Array} instance The node list or array of nodes
+ * (Node or DOM nodes) to map over.
+ * @param {Function} fn The function to apply. It receives 1 argument:
+ * the current Node instance.
+ * @param {Object} context optional An optional context to apply the
+ * function with. The default context is the current Node
+ * instance.
+ */
+NodeList.map = function(instance, fn, context) {
+ return NodeList.getDOMNodes(instance).map(Y.one).map(
+ function(node) {
+ return fn.call(context || node, node);
+ }
+ );
+};
+
+/**
+ * NodeList is crying out for map.
+ *
+ * @param {Function} fn The function to apply. It receives 1 argument:
+ * the current Node instance.
+ * @param {Object} context optional An optional context to apply the
+ * function with. The default context is the current Node
+ * instance.
+ */
+NodeList.prototype.map = function(fn, context) {
+ return NodeList.map(this, fn, context);
+};
+
+/**
+ * Returns a function that gets the named attribute from an object.
+ *
+ * @param {String} name The attribute to get.
+ */
+var attrgetter = function(name) {
+ return function(thing) {
+ return thing[name];
+ };
+};
+
+/**
+ * Returns a function that gets the named attribute from an array of
+ * objects (returning those attributes as an array).
+ *
+ * @param {String} name The attribute to select.
+ */
+var attrselect = function(name) {
+ return function(things) {
+ return Y.Array(things).map(attrgetter(name));
+ };
+};
+
+// Exports.
+namespace.attrgetter = attrgetter;
+namespace.attrselect = attrselect;
+
+}, "0.1", {requires: ["array-extras", "node"]});
=== added directory 'lib/lp/app/javascript/extras/tests'
=== added file 'lib/lp/app/javascript/extras/tests/test_extras.html'
--- lib/lp/app/javascript/extras/tests/test_extras.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/extras/tests/test_extras.html 2011-08-05 09:06:40 +0000
@@ -0,0 +1,29 @@
+<!DOCTYPE
+ HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+ "http://www.w3.org/TR/html4/strict.dtd";>
+<html>
+ <head>
+ <title>Launchpad Extras</title>
+
+ <!-- YUI and test setup -->
+ <link rel="stylesheet" href="../../testing/test.css" />
+ <script type="text/javascript"
+ src="../../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
+ <script type="text/javascript"
+ src="../../testing/testrunner.js"></script>
+
+ <!-- The module under test -->
+ <script type="text/javascript"
+ src="../extras.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript"
+ src="test_extras.js"></script>
+
+ </head>
+ <body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.extras.test</li>
+ </ul>
+ </body>
+</html>
=== added file 'lib/lp/app/javascript/extras/tests/test_extras.js'
--- lib/lp/app/javascript/extras/tests/test_extras.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/extras/tests/test_extras.js 2011-08-05 09:06:40 +0000
@@ -0,0 +1,113 @@
+/**
+ * Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Tests for Extras.
+ *
+ * @module lp.extras
+ * @submodule test
+ */
+
+YUI.add('lp.extras.test', function(Y) {
+
+ var namespace = Y.namespace('lp.extras.test');
+
+ var Assert = Y.Assert;
+ var ArrayAssert = Y.ArrayAssert;
+
+ var suite = new Y.Test.Suite("extras Tests");
+ var extras = Y.lp.extras;
+
+ var TestNodeListMap = {
+ name: 'TestNodeListMap',
+
+ test_static: function() {
+ var nodes = [
+ Y.Node.create("<div />"),
+ Y.Node.create("<label />"),
+ Y.Node.create("<strong />")
+ ];
+ ArrayAssert.itemsAreSame(
+ nodes,
+ Y.NodeList.map(
+ nodes, function(node) { return node; })
+ );
+ ArrayAssert.itemsAreSame(
+ ["DIV", "LABEL", "STRONG"],
+ Y.NodeList.map(nodes, function(node) {
+ return node.get("tagName");
+ })
+ );
+ },
+
+ test_static_with_DOM_nodes: function() {
+ // NodeList.map converts DOM nodes into Y.Node instances.
+ var nodes = [
+ document.createElement("div"),
+ document.createElement("label"),
+ document.createElement("strong")
+ ];
+ Y.NodeList.map(nodes, function(node) {
+ Assert.isInstanceOf(Y.Node, node);
+ });
+ ArrayAssert.itemsAreSame(
+ ["DIV", "LABEL", "STRONG"],
+ Y.NodeList.map(nodes, function(node) {
+ return node.get("tagName");
+ })
+ );
+ },
+
+ test_method: function() {
+ var nodes = [
+ Y.Node.create("<div />"),
+ Y.Node.create("<label />"),
+ Y.Node.create("<strong />")
+ ];
+ var nodelist = new Y.NodeList(nodes);
+ ArrayAssert.itemsAreSame(
+ nodes,
+ nodelist.map(function(node) { return node; })
+ );
+ ArrayAssert.itemsAreSame(
+ ["DIV", "LABEL", "STRONG"],
+ nodelist.map(function(node) {
+ return node.get("tagName");
+ })
+ );
+ }
+
+ };
+
+ var TestAttributeFunctions = {
+ name: 'TestAttributeFunctions',
+
+ test_attrgetter: function() {
+ var subject = {foo: 123, bar: 456};
+ Assert.areSame(123, extras.attrgetter("foo")(subject));
+ Assert.areSame(456, extras.attrgetter("bar")(subject));
+ },
+
+ test_attrselect: function() {
+ var subject = [
+ {foo: 1, bar: 5},
+ {foo: 2, bar: 6},
+ {foo: 3, bar: 7},
+ {foo: 4, bar: 8}
+ ];
+ ArrayAssert.itemsAreSame(
+ [1, 2, 3, 4], extras.attrselect("foo")(subject));
+ ArrayAssert.itemsAreSame(
+ [5, 6, 7, 8], extras.attrselect("bar")(subject));
+ }
+
+ };
+
+ // Populate the suite.
+ suite.add(new Y.Test.Case(TestNodeListMap));
+ suite.add(new Y.Test.Case(TestAttributeFunctions));
+
+ // Exports.
+ namespace.suite = suite;
+
+}, "0.1", {requires: ["test", "lp.extras"]});
=== modified file 'lib/lp/app/javascript/formwidgets/formwidgets.js'
--- lib/lp/app/javascript/formwidgets/formwidgets.js 2011-07-18 10:50:01 +0000
+++ lib/lp/app/javascript/formwidgets/formwidgets.js 2011-08-05 09:06:40 +0000
@@ -386,13 +386,7 @@
*/
choices: {
getter: function() {
- /* I think this is a YUI3 wart; I can't see any way to
- map() over a NodeList, so I must push the elements
- one by one into an array first. */
- var options = Y.Array([]);
- this.fieldNode.all("select > option").each(
- function(option) { options.push(option); });
- return options.map(
+ return this.fieldNode.all("select > option").map(
function(option) {
return {
value: option.get("value"),
@@ -623,4 +617,5 @@
}, "0.1", {"requires": ["node", "dom", "io", "widget", "lp.client",
- "lazr.anim", "array-extras", "transition"]});
+ "lp.extras", "lazr.anim", "array-extras",
+ "transition"]});
=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_formwidgets.html'
--- lib/lp/app/javascript/formwidgets/tests/test_formwidgets.html 2011-07-18 10:50:01 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_formwidgets.html 2011-08-05 09:06:40 +0000
@@ -23,12 +23,8 @@
src="../../lazr/lazr.js"></script>
<script type="text/javascript"
src="../../overlay/overlay.js"></script>
- <!-- <script type="text/javascript" -->
- <!-- src="../../picker/picker.js"></script> -->
- <!-- <script type="text/javascript" -->
- <!-- src="../../picker/person_picker.js"></script> -->
- <!-- <script type="text/javascript" -->
- <!-- src="../../picker/picker_patcher.js"></script> -->
+ <script type="text/javascript"
+ src="../../extras/extras.js"></script>
<!-- The module under test -->
<script type="text/javascript"
=== modified file 'lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js'
--- lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js 2011-07-18 10:50:01 +0000
+++ lib/lp/app/javascript/formwidgets/tests/test_formwidgets.js 2011-08-05 09:06:40 +0000
@@ -18,17 +18,7 @@
var suite = new Y.Test.Suite("formwidgets Tests");
var widgets = Y.lp.app.formwidgets;
- var attrgetter = function(name) {
- return function(thing) {
- return thing[name];
- };
- };
-
- var attrselect = function(name) {
- return function(things) {
- return Y.Array(things).map(attrgetter(name));
- };
- };
+ var attrselect = Y.lp.extras.attrselect;
var testFormRowWidget = {
name: 'TestFormRowWidget',
@@ -602,4 +592,4 @@
}, "0.1", {"requires": [
'test', 'console', 'node-event-simulate',
- 'lp.app.formwidgets']});
+ 'lp.app.formwidgets', 'lp.extras']});
=== modified file 'lib/lp/registry/javascript/distroseries/tests/test_widgets.html'
--- lib/lp/registry/javascript/distroseries/tests/test_widgets.html 2011-07-18 10:50:01 +0000
+++ lib/lp/registry/javascript/distroseries/tests/test_widgets.html 2011-08-05 09:06:40 +0000
@@ -31,6 +31,8 @@
src="../../../../app/javascript/picker/picker_patcher.js"></script>
<script type="text/javascript"
src="../../../../app/javascript/formwidgets/formwidgets.js"></script>
+ <script type="text/javascript"
+ src="../../../../app/javascript/extras/extras.js"></script>
<!-- The module under test -->
<script type="text/javascript"
=== modified file 'lib/lp/registry/javascript/distroseries/tests/test_widgets.js'
--- lib/lp/registry/javascript/distroseries/tests/test_widgets.js 2011-07-18 10:54:57 +0000
+++ lib/lp/registry/javascript/distroseries/tests/test_widgets.js 2011-08-05 09:06:40 +0000
@@ -19,17 +19,7 @@
var widgets = Y.lp.registry.distroseries.widgets;
var formwidgets = Y.lp.app.formwidgets;
- var attrgetter = function(name) {
- return function(thing) {
- return thing[name];
- };
- };
-
- var attrselect = function(name) {
- return function(things) {
- return Y.Array(things).map(attrgetter(name));
- };
- };
+ var attrselect = Y.lp.extras.attrselect;
var testParentSeriesListWidget = {
name: 'TestParentSeriesListWidget',
@@ -487,4 +477,4 @@
}, "0.1", {"requires": [
'test', 'console', 'node-event-simulate',
'lp.registry.distroseries.widgets', 'lp.app.formwidgets',
- 'lp.app.formwidgets.test']});
+ 'lp.app.formwidgets.test', 'lp.extras']});
Follow ups
