← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/picker-wiring-812255 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-wiring-812255 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #812255 in Launchpad itself: "Person picker can show "Pick me" when it shouldn't"
  https://bugs.launchpad.net/launchpad/+bug/812255
  Bug #812257 in Launchpad itself: "Person picker sometimes shows "Remove person" instead of "Remove team""
  https://bugs.launchpad.net/launchpad/+bug/812257

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-wiring-812255/+merge/68629

This branch fixes a few picker bugs as well as performing as significant refactor and adding extra yui test coverage for person pickers.
Bugs 812255 and 812257 were the main focus but work was also done to implement a better fix for bug 798778 (which was previously closed).
A previously implemented hack to control the creation of the new PersonPicker widget was also removed.

== Implementation ==

There are 2 ways of instantiating a picker:

Y.lp.app.picker.addPickerPatcher(...)
Y.lp.app.picker.create(...)

The first way is used to create a picker invoked from the little yellow edit icon; the second way creates a picker from the Choose... link.

The root cause of the bugs was that code existed in the addPickerPatcher() method to wire up the "Assign me" and "Remove" links. This code was different from that used by the create() method and so if a PersonPicker was instantiated using create(), things weren't wired up properly. Besides all that, the code in question implements functionality which really does belong in the PersonPicker implementation.

The main work involved:
- moving the code for the "Assign Me" and "Remove" links to the PersonPicker
- add a picker_type property to the picker view classes to allow the Javascript hack to be removed
- add a new "selected_value" attribute to the core Picker widget
- control the rendering of the "Assign me" and "Remove" links based on this new selected_value attribute
- use an onChange event handler for the new "selected_value" attribute to update the links when the user makes a new selection using the picker
- delete unnecessary and improperly placed code by removing config for the "Assign me" and "Remove" links from the core picker implementation

The use of the selected_value attribute removes the need to "screen scrape" to read the picker value from the associated form widget and is a much cleaner implementation.

Bug 798778 was previously fixed by hiding the "Assign me" and "Remove" links when a search was done. This was not the best approach, in part because once a search was done, the "Remove" link was made invisible and could *never* be accessed again without a page refresh. The solution in this branch is to move the "Assign me" and "Remove" links above the search box - this separates them from the search results as requested in the bug and avoids the issues with the current solution.

When the picker is created using the Choose... link, the "Remove" link is hidden. It adds little value in this case since all that needs to be done is to clear the associated text field, and having the "Remove" link is not clear as to exactly what it does in this case.

== Tests ==

Relevant tests for the "Assign me" and "Remove links" were moved from test_picker_patcher.js to test_personpicker.js. The test coverage inside test_personpicker.js was extended to cover both types of person picker instantiation. This was done by defining common tests and using Y.merge() to mix in the relevant picker setup. This approach ensures consistent test coverage between the picker types without cut and paste code.

YUI tests run:
  test_picker.js
  test_personpicker.js
  test_pickerpatcher.js

In the absence of windmill tests, bug task assigning, branch reviewer editing, and recipe owner editing were tested by hand - these use cases cover the different application scenarios where a person picker is used.

== Lint ==

Linting changed files:
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/app/browser/lazrjs.py
  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.js
  lib/lp/app/javascript/picker/tests/test_picker.js
  lib/lp/app/javascript/picker/tests/test_picker_patcher.js
  lib/lp/app/widgets/popup.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/javascript/bugtask_index.js

./lib/lp/app/javascript/picker/person_picker.js
      13: 'PersonPicker' has not been fully defined yet.
      81: Expected '!==' and instead saw '!='.
      83: Expected '===' and instead saw '=='.
      84: Expected '===' and instead saw '=='.
./lib/lp/app/javascript/picker/picker.js
      67: 'Picker' has not been fully defined yet.
     258: Move 'var' declarations to the top of the function.
     258: Stopping.  (24% scanned).
      -1: JSLINT had a fatal error.
     598: Line exceeds 78 characters.
./lib/lp/app/javascript/picker/picker_patcher.js
      70: Expected '!==' and instead saw '!='.
     355: Expected '!==' and instead saw '!='.
./lib/lp/app/javascript/picker/tests/test_personpicker.js
     344: Expected '!==' and instead saw '!='.
./lib/lp/app/javascript/picker/tests/test_picker.js
     196: Move 'var' declarations to the top of the function.
     196: Stopping.  (20% scanned).
      -1: JSLINT had a fatal error.
      49: Line exceeds 78 characters.
     465: Line exceeds 78 characters.
     516: Line exceeds 78 characters.
     960: Line exceeds 78 characters.
./lib/lp/bugs/javascript/bugtask_index.js
     839: Expected '===' and instead saw '=='.
-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-wiring-812255/+merge/68629
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-wiring-812255 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-07-04 13:24:48 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-07-21 05:12:40 +0000
@@ -838,7 +838,7 @@
     }
 .extra-form-buttons {
     text-align: center;
-    height: 3em;
+    padding-top: 1em;
     white-space: nowrap;
     }
 .extra-form-buttons button {

=== modified file 'lib/lp/app/browser/lazrjs.py'
--- lib/lp/app/browser/lazrjs.py	2011-07-18 00:45:28 +0000
+++ lib/lp/app/browser/lazrjs.py	2011-07-21 05:12:40 +0000
@@ -240,9 +240,7 @@
 
     def __init__(self, context, exported_field, default_html,
                  content_box_id=None, header='Select an item',
-                 step_title='Search', assign_me_text='Pick me',
-                 remove_person_text='Remove person',
-                 remove_team_text='Remove team',
+                 step_title='Search',
                  null_display_value='None',
                  edit_view="+edit", edit_url=None, edit_title=''):
         """Create a widget wrapper.
@@ -255,9 +253,6 @@
             Automatically generated if this is not provided.
         :param header: The large text at the top of the picker.
         :param step_title: Smaller line of text below the header.
-        :param assign_me_text: Override default button text: "Pick me"
-        :param remove_person_text: Override default link text: "Remove person"
-        :param remove_team_text: Override default link text: "Remove team"
         :param null_display_value: This will be shown for a missing value
         :param edit_view: The view name to use to generate the edit_url if
             one is not specified.
@@ -271,9 +266,6 @@
         self.default_html = default_html
         self.header = header
         self.step_title = step_title
-        self.assign_me_text = assign_me_text
-        self.remove_person_text = remove_person_text
-        self.remove_team_text = remove_team_text
         self.null_display_value = null_display_value
 
         # JSON encoded attributes.
@@ -283,20 +275,35 @@
             self.exported_field.vocabularyName)
 
     @property
+    def picker_type(self):
+        return 'default'
+
+    @property
     def selected_value_metadata(self):
         return None
 
     @property
+    def selected_value(self):
+        """ String representation of field value associated with the picker.
+
+        Default implementation is to return the 'name' attribute.
+        """
+        val = getattr(self.context, self.exported_field.__name__)
+        if val is not None and hasattr(val, 'name'):
+            return getattr(val, 'name')
+        return None
+
+    @property
     def config(self):
+        return self.getConfig()
+
+    def getConfig(self):
         return dict(
+            picker_type=self.picker_type,
             header=self.header, step_title=self.step_title,
+            selected_value=self.selected_value,
             selected_value_metadata=self.selected_value_metadata,
-            assign_me_text=self.assign_me_text,
-            remove_person_text=self.remove_person_text,
-            remove_team_text=self.remove_team_text,
             null_display_value=self.null_display_value,
-            show_remove_button=self.optional_field,
-            show_assign_me_button=self.show_assign_me_button,
             show_search_box=self.show_search_box)
 
     @property
@@ -313,6 +320,53 @@
     def show_search_box(self):
         return IHugeVocabulary.providedBy(self.vocabulary)
 
+
+class InlinePersonEditPickerWidget(InlineEditPickerWidget):
+    def __init__(self, context, exported_field, default_html,
+                 content_box_id=None, header='Select an item',
+                 step_title='Search', assign_me_text='Pick me',
+                 remove_person_text='Remove person',
+                 remove_team_text='Remove team',
+                 null_display_value='None',
+                 edit_view="+edit", edit_url=None, edit_title=''):
+        """Create a widget wrapper.
+
+        :param context: The object that is being edited.
+        :param exported_field: The attribute being edited. This should be
+            a field from an interface of the form ISomeInterface['fieldname']
+        :param default_html: Default display of attribute.
+        :param content_box_id: The HTML id to use for this widget.
+            Automatically generated if this is not provided.
+        :param header: The large text at the top of the picker.
+        :param step_title: Smaller line of text below the header.
+        :param assign_me_text: Override default button text: "Pick me"
+        :param remove_person_text: Override default link text: "Remove person"
+        :param remove_team_text: Override default link text: "Remove team"
+        :param null_display_value: This will be shown for a missing value
+        :param edit_view: The view name to use to generate the edit_url if
+            one is not specified.
+        :param edit_url: The URL to use for editing when the user isn't logged
+            in and when JS is off.  Defaults to the edit_view on the context.
+        :param edit_title: Used to set the title attribute of the anchor.
+        """
+        super(InlinePersonEditPickerWidget, self).__init__(
+            context, exported_field, default_html, content_box_id, header,
+            step_title, null_display_value,
+            edit_view, edit_url, edit_title)
+
+        self.assign_me_text = assign_me_text
+        self.remove_person_text = remove_person_text
+        self.remove_team_text = remove_team_text
+
+    @property
+    def picker_type(self):
+        return 'person'
+
+    @property
+    def selected_value_metadata(self):
+        val = getattr(self.context, self.exported_field.__name__)
+        return get_person_picker_entry_metadata(val)
+
     @property
     def show_assign_me_button(self):
         # show_assign_me_button is true if user is in the vocabulary.
@@ -320,12 +374,15 @@
         user = getUtility(ILaunchBag).user
         return user and user in vocabulary
 
-
-class InlinePersonEditPickerWidget(InlineEditPickerWidget):
-    @property
-    def selected_value_metadata(self):
-        val = getattr(self.context, self.exported_field.__name__)
-        return get_person_picker_entry_metadata(val)
+    def getConfig(self):
+        config = super(InlinePersonEditPickerWidget, self).getConfig()
+        config.update(dict(
+            show_remove_button=self.optional_field,
+            show_assign_me_button=self.show_assign_me_button,
+            assign_me_text=self.assign_me_text,
+            remove_person_text=self.remove_person_text,
+            remove_team_text=self.remove_team_text))
+        return config
 
 
 class InlineMultiCheckboxWidget(WidgetBase):

=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2011-07-19 03:27:34 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2011-07-21 05:12:40 +0000
@@ -24,7 +24,6 @@
         var assign_me_text = 'Pick me';
         var remove_person_text = 'Remove person';
         var remove_team_text = 'Remove team';
-
         if (cfg !== undefined) {
             if (cfg.show_assign_me_button !== undefined) {
                 show_assign_me_button = cfg.show_assign_me_button;
@@ -59,13 +58,50 @@
         this.constructor.superclass.show.call(this);
     },
 
+    _update_button_text: function() {
+        var link_text = this._remove_person_text;
+        if (this.get('selected_value_metadata') === 'team') {
+            link_text = this._remove_team_text;
+        }
+        this.remove_button.set('text', link_text);
+    },
+
+    _show_hide_buttons: function () {
+        var selected_value = this.get('selected_value');
+        if (this.remove_button) {
+            if (selected_value === null) {
+                this.remove_button.addClass('yui3-picker-hidden');
+            } else {
+                this.remove_button.removeClass('yui3-picker-hidden');
+                this._update_button_text();
+            }
+        }
+
+        if (this.assign_me_button) {
+            if ((selected_value != null
+                && LP.links.me.match('~'+selected_value + "$")
+                    == '~'+selected_value)
+                || LP.links.me == selected_value) {
+                this.assign_me_button.addClass('yui3-picker-hidden');
+            } else {
+                this.assign_me_button.removeClass('yui3-picker-hidden');
+            }
+        }
+    },
+
     remove: function () {
-        this.fire('save', {value: ''});
+        this.hide();
+        this.fire(Y.lazr.picker.Picker.SAVE, {value: null});
     },
 
     assign_me: function () {
         var name = LP.links.me.replace('/~', '');
-        this.fire('save', {value: name});
+        this.fire(Y.lazr.picker.Picker.SAVE, {
+            image: '/@@/person',
+            title: 'Me',
+            api_uri: LP.links.me,
+            value: name
+        });
     },
 
     _renderPersonPickerUI: function() {
@@ -92,24 +128,17 @@
             this._extra_buttons.appendChild(assign_me_button);
             this.assign_me_button = assign_me_button;
         }
+        this._show_hide_buttons();
+        this.after("selected_valueChange", function(e) {
+            this._show_hide_buttons();
+        });
     },
 
     syncUI: function() {
         // call Picker's sync
         this.constructor.superclass.syncUI.apply(this, arguments);
-        this._footer_slot_box.appendChild(this._extra_buttons);
-    },
-
-    bindUI: function() {
-        this.constructor.superclass.bindUI.apply(this, arguments);
-        this.after('resultsChange', function(e) {
-            var results = this.get('results');
-            if (this._search_input.get('value') && !results.length) {
-                this._extra_buttons.removeClass('unseen');
-            } else {
-                this._extra_buttons.addClass('unseen');
-            }
-        });
+        this._search_input.insert(
+            this._extra_buttons, this._search_input.get('parentNode'));
     }
 });
 PersonPicker.NAME = 'picker';

=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js	2011-07-19 15:16:37 +0000
+++ lib/lp/app/javascript/picker/picker.js	2011-07-21 05:12:40 +0000
@@ -58,6 +58,8 @@
     NO_RESULTS_SEARCH_MESSAGE = 'no_results_search_message',
     RENDERUI = "renderUI",
     BINDUI = "bindUI",
+    ASSOCIATED_FIELD_ID = 'associated_field_id',
+    SELECTED_VALUE = 'selected_value',
     SELECTED_VALUE_METADATA = 'selected_value_metadata';
 
 
@@ -182,12 +184,22 @@
         }
 
         if (!Y.Lang.isUndefined(cfg)) {
+            // The picker's associated field.
+            if (Y.Lang.isValue(cfg[ASSOCIATED_FIELD_ID])) {
+                this.plug(TextFieldPickerPlugin,
+                            {input_element:
+                                '[id="'+cfg[ASSOCIATED_FIELD_ID]+'"]'});
+            }
             // Meta information associated with the picker's associated
             // field's current value.
             if (Y.Lang.isValue(cfg[SELECTED_VALUE_METADATA])) {
                 this.set(
                     SELECTED_VALUE_METADATA, cfg[SELECTED_VALUE_METADATA]);
             }
+            // The value of the picker's associated field.
+            if (Y.Lang.isValue(cfg[SELECTED_VALUE])) {
+                this.set(SELECTED_VALUE, cfg[SELECTED_VALUE]);
+            }
         }
     },
 
@@ -784,7 +796,12 @@
     }
     });
 
+/**
+ * Some constants.
+ */
 Picker.NAME = PICKER;
+Picker.SAVE = SAVE;
+Picker.SEARCH = SEARCH;
 
 /**
  * The details index of the save result.
@@ -868,6 +885,14 @@
     current_search_string: {value: ''},
 
     /**
+     * The string representation of the value selected by using this picker.
+     *
+     * @attribute selected_value
+     * @type String
+     */
+    selected_value: {value: null},
+
+    /**
      * Meta information about the current state of the associated field,
      * whose value is selected by using this picker.
      *
@@ -1016,16 +1041,24 @@
         this.doAfter('save', function (e) {
             var result = e.details[Picker.SAVE_RESULT];
             this.get('host').set(SELECTED_VALUE_METADATA, result.metadata);
-            input.set("value",  result.value);
+            this.get('host').set(SELECTED_VALUE, result.value);
+            if (result.value != null) {
+                input.set("value",  result.value);
+            } else {
+                input.set("value", '');
+            }
             // If the search input isn't blurred before it is focused,
             // then the I-beam disappears.
             input.blur();
             input.focus();
         });
         this.doAfter('show', function() {
+            var selected_value = null;
             if ( input.get("value") ) {
-                this.get('host')._search_input.set('value', input.get("value"));
+                selected_value = input.get("value");
             }
+            this.get('host')._search_input.set('value', selected_value);
+            this.get('host').set(SELECTED_VALUE, selected_value);
         });
     }
 });

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2011-07-19 03:27:34 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2011-07-21 05:12:40 +0000
@@ -18,14 +18,12 @@
  * @param {String} attribute_name The attribute on the resource being
  *                                modified.
  * @param {String} content_box_id
- * @param {Object} config Object literal of config name/value pairs.
+ * @param {Object} config Object literal of config name/value pairs. The
+ *                        values listed below are common for all picker types.
+ *     config.picker_type: the type of picker to create (default or person).
  *     config.header: a line of text at the top of the widget.
  *     config.step_title: overrides the subtitle.
  *     config.null_display_value: Override the default 'None' text.
- *     config.show_remove_button: Should the remove button be shown?
- *         Defaults to false, should be a boolean.
- *     config.show_assign_me_button: Should the 'assign me' button be shown?
- *         Defaults to false, should be a boolean.
  *     config.show_search_box: Should the search box be shown.
  *         Vocabularies that are not huge should not have a search box.
  */
@@ -37,8 +35,6 @@
         return;
     }
 
-    var show_remove_button = false;
-    var show_assign_me_button = false;
     var null_display_value = 'None';
     var show_search_box = true;
 
@@ -48,22 +44,9 @@
         if (config.null_display_value !== undefined) {
             null_display_value = config.null_display_value;
         }
-
-        if (config.show_remove_button !== undefined) {
-            show_remove_button = config.show_remove_button;
-        }
-
-        if (config.show_assign_me_button !== undefined) {
-            show_assign_me_button = config.show_assign_me_button;
-        }
-
         if (config.show_search_box !== undefined) {
             show_search_box = config.show_search_box;
         }
-        if (config.picker_type === undefined &&
-                (show_assign_me_button || show_remove_button)) {
-            config.picker_type = 'person';
-        }
     }
 
     var content_box = Y.one('#' + content_box_id);
@@ -79,82 +62,31 @@
                 '</div>'));
     };
 
-    var update_button_text = function() {
-        var link_text = picker._remove_person_text;
-        if (picker.get('selected_value_metadata') === 'team') {
-            link_text = picker._remove_team_text;
-        }
-        remove_button.set('text', link_text);
-    };
-
-    var show_hide_buttons = function () {
-        var link = content_box.one('.yui3-activator-data-box a');
-        if (remove_button) {
-            if (link === null || !show_remove_button) {
-                remove_button.addClass('yui3-picker-hidden');
-            } else {
-                remove_button.removeClass('yui3-picker-hidden');
-                update_button_text();
-            }
-        }
-
-        if (assign_me_button) {
-            if (link !== null
-                && link.get('href').match(LP.links.me + "$") == LP.links.me) {
-                assign_me_button.addClass('yui3-picker-hidden');
-            } else {
-                assign_me_button.removeClass('yui3-picker-hidden');
-            }
-        }
-    };
-
     var save = function (picker_result) {
         activator.renderProcessing();
         var success_handler = function (entry) {
+          var to_render = null_display_value;
+          var selected_value = null;
+          if (entry.get(attribute_name) != null) {
+              to_render = entry.getHTML(attribute_name);
+              selected_value = picker_result.api_uri;
+          }
+          // NB We need to set the selected_value_metadata attribute first
+          // because we listen for changes to selected_value.
           picker.set('selected_value_metadata', picker_result.metadata);
-          activator.renderSuccess(entry.getHTML(attribute_name));
-          show_hide_buttons();
-        };
-
-        var patch_payload = {};
-        patch_payload[attribute_name] = Y.lp.client.get_absolute_uri(
-            picker_result.api_uri);
-
-        var client = new Y.lp.client.Launchpad();
-        client.patch(picker._resource_uri, patch_payload, {
-            accept: 'application/json;include=lp_html',
-            on: {
-                success: success_handler,
-                failure: failure_handler
-            }
-        });
-    };
-
-    // Create a new assign_me and remove_assignee function that uses the
-    // patcher save method instead of the standard picker save.
-    var assign_me = function () {
-        picker.hide();
-        save({
-            image: '/@@/person',
-            title: 'Me',
-            api_uri: LP.links.me
-        });
-    };
-
-    var remove = function () {
-        picker.hide();
-        activator.renderProcessing();
-        var success_handler = function (entry) {
-            activator.renderSuccess(Y.Node.create(null_display_value));
-            show_hide_buttons();
-        };
-
-        var patch_payload = {};
-        patch_payload[attribute_name] = null;
-
-        var client = new Y.lp.client.Launchpad();
-        // Use picker._resource_uri, since it might have been changed
-        // from the outside after the widget has already been initialized.
+          picker.set('selected_value', selected_value);
+          activator.renderSuccess(to_render);
+        };
+
+        var patch_payload = {};
+        if (Y.Lang.isValue(picker_result.api_uri)) {
+            patch_payload[attribute_name] = Y.lp.client.get_absolute_uri(
+                picker_result.api_uri);
+        } else {
+            patch_payload[attribute_name] = null;
+        }
+
+        var client = new Y.lp.client.Launchpad();
         client.patch(picker._resource_uri, patch_payload, {
             accept: 'application/json;include=lp_html',
             on: {
@@ -167,16 +99,6 @@
     config.save = save;
     var picker = namespace.create(vocabulary, config);
     picker._resource_uri = resource_uri;
-    var remove_button = picker.remove_button;
-    var assign_me_button = picker.assign_me_button;
-    if (show_remove_button) {
-        Y.Event.purgeElement(remove_button);
-        remove_button.on('click', remove);
-    }
-    if (show_assign_me_button) {
-        Y.Event.purgeElement(assign_me_button);
-        assign_me_button.on('click', assign_me);
-    }
 
     // If we are to pre-load the vocab, we need a spinner.
     // We set it up here because we only want to do it once and the
@@ -191,16 +113,13 @@
         if (!show_search_box) {
           config.temp_spinner.removeClass('unseen');
           picker.set('min_search_chars', 0);
-          picker.fire('search', '');
+          picker.fire(Y.lazr.picker.Picker.SEARCH, '');
           picker.get('contentBox').one(
               '.yui3-picker-search-box').addClass('unseen');
         }
         picker.show();
     });
     activator.render();
-
-    show_hide_buttons();
-
     return picker;
 };
 
@@ -384,6 +303,7 @@
     }
 
     var new_config = Y.merge(config, {
+        associated_field_id: associated_field_id,
         align: {
             points: [Y.WidgetPositionAlign.CC,
                      Y.WidgetPositionAlign.CC]
@@ -402,10 +322,6 @@
     } else {
         picker = new Y.lazr.picker.Picker(new_config);
     }
-    if (associated_field_id != null) {
-        picker.plug(Y.lazr.picker.TextFieldPickerPlugin,
-                    {input_element: '[id="'+associated_field_id+'"]'});
-    }
 
     // We don't want the default save to fire since this hides
     // the form. We want to do this ourselves after any validation has had a
@@ -416,6 +332,7 @@
         Y.log('Got save event.');
         var picker_result = e.details[Y.lazr.picker.Picker.SAVE_RESULT];
         var do_save = function() {
+            picker.hide();
             if (Y.Lang.isFunction(config.save)) {
                 config.save(picker_result);
             }
@@ -549,7 +466,7 @@
         }
     };
 
-    picker.after('search', search_handler);
+    picker.after(Y.lazr.picker.Picker.SEARCH, search_handler);
 
     picker.render();
     picker.hide();

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-07-15 02:20:36 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2011-07-21 05:12:40 +0000
@@ -36,11 +36,11 @@
 var suite = new Y.Test.Suite("PersonPicker Tests");
 
 /*
- * Test cases for assign and remove buttons.
+ * Test cases for person picker functionality.
  */
-suite.add(new Y.Test.Case({
+var commonPersonPickerTests = {
 
-    name: 'picker_assign_remove_button',
+    name: 'common_person_picker',
 
     setUp: function() {
         this.ME = '/~me';
@@ -51,17 +51,19 @@
         this.vocabulary = [
             {
                 "value": "me",
+                "metadata": "person",
                 "title": "Me",
                 "css": "sprite-person",
                 "description": "me@xxxxxxxxxxx",
-                "api_uri": "~/me"
+                "api_uri": "/~me"
             },
             {
-                "value": "someone",
-                "title": "Someone Else",
-                "css": "sprite-person",
+                "value": "someteam",
+                "metadata": "team",
+                "title": "Some Team",
+                "css": "sprite-team",
                 "description": "someone@xxxxxxxxxxx",
-                "api_uri": "~/someone"
+                "api_uri": "/~someteam"
             }
         ];
 
@@ -73,7 +75,10 @@
                 // our setup assumes success, so we just do the success
                 // callback.
                 var entry_repr = {
-                  'lp_html': {'test_link': '<a href="/~me">Content</a>'}
+                  'test_link': representation.test_link,
+                  'lp_html': {
+                      'test_link': '<a href="'
+                                    +representation.test_link+'">Content</a>'}
                 };
                 var result = new Y.lp.client.Entry(
                     null, entry_repr, "a_self_link");
@@ -86,25 +91,42 @@
         delete window.LP;
     },
 
-    create_picker: function() {
-        var config = {
-            "step_title": "Choose someone",
-            "header": "Pick Someone",
-            "validate_callback": null,
-            "show_search_box": true,
-            "picker_type": 'person'
-            };
-        this.picker = Y.lp.app.picker.addPickerPatcher(
-                this.vocabulary,
-                "foo/bar",
-                "test_link",
-                "picker_id",
-                config);
+    _picker_params: function(
+        show_assign_me_button, show_remove_button,
+        selected_value, selected_value_metadata) {
+        return {
+            "show_assign_me_button": show_assign_me_button,
+            "show_remove_button": show_remove_button,
+            "selected_value": selected_value,
+            "selected_value_metadata": selected_value_metadata
+        };
+    },
+
+    _check_button_state: function(btn_class, is_visible) {
+        var assign_me_button = Y.one(btn_class);
+        Assert.isNotNull(assign_me_button);
+        if (is_visible) {
+            Assert.isFalse(
+                assign_me_button.hasClass('yui3-picker-hidden'),
+                btn_class + " should be visible but is hidden");
+        } else {
+            Assert.isTrue(
+                assign_me_button.hasClass('yui3-picker-hidden'),
+                btn_class + " should be hidden but is visible");
+        }
+    },
+
+    _check_assign_me_button_state: function(is_visible) {
+        this._check_button_state('.yui-picker-assign-me-button', is_visible);
+    },
+
+    _check_remove_button_state: function(is_visible) {
+        this._check_button_state('.yui-picker-remove-button', is_visible);
     },
 
     test_search_field_focus: function () {
         // The search field has focus when the picker is shown.
-        this.create_picker();
+        this.create_picker(this._picker_params(true, true));
         this.picker.render();
         this.picker.hide();
 
@@ -118,7 +140,7 @@
 
     test_buttons_save: function () {
         // The assign/remove links save the correct values.
-        this.picker = new Y.lazr.picker.PersonPicker();
+        this.create_picker(this._picker_params(true, true));
         this.picker.render();
         this.picker.show();
 
@@ -130,47 +152,300 @@
         });
         var remove = Y.one('.yui-picker-remove-button');
         remove.simulate('click');
-        Y.Assert.areEqual('', data);
+        Y.Assert.areEqual(null, data);
 
         var assign_me = Y.one('.yui-picker-assign-me-button');
         assign_me.simulate('click');
         Y.Assert.areEqual('me', data);
     },
 
-    _change_results: function (picker, no_result) {
-        // simulates search by setting a value and the result
-        picker._search_input.set('value', 'foo');
-        if (no_result) {
-            picker.set('results', []);
-        } else {
-            picker.set('results', this.vocabulary);
-        }
-    },
-
-    test_buttons_vanish: function () {
-        // The assign/remove links are hidden when a search is performed.
-        this.create_picker();
-        this.picker.render();
-        this._change_results(this.picker, false);
-        Y.Assert.isTrue(this.picker._extra_buttons.hasClass('unseen'));
-
-        this._change_results(this.picker, true);
-        Y.Assert.isFalse(this.picker._extra_buttons.hasClass('unseen'));
-    },
-
-    test_buttons_reappear: function () {
-        // The assign/remove links are shown again after doing a search and
-        // selecting a result. Doing a search hides the links so we need to
-        // ensure they are made visible again.
-        this.create_picker();
-        this.picker.render();
-        this._change_results(this.picker, false);
-        Y.Assert.isTrue(this.picker._extra_buttons.hasClass('unseen'));
-        simulate(
-            this.picker.get('boundingBox'), '.yui3-picker-results li', 'click');
-        Y.Assert.isFalse(this.picker._extra_buttons.hasClass('unseen'));
-    }
-}));
+    test_picker_assign_me_button_text: function() {
+        // The assign me button text is correct.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        var assign_me_button = Y.one('.yui-picker-assign-me-button');
+        Assert.areEqual('Assign Moi', assign_me_button.get('innerHTML'));
+    },
+
+    test_picker_remove_person_button_text: function() {
+        // The remove button text is correct.
+        this.create_picker(this._picker_params(true, true, "fred", "person"));
+        this.picker.render();
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+    },
+
+    test_picker_remove_team_button_text: function() {
+        // The remove button text is correct.
+        this.create_picker(this._picker_params(true, true, "cats", "team"));
+        this.picker.render();
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
+    },
+
+    test_picker_has_assign_me_button: function() {
+        // The assign me button is shown.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        this._check_assign_me_button_state(true);
+    },
+
+    test_picker_no_assign_me_button_unless_configured: function() {
+        // The assign me button is only rendered if show_assign_me_button
+        // config setting is true.
+        this.create_picker(this._picker_params(false, true));
+        this.picker.render();
+        Assert.isNull(Y.one('.yui-picker-assign-me-button'));
+    },
+
+    test_picker_no_assign_me_button_if_value_is_me: function() {
+        // The assign me button is not shown if the picker is created for a
+        // field where the value is "me".
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        this.picker.render();
+        this._check_assign_me_button_state(false);
+    },
+
+    test_picker_no_remove_button_if_null_value: function() {
+        // The remove button is not shown if the picker is created for a field
+        // which has a null value.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        this._check_remove_button_state(false);
+    },
+
+    test_picker_has_remove_button_if_value: function() {
+        // The remove button is shown if the picker is created for a field
+        // which has a value.
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        this.picker.render();
+        this._check_remove_button_state(true);
+    },
+
+    test_picker_no_remove_button_unless_configured: function() {
+        // The remove button is only rendered if show_remove_button setting is
+        // true.
+        this.create_picker(this._picker_params(true, false, "me"), this.ME);
+        this.picker.render();
+        Assert.isNull(Y.one('.yui-picker-remove-button'));
+    }
+};
+
+/*
+ * Test cases for person picker functionality when created using
+ * addPickerPatcher.
+ */
+var pickerPatcherPersonPickerTests = {
+
+    name: 'picker_patcher_person_picker',
+
+    create_picker: function(params, field_value) {
+        if (field_value !== undefined) {
+            var data_box = Y.one('#picker_id .yui3-activator-data-box');
+            data_box.appendChild(Y.Node.create('<a>Content</a>'));
+            data_box.one('a').set('href', field_value);
+        }
+
+        var config = {
+            "picker_type": "person",
+            "step_title": "Choose someone",
+            "header": "Pick Someone",
+            "validate_callback": null,
+            "show_search_box": true,
+            "show_assign_me_button": params.show_assign_me_button,
+            "show_remove_button": params.show_remove_button,
+            "selected_value": params.selected_value,
+            "selected_value_metadata": params.selected_value_metadata,
+            "assign_me_text": "Assign Moi",
+            "remove_person_text": "Remove someone",
+            "remove_team_text": "Remove some team"
+            };
+        this.picker = Y.lp.app.picker.addPickerPatcher(
+                this.vocabulary,
+                "foo/bar",
+                "test_link",
+                "picker_id",
+                config);
+    },
+
+    test_picker_assign_me_button_hide_on_save: function() {
+        // The assign me button is shown initially but hidden if the picker
+        // saves a value equal to 'me'.
+        this.create_picker(this._picker_params(true, true));
+        this._check_assign_me_button_state(true);
+        this.picker.set('results', this.vocabulary);
+        this.picker.render();
+        simulate(
+            this.picker.get('boundingBox').one('.yui3-picker-results'),
+                'li:nth-child(1)', 'click');
+        this._check_assign_me_button_state(false);
+    },
+
+    test_picker_remove_button_clicked: function() {
+        // The remove button is hidden once a picker value has been removed.
+        // And the assign me button is shown.
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        this.picker.render();
+        this._check_assign_me_button_state(false);
+        var remove = Y.one('.yui-picker-remove-button');
+        remove.simulate('click');
+        this._check_remove_button_state(false);
+        this._check_assign_me_button_state(true);
+    },
+
+    test_picker_assign_me_button_clicked: function() {
+        // The assign me button is hidden once it is clicked.
+        // And the remove button is shown.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        var assign_me = Y.one('.yui-picker-assign-me-button');
+        assign_me.simulate('click');
+        this._check_remove_button_state(true);
+        this._check_assign_me_button_state(false);
+    },
+
+    test_picker_assign_me_updates_remove_text: function() {
+        // When Assign me is used, the Remove button text is updated from
+        // the team removal text to the person removal text.
+        this.create_picker(this._picker_params(true, true, "cats", "team"));
+        this.picker.render();
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
+        var assign_me = Y.one('.yui-picker-assign-me-button');
+        assign_me.simulate('click');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+    },
+
+    test_picker_save_updates_remove_text: function() {
+        // When save is called, the Remove button text is updated according to
+        // the newly saved value.
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+        this.picker.set('results', this.vocabulary);
+        this.picker.render();
+        simulate(
+            this.picker.get('boundingBox').one('.yui3-picker-results'),
+                'li:nth-child(2)', 'click');
+        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
+    }
+};
+
+/*
+ * Test cases for person picker functionality when created using
+ * addPickerPatcher.
+ */
+var createDirectPersonPickerTests = {
+
+    name: 'create_direct_person_picker',
+
+    tearDown: function() {
+        cleanup_widget(this.picker);
+        if (this.search_input != null) {
+            this.search_input.remove();
+        }
+        delete window.LP;
+    },
+
+    create_picker: function(params, field_value) {
+        var associated_field_id;
+        this.search_input = Y.Node.create(
+                '<input id="field_initval" value="foo"/>');
+        Y.one(document.body).appendChild(this.search_input);
+        associated_field_id = 'field_initval';
+        var text_field = Y.one('#field_initval');
+        if (field_value !== undefined) {
+            text_field.set('text', field_value);
+        }
+        var config = {
+            "picker_type": "person",
+            "associated_field_id": associated_field_id,
+            "show_assign_me_button": params.show_assign_me_button,
+            "show_remove_button": params.show_remove_button,
+            "selected_value": params.selected_value,
+            "selected_value_metadata": params.selected_value_metadata,
+            "assign_me_text": "Assign Moi",
+            "remove_person_text": "Remove someone",
+            "remove_team_text": "Remove some team"
+            };
+        this.picker = Y.lp.app.picker.create(
+                            this.vocabulary, config, associated_field_id);
+    },
+
+    test_picker_assign_me_button_hide_on_save: function() {
+        // The assign me button is shown initially but hidden if the picker
+        // saves a value equal to 'me'.
+        this.create_picker(this._picker_params(true, true));
+        this._check_assign_me_button_state(true);
+        this.picker.set('results', this.vocabulary);
+        this.picker.render();
+        simulate(
+            this.picker.get('boundingBox').one('.yui3-picker-results'),
+                'li:nth-child(1)', 'click');
+        this._check_assign_me_button_state(false);
+    },
+
+    test_picker_remove_button_clicked: function() {
+        // The remove button is hidden once a picker value has been removed.
+        // And the assign me button is shown.
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        this.picker.render();
+        this._check_assign_me_button_state(false);
+        var remove = Y.one('.yui-picker-remove-button');
+        remove.simulate('click');
+        this._check_remove_button_state(false);
+        this._check_assign_me_button_state(true);
+    },
+
+    test_picker_assign_me_button_clicked: function() {
+        // The assign me button is hidden once it is clicked.
+        // And the remove button is shown.
+        this.create_picker(this._picker_params(true, true));
+        this.picker.render();
+        var assign_me = Y.one('.yui-picker-assign-me-button');
+        assign_me.simulate('click');
+        this._check_remove_button_state(true);
+        this._check_assign_me_button_state(false);
+    },
+
+    test_picker_assign_me_updates_remove_text: function() {
+        // When Assign me is used, the Remove button text is updated from
+        // the team removal text to the person removal text.
+        this.create_picker(this._picker_params(true, true, "cats", "team"));
+        this.picker.render();
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
+        var assign_me = Y.one('.yui-picker-assign-me-button');
+        assign_me.simulate('click');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+    },
+
+    test_picker_save_updates_remove_text: function() {
+        // When save is called, the Remove button text is updated according to
+        // the newly saved value.
+        this.create_picker(this._picker_params(true, true, "me"), this.ME);
+        var remove_button = Y.one('.yui-picker-remove-button');
+        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
+        this.picker.set('results', this.vocabulary);
+        this.picker.render();
+        simulate(
+            this.picker.get('boundingBox').one('.yui3-picker-results'),
+                'li:nth-child(2)', 'click');
+        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
+    }
+};
+
+suite.add(new Y.Test.Case(
+  Y.merge(
+      commonPersonPickerTests,
+      pickerPatcherPersonPickerTests)
+));
+
+suite.add(new Y.Test.Case(
+  Y.merge(
+      commonPersonPickerTests,
+      createDirectPersonPickerTests)
+));
 
 // Hook for the test runner to get test results.
 var handle_complete = function(data) {

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker.js'
--- lib/lp/app/javascript/picker/tests/test_picker.js	2011-07-19 15:16:37 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker.js	2011-07-21 05:12:40 +0000
@@ -35,6 +35,7 @@
 
     setUp: function() {
         this.picker = new Y.lazr.picker.Picker({
+            "selected_value": 'foo',
             "selected_value_metadata": 'foobar'
         });
     },
@@ -49,6 +50,7 @@
     },
 
     test_picker_initialisation: function() {
+        Assert.areEqual('foo', this.picker.get('selected_value'));
         Assert.areEqual('foobar', this.picker.get('selected_value_metadata'));
     },
 
@@ -959,11 +961,11 @@
 
     setUp: function() {
         this.search_input = Y.Node.create(
-                '<input id="field.initval" value="foo" />');
-        var node = Y.one(document.body).appendChild(this.search_input);
-        this.picker = new Y.lazr.picker.Picker();
-        this.picker.plug(Y.lazr.picker.TextFieldPickerPlugin,
-                         {input_element: '[id="field.initval"]'});
+                '<input id="field.initval" value="foo"/>');
+        Y.one(document.body).appendChild(this.search_input);
+        this.picker = new Y.lazr.picker.Picker({
+            associated_field_id: 'field.initval'
+        });
     },
 
     tearDown: function() {
@@ -991,6 +993,8 @@
             'first', Y.one('[id="field.initval"]').get("value"));
         Assert.areEqual(
             'new_metadata', this.picker.get('selected_value_metadata'));
+        Assert.areEqual(
+            'first', this.picker.get('selected_value'));
         Assert.isTrue(got_focus, "focus didn't go to the search input.");
     }
 

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker_patcher.js'
--- lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2011-07-19 12:22:03 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker_patcher.js	2011-07-21 05:12:40 +0000
@@ -248,246 +248,6 @@
 }));
 
 /*
- * Test cases for assign and remove buttons.
- */
-suite.add(new Y.Test.Case({
-
-    name: 'picker_assign_remove_button',
-
-    setUp: function() {
-        var i;
-        this.vocabulary = new Array(121);
-        for (i = 0; i <5; i++) {
-            this.vocabulary[i] = {
-                "value": "value-" + i,
-                "title": "title-" + i,
-                "css": "sprite-person",
-                "description": "description-" + i,
-                "metadata": "team",
-                "api_uri": "~/fred-" + i};
-        }
-        this.ME = '/~me';
-        window.LP = {
-            links: {
-                me: this.ME
-            },
-            cache: {}
-        };
-
-        // We patch Launchpad client to return some fake data for the patch
-        // operation.
-        Y.lp.client.Launchpad = function() {};
-        Y.lp.client.Launchpad.prototype.patch =
-            function(uri, representation, config, headers) {
-                // our setup assumes success, so we just do the success
-                // callback.
-                var entry_repr = {
-                  'lp_html': {'test_link': '<a href="/~me">Content</a>'}
-                };
-                var result = new Y.lp.client.Entry(
-                    null, entry_repr, "a_self_link");
-                config.on.success(result);
-            };
-
-    },
-
-    tearDown: function() {
-        cleanup_widget(this.picker);
-        delete window.LP;
-    },
-
-    create_picker: function(params, field_value) {
-        if (field_value !== undefined) {
-            var data_box = Y.one('#picker_id .yui3-activator-data-box');
-            data_box.appendChild(Y.Node.create('<a>Content</a>'));
-            data_box.one('a').set('href', field_value);
-        }
-
-        var config = {
-            "step_title": "Choose someone",
-            "header": "Pick Someone",
-            "validate_callback": null,
-            "show_search_box": true,
-            "show_assign_me_button": params.show_assign_me_button,
-            "show_remove_button": params.show_remove_button,
-            "selected_value_metadata": params.selected_value_metadata,
-            "assign_me_text": "Assign Moi",
-            "remove_person_text": "Remove someone",
-            "remove_team_text": "Remove some team"
-            };
-        this.picker = Y.lp.app.picker.addPickerPatcher(
-                this.vocabulary,
-                "foo/bar",
-                "test_link",
-                "picker_id",
-                config);
-    },
-
-    _picker_params: function(
-        show_assign_me_button, show_remove_button, selected_value_metadata) {
-        return {
-            "show_assign_me_button": show_assign_me_button,
-            "show_remove_button": show_remove_button,
-            "selected_value_metadata": selected_value_metadata
-        };
-    },
-
-    _check_button_state: function(btn_class, is_visible) {
-        var assign_me_button = Y.one(btn_class);
-        Assert.isNotNull(assign_me_button);
-        if (is_visible) {
-            Assert.isFalse(
-                assign_me_button.hasClass('yui3-picker-hidden'),
-                btn_class + " should be visible but is hidden");
-        } else {
-            Assert.isTrue(
-                assign_me_button.hasClass('yui3-picker-hidden'),
-                btn_class + " should be hidden but is visible");
-        }
-    },
-
-    _check_assign_me_button_state: function(is_visible) {
-        this._check_button_state('.yui-picker-assign-me-button', is_visible);
-    },
-
-    _check_remove_button_state: function(is_visible) {
-        this._check_button_state('.yui-picker-remove-button', is_visible);
-    },
-
-    test_picker_assign_me_button_text: function() {
-        // The assign me button text is correct.
-        this.create_picker(this._picker_params(true, true));
-        this.picker.render();
-        var assign_me_button = Y.one('.yui-picker-assign-me-button');
-        Assert.areEqual('Assign Moi', assign_me_button.get('innerHTML'));
-    },
-
-    test_picker_remove_person_button_text: function() {
-        // The remove button text is correct.
-        this.create_picker(this._picker_params(true, true, "person"));
-        this.picker.render();
-        var remove_button = Y.one('.yui-picker-remove-button');
-        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
-    },
-
-    test_picker_remove_team_button_text: function() {
-        // The remove button text is correct.
-        this.create_picker(this._picker_params(true, true, "team"), this.ME);
-        this.picker.render();
-        var remove_button = Y.one('.yui-picker-remove-button');
-        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
-    },
-
-    test_picker_has_assign_me_button: function() {
-        // The assign me button is shown.
-        this.create_picker(this._picker_params(true, true));
-        this.picker.render();
-        this._check_assign_me_button_state(true);
-    },
-
-    test_picker_no_assign_me_button_unless_configured: function() {
-        // The assign me button is only rendered if show_assign_me_button
-        // config setting is true.
-        this.create_picker(this._picker_params(false, true));
-        this.picker.render();
-        Assert.isNull(Y.one('.yui-picker-assign-me-button'));
-    },
-
-    test_picker_no_assign_me_button_if_value_is_me: function() {
-        // The assign me button is not shown if the picker is created for a
-        // field where the value is "me".
-        this.create_picker(this._picker_params(true, true), this.ME);
-        this.picker.render();
-        this._check_assign_me_button_state(false);
-    },
-
-    test_picker_assign_me_button_hide_on_save: function() {
-        // The assign me button is shown initially but hidden if the picker
-        // saves a value equal to 'me'.
-        this.create_picker(this._picker_params(true, true));
-        this._check_assign_me_button_state(true);
-        this.picker.set('results', this.vocabulary);
-        this.picker.render();
-        simulate(
-            this.picker.get('boundingBox').one('.yui3-picker-results'),
-                'li:nth-child(1)', 'click');
-        this._check_assign_me_button_state(false);
-    },
-
-    test_picker_no_remove_button_if_null_value: function() {
-        // The remove button is not shown if the picker is created for a field
-        // which has a null value.
-        this.create_picker(this._picker_params(true, true));
-        this.picker.render();
-        this._check_remove_button_state(false);
-    },
-
-    test_picker_has_remove_button_if_value: function() {
-        // The remove button is shown if the picker is created for a field
-        // which has a value.
-        this.create_picker(this._picker_params(true, true), this.ME);
-        this.picker.render();
-        this._check_remove_button_state(true);
-    },
-
-    test_picker_no_remove_button_unless_configured: function() {
-        // The remove button is only rendered if show_remove_button setting is
-        // true.
-        this.create_picker(this._picker_params(true, false), this.ME);
-        this.picker.render();
-        Assert.isNull(Y.one('.yui-picker-remove-button'));
-    },
-
-    test_picker_remove_button_clicked: function() {
-        // The remove button is hidden once a picker value has been removed.
-        // And the assign me button is shown.
-        this.create_picker(this._picker_params(true, true), this.ME);
-        this.picker.render();
-        var remove = Y.one('.yui-picker-remove-button');
-        remove.simulate('click');
-        this._check_remove_button_state(false);
-        this._check_assign_me_button_state(true);
-    },
-
-    test_picker_assign_me_button_clicked: function() {
-        // The assign me button is hidden once it is clicked.
-        // And the remove button is shown.
-        this.create_picker(this._picker_params(true, true));
-        this.picker.render();
-        var assign_me = Y.one('.yui-picker-assign-me-button');
-        assign_me.simulate('click');
-        this._check_remove_button_state(true);
-        this._check_assign_me_button_state(false);
-    },
-
-    test_picker_assign_me_updates_remove_text: function() {
-        // When Assign me is used, the Remove button text is updated from
-        // the team removal text to the person removal text.
-        this.create_picker(this._picker_params(true, true, "team"), this.ME);
-        this.picker.render();
-        var remove_button = Y.one('.yui-picker-remove-button');
-        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
-        var assign_me = Y.one('.yui-picker-assign-me-button');
-        assign_me.simulate('click');
-        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
-    },
-
-    test_picker_save_updates_remove_text: function() {
-        // When save is called, the Remove button text is updated according to
-        // the newly saved value.
-        this.create_picker(this._picker_params(true, true), this.ME);
-        var remove_button = Y.one('.yui-picker-remove-button');
-        Assert.areEqual('Remove someone', remove_button.get('innerHTML'));
-        this.picker.set('results', this.vocabulary);
-        this.picker.render();
-        simulate(
-            this.picker.get('boundingBox').one('.yui3-picker-results'),
-                'li:nth-child(2)', 'click');
-        Assert.areEqual('Remove some team', remove_button.get('innerHTML'));
-    }
-}));
-
-/*
  * Test cases for a picker with a large vocabulary.
  */
 suite.add(new Y.Test.Case({

=== modified file 'lib/lp/app/widgets/popup.py'
--- lib/lp/app/widgets/popup.py	2011-07-19 03:27:34 +0000
+++ lib/lp/app/widgets/popup.py	2011-07-21 05:12:40 +0000
@@ -105,6 +105,17 @@
                          class="%(cssClass)s" />""" % d
 
     @property
+    def selected_value(self):
+        """ String representation of field value associated with the picker.
+
+        Default implementation is to return the 'name' attribute.
+        """
+        val = self._getFormValue()
+        if val is not None and hasattr(val, 'name'):
+            return getattr(val, 'name')
+        return None
+
+    @property
     def selected_value_metadata(self):
         return None
 
@@ -116,6 +127,7 @@
     def config(self):
         return dict(
             picker_type=self.picker_type,
+            selected_value=self.selected_value,
             selected_value_metadata=self.selected_value_metadata,
             header=self.header_text, step_title=self.step_title_text,
             extra_no_results_message=self.extra_no_results_message,
@@ -190,7 +202,7 @@
 
     include_create_team_link = False
     show_assign_me_button = 'true'
-    show_remove_button = 'true'
+    show_remove_button = 'false'
 
     @property
     def picker_type(self):

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-07-19 14:33:39 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-07-21 05:12:40 +0000
@@ -3609,6 +3609,8 @@
             'bugtask_path': '/'.join(
                 [''] + canonical_url(self.context).split('/')[3:]),
             'prefix': get_prefix(self.context),
+            'assignee_value': self.context.assignee
+                and self.context.assignee.name,
             'assignee_is_team': self.context.assignee
                 and self.context.assignee.is_team,
             'assignee_vocabulary': assignee_vocabulary,

=== modified file 'lib/lp/bugs/browser/widgets/bugtask.py'
--- lib/lp/bugs/browser/widgets/bugtask.py	2011-06-14 20:43:01 +0000
+++ lib/lp/bugs/browser/widgets/bugtask.py	2011-07-21 05:12:40 +0000
@@ -58,7 +58,10 @@
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.app.widgets.helpers import get_widget_template
 from lp.app.widgets.itemswidgets import LaunchpadRadioWidget
-from lp.app.widgets.popup import VocabularyPickerWidget
+from lp.app.widgets.popup import (
+    PersonPickerWidget,
+    VocabularyPickerWidget,
+    )
 from lp.app.widgets.textwidgets import (
     StrippedTextWidget,
     URIWidget,
@@ -90,7 +93,7 @@
         #
         # See zope.app.form.interfaces.IInputWidget.
         self.required = False
-        self.assignee_chooser_widget = VocabularyPickerWidget(
+        self.assignee_chooser_widget = PersonPickerWidget(
             context, context.vocabulary, request)
         self.setUpNames()
 

=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js	2011-07-19 14:42:52 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js	2011-07-21 05:12:40 +0000
@@ -862,8 +862,11 @@
         // an assignee. We check to see if an assignee is a contributor and if
         // they are not, the user is asked to confirm their selection.
         var validate_assignee = function(picker, value, save_fn, cancel_fn) {
-            if (value === null) {
-                return true;
+            if (value === null || value.api_uri == null) {
+                if (Y.Lang.isFunction(save_fn)) {
+                    save_fn();
+                    return;
+                }
             }
             var assignee_uri = Y.lp.client.normalize_uri(value.api_uri);
             assignee_uri = Y.lp.client.get_absolute_uri(assignee_uri);
@@ -923,8 +926,10 @@
             conf.bugtask_path,
             "assignee_link",
             assignee_content.get('id'),
-            {"step_title": step_title,
+            {"picker_type": "person",
+             "step_title": step_title,
              "header": "Change assignee",
+             "selected_value": conf.assignee_value,
              "selected_value_metadata": conf.assignee_is_team?"team":"person",
              "assign_me_text": "Assign me",
              "remove_person_text": "Remove assignee",