Merge lp:~wallyworld/launchpad/new-team-picker-load-form into lp:launchpad

[Ian Booth]

Load the team form from the model and make it available for the New Team functionality in the pickers.

== Implementation ==

The branch continues the work to add team creation capability to the person picker. The previous branch hard coded a sample team form, this branch provides the infrastructure to fetch the actual team form html from the model using an XHR ++form++ call. There are also some small fixes noted in the review comments from the previous branch.

There may be several pickers which need to have the create team form html provided to them. We don't want to make a separate XHR call for each picker, so what happens is that the first picker to be processed makes the call and the others are queued up and provided with the result when it arrives. A YUI namespace is used to coordinate the workflow since the picker javascript can live inside separate yui instances and events don't propagate across instances.

The XHR call to fetch the create team form happens from the picker initialisation. By the time the user clicks on the New Team link, the form should already have been loaded and rendered, but if not, the user sees a big spinner which is replaced by the form automatically when it arrives.

This branch sees a form containing every team attribute rendered. The next branch will implement a form with only a subset of the attributes eg name, displayname, subscription policy etc.

== Tests ==

Existing yui tests were modified to account for the XHR call and stub that out. A test was added to ensure only one XHR call is made for multiple pickers.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/picker/person_picker.js
  lib/lp/app/javascript/picker/picker.js
  lib/lp/app/javascript/picker/picker_patcher.js
  lib/lp/app/javascript/picker/tests/test_personpicker.html
  lib/lp/app/javascript/picker/tests/test_personpicker.js
  lib/lp/app/javascript/picker/tests/test_picker_patcher.js
  lib/lp/registry/javascript/sharing/shareepicker.js
  lib/lp/registry/javascript/sharing/tests/test_shareepicker.js

[Richard Harding (rharding)]

Ian, just giving a quick look and was curious if you'd thought about doing the load_new_team_form purely with events? The pickers would listen to an event: Y.on('new_team_form:load') and then the ajax code could just fire that event with the html it fetched to help decouple the whole thing and make it easier to test each endpoint?

[Ian Booth (wallyworld)]

Hi Rick

It was buried in my long covering letter - yes I did consider it, but this approach will suffer the same sad fate as when it was tried previously - there are separate yui instances involved due to js embedded in the widget TAL so event propagation won't work across instances and global events are verboten. So I have to use the pattern introduced previously ie coordinate things using a namespace.

[Curtis Hovey (sinzui)]

Thank you for this branch. I have a few comments that need fixing before you land this branch.

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js 2012-06-21 07:43:17 +0000
+++ lib/lp/app/javascript/picker/person_picker.js 2012-06-25 04:17:20 +0000
...

We prefer function expressions because they clearly show that the
function is a value of a variable. function statements are always hoisted
to the top of where scope is defined to ensure you can call them before they
are defined. Browsers and developers handle this badly.

+ function on_success(id, response, picker) {
        var on_success = function(id, response, picker) {

...
+ function on_failure(id, response, picker) {
        var on_failure = function(id, response, picker) {

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-06-21 03:48:05 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-06-25 04:17:20 +0000
...
I don't think the   belongs in this test, or in code to make this
visible to webkit...this link is missing action-icon. lazr-btn was intended
for <button>. I think a lot of code uses it as a marker, but we do not
have css rules to make it present weill with links.
+ <a id="edit-anotherpickertest-btn"
+ class="sprite edit lazr-btn yui3-activator-act"
+ href="/fnord/+edit-people"
+ >&nbsp;Edit</a>

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js 2012-06-22 14:12:33 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js 2012-06-25 04:17:20 +0000
...

@@ -325,6 +329,32 @@
This markup is invalid. The YUI test will not play in some browsers, notably
IE which knows that <tr> elements may only contain <td>. Some browsers will
will add the <td> for you in the DOM...but that means scripts may break
because they assume there is no <td>.
+ _simple_team_form: function() {
+ return '<table><tr>' +
+ '<input id="field.name">' +
+ '<input id="field.displayname"></tr></table>';
+ },

review: Approve (code)

[Ian Booth (wallyworld)]

Thanks for the review.

>
> === modified file 'lib/lp/app/javascript/picker/person_picker.js'
> --- lib/lp/app/javascript/picker/person_picker.js 2012-06-21 07:43:17 +0000
> +++ lib/lp/app/javascript/picker/person_picker.js 2012-06-25 04:17:20 +0000
> ...
>
> We prefer function expressions because they clearly show that the
> function is a value of a variable. function statements are always hoisted
> to the top of where scope is defined to ensure you can call them before they
> are defined. Browsers and developers handle this badly.
>

Fair point. I would never do this myself if I were writing the code from
scratch. This code pattern was lifted from formoverlay, and is also used
in subscribers_list and filebug_dupfinder. I'll file a bug to ensure
these other places are fixed.

>
> === modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
> --- lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-06-21 03:48:05 +0000
> +++ lib/lp/app/javascript/picker/tests/test_personpicker.html 2012-06-25 04:17:20 +0000
> ...
> I don't think the &nbsp; belongs in this test, or in code to make this
> visible to webkit...this link is missing action-icon. lazr-btn was intended
> for <button>. I think a lot of code uses it as a marker, but we do not
> have css rules to make it present weill with links.
> + <a id="edit-anotherpickertest-btn"
> + class="sprite edit lazr-btn yui3-activator-act"
> + href="/fnord/+edit-people"
> + >&nbsp;Edit</a>
>
>

Ah, the &nbsp; is old markup originally added to the test harness by
copying old rendered lp html before the recent css changes and this html
in the test was copied to create anotherpicker anchor for this branch.
The tests don't seem to care either way. I'll certainly amend the markup
to confirm with the recent changes.

> === modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
> --- lib/lp/app/javascript/picker/tests/test_personpicker.js 2012-06-22 14:12:33 +0000
> +++ lib/lp/app/javascript/picker/tests/test_personpicker.js 2012-06-25 04:17:20 +0000
> ...
>
> @@ -325,6 +329,32 @@
> This markup is invalid. The YUI test will not play in some browsers, notably
> IE which knows that <tr> elements may only contain <td>. Some browsers will
> will add the <td> for you in the DOM...but that means scripts may break
> because they assume there is no <td>.
> + _simple_team_form: function() {
> + return '<table><tr>' +
> + '<input id="field.name">' +
> + '<input id="field.displayname"></tr></table>';
> + },
>

Yes, you are correct. Clearly my attempt at some trivial html for the
test failed. Will fix.

=== modified file 'lib/lp/app/javascript/formoverlay/formoverlay.js'
--- lib/lp/app/javascript/formoverlay/formoverlay.js	2011-08-31 02:58:52 +0000
+++ lib/lp/app/javascript/formoverlay/formoverlay.js	2012-06-26 00:18:26 +0000
@@ -523,17 +523,17 @@
      * @param url {String} The URL from where to load the form content.
      */
     loadFormContentAndRender: function (url) {
-        function on_success(id, response, overlay) {
+        var on_success = function(id, response, overlay) {
             overlay.set('form_content', response.responseText);
             overlay.renderUI();
             overlay.bindUI();
-        }
-        function on_failure(id, response, overlay) {
+        };
+        var on_failure = function(id, response, overlay) {
             overlay.set(
                 'form_content',
                 "Sorry, an error occurred while loading the form.");
             overlay.renderUI();
-        }
+        };
         var cfg = {
             on: {success: on_success, failure: on_failure},
             "arguments": this

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2012-06-21 07:43:17 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2012-06-26 00:18:26 +0000
@@ -18,62 +18,50 @@
         if (!Y.Lang.isValue(LP.links.me)) {
             this.set('show_assign_me_button', false);
         }
-        this.set('new_team_template', this._new_team_template());
-        this.set('new_team_form', this._new_team_form());
+        if (this.get('show_create_team')) {
+            // We need to provide the 'New team' link.
+            // There could be several pickers and we only want to make the XHR
+            // call to get the form once. So first one gets to do the call and
+            // subsequent ones register the to be notified of the result.
+            this.team_form_node = Y.Node.create(this._new_team_template());
+            this.form_namespace = Y.namespace('lp.app.picker.teamform');
+            var form_callbacks = this.form_namespace.form_callbacks;
+            var perform_load = false;
+            if (!Y.Lang.isArray(form_callbacks)) {
+                perform_load = true;
+                form_callbacks = [];
+                this.form_namespace.form_callbacks = form_callbacks;
+            }
+            form_callbacks.push({
+                picker: this,
+                callback: this._render_new_team_form});
+            this._load_new_team_form(perform_load);
+        }
     },
 
     _new_team_template: function() {
         return [
           '<div class="new-team-node">',
-          '<div class="step-on" style="width: 100%;"></div>',
-          '<div class="transparent important-notice-popup">',
-            '{{> new_team_form}}',
-            '<div class="extra-form-buttons">',
-              '<button class="yes_button" type="button"></button>',
-              '<button class="no_button" type="button"></button>',
+          '<div id=new-team-form-placeholder ',
+              'class="yui3-overlay-indicator-content">',
+              '<img src="/@@/spinner-big/">',
+          '</div>',
+          '<div class="extra-form-buttons hidden">',
+              '<button class="yes_button" type="button">',
+                  'Create Team</button>',
+              '<button class="no_button" type="button">Cancel</button>',
             '</div>',
           '</div>',
           '</div>'].join('');
     },
 
-    _new_team_form: function() {
-        // TODO - get the form using ++form++
-        return [
-        "<table id='launchpad-form-widgets' class='form'>",
-        "<tbody><tr><td colspan='2'><div>",
-        "<label for='field.name'>Name:</label><div>",
-        "<input type='text' value='' size='20'",
-        "    name='field.name' id='field.name'",
-        "    class='lowerCaseText textType'></div>",
-        "<p class='formHelp'>",
-        "    A short unique name, beginning with a lower-case letter",
-        "    or number, and containing only letters, numbers, dots,",
-        "    hyphens, or plus signs.</p>",
-        "</div></td></tr><tr><td colspan='2'><div>",
-        "<label for='field.displayname'>Display Name:</label><div>",
-        "<input type='text' value='' size='20'",
-        "    name='field.displayname' id='field.displayname'",
-        "    class='textType'></div>",
-        "<p class='formHelp'>",
-        "    This team's name as you would like it displayed",
-        "    throughout Launchpad.</p>",
-        "</div></td></tr><tr><td colspan='2'><div>",
-        "<label for='field.visibility'>Visibility:</label>",
-        "<div><div><div class='value'>",
-        "<select size='1'",
-        "    name='field.visibility' id='field.visibility'>",
-        "<option value='PUBLIC' selected='selected'>Public</option>",
-        "<option value='PRIVATE'>Private</option></select></div>",
-        "</div></div><p class='formHelp'>",
-        "    Anyone can see a public team's data. Only team members",
-        "    and Launchpad admins can see private team data.",
-        "    Private teams cannot become public.</p>",
-        "</div></td></tr></tbody></table>"
-        ].join('');
-    },
-
     hide: function() {
         this.get('boundingBox').setStyle('display', 'none');
+        // We want to cancel the new team form is there is one rendered.
+        var node = this.get('contentBox').one('.new-team-node');
+        if (Y.Lang.isValue(node) && !node.hasClass('hidden')) {
+            this.hide_extra_content(node, false);
+        }
         Y.lazr.picker.Picker.prototype.hide.call(this);
     },
 
@@ -128,72 +116,113 @@
         });
     },
 
-    _cancel_new_team: function(picker) {
-        var node = picker.get('contentBox').one('.new-team-node');
-        picker.hide_extra_content(node);
+    _cancel_new_team: function() {
+        var node = this.get('contentBox').one('.new-team-node');
+        this.hide_extra_content(node);
     },
 
-    _save_new_team: function(picker) {
-        var node = picker.get('contentBox').one('.new-team-node');
-        var team_name = Y.Node.getDOMNode(node.one('[id=field.name]')).value;
-        var team_display_name =
-            Y.Node.getDOMNode(node.one('[id=field.displayname]')).value;
-        picker.hide_extra_content(node);
+    _save_new_team: function() {
+        var node = this.get('contentBox').one('.new-team-node');
+        var team_name = node.one('[id=field.name]').get('value');
+        var team_display_name = node.one('[id=field.displayname]')
+            .get('value');
+        this.hide_extra_content(node, false);
         // TODO - make back end call to save team
         var value = {
             "api_uri": "/~" + team_name,
             "title": team_display_name,
             "value": team_name,
             "metadata": "team"};
-        picker.fire('validate', value);
-    },
-
-    show_new_team_form: function () {
-        var partials = {new_team_form: this.get('new_team_form')};
-        var html = Y.lp.mustache.to_html(
-            this.get('new_team_template'), {}, partials);
-        var self = this;
+        this.fire('validate', value);
+    },
+
+    _load_new_team_form: function (perform_load) {
+        // Load the new team form from the model using an XHR call.
+        // If perform_load is true, this is the first invocation of this method
+        // across all pickers so we do the XHR call and send the result to all
+        // registered pickers.
+        // If perform_load is false, another picker is making the XNR call and
+        // all we want to do is receive and render the preloaded_team_form.
+        // We first check though that the result hasn't arrived already.
+        var preloaded_team_form = this.form_namespace.team_form;
+        if (Y.Lang.isValue(preloaded_team_form)) {
+            this._render_new_team_form(preloaded_team_form, true);
+            return;
+        }
+        if (!perform_load) {
+            return;
+        }
+
+        var on_success = function(id, response, picker) {
+            Y.Array.each(picker.form_namespace.form_callbacks,
+                function(callback_info) {
+                Y.bind(
+                    callback_info.callback, callback_info.picker,
+                    response.responseText, true)();
+            });
+            picker.form_namespace.team_form = response.responseText;
+        };
+        var on_failure = function(id, response, picker) {
+            Y.Array.each(picker.form_namespace.form_callbacks,
+                function(callback_info) {
+                Y.bind(
+                    callback_info.callback, callback_info.picker,
+                    'Sorry, an error occurred while loading the form.',
+                    false)();
+            });
+        };
+        var cfg = {
+            on: {success: on_success, failure: on_failure},
+            "arguments": this
+            };
+        var uri = Y.lp.client.get_absolute_uri('people/+newteam/++form++');
+        uri = uri.replace('api/devel', '');
+        this.get("io_provider").io(uri, cfg);
+    },
+
+    _render_new_team_form: function(form_html, show_submit) {
+        // Poke the actual team form into the DOM and wire up the save and
+        // cancel buttons.
+        this.team_form_node.one('#new-team-form-placeholder')
+            .replace(form_html);
         var button_callback = function(e, callback_fn) {
             e.halt();
             if (Y.Lang.isFunction(callback_fn) ) {
-                callback_fn(self);
+                Y.bind(callback_fn, this)();
             }
         };
-        var team_form_node = Y.Node.create(html);
-        team_form_node.one(".yes_button")
-            .set('text', 'Create Team')
-            .on('click', function(e) {
-                button_callback(e, self._save_new_team);
-            });
+        var submit_button = this.team_form_node.one(".yes_button");
+        if (show_submit) {
+                submit_button.on(
+                    'click', button_callback, this, this._save_new_team);
+        } else {
+            submit_button.addClass('hidden');
+        }
+        this.team_form_node.one(".no_button")
+            .on('click', button_callback, this, this._cancel_new_team);
+        this.team_form_node.one('.extra-form-buttons')
+            .removeClass('hidden');
+    },
 
-        team_form_node.one(".no_button")
-            .set('text', 'Cancel')
-            .on('click', function(e) {
-                button_callback(e, self._cancel_new_team);
-            });
-        this.get('contentBox').one('.yui3-widget-bd')
-            .insert(team_form_node, 'before');
+    show_new_team_form: function() {
         this.show_extra_content(
-            team_form_node.one(".important-notice-popup"),
-            "Enter new team details");
+            this.team_form_node, "Enter new team details");
     },
 
     _assign_me_button_html: function() {
         return [
-            '<a class="yui-picker-assign-me-button bg-image ',
+            '<a class="yui-picker-assign-me-button sprite person ',
             'js-action" href="javascript:void(0)" ',
-            'style="background-image: url(/@@/person); ',
-            'padding-right: 1em">',
+            'style="padding-right: 1em">',
             this.get('assign_me_text'),
             '</a>'].join('');
     },
 
     _remove_button_html: function() {
         return [
-            '<a class="yui-picker-remove-button bg-image js-action" ',
-            'href="javascript:void(0)" ',
-            'style="background-image: url(/@@/remove); ',
-            'padding-right: 1em">',
+            '<a class="yui-picker-remove-button sprite remove ',
+            'js-action" href="javascript:void(0)" ',
+            'style="padding-right: 1em">',
             this.get('remove_person_text'),
             '</a>'].join('');
     },
@@ -250,7 +279,16 @@
         min_search_chars: {value: 2},
         show_create_team: {value: false},
         new_team_template: {value: null},
-        new_team_form: {value: null}
+        new_team_form: {value: null},
+      /**
+       * The object that provides the io function for doing XHR requests.
+       *
+       * @attribute io_provider
+       * @type object
+       * @default Y
+       */
+      io_provider: {value: Y}
     }
 });
-}, "0.1", {"requires": ["base", "node", "lazr.picker", "lp.mustache"]});
+}, "0.1", {"requires": [
+    "base", "node", "lazr.picker"]});

=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js	2012-06-22 14:12:33 +0000
+++ lib/lp/app/javascript/picker/picker.js	2012-06-26 00:18:26 +0000
@@ -825,56 +825,81 @@
     /*
      * Insert the extra content into the form and animate its appearance.
      */
-    show_extra_content: function(extra_content, header) {
+    show_extra_content: function(extra_content, header, steptitle, progress) {
         if (Y.Lang.isValue(header)) {
-            this.set('picker_header', this.get('headerContent'));
+            if (!Y.Lang.isValue(this.get('saved_header'))) {
+                this.set('saved_header', this.get('headerContent'));
+            }
             this.set(
                 'headerContent',
                 Y.Node.create("<h2></h2>").set('text', header));
         }
-        this.get('contentBox').one('.yui3-widget-bd').hide();
-        this.get('contentBox').all('.steps').hide();
-        var duration = 0;
-        if (this.get('use_animation')) {
-            duration = 0.9;
-        }
-        var fade_in = new Y.Anim({
-            node: extra_content,
-            to: {opacity: 1},
-            duration: duration
-        });
-        fade_in.run();
+        if (Y.Lang.isValue(steptitle)) {
+            if (!Y.Lang.isValue(this.get('saved_steptitle'))) {
+                this.set('saved_steptitle', this.get('steptitle'));
+            }
+            this.set('steptitle', steptitle);
+        }
+        if (Y.Lang.isValue(progress)) {
+            if (!Y.Lang.isValue(this.get('saved_progress'))) {
+                this.set('saved_progress', this.get('progress'));
+            }
+            this.set('progress', progress);
+        }
+        var contentBox = this.get('contentBox');
+        var original_content = contentBox.one('.yui3-widget-bd');
+        var extra_content_id = extra_content.get('id');
+        if (!Y.Lang.isValue(contentBox.one('#'+extra_content_id))) {
+            extra_content.addClass('important-notice-popup');
+            original_content.insert(extra_content, 'before');
+        }
+        this._fade_in(extra_content, original_content);
     },
 
-    hide_extra_content: function(extra_content_node) {
-        var saved_header = this.get('picker_header');
+    hide_extra_content: function(extra_content_node, use_animation) {
+        var saved_header = this.get('saved_header');
         if (Y.Lang.isValue(saved_header)) {
             this.set('headerContent', saved_header);
-            this.set('picker_header', null);
-        }
-        this.get('contentBox').all('.steps').show();
+            this.set('saved_header', null);
+        }
+        var saved_steptitle = this.get('saved_steptitle');
+        if (Y.Lang.isValue(saved_steptitle)) {
+            this.set('steptitle', saved_steptitle);
+            this.set('saved_steptitle', null);
+        }
+        var saved_progress = this.get('saved_progress');
+        if (Y.Lang.isValue(saved_progress)) {
+            this.set('progress', saved_progress);
+            this.set('saved_progress', null);
+        }
         var content_node = this.get('contentBox').one('.yui3-widget-bd');
-        if (extra_content_node !== null) {
-            extra_content_node.get('parentNode')
-                .removeChild(extra_content_node);
-            content_node.addClass('transparent');
-            content_node.setStyle('opacity', 0);
-            content_node.show();
-            var duration = 0;
-            if (this.get('use_animation')) {
-                duration = 0.6;
-            }
-            var content_fade_in = new Y.Anim({
-                node: content_node,
-                to: {opacity: 1},
-                duration: duration
-            });
-            content_fade_in.run();
-        } else {
+        this._fade_in(content_node, extra_content_node, use_animation);
+    },
+
+    _fade_in: function(content_node, old_content, use_animation) {
+        content_node.removeClass('hidden');
+        if (old_content === null) {
             content_node.removeClass('transparent');
             content_node.setStyle('opacity', 1);
             content_node.show();
-        }
+            return;
+        }
+        old_content.addClass('hidden');
+        if (!Y.Lang.isValue(use_animation)) {
+            use_animation = this.get('use_animation');
+        }
+        if (!use_animation) {
+            old_content.setStyle('opacity', 1);
+            return;
+        }
+        content_node.addClass('transparent');
+        content_node.setStyle('opacity', 0);
+        var fade_in = new Y.Anim({
+            node: content_node,
+            to: {opacity: 1},
+            duration: 0.8
+        });
+        fade_in.run();
     },
 
     /*

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2012-06-21 03:48:05 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2012-06-26 00:18:26 +0000
@@ -247,14 +247,11 @@
 
     var node = Y.Node.create(
         ['<div class="validation-node">',
-          '<div class="step-on" style="width: 100%;"></div>',
-          '<div class="transparent important-notice-popup">',
             '<div class="validation-content-placeholder"></div>',
             '<div class="extra-form-buttons">',
               '<button class="yes_button" type="button"></button>',
               '<button class="no_button" type="button"></button>',
             '</div>',
-          '</div>',
         '</div>'].join(''));
 
     var button_callback = function(e, callback_fn) {
@@ -272,8 +269,7 @@
         .on('click', function(e) { button_callback(e, no_fn); });
 
     node.one(".validation-content-placeholder").replace(content);
-    picker.get('contentBox').one('.yui3-widget-bd').insert(node, 'before');
-    picker.show_extra_content(node.one(".important-notice-popup"));
+    picker.show_extra_content(node);
 };
 
 /*
@@ -315,6 +311,9 @@
 function reset_form(picker) {
     var validation_node = picker.get('contentBox').one('.validation-node');
     picker.hide_extra_content(validation_node);
+    if (Y.Lang.isValue(validation_node)) {
+        validation_node.remove(true);
+    }
 }
 
 
@@ -348,7 +347,6 @@
         validators, picker, picker_result, do_save, do_cancel) {
     if (validators.length === 0) {
         do_save();
-        reset_form(picker);
         return;
     }
     var validator_callback = validators.pop();
@@ -495,6 +493,7 @@
         Y.log('Got save event.');
         var picker_result = e.details[Y.lazr.picker.Picker.SAVE_RESULT];
         user_has_searched = false;
+        reset_form(picker);
         picker.hide();
         if (Y.Lang.isFunction(config.save)) {
             config.save(picker_result);

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-06-21 03:48:05 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-06-26 00:18:26 +0000
@@ -36,8 +36,6 @@
       <script type="text/javascript"
           src="../../../../../../build/js/lp/app/lazr/lazr.js"></script>
       <script type="text/javascript"
-          src="../../../../../../build/js/lp/app/mustache.js"></script>
-      <script type="text/javascript"
           src="../../../../../../build/js/lp/app/overlay/overlay.js"></script>
       <script type="text/javascript"
           src="../../../../../../build/js/lp/app/effects/effects.js"></script>
@@ -49,6 +47,8 @@
           src="../../../../../../build/js/lp/app/picker/picker.js"></script>
       <script type="text/javascript"
           src="../../../../../../build/js/lp/app/picker/picker_patcher.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/testing/mockio.js"></script>
 
 
       <!-- The module under test. -->
@@ -69,9 +69,26 @@
                 <span>
                   A picker widget test
                   <a id="edit-pickertest-btn"
-                    class="sprite edit lazr-btn yui3-activator-act"
-                    href="/fnord/+edit-people"
-                    >&nbsp;Edit</a>
+                    class="sprite edit action-icon yui3-activator-act"
+                    href="/fnord/+edit-people"
+                    >Edit</a>
+                </span>
+                <span class="yui3-activator-data-box">
+                </span>
+                <span class="yui3-activator-message-box yui3-activator-hidden"></span>
+              </span>
+            </span>
+        </div>
+
+        <div class="yui3-widget yui3-activator yui3-activator-focused">
+            <span id="anotherpicker_id" class="yui3-activator-content yui3-activator-success">
+              <span id="anotherpickertest">
+                <span>
+                  Another picker widget test
+                  <a id="edit-anotherpickertest-btn"
+                    class="sprite edit action-icon yui3-activator-act"
+                    href="/fnord/+edit-people"
+                    >Edit</a>
                 </span>
                 <span class="yui3-activator-data-box">
                 </span>

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-22 14:12:33 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-26 00:18:26 +0000
@@ -3,7 +3,7 @@
 
 YUI().use('test', 'console', 'plugin',
            'lazr.picker', 'lazr.person-picker', 'lp.app.picker',
-           'lp.app.mustache', 'node-event-simulate', function(Y) {
+           'lp.testing.mockio', 'node-event-simulate', function(Y) {
 
     var Assert = Y.Assert;
 
@@ -88,6 +88,10 @@
 
         tearDown: function() {
             cleanup_widget(this.picker);
+            var form_namespace = Y.namespace('lp.app.picker.teamform');
+            form_namespace.form_callbacks = null;
+            form_namespace.team_form = null;
+            delete this.mockio;
             delete window.LP;
         },
 
@@ -325,6 +329,32 @@
             Assert.isNull(Y.one('.yui-picker-new-team-button'));
         },
 
+        _simple_team_form: function() {
+            return '<table><tr><td></td>' +
+                '<input id="field.name">' +
+                '<input id="field.displayname"></td></tr></table>';
+        },
+
+        test_picker_new_team_xhr_calls: function() {
+            // Only one XHR call is made to fetch the team form even if more
+            // than one picker is used.
+            this.create_picker(this._picker_params(false, false, true));
+            var config = {
+                "io_provider": this.mockio,
+                "use_animation": false,
+                "picker_type": "person",
+                "show_create_team": true
+                };
+            var another = Y.lp.app.picker.addPickerPatcher(
+                    this.vocabulary,
+                    "foo/bar",
+                    "test_link",
+                    "anotherpicker_id",
+                    config);
+            Y.Assert.areEqual(1, this.mockio.requests.length);
+            another.destroy();
+        },
+
         test_picker_new_team_button_click_shows_form: function() {
             // Clicking the new team button displays the new team form.
             this.create_picker(this._picker_params(true, true, true));
@@ -336,7 +366,8 @@
                 'Enter new team details',
                 this.picker.get('headerContent').get('text'));
             Y.Assert.isNotNull(
-                this.picker.get('contentBox').one('[id=field.name]'));
+                this.picker.get('contentBox')
+                    .one('input[id=field.name]'));
             Y.Assert.areEqual('none',
                 this.picker.get('contentBox').one('.yui3-widget-bd')
                     .getStyle('display'));
@@ -360,8 +391,9 @@
             Y.Assert.areEqual(
                 'Pick Someone',
                 this.picker.get('headerContent').get('text'));
-            Y.Assert.isNull(
-                this.picker.get('contentBox').one('[id=field.name]'));
+            Y.Assert.isNotNull(
+                this.picker.get('contentBox').one('input[id=field.name]')
+                    .ancestor('div.hidden'));
             Y.Assert.isNotNull(
                 this.picker.get('contentBox').one('.yui3-picker-search'));
         },
@@ -385,8 +417,8 @@
             var new_team =
                 picker_content.one('.yui-picker-new-team-button');
             new_team.simulate('click');
-            var team_name = picker_content.one('[id=field.name]');
-            Y.Node.getDOMNode(team_name).value = 'fred';
+            var team_name = picker_content.one('input[id=field.name]');
+            team_name.set('value', 'fred');
             var form_buttons = picker_content.one('.extra-form-buttons');
             simulate(
                 form_buttons, 'button:nth-child(1)', 'click');
@@ -409,7 +441,9 @@
                 data_box.one('a').set('href', field_value);
             }
 
+            this.mockio = new Y.lp.testing.mockio.MockIo();
             var config = {
+                "io_provider": this.mockio,
                 "use_animation": false,
                 "picker_type": "person",
                 "step_title": "Choose someone",
@@ -431,6 +465,14 @@
                     "test_link",
                     "picker_id",
                     config);
+            if (params.show_create_team) {
+                Y.Assert.areEqual(
+                    'file:////people/+newteam/++form++',
+                    this.mockio.last_request.url);
+                this.mockio.success({
+                    responseText: this._simple_team_form(),
+                    responseHeaders: {'Content-Type': 'text/html'}});
+            }
         }
     };
 
@@ -444,6 +486,9 @@
 
         tearDown: function() {
             cleanup_widget(this.picker);
+            var form_namespace = Y.namespace('lp.app.picker.teamform');
+            form_namespace.form_callbacks = null;
+            form_namespace.team_form = null;
             this.search_input.remove();
             delete window.LP;
         },
@@ -458,7 +503,10 @@
             if (field_value !== undefined) {
                 text_field.set('text', field_value);
             }
+            this.mockio = new Y.lp.testing.mockio.MockIo();
             var config = {
+                "io_provider": this.mockio,
+                "use_animation": false,
                 "picker_type": "person",
                 "header": "Pick Someone",
                 "associated_field_id": associated_field_id,
@@ -473,6 +521,14 @@
                 };
             this.picker = Y.lp.app.picker.create(
                                 this.vocabulary, config, associated_field_id);
+            if (params.show_create_team) {
+                Y.Assert.areEqual(
+                    'file:////people/+newteam/++form++',
+                    this.mockio.last_request.url);
+                this.mockio.success({
+                    responseText: this._simple_team_form(),
+                    responseHeaders: {'Content-Type': 'text/html'}});
+            }
         }
     };
 

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.js'
--- lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2012-04-06 17:28:25 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2012-06-26 00:18:26 +0000
@@ -71,6 +71,7 @@
 
         create_picker: function(validate_callback, extra_config) {
             var config = {
+                    "use_animation": false,
                     "step_title": "Choose someone",
                     "header": "Pick Someone",
                     "null_display_value": "No one",
@@ -96,7 +97,7 @@
         create_picker_direct: function(associated_field) {
             this.picker = Y.lp.app.picker.create(
                 this.vocabulary,
-                undefined,
+                {use_animation: false},
                 associated_field);
             var self = this;
             this.picker.subscribe('save', function(e) {
@@ -118,7 +119,8 @@
         yesno_validate_callback: function(expected_value) {
             return function(picker, value, save_fn, cancel_fn) {
                 Assert.areEqual(
-                        expected_value, value.api_uri, "unexpected picker value");
+                    expected_value,
+                    value.api_uri, "unexpected picker value");
                 if (value === null) {
                     return true;
                 }
@@ -189,9 +191,10 @@
             Assert.areEqual('message', footer_slot.get('text'));
         },
 
-        test_footer_node_preserved_without_extra_no_results_message: function () {
-            // If "extra_no_results_message" is not defined, the footer slot node
-            // should be preserved.
+        test_footer_node_preserved_without_extra_no_results_message:
+            function () {
+            // If "extra_no_results_message" is not defined, the footer slot
+            // node should be preserved.
             this.create_picker();
             var footer_node = Y.Node.create("<span>foobar</span>");
             this.picker.set('footer_slot', footer_node);
@@ -211,7 +214,8 @@
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
                     'li:nth-child(2)', 'click');
-            var yesno = this.picker.get('contentBox').one('.extra-form-buttons');
+            var yesno = this.picker.get('contentBox')
+                .one('.extra-form-buttons');
 
             simulate(
                     yesno, 'button:nth-child(1)', 'click');
@@ -232,7 +236,8 @@
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
                     'li:nth-child(2)', 'click');
-            var yesno = this.picker.get('contentBox').one('.extra-form-buttons');
+            var yesno = this.picker.get('contentBox')
+                .one('.extra-form-buttons');
 
             simulate(
                     yesno, 'button:nth-child(1)', 'click');
@@ -249,7 +254,8 @@
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
                     'li:nth-child(2)', 'click');
-            var yesno = this.picker.get('contentBox').one('.extra-form-buttons');
+            var yesno = this.picker.get('contentBox')
+                .one('.extra-form-buttons');
             simulate(
                 yesno, 'button:nth-child(2)', 'click');
             Assert.isNull(this.saved_picker_value);
@@ -260,7 +266,8 @@
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
                     'li:nth-child(2)', 'click');
-            var yesno = this.picker.get('contentBox').one('.extra-form-buttons');
+            var yesno = this.picker.get('contentBox')
+                .one('.extra-form-buttons');
             // Click the Yes button.
             simulate(yesno, 'button:nth-child(1)', 'click');
             yesno = this.picker.get('contentBox').one('.extra-form-buttons');
@@ -329,7 +336,8 @@
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
                     'li:nth-child(2)', 'click');
-            var yesno = this.picker.get('contentBox').one('.extra-form-buttons');
+            var yesno = this.picker.get('contentBox')
+                .one('.extra-form-buttons');
             // Click the Yes button.
             simulate(yesno, 'button:nth-child(1)', 'click');
             yesno = this.picker.get('contentBox').one('.extra-form-buttons');
@@ -339,8 +347,8 @@
         },
 
         test_connect_select_menu: function() {
-            // connect_select_menu() connects the select menu's onchange event to
-            // copy the selected value to the text input field.
+            // connect_select_menu() connects the select menu's onchange event
+            // to copy the selected value to the text input field.
             this.text_input = Y.Node.create(
                     '<input id="field.testfield" value="foo" />');
             var node = Y.one(document.body).appendChild(this.text_input);
@@ -390,7 +398,8 @@
                     'li:nth-child(2)', 'click');
             // expected_text is a little weird since breaks between p tags and
             // buttons are lost.
-            var expected_text = 'This action will reveal this team\'s name to ' +
+            var expected_text =
+                'This action will reveal this team\'s name to ' +
                 'the public.ContinueChoose Again';
             var text = Y.one(".validation-node").get('text');
             Assert.areEqual(expected_text, text);
@@ -473,8 +482,9 @@
         },
 
         test_picker_displays_warning_by_default: function() {
-            // If show_search_box is not supplied in config, it defaults to true.
-            // Thus the picker will refuse to display more than 120 values.
+            // If show_search_box is not supplied in config, it defaults to
+            // true. Thus the picker will refuse to display more than 120
+            // values.
             this.create_picker();
             this.picker.set('min_search_chars', 0);
             this.picker.fire('search', '');
@@ -536,9 +546,9 @@
             this.mock_io.failure(
                 {responseHeaders: this.get_oops_headers('OOPS')});
             Assert.areEqual(
-                "Sorry, something went wrong with your search. We've recorded " +
-                "what happened, and we'll fix it as soon as possible. " +
-                "(Error ID: OOPS)",
+                "Sorry, something went wrong with your search. " +
+                "We've recorded what happened, and we'll fix it as soon " +
+                "as possible. (Error ID: OOPS)",
                 this.picker.get('error'));
         },
 
@@ -615,9 +625,10 @@
         },
 
         test_automated_search_error_ignored_if_user_has_searched: function() {
-            // If an automated search (like loading branch suggestions) returns an
-            // error and the user has submitted a search, then the error from the
-            // automated search is ignored so as not to confuse the user.
+            // If an automated search (like loading branch suggestions) returns
+            // an error and the user has submitted a search, then the error
+            // from the automated search is ignored so as not to confuse the
+            // user.
             var mock_io = new Y.lp.testing.mockio.MockIo();
             var picker = this.create_picker(mock_io);
             picker.fire('search', 'test');
@@ -629,7 +640,7 @@
 
     }));
 
-}, '0.1', {'requires': ['test', 'console', 'lp.pickert', 'node', 'lp', 'lp.client',
+}, '0.1', {'requires': ['test', 'console', 'lp.app.picker', 'node',
         'event-focus', 'event-simulate', 'lazr.picker', 'lazr.person-picker',
         'lp.app.picker', 'node-event-simulate', 'escape', 'event',
-        'lp.testing.mockio',]});
+        'lp.testing.mockio', 'lp', 'lp.client']});

=== modified file 'lib/lp/app/javascript/subscribers/subscribers_list.js'
--- lib/lp/app/javascript/subscribers/subscribers_list.js	2012-06-07 19:43:37 +0000
+++ lib/lp/app/javascript/subscribers/subscribers_list.js	2012-06-26 00:18:26 +0000
@@ -255,7 +255,7 @@
     var loader = this;
 
     // Fetch the person and add a subscription.
-    function on_success(subscribers) {
+    var on_success = function(subscribers) {
         loader._loadSubscribersFromList(subscribers);
         // We may need to set up the subscribe me link.
         // This has to be done after subscribers have been loaded so that we
@@ -264,7 +264,7 @@
             loader._setupSubscribeMe();
         }
         loader.subscribers_list.stopActivity();
-    }
+    };
 
     var config = { on: {
         success: on_success,
@@ -484,7 +484,7 @@
     }
     var loader = this;
 
-    function on_success() {
+    var on_success = function() {
         var unsubscribe_callback = loader._getUnsubscribeCallback();
         if (update_subscribers_list) {
             loader.subscribers_list.stopSubscriberActivity(subscriber, true);
@@ -502,8 +502,8 @@
             };
             loader._updateSubscribeMeLink(true);
         }
-    }
-    function on_failure(t_id, response) {
+    };
+    var on_failure = function(t_id, response) {
         if (update_subscribers_list) {
             loader.subscribers_list.stopSubscriberActivity(
                 subscriber, false, function() {
@@ -524,7 +524,7 @@
             false,
             error_msg
         );
-    }
+    };
     var config = {
         on: { success: on_success,
               failure: on_failure },

=== modified file 'lib/lp/bugs/javascript/filebug_dupefinder.js'
--- lib/lp/bugs/javascript/filebug_dupefinder.js	2012-05-25 14:38:42 +0000
+++ lib/lp/bugs/javascript/filebug_dupefinder.js	2012-06-26 00:18:26 +0000
@@ -177,7 +177,7 @@
         search_button.removeClass(UNSEEN);
     }
 
-    function on_success(transaction_id, response, args) {
+    var on_success = function(transaction_id, response, args) {
         // Hide the spinner and show the duplicates.
         Y.one('#spinner').addClass(UNSEEN);
 
@@ -216,7 +216,7 @@
         // Finally, change the label on the search button and show it again.
         search_button.set('value', 'Check again');
         search_button.removeClass(UNSEEN);
-    }
+    };
 
     var search_term = encodeURI(search_field.get('value'));
     var search_url = search_url_base + '?title=' + search_term;

=== modified file 'lib/lp/registry/javascript/sharing/shareepicker.js'
--- lib/lp/registry/javascript/sharing/shareepicker.js	2012-04-26 21:01:19 +0000
+++ lib/lp/registry/javascript/sharing/shareepicker.js	2012-06-26 00:18:26 +0000
@@ -29,10 +29,6 @@
     },
     sharing_permissions: {
         value: []
-    },
-    // Override for testing
-    anim_duration: {
-        value: 1
     }
 };
 
@@ -77,26 +73,6 @@
         });
     },
 
-    _fade_in: function(content_node, old_content) {
-        content_node.removeClass('unseen');
-        if (old_content === null) {
-            return;
-        }
-        old_content.addClass('unseen');
-        var anim_duration = this.get('anim_duration');
-        if (anim_duration === 0) {
-            return;
-        }
-        content_node.addClass('transparent');
-        content_node.setStyle('opacity', 0);
-        var fade_in = new Y.Anim({
-            node: content_node,
-            to: {opacity: 1},
-            duration: anim_duration
-        });
-        fade_in.run();
-    },
-
     _display_step_one: function() {
         this.set('headerContent', this.step_one_header);
         this.set(

=== modified file 'lib/lp/registry/javascript/sharing/tests/test_shareepicker.js'
--- lib/lp/registry/javascript/sharing/tests/test_shareepicker.js	2012-04-26 21:01:19 +0000
+++ lib/lp/registry/javascript/sharing/tests/test_shareepicker.js	2012-06-26 00:18:26 +0000
@@ -54,7 +54,7 @@
 
         _create_picker: function(overrides) {
             var config = {
-                anim_duration: 0,
+                use_animation: false,
                 progressbar: true,
                 progress: 50,
                 headerContent: "<h2>Share with a user or team</h2>",
@@ -111,7 +111,7 @@
             // The progress should be 75%
             Y.Assert.areEqual(75, this.picker.get('progress'));
             // The first step ui should be hidden.
-            Y.Assert.isTrue(cb.one('.yui3-widget-bd').hasClass('unseen'));
+            Y.Assert.isTrue(cb.one('.yui3-widget-bd').hasClass('hidden'));
             // The step title should be updated according to the selected
             // person. The title should remain unchanged.
             Y.Assert.areEqual(
@@ -122,7 +122,7 @@
                 'Select sharing policies for Fred', steptitle);
             // The second step ui should be visible.
             var step_two_content = cb.one('.picker-content-two');
-            Y.Assert.isFalse(step_two_content.hasClass('unseen'));
+            Y.Assert.isFalse(step_two_content.hasClass('hidden'));
             // The second step ui should contain input buttons for each access
             // policy type for each sharing permission.
             Y.Array.each(this.information_types, function(info_type) {
@@ -281,7 +281,7 @@
             // The progress should be 50%
             Y.Assert.areEqual(50, this.picker.get('progress'));
             // The first step ui should be visible.
-            Y.Assert.isFalse(cb.one('.yui3-widget-bd').hasClass('unseen'));
+            Y.Assert.isFalse(cb.one('.yui3-widget-bd').hasClass('hidden'));
             // The title and step title should be updated.
             Y.Assert.areEqual(
                 'Share with a user or team',
@@ -291,7 +291,7 @@
                 'Search for user or exclusive team with whom to share',
                 steptitle);
             // The second step ui should be hidden.
-            Y.Assert.isTrue(step_two_content.hasClass('unseen'));
+            Y.Assert.isTrue(step_two_content.hasClass('hidden'));
         },
 
         // Test that a selection made in step two is correctly passed to the