Merge lp:~rvb/launchpad/confirmationoverlay-button-optional into lp:launchpad

Proposed by Raphaël Badin on 2011-09-13
Status: Merged
Approved by: Raphaël Badin on 2011-09-13
Approved revision: no longer in the source branch.
Merged at revision: 13944
Proposed branch: lp:~rvb/launchpad/confirmationoverlay-button-optional
Merge into: lp:launchpad
Diff against target: 140 lines (+65/-20)
2 files modified
lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js (+34/-20)
lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js (+31/-0)
To merge this branch: bzr merge lp:~rvb/launchpad/confirmationoverlay-button-optional
Reviewer Review Type Date Requested Status
Graham Binns (community) code 2011-09-13 Approve on 2011-09-13
Review via email: mp+75154@code.launchpad.net

Commit message

[r=gmb][no-qa] The 'button' parameter is optional when creating a ConfirmationOverlay.

Description of the change

This branch improves the ConfirmationOverlay so that it can be used without providing a button to the constructor. This way, ConfirmationOverlay can be used in pure Javascript environments to only protect method calls like this:

/// JS
var method = function() {
    alert('This was a joke!');
};
var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
    submit_fn: method,
    form_content: 'Are you *really* sure? This will kill kittens!',
    headerContent: 'Confirm?'
    });
co.show();
/// \JS

= Tests =

lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.html

= QA =

None.

To post a comment you must log in.
Graham Binns (gmb) wrote :

Hi Raphaël,

Nice branch; I've only got one minor tweak for you to make before this lands:

[1]

48 + if (button !== null) {

Since we're in a YUI environment, you should use if(Y.Lang.isValue(button)) here rather than checking for !== null.

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js'
--- lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js 2011-09-12 14:21:07 +0000
+++ lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js 2011-09-13 11:25:49 +0000
@@ -22,6 +22,9 @@
22 * a chance to cancel the form submission. Note that the button22 * a chance to cancel the form submission. Note that the button
23 * can be simply 'disabled' if it's desirable to prevent the usage23 * can be simply 'disabled' if it's desirable to prevent the usage
24 * of that button if the user's browser has no Javascript support.24 * of that button if the user's browser has no Javascript support.
25 * It can also be used without providing a button to the constructor;
26 * in this case, the caller is responsible for calling show() manually
27 * and also providing a function to be called (submit_fn).
25 *28 *
26 * @class ConfirmationOverlay29 * @class ConfirmationOverlay
27 * @namespace lp.app30 * @namespace lp.app
@@ -35,8 +38,8 @@
35ConfirmationOverlay.ATTRS = {38ConfirmationOverlay.ATTRS = {
3639
37 /**40 /**
38 * The input button what should be 'guarded' by this confirmation41 * An (optional) input button that should be 'guarded' by this
39 * overlay.42 * confirmation overlay.
40 *43 *
41 * @attribute button44 * @attribute button
42 * @type Node45 * @type Node
@@ -127,8 +130,6 @@
127 this.set('form_submit_button', submit_button);130 this.set('form_submit_button', submit_button);
128 this.set('form_cancel_button', cancel_button);131 this.set('form_cancel_button', cancel_button);
129132
130 this.set('submit_form', this.get('button').ancestor('form'));
131
132 var self = this;133 var self = this;
133 var submit_fn = this.get('submit_fn');134 var submit_fn = this.get('submit_fn');
134 if (submit_fn === null) {135 if (submit_fn === null) {
@@ -148,15 +149,18 @@
148 this.set('form_submit_callback', submit_form_fn);149 this.set('form_submit_callback', submit_form_fn);
149 }150 }
150151
151 // Enable the button if it's disabled.152 var button = this.get('button');
152 this.get('button').set('disabled', false);153 if (Y.Lang.isValue(button)) {
153154 this.set('submit_form', this.get('button').ancestor('form'));
154 // Wire this._handleButtonClicked to the button.155
155 this.get(156 // Enable the button if it's disabled.
156 'button').on('click', Y.bind(this._handleButtonClicked, this));157 button.set('disabled', false);
157158
158 // Hide the overlay.159 // Wire this._handleButtonClicked to the button.
159 this.hide();160 button.on('click', Y.bind(this._handleButtonClicked, this));
161 }
162 // Hide the overlay.
163 this.hide();
160 },164 },
161165
162 /**166 /**
@@ -172,6 +176,23 @@
172 },176 },
173177
174 /**178 /**
179 * Update overlay's content and show the overlay.
180 *
181 * @method show
182 *
183 */
184 show: function() {
185 // Update the overlay's content.
186 this._renderUIFormOverlay();
187 this._fillContent();
188 this._positionOverlay();
189 this._setFormContent();
190 // Render and display the overlay.
191 this.render();
192 ConfirmationOverlay.superclass.show.call(this);
193 },
194
195 /**
175 * Prevent form submission and display the confirmation overlay.196 * Prevent form submission and display the confirmation overlay.
176 *197 *
177 * @method _handleButtonClicked198 * @method _handleButtonClicked
@@ -181,13 +202,6 @@
181 if (display_confirmation_fn === null || display_confirmation_fn()) {202 if (display_confirmation_fn === null || display_confirmation_fn()) {
182 // Stop the event to prevent the form submission.203 // Stop the event to prevent the form submission.
183 e.preventDefault();204 e.preventDefault();
184 // Update the overlay's content.
185 this._renderUIFormOverlay();
186 this._fillContent();
187 this._positionOverlay();
188 this._setFormContent();
189 // Render and display the overlay.
190 this.render();
191 this.show();205 this.show();
192 }206 }
193 },207 },
194208
=== modified file 'lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js'
--- lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js 2011-09-12 14:21:18 +0000
+++ lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js 2011-09-13 11:25:49 +0000
@@ -189,6 +189,37 @@
189189
190}));190}));
191191
192suite.add(new Y.Test.Case({
193
194 name: 'confirmation_overlay_buttonless',
195
196 tearDown: function() {
197 if (this.overlay !== null) {
198 this.overlay.destroy();
199 }
200 },
201
202 test_callback_called: function() {
203 // A ConfirmationOverlay can be constructed without passing a button.
204 // The creator is responsible for calling show() manually.
205 var called = false;
206 var callback = function() {
207 called = true;
208 };
209
210 this.overlay = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
211 submit_fn: callback
212 });
213 Y.Assert.isFalse(this.overlay.get('visible'));
214 this.overlay.show();
215 Y.Assert.isTrue(this.overlay.get('visible'));
216 this.overlay.form_node.one('.ok-btn').simulate('click');
217 Y.Assert.isFalse(this.overlay.get('visible'));
218 // The callback has been called.
219 Y.Assert.isTrue(called);
220 }
221
222}));
192223
193Y.lp.testing.Runner.run(suite);224Y.lp.testing.Runner.run(suite);
194225