j.c.sackett has proposed merging
lp:~jcsackett/launchpad/picker-buttons-should-disappear into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #707234 in Launchpad itself: "Ajax controls disabled on bugs with many
tasks due to multiple redundant copies of person picker make_picker function in
view-source:https://bugs.launchpad.net/launchpad-project/+bugs?advanced=1";
https://bugs.launchpad.net/launchpad/+bug/707234
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/picker-buttons-should-disappear/+merge/65562
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.
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.
--
https://code.launchpad.net/~jcsackett/launchpad/picker-buttons-should-disappear/+merge/65562
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~jcsackett/launchpad/picker-buttons-should-disappear into lp:launchpad.
=== modified file 'lib/lp/app/javascript/tests/test_personpicker.js'
--- lib/lp/app/javascript/tests/test_personpicker.js 2011-06-14 14:38:47 +0000
+++ lib/lp/app/javascript/tests/test_personpicker.js 2011-06-22 20:04:32 +0000
@@ -19,6 +19,22 @@
links: {me: '/~no-one'},
cache: {}
};
+ this.vocabulary = [
+ {
+ "value": "fred",
+ "title": "Fred",
+ "css": "sprite-person",
+ "description": "[email protected]",
+ "api_uri": "~/fred"
+ },
+ {
+ "value": "frieda",
+ "title": "Frieda",
+ "css": "sprite-person",
+ "description": "[email protected]",
+ "api_uri": "~/frieda"
+ }
+ ];
},
test_render: function () {
@@ -61,11 +77,30 @@
personpicker = new Y.lp.app.widgets.PersonPicker(cfg);
personpicker.render();
personpicker.show();
-
+
Y.Assert.isNotNull(Y.one('.extra-form-buttons'));
Y.Assert.isUndefined(personpicker.remove_button);
Y.Assert.isUndefined(personpicker.assign_me_button);
-
+ },
+
+ _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'));
}
}));
=== modified file 'lib/lp/app/javascript/widgets.js'
--- lib/lp/app/javascript/widgets.js 2011-06-21 21:43:31 +0000
+++ lib/lp/app/javascript/widgets.js 2011-06-22 20:04:32 +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, {
@@ -116,8 +118,8 @@
this.fire('save', {value: name});
},
- renderUI: function() {
- this.constructor.superclass.renderUI.call(this);
+ _renderPersonPickerUI: function() {
+ //this.constructor.superclass.renderUI.apply(this, arguments);
var remove_button, assign_me_button;
var remove_button_text = "Remove assignee";
var assign_me_button_text = "Assign me";
@@ -147,9 +149,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';
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
