launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-textfield-focus into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #797811 in Launchpad itself: "person picker text box should have focus when opened"
  https://bugs.launchpad.net/launchpad/+bug/797811

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-textfield-focus/+merge/65608

The recently added person picker broke the search input field receiving focus when the picker was shown.

== Implementation ==

The picker overrode show() and hide() but forgot to call super()

== Tests ==

Added new test_search_field_focus to test_personpicker.js

== Lint ==

jslint: 2 files to lint.
jslint: No problem found in '/home/ian/projects/lp-branches/devel-sandbox/lib/lp/app/javascript/tests/test_personpicker.js'.
jslint: No problem found in '/home/ian/projects/lp-branches/devel-sandbox/lib/lp/app/javascript/widgets.js'.

-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-textfield-focus/+merge/65608
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-textfield-focus 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-23 02:49:24 +0000
@@ -32,6 +32,19 @@
             Y.Assert.isNotUndefined(personpicker.remove_button);
         },
 
+        test_search_field_focus: function () {
+            var personpicker = new Y.lp.app.widgets.PersonPicker();
+            personpicker.render();
+            personpicker.hide();
+
+            var got_focus = false;
+            personpicker._search_input.on('focus', function(e) {
+                got_focus = true;
+            });
+            personpicker.show();
+            Y.Assert.isTrue(got_focus, "search input did not get focus.");
+        },
+
         test_buttons: function () {
             var personpicker = new Y.lp.app.widgets.PersonPicker();
             personpicker.render();

=== 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-23 02:49:24 +0000
@@ -101,10 +101,12 @@
 
     hide: function() {
         this.get('boundingBox').setStyle('display', 'none');
+        this.constructor.superclass.hide.call(this);
     },
 
     show: function() {
         this.get('boundingBox').setStyle('display', 'block');
+        this.constructor.superclass.show.call(this);
     },
 
     remove: function () {