Merge lp:~rharding/launchpad/replace_foldables into lp:launchpad

[Richard Harding]

[r=wallyworld][no-qa] Remove the mochikit foldable ui interaction from being used, replace with YUI lp.app.foldable.

= Summary =
We're removing mochikit library which this foldable ui interaction relies on. In discussions with Curtis and others, it was determined that this was an effort to help with spam management. Since that issues isn't as prevalent these days, it won't be missed if the feature was just removed.

== Proposed Fix ==
Remove the function.

== Implementation Details ==
Removed

== Tests ==
google-chrome lib/lp/app/javascript/tests/test_foldable.html

== Demo and Q/A ==
Load a bug with a quoted comment and verify that you can close/

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/foldable.js
  lib/lp/app/javascript/tests/test_foldable.html
  lib/lp/app/javascript/tests/test_foldable.js

[Ian Booth (wallyworld)]

Looks good. I have some questions/suggstions:

1. init method name: Y.lp.app.foldable.init();

Should we make it Y.lp.app.foldable.activate_foldables();
to be consistent with activate_collapsables() naming? The suggested name is also more indicative of what we are doing ie lookign for all foldable elements and activating each one. init() implies more that a module is being initialised rather than many page elements.

2. The old code:
if (quoted_lines && quoted_lines.length <= 11) {
was replaced with
if (quoted_lines && quoted_lines.size() <= 12) {

Which is correct?
I'd like to see a test around the corner case of 12 lines
ie test that exactly 12 lines is not folded but 13 lines is

review: Needs Fixing (code)

[Ian Booth (wallyworld)]

Excellent. Thanks for adding the suggested improvements.

review: Approve (code)

=== added file 'lib/lp/app/javascript/foldables.js'
--- lib/lp/app/javascript/foldables.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/foldables.js	2012-01-10 17:43:30 +0000
@@ -0,0 +1,79 @@
+/**
+ * Copyright 2011 Canonical Ltd. This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Making foldable sections of html
+ *
+ * Usage:
+ *  lp.app.foldables.activate();
+ *
+ * @module lp.app.foldables
+ */
+
+YUI.add('lp.app.foldables', function(Y) {
+
+    var module = Y.namespace('lp.app.foldables');
+    var VOID_URL = '_:void(0);'.replace('_', 'javascript');
+
+    var toggleFoldable = function(e) {
+        var ELEMENT_NODE = 1;
+        var node = e.currentTarget;
+        while (node.get('nextSibling')) {
+            node = node.get('nextSibling');
+            if (node.get('nodeType') !== ELEMENT_NODE) {
+                continue;
+            }
+            if (!node.hasClass('foldable') &&
+                !node.hasClass('foldable-quoted')) {
+                continue;
+            }
+            if (node.getStyle('display') === 'none') {
+                node.setStyle('display', 'inline');
+            } else {
+                node.setStyle('display', 'none');
+            }
+        }
+    };
+
+    module.activate = function () {
+        // Create links to toggle the display of foldable content.
+        var included = Y.all('span.foldable');
+        var quoted = Y.all('span.foldable-quoted');
+        quoted.each(function (n) {
+            included.push(n);
+        });
+
+        included.each(function (span, index, list) {
+            if (span.hasClass('foldable-quoted')) {
+                var quoted_lines = span.all('br');
+                debugger;
+                if (quoted_lines && quoted_lines.size() <= 11) {
+                    // We do not hide short quoted passages (12 lines) by
+                    // default.
+                    return;
+                }
+            }
+
+            var ellipsis = Y.Node.create('<a/>');
+            ellipsis.setStyle('textDecoration', 'underline');
+            ellipsis.set('href', VOID_URL);
+            ellipsis.on('click', toggleFoldable);
+            ellipsis.appendChild(Y.Node.create('[...]'));
+
+            span.get('parentNode').insertBefore(ellipsis, span);
+            span.insertBefore(Y.Node.create('<br/>'), span.get('firstChild'));
+            span.setStyle('display', 'none');
+            if (span.get('nextSibling')) {
+                // Text lines follows this span.
+                var br = Y.Node.create('<br/>');
+                span.get('parentNode').insertBefore(
+                    br,
+                    span.get('nextSibling')
+                );
+            }
+        });
+    };
+
+}, '0.1.0', {
+    requires: ['base', 'node']
+});

=== modified file 'lib/lp/app/javascript/lp-mochi.js'
--- lib/lp/app/javascript/lp-mochi.js	2011-06-30 17:38:28 +0000
+++ lib/lp/app/javascript/lp-mochi.js	2012-01-10 17:43:30 +0000
@@ -13,59 +13,6 @@
     return node;
 }
 
-function toggleFoldable(e) {
-    var ELEMENT_NODE = 1;
-    var node = this;
-    while (node.nextSibling) {
-        node = node.nextSibling;
-        if (node.nodeType != ELEMENT_NODE) {
-            continue;
-        }
-        if (node.className.indexOf('foldable') == -1) {
-            continue;
-        }
-        if (node.style.display == 'none') {
-            node.style.display = 'inline';
-        } else {
-            node.style.display = 'none';
-        }
-    }
-}
-
-function activateFoldables() {
-    // Create links to toggle the display of foldable content.
-    var included = getElementsByTagAndClassName(
-        'span', 'foldable', document);
-    var quoted = getElementsByTagAndClassName(
-        'span', 'foldable-quoted', document);
-    var elements = concat(included, quoted);
-    for (var i = 0; i < elements.length; i++) {
-        var span = elements[i];
-        if (span.className == 'foldable-quoted') {
-            var quoted_lines = span.getElementsByTagName('br');
-            if (quoted_lines && quoted_lines.length <= 11) {
-                // We do not hide short quoted passages (12 lines) by default.
-                continue;
-            }
-        }
-
-        var ellipsis = document.createElement('a');
-        ellipsis.style.textDecoration = 'underline';
-        ellipsis.href = VOID_URL;
-        ellipsis.onclick = toggleFoldable;
-        ellipsis.appendChild(document.createTextNode('[...]'));
-
-        span.parentNode.insertBefore(ellipsis, span);
-        span.insertBefore(document.createElement('br'), span.firstChild);
-        span.style.display = 'none';
-        if (span.nextSibling) {
-            // Text lines follows this span.
-            var br = document.createElement('br');
-            span.parentNode.insertBefore(br, span.nextSibling);
-        }
-    }
-}
-
 function convertTextInputToTextArea(text_input_id, rows) {
     var current_text_input = getElement(text_input_id);
     var new_text_area = document.createElement("textarea");

=== added file 'lib/lp/app/javascript/tests/test_foldables.html'
--- lib/lp/app/javascript/tests/test_foldables.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_foldables.html	2012-01-10 17:43:30 +0000
@@ -0,0 +1,28 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
+  "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+  <head>
+  <title>Foldable Text 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="../foldables.js"></script>
+
+  <!-- The test suite -->
+  <script type="text/javascript" src="test_foldables.js"></script>
+
+</head>
+<body class="yui3-skin-sam">
+    <div id="target"></div>
+    <ul id="suites">
+        <li>lp.app.foldables.test</li>
+    </ul>
+</body>
+</html>

=== added file 'lib/lp/app/javascript/tests/test_foldables.js'
--- lib/lp/app/javascript/tests/test_foldables.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_foldables.js	2012-01-10 17:43:30 +0000
@@ -0,0 +1,131 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.app.foldables.test', function (Y) {
+
+    var test_foldables = Y.namespace('lp.app.foldables.test');
+    var suite = new Y.Test.Suite('Foldable Tests');
+
+    var quote_comment = ['<p>Mister X wrote:<br />',
+        '<span class="foldable-quoted">',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '&gt; This is a quoted line<br />',
+        '</span>',
+        'This is a reply to the line above.<br />',
+        'This is a continuation line.</p>'].join('');
+
+    var longer_comment = [
+        '<p>Attribution line<br />',
+        '<span class="foldable-quoted">',
+        '&gt; First line in the first paragraph.<br />',
+        '&gt; Second line in the first paragraph.<br />',
+        '&gt; First line in the second paragraph.<br />',
+        '&gt; Second line in the second paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '&gt; First line in the third paragraph.<br />',
+        '</span></p>'
+    ];
+
+    var foldable_comment = [
+        '<p><span class="foldable" style="display: none; "><br>',
+        '-----BEGIN PGP SIGNED MESSAGE-----<br>',
+        'Hash: SHA1',
+        '</span></p>'
+    ].join('');
+
+    suite.add(new Y.Test.Case({
+
+        name: 'foldable_tests',
+
+        _add_comment: function (comment) {
+            var cnode = Y.Node.create('<div/>');
+            cnode.set('innerHTML', comment);
+            Y.one('#target').appendChild(cnode);
+        },
+
+        tearDown: function () {
+            Y.one('#target').setContent('');
+        },
+
+        test_namespace_exists: function () {
+            Y.Assert.isObject(Y.lp.app.foldables,
+                'Foldable should be found');
+        },
+
+        test_inserts_ellipsis: function () {
+            this._add_comment(longer_comment);
+            Y.lp.app.foldables.activate();
+            Y.Assert.isObject(Y.one('a'));
+            Y.Assert.areSame('[...]', Y.one('a').getContent());
+        },
+
+        test_hides_quote: function () {
+            this._add_comment(longer_comment);
+            Y.lp.app.foldables.activate();
+            var quote = Y.one('.foldable-quoted');
+            Y.Assert.areSame(quote.getStyle('display'), 'none');
+        },
+
+        test_doesnt_hide_short: function () {
+            debugger;
+            this._add_comment(quote_comment);
+            Y.lp.app.foldables.activate();
+            Y.Assert.isNull(Y.one('a'));
+            var quote = Y.one('.foldable-quoted');
+            // this one should be visible since it's only 12 lines
+            Y.Assert.areSame(quote.getStyle('display'), 'inline');
+        },
+
+        test_clicking_link_shows: function () {
+            this._add_comment(longer_comment);
+            Y.lp.app.foldables.activate();
+
+            var quote = Y.one('.foldable-quoted');
+            // it should be hidden to start since it's 13 lines long
+            Y.Assert.areSame(quote.getStyle('display'), 'none');
+
+            var link = Y.one('a');
+            link.simulate('click');
+            var quote = Y.one('.foldable-quoted');
+            Y.Assert.areSame(quote.getStyle('display'), 'inline');
+
+            // Make sure that if clicked again it hides.
+            link.simulate('click');
+            Y.Assert.areSame(quote.getStyle('display'), 'none');
+        },
+
+        test_foldable: function () {
+            this._add_comment(foldable_comment);
+            Y.lp.app.foldables.activate();
+            var link = Y.one('a');
+            link.simulate('click');
+
+            var quote = Y.one('.foldable');
+            Y.Assert.areSame(quote.getStyle('display'), 'inline');
+
+            // Make sure that if clicked again it hides.
+            link.simulate('click');
+            Y.Assert.areSame(quote.getStyle('display'), 'none');
+        }
+    }));
+
+    test_foldables.suite = suite;
+
+}, '0.1', {
+    requires: ['test', 'node-event-simulate', 'node', 'lp.app.foldables']
+});

=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt	2012-01-06 12:34:37 +0000
+++ lib/lp/app/templates/base-layout-macros.pt	2012-01-10 17:43:30 +0000
@@ -115,13 +115,14 @@
     </script>
 
   <script id="base-layout-load-scripts" type="text/javascript">
-    LPS.use('node', 'event-delegate', 'lp', 'lp.app.links', 'lp.app.longpoll',
-        'lp.app.inlinehelp', function(Y) {
+    LPS.use('node', 'event-delegate', 'lp', 'lp.app.foldables', 'lp.app.links',
+        'lp.app.longpoll', 'lp.app.inlinehelp', function(Y) {
         Y.on('load', function(e) {
             sortables_init();
             Y.lp.app.inlinehelp.init_help();
             Y.lp.activate_collapsibles();
-            activateFoldables();
+            Y.lp.app.foldables.activate();
+
             activateConstrainBugExpiration();
             Y.lp.app.links.check_valid_lp_links();
             // Longpolling will only start if