← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jcsackett/launchpad/cleanup-pickers-with-base-create into lp:launchpad

 

j.c.sackett has proposed merging lp:~jcsackett/launchpad/cleanup-pickers-with-base-create into lp:launchpad.

Requested reviews:
  Richard Harding (rharding)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/cleanup-pickers-with-base-create/+merge/110921

Summary
=======
The picker javascript code is a mess of competing styles, showing its long
history in the LP code tree.

As part of general tech-debt cleanup and updating our JS to more modern uses
of YUI, we need to start cleaning this up. Using Y.Base.create where possible
is one step in so doing.

Preimp
======
Spoke with Rick Harding about c-style constants and how best to replace them.

Implementation
==============
A bunch of constants, seemingly influenced by c-style coding have been removed
from the base picker.

The base picker, as well as the person picker, have been updated to use
Y.Base.create.

Both have been updated to define their own render/bind/syncUI methods rather
than chaining event calls after the normal cycle--this is in accordance with
standard means of writing yui widgets.

A number of attributes have been created as part of the ATTR block for both,
simplifying the initializers. More of this could probably be done, but the
diff is getting big, and this will not be my last branch on this effort.

Tests
=====
bin/test -vvct picker --layer=YUI

QA
==
Go play with all instances of the picker, esp the person picker. They should
function as they always have.

LoC
===
This removes 100+ LoC.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/picker/tests/test_personpicker.html
  lib/lp/app/javascript/picker/person_picker.js
  lib/lp/app/javascript/picker/tests/test_personpicker.js
  lib/lp/app/javascript/picker/picker_patcher.js
  lib/lp/app/javascript/picker/picker.js
-- 
https://code.launchpad.net/~jcsackett/launchpad/cleanup-pickers-with-base-create/+merge/110921
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.
=== modified file 'lib/lp/app/javascript/picker/person_picker.js'
--- lib/lp/app/javascript/picker/person_picker.js	2011-09-19 00:35:49 +0000
+++ lib/lp/app/javascript/picker/person_picker.js	2012-06-18 21:59:29 +0000
@@ -6,73 +6,36 @@
  */
 YUI.add('lazr.person-picker', function(Y) {
 
+var ns = Y.namespace('lazr.picker');
 /*
  * Extend the picker into the PersonPicker
  */
-var PersonPicker;
-
-PersonPicker = function() {
-    PersonPicker.superclass.constructor.apply(this, arguments);
-    this._extra_buttons = Y.Node.create('<div class="extra-form-buttons"/>');
-    Y.after(this._renderPersonPickerUI, this, 'renderUI');
-};
-
-Y.extend(PersonPicker, Y.lazr.picker.Picker, {
+
+ns.PersonPicker = Y.Base.create('picker', Y.lazr.picker.Picker, [], {
+
     initializer: function(cfg) {
-        PersonPicker.superclass.initializer.apply(this, arguments);
+        Y.lazr.picker.Picker.prototype.initializer.apply(this, arguments);
 
-        var show_assign_me_button = true;
-        var show_remove_button = true;
-        var assign_me_text = 'Pick me';
-        var remove_person_text = 'Remove person';
-        var remove_team_text = 'Remove team';
-        var min_search_chars = 2;
-        if (cfg !== undefined) {
-            if (cfg.show_assign_me_button !== undefined) {
-                show_assign_me_button = cfg.show_assign_me_button;
-            }
-            if (cfg.show_remove_button !== undefined) {
-                show_remove_button = cfg.show_remove_button;
-            }
-            if (cfg.assign_me_text !== undefined) {
-                assign_me_text = cfg.assign_me_text;
-            }
-            if (cfg.remove_person_text !== undefined) {
-                remove_person_text = cfg.remove_person_text;
-            }
-            if (cfg.remove_team_text !== undefined) {
-                remove_team_text = cfg.remove_team_text;
-            }
-            if (cfg.min_search_chars !== undefined) {
-                min_search_chars = cfg.min_search_chars;
-            }
+        // If the user isn't logged in, override the show_assign_me value.
+        if (!Y.Lang.isValue(LP.links.me)) {
+            this.set('show_assign_me_button', false); 
         }
-        this.set('min_search_chars', min_search_chars);
-
-        /* Only show assign-me when the user is logged-in. */
-        show_assign_me_button = (
-            show_assign_me_button && Y.Lang.isValue(LP.links.me));
-        this._show_assign_me_button = show_assign_me_button;
-        this._show_remove_button = show_remove_button;
-        this._assign_me_text = assign_me_text;
-        this._remove_person_text = remove_person_text;
-        this._remove_team_text = remove_team_text;
     },
 
     hide: function() {
         this.get('boundingBox').setStyle('display', 'none');
-        this.constructor.superclass.hide.call(this);
+        Y.lazr.picker.Picker.prototype.hide.call(this);
     },
 
     show: function() {
         this.get('boundingBox').setStyle('display', 'block');
-        this.constructor.superclass.show.call(this);
+        Y.lazr.picker.Picker.prototype.show.call(this);
     },
 
     _update_button_text: function() {
-        var link_text = this._remove_person_text;
+        var link_text = this.get('remove_person_text');
         if (this.get('selected_value_metadata') === 'team') {
-            link_text = this._remove_team_text;
+            link_text = this.get('remove_team_text');
         }
         this.remove_button.set('text', link_text);
     },
@@ -100,12 +63,12 @@
 
     remove: function () {
         this.hide();
-        this.fire(Y.lazr.picker.Picker.SAVE, {value: null});
+        this.fire('save', {value: null});
     },
 
     assign_me: function () {
         var name = LP.links.me.replace('/~', '');
-        this.fire(Y.lazr.picker.Picker.SAVE, {
+        this.fire('save', {
             image: '/@@/person',
             title: 'Me',
             api_uri: LP.links.me,
@@ -113,40 +76,52 @@
         });
     },
 
-    _renderPersonPickerUI: function() {
+    renderUI: function() {
+        Y.lazr.picker.Picker.prototype.renderUI.apply(this, arguments);
+        var extra_buttons = this.get('extra_buttons');
         var remove_button, assign_me_button;
 
-        if (this._show_remove_button) {
+        if (this.get('show_remove_button')) {
             remove_button = Y.Node.create(
                 '<a class="yui-picker-remove-button bg-image" ' +
                 'href="javascript:void(0)" ' +
                 'style="background-image: url(/@@/remove); padding-right: ' +
-                '1em">' + this._remove_person_text + '</a>');
+                '1em">' + this.get('remove_person_text') + '</a>');
             remove_button.on('click', this.remove, this);
-            this._extra_buttons.appendChild(remove_button);
+            extra_buttons.appendChild(remove_button);
             this.remove_button = remove_button;
         }
 
-        if (this._show_assign_me_button) {
+        if (this.get('show_assign_me_button')) {
             assign_me_button = Y.Node.create(
                 '<a class="yui-picker-assign-me-button bg-image" ' +
                 'href="javascript:void(0)" ' +
                 'style="background-image: url(/@@/person)">' +
-                this._assign_me_text + '</a>');
+                this.get('assign_me_text') + '</a>');
             assign_me_button.on('click', this.assign_me, this);
-            this._extra_buttons.appendChild(assign_me_button);
+            extra_buttons.appendChild(assign_me_button);
             this.assign_me_button = assign_me_button;
         }
         this._search_input.insert(
-            this._extra_buttons, this._search_input.get('parentNode'));
+            extra_buttons, this._search_input.get('parentNode'));
         this._show_hide_buttons();
         this.after("selected_valueChange", function(e) {
             this._show_hide_buttons();
         });
     }
+}, {
+    ATTRS: {
+        extra_buttons: {
+            valueFn: function () {
+                return Y.Node.create('<div class="extra-form-buttons"/>')
+            } 
+        },
+        show_assign_me_button: { value: true },
+        show_remove_button: {value: true },
+        assign_me_text: {value: 'Pick me'},
+        remove_person_text: {value: 'Remove person'},
+        remove_team_text: {value: 'Remove team'},
+        min_search_chars: {value: 2}
+    }
 });
-PersonPicker.NAME = 'picker';
-var namespace = Y.namespace('lazr.picker');
-namespace.PersonPicker = PersonPicker;
-
-}, "0.1", {"requires": ["lazr.picker"]});
+}, "0.1", {"requires": ["base", "node", "lazr.picker"]});

=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js	2012-03-22 22:38:09 +0000
+++ lib/lp/app/javascript/picker/picker.js	2012-06-18 21:59:29 +0000
@@ -1,6 +1,7 @@
 /* Copyright (c) 2009, Canonical Ltd. All rights reserved. */
 
 YUI.add('lazr.picker', function(Y) {
+var ns = Y.namespace('lazr.picker');
 
 /**
  * Module containing the Lazr searchable picker.
@@ -9,156 +10,91 @@
  * @namespace lazr.picker
  */
 
+
+// Alias getClassName to minimize line wrapping gymnastics later.
+var getCN = Y.ClassNameManager.getClassName;
+
 /**
  * A picker is a pop-up widget containing a search field and displaying a list
  * of found results.
- *
- * @class Picker
- * @extends lazr.PrettyOverlay
- * @constructor
  */
-
-var PICKER  = 'picker',
-    BOUNDING_BOX = 'boundingBox',
-    CONTENT_BOX  = 'contentBox',
-
-    // Local aliases
-    getCN = Y.ClassNameManager.getClassName,
-
-    // CSS Classes
-    C_SEARCH = getCN(PICKER, 'search'),
-    C_SEARCH_BOX = getCN(PICKER, 'search-box'),
-    C_SEARCH_SLOT = getCN(PICKER, 'search-slot'),
-    C_FOOTER_SLOT = getCN(PICKER, 'footer-slot'),
-    C_SEARCH_MODE = getCN(PICKER, 'search-mode'),
-    C_FILTER = getCN(PICKER, 'filter'),
-    C_RESULTS = getCN(PICKER, 'results'),
-    C_RESULT_TITLE = getCN(PICKER, 'result-title'),
-    C_RESULT_DESCRIPTION = getCN(PICKER, 'result-description'),
-    C_ERROR = getCN(PICKER, 'error'),
-    C_ERROR_MODE = getCN(PICKER, 'error-mode'),
-    C_NO_RESULTS = getCN(PICKER, 'no-results'),
-    C_BATCHES = getCN(PICKER, 'batches'),
-    C_SELECTED_BATCH = getCN(PICKER, 'selected-batch'),
-
-    // Events
-    SAVE = 'save',
-    SEARCH = 'search',
-    VALIDATE = 'validate',
-
-    // Property constants
-    MIN_SEARCH_CHARS = 'min_search_chars',
-    CURRENT_SEARCH_STRING = 'current_search_string',
-    ERROR = 'error',
-    RESULTS = 'results',
-    BATCHES = 'batches',
-    BATCH_COUNT = 'batch_count',
-    SEARCH_SLOT = 'search_slot',
-    FOOTER_SLOT = 'footer_slot',
-    SELECTED_BATCH = 'selected_batch',
-    SEARCH_MODE = 'search_mode',
-    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',
-    FILTER_OPTIONS = 'filter_options',
-    CURRENT_FILTER_VALUE = 'current_filter_value';
-
-
-var Picker;
-Picker = function () {
-    Picker.superclass.constructor.apply(this, arguments);
-
-    Y.after(this._renderUIPicker, this, RENDERUI);
-    Y.after(this._bindUIPicker, this, BINDUI);
-    Y.after(this._syncUIPicker, this, BINDUI);
-};
-
-Y.extend(Picker, Y.lazr.PrettyOverlay, {
-    /**
-     * The search input box node.
-     *
-     * @property _search_input
-     * @type Node
-     * @private
-     */
-    _search_input: null,
-
-    /**
-     * The search button node.
-     *
-     * @property _search_button
-     * @type Node
-     * @private
-     */
-    _search_button: null,
-
-    /**
-     * The node containing filter options.
-     *
-     * @property _filter_box
-     * @type Node
-     * @private
-     */
-    _filter_box: null,
-
-    /**
-     * The node containing search results.
-     *
-     * @property _results_box
-     * @type Node
-     * @private
-     */
-    _results_box: null,
-
-    /**
-     * The node containing the extra form inputs.
-     *
-     * @property _search_slot_box
-     * @type Node
-     * @private
-     */
-    _search_slot_box: null,
-
-    /**
-     * The node containing the batches.
-     *
-     * @property _batches_box
-     * @type Node
-     * @private
-     */
-    _batches_box: null,
-
-    /**
-     * The node containing the previous batch button.
-     *
-     * @property _prev_button
-     * @type Node
-     * @private
-     */
-    _prev_button: null,
-
-    /**
-     * The node containing the next batch button.
-     *
-     * @property _next_button
-     * @type Node
-     * @private
-     */
-    _next_button: null,
-
-    /**
-     * The node containing an error message if any.
-     *
-     * @property _error_box
-     * @type Node
-     * @private
-     */
-    _error_box: null,
+ns.Picker = Y.Base.create('picker', Y.lazr.PrettyOverlay, [], {
 
     initializer: function(cfg) {
+        this._search_input = null;
+
+        /**
+         * The search button node.
+         *
+         * @property _search_button
+         * @type Node
+         * @private
+         */
+        this._search_button = null;
+
+        /**
+         * The node containing filter options.
+         *
+         * @property _filter_box
+         * @type Node
+         * @private
+         */
+        this._filter_box = null;
+
+        /**
+         * The node containing search results.
+         *
+         * @property _results_box
+         * @type Node
+         * @private
+         */
+        this._results_box = null;
+
+        /**
+         * The node containing the extra form inputs.
+         *
+         * @property _search_slot_box
+         * @type Node
+         * @private
+         */
+        this._search_slot_box = null;
+
+        /**
+         * The node containing the batches.
+         *
+         * @property _batches_box
+         * @type Node
+         * @private
+         */
+         this._batches_box = null;
+
+        /**
+         * The node containing the previous batch button.
+         *
+         * @property _prev_button
+         * @type Node
+         * @private
+         */
+        this._prev_button = null;
+
+        /**
+         * The node containing the next batch button.
+         *
+         * @property _next_button
+         * @type Node
+         * @private
+         */
+        this._next_button = null;
+
+        /**
+         * The node containing an error message if any.
+         *
+         * @property _error_box
+         * @type Node
+         * @private
+         */
+        this._error_box = null;
+
         /**
          * Fires when the user presses the 'Search' button.
          * The event details contain the search string entered by the user.
@@ -173,7 +109,7 @@
          * @event search
          * @preventable _defaultSearch
          */
-        this.publish(SEARCH, { defaultFn: this._defaultSearch });
+        this.publish('search', { defaultFn: this._defaultSearch });
 
         /**
          * Fires when the user selects one of the result. The event details
@@ -182,7 +118,7 @@
          * @event validate
          * @preventable _defaultValidate
          */
-        this.publish(VALIDATE, { defaultFn: this._defaultValidate } );
+        this.publish('validate', { defaultFn: this._defaultValidate } );
 
         /**
          * Fires on successful validation of the selected result.
@@ -192,7 +128,7 @@
          * @event save
          * @preventable _defaultSave
          */
-        this.publish(SAVE, { defaultFn: this._defaultSave } );
+        this.publish('save', { defaultFn: this._defaultSave } );
 
 
         // Subscribe to the cancel event so that we can clear the widget when
@@ -210,26 +146,26 @@
 
         if (!Y.Lang.isUndefined(cfg)) {
             // The picker's associated field.
-            if (Y.Lang.isValue(cfg[ASSOCIATED_FIELD_ID])) {
+            if (Y.Lang.isValue(cfg.associated_field_id)) {
                 this.plug(TextFieldPickerPlugin,
                             {input_element:
-                                '[id="'+cfg[ASSOCIATED_FIELD_ID]+'"]'});
+                                '[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])) {
+            if (Y.Lang.isValue(cfg.selected_value_metadata)) {
                 this.set(
-                    SELECTED_VALUE_METADATA, cfg[SELECTED_VALUE_METADATA]);
+                    '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]);
+            if (Y.Lang.isValue(cfg.selected_value)) {
+                this.set('selected_value', cfg.selected_value);
             }
             // Optional filter support
-            if (Y.Lang.isValue(cfg[FILTER_OPTIONS])) {
-                this.set(FILTER_OPTIONS, cfg[FILTER_OPTIONS]);
-                if (Y.Lang.isValue(cfg[CURRENT_FILTER_VALUE])) {
-                    this.set(CURRENT_FILTER_VALUE, cfg[CURRENT_FILTER_VALUE]);
+            if (Y.Lang.isValue(cfg.filter_options)) {
+                this.set('filter_options', cfg.filter_options);
+                if (Y.Lang.isValue(cfg.current_filter_value)) {
+                    this.set('current_filter_value', cfg.current_filter_value);
                 }
             }
         }
@@ -242,7 +178,7 @@
      * @protected
      */
     _syncSearchSlotUI: function() {
-        var search_slot = this.get(SEARCH_SLOT);
+        var search_slot = this.get('search_slot');
 
         // Clear previous slot contents.
         this._search_slot_box.set('innerHTML', '');
@@ -259,7 +195,7 @@
      * @protected
      */
     _syncFooterSlotUI: function() {
-        var footer_slot = this.get(FOOTER_SLOT);
+        var footer_slot = this.get('footer_slot');
 
         // Clear previous slot contents.
         this._footer_slot_box.set('innerHTML', '');
@@ -276,10 +212,10 @@
      * @protected
      */
     _getBatches: function() {
-        var batches = this.get(BATCHES);
+        var batches = this.get('batches');
 
         if (batches === null) {
-            var batch_count = this.get(BATCH_COUNT);
+            var batch_count = this.get('batch_count');
             if (batch_count === null) {
                 batches = [];
             }
@@ -320,10 +256,10 @@
         // in _syncSelectedBatchUI.
         this._prev_button = Y.Node.create(Y.lazr.ui.PREVIOUS_BUTTON);
         this._prev_button.on('click', function (e) {
-            var selected = this.get(SELECTED_BATCH) - 1;
-            this.set(SELECTED_BATCH, selected);
+            var selected = this.get('selected_batch') - 1;
+            this.set('selected_batch', selected);
             this.fire(
-                SEARCH, this.get(CURRENT_SEARCH_STRING),
+                'search', this.get('current_search_string'),
                 batches[selected].value);
         }, this);
         this._batches_box.appendChild(this._prev_button);
@@ -335,19 +271,19 @@
             this._batches_box.appendChild(batch_item);
 
             batch_item.on('click', function (e) {
-                this.set(SELECTED_BATCH, i);
+                this.set('selected_batch', i);
                 this.fire(
-                    SEARCH, this.get(CURRENT_SEARCH_STRING), data.value);
+                    'search', this.get('current_search_string'), data.value);
             }, this);
         }, this);
 
         this._next_button = Y.Node.create(Y.lazr.ui.NEXT_BUTTON);
         this._batches_box.appendChild(this._next_button);
         this._next_button.on('click', function (e) {
-            var selected = this.get(SELECTED_BATCH) + 1;
-            this.set(SELECTED_BATCH, selected);
+            var selected = this.get('selected_batch') + 1;
+            this.set('selected_batch', selected);
             this.fire(
-                SEARCH, this.get(CURRENT_SEARCH_STRING),
+                'search', this.get('current_search_string'),
                 batches[selected].value);
         }, this);
     },
@@ -359,12 +295,13 @@
      * @protected
      */
     _syncSelectedBatchUI: function() {
-        var idx = this.get(SELECTED_BATCH);
+        var idx = this.get('selected_batch');
         var items = this._batches_box.all('span');
         if (items.size()) {
+            var selected_batch_class = getCN('picker', 'selected-batch');
             this._prev_button.set('disabled', idx === 0);
-            items.removeClass(C_SELECTED_BATCH);
-            items.item(idx).addClass(C_SELECTED_BATCH);
+            items.removeClass(selected_batch_class);
+            items.item(idx).addClass(selected_batch_class);
             this._next_button.set('disabled', idx+1 === items.size());
         }
     },
@@ -403,7 +340,7 @@
      */
     _renderTitleUI: function(data) {
         var li_title = Y.Node.create('<a href="#"></a>')
-            .addClass(C_RESULT_TITLE)
+            .addClass(getCN('picker', 'result-title'))
             .addClass('js-action');
         li_title.on('click', function (e, value) {
                 e.preventDefault();
@@ -486,7 +423,8 @@
      */
     _renderDescriptionUI: function(data) {
         var li_desc = Y.Node.create(
-            '<div><br /></div>').addClass(C_RESULT_DESCRIPTION);
+            '<div><br /></div>').addClass(
+                getCN('picker', 'result-description'));
         if (data.description) {
             li_desc.replaceChild(
                 document.createTextNode(data.description),
@@ -505,7 +443,7 @@
         }
         var details_node = Y.Node.create('<div></div>')
             .addClass('sprite')
-            .addClass(C_RESULT_DESCRIPTION);
+            .addClass(getCN('picker', 'result-description'));
         if (Y.Lang.isArray(data.details)) {
             var data_node = Y.Node.create('<div></div>');
             var escaped_details = [];
@@ -542,7 +480,7 @@
                 .set('text', 'Select ' + data.title));
         links[0].on('click', function (e, value) {
             e.preventDefault();
-            this.fire(VALIDATE, value);
+            this.fire('validate', value);
             }, this, data);
         links.push(this._text_or_link(
             'View details', data.alt_title_link, data.link_css));
@@ -564,14 +502,14 @@
      * @protected
      */
     _syncResultsUI: function() {
-        var results = this.get(RESULTS);
+        var results = this.get('results');
 
         // Remove any previous results.
         Y.Event.purgeElement(this._results_box, true);
         this._results_box.set('innerHTML', '');
         this._filter_box.set('innerHTML', '');
 
-        var expander_id = this.get(BOUNDING_BOX).get('id');
+        var expander_id = this.get('boundingBox').get('id');
         Y.Array.each(results, function(data, i) {
             // Sort out the badges div.
             var li_badges = this._renderTitleBadgesUI(data);
@@ -606,12 +544,12 @@
                     li_desc, li_details, {group_id: expander_id});
                 li.expander.setUp(true);
                 li_title.on('click', function (e, value) {
-                    this.fire(VALIDATE, value);
+                    this.fire('validate', value);
                     }, this, data);
             } else {
                 // Attach implicit valdate/save handler.
                 li.on('click', function (e, value) {
-                    this.fire(VALIDATE, value);
+                    this.fire('validate', value);
                     }, this, data);
             }
 
@@ -620,23 +558,24 @@
 
         // If the user has entered a search and there ain't no results,
         // display the message about no items matching.
+        var no_results_class = getCN('picker', 'no-results');
         if (this._search_input.get('value') && !results.length) {
             var msg = Y.Node.create('<li></li>');
             msg.appendChild(
                 document.createTextNode(
-                    Y.substitute(this.get(NO_RESULTS_SEARCH_MESSAGE),
+                    Y.substitute(this.get('no_results_search_message'),
                     {query: this._search_input.get('value')})));
             this._results_box.appendChild(msg);
-            this._results_box.addClass(C_NO_RESULTS);
+            this._results_box.addClass(no_results_class);
             this._syncFilterUI();
         } else {
-            this._results_box.removeClass(C_NO_RESULTS);
+            this._results_box.removeClass(no_results_class);
             if (results.length) {
-                var filters = this.get(FILTER_OPTIONS);
-                var current_filter_value = this.get(CURRENT_FILTER_VALUE);
+                var filters = this.get('filter_options');
+                var current_filter_value = this.get('current_filter_value');
                 if (filters.length  > 0 &&
                         !Y.Lang.isValue(current_filter_value)) {
-                    this.set(CURRENT_FILTER_VALUE, filters[0].title);
+                    this.set('current_filter_value', filters[0].title);
                 }
                 this._syncFilterUI();
             }
@@ -650,7 +589,7 @@
      * @protected
      */
     _syncProgressUI: function() {
-        var results = this.get(RESULTS);
+        var results = this.get('results');
         if (results.length) {
             // Set PrettyOverlay's green progress bar to 100%.
             this.set('progress', 100);
@@ -666,11 +605,11 @@
      */
     _syncFilterUI: function() {
         // Check that we need to display the filter UI.
-        var filters = this.get(FILTER_OPTIONS);
+        var filters = this.get('filter_options');
         if( filters.length === 0 ) {
             return;
         }
-        var current_filter_value = this.get(CURRENT_FILTER_VALUE);
+        var current_filter_value = this.get('current_filter_value');
         if (!Y.Lang.isValue(current_filter_value)) {
             return;
         }
@@ -696,7 +635,7 @@
                 // event.
                 filter_link.on('click', function (e) {
                     e.halt();
-                    picker.set(CURRENT_FILTER_VALUE, filter.title);
+                    picker.set('current_filter_value', filter.title);
                     var search_string = Y.Lang.trim(
                         picker._search_input.get('value'));
                     picker._performSearch(search_string, filter.name);
@@ -719,13 +658,14 @@
      * @protected
      */
     _syncSearchModeUI: function() {
-        var search_mode = this.get(SEARCH_MODE);
+        var search_mode = this.get('search_mode');
         this._search_input.set('disabled', search_mode);
         this._search_button.set('disabled', search_mode);
+        var search_class = getCN('picker', 'search-mode');
         if (search_mode) {
-            this.get(BOUNDING_BOX).addClass(C_SEARCH_MODE);
+            this.get('boundingBox').addClass(search_class);
         } else {
-            this.get(BOUNDING_BOX).removeClass(C_SEARCH_MODE);
+            this.get('boundingBox').removeClass(search_class);
             // If the search input isn't blurred before it is focused,
             // then the I-beam disappears.
             this._search_input.blur();
@@ -740,27 +680,23 @@
      * @protected
      */
     _syncErrorUI: function() {
-        var error = this.get(ERROR);
+        var error = this.get('error');
         this._error_box.set('innerHTML', '');
+        var error_class = getCN('picker', 'error-mode');
         if (error === null) {
-            this.get(BOUNDING_BOX).removeClass(C_ERROR_MODE);
+            this.get('boundingBox').removeClass(error_class);
         } else {
             this._error_box.appendChild(document.createTextNode(error));
-            this.get(BOUNDING_BOX).addClass(C_ERROR_MODE);
+            this.get('boundingBox').addClass(error_class);
         }
     },
 
     /**
      * Create the widget's HTML components.
-     * <p>
-     * This method is invoked after renderUI is invoked for the Widget class
-     * using YUI's aop infrastructure.
-     * </p>
      *
-     * @method _renderUIPicker
-     * @protected
+     * @method renderUI
      */
-    _renderUIPicker: function() {
+    renderUI: function() {
         this._search_button = Y.Node.create(Y.lazr.ui.SEARCH_BUTTON);
 
         var search_box = Y.Node.create([
@@ -770,31 +706,31 @@
             '<div></div></div>'].join(""));
 
         this._search_input = search_box.one('input');
-        this._search_input.addClass(C_SEARCH);
+        this._search_input.addClass(getCN('picker', 'search'));
 
         this._error_box = search_box.one('div');
-        this._error_box.addClass(C_ERROR);
+        this._error_box.addClass(getCN('picker', 'error'));
 
         // The search button is floated right to avoid problems with
         // the input width in Safari 3.
         search_box.insertBefore(this._search_button, this._search_input);
-        search_box.addClass(C_SEARCH_BOX);
+        search_box.addClass(getCN('picker', 'search-box'));
 
         this._search_slot_box = Y.Node.create('<div></div>');
-        this._search_slot_box.addClass(C_SEARCH_SLOT);
+        this._search_slot_box.addClass(getCN('picker', 'search-slot'));
         search_box.appendChild(this._search_slot_box);
 
         this._filter_box = Y.Node.create('<div></div>');
-        this._filter_box.addClass(C_FILTER);
+        this._filter_box.addClass(getCN('picker', 'filter'));
 
         this._results_box = Y.Node.create('<ul></ul>');
-        this._results_box.addClass(C_RESULTS);
+        this._results_box.addClass(getCN('picker', 'results'));
 
         this._batches_box = Y.Node.create('<div></div>');
-        this._batches_box.addClass(C_BATCHES);
+        this._batches_box.addClass(getCN('picker', 'batches'));
 
         this._footer_slot_box = Y.Node.create('<div></div>');
-        this._footer_slot_box.addClass(C_FOOTER_SLOT);
+        this._footer_slot_box.addClass(getCN('picker', 'footer-slot'));
 
         var body = Y.Node.create('<div></div>');
         body.appendChild(search_box);
@@ -810,15 +746,10 @@
 
     /**
      * Bind the widget's DOM elements to their event handlers.
-     * <p>
-     * This method is invoked after bindUI is invoked for the Widget class
-     * using YUI's aop infrastructure.
-     * </p>
      *
-     * @method _bindUIPicker
-     * @protected
+     * @method bindUI
      */
-    _bindUIPicker: function() {
+    bindUI: function() {
         Y.on('click', this._defaultSearchUserAction, this._search_button,
              this);
 
@@ -845,7 +776,7 @@
         this.after('resultsChange', function (e) {
             this._syncResultsUI();
             this._syncProgressUI();
-            this.set(SEARCH_MODE, false);
+            this.set('search_mode', false);
         }, this);
 
         // Update the search slot box whenever the "search_slot" property
@@ -888,15 +819,10 @@
 
     /**
      * Synchronize the search box, error message and results with the UI.
-     * <p>
-     * This method is invoked after syncUI is invoked for the Widget class
-     * using YUI's aop infrastructure.
-     * </p>
      *
-     * @method _syncUIPicker
-     * @protected
+     * @method syncUI
      */
-    _syncUIPicker: function() {
+    syncUI: function() {
         this._syncResultsUI();
         this._syncProgressUI();
         this._syncSearchModeUI();
@@ -914,13 +840,13 @@
      * @protected
      */
     _clear: function() {
-        this.set(CURRENT_SEARCH_STRING, '');
-        this.set(ERROR, '');
-        this.set(RESULTS, []);
-        this.set(BATCHES, null);
-        this.set(BATCH_COUNT, null);
-        this.set(SELECTED_BATCH, 0);
-        this.set(CURRENT_FILTER_VALUE, null);
+        this.set('current_search_string', '');
+        this.set('error', '');
+        this.set('results', []);
+        this.set('batches', null);
+        this.set('batch_count', null);
+        this.set('selected_batch', 0);
+        this.set('current_filter_value', null);
         this._search_input.set('value', '');
         this._results_box.set('innerHTML', '');
         this._filter_box.set('innerHTML', '');
@@ -936,7 +862,7 @@
      */
     _defaultSearchUserAction: function(e) {
         e.preventDefault();
-        this.set(CURRENT_FILTER_VALUE, null);
+        this.set('current_filter_value', null);
         var search_string = Y.Lang.trim(this._search_input.get('value'));
         this._performSearch(search_string);
     },
@@ -949,19 +875,19 @@
      * @param filter_name The name of a filter to use to limit the results.
      */
     _performSearch: function(search_string, filter_name) {
-        if (search_string.length < this.get(MIN_SEARCH_CHARS)) {
+        if (search_string.length < this.get('min_search_chars')) {
             var msg =  Y.substitute(
                 "Please enter at least {min} characters.",
-                {min: this.get(MIN_SEARCH_CHARS)});
-            this.set(ERROR, msg);
+                {min: this.get('min_search_chars')});
+            this.set('error', msg);
         } else {
             // Reset the selected batch for new searches.
-            var current_search_string = this.get(CURRENT_SEARCH_STRING);
+            var current_search_string = this.get('current_search_string');
             if (current_search_string !== search_string) {
-                this.set(SELECTED_BATCH, 0);
+                this.set('selected_batch', 0);
             }
-            this.set(CURRENT_SEARCH_STRING, search_string);
-            this.fire(SEARCH, search_string, undefined, undefined,
+            this.set('current_search_string', search_string);
+            this.fire('search', search_string, undefined, undefined,
                 filter_name);
         }
     },
@@ -975,8 +901,8 @@
      * @protected
      */
     _defaultSearch: function(e) {
-        this.set(ERROR, null);
-        this.set(SEARCH_MODE, true);
+        this.set('error', null);
+        this.set('search_mode', true);
     },
 
     /**
@@ -988,8 +914,8 @@
      * @protected
      */
     _defaultCancel : function(e) {
-        Picker.superclass._defaultCancel.apply(this, arguments);
-        this.set(SEARCH_MODE, false);
+        Y.lazr.PrettyOverlay.prototype._defaultCancel.apply(this, arguments);
+        this.set('search_mode', false);
         if ( this.get('clear_on_cancel') ) {
             this._clear();
         }
@@ -1020,7 +946,7 @@
      * @protected
      */
     _defaultValidate : function(e) {
-        this.fire(SAVE, e);
+        this.fire('save', e);
     },
 
     /**
@@ -1031,243 +957,213 @@
      * @protected
      */
     _defaultSelectBatch: function(e) {
-        this.set(SEARCH_MODE, true);
-    }
-    });
-
-/**
- * Some constants.
- */
-Picker.NAME = PICKER;
-Picker.SAVE = SAVE;
-Picker.SEARCH = SEARCH;
-Picker.VALIDATE = VALIDATE;
-
-/**
- * The details index of the save result.
- *
- * @static
- * @property SAVE_RESULT
- */
-Picker.SAVE_RESULT = 0;
-
-/**
- * The details index of the search string.
- *
- * @static
- * @property SEARCH_STRING
- */
-Picker.SEARCH_STRING = 0;
-
-/**
- * The details index of the selected batch value.
- *
- * @static
- * @property SELECTED_BATCH_VALUE
- */
-Picker.SELECTED_BATCH_VALUE = 1;
-
-
-Picker.ATTRS = {
-    /**
-     * Whether or not the search box and result list should be cleared when
-     * the save event is fired.
-     *
-     * @attribute clear_on_save
-     * @type Boolean
-     */
-    clear_on_save: { value: true },
-
-    /**
-     * Whether or not the search box and result list should be cleared when
-     * the cancel event is fired.
-     *
-     * @attribute clear_on_cancel
-     * @type Boolean
-     */
-    clear_on_cancel: { value: false },
-
-    /**
-     * A CSS selector for the DOM element that will activate (show) the picker
-     * once clicked.
-     *
-     * @attribute picker_activator
-     * @type String
-     */
-    picker_activator: { value: null },
-
-    /**
-     * An extra CSS class to be added to the picker_activator, generally used
-     * to distinguish regular links from js-triggering ones.
-     *
-     * @attribute picker_activator_css_class
-     * @type String
-     */
-    picker_activator_css_class: { value: 'js-action' },
-
-    /**
-     * Minimum number of characters that need to be entered in the search
-     * string input before a search event will be fired. The search string
-     * will be trimmed before testing the length.
-     *
-     * @attribute min_search_chars
-     * @type Integer
-     */
-    min_search_chars: { value: 3 },
-
-    /**
-     * The current search string, which is needed when clicking on a different
-     * batch if the search input has been modified.
-     *
-     * @attribute current_search_string
-     * @type String
-     */
-    current_search_string: {value: ''},
-
-    /**
-     * The string representation of the current filter.
-     *
-     * @attribute current_filter_value
-     * @type String
-     */
-    current_filter_value: {value: null},
-
-    /**
-     * A list of attribute name values used to construct the filtering options
-     * for this picker..
-     *
-     * @attribute filter_options
-     * @type Object
-     */
-    filter_options: {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.
-     *
-     * @attribute selected_value_metadata
-     * @type String
-     */
-    selected_value_metadata: {value: null},
-
-    /**
-     * Results currently displayed by the widget. Updating this value
-     * automatically updates the display.
-     *
-     * @attribute results
-     * @type Array
-     */
-    results: { value: [] },
-
-    /**
-     * This adds any form fields you want below the search field.
-     * Updating this value automatically updates the display, but only
-     * if the widget has already been rendered. Otherwise, the change
-     * event never fires.
-     *
-     * @attribute search_slot
-     * @type Node
-     */
-    search_slot: {value: null},
-
-    /**
-     * A place for custom html at the bottom of the widget. When there
-     * are no search results the search_slot and the footer_slot are
-     * right next to each other.
-     * Updating this value automatically updates the display, but only
-     * if the widget has already been rendered. Otherwise, the change
-     * event never fires.
-     *
-     * @attribute footer_slot
-     * @type Node
-     */
-    footer_slot: {value: null},
-
-    /**
-     * Batches currently displayed in the widget, which can be
-     * clicked to change the batch of results being displayed. Updating
-     * this value automatically updates the display.
-     *
-     * This an array of object containing the two keys, name (used as
-     * the batch label) and value (used as additional details to SEARCH
-     * event).
-     *
-     * @attribute batches
-     * @type Array
-     */
-    batches: {value: null},
-
-    /**
-     * For simplified batch creation, you can set this to the number of
-     * batches in the search results.  In this case, the batch labels
-     * and values are automatically calculated.  The batch name (used as the
-     * batch label) will be the batch number starting from 1.  The batch value
-     * (used as additional details to the SEARCH event) will be the batch
-     * number, starting from zero.
-     *
-     * If 'batches' is set (see above), batch_count is ignored.
-     *
-     * @attribute batch_count
-     * @type Integer
-     */
-    batch_count: {value: null},
-
-    /**
-     * Batch currently selected.
-     *
-     * @attribute selected_batch
-     * @type Integer
-     */
-    selected_batch: {
-        value: 0,
-        getter: function (value) {
-            return value || 0;
-        },
-        validator: function (value) {
-            var batches = this._getBatches();
-            return Y.Lang.isNumber(value) &&
-                   value >= 0 &&
-                   value < batches.length;
-        }},
-
-    /**
-     * Flag indicating if the widget is currently in search mode (so users
-     * has triggered a search and we are waiting for results.)
-     *
-     * @attribute search_mode
-     * @type Boolean
-     */
-    search_mode: { value: false },
-
-    /**
-     * The current error message. This puts the widget in 'error-mode',
-     * setting this value to null clears that state.
-     *
-     * @attribute error
-     * @type String
-     */
-    error: { value: null },
-
-    /**
-     * The message to display when the search returned no results. This string
-     * can contain a 'query' placeholder
-     *
-     * @attribute no_results_search_message
-     * @type String
-     * @default No items matched "{query}".
-     */
-    no_results_search_message: {
-        value: 'No items matched "{query}".'
-    }
-};
+        this.set('search_mode', true);
+    }
+}, {
+    ATTRS:  {
+        /**
+         * Whether or not the search box and result list should be cleared when
+         * the save event is fired.
+         *
+         * @attribute clear_on_save
+         * @type Boolean
+         */
+        clear_on_save: { value: true },
+
+        /**
+         * Whether or not the search box and result list should be cleared when
+         * the cancel event is fired.
+         *
+         * @attribute clear_on_cancel
+         * @type Boolean
+         */
+        clear_on_cancel: { value: false },
+
+        /**
+         * A CSS selector for the DOM element that will activate (show) the picker
+         * once clicked.
+         *
+         * @attribute picker_activator
+         * @type String
+         */
+        picker_activator: { value: null },
+
+        /**
+         * An extra CSS class to be added to the picker_activator, generally used
+         * to distinguish regular links from js-triggering ones.
+         *
+         * @attribute picker_activator_css_class
+         * @type String
+         */
+        picker_activator_css_class: { value: 'js-action' },
+
+        /**
+         * Minimum number of characters that need to be entered in the search
+         * string input before a search event will be fired. The search string
+         * will be trimmed before testing the length.
+         *
+         * @attribute min_search_chars
+         * @type Integer
+         */
+        min_search_chars: { value: 3 },
+
+        /**
+         * The current search string, which is needed when clicking on a different
+         * batch if the search input has been modified.
+         *
+         * @attribute current_search_string
+         * @type String
+         */
+        current_search_string: {value: ''},
+
+        /**
+         * The string representation of the current filter.
+         *
+         * @attribute current_filter_value
+         * @type String
+         */
+        current_filter_value: {value: null},
+
+        /**
+         * A list of attribute name values used to construct the filtering options
+         * for this picker..
+         *
+         * @attribute filter_options
+         * @type Object
+         */
+        filter_options: {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.
+         *
+         * @attribute selected_value_metadata
+         * @type String
+         */
+        selected_value_metadata: {value: null},
+
+        /**
+         * Results currently displayed by the widget. Updating this value
+         * automatically updates the display.
+         *
+         * @attribute results
+         * @type Array
+         */
+        results: { value: [] },
+
+        /**
+         * This adds any form fields you want below the search field.
+         * Updating this value automatically updates the display, but only
+         * if the widget has already been rendered. Otherwise, the change
+         * event never fires.
+         *
+         * @attribute search_slot
+         * @type Node
+         */
+        search_slot: {value: null},
+
+        /**
+         * A place for custom html at the bottom of the widget. When there
+         * are no search results the search_slot and the footer_slot are
+         * right next to each other.
+         * Updating this value automatically updates the display, but only
+         * if the widget has already been rendered. Otherwise, the change
+         * event never fires.
+         *
+         * @attribute footer_slot
+         * @type Node
+         */
+        footer_slot: {value: null},
+
+        /**
+         * Batches currently displayed in the widget, which can be
+         * clicked to change the batch of results being displayed. Updating
+         * this value automatically updates the display.
+         *
+         * This an array of object containing the two keys, name (used as
+         * the batch label) and value (used as additional details to 'search' 
+         * event).
+         *
+         * @attribute batches
+         * @type Array
+         */
+        batches: {value: null},
+
+        /**
+         * For simplified batch creation, you can set this to the number of
+         * batches in the search results.  In this case, the batch labels
+         * and values are automatically calculated.  The batch name (used as the
+         * batch label) will be the batch number starting from 1.  The batch value
+         * (used as additional details to the 'search' event) will be the batch
+         * number, starting from zero.
+         *
+         * If 'batches' is set (see above), batch_count is ignored.
+         *
+         * @attribute batch_count
+         * @type Integer
+         */
+        batch_count: {value: null},
+
+        /**
+         * Batch currently selected.
+         *
+         * @attribute selected_batch
+         * @type Integer
+         */
+        selected_batch: {
+            value: 0,
+            getter: function (value) {
+                return value || 0;
+            },
+            validator: function (value) {
+                var batches = this._getBatches();
+                return Y.Lang.isNumber(value) &&
+                       value >= 0 &&
+                       value < batches.length;
+            }},
+
+        /**
+         * Flag indicating if the widget is currently in search mode (so users
+         * has triggered a search and we are waiting for results.)
+         *
+         * @attribute search_mode
+         * @type Boolean
+         */
+        search_mode: { value: false },
+
+        /**
+         * The current error message. This puts the widget in 'error-mode',
+         * setting this value to null clears that state.
+         *
+         * @attribute error
+         * @type String
+         */
+        error: { value: null },
+
+        /**
+         * The message to display when the search returned no results. This string
+         * can contain a 'query' placeholder
+         *
+         * @attribute no_results_search_message
+         * @type String
+         * @default No items matched "{query}".
+         */
+        no_results_search_message: {
+            value: 'No items matched "{query}".'
+        }
+    }
+});
+
+
+ns.Picker.SAVE_RESULT = 0;
 
 
 /**
@@ -1296,9 +1192,9 @@
     initializer: function(config) {
         var input = Y.one(config.input_element);
         this.doAfter('save', function (e) {
-            var result = e.details[Picker.SAVE_RESULT];
-            this.get('host').set(SELECTED_VALUE_METADATA, result.metadata);
-            this.get('host').set(SELECTED_VALUE, result.value);
+            var result = e.details[ns.Picker.SAVE_RESULT];
+            this.get('host').set('selected_value_metadata', result.metadata);
+            this.get('host').set('selected_value', result.value);
             input.set("value",  result.value || '');
             // If the search input isn't blurred before it is focused,
             // then the I-beam disappears.
@@ -1313,17 +1209,15 @@
             // IE doesn't like setting the value to null, so use ''
             // instead.
             this.get('host')._search_input.set('value', selected_value || '');
-            this.get('host').set(SELECTED_VALUE, selected_value);
+            this.get('host').set('selected_value', selected_value);
         });
     }
 });
 
-var namespace = Y.namespace('lazr.picker');
-namespace.Picker = Picker;
-namespace.TextFieldPickerPlugin = TextFieldPickerPlugin;
+ns.TextFieldPickerPlugin = TextFieldPickerPlugin;
 
 }, "0.1", {"skinnable": true,
-           "requires": ["oop", "escape", "event", "event-focus", "node",
+           "requires": ["oop", "escape", "event", "event-focus", "base", "node",
                         "plugin", "substitute", "widget", "widget-stdmod",
                         "lazr.overlay", "lp.anim", "lazr.base",
                         "lp.app.widgets.expander"]

=== modified file 'lib/lp/app/javascript/picker/picker_patcher.js'
--- lib/lp/app/javascript/picker/picker_patcher.js	2012-03-21 01:26:45 +0000
+++ lib/lp/app/javascript/picker/picker_patcher.js	2012-06-18 21:59:29 +0000
@@ -177,7 +177,7 @@
         if (!show_search_box) {
           config.temp_spinner.removeClass('unseen');
           picker.set('min_search_chars', 0);
-          picker.fire(Y.lazr.picker.Picker.SEARCH, '');
+          picker.fire('search', '');
           picker.get('contentBox').one(
               '.yui3-picker-search-box').addClass('unseen');
         }
@@ -520,7 +520,7 @@
         }
         picker._defaultSave(e);
     };
-    picker.after(Y.lazr.picker.Picker.SAVE, save_handler);
+    picker.after('save', save_handler);
 
     picker.subscribe('cancel', function (e) {
         Y.log('Got cancel event.');
@@ -666,7 +666,7 @@
         }
     };
 
-    picker.after(Y.lazr.picker.Picker.SEARCH, search_handler);
+    picker.after('search', search_handler);
 
     picker.render();
     picker.hide();

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.html'
--- lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-04-25 18:17:08 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.html	2012-06-18 21:59:29 +0000
@@ -60,10 +60,6 @@
 
     </head>
     <body class="yui3-skin-sam">
-        <ul id="suites">
-            <!-- <li>lp.large_indicator.test</li> -->
-            <li>lp.person_picker.test</li>
-        </ul>
 
         <div class="yui3-widget yui3-activator yui3-activator-focused">
             <span id="picker_id" class="yui3-activator-content yui3-activator-success">

=== modified file 'lib/lp/app/javascript/picker/tests/test_personpicker.js'
--- lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-02-06 18:16:06 +0000
+++ lib/lp/app/javascript/picker/tests/test_personpicker.js	2012-06-18 21:59:29 +0000
@@ -157,8 +157,7 @@
             var data = null;
             this.picker.on('save', function (result) {
                 data = result.value;
-            });
-            var remove = Y.one('.yui-picker-remove-button');
+            }); var remove = Y.one('.yui-picker-remove-button');
             remove.simulate('click');
             Y.Assert.areEqual(null, data);
 


Follow ups