Merge lp:~salgado/lazr-js/picker-refactoring2 into lp:lazr-js

Proposed by Guilherme Salgado
Status: Superseded
Proposed branch: lp:~salgado/lazr-js/picker-refactoring2
Merge into: lp:lazr-js
Prerequisite: lp:~salgado/lazr-js/picker-refactoring
Diff against target: 236 lines (+141/-7)
3 files modified
examples/picker/index.html (+2/-3)
src-js/lazrjs/picker/picker.js (+88/-3)
src-js/lazrjs/picker/tests/picker.js (+51/-1)
To merge this branch: bzr merge lp:~salgado/lazr-js/picker-refactoring2
Reviewer Review Type Date Requested Status
Canonical Launchpad Engineering Pending
Review via email: mp+14797@code.launchpad.net

This proposal has been superseded by a proposal from 2009-11-12.

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

Add a new config variable to the picker to specify the DOM element to which picker.show() should be hooked up.

That way callsites don't need to do it themselves.

134. By Guilherme Salgado

merge from trunk

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'examples/picker/index.html'
--- examples/picker/index.html 2009-10-22 20:22:12 +0000
+++ examples/picker/index.html 2009-11-12 20:44:11 +0000
@@ -65,6 +65,8 @@
6565
6666
67 var picker = new Y.Picker({67 var picker = new Y.Picker({
68 picker_activator: '#show-widget',
69 clear_on_cancel: true,
68 align: {70 align: {
69 points: [Y.WidgetPositionExt.CC, Y.WidgetPositionExt.CC]71 points: [Y.WidgetPositionExt.CC, Y.WidgetPositionExt.CC]
70 },72 },
@@ -129,9 +131,6 @@
129 });131 });
130 });132 });
131133
132 Y.on('click', function () {
133 picker.show();
134 }, '#show-widget');
135 Y.on('change', function () {134 Y.on('change', function () {
136 picker.set(135 picker.set(
137 'min_search_chars',136 'min_search_chars',
138137
=== modified file 'src-js/lazrjs/picker/picker.js'
--- src-js/lazrjs/picker/picker.js 2009-10-22 19:27:45 +0000
+++ src-js/lazrjs/picker/picker.js 2009-11-12 20:44:11 +0000
@@ -166,6 +166,20 @@
166 * @preventable _defaultSave166 * @preventable _defaultSave
167 */167 */
168 this.publish(SAVE, { defaultFn: this._defaultSave } );168 this.publish(SAVE, { defaultFn: this._defaultSave } );
169
170 // Subscribe to the cancel event so that we can clear the widget when
171 // requested.
172 this.subscribe('cancel', this._defaultCancel);
173
174 if ( this.get('picker_activator') ) {
175 var element = Y.one(this.get('picker_activator'));
176 element.on('click', function(e) {
177 e.halt();
178 this.show();
179 }, this);
180 element.addClass(this.get('picker_activator_css_class'));
181 }
182
169 },183 },
170184
171 /**185 /**
@@ -499,7 +513,7 @@
499 // is changed, and reset the selected one to the first one.513 // is changed, and reset the selected one to the first one.
500 this.after('batchesChange', function (e) {514 this.after('batchesChange', function (e) {
501 this._syncBatchesUI();515 this._syncBatchesUI();
502 if (this.get(SELECTED_BATCH) == 0){516 if (this.get(SELECTED_BATCH) === 0){
503 // If the attribute is already set to the same value,517 // If the attribute is already set to the same value,
504 // the 'after' events won't be triggered, so we have518 // the 'after' events won't be triggered, so we have
505 // to trigger it manually.519 // to trigger it manually.
@@ -550,6 +564,21 @@
550 this._search_input.focus();564 this._search_input.focus();
551 },565 },
552566
567 /*
568 * Clear all elements of the picker, resetting it to its original state.
569 *
570 * @method _clear
571 * @param e {Object} The event object.
572 * @protected
573 */
574 _clear: function() {
575 this._search_input.set('value', '');
576 this.set('error', '');
577 this.set('results', [{}]);
578 this._results_box.set('innerHTML', '');
579 this.set('batches', []);
580 },
581
553 /**582 /**
554 * Handle clicks on the 'Search' button or entering the enter key in the583 * Handle clicks on the 'Search' button or entering the enter key in the
555 * search field. This fires the search event.584 * search field. This fires the search event.
@@ -585,8 +614,25 @@
585 },614 },
586615
587 /**616 /**
588 * By default, the save event simply hides the widget. The search entered617 * By default, the cancel event just hides the widget, but you can
589 * by the user is passed in the first details attribute of the event.618 * have it also cleared by setting clear_on_cancel to 'true'.
619 *
620 * @method _defaultCancel
621 * @param e {Event.Facade} An Event Facade object.
622 * @protected
623 */
624 _defaultCancel : function(e) {
625 Picker.superclass._defaultCancel.apply(this, arguments);
626 if ( this.get('clear_on_cancel') ) {
627 this._clear();
628 }
629 },
630
631 /**
632 * By default, the save event clears and hides the widget, but you can
633 * have it not cleared by setting clear_on_save to 'false'. The search
634 * entered by the user is passed in the first details attribute of the
635 * event.
590 *636 *
591 * @method _defaultSave637 * @method _defaultSave
592 * @param e {Event.Facade} An Event Facade object.638 * @param e {Event.Facade} An Event Facade object.
@@ -594,6 +640,9 @@
594 */640 */
595 _defaultSave : function(e) {641 _defaultSave : function(e) {
596 this.hide();642 this.hide();
643 if ( this.get('clear_on_save') ) {
644 this._clear();
645 }
597 },646 },
598647
599 /**648 /**
@@ -637,6 +686,42 @@
637686
638Picker.ATTRS = {687Picker.ATTRS = {
639 /**688 /**
689 * Whether or not the search box and result list should be cleared when
690 * the save event is fired.
691 *
692 * @attribute clear_on_save
693 * @type Boolean
694 */
695 clear_on_save: { value: true },
696
697 /**
698 * Whether or not the search box and result list should be cleared when
699 * the cancel event is fired.
700 *
701 * @attribute clear_on_cancel
702 * @type Boolean
703 */
704 clear_on_cancel: { value: false },
705
706 /**
707 * A CSS selector for the DOM element that will activate (show) the picker
708 * once clicked.
709 *
710 * @attribute picker_activator
711 * @type String
712 */
713 picker_activator: { value: null },
714
715 /**
716 * An extra CSS class to be added to the picker_activator, generally used
717 * to distinguish regular links from js-triggering ones.
718 *
719 * @attribute picker_activator_css_class
720 * @type String
721 */
722 picker_activator_css_class: { value: 'js-action' },
723
724 /**
640 * Minimum number of characters that need to be entered in the search725 * Minimum number of characters that need to be entered in the search
641 * string input before a search event will be fired. The search string726 * string input before a search event will be fired. The search string
642 * will be trimmed before testing the length.727 * will be trimmed before testing the length.
643728
=== modified file 'src-js/lazrjs/picker/tests/picker.js'
--- src-js/lazrjs/picker/tests/picker.js 2009-10-21 21:43:07 +0000
+++ src-js/lazrjs/picker/tests/picker.js 2009-11-12 20:44:11 +0000
@@ -206,7 +206,7 @@
206 [false, true, false, true], results.hasClass(Y.lazr.ui.CSS_ODD));206 [false, true, false, true], results.hasClass(Y.lazr.ui.CSS_ODD));
207 },207 },
208208
209 test_clicking_search_button_fires_save_event: function () {209 test_clicking_search_button_fires_search_event: function () {
210 this.picker.render();210 this.picker.render();
211211
212 var bb = this.picker.get('boundingBox');212 var bb = this.picker.get('boundingBox');
@@ -379,6 +379,14 @@
379 }, 3000);379 }, 3000);
380 },380 },
381381
382 test_cancel_event_hides_widget: function () {
383 this.picker.render();
384
385 this.picker.fire('cancel', 'bogus');
386 Assert.isFalse(
387 this.picker.get('visible'), "The widget should be hidden.");
388 },
389
382 test_save_event_hides_widget: function () {390 test_save_event_hides_widget: function () {
383 this.picker.render();391 this.picker.render();
384392
@@ -387,6 +395,48 @@
387 this.picker.get('visible'), "The widget should be hidden.");395 this.picker.get('visible'), "The widget should be hidden.");
388 },396 },
389397
398 test_save_event_clears_widget_by_default: function () {
399 this.picker.render();
400
401 this.picker._search_input.set('value', 'foo');
402 this.picker.fire('save', 'bogus');
403 Assert.areEqual(
404 '', this.picker._search_input.get('value'),
405 "The widget hasn't been cleared");
406 },
407
408 test_save_does_not_clear_widget_when_clear_on_save_is_false: function () {
409 picker = new Y.Picker({clear_on_save: false});
410 picker.render();
411
412 picker._search_input.set('value', 'foo');
413 picker.fire('save', 'bogus');
414 Assert.areEqual(
415 'foo', picker._search_input.get('value'),
416 "The widget has been cleared but it should not");
417 },
418
419 test_cancel_event_does_not_clear_widget_by_default: function () {
420 this.picker.render();
421
422 this.picker._search_input.set('value', 'foo');
423 this.picker.fire('cancel', 'bogus');
424 Assert.areEqual(
425 'foo', this.picker._search_input.get('value'),
426 "The widget has been cleared but it should not");
427 },
428
429 test_cancel_event_clears_widget_when_clear_on_cancel_is_true: function () {
430 picker = new Y.Picker({clear_on_cancel: true});
431 picker.render();
432
433 picker._search_input.set('value', 'foo');
434 picker.fire('cancel', 'bogus');
435 Assert.areEqual(
436 '', picker._search_input.get('value'),
437 "The widget hasn't been cleared");
438 },
439
390 test_search_clears_any_eror: function () {440 test_search_clears_any_eror: function () {
391 this.picker.render();441 this.picker.render();
392 this.picker.set('error', "An error");442 this.picker.set('error', "An error");

Subscribers

People subscribed via source and target branches