Merge lp:~jcsackett/launchpad/picker-buttons-should-disappear into lp:launchpad

[j.c.sackett]

[bug=798778] [r=allenap] Hides the extra buttons on personpicker when a search is made.

Summary
=======
Personpicker introduces assign me and remove assignee buttons to all person assigning activities. However, it's confusing to see the assign me and remove me buttons when there are results from a search in the picker.

This branch makes the buttons disappear if there are search results, but provides the buttons if no results are found, in which case one might concievably want to assign one's self.

Proposed fix
============
Using the YUI event model, we want to detect the presence of results and hide the extra buttons if they exist after results are loaded.

Pre-implementation notes
========================
Spoke with Curtis Hovey, explaining likely implementation.

Implementation details
======================
The personpicker now has overrided the picker's bindUI call, adding a section that creates a handler for the 'resultChange' event fired by the picker. This handler checks for the result condition (e.g. are there results, and was a search done) and determines whether to apply the unseen class to the extra buttons based on that information.

There are also some drive by fixes to the method invocations in personpicker, to make better use of the YUI event and widget model.

Tests
=====
firefox lib/lp/app/javascript/tests/test_personpicker.html

Demo and Q/A
============
Do a search with the personpicker (e.g. on launchpad.dev/evolution/+edit-people). If you search for a value with results (e.g. 'name') the extra buttons are not shown. If you search for a value without results (e.g. 'nobody') the buttons are displayed.

Lint
====
Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/widgets.js
  lib/lp/app/javascript/tests/test_personpicker.js
  lib/lp/app/javascript/tests/test_picker.js

./lib/lp/app/javascript/widgets.js
      14: 'Picker' has not been fully defined yet.
      79: 'PersonPicker' has not been fully defined yet.

The errors can be ignored as they're part of the javascript extension.

[Gavin Panella (allenap)]

review: Approve

=== modified file 'lib/lp/app/javascript/tests/test_personpicker.js'
--- lib/lp/app/javascript/tests/test_personpicker.js	2011-06-29 14:05:39 +0000
+++ lib/lp/app/javascript/tests/test_personpicker.js	2011-06-30 13:58:58 +0000
@@ -19,6 +19,22 @@
                 links: {me: '/~no-one'},
                 cache: {}
             };
+            this.vocabulary = [
+                {
+                    "value": "fred",
+                    "title": "Fred",
+                    "css": "sprite-person",
+                    "description": "[e-mail]",
+                    "api_uri": "~/fred"
+                },
+                {
+                    "value": "frieda",
+                    "title": "Frieda",
+                    "css": "sprite-person",
+                    "description": "[e-mail]",
+                    "api_uri": "~/frieda"
+                }
+            ];
         },
 
         test_render: function () {
@@ -65,6 +81,7 @@
             var assign_me = Y.one('.yui-picker-assign-me-button');
             assign_me.simulate('click');
             Y.Assert.areEqual('no-one', data);
+            personpicker.destroy();
         },
 
         test_buttons_config: function () {
@@ -81,6 +98,27 @@
             Y.Assert.isUndefined(personpicker.remove_button);
             Y.Assert.isUndefined(personpicker.assign_me_button);
             personpicker.destroy();
+        },
+
+        _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 () {
+            personpicker = new Y.lp.app.widgets.PersonPicker(cfg);
+            personpicker.render();
+            this._change_results(personpicker, false);
+            Y.Assert.isTrue(personpicker._extra_buttons.hasClass('unseen'));
+
+            this._change_results(personpicker, true);
+            Y.Assert.isFalse(personpicker._extra_buttons.hasClass('unseen'));
+            personpicker.destroy();
         }
     }));
 

=== modified file 'lib/lp/app/javascript/widgets.js'
--- lib/lp/app/javascript/widgets.js	2011-06-30 08:31:37 +0000
+++ lib/lp/app/javascript/widgets.js	2011-06-30 13:58:58 +0000
@@ -78,6 +78,8 @@
 var 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.after(this._syncPersonPickerResultsUI, this, 'resultsChange');
 };
 
 Y.extend(PersonPicker, namespace.Picker, {
@@ -128,8 +130,7 @@
         this.fire('save', {value: name});
     },
 
-    renderUI: function() {
-        this.constructor.superclass.renderUI.call(this);
+    _renderPersonPickerUI: function() {
         var remove_button, assign_me_button;
 
         if (this._show_remove_button) {
@@ -157,9 +158,21 @@
 
     syncUI: function() {
         // call Picker's sync
-        this.constructor.superclass.syncUI.call(this);
+        this.constructor.superclass.syncUI.apply(this, arguments);
         footer_slot = Y.one(footer_label);
         footer_slot.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');
+            }
+        });
     }
 });
 PersonPicker.NAME = 'person-picker';