launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04108
[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