launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/picker-close-during-search-847789 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #847789 in Launchpad itself: "Closing the person picker during a search puts the picker in infinite loop"
  https://bugs.launchpad.net/launchpad/+bug/847789

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/picker-close-during-search-847789/+merge/75667

Ensure that if a picker is closed during a search, search mode is turned of so that the picker works properly when opened again.

== Implementation ==

A trivial fix - set the picker 'search_mode' attribute to false in the cancel event handler.

== Tests ==

Add new yui test in test_picker.js

  - test_cancel_event_resets_search_mode()

== Lint ==

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

./lib/lp/app/javascript/picker/picker.js
      70: 'Picker' has not been fully defined yet.


-- 
https://code.launchpad.net/~wallyworld/launchpad/picker-close-during-search-847789/+merge/75667
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/picker-close-during-search-847789 into lp:launchpad.

=== modified file 'lib/lp/app/javascript/picker/picker.js'
--- lib/lp/app/javascript/picker/picker.js	2011-09-09 13:51:31 +0000
+++ lib/lp/app/javascript/picker/picker.js	2011-09-16 03:58:24 +0000
@@ -957,6 +957,7 @@
      */
     _defaultCancel : function(e) {
         Picker.superclass._defaultCancel.apply(this, arguments);
+        this.set(SEARCH_MODE, false);
         if ( this.get('clear_on_cancel') ) {
             this._clear();
         }

=== modified file 'lib/lp/app/javascript/picker/tests/test_picker.js'
--- lib/lp/app/javascript/picker/tests/test_picker.js	2011-09-05 00:22:35 +0000
+++ lib/lp/app/javascript/picker/tests/test_picker.js	2011-09-16 03:58:24 +0000
@@ -716,6 +716,14 @@
             this.picker.get('visible'), "The widget should be hidden.");
     },
 
+    test_cancel_event_resets_search_mode: function () {
+        this.picker.render();
+        this.picker.set('search_mode', true);
+        Assert.isTrue(this.picker.get('search_mode'));
+        this.picker.fire('cancel', 'bogus');
+        Assert.isFalse(this.picker.get('search_mode'));
+    },
+
     test_save_event_hides_widget: function () {
         this.picker.render();