launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06069
[Merge] lp:~rharding/launchpad/replace_foldables into lp:launchpad
Richard Harding has proposed merging lp:~rharding/launchpad/replace_foldables into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rharding/launchpad/replace_foldables/+merge/87924
= 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
--
https://code.launchpad.net/~rharding/launchpad/replace_foldables/+merge/87924
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rharding/launchpad/replace_foldables into lp:launchpad.
=== added file 'lib/lp/app/javascript/foldable.js'
--- lib/lp/app/javascript/foldable.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/foldable.js 2012-01-10 10:48:09 +0000
@@ -0,0 +1,77 @@
+/**
+ * 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.foldable.init();
+ *
+ * @module lp.app.foldable
+ */
+YUI.add('lp.app.foldable', function(Y) {
+
+ var module = Y.namespace('lp.app.foldable');
+ 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.init = 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');
+ if (quoted_lines && quoted_lines.size() <= 12) {
+ // 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 10:48:09 +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_foldable.html'
--- lib/lp/app/javascript/tests/test_foldable.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_foldable.html 2012-01-10 10:48:09 +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="../foldable.js"></script>
+
+ <!-- The test suite -->
+ <script type="text/javascript" src="test_foldable.js"></script>
+
+</head>
+<body class="yui3-skin-sam">
+ <div id="target"></div>
+ <ul id="suites">
+ <li>lp.foldable.test</li>
+ </ul>
+</body>
+</html>
=== added file 'lib/lp/app/javascript/tests/test_foldable.js'
--- lib/lp/app/javascript/tests/test_foldable.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_foldable.js 2012-01-10 10:48:09 +0000
@@ -0,0 +1,122 @@
+/* Copyright (c) 2011, Canonical Ltd. All rights reserved. */
+
+YUI.add('lp.foldable.test', function (Y) {
+
+ var test_foldable = Y.namespace('lp.foldable.test');
+ var suite = new Y.Test.Suite('Foldable Tests');
+
+ var quote_comment = ['<p>Mister X wrote:<br />',
+ '<span class="foldable-quoted">',
+ '> This is a quoted line<br />',
+ '</span>',
+ 'This is a reply to the line above.<br />',
+ 'This is a continuation line.</p>'].join('');
+
+ var multiple_quoted_comment = [
+ '<p>Attribution line<br />',
+ '<span class="foldable-quoted">',
+ '> First line in the first paragraph.<br />',
+ '> Second line in the first paragraph.<br />',
+ '><br />',
+ '> First line in the second paragraph.<br />',
+ '> Second line in the second paragraph.<br />',
+ '><br />',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '> First line in the third paragraph.<br />',
+ '> Second line in the third paragraph.',
+ '</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.foldable,
+ 'Foldable should be found');
+ },
+
+ test_inserts_ellipsis: function () {
+ this._add_comment(multiple_quoted_comment);
+ Y.lp.app.foldable.init();
+ Y.Assert.isObject(Y.one('a'));
+ Y.Assert.areSame('[...]', Y.one('a').getContent());
+ },
+
+ test_hides_quote: function () {
+ this._add_comment(multiple_quoted_comment);
+ Y.lp.app.foldable.init();
+ var quote = Y.one('.foldable-quoted');
+ Y.Assert.areSame(quote.getStyle('display'), 'none');
+ },
+
+ test_doesnt_hide_short: function () {
+ this._add_comment(quote_comment);
+ Y.lp.app.foldable.init();
+ Y.Assert.isNull(Y.one('a'));
+ var quote = Y.one('.foldable-quoted');
+ Y.Assert.areSame(quote.getStyle('display'), 'inline');
+ },
+
+ test_clicking_link_shows: function () {
+ this._add_comment(multiple_quoted_comment);
+ Y.lp.app.foldable.init();
+
+ 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.foldable.init();
+ 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_foldable.suite = suite;
+
+}, '0.1', {
+ requires: ['test', 'node-event-simulate', 'node', 'lp.app.foldable']
+});
=== 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 10:48:09 +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.foldable', '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.foldable.init();
+
activateConstrainBugExpiration();
Y.lp.app.links.check_valid_lp_links();
// Longpolling will only start if
