← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/launchpad/expand-diffs into lp:launchpad

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/expand-diffs into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/launchpad/expand-diffs/+merge/66479

Expander widget.

This is part of the work to introduce expandable revision information on the branch page.  We've created a single, simple expander widget that can be used either to hide or reveal existing parts of a page, or to load details on the fly.  There was an existing "collapsible" for folding existing parts of the page, as well as several ad-hoc implementations.  The collapsible was too monolothic for easy extension to Ajax.

Test:
{{{
./bin/test -vvc -t expander
}}}

Danilo is converting existing expanders to use this widget.


No lint,

Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/expand-diffs/+merge/66479
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/expand-diffs into lp:launchpad.
=== added file 'lib/lp/app/javascript/expander.js'
--- lib/lp/app/javascript/expander.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/expander.js	2011-06-30 15:43:27 +0000
@@ -0,0 +1,233 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Expander widget.  Can be used to let the user toggle the visibility of
+ * existing elements on the page, or to make the page load elements on demand
+ * as the user expands them.
+ *
+ * Synonyms: collapsible, foldable.
+ *
+ * Each expander needs two tags as "connection points":
+ *  * Icon tag, to be marked up with the expander icon.
+ *  * Content tag, to be exposed by the expander.
+ *
+ * Either may have initial contents.  The initial contents of the icon tag
+ * stays in place, so it could say something like "Details..." that explains
+ * what the icon means.  You'll want to hide it using the "unseen" class if
+ * these contents should only be shown once the expander has been set up.
+ *
+ * Any initial contents of the content tag will be revealed when the expander
+ * is opened; hide them using the "unseen" class if they should not be shown
+ * when the expander has not been enabled.  An optional loader function may
+ * produce new contents for this tag when the user first opens the expander.
+ *
+ * If you provide a loader function, the expander runs it when the user first
+ * opens it.  The loader should produce a DOM node or node list(it may do this
+ * asynchronously) and feed that back to the expander by passing it to the
+ * expander's "receive" method.  The loader gets a reference to the expander
+ * as its first argument.
+ *
+ * The expander is set up in its collapsed state by default.  If you want it
+ * created in its expanded state instead, mark your content tag with the
+ * "expanded" class.
+ *
+ * @module lp.app.widgets.expander
+ * @requires node, event
+ */
+
+YUI.add('lp.app.widgets.expander', function(Y) {
+
+var namespace = Y.namespace('lp.app.widgets.expander');
+
+/*
+ * Create an expander.
+ *
+ * @param icon_node Node to serve as connection point for the expander icon.
+ * @param content_node Node to serve as connection point for expander content.
+ * @param config Object with additional parameters.
+ *     loader: A function that will produce a Node or NodeList to replace the
+ *         contents of the content tag.  Receives the Expander object
+ *         "expander" as its argument.  Once the loader has constructed the
+ *         output Node or NodeList it wants to display ("output"), it calls
+ *         expander.receive(output) to update the content node.
+ */
+function Expander (icon_node, content_node, config) {
+    if (!Y.Lang.isObject(icon_node)) {
+        throw new Error("No icon node given.");
+    }
+    if (!Y.Lang.isObject(content_node)) {
+        throw new Error("No content node given.");
+    }
+    this.icon_node = icon_node;
+    this.content_node = content_node;
+    if (Y.Lang.isValue(config)) {
+        this.config = config;
+    } else {
+        this.config = {};
+    }
+    this.loaded = !Y.Lang.isValue(this.config.loader);
+
+    // Is setup complete?  Skip any animations until it is.
+    this.fully_set_up = false;
+}
+namespace.Expander = Expander;
+
+namespace.Expander.prototype = {
+    /*
+     * CSS classes.
+     */
+    css_classes: {
+        expanded: 'expanded',
+        unseen: 'unseen'
+    },
+
+    /*
+     * Return sprite name for given expander state.
+     */
+    nameSprite: function (expanded) {
+        if (expanded) {
+            return 'treeExpanded';
+        } else {
+            return 'treeCollapsed';
+        }
+    },
+
+    /*
+     * Is the content node currently expanded?
+     */
+    isExpanded: function () {
+        return this.content_node.hasClass(this.css_classes.expanded);
+    },
+
+    /*
+     * Either add or remove given CSS class from the content tag.
+     *
+     * @param want_class Whether this class is desired for the content tag.
+     *     If it is, then the function may need to add it; if it isn't, then
+     *     the function may need to remove it.
+     * @param class_name CSS class name.
+     */
+    setContentClassIf: function (want_class, class_name) {
+        if (want_class) {
+            this.content_node.addClass(class_name);
+        } else {
+            this.content_node.removeClass(class_name);
+        }
+    },
+
+    /*
+     * Record the expanded/collapsed state of the content tag.
+     */
+    setExpanded: function (is_expanded) {
+        this.setContentClassIf(is_expanded, this.css_classes.expanded);
+    },
+
+    /*
+     * Hide or reveal the content node (by adding the "unseen" class to it).
+     *
+     * @param expand Are we expanding?  If not, we must be collapsing.
+     */
+    foldContentNode: function (expand) {
+        this.setContentClassIf(!expand, this.css_classes.unseen);
+    },
+
+    revealIcon: function () {
+        this.icon_node
+            .addClass('sprite').addClass('js-action')
+            .removeClass('unseen');
+    },
+
+    /*
+     * Set icon to either the "expanded" or the "collapsed" state.
+     *
+     * @param expand Are we expanding?  If not, we must be collapsing.
+     */
+    setIcon: function (expand) {
+        this.icon_node
+            .removeClass(this.nameSprite(!expand))
+            .addClass(this.nameSprite(expand));
+    },
+
+    /*
+     * Process the output node being produced by the loader.  To be invoked
+     * by a custom loader when it's done.
+     *
+     * @param output A Node or NodeList to replace the contents of the content
+     *     tag with.
+     */
+    receive: function (output) {
+        // We'll animate this later (if this.fully_set_up is false).
+        this.content_node.setContent(output);
+    },
+
+    /*
+     * Invoke the loader, and record the fact that the loader has been
+     * started.
+     */
+    load: function () {
+        this.loaded = true;
+        this.config.loader(this);
+    },
+
+    /*
+     * Set the expander's DOM elements to a consistent, operational state.
+     *
+     * @param expanded Whether the expander is to be rendered in its expanded
+     *     state.  If not, it must be in the collapsed state.
+     */
+    render: function (expanded) {
+        this.foldContentNode(expanded);
+        this.setIcon(expanded);
+        if (expanded && !this.loaded) {
+            this.load();
+        }
+        this.setExpanded(expanded);
+    },
+
+    /*
+     * Set up an expander's DOM and event handler.
+     *
+     */
+    setUp: function () {
+        var expander = this;
+        function click_handler (e) {
+            e.preventDefault();
+            expander.render(!expander.isExpanded());
+        }
+
+        this.render(this.isExpanded());
+        this.icon_node.on('click', click_handler);
+        this.revealIcon();
+        this.fully_set_up = true;
+        return this;
+    }
+};
+
+/*
+ * Initialize expanders based on CSS selectors.
+ *
+ * @param widget_select CSS selector to specify each tag that will have an
+ *     expander created inside it.
+ * @param icon_select CSS selector for the icon tag inside each tag matched
+ *     by widget_select.
+ * @param content_select CSS selector for the content tag inside each tag
+ *     matched by widget_select.
+ * @param loader Optional loader function for each expander that is set up.
+ *     Must take an Expander as its argument, create a Node or NodeList with
+ *     the output to be displayed, and feed the output to the expander's
+ *     "receive" method.
+ */
+function createByCSS(widget_select, icon_select, content_select, loader) {
+    var config = {
+        loader: loader
+    };
+    var expander_factory = function (widget) {
+        var expander = new Expander(
+            widget.one(icon_select), widget.one(content_select), config);
+        expander.setUp();
+    };
+    Y.all(widget_select).each(expander_factory);
+}
+namespace.createByCSS = createByCSS;
+
+}, "0.1", {"requires": ["node"]});

=== added file 'lib/lp/app/javascript/tests/test_expander.html'
--- lib/lp/app/javascript/tests/test_expander.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_expander.html	2011-06-30 15:43:27 +0000
@@ -0,0 +1,32 @@
+<html>
+  <head>
+    <title>Expander widget</title>
+    <!-- YUI 3.0 Setup -->
+    <script type="text/javascript" src="../../../../canonical/launchpad/icing/yui/yui/yui.js"></script>
+    <script type="text/javascript"
+      src="../../../../canonical/launchpad/icing/lazr/build/lazr.js"></script>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssreset/reset.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssfonts/fonts.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/icing/yui/cssbase/base.css"/>
+    <link rel="stylesheet"
+      href="../../../../canonical/launchpad/javascript/test.css" />
+    <script type="text/javascript" src="../../../app/javascript/client.js"></script>
+    <script type="text/javascript" src="../../../app/javascript/lp.js"></script>
+
+    <!-- The module under test -->
+    <script type="text/javascript" src="../expander.js"></script>
+
+    <!-- The test suite -->
+    <script type="text/javascript" src="test_expander.js"></script>
+  </head>
+  <body class="yui3-skin-sam">
+
+    <!-- The example markup required by the script to run -->
+    <div class="test-hook">
+    </div>
+
+  </body>
+</html>

=== added file 'lib/lp/app/javascript/tests/test_expander.js'
--- lib/lp/app/javascript/tests/test_expander.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_expander.js	2011-06-30 15:43:27 +0000
@@ -0,0 +1,199 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ */
+
+YUI({
+    base: '../../../../canonical/launchpad/icing/yui/',
+    filter: 'raw', combine: false,
+    fetchCSS: false
+    }).use('test', 'console', 'node', 'node-event-simulate',
+           'lp.app.widgets.expander', function(Y) {
+
+    var suite = new Y.Test.Suite("lp.app.widgets.expander Tests");
+    var module = Y.lp.app.widgets.expander;
+
+    suite.add(new Y.Test.Case({
+        name: 'expandable',
+
+        setUp: function () {
+            this.findTestHookTag().setContent('');
+        },
+
+        findTestHookTag: function () {
+            return Y.one('.test-hook');
+        },
+
+        makeNode: function (css_class) {
+            var node = Y.Node.create('<div></div>');
+            if (css_class !== undefined) {
+                node.addClass(css_class);
+            }
+            return node;
+        },
+
+        makeExpanderHooks: function (args) {
+            if (!Y.Lang.isValue(args)) {
+                args = {};
+            }
+            var root = this.makeNode();
+            var hook = root.appendChild(this.makeNode('hook'));
+            var icon = hook.appendChild(this.makeNode('icon'));
+            var content = hook.appendChild(this.makeNode('content'));
+            if (args.expanded) {
+                content.addClass('expanded');
+            }
+            return root;
+        },
+
+        makeExpander: function (root, args) {
+            if (!Y.Lang.isValue(args)) {
+                args = {};
+            }
+            if (root === undefined) {
+                root = this.makeExpanderHooks();
+            }
+            return new module.Expander(
+                root.one('.icon'), root.one('.content'), args.config).setUp();
+        },
+
+        test_loaded_is_true_if_no_loader_is_defined: function () {
+            var icon = Y.Node.create('<p></p>'),
+                content = Y.Node.create('<p></p>');
+            var expander = new module.Expander(icon, content);
+            Y.Assert.isTrue(expander.loaded);
+        },
+
+        test_loaded_is_false_if_loader_is_defined: function () {
+            var icon = Y.Node.create('<p></p>'),
+                content = Y.Node.create('<p></p>');
+            var config = {loader: function () {}};
+            var expander = new module.Expander(icon, content, config);
+            Y.Assert.isFalse(expander.loaded);
+        },
+
+        test_setUp_preserves_icon_content: function () {
+            var root = this.makeExpanderHooks();
+            root.one('.icon').set('text', "Click here");
+            var icon = this.makeExpander(root).icon_node;
+            Y.Assert.areEqual("Click here", icon.get('text'));
+        },
+
+        test_setUp_creates_collapsed_icon_by_default: function () {
+            var icon = this.makeExpander().icon_node;
+            Y.Assert.isTrue(icon.hasClass('sprite'));
+            Y.Assert.isFalse(icon.hasClass('treeExpanded'));
+            Y.Assert.isTrue(icon.hasClass('treeCollapsed'));
+        },
+
+        test_setUp_reveals_icon: function () {
+            var root = this.makeExpanderHooks();
+            var icon = root.one('.icon');
+            icon.addClass('unseen');
+            var expander = this.makeExpander(root);
+            Y.Assert.isFalse(icon.hasClass('unseen'));
+            Y.Assert.isTrue(icon.hasClass('js-action'));
+        },
+
+        test_setUp_hides_content_by_default: function () {
+            var content = this.makeExpander().content_node;
+            Y.Assert.isTrue(content.hasClass('unseen'));
+        },
+
+        test_setUp_creates_expanded_icon_if_content_is_expanded: function () {
+            var root = this.makeExpanderHooks({expanded: true});
+            var icon = this.makeExpander(root).icon_node;
+            Y.Assert.isTrue(icon.hasClass('treeExpanded'));
+            Y.Assert.isFalse(icon.hasClass('treeCollapsed'));
+        },
+
+        test_setUp_reveals_content_if_content_is_expanded: function () {
+            var root = this.makeExpanderHooks({expanded: true});
+            var content = this.makeExpander(root).content_node;
+            Y.Assert.isFalse(content.hasClass('unseen'));
+        },
+
+        test_setUp_does_not_run_loader_by_default: function () {
+            var loader_has_run = false;
+            var loader = function () {
+                loader_has_run = true;
+            };
+            this.makeExpander(
+                this.makeExpanderHooks(), {config: {loader: loader}});
+            Y.Assert.isFalse(loader_has_run);
+        },
+
+        test_setUp_runs_loader_if_content_is_expanded: function () {
+            var loader_has_run = false;
+            var loader = function () {
+                loader_has_run = true;
+            };
+            this.makeExpander(
+                this.makeExpanderHooks({expanded: true}),
+                {config: {loader: loader}});
+            Y.Assert.isTrue(loader_has_run);
+        },
+
+        test_setUp_installs_click_handler: function () {
+            var expander = this.makeExpander();
+            var render_has_run = false;
+            var fake_render = function () {
+                render_has_run = true;
+            };
+            expander.render = fake_render;
+            expander.icon_node.simulate('click');
+            Y.Assert.isTrue(render_has_run);
+        },
+
+        test_createByCSS_creates_expander: function () {
+            var root = this.makeExpanderHooks();
+            this.findTestHookTag().appendChild(root);
+            module.createByCSS('.hook', '.icon', '.content');
+            Y.Assert.isTrue(root.one('.content').hasClass('unseen'));
+        },
+
+        test_toggle_retains_content: function () {
+            var root = this.makeExpanderHooks();
+            root.one('.content').set('text', "Contents here");
+            var expander = this.makeExpander(root);
+            root.one('.icon').simulate('click');
+            root.one('.icon').simulate('click');
+            Y.Assert.areEqual(
+                "Contents here", expander.content_node.get('text'));
+        },
+
+        test_loader_runs_only_once: function () {
+            var loader_runs = 0;
+            var loader = function () {
+                loader_runs++;
+            };
+            var expander = this.makeExpander(
+                this.makeExpanderHooks(), {config: {loader: loader}});
+            expander.icon_node.simulate('click');
+            expander.icon_node.simulate('click');
+            expander.icon_node.simulate('click');
+            Y.Assert.areEqual(1, loader_runs);
+        },
+
+        test_receive_replaces_contents: function () {
+            var expander = this.makeExpander();
+            var ajax_result = this.makeNode("ajax-result");
+            expander.receive(ajax_result);
+            Y.Assert.isTrue(expander.content_node.hasChildNodes());
+            var children = expander.content_node.get('children');
+            Y.Assert.areEqual(1, children.size());
+            Y.Assert.areEqual(ajax_result, children.item(0));
+        }
+    }));
+
+    var handle_complete = function(data) {
+        window.status = '::::' + JSON.stringify(data);
+    };
+    Y.Test.Runner.on('complete', handle_complete);
+    Y.Test.Runner.add(suite);
+
+    var console = new Y.Console({newestOnTop: false});
+    console.render('#log');
+
+    Y.on('domready', function() {Y.Test.Runner.run();});
+});
+

=== modified file 'lib/lp/code/templates/branch-index.pt'
--- lib/lp/code/templates/branch-index.pt	2011-04-11 01:30:37 +0000
+++ lib/lp/code/templates/branch-index.pt	2011-06-30 15:43:27 +0000
@@ -31,9 +31,11 @@
   <script type="text/javascript"
           tal:content="string:
     LPS.use('node', 'event', 'widget', 'plugin', 'overlay',
-              'lazr.choiceedit', 'lp.code.branch.status',
+              'lazr.choiceedit',
+              'lp.code.branch.status',
               'lp.code.branchmergeproposal.diff',
-              'lp.code.branch.subscription', function(Y) {
+              'lp.code.branch.subscription',
+              function(Y) {
 
         Y.on('load', function(e) {
             var logged_in = LP.links['me'] !== undefined;
@@ -74,7 +76,7 @@
     <tal:registrant replace="structure context/registrant/fmt:link" />
   on
     <tal:created-on replace="structure context/date_created/fmt:date" />
-  and last modified on 
+  and last modified on
     <tal:last-modified replace="structure context/date_last_modified/fmt:date" />
 </tal:registering>
 


Follow ups