← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~rvb/launchpad/widgetify-bug-796669 into lp:launchpad

 

Raphaël Victor Badin has proposed merging lp:~rvb/launchpad/widgetify-bug-796669 into lp:launchpad with lp:~rvb/launchpad/bug-798522 as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #796669 in Launchpad itself: "The lp.registry.distroseriesdifferences_details module (javascript) is very hard to test."
  https://bugs.launchpad.net/launchpad/+bug/796669

For more details, see:
https://code.launchpad.net/~rvb/launchpad/widgetify-bug-796669/+merge/70308

This branch is the first step toward cleaning up the design of lib/lp/registry/javascript/distroseriesdifferences_details.js to make it more easily testable. This branch refactors all the code related to the expandable row into an YUI3 Object and adds tests for it. All the rest of the methods are moved inside the module itself (as opposed to the setup_expandable_rows object).

= Tests =

lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html

= Q/A =

This should not add any new functionality so all the previous actions should be available on this page: https://dogfood.launchpad.net/ubuntu/oneiric/+localpackagediffs
- opening up the details row for a DSD
- blacklisting DSD
- adding comments
- requesting packagediffs computations
-- 
https://code.launchpad.net/~rvb/launchpad/widgetify-bug-796669/+merge/70308
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/widgetify-bug-796669 into lp:launchpad.
=== modified file 'lib/lp/registry/javascript/distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-08-09 11:56:43 +0000
+++ lib/lp/registry/javascript/distroseriesdifferences_details.js	2011-08-09 11:56:45 +0000
@@ -14,7 +14,9 @@
 /**
  * Create one Launchpad client that will be used with multiple requests.
  */
-var lp_client = new Y.lp.client.Launchpad();
+namespace.lp_client = new Y.lp.client.Launchpad();
+
+namespace.io = Y.io;
 
 /*
  * XXX: rvb 2011-08-01 bug=796669: At present this module it is
@@ -23,333 +25,102 @@
  * it can be unit tested without having to set-up the world each time.
  */
 
-/*
- * Setup the expandable rows for each difference.
- *
- * @method setup_expandable_rows
- */
-namespace.setup_expandable_rows = function() {
-
-    var blacklist_handler = function(e, dsd_link, source_name,
-                                     latest_comment_container,
-                                     add_comment_placeholder) {
-        // We only want to select the new radio if the update is
-        // successful.
+function ExpandableRowWidget(config) {
+    ExpandableRowWidget.superclass.constructor.apply(this, arguments);
+}
+
+ExpandableRowWidget.NAME = "expandableRowWidget";
+
+Y.extend(ExpandableRowWidget, Y.Base, {
+    initializer: function(cfg) {
+        this._toggle = cfg.toggle;
+        this._row = this._toggle.ancestor('tr');
+        this._toggle.addClass('treeCollapsed').addClass('sprite');
+        this._toggle.on("click", this.expander_handler, this);
+    },
+
+    parse_row_data: function() {
+        var source_name = this._row.one('a.toggle-extra').get('text');
+        var rev_link = this._row
+            .one('a.toggle-extra').get('href').split('/').reverse();
+        var parent_series_name = rev_link[0];
+        var parent_distro_name = rev_link[1];
+        var nb_columns = this._row.all('td').size();
+        var res = {
+            source_name: source_name,
+            parent_series_name: parent_series_name,
+            parent_distro_name: parent_distro_name,
+            nb_columns: nb_columns};
+        return res;
+    },
+
+    expander_handler: function(e) {
         e.preventDefault();
-        var blacklist_options_container = this.ancestor('div');
-        blacklist_comment_overlay(
-            e, dsd_link, source_name, latest_comment_container,
-            add_comment_placeholder, blacklist_options_container);
-    };
-
-    var blacklist_comment_overlay = function(e, dsd_link, source_name,
-                                             latest_comment_container,
-                                             add_comment_placeholder,
-                                             blacklist_options_container) {
-        var comment_form = Y.Node.create("<form />")
-            .appendChild(Y.Node.create("<textarea></textarea>")
-                .set("name", "comment")
-                .set("rows", "3")
-                .set("cols", "60"));
-
-        /* Buttons */
-        var submit_button = Y.Node.create(
-            '<button type="submit" class="lazr-pos lazr-btn" />')
-               .set("text", "OK");
-        var cancel_button = Y.Node.create(
-            '<button type="button" class="lazr-neg lazr-btn" />')
-               .set("text", "Cancel");
-
-        var submit_callback = function(data) {
-            overlay.hide();
-            var comment = "";
-            if (data.comment !== undefined) {
-                comment = data.comment[0];
-            }
-            blacklist_submit_handler(
-                e, dsd_link, source_name, comment, latest_comment_container,
-                add_comment_placeholder, blacklist_options_container);
-
-        };
-        var origin = blacklist_options_container.one('.blacklist-options');
-        var overlay = new Y.lazr.FormOverlay({
-            align: {
-                 /* Align the centre of the overlay with the centre of the
-                    node containing the blacklist options. */
-                 node: origin,
-                 points: [
-                     Y.WidgetPositionAlign.CC,
-                     Y.WidgetPositionAlign.CC
-                 ]
-             },
-             headerContent: "<h2>Add an optional comment</h2>",
-             form_content: comment_form,
-             form_submit_button: submit_button,
-             form_cancel_button: cancel_button,
-             form_submit_callback: submit_callback,
-             visible: true
-        });
-        overlay.render();
-    };
-
-    var blacklist_submit_handler = function(e, dsd_link, source_name,
-                                            comment, latest_comment_container,
-                                            add_comment_placeholder,
-                                            blacklist_options_container) {
-        // Disable all the inputs.
-        blacklist_options_container.all('input').set('disabled', 'disabled');
-        e.target.prepend('<img src="/@@/spinner" />');
-
-        var method_name = (e.target.get('value') === 'NONE') ?
-            'unblacklist' : 'blacklist';
-        var blacklist_all = (e.target.get('value') === 'BLACKLISTED_ALWAYS');
-
-        var diff_rows = Y.all('tr.' + source_name);
-
-        var config = {
-            on: {
-                success: function(updated_entry, args) {
-                    // Let the user know this item is now blacklisted.
-                    blacklist_options_container.one('img').remove();
-                    blacklist_options_container.all(
-                        'input').set('disabled', false);
-                    e.target.set('checked', true);
-                    Y.each(diff_rows, function(diff_row) {
-                        var fade_to_gray = new Y.Anim({
-                            node: diff_row,
-                            from: { backgroundColor: '#FFFFFF'},
-                            to: { backgroundColor: '#EEEEEE'}
-                            });
-                        if (method_name === 'unblacklist') {
-                            fade_to_gray.set('reverse', true);
-                            }
-                        fade_to_gray.run();
-                        });
-                    add_comment(
-                        updated_entry, add_comment_placeholder,
-                        latest_comment_container);
-                 },
-                failure: function(id, response) {
-                    blacklist_options_container.one('img').remove();
-                    blacklist_options_container.all(
-                        'input').set('disabled', false);
-                }
-            },
-            parameters: {
-                all: blacklist_all,
-                comment: comment
-            }
-        };
-
-        lp_client.named_post(dsd_link, method_name, config);
-
-    };
-
-    /**
-     * Link the click event for these blacklist options to the correct
-     * api uri.
-     *
-     * @param blacklist_options {Node} The node containing the blacklist
-     *     options.
-     * @param source_name {string} The name of the source to update.
-     * @param dsd_link {string} The uri for the distroseriesdifference object.
-     * @param latest_comment_container {Node} The node containing the last
-     *     comment.
-     * @param add_comment_placeholder {Node} The node containing the "add
-     *     comment" slot.
-     */
-    var setup_blacklist_options = function(
-        blacklist_options, source_name, dsd_link, latest_comment_container,
-        add_comment_placeholder) {
-        Y.on('click', blacklist_handler, blacklist_options.all('input'),
-             blacklist_options, dsd_link, source_name,
-             latest_comment_container, add_comment_placeholder);
-    };
-
-    /**
-     * Update the latest comment on the difference row.
-     *
-     * @param comment_entry {lp.client.Entry} An object representing
-     *     a DistroSeriesDifferenceComment.
-     * @param placeholder {Node}
-     *     The node in which to put the latest comment HTML fragment. The
-     *     contents of this node will be replaced.
-     */
-    var update_latest_comment = function(comment_entry, placeholder) {
-        var comment_latest_fragment_url =
-            comment_entry.get("web_link") + "/+latest-comment-fragment";
-        var config = {
-            on: {
-                success: function(comment_latest_fragment_html) {
-                    placeholder.set(
-                        "innerHTML", comment_latest_fragment_html);
-                    Y.lazr.anim.green_flash({node: placeholder}).run();
-                },
-                failure: function(id, response) {
-                    Y.lazr.anim.red_flash({node: placeholder}).run();
-                }
-            },
-            accept: Y.lp.client.XHTML
-        };
-        lp_client.get(comment_latest_fragment_url, config);
-    };
-
-    /**
-     * This method adds a comment in the UI. It appends a comment to the
-     * list of comments and updates the latest comments slot.
-     *
-     * @param comment_entry {Comment} A comment as returns by the api.
-     * @param add_comment_placeholder {Node} The node that contains the
-     *     relevant comment fields.
-     * @param latest_comment_placeholder {Node} The node that contains the
-     *     latest comment.
-     */
-    var add_comment = function(comment_entry, add_comment_placeholder,
-                               latest_comment_placeholder) {
-        // Grab the XHTML representation of the comment
-        // and prepend it to the list of comments.
-        var config = {
-            on: {
-                success: function(comment_html) {
-                    var comment_node = Y.Node.create(comment_html);
-                    add_comment_placeholder.insert(comment_node, 'before');
-                    var reveal = Y.lazr.effects.slide_out(comment_node);
-                    reveal.on("end", function() {
-                        Y.lazr.anim.green_flash(
-                            {node: comment_node}).run();
-                    });
-                    reveal.run();
-                }
-            },
-            accept: Y.lp.client.XHTML
-        };
-        lp_client.get(comment_entry.get('self_link'), config);
-        update_latest_comment(comment_entry, latest_comment_placeholder);
-    };
-
-    /**
-     * Handle the add comment event triggered by the 'add comment' form.
-     *
-     * This method adds a comment via the API and update the UI.
-     *
-     * @param comment_form {Node} The node that contains the relevant comment
-     *     fields.
-     * @param latest_comment_placeholder {Node} The node that contains the
-     *     latest comment.
-     * @param api_uri {string} The uri for the distroseriesdifference to which
-     *     the comment is to be added.
-     * @param cb_success {function} Called when a comment has successfully
-     *     been added. (Deferreds would be awesome right about now.)
-     */
-    var add_comment_handler = function(
-            comment_form, latest_comment_placeholder, api_uri, cb_success) {
-
-        var comment_area = comment_form.one('textarea');
-        var comment_text = comment_area.get('value');
-
-        // Treat empty comments as mistakes.
-        if (Y.Lang.trim(comment_text).length === 0) {
-            Y.lazr.anim.red_flash({node: comment_area}).run();
-            return;
-        }
-
-        var success_handler = function(comment_entry) {
-            add_comment(
-                comment_entry, comment_form, latest_comment_placeholder);
-            comment_form.one('textarea').set('value', '');
-            cb_success();
-        };
-        var failure_handler = function(id, response) {
-            // Re-enable field with red flash.
-            Y.lazr.anim.red_flash({node: comment_form}).run();
-        };
-
-        var config = {
-            on: {
-                success: success_handler,
-                failure: failure_handler,
-                start: function() {
-                    // Show a spinner.
-                    comment_form.one('div.widget-bd')
-                        .append('<img src="/@@/spinner" />');
-                    // Disable the textarea and button.
-                    comment_form.all('textarea,button')
-                        .setAttribute('disabled', 'disabled');
-                },
-                end: function() {
-                    // Remove the spinner.
-                    comment_form.all('img').remove();
-                    // Enable the form.
-                    comment_form.all('textarea,button')
-                        .removeAttribute('disabled');
-                }
-            },
-            parameters: {
-                comment: comment_text
-            }
-        };
-        lp_client.named_post(api_uri, 'addComment', config);
-    };
-
-    /**
-     * Add the comment fields ready for sliding out.
-     *
-     * This method adds the markup for a slide-out comment and sets
-     * the event handlers.
-     *
-     * @param placeholder {Node} The node that is to contain the comment
-     *     fields.
-     * @param api_uri {string} The uri for the distroseriesdifference to which
-     *     the comment is to be added.
-     */
-    var setup_add_comment = function(
-            placeholder, latest_comment_placeholder, api_uri) {
-        placeholder.insert([
-            '<a class="widget-hd js-action sprite add" href="">',
-            '  Add comment</a>',
-            '<div class="widget-bd lazr-closed" ',
-            '     style="height:0;overflow:hidden">',
-            '  <textarea></textarea><button>Save comment</button>',
-            '</div>'
-            ].join(''), 'replace');
-
-        // The comment area should slide in when the 'Add comment'
-        // action is clicked.
-        var slide_anim = null;
-        var slide = function(direction) {
-            // Slide out if direction is true, slide in if direction
-            // is false, otherwise do the opposite of what's being
-            // animated right now.
-            if (slide_anim === null) {
-                slide_anim = Y.lazr.effects.slide_out(
-                    placeholder.one('div.widget-bd'));
-                if (Y.Lang.isBoolean(direction)) {
-                    slide_anim.set("reverse", !direction);
-                }
-            }
-            else {
-                if (Y.Lang.isBoolean(direction)) {
-                    slide_anim.set("reverse", !direction);
-                }
-                else {
-                    slide_anim.set('reverse', !slide_anim.get('reverse'));
-                }
-                slide_anim.stop();
-            }
-            slide_anim.run();
-        };
-        var slide_in = function() { slide(false); };
-
-        placeholder.one('a.widget-hd').on(
-            'click', function(e) { e.preventDefault(); slide(); });
-
-        placeholder.one('button').on('click', function(e) {
-            e.preventDefault();
-            add_comment_handler(
-                placeholder, latest_comment_placeholder,
-                api_uri, slide_in);
-        });
-    };
+        var parsed = this.parse_row_data();
+        this._toggle.toggleClass('treeCollapsed').toggleClass('treeExpanded');
+
+        // Only insert if there isn't already a container row there.
+        var detail_row = this._row.next();
+        if (detail_row === null ||
+            !detail_row.hasClass('diff-extra')) {
+            details_row = Y.Node.create([
+                '<table><tr class="diff-extra unseen ',
+                parsed.source_name + '">',
+                '  <td colspan="'+parsed.nb_columns+'">',
+                '    <div class="diff-extra-container"></div>',
+                '  </td></tr></table>'
+                ].join('')).one('tr');
+            this._row.insert(details_row, 'after');
+            var uri = this._toggle.get('href');
+            this.get_extra_diff_info(
+                uri, this._row, details_row.one('td'), parsed.source_name,
+                parsed.parent_distro_name, parsed.parent_series_name);
+        }
+        details_row.toggleClass('unseen');
+    },
+
+    setup_extra_diff_info: function(master_container, container,
+                                    source_name, parent_distro_name,
+                                    parent_series_name, response) {
+        container.one('div.diff-extra-container').insert(
+            response.responseText, 'replace');
+        var api_uri = [
+            LP.cache.context.self_link,
+            '+source',  source_name, '+difference',
+            parent_distro_name, parent_series_name
+           ].join('/');
+        var latest_comment_container =
+            master_container.one('td.latest-comment-fragment');
+        // The add comment slot is only available when the user has the
+        // right to add comments.
+        var add_comment_placeholder =
+            container.one('div.add-comment-placeholder');
+        if (add_comment_placeholder !== null) {
+            namespace.setup_add_comment(
+                add_comment_placeholder,
+                latest_comment_container,
+                api_uri);
+        }
+        // The blacklist slot with a class 'blacklist-options' is only
+        // available when the user has the right to blacklist.
+        var blacklist_slot = container.one('div.blacklist-options');
+        if (blacklist_slot !== null) {
+            namespace.setup_blacklist_options(
+                blacklist_slot, source_name, api_uri,
+                latest_comment_container,
+                add_comment_placeholder);
+        }
+        // If the user has not the right to blacklist, we disable
+        // the blacklist slot.
+        var disabled_blacklist_slot = container.one(
+            'div.blacklist-options-disabled');
+        if (disabled_blacklist_slot !== null) {
+            disabled_blacklist_slot
+                .all('input').set('disabled', 'disabled');
+        }
+        // Set-up diffs and the means to request them.
+        namespace.setup_packages_diff_states(container, api_uri);
+    },
 
     /**
      * Get the extra information for this diff to display.
@@ -369,61 +140,23 @@
      *     The name of the distroseries in which a different version of
      *     the source package exists.
      */
-    var get_extra_diff_info = function(uri, master_container, container,
-                                       source_name, parent_distro_name,
-                                       parent_series_name) {
+    get_extra_diff_info: function(uri, master_container, container,
+                                  source_name, parent_distro_name,
+                                  parent_series_name) {
         var in_progress_message = Y.lp.soyuz.base.makeInProgressNode(
             'Fetching difference details ...');
         container.one('div.diff-extra-container').insert(
             in_progress_message, 'replace');
         var success_cb = function(transaction_id, response, args) {
-            args.container.one('div.diff-extra-container').insert(
-                response.responseText, 'replace');
-            var api_uri = [
-                LP.cache.context.self_link,
-                '+source',
-                source_name,
-                '+difference',
-                parent_distro_name,
-                parent_series_name
-                ].join('/');
-            var latest_comment_container =
-                args.master_container.one('td.latest-comment-fragment');
-            // The add comment slot is only available when the user has the
-            // right to add comments.
-            var add_comment_placeholder =
-                args.container.one('div.add-comment-placeholder');
-            if (add_comment_placeholder !== null) {
-                setup_add_comment(
-                    add_comment_placeholder,
-                    latest_comment_container,
-                    api_uri);
-            }
-            // The blacklist slot with a class 'blacklist-options' is only
-            // available when the user has the right to blacklist.
-            var blacklist_slot = args.container.one('div.blacklist-options');
-            if (blacklist_slot !== null) {
-                setup_blacklist_options(
-                    blacklist_slot, source_name, api_uri,
-                    latest_comment_container,
-                    add_comment_placeholder);
-            }
-            // If the user has not the right to blacklist, we disable
-            // the blacklist slot.
-            var disabled_blacklist_slot = args.container.one(
-                'div.blacklist-options-disabled');
-            if (disabled_blacklist_slot !== null) {
-                disabled_blacklist_slot
-                    .all('input').set('disabled', 'disabled');
-            }
-            // Set-up diffs and the means to request them.
-            namespace.setup_packages_diff_states(args.container, api_uri);
+            this.setup_extra_diff_info(
+                master_container, container, source_name, parent_distro_name,
+                parent_series_name, response);
         };
 
         var failure_cb = function(transaction_id, response, args) {
             var retry_handler = function(e) {
                 e.preventDefault();
-                get_extra_diff_info(
+                this.get_extra_diff_info(
                     args.uri, args.master_container, args.container,
                     args.source_name, args.parent_distro_name,
                     args.parent_series_name);
@@ -440,6 +173,7 @@
 
         var config = {
             headers: {'Accept': 'application/json;'},
+            context: this,
             on: {
                 'success': success_cb,
                 'failure': failure_cb
@@ -451,50 +185,341 @@
                 'source_name': source_name
             }
         };
-        Y.io(uri, config);
-
-    };
-
-    var expander_handler = function(e) {
+        namespace.io(uri, config);
+
+    }
+});
+
+namespace.ExpandableRowWidget = ExpandableRowWidget;
+
+
+namespace.blacklist_handler = function(e, dsd_link, source_name,
+                                       latest_comment_container,
+                                       add_comment_placeholder) {
+    // We only want to select the new radio if the update is
+    // successful.
+    e.preventDefault();
+    var blacklist_options_container = this.ancestor('div');
+    namespace.blacklist_comment_overlay(
+        e, dsd_link, source_name, latest_comment_container,
+        add_comment_placeholder, blacklist_options_container);
+};
+
+namespace.blacklist_comment_overlay = function(e, dsd_link, source_name,
+                                              latest_comment_container,
+                                              add_comment_placeholder,
+                                              blacklist_options_container) {
+    var comment_form = Y.Node.create("<form />")
+        .appendChild(Y.Node.create("<textarea></textarea>")
+            .set("name", "comment")
+            .set("rows", "3")
+            .set("cols", "60"));
+
+    /* Buttons */
+    var submit_button = Y.Node.create(
+        '<button type="submit" class="lazr-pos lazr-btn" />')
+           .set("text", "OK");
+    var cancel_button = Y.Node.create(
+        '<button type="button" class="lazr-neg lazr-btn" />')
+            .set("text", "Cancel");
+
+    var submit_callback = function(data) {
+        overlay.hide();
+        var comment = "";
+        if (data.comment !== undefined) {
+            comment = data.comment[0];
+        }
+        namespace.blacklist_submit_handler(
+            e, dsd_link, source_name, comment, latest_comment_container,
+            add_comment_placeholder, blacklist_options_container);
+    };
+    var origin = blacklist_options_container.one('.blacklist-options');
+    var overlay = new Y.lazr.FormOverlay({
+        align: {
+             /* Align the centre of the overlay with the centre of the
+                node containing the blacklist options. */
+             node: origin,
+             points: [
+                 Y.WidgetPositionAlign.CC,
+                 Y.WidgetPositionAlign.CC
+             ]
+         },
+         headerContent: "<h2>Add an optional comment</h2>",
+         form_content: comment_form,
+         form_submit_button: submit_button,
+         form_cancel_button: cancel_button,
+         form_submit_callback: submit_callback,
+         visible: true
+    });
+    overlay.render();
+};
+
+namespace.blacklist_submit_handler = function(e, dsd_link, source_name,
+                                              comment,
+                                              latest_comment_container,
+                                              add_comment_placeholder,
+                                              blacklist_options_container) {
+    // Disable all the inputs.
+    blacklist_options_container.all('input').set('disabled', 'disabled');
+    e.target.prepend('<img src="/@@/spinner" />');
+
+    var method_name = (e.target.get('value') === 'NONE') ?
+        'unblacklist' : 'blacklist';
+    var blacklist_all = (e.target.get('value') === 'BLACKLISTED_ALWAYS');
+
+    var diff_rows = Y.all('tr.' + source_name);
+
+    var config = {
+        on: {
+            success: function(updated_entry, args) {
+                // Let the user know this item is now blacklisted.
+                blacklist_options_container.one('img').remove();
+                blacklist_options_container.all(
+                    'input').set('disabled', false);
+                e.target.set('checked', true);
+                Y.each(diff_rows, function(diff_row) {
+                    var fade_to_gray = new Y.Anim({
+                        node: diff_row,
+                        from: { backgroundColor: '#FFFFFF'},
+                        to: { backgroundColor: '#EEEEEE'}
+                        });
+                    if (method_name === 'unblacklist') {
+                        fade_to_gray.set('reverse', true);
+                        }
+                    fade_to_gray.run();
+                    });
+                namespace.add_comment(
+                    updated_entry, add_comment_placeholder,
+                    latest_comment_container);
+            },
+            failure: function(id, response) {
+                blacklist_options_container.one('img').remove();
+                blacklist_options_container.all(
+                    'input').set('disabled', false);
+            }
+        },
+        parameters: {
+            all: blacklist_all,
+            comment: comment
+        }
+    };
+
+    namespace.lp_client.named_post(dsd_link, method_name, config);
+
+};
+
+/**
+ * Link the click event for these blacklist options to the correct
+ * api uri.
+ *
+ * @param blacklist_options {Node} The node containing the blacklist
+ *     options.
+ * @param source_name {string} The name of the source to update.
+ * @param dsd_link {string} The uri for the distroseriesdifference object.
+ * @param latest_comment_container {Node} The node containing the last
+ *     comment.
+ * @param add_comment_placeholder {Node} The node containing the "add
+ *     comment" slot.
+ */
+namespace.setup_blacklist_options = function(
+    blacklist_options, source_name, dsd_link, latest_comment_container,
+    add_comment_placeholder) {
+    Y.on('click', namespace.blacklist_handler,
+         blacklist_options.all('input'),
+         blacklist_options, dsd_link, source_name,
+         latest_comment_container, add_comment_placeholder);
+};
+
+/**
+ * Update the latest comment on the difference row.
+ *
+ * @param comment_entry {lp.client.Entry} An object representing
+ *     a DistroSeriesDifferenceComment.
+ * @param placeholder {Node}
+ *     The node in which to put the latest comment HTML fragment. The
+ *     contents of this node will be replaced.
+ */
+namespace.update_latest_comment = function(comment_entry, placeholder) {
+    var comment_latest_fragment_url =
+        comment_entry.get("web_link") + "/+latest-comment-fragment";
+    var config = {
+        on: {
+            success: function(comment_latest_fragment_html) {
+                placeholder.set(
+                    "innerHTML", comment_latest_fragment_html);
+                Y.lazr.anim.green_flash({node: placeholder}).run();
+            },
+            failure: function(id, response) {
+                Y.lazr.anim.red_flash({node: placeholder}).run();
+            }
+        },
+        accept: Y.lp.client.XHTML
+    };
+    namespace.lp_client.get(comment_latest_fragment_url, config);
+};
+
+/**
+ * This method adds a comment in the UI. It appends a comment to the
+ * list of comments and updates the latest comments slot.
+ *
+ * @param comment_entry {Comment} A comment as returns by the api.
+ * @param add_comment_placeholder {Node} The node that contains the
+ *     relevant comment fields.
+ * @param latest_comment_placeholder {Node} The node that contains the
+ *     latest comment.
+ */
+namespace.add_comment = function(comment_entry, add_comment_placeholder,
+                                latest_comment_placeholder) {
+    // Grab the XHTML representation of the comment
+    // and prepend it to the list of comments.
+    var config = {
+        on: {
+            success: function(comment_html) {
+                var comment_node = Y.Node.create(comment_html);
+                add_comment_placeholder.insert(comment_node, 'before');
+                var reveal = Y.lazr.effects.slide_out(comment_node);
+                reveal.on("end", function() {
+                    Y.lazr.anim.green_flash(
+                        {node: comment_node}).run();
+                });
+                reveal.run();
+            }
+        },
+        accept: Y.lp.client.XHTML
+    };
+    namespace.lp_client.get(comment_entry.get('self_link'), config);
+    namespace.update_latest_comment(
+        comment_entry, latest_comment_placeholder);
+};
+
+/**
+ * Handle the add comment event triggered by the 'add comment' form.
+ *
+ * This method adds a comment via the API and update the UI.
+ *
+ * @param comment_form {Node} The node that contains the relevant comment
+ *     fields.
+ * @param latest_comment_placeholder {Node} The node that contains the
+ *     latest comment.
+ * @param api_uri {string} The uri for the distroseriesdifference to which
+ *     the comment is to be added.
+ * @param cb_success {function} Called when a comment has successfully
+ *     been added. (Deferreds would be awesome right about now.)
+ */
+namespace.add_comment_handler = function(comment_form,
+                                         latest_comment_placeholder, api_uri,
+                                         cb_success) {
+    var comment_area = comment_form.one('textarea');
+    var comment_text = comment_area.get('value');
+
+    // Treat empty comments as mistakes.
+    if (Y.Lang.trim(comment_text).length === 0) {
+        Y.lazr.anim.red_flash({node: comment_area}).run();
+        return;
+    }
+
+    var success_handler = function(comment_entry) {
+        namespace.add_comment(
+            comment_entry, comment_form, latest_comment_placeholder);
+        comment_form.one('textarea').set('value', '');
+        cb_success();
+    };
+    var failure_handler = function(id, response) {
+        // Re-enable field with red flash.
+        Y.lazr.anim.red_flash({node: comment_form}).run();
+    };
+
+    var config = {
+        on: {
+            success: success_handler,
+            failure: failure_handler,
+            start: function() {
+                // Show a spinner.
+                comment_form.one('div.widget-bd')
+                    .append('<img src="/@@/spinner" />');
+                // Disable the textarea and button.
+                comment_form.all('textarea,button')
+                    .setAttribute('disabled', 'disabled');
+            },
+            end: function() {
+                // Remove the spinner.
+                comment_form.all('img').remove();
+                // Enable the form.
+                comment_form.all('textarea,button')
+                    .removeAttribute('disabled');
+            }
+        },
+        parameters: {
+            comment: comment_text
+        }
+    };
+    namespace.lp_client.named_post(api_uri, 'addComment', config);
+};
+
+/**
+ * Add the comment fields ready for sliding out.
+ *
+ * This method adds the markup for a slide-out comment and sets
+ * the event handlers.
+ *
+ * @param placeholder {Node} The node that is to contain the comment
+ *     fields.
+ * @param api_uri {string} The uri for the distroseriesdifference to which
+ *     the comment is to be added.
+ */
+namespace.setup_add_comment = function(placeholder,
+                                       latest_comment_placeholder,
+                                       api_uri) {
+    placeholder.insert([
+        '<a class="widget-hd js-action sprite add" href="">',
+        '  Add comment</a>',
+        '<div class="widget-bd lazr-closed" ',
+        '     style="height:0;overflow:hidden">',
+        '  <textarea></textarea><button>Save comment</button>',
+        '</div>'
+        ].join(''), 'replace');
+
+    // The comment area should slide in when the 'Add comment'
+    // action is clicked.
+    var slide_anim = null;
+    var slide = function(direction) {
+        // Slide out if direction is true, slide in if direction
+        // is false, otherwise do the opposite of what's being
+        // animated right now.
+        if (slide_anim === null) {
+            slide_anim = Y.lazr.effects.slide_out(
+                placeholder.one('div.widget-bd'));
+            if (Y.Lang.isBoolean(direction)) {
+                slide_anim.set("reverse", !direction);
+            }
+        }
+        else {
+            if (Y.Lang.isBoolean(direction)) {
+                slide_anim.set("reverse", !direction);
+            }
+            else {
+                slide_anim.set('reverse', !slide_anim.get('reverse'));
+            }
+            slide_anim.stop();
+        }
+        slide_anim.run();
+    };
+    var slide_in = function() { slide(false); };
+
+    placeholder.one('a.widget-hd').on(
+        'click', function(e) { e.preventDefault(); slide(); });
+    placeholder.one('button').on('click', function(e) {
         e.preventDefault();
-        var toggle = e.currentTarget;
-        var row = toggle.ancestor('tr');
-        toggle.toggleClass('treeCollapsed').toggleClass('treeExpanded');
-
-        // Only insert if there isn't already a container row there.
-        var next_row = row.next();
-        var details_row;
-        if (next_row === null || !next_row.hasClass('diff-extra')) {
-            var source_name = row.one('a.toggle-extra').get('text');
-            var rev_link = row
-                .one('a.toggle-extra').get('href').split('/').reverse();
-            var parent_series_name = rev_link[0];
-            var parent_distro_name = rev_link[1];
-            var nb_columns = row.all('td').size();
-            details_row = Y.Node.create([
-                '<table><tr class="diff-extra unseen ' + source_name + '">',
-                '  <td colspan="'+nb_columns+'">',
-                '    <div class="diff-extra-container"></div>',
-                '  </td></tr></table>'
-                ].join('')).one('tr');
-            row.insert(details_row, 'after');
-            var uri = toggle.get('href');
-            get_extra_diff_info(
-                uri, row, details_row.one('td'), source_name,
-                parent_distro_name, parent_series_name);
-        } else {
-            details_row = next_row;
-        }
-        details_row.toggleClass('unseen');
-
-    };
-
-
+        namespace.add_comment_handler(
+            placeholder, latest_comment_placeholder,
+            api_uri, slide_in);
+    });
+};
+
+namespace.setup = function() {
     Y.all('table.listing a.toggle-extra').each(function(toggle){
-        toggle.addClass('treeCollapsed').addClass('sprite');
-        toggle.on("click", expander_handler);
+        var row = new namespace.ExpandableRowWidget({toggle: toggle});
     });
-
 };
 
 
@@ -528,7 +553,7 @@
     }
     return undefined;
 };
-
+ 
 namespace.add_link_to_package_diff = function(container, url_uri) {
     var y_config = {
         headers: {'Accept': 'application/json;'},
@@ -553,7 +578,7 @@
             }
         }
     };
-    lp_client.named_get(url_uri , null, y_config);
+    namespace.lp_client.named_get(url_uri , null, y_config);
 };
 
 /**
@@ -579,7 +604,7 @@
         ].join('/');
     var build_package_diff_update_config = {
         uri: package_diff_uri,
-        lp_client: lp_client,
+        lp_client: namespace.lp_client,
         parent: parent,
         /**
         * This function knows how to update a package diff status.
@@ -642,7 +667,7 @@
     // one clickable node.
     container.all('.package-diff-compute-request').on('click', function(e) {
         e.preventDefault();
-        compute_package_diff(container, dsd_link);
+        namespace.compute_package_diff(container, dsd_link);
     });
 };
 
@@ -681,7 +706,7 @@
 * @param row {Node} The container node for this package diff.
 * @param dsd_link {string} The uri for the distroseriesdifference object.
 */
-var compute_package_diff = function(container, dsd_link) {
+namespace.compute_package_diff = function(container, dsd_link) {
     var in_progress_message = Y.lp.soyuz.base.makeInProgressNode(
         'Computing package diff...');
     namespace.add_msg_node(container, in_progress_message);
@@ -704,7 +729,7 @@
         container.one('p.update-in-progress-message').remove();
         var recompute = function(e) {
             e.preventDefault();
-            compute_package_diff(container, dsd_link);
+            namespace.compute_package_diff(container, dsd_link);
         };
 
         var msg = response.responseText;
@@ -727,7 +752,7 @@
             'dsd_link': dsd_link
         }
     };
-    lp_client.named_post(dsd_link, 'requestPackageDiffs', config);
+    namespace.lp_client.named_post(dsd_link, 'requestPackageDiffs', config);
 };
 
 }, "0.1", {"requires": ["event-simulate", "io-base",

=== modified file 'lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html'
--- lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html	2011-07-08 06:06:15 +0000
+++ lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.html	2011-08-09 11:56:45 +0000
@@ -15,6 +15,8 @@
   <script type="text/javascript" src="../../../app/javascript/client.js"></script>
   <script type="text/javascript" src="../../../soyuz/javascript/base.js"></script>
   <script type="text/javascript" src="../../../soyuz/javascript/lp_dynamic_dom_updater.js"></script>
+  <script type="text/javascript" src="../../../app/javascript/lazr/lazr.js"></script>
+  <script type="text/javascript" src="../../../app/javascript/anim/anim.js"></script>
 
   <!-- The module under test -->
   <script type="text/javascript" src="../distroseriesdifferences_details.js"></script>
@@ -24,7 +26,7 @@
 </head>
 
 <body class="yui3-skin-sam">
-  <h1>Testing the Source package differences javascript</h1>
+  <h1>Testing the DistroSeriesDifferenceDetails javascript</h1>
 
   <h2>Errors</h2>
   <div id="errors"></div>
@@ -32,38 +34,5 @@
   <div id="placeholder" style="display:none;">
   </div>
 
-  <div id="placeholder_base" style="display:none;">
-    <table class="listing">
-      <tbody><tr><td>
-        <div class="diff-extra-container">
-          <input name="field.selected_differences" type="checkbox" />
-          <a class="toggle-extra"
-            href="/deribuntu/deriwarty/+source/evolution/+difference/ubuntu/warty">
-            evolution</a>
-          <span class="package-diff-button"></span>
-          <dt class="package-diff-placeholder">
-          <span class="package-diff-compute-request">
-            <a class="js-action sprite add" href="">
-              Compute differences from last common version:
-            </a>
-          </span></dt>
-          <dd>
-            <ul class="package-diff-status">
-              <li>
-                <span id="derived" class="PENDING">
-                1.2.1 to Derilucid version: 1.2.4
-                </span>
-              </li>
-              <li>
-                <span id="parent" class="request-derived-diff">
-                  1.2.1 to Lucid version: 1.2.3
-                </span>
-              </li>
-            </ul>
-          </dd>
-        </div>
-      </td></tr></tbody>
-    </table>
-  </div>
 </body>
 </html>

=== modified file 'lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js'
--- lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js	2011-08-09 11:56:43 +0000
+++ lib/lp/registry/javascript/tests/test_distroseriesdifferences_details.js	2011-08-09 11:56:45 +0000
@@ -7,18 +7,137 @@
         'lp.soyuz.dynamic_dom_updater', 'event-simulate', "io-base",
         'lp.registry.distroseriesdifferences_details', function(Y) {
 
-var Assert = Y.Assert;
-var ArrayAssert = Y.ArrayAssert;
 var suite = new Y.Test.Suite("Distroseries differences Tests");
 var dsd_details = Y.lp.registry.distroseriesdifferences_details;
 var dsd_uri = '/duntu/dwarty/+source/evolution/+difference/ubuntu/warty';
 
-var placeholder_content = Y.one('#placeholder_base').get('innerHTML');
+var table_html = [
+    '<table class="listing"><tbody>',
+    '  <tr class="evolution">',
+    '    <td>',
+    '      <a href="/deribuntu/deriwarty/+source/evolution/+difference/ubuntu/warty"',
+    '         class="js-action toggle-extra treeCollapsed ',
+    '         sprite">evolution</a>',
+    '    </td>',
+    '    <td>',
+    '      <a href="/ubuntu/warty" class="parent-name">Warty</a>',
+    '    </td>',
+    '    <td>',
+    '      <a href="/ubuntu/warty/+source/evolution/2.0.9-1ubuntu2"',
+    '         class="parent-version">',
+    '         2.0.9-1ubuntu2</a>',
+    '    </td>',
+    '    <td>',
+    '      <a href="/deribuntu/deriwarty/+source/evolution/2.0.8-4deribuntu1"',
+    '         class="derived-version">',
+    '         2.0.8-4deribuntu1</a>',
+    '    </td>',
+    '    <td class="packagesets"></td>',
+    '    <td class="last-changed"></td>',
+    '    <td class="latest-comment-fragment"></td>',
+    '  </tr>',
+    '</tbody></table>'
+    ].join('');
+
+
+var testExpandableRowWidget = {
+
+    name: 'expandable-row-widget',
+
+    setUp: function() {
+        Y.one("#placeholder")
+            .empty()
+            .appendChild(Y.Node.create(table_html));
+        this.toggle = Y.one('table.listing a.toggle-extra');
+     },
+
+    test_initializer: function() {
+       var row = new dsd_details.ExpandableRowWidget({toggle: this.toggle});
+       Y.Assert.isTrue(this.toggle.hasClass('treeCollapsed'));
+       Y.Assert.isTrue(this.toggle.hasClass('sprite'));
+    },
+
+    test_parse_row_data: function() {
+        var row = new dsd_details.ExpandableRowWidget({toggle: this.toggle});
+        var parsed = row.parse_row_data();
+        var res = {
+            source_name: 'evolution',
+            parent_series_name: 'warty',
+            parent_distro_name: 'ubuntu',
+            nb_columns: 7};
+        Y.ObjectAssert.areEqual(res, parsed);
+    },
+
+    test_expander_handler_adds_new_row: function() {
+        var row = new dsd_details.ExpandableRowWidget({toggle: this.toggle});
+        row._toggle.simulate('click');
+        var new_row = row._row.next();
+        Y.Assert.isTrue(new_row.hasClass('evolution'));
+        Y.Assert.isTrue(new_row.hasClass('diff-extra'));
+        Y.Assert.areEqual(7, new_row.one('td').getAttribute('colspan'));
+    },
+
+    test_expand_handler_toggles_hiding: function() {
+        var row = new dsd_details.ExpandableRowWidget({toggle: this.toggle});
+        Y.Assert.isTrue(row._toggle.hasClass('treeCollapsed'));
+        // First click opens up the new row.
+        row._toggle.simulate('click');
+        var new_row = row._row.next();
+        Y.Assert.isTrue(row._toggle.hasClass('treeExpanded'));
+        Y.Assert.isFalse(new_row.hasClass('unseen'));
+        Y.Assert.isTrue(new_row.one('div').hasClass('diff-extra-container'));
+        // Second click hides it.
+        row._toggle.simulate('click');
+        Y.Assert.isTrue(row._toggle.hasClass('treeCollapsed'));
+        Y.Assert.isTrue(new_row.hasClass('unseen'));
+    }
+
+};
+
+var extra_table_html = [
+    '<table class="listing">',
+    '  <tbody><tr><td>',
+    '    <div class="diff-extra-container">',
+    '      <input name="field.selected_differences" type="checkbox" />',
+    '      <a class="toggle-extra"',
+    '        href="/deribuntu/deriwarty/+source/evolution/+difference/ubuntu/warty">',
+    '        evolution</a>',
+    '      <span class="package-diff-button"></span>',
+    '      <dt class="package-diff-placeholder">',
+    '       <span class="package-diff-compute-request">',
+    '        <a class="js-action sprite add" href="">',
+    '          Compute differences from last common version:',
+    '        </a>',
+    '      </span></dt>',
+    '      <dd>',
+    '        <ul class="package-diff-status">',
+    '          <li>',
+    '            <span id="derived" class="PENDING">',
+    '             1.2.1 to Derilucid version: 1.2.4',
+    '            </span>',
+    '          </li>',
+    '          <li>',
+    '            <span id="parent" class="request-derived-diff">',
+    '              1.2.1 to Lucid version: 1.2.3',
+    '            </span>',
+    '          </li>',
+    '        </ul>',
+    '      </dd>',
+    '    </div>',
+    '  </td></tr></tbody>',
+    '</table>'
+    ].join('');
 
 var testPackageDiffUpdate = {
 
     name: 'package-diff',
 
+    setUp: function() {
+        Y.one("#placeholder")
+            .empty()
+            .appendChild(Y.Node.create(extra_table_html));
+    },
+
     test_vocabulary_helper: function() {
         // The vocabulary helper extracts the selected item from a
         // jsonified vocabulary.
@@ -27,23 +146,21 @@
             {"token": "COMPLETED", "selected": true, "title": "Completed"},
             {"token": "FAILED", "title": "Failed"}];
         var res = dsd_details.get_selected(voc);
-        Assert.areEqual('COMPLETED', res.token);
-        Assert.areEqual('Completed', res.title);
+        Y.Assert.areEqual('COMPLETED', res.token);
+        Y.Assert.areEqual('Completed', res.title);
         var voc_nothing_selected = [
             {"token": "PENDING", "title": "Pending"},
             {"token": "COMPLETED", "title": "Completed"},
             {"token": "FAILED", "title": "Failed"}];
-        Assert.isUndefined(dsd_details.get_selected(voc_nothing_selected));
+        Y.Assert.isUndefined(dsd_details.get_selected(voc_nothing_selected));
     },
 
     test_add_msg_node: function() {
-        var placeholder = Y.one('#placeholder');
-        placeholder.set('innerHTML', placeholder_content);
         var msg_txt = 'Exemple text';
         var msg_node = Y.Node.create(msg_txt);
         placeholder = Y.one('#placeholder');
         dsd_details.add_msg_node(placeholder, msg_node);
-        Assert.areEqual(
+        Y.Assert.areEqual(
             placeholder.one('.package-diff-placeholder').get('innerHTML'),
             msg_txt);
     }
@@ -55,7 +172,10 @@
     name: 'package-diff-update-interaction',
 
     setUp: function() {
-       var first_poll = true;
+        Y.one("#placeholder")
+            .empty()
+            .appendChild(Y.Node.create(extra_table_html));
+        var first_poll = true;
         pending_voc = [
             {"token": "PENDING", "selected": true, "title": "Pending"},
             {"token": "COMPLETED", "title": "Completed"},
@@ -89,7 +209,6 @@
         // '.package-diff-compute-request'.
         // bug=746277.
         var placeholder = Y.one('#placeholder');
-        placeholder.set('innerHTML', placeholder_content);
         placeholder
             .one('#derived')
                 .removeClass('PENDING')
@@ -109,18 +228,17 @@
         var package_diff = Y.one('#parent');
 
         // The request has not been triggered.
-        Assert.isTrue(package_diff.hasClass('request-derived-diff'));
+        Y.Assert.isTrue(package_diff.hasClass('request-derived-diff'));
     },
 
     test_request_package_diff_computation: function() {
         // A click on the button changes the package diff status and requests
         // the package diffs computation via post.
         var placeholder = Y.one('#placeholder');
-        placeholder.set('innerHTML', placeholder_content);
         placeholder
             .one('#derived')
-                .removeClass('PENDING')
-                    .addClass('FAILED');
+            .removeClass('PENDING')
+            .addClass('FAILED');
         dsd_details.setup_packages_diff_states(
             placeholder.one('.diff-extra-container'), dsd_uri);
         var func_req;
@@ -135,16 +253,16 @@
         button.simulate('click');
         var package_diff = Y.one('#parent');
 
-        Assert.isTrue(package_diff.hasClass('PENDING'));
-        Assert.isFalse(package_diff.hasClass('request-derived-diff'));
-        Assert.areEqual('requestPackageDiffs', func_req);
-        Assert.isNotUndefined(package_diff.updater);
+        Y.Assert.isTrue(package_diff.hasClass('PENDING'));
+        Y.Assert.isFalse(package_diff.hasClass('request-derived-diff'));
+        Y.Assert.areEqual('requestPackageDiffs', func_req);
+        Y.Assert.isNotUndefined(package_diff.updater);
 
         // Let the polling happen.
         this.wait(function() {
-            Assert.isTrue(package_diff.hasClass('PENDING'));
+            Y.Assert.isTrue(package_diff.hasClass('PENDING'));
                 this.wait(function() {
-                    Assert.isTrue(package_diff.hasClass('COMPLETED'));
+                    Y.Assert.isTrue(package_diff.hasClass('COMPLETED'));
                 }, dsd_details.poll_interval);
          }, dsd_details.poll_interval);
     },
@@ -153,16 +271,15 @@
         // The polling has started on the pending package diff. The
         // status is being updated.
         var placeholder = Y.one('#placeholder');
-        placeholder.set('innerHTML', placeholder_content);
         dsd_details.setup_packages_diff_states(
             placeholder.one('.diff-extra-container'), dsd_uri);
         var package_diff = Y.one('#derived');
-        Assert.isTrue(package_diff.hasClass('PENDING'));
-        Assert.isFalse(package_diff.hasClass('request-derived-diff'));
+        Y.Assert.isTrue(package_diff.hasClass('PENDING'));
+        Y.Assert.isFalse(package_diff.hasClass('request-derived-diff'));
         this.wait(function() {
-            Assert.isTrue(package_diff.hasClass('PENDING'));
+            Y.Assert.isTrue(package_diff.hasClass('PENDING'));
                 this.wait(function() {
-                    Assert.isTrue(package_diff.hasClass('COMPLETED'));
+                    Y.Assert.isTrue(package_diff.hasClass('COMPLETED'));
                 }, dsd_details.poll_interval);
         }, dsd_details.poll_interval);
     }
@@ -170,6 +287,7 @@
 
 suite.add(new Y.Test.Case(testPackageDiffUpdate));
 suite.add(new Y.Test.Case(testPackageDiffUpdateInteraction));
+suite.add(new Y.Test.Case(testExpandableRowWidget));
 
 Y.lp.testing.Runner.run(suite);
 

=== modified file 'lib/lp/registry/templates/distroseries-localdifferences.pt'
--- lib/lp/registry/templates/distroseries-localdifferences.pt	2011-07-29 12:51:34 +0000
+++ lib/lp/registry/templates/distroseries-localdifferences.pt	2011-08-09 11:56:45 +0000
@@ -219,9 +219,10 @@
     </div>
 <script type="text/javascript">
 LPS.use('lp.registry.distroseriesdifferences_details', function(Y) {
-  diff_module = Y.lp.registry.distroseriesdifferences_details
 
-  Y.on('domready', diff_module.setup_expandable_rows);
+  Y.on('domready', function() {
+    Y.lp.registry.distroseriesdifferences_details.setup();
+  });
 });
 </script>
 <script type="text/javascript">