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
1=== modified file 'lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js'
2--- lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js 2011-09-12 14:21:07 +0000
3+++ lib/lp/app/javascript/confirmationoverlay/confirmationoverlay.js 2011-09-13 11:25:49 +0000
4@@ -22,6 +22,9 @@
5 * a chance to cancel the form submission. Note that the button
6 * can be simply 'disabled' if it's desirable to prevent the usage
7 * of that button if the user's browser has no Javascript support.
8+ * It can also be used without providing a button to the constructor;
9+ * in this case, the caller is responsible for calling show() manually
10+ * and also providing a function to be called (submit_fn).
11 *
12 * @class ConfirmationOverlay
13 * @namespace lp.app
14@@ -35,8 +38,8 @@
15 ConfirmationOverlay.ATTRS = {
16
17 /**
18- * The input button what should be 'guarded' by this confirmation
19- * overlay.
20+ * An (optional) input button that should be 'guarded' by this
21+ * confirmation overlay.
22 *
23 * @attribute button
24 * @type Node
25@@ -127,8 +130,6 @@
26 this.set('form_submit_button', submit_button);
27 this.set('form_cancel_button', cancel_button);
28
29- this.set('submit_form', this.get('button').ancestor('form'));
30-
31 var self = this;
32 var submit_fn = this.get('submit_fn');
33 if (submit_fn === null) {
34@@ -148,15 +149,18 @@
35 this.set('form_submit_callback', submit_form_fn);
36 }
37
38- // Enable the button if it's disabled.
39- this.get('button').set('disabled', false);
40-
41- // Wire this._handleButtonClicked to the button.
42- this.get(
43- 'button').on('click', Y.bind(this._handleButtonClicked, this));
44-
45- // Hide the overlay.
46- this.hide();
47+ var button = this.get('button');
48+ if (Y.Lang.isValue(button)) {
49+ this.set('submit_form', this.get('button').ancestor('form'));
50+
51+ // Enable the button if it's disabled.
52+ button.set('disabled', false);
53+
54+ // Wire this._handleButtonClicked to the button.
55+ button.on('click', Y.bind(this._handleButtonClicked, this));
56+ }
57+ // Hide the overlay.
58+ this.hide();
59 },
60
61 /**
62@@ -172,6 +176,23 @@
63 },
64
65 /**
66+ * Update overlay's content and show the overlay.
67+ *
68+ * @method show
69+ *
70+ */
71+ show: function() {
72+ // Update the overlay's content.
73+ this._renderUIFormOverlay();
74+ this._fillContent();
75+ this._positionOverlay();
76+ this._setFormContent();
77+ // Render and display the overlay.
78+ this.render();
79+ ConfirmationOverlay.superclass.show.call(this);
80+ },
81+
82+ /**
83 * Prevent form submission and display the confirmation overlay.
84 *
85 * @method _handleButtonClicked
86@@ -181,13 +202,6 @@
87 if (display_confirmation_fn === null || display_confirmation_fn()) {
88 // Stop the event to prevent the form submission.
89 e.preventDefault();
90- // Update the overlay's content.
91- this._renderUIFormOverlay();
92- this._fillContent();
93- this._positionOverlay();
94- this._setFormContent();
95- // Render and display the overlay.
96- this.render();
97 this.show();
98 }
99 },
100
101=== modified file 'lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js'
102--- lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js 2011-09-12 14:21:18 +0000
103+++ lib/lp/app/javascript/confirmationoverlay/tests/test_confirmationoverlay.js 2011-09-13 11:25:49 +0000
104@@ -189,6 +189,37 @@
105
106 }));
107
108+suite.add(new Y.Test.Case({
109+
110+ name: 'confirmation_overlay_buttonless',
111+
112+ tearDown: function() {
113+ if (this.overlay !== null) {
114+ this.overlay.destroy();
115+ }
116+ },
117+
118+ test_callback_called: function() {
119+ // A ConfirmationOverlay can be constructed without passing a button.
120+ // The creator is responsible for calling show() manually.
121+ var called = false;
122+ var callback = function() {
123+ called = true;
124+ };
125+
126+ this.overlay = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
127+ submit_fn: callback
128+ });
129+ Y.Assert.isFalse(this.overlay.get('visible'));
130+ this.overlay.show();
131+ Y.Assert.isTrue(this.overlay.get('visible'));
132+ this.overlay.form_node.one('.ok-btn').simulate('click');
133+ Y.Assert.isFalse(this.overlay.get('visible'));
134+ // The callback has been called.
135+ Y.Assert.isTrue(called);
136+ }
137+
138+}));
139
140 Y.lp.testing.Runner.run(suite);
141