Merge lp:~methanal-developers/methanal/form-row-striping into lp:methanal

[Jonathan Jacobs]

[Tristan Seligmann (mithrandi)]

review: Approve

[Jonathan Jacobs (jjacobs)]

Sorry to be a pain, but after actually investigating restriping on dep changes it turns out that it's not slow on IE and wasn't very hard to implement.

[Tristan Seligmann (mithrandi)]

148 + throw new Error('"pred" must be a function or null');

This should read "a function, null, or undefined".

review: Needs Fixing

[lp:~methanal-developers/methanal/form-row-striping]

144. By Jonathan Jacobs on 2010-07-08

Improve exception description.

=== modified file 'methanal/js/Methanal/Tests/TestUtil.js'
--- methanal/js/Methanal/Tests/TestUtil.js	2010-02-23 10:41:06 +0000
+++ methanal/js/Methanal/Tests/TestUtil.js	2010-07-08 18:27:43 +0000
@@ -290,6 +290,83 @@
         self.assertArraysEqual(
             Methanal.Util.divmod(23, 12),
             [1, 11]);
+    },
+
+
+    /**
+     * L{Methanal.Util.nthItem} applies a function to the nth item in an
+     * C{Array}.
+     */
+    function test_nthItem(self) {
+        var seq = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
+
+        function gatherNth(start, n) {
+            var res = []
+            Methanal.Util.nthItem(seq, start, n, function (v) {
+                res.push(v);
+            });
+            return res;
+        }
+
+        self.assertArraysEqual(
+            gatherNth(-1, 3),
+            [3, 2, 1]);
+
+        self.assertArraysEqual(
+            gatherNth(1, 0),
+            seq);
+
+        self.assertArraysEqual(
+            gatherNth(2, 0),
+            [2, 4, 6, 8, 10]);
+
+        self.assertArraysEqual(
+            gatherNth(2, 1),
+            [1, 3, 5, 7, 9]);
+
+        self.assertArraysEqual(
+            gatherNth(3, 3),
+            [3, 6, 9]);
+
+        self.assertArraysEqual(
+            gatherNth(100, 0),
+            []);
+
+        self.assertArraysEqual(
+            gatherNth(100, 1),
+            [1]);
+    },
+
+
+    /**
+     * L{Methanal.Util.filter} gathers only those elements from a sequence for
+     * which the predicate is true. A predicate of C{null} will filter values
+     * whose truth value is false, e.g. C{0}, C{''} etc.  A predicate of
+     * C{undefined} will filter values that are C{null} or C{undefined}.
+     * Otherwise the predicate must be a function.
+     */
+    function test_filter(self) {
+        var filter = Methanal.Util.filter;
+        var seq = [0, undefined, 1, null, 2, '', 3];
+
+        function isOdd(x) {
+            return x % 2 !== 0;
+        };
+
+        self.assertArraysEqual(
+            filter(null, seq),
+            [1, 2, 3]);
+
+        self.assertArraysEqual(
+            filter(undefined, seq),
+            [0, 1, 2, '', 3]);
+
+        self.assertArraysEqual(
+            filter(isOdd, filter(null, seq)),
+            [1, 3]);
+
+        self.assertThrows(Error,
+            function () { filter('hello', seq); });
     });
 
 

=== modified file 'methanal/js/Methanal/Tests/TestView.js'
--- methanal/js/Methanal/Tests/TestView.js	2010-03-09 08:57:31 +0000
+++ methanal/js/Methanal/Tests/TestView.js	2010-07-08 18:27:43 +0000
@@ -43,7 +43,7 @@
      * Create a C{Methanal.View.LiveForm}.
      */
     function createForm(self, viewOnly) {
-        var controlNames = {};
+        var controlNames = [];
         form = Methanal.Tests.TestView.MockLiveForm(controlNames, viewOnly);
         Methanal.Util.nodeInserted(form);
         return form;
@@ -169,9 +169,9 @@
     function testControls(self, controls, testingFunc) {
         var map = Methanal.Util.map;
 
-        var controlNames = {};
+        var controlNames = [];
         map(function (control) {
-            controlNames[control.name] = 1;
+            controlNames.push(control.name);
         }, controls);
 
         var form = Methanal.Tests.TestView.MockLiveForm(controlNames);

=== modified file 'methanal/js/Methanal/Util.js'
--- methanal/js/Methanal/Util.js	2010-03-16 11:55:09 +0000
+++ methanal/js/Methanal/Util.js	2010-07-08 18:27:43 +0000
@@ -365,6 +365,8 @@
 
 /**
  * Apply C{f} over each value in C{seq} and gather the results.
+ *
+ * @rtype: C{Array}
  */
 Methanal.Util.map = function map(f, seq) {
     if (typeof f !== 'function') {
@@ -380,6 +382,39 @@
 
 
 /**
+ * Gather elements from C{seq} for which C{pred(seq[index])} is C{true}.
+ *
+ * There are two special cases for C{pred}:
+ *
+ *   1. C{null} means filter out things that are not a true value (like
+ *      filter(None, seq) in Python), e.g. C{null}, C{undefined}, C{''}, C{0},
+ *      etc.
+ *
+ *   2. C{undefined} means filter out things that are C{null} or C{undefined}.
+ *
+ * @rtype: C{Array}
+ */
+Methanal.Util.filter = function filter(pred, seq) {
+    if (pred === null) {
+        pred = function (x) { return !!x; }
+    } else if (pred === undefined) {
+        pred = function (x) { return x !== null && x !== undefined; }
+    } else if (typeof pred !== 'function') {
+        throw new Error('"pred" must be a function, null or undefined');
+    }
+    var results = [];
+    for (var i = 0; i < seq.length; ++i) {
+        var x = seq[i];
+        if (pred(x)) {
+            results.push(x);
+        }
+    }
+    return results;
+};
+
+
+
+/**
  * Find the quotient and remainder of two numbers.
  */
 Methanal.Util.divmod = function divmod(x, y) {
@@ -441,6 +476,30 @@
 
 
 /**
+ * Transform an array of strings into a mapping.
+ *
+ * This has the effect of creating a collection of unique C{String} elements,
+ * like a set. For a more complete set implementation consider using
+ * L{Methanal.Util.StringSet}.
+ *
+ * @type  seq: C{Array} of C{String}
+ * @param seq: Strings to initialise the set with.
+ *
+ * @rtype: C{object} mapping C{String} to C{Boolean}
+ */
+Methanal.Util.arrayToMapping = function arrayToMapping(seq) {
+    var s = {};
+    if (seq) {
+        for (var i = 0; i < seq.length; ++i) {
+            s[seq[i]] = true;
+        }
+    }
+    return s;
+};
+
+
+
+/**
  * An unordered collection of unique C{String} elements.
  */
 Divmod.Class.subclass(Methanal.Util, 'StringSet').methods(
@@ -451,13 +510,7 @@
      * @param seq: Strings to initialise the set with
      */
     function __init__(self, seq) {
-        var s = {};
-        if (seq) {
-            for (var i = 0; i < seq.length; ++i) {
-                s[seq[i]] = true;
-            }
-        }
-        self._set = s;
+        self._set = Methanal.Util.arrayToMapping(seq);
     },
 
 
@@ -876,3 +929,43 @@
         return node;
     };
 };
+
+
+
+/**
+ * Apply a function over every nth item in a sequence.
+ *
+ * Selection is made by evaluating C{an + b}, where C{n} is each index in
+ * C{seq}. For example, every odd (first, third, fifth, etc.) item can be
+ * selected with C{a=2} and C{b=1}, and every even (second, fourth, sixth,
+ * etc.) item can be selected with C{a=2} and C{b=0}.
+ *
+ * This function is intended to match the CSS3 C{:nth-child()} pseudo-selector,
+ * and as such treats the values C{a} and C{b} as 1-based values. It can be
+ * less confusing to think about the indices as the first, second, etc. as
+ * opposed to the element at index 0, 1, etc.
+ *
+ * @type  seq: C{Array}
+ * @param seq: Sequence from which to select elements from.
+ *
+ * @type  a: C{Number}
+ * @param a: Integer value, effectively used to divide C{seq} into groups of
+ *     C{a} elements.
+ *
+ * @type  b: C{Number}
+ * @param b: Integer value, effectived used to select the C{bth} value in each
+ *     group of C{a} elements.
+ *
+ * @type  fn: C{function} taking 1 argument
+ * @param fn: Function to apply to every nth item in C{seq}.
+ *
+ * @see: U{http://www.w3.org/TR/css3-selectors/#nth-child-pseudo}
+ */
+Methanal.Util.nthItem = function nthItem(seq, a, b, fn) {
+    for (var n = 0; n <= seq.length; ++n) {
+        var index = (a * n + b) - 1;
+        if (index >= 0 && index < seq.length) {
+            fn(seq[index]);
+        }
+    }
+};

=== modified file 'methanal/js/Methanal/View.js'
--- methanal/js/Methanal/View.js	2010-07-06 14:14:44 +0000
+++ methanal/js/Methanal/View.js	2010-07-08 18:27:43 +0000
@@ -96,7 +96,9 @@
  *
  * @type update: C{function} taking C{String}, C{Array}
  * @ivar update: Called for each output control name with a sequence of values
- *     from handler functions
+ *     from handler functions, which can optionally return a C{boolean}
+ *     indicating whether anything changed in an update, which is then returned
+ *     from L{changed}.
  */
 Divmod.Class.subclass(Methanal.View, '_HandlerCache').methods(
     function __init__(self, getData, update) {
@@ -117,6 +119,7 @@
      *     relevant
      */
     function _updateOutputs(self, outputs) {
+        var updated = false;
         for (var output in outputs) {
             var results = [];
             for (var handlerID in self._outputToHandlers[output]) {
@@ -124,9 +127,10 @@
                 results.push(handler.value);
             }
             if (results.length > 0) {
-                self.update(output, results);
+                updated |= self.update(output, results);
             }
         }
+        return updated;
     },
 
 
@@ -198,6 +202,9 @@
      *
      * @type  input: C{String}
      * @param input: Input control name
+     *
+     * @rtype:  C{boolean}
+     * @return: Did any changes in the outputs occur during the updates?
      */
     function changed(self, input) {
         var handlers = self._inputToHandlers[input];
@@ -210,7 +217,7 @@
             }
         }
 
-        self._updateOutputs(outputs);
+        return self._updateOutputs(outputs);
     });
 
 
@@ -251,6 +258,9 @@
  * @type fullyLoaded: C{boolean}
  * @ivar fullyLoaded: Have all the form inputs reported that they're finished
  *     loading?
+ *
+ * @type enableControlStriping: C{boolean}
+ * @ivar enableControlStriping: Perform visual striping of active form controls?
  */
 Nevow.Athena.Widget.subclass(Methanal.View, 'FormBehaviour').methods(
     /**
@@ -288,8 +298,10 @@
         var control = self.getControl(name);
         result = Methanal.Util.reduce(_and, values, true);
         self.freeze();
+        var changed = result !== control.active;
         control.setActive(result);
         self.thaw();
+        return changed;
     },
 
 
@@ -311,11 +323,11 @@
 
         self._validatorCache = Methanal.View._HandlerCache(
             getData,
-            function (name, values) { self._validatorUpdate(name, values); });
+            function (name, values) { return self._validatorUpdate(name, values); });
 
         self._depCache = Methanal.View._HandlerCache(
             getData,
-            function (name, values) { self._depUpdate(name, values); });
+            function (name, values) { return self._depUpdate(name, values); });
 
         self.controlsLoaded = false;
         self.fullyLoaded = false;
@@ -406,6 +418,37 @@
 
 
     /**
+     * Perform control striping.
+     *
+     * If L{enableControlStriping} is C{false}, striping is not performed.
+     */
+    function _stripeControls(self) {
+        if (self.enableControlStriping) {
+            self.stripeControls();
+        }
+    },
+
+
+    /**
+     * Perform control striping.
+     */
+    function stripeControls(self) {
+        var activeControlNames = Methanal.Util.filter(function (name) {
+            var control = self.getControl(name);
+            Methanal.Util.removeElementClass(
+                control.widgetParent.node, 'methanal-control-even');
+            return control.active;
+        }, self._controlNamesOrdered);
+
+        Methanal.Util.nthItem(activeControlNames, 2, 0,
+            function (controlName) {
+                var node = self.getControl(controlName).widgetParent.node;
+                Methanal.Util.addElementClass(node, 'methanal-control-even');
+            });
+    },
+
+
+    /**
      * Perform final form initialisation tasks.
      *
      * Once all controls in L{controlNames} have reported in and L{setActions}
@@ -423,6 +466,7 @@
             Methanal.Util.addElementClass(node, 'dependancy-parent');
         }
         self.refresh();
+        self._stripeControls();
     },
 
 
@@ -503,8 +547,11 @@
      * An event that is called when a form input's value is changed.
      */
     function valueChanged(self, control) {
-        self._depCache.changed(control.name);
+        var depsChanged = self._depCache.changed(control.name);
         self.validate(control);
+        if (depsChanged) {
+            self._stripeControls();
+        }
     },
 
 
@@ -748,9 +795,14 @@
  */
 Methanal.View.FormBehaviour.subclass(Methanal.View, 'LiveForm').methods(
     function __init__(self, node, viewOnly, controlNames) {
+        self.enableControlStriping = true;
         Methanal.View.LiveForm.upcall(self, '__init__', node);
         self.viewOnly = viewOnly;
-        self.controlNames = controlNames;
+        if (!(controlNames instanceof Array)) {
+            throw new Error('"controlNames" must be an Array of control names');
+        }
+        self.controlNames = Methanal.Util.arrayToMapping(controlNames);
+        self._controlNamesOrdered = controlNames;
         self.formInit();
     },
 
@@ -1126,6 +1178,10 @@
         self.name = name;
         self.setValid();
         self.controlNames = controlNames;
+        self._controlNamesOrdered = [];
+        for (var key in self.controlNames) {
+            self._controlNamesOrdered.push(key);
+        }
     },
 
 

=== modified file 'methanal/static/styles/methanal.css'
--- methanal/static/styles/methanal.css	2010-07-06 14:14:44 +0000
+++ methanal/static/styles/methanal.css	2010-07-08 18:27:43 +0000
@@ -15,8 +15,12 @@
     float: left;
 }
 
-.methanal-control-odd {
-    background-color: #ddd;
+.methanal-form-group .methanal-control-even {
+    background-color: #d3d3ea;
+}
+
+.methanal-control-even {
+    background-color: #f0f0fc;
 }
 
 .methanal-control-error {

=== modified file 'methanal/view.py'
--- methanal/view.py	2010-03-11 08:43:14 +0000
+++ methanal/view.py	2010-07-08 18:27:43 +0000
@@ -172,8 +172,8 @@
 
 
     def getInitialArguments(self):
-        keys = (c.name for c in self.getAllControls())
-        return [dict.fromkeys(keys, 1)]
+        keys = [c.name for c in self.getAllControls()]
+        return [keys]
 
 
     def getAllControls(self):