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

[Ian Booth]

Add simple team creation form and support for saving new team to enhanced person picker.

== Implementation ==

This branch makes the new team form accessible via selected person pickers functional. It renders a smaller team creation form, showing just name, displayname, visibility, subscription policy. When the details are entered and Create Team is clicked, the new team is created, the picker is closed, and the field's value is update just the same as is an existing person or team were chosen.

If there are any errors saving, eg team already exists, the form stays open and the errors are displayed as per other launchpad forms. This is done using the recently added ajax form validation infrastructure.

Part of the implementation involved ripping code out of the person_picker widget and putting it inside a new CreateTeamForm widget. This accounts for the large chunks of red in the diff.

Next step - enhance the form so that the drop down for subscription policy is implemented using a choice source popup.

== Tests ==

Add yui tests for the new team.js
Add test for new SimpleTeamAddView
Add person_picker yui tests

== 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/team.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_team.html
  lib/lp/app/javascript/picker/tests/test_team.js
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/team.py
  lib/lp/registry/browser/tests/test_team.py

[Richard Harding (rharding)]

Ian, thanks for the work. In light of the recent discussions of our JS being a bit complicated and difficult to reuse, I've gotten very picky in this review. I apologize up front, but I hope that by simplifying and working on standardizing some we can make life a bit easier and point out more reusable bits as we come across code with names that match up.

The overall summary is that, while you're creating a team form specific class, that naming convention doesn't need to carry over through all of the code inside that class. It's simpler to drop it. It then starts to point out common method conventions, like render, errorHandler, etc.

Another is that there's a lot of use of Y.bind() when it's not necessary. It makes the code a bit less readable as it's carrying a lot of params for Y.bind() to function. I've written out one of the methods below to show how it could be done without any of the Y.bind() calls.

#221
    why not just stick the TEAM_CREATED and CANCEL_TEAM directly on the ns back after line 214?

#232
    In YUI there are some usual naming conventions. There's things like contentBox, etc. In 3.5 there's a standard 'container' used for things like this new_team_form. Could this be updated to be 'container' in this object?

    In a similar way can the _new_team_template just be a generic _get_template? You're in an object called CreateTeamForm so the new team bit is known and understood.

#233
    Along the same lines, we don't really need to prefix things as team_form_error_handler, but merely error_handler or maybe form_error_handler if required.

#237
    Since you've created the namespace for lp.app.picker.team and this is a form in that namespace, I'd suggest not creating a new namespace, but working off the current one. ns.form = {};
    You'd not need to store the namespace on the object instance since it's not really an instance variable. It's a namespace and global to all YUI code in this block.
    Then the #238 would just be callbacks = ns.form.callbacks;

#283
    I'd prefer for the event to just get a callback and that the event handler didn't do work for the widget. I'd move the stuff in on_success to CreateTeamForm._handle_success/_handle_failure and have the logic enclosed in them. Here it's difficult to find and test these handlers because they're defined within the _load_new_team_form method and doing too much.

    The on_success method might look something more like:
        success: function (id, response) {
            Y.Array.each(ns.form.callbacks, function (cb) {
                 cb._handle_success.call(cb, responseText);
            }
        }

    You'd not need to pass the callback method in #247, and the new _handle_success method would be able to bind events/etc. You'd only be pushing instance onto the callback stack.

        callbacks.push(this);

#314
    /render_new_team_form/render

#352
s/SubmitSpinner/Spinner

#402
    Setting a node = to form_data.id isn't clicking for me. Should it be the id of that node or is it meant to be the node itself?

#407
    None of those calls in your callback require the scope of the local instance. You can just call the methods. You can use the typical JS trick...

review: Needs Fixing

[Curtis Hovey (sinzui)]

I suspect lines 512 and 680 will fail in IE 8 because it does not support nth-child(). We fixed a few cases of this recently.

There are two choices:
1. use('selector-css3') to add support for IE8
2. replace :nth-child(1) with :first-child which I think is easier to read.

review: Needs Fixing (code)

[Richard Harding (rharding)]

> I suspect lines 512 and 680 will fail in IE 8 because it does not support nth-
> child(). We fixed a few cases of this recently.
>
> There are two choices:
> 1. use('selector-css3') to add support for IE8
> 2. replace :nth-child(1) with :first-child which I think is easier to read.

Or perhaps it could just grab the button based on the css class. It looks like it has a class you can search instead.

[Ian Booth (wallyworld)]

Hi Rick

Thanks for the very thorough review. I do appreciate it.

>
> The overall summary is that, while you're creating a team form specific class, that naming convention doesn't need to carry over through all of the code inside that class. It's simpler to drop it. It then starts to point out common method conventions, like render, errorHandler, etc.
>

[1] I lot of the naming issues (well almost all) are because I
originally had this code inside the picker widget and moved it out into
a new, separate widget. When the code lived in it's original home, the
variable and method names made more sense.

> Another is that there's a lot of use of Y.bind() when it's not necessary. It makes the code a bit less readable as it's carrying a lot of params for Y.bind() to function. I've written out one of the methods below to show how it could be done without any of the Y.bind() calls.
>
> #221
> why not just stick the TEAM_CREATED and CANCEL_TEAM directly on the ns back after line 214?
>

I was following existing convention used in most of the lazr-js
codebase. I am guessing, but believe the reason for doing it this wy is
so that the event names can be used internally without having to
dereference off the namespace. I can see the reasoning for doing it this
way.

> #232
> In YUI there are some usual naming conventions. There's things like contentBox, etc. In 3.5 there's a standard 'container' used for things like this new_team_form. Could this be updated to be 'container' in this object?
>
> In a similar way can the _new_team_template just be a generic _get_template? You're in an object called CreateTeamForm so the new team bit is known and understood.
>

Yes, will fix. See [1].

> #233
> Along the same lines, we don't really need to prefix things as team_form_error_handler, but merely error_handler or maybe form_error_handler if required.
>

Yes, will fix. See [1].

> #237
> Since you've created the namespace for lp.app.picker.team and this is a form in that namespace, I'd suggest not creating a new namespace, but working off the current one. ns.form = {};
> You'd not need to store the namespace on the object instance since it's not really an instance variable. It's a namespace and global to all YUI code in this block.
> Then the #238 would just be callbacks = ns.form.callbacks;
>

Yes, will fix. See [1].

> #283
> I'd prefer for the event to just get a callback and that the event handler didn't do work for the widget. I'd move the stuff in on_success to CreateTeamForm._handle_success/_handle_failure and have the logic enclosed in them. Here it's difficult to find and test these handlers because they're defined within the _load_new_team_form method and doing too much.
>
> The on_success method might look something more like:
> success: function (id, response) {
> Y.Array.each(ns.form.callbacks, function (cb) {
> cb._handle_success.call(cb, responseText);
> }
> }
>
> You'd not need to pass the callback method in #247, and the new _handle_success method would be able to bind events/etc. You'd only be pushing instance onto the callback stack.
>
> ...

[Ian Booth (wallyworld)]

On 26/06/12 23:55, Curtis Hovey wrote:
> Review: Needs Fixing code
>
> I suspect lines 512 and 680 will fail in IE 8 because it does not support nth-child(). We fixed a few cases of this recently.
>
> There are two choices:
> 1. use('selector-css3') to add support for IE8
> 2. replace :nth-child(1) with :first-child which I think is easier to read.
>

Ah, right you are. And nth-child is used in the picker validation stuff
too from memory. I'll fix that also.

[Richard Harding (rharding)]

> > #283
> > I'd prefer for the event to just get a callback and that the event
> handler didn't do work for the widget. I'd move the stuff in on_success to
> CreateTeamForm._handle_success/_handle_failure and have the logic enclosed in
> them. Here it's difficult to find and test these handlers because they're
> defined within the _load_new_team_form method and doing too much.
> >
> > The on_success method might look something more like:
> > success: function (id, response) {
> > Y.Array.each(ns.form.callbacks, function (cb) {
> > cb._handle_success.call(cb, responseText);
> > }
> > }
> >
> > You'd not need to pass the callback method in #247, and the new
> _handle_success method would be able to bind events/etc. You'd only be pushing
> instance onto the callback stack.
> >
> > callbacks.push(this);
> >
>
>
> In general I do try and keep event handlers lean and mean and move out
> the work to methods for testability. I didn't think the callbacks were
> too complicated in this case. Nevertheless, I'll fix them up.

I think the sign here was that you were sending the callback stack both the object and a method on that object to be used by the callback bits. Only one should do and you can treat it more like a list, etc.

> > #407
> > None of those calls in your callback require the scope of the local
> instance. You can just call the methods. You can use the typical JS trick of
> storing that = this; at the top of the method to aid in getting access to the
> methods themselves.
> >
>
> Till this branch I have been doing the that = this thing in my work but
> thought that the approach was a bit kludgy since my reading of the
> Y.bind docs seemed to indicate that using bind and passing in 'this'
> seemed more kosher. If you tell me it is definitely the preferred
> approach to use that = this, I'll revert to that pattern. When I've been
> doing it, I use self = this as that naming seems more logical to me.

The that = this is just standard JS. I think it goes back to JS the Good Parts book, but I'd have to go look it up. Y almost never use Y.bind and it's purely selfish. I find the that=this format easier to read, less to type, etc.

Really the one time I do use Y.bind is when the scope isn't this, but some other arbitrary object. It could be anything in there and sometimes happens I want to do some event magic with the scope of some other object I know about (combining Views and such).

> > So I'd write the _save_new_team method something like this:
> https://pastebin.canonical.com/68867/
> >
> > #606
> > Why do you wrap simulate and fire the event that way vs getting the node
> and calling node.simulate()? That's what the module node-event-simulate
> allows.
> >
>
> Most/all of our existing codebase does it that way. Perhaps it was due
> to earlier limitations in YUI. Not sure.

It might be. Perhaps it was before the node-event-simulate module.

[Ian Booth (wallyworld)]

I've made the requested changes, so this should be good for another look.

[Richard Harding (rharding)]

Thanks for the update Ian. This looks and reads much smoother. A few small items below, but the code looks good. Approving per these.

#215
There's no need for the NAME. It's never used. Just put that string into the Base.create call. The two events don't need to be defined before hand. Just do ns.TEAM_CREATED and adjust the calls in the code to use ns.

#237
There's no need for the intermediate widgets reference. It can be condensed to just:
    var perform_load = false;
    if (!Y.Lang.isArray(ns.widgets)) {
        perform_load = true;
        ns.widgets = [];
    }
    ns.widgets.push(this);
    this._load_form(perform_load);

#271
Same as above, just reference ns.team_form.

#356
It's JS convention to use the Y.Lang.isNull() check here.
https://dev.launchpad.net/JavaScriptReviewNotes?highlight=%28Y.Lang%29

#381
There's an extra indentation level.

#393
The that = this; should be hoisted to the top of the method and then used in the #378 method:

this.error_handler.showError = function (error_msg) {
    that._hideSpinner(submit_link);
    that.error_handler.handleFormValidationError(error_msg, [], []);

}

#659/660
The inputs should be self closing tags with />

review: Approve

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2012-06-26 17:24:46 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2012-07-02 04:32:06 +0000
@@ -20,48 +20,31 @@
         }
         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);
+            // Set up the widget to provide the form.
+            var ctns = Y.lp.app.picker.team;
+            this.new_team_widget = new ctns.CreateTeamForm({
+                io_provider: cfg.io_provider
+            });
+            this.new_team_widget.subscribe(
+                ctns.CANCEL_TEAM, function(e) {
+                    this.hide_new_team_form();
+                }, this);
+            this.new_team_widget.subscribe(
+                ctns.TEAM_CREATED, function(e) {
+                    this.fire('save', e.details[0]);
+                }, this);
         }
     },
 
-    _new_team_template: function() {
-        return [
-          '<div class="new-team-node">',
-          '<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('');
-    },
-
     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);
+        if (Y.Lang.isValue(this.new_team_widget)) {
+            this.new_team_widget.hide();
+            this.hide_extra_content(
+                this.new_team_widget.get('container'), false);
         }
+
         Y.lazr.picker.Picker.prototype.hide.call(this);
     },
 
@@ -116,97 +99,18 @@
         });
     },
 
-    _cancel_new_team: function() {
-        var node = this.get('contentBox').one('.new-team-node');
-        this.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"};
-        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) ) {
-                Y.bind(callback_fn, this)();
-            }
-        };
-        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');
-    },
-
     show_new_team_form: function() {
-        this.show_extra_content(
-            this.team_form_node, "Enter new team details");
+        var form = this.new_team_widget.get('container');
+        this.show_extra_content(form, "Enter new team details");
+        this.new_team_widget.show();
+        this.set('centered', true);
+    },
+
+    hide_new_team_form: function() {
+        this.new_team_widget.hide();
+        var form = this.new_team_widget.get('container');
+        this.hide_extra_content(form);
+        this.set('centered', true);
     },
 
     _assign_me_button_html: function() {
@@ -277,18 +181,8 @@
         remove_person_text: {value: 'Remove person'},
         remove_team_text: {value: 'Remove team'},
         min_search_chars: {value: 2},
-        show_create_team: {value: false},
-        new_team_template: {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}
+        show_create_team: {value: false}
     }
 });
 }, "0.1", {"requires": [
-    "base", "node", "lazr.picker"]});
+    "base", "node", "lazr.picker", "lp.app.picker.team"]});

=== added file 'lib/lp/app/javascript/picker/team.js'
--- lib/lp/app/javascript/picker/team.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/picker/team.js	2012-07-02 04:32:06 +0000
@@ -0,0 +1,218 @@
+/* Copyright 2012 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * @namespace Y.app.picker.team
+ * @requires lazr.picker, lazr.person-picker
+ */
+YUI.add('lp.app.picker.team', function(Y) {
+
+var ns = Y.namespace('lp.app.picker.team');
+ns.TEAM_CREATED = 'teamCreated';
+ns.CANCEL_TEAM = 'cancelTeam';
+
+ns.CreateTeamForm = Y.Base.create("createTeamWidget", Y.Base, [], {
+    initializer: function(cfg) {
+        this.publish(ns.TEAM_CREATED);
+        this.publish(ns.CANCEL_TEAM);
+        // We need to provide the 'New team' link functionality.
+        // 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.get('container').appendChild(this._get_template());
+        this.error_handler =
+            new Y.lp.client.FormErrorHandler({
+                form: this.get('container')
+            });
+        var perform_load = false;
+        if (!Y.Lang.isArray(ns.widgets)) {
+            perform_load = true;
+            ns.widgets = [];
+        }
+        ns.widgets.push(this);
+        this._load_form(perform_load);
+    },
+
+    _get_template: function() {
+        return [
+          '<div id=form-placeholder ',
+              'class="yui3-overlay-indicator-content">',
+              '<img src="/@@/spinner-big/">',
+          '</div>',
+          '<div class="extra-form-buttons hidden">',
+              '<button class="yes_button" type="submit" ',
+              'name="field.actions.create" value="Create Team">',
+              'Create Team</button>',
+              '<button class="no_button" type="button">Cancel</button>',
+            '</div>',
+          '</div>'].join('');
+    },
+
+    _load_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.
+        if (Y.Lang.isValue(ns.team_form)) {
+            this.render(ns.team_form, true);
+            return;
+        }
+        if (!perform_load) {
+            return;
+        }
+
+        var on_success = function(id, response) {
+            ns.team_form = response.responseText;
+            Y.Array.each(ns.widgets,
+                function(widget) {
+                    widget.render(ns.team_form, true);
+                });
+        };
+        var on_failure = function(id, response) {
+            Y.Array.each(ns.widgets,
+                function(widget) {
+                    widget.render(
+                        'Sorry, an error occurred while loading the form.',
+                        false);
+            }   );
+        };
+        var cfg = {
+            on: {success: on_success, failure: on_failure}
+            };
+        var uri = Y.lp.client.get_absolute_uri(
+            'people/+simplenewteam/++form++');
+        uri = uri.replace('api/devel', '');
+        this.get("io_provider").io(uri, cfg);
+    },
+
+    render: function(form_html, show_submit) {
+        // Poke the actual team form into the DOM and wire up the save and
+        // cancel buttons.
+        var container = this.get('container');
+        container.one('#form-placeholder').replace(form_html);
+        var submit_button = container.one(".yes_button");
+        if (show_submit) {
+            container.on('submit', function(e) {
+                    e.halt();
+                    this._save_new_team();
+                }, this);
+        } else {
+            submit_button.addClass('hidden');
+        }
+        container.one(".no_button")
+            .on('click', function(e) {
+                e.halt();
+                this.fire(ns.CANCEL_TEAM);
+            }, this);
+        container.one('.extra-form-buttons').removeClass('hidden');
+    },
+
+    show: function() {
+        var form_elements = this.get('container').get('elements');
+        if (form_elements.size() > 0) {
+            form_elements.item(0).focus();
+        }
+    },
+
+    hide: function() {
+        this.error_handler.clearFormErrors();
+    },
+
+    /**
+     * Show the submit spinner.
+     *
+     * @method _showSpinner
+     */
+    _showSpinner: function(submit_link) {
+        var spinner_node = Y.Node.create(
+        '<img class="spinner" src="/@@/spinner" alt="Creating..." />');
+        submit_link.insert(spinner_node, 'after');
+    },
+
+    /**
+     * Hide the submit spinner.
+     *
+     * @method _hideSpinner
+     */
+    _hideSpinner: function(submit_link) {
+        var spinner = submit_link.get('parentNode').one('.spinner');
+        if (!Y.Lang.isNull(spinner)) {
+            spinner.remove(true);
+        }
+    },
+
+    _save_team_success: function(response, team_data) {
+        var value = {
+            "api_uri": "/~" + team_data['field.name'],
+            "title": team_data['field.displayname'],
+            "value": team_data['field.name'],
+            "metadata": "team"};
+        this.fire(ns.TEAM_CREATED, value);
+        var container = this.get('container');
+        container.all('button').detachAll();
+        container.all('.spinner').remove(true);
+        container.empty();
+        container.appendChild(this._get_template());
+        this.render(ns.team_form, true);
+    },
+
+    _save_new_team: function() {
+        var that = this;
+        var submit_link = Y.one("[name='field.actions.create']");
+        this.error_handler.showError = function (error_msg) {
+            that._hideSpinner(submit_link);
+            that.error_handler.handleFormValidationError(error_msg, [], []);
+            };
+        var uri = Y.lp.client.get_absolute_uri('people/+simplenewteam');
+        uri = uri.replace('api/devel', '');
+        var form_data = {};
+        var container = this.get('container');
+        container.all("[name^='field.']").each(function(field) {
+            form_data[field.get('name')] = field.get('value');
+        });
+        form_data.id = container;
+        var y_config = {
+            method: "POST",
+            headers: {'Accept': 'application/json;'},
+            on: {
+                start: function() {
+                    that.error_handler.clearFormErrors();
+                    that._showSpinner(submit_link);
+                },
+                end: function () {
+                    that._hideSpinner(submit_link);
+                },
+                failure: this.error_handler.getFailureHandler(),
+                success: function (id, response, team_data) {
+                    that._save_team_success(response, team_data);
+                }
+            },
+            'arguments': form_data
+        };
+        y_config.form = form_data;
+        this.get("io_provider").io(uri, y_config);
+    }
+}, {
+    ATTRS: {
+        /**
+         * The form used to enter the new team details.
+         */
+        container: {
+            valueFn: function() {return Y.Node.create('<form/>');}
+        },
+        /**
+        * 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"]});
+

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-06-26 00:15:11 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-07-02 04:32:06 +0000
@@ -48,6 +48,8 @@
       <script type="text/javascript"
           src="../../../../../../build/js/lp/app/picker/picker_patcher.js"></script>
       <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/picker/team.js"></script>
+      <script type="text/javascript"
           src="../../../../../../build/js/lp/app/testing/mockio.js"></script>
 
 

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-26 17:24:46 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-07-02 04:32:06 +0000
@@ -2,9 +2,9 @@
  * GNU Affero General Public License version 3 (see the file LICENSE).
  */
 
-YUI().use('test', 'console', 'plugin',
+YUI().use('test', 'console', 'plugin', 'node-event-simulate',
            'lazr.picker', 'lazr.person-picker', 'lp.app.picker',
-           'lp.testing.mockio', 'node-event-simulate', function(Y) {
+           'lp.app.picker.team', 'lp.testing.mockio', function(Y) {
 
     var Assert = Y.Assert;
 
@@ -89,9 +89,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;
+            var ns = Y.namespace('lp.app.picker.team');
+            ns.widgets = null;
+            ns.team_form = null;
             delete this.mockio;
             delete window.LP;
         },
@@ -266,7 +266,7 @@
             this.picker.render();
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
-                    'li:nth-child(1)', 'click');
+                    'li:first-child', 'click');
             this._check_assign_me_button_state(false);
         },
 
@@ -318,7 +318,7 @@
             this.picker.render();
             simulate(
                 this.picker.get('boundingBox').one('.yui3-picker-results'),
-                    'li:nth-child(2)', 'click');
+                    'li + li', 'click');
             Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
         },
 
@@ -331,9 +331,10 @@
         },
 
         _simple_team_form: function() {
-            return '<table><tr><td></td>' +
-                '<input id="field.name">' +
-                '<input id="field.displayname"></td></tr></table>';
+            return '<table><tr><td>' +
+                '<input id="field.name" name="field.name">' +
+                '<input id="field.displayname" ' +
+                'name="field.displayname"></td></tr></table>';
         },
 
         test_picker_new_team_xhr_calls: function() {
@@ -387,13 +388,13 @@
             var form_buttons = this.picker.get('contentBox')
                 .one('.extra-form-buttons');
             simulate(
-                form_buttons, 'button:nth-child(2)', 'click');
+                form_buttons, 'button.no_button', 'click');
             Y.Assert.areEqual(
                 'Pick Someone',
                 this.picker.get('headerContent').get('text'));
             Y.Assert.isNotNull(
                 this.picker.get('contentBox').one('input[id="field.name"]')
-                    .ancestor('div.hidden'));
+                    .ancestor('form.hidden'));
             Y.Assert.isNotNull(
                 this.picker.get('contentBox').one('.yui3-picker-search'));
         },
@@ -421,7 +422,10 @@
             team_name.set('value', 'fred');
             var form_buttons = picker_content.one('.extra-form-buttons');
             simulate(
-                form_buttons, 'button:nth-child(1)', 'click');
+                form_buttons, 'button.yes_button', 'click');
+            this.mockio.success({
+                responseText: '',
+                responseHeaders: {'Content-Type': 'application/jaon'}});
             Y.Assert.isTrue(result_published);
         }
     };
@@ -467,7 +471,7 @@
                     config);
             if (params.show_create_team) {
                 Y.Assert.areEqual(
-                    'file:////people/+newteam/++form++',
+                    'file:////people/+simplenewteam/++form++',
                     this.mockio.last_request.url);
                 this.mockio.success({
                     responseText: this._simple_team_form(),
@@ -486,9 +490,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;
+            var ns = Y.namespace('lp.app.picker.team');
+            ns.widgets = null;
+            ns.team_form = null;
             this.search_input.remove();
             delete window.LP;
         },
@@ -523,7 +527,7 @@
                                 this.vocabulary, config, associated_field_id);
             if (params.show_create_team) {
                 Y.Assert.areEqual(
-                    'file:////people/+newteam/++form++',
+                    'file:////people/+simplenewteam/++form++',
                     this.mockio.last_request.url);
                 this.mockio.success({
                     responseText: this._simple_team_form(),

=== added file 'lib/lp/app/javascript/picker/tests/test_team.html'
--- lib/lp/app/javascript/picker/tests/test_team.html	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/picker/tests/test_team.html	2012-07-02 04:32:06 +0000
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd.  This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+
+<html>
+  <head>
+      <title>test team widget</title>
+
+      <!-- YUI and test setup -->
+      <script type="text/javascript"
+              src="../../../../../../build/js/yui/yui/yui.js">
+      </script>
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/console-core.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+      <link rel="stylesheet"
+      href="../../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+      <script type="text/javascript"
+              src="../../../../../../build/js/lp/app/testing/testrunner.js"></script>
+
+      <link rel="stylesheet" href="../../../../app/javascript/testing/test.css" />
+
+      <!-- Dependencies -->
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/client.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/lp.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/lazr/lazr.js"></script>
+      <script type="text/javascript"
+          src="../../../../../../build/js/lp/app/testing/mockio.js"></script>
+
+      <!-- The module under test. -->
+      <script type="text/javascript" src="../team.js"></script>
+
+      <!-- The test suite. -->
+      <script type="text/javascript" src="test_team.js"></script>
+
+    </head>
+    <body class="yui3-skin-sam">
+        <ul id="suites">
+            <li>lp.app.picker.team.test</li>
+        </ul>
+        <div id="fixture"></div>
+    </body>
+</html>

=== added file 'lib/lp/app/javascript/picker/tests/test_team.js'
--- lib/lp/app/javascript/picker/tests/test_team.js	1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/picker/tests/test_team.js	2012-07-02 04:32:06 +0000
@@ -0,0 +1,110 @@
+/* Copyright 2011 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ */
+
+YUI.add('lp.app.picker.team.test', function (Y) {
+    var tests = Y.namespace('lp.app.picker.team.test');
+    tests.suite = new Y.Test.Suite(
+        'lp.app.picker.team Tests');
+
+    tests.suite.add(new Y.Test.Case({
+        name: 'lp.app.picker.team_tests',
+
+
+        setUp: function() {
+        },
+
+        tearDown: function() {
+            delete this.mockio;
+            if (this.fixture !== undefined) {
+                this.fixture.empty(true);
+            }
+            delete this.fixture;
+            var ns = Y.namespace('lp.app.picker.team');
+            ns.widgets = null;
+            ns.team_form = null;
+        },
+
+        _simple_team_form: function() {
+            return '<table><tr><td>' +
+                '<input id="field.name" name="field.name"/>' +
+                '<input id="field.displayname" ' +
+                'name="field.displayname"/></td></tr></table>';
+        },
+
+        create_widget: function() {
+            this.mockio = new Y.lp.testing.mockio.MockIo();
+            this.widget = new Y.lp.app.picker.team.CreateTeamForm({
+                "io_provider": this.mockio
+            });
+            Y.Assert.areEqual(
+                'file:////people/+simplenewteam/++form++',
+                this.mockio.last_request.url);
+            this.mockio.success({
+                responseText: this._simple_team_form(),
+                responseHeaders: {'Content-Type': 'text/html'}});
+            this.fixture = Y.one('#fixture');
+            this.fixture.appendChild(this.widget.get('container'));
+        },
+
+        test_library_exists: function () {
+            Y.Assert.isObject(Y.lp.app.picker.team,
+                "Could not locate the lp.app.team module");
+        },
+
+        test_widget_can_be_instantiated: function() {
+            this.create_widget();
+            Y.Assert.isInstanceOf(
+                Y.lp.app.picker.team.CreateTeamForm, this.widget,
+                "Create Team Form failed to be instantiated");
+        },
+
+        test_new_team_save: function() {
+            // Clicking the save button on the new team form creates the team.
+            this.create_widget();
+
+            var save_success_called = false;
+            this.widget._save_team_success = function(response, team_data) {
+                Y.Assert.areEqual('fred', team_data['field.name']);
+                save_success_called = true;
+            };
+            var team_name = Y.one("input[id='field.name']");
+            team_name.set('value', 'fred');
+            var form_buttons = Y.one('.extra-form-buttons');
+            form_buttons.one('button.yes_button').simulate('click');
+            this.mockio.success({
+                responseText: '',
+                responseHeaders: {'Content-Type': 'application/json'}});
+            Y.Assert.isTrue(save_success_called);
+        },
+
+        test_save_team_success: function() {
+            // The save team success callback publishes the expected event and
+            // clears the form.
+            this.create_widget();
+            var event_publishd = false;
+            this.widget.subscribe(Y.lp.app.picker.team.TEAM_CREATED,
+                function(e) {
+                    var data = e.details[0];
+                    Y.Assert.areEqual('fred', data.value);
+                    Y.Assert.areEqual('Fred', data.title);
+                    Y.Assert.areEqual('/~fred', data.api_uri);
+                    Y.Assert.areEqual('team', data.metadata);
+                    event_publishd = true;
+                });
+            var ns = Y.namespace('lp.app.picker.team');
+            ns.team_form = '<p>test</p>';
+            var team_data = {
+                'field.name': 'fred',
+                'field.displayname': 'Fred'
+            };
+            this.widget._save_team_success('', team_data);
+            Y.Assert.isTrue(event_publishd);
+            Y.Assert.areEqual('test', Y.one('form p').get('text'));
+        }
+    }));
+
+}, '0.1', {
+    'requires': ['test', 'console', 'event', 'node-event-simulate',
+        'lp.client', 'lp.testing.mockio', 'lp.app.picker.team']
+});

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2012-05-17 07:46:56 +0000
+++ lib/lp/registry/browser/configure.zcml	2012-07-02 04:32:06 +0000
@@ -1216,6 +1216,12 @@
             class="lp.registry.browser.team.TeamAddView"
             permission="launchpad.AnyPerson"
             template="../templates/people-newteam.pt"/>
+        <browser:page
+            name="+simplenewteam"
+            for="lp.registry.interfaces.person.IPersonSet"
+            class="lp.registry.browser.team.SimpleTeamAddView"
+            permission="launchpad.AnyPerson"
+            template="../../app/templates/generic-edit.pt"/>
         <browser:pages
             for="lp.registry.interfaces.person.IPersonSet"
             permission="zope.Public"

=== modified file 'lib/lp/registry/browser/team.py'
--- lib/lp/registry/browser/team.py	2012-04-20 05:00:49 +0000
+++ lib/lp/registry/browser/team.py	2012-07-02 04:32:06 +0000
@@ -84,6 +84,7 @@
 from lp.app.validators.validation import validate_new_team_email
 from lp.app.widgets.itemswidgets import (
     LabeledMultiCheckBoxWidget,
+    LaunchpadDropdownWidget,
     LaunchpadRadioWidget,
     LaunchpadRadioWidgetWithDescription,
     )
@@ -1013,7 +1014,8 @@
         super(TeamAddView, self).setUpFields()
         self.setUpVisibilityField()
 
-    @action('Create Team', name='create')
+    @action('Create Team', name='create',
+        failure=LaunchpadFormView.ajax_failure_handler)
     def create_action(self, action, data):
         name = data.get('name')
         displayname = data.get('displayname')
@@ -1040,6 +1042,8 @@
                 "provider might use 'greylisting', which could delay the "
                 "message for up to an hour or two.)" % email)
 
+        if self.request.is_ajax:
+            return ''
         self.next_url = canonical_url(team)
 
     def _validateVisibilityConsistency(self, value):
@@ -1059,6 +1063,28 @@
         return None
 
 
+class SimpleTeamAddView(TeamAddView):
+    """View for adding a new team using a Javascript form.
+
+    This view is used to render a form used to create a new team. The form is
+    displayed in a popup overlay and submission is done using an XHR call.
+    """
+
+    for_input = True
+    schema = ITeam
+    next_url = None
+
+    field_names = [
+        "name", "displayname", "visibility", "subscriptionpolicy",
+        "teamowner"]
+
+    # Use a dropdown - Javascript will be used to change this to a choice
+    # popup widget.
+    custom_widget(
+        'subscriptionpolicy', LaunchpadDropdownWidget,
+        orientation='vertical')
+
+
 class ProposedTeamMembersEditView(LaunchpadFormView):
     schema = Interface
     label = 'Proposed team members'

=== modified file 'lib/lp/registry/browser/tests/test_team.py'
--- lib/lp/registry/browser/tests/test_team.py	2012-06-13 10:17:35 +0000
+++ lib/lp/registry/browser/tests/test_team.py	2012-07-02 04:32:06 +0000
@@ -559,6 +559,32 @@
             browser.getControl(name="field.visibility").value)
 
 
+class TestSimpleTeamAddView(TestCaseWithFactory):
+
+    layer = LaunchpadFunctionalLayer
+    view_name = '+simplenewteam'
+
+    def test_create_team(self):
+        personset = getUtility(IPersonSet)
+        team_name = self.factory.getUniqueString()
+        form = {
+            'field.name': team_name,
+            'field.displayname': 'New Team',
+            'field.visibility': 'PRIVATE',
+            'field.subscriptionpolicy': 'RESTRICTED',
+            'field.actions.create': 'Create',
+            }
+        login_celebrity('admin')
+        create_initialized_view(
+            personset, name=self.view_name, form=form)
+        team = personset.getByName(team_name)
+        self.assertIsNotNone(team)
+        self.assertEqual('New Team', team.displayname)
+        self.assertEqual(PersonVisibility.PRIVATE, team.visibility)
+        self.assertEqual(
+            TeamSubscriptionPolicy.RESTRICTED, team.subscriptionpolicy)
+
+
 class TestTeamMenu(TestCaseWithFactory):
 
     layer = DatabaseFunctionalLayer