Merge lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497 into lp:launchpad
- private-dupe-bug-warning2-943497
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 15719 |
Proposed branch: | lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497 |
Merge into: | lp:launchpad |
Diff against target: |
809 lines (+434/-98) 7 files modified
lib/canonical/launchpad/icing/css/forms.css (+1/-2) lib/canonical/launchpad/icing/css/modifiers.css (+8/-0) lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css (+10/-0) lib/lp/app/javascript/formoverlay/formoverlay.js (+14/-11) lib/lp/bugs/javascript/duplicates.js (+278/-29) lib/lp/bugs/javascript/tests/test_duplicates.html (+1/-0) lib/lp/bugs/javascript/tests/test_duplicates.js (+122/-56) |
To merge this branch: | bzr merge lp:~wallyworld/launchpad/private-dupe-bug-warning2-943497 |
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email:
|
Commit message
Initial work to improve the bug duplicate form overlay to allow the selected bug to be previewed prior to submission.
Description of the change
== Implementation ==
This branch fixes a number of issues with the mark bug as duplicate form. See the linked bugs. The work is currently only for the bug selection widget used when marking bugs as duplicates. It would need to be generalised to be useful when selecting bugs to link to branches etc.
Not quite everything is implemented in this branch but it's big enough to be put up for review. The bit that is missing is marked with a TODO.
When a bug number is entered, a web service API call is made to get the json data for the bug. This data is presented to the user to allow then to confirm the bug is what they want. The TODO is because not all necessary data is returned and is currently hard coded. A new service API needs to be added to provided the required data.
Some css changes were necessary. Here some of the "non obvious" ones...
Needed to make spacing around form overlay form table correct due to css rule ordering :-
13 table.form, table.extra-options {
...
16 - margin: 1em 0;
17 + margin: 1em 0 inherit inherit;
Needed to un-squash form buttons when they are not little lazr icons.
48 +.yui3-
49 + margin-right: 0.7em;
50 + }
51
Expand out the error div on the form overlay when there is an error so it is not all squashed up when the form is narrow.
58 +.yui3-
59 + min-width: 250px;
60 + text-align: center;
61 + }
62 +
== Demo and QA ==
Here's an early video of it in operation.
http://
This video has a number of minor flaws (eg uncentred form, text alignment). These are addressed in this branch.
== Tests ==
Add a number of new yui tests and update existing ones.
== Lint ==
Linting changed files:
lib/canonical
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Clean except for some css noise.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/icing/css/forms.css' | |||
2 | --- lib/canonical/launchpad/icing/css/forms.css 2012-06-23 13:44:13 +0000 | |||
3 | +++ lib/canonical/launchpad/icing/css/forms.css 2012-07-27 01:21:21 +0000 | |||
4 | @@ -90,7 +90,6 @@ | |||
5 | 90 | .extra-form-buttons { | 90 | .extra-form-buttons { |
6 | 91 | text-align: center; | 91 | text-align: center; |
7 | 92 | padding-top: 1em; | 92 | padding-top: 1em; |
8 | 93 | white-space: nowrap; | ||
9 | 94 | } | 93 | } |
10 | 95 | .extra-form-buttons button { | 94 | .extra-form-buttons button { |
11 | 96 | margin-right: 0.7em; | 95 | margin-right: 0.7em; |
12 | @@ -140,7 +139,7 @@ | |||
13 | 140 | table.form, table.extra-options { | 139 | table.form, table.extra-options { |
14 | 141 | /* Many forms are laid out using tables, with appropriate spacing: */ | 140 | /* Many forms are laid out using tables, with appropriate spacing: */ |
15 | 142 | /* http://launchpad.dev/firefox/+edit */ | 141 | /* http://launchpad.dev/firefox/+edit */ |
17 | 143 | margin: 1em 0; | 142 | margin: 1em 0 inherit inherit; |
18 | 144 | width: 100%; | 143 | width: 100%; |
19 | 145 | } | 144 | } |
20 | 146 | table.form th { | 145 | table.form th { |
21 | 147 | 146 | ||
22 | === modified file 'lib/canonical/launchpad/icing/css/modifiers.css' | |||
23 | --- lib/canonical/launchpad/icing/css/modifiers.css 2012-07-23 03:40:33 +0000 | |||
24 | +++ lib/canonical/launchpad/icing/css/modifiers.css 2012-07-27 01:21:21 +0000 | |||
25 | @@ -112,6 +112,14 @@ | |||
26 | 112 | height: 10px; | 112 | height: 10px; |
27 | 113 | } | 113 | } |
28 | 114 | 114 | ||
29 | 115 | .ellipsis { | ||
30 | 116 | white-space: nowrap; | ||
31 | 117 | overflow: hidden; | ||
32 | 118 | text-overflow: ellipsis; | ||
33 | 119 | -o-text-overflow: ellipsis; | ||
34 | 120 | -ms-text-overflow: ellipsis; | ||
35 | 121 | } | ||
36 | 122 | |||
37 | 115 | .exception { | 123 | .exception { |
38 | 116 | color: #cc0000; | 124 | color: #cc0000; |
39 | 117 | } | 125 | } |
40 | 118 | 126 | ||
41 | === modified file 'lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css' | |||
42 | --- lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2012-05-12 01:56:24 +0000 | |||
43 | +++ lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2012-07-27 01:21:21 +0000 | |||
44 | @@ -21,12 +21,22 @@ | |||
45 | 21 | text-align: right; | 21 | text-align: right; |
46 | 22 | } | 22 | } |
47 | 23 | 23 | ||
48 | 24 | .yui3-lazr-formoverlay-form div.yui3-lazr-formoverlay-actions button { | ||
49 | 25 | margin-right: 0.7em; | ||
50 | 26 | } | ||
51 | 27 | |||
52 | 24 | .yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors { | 28 | .yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors { |
53 | 25 | padding-top: 0; | 29 | padding-top: 0; |
54 | 26 | padding-bottom: 0; | 30 | padding-bottom: 0; |
55 | 27 | color: red; | 31 | color: red; |
56 | 28 | } | 32 | } |
57 | 29 | 33 | ||
58 | 34 | .yui3-lazr-formoverlay-form .yui3-lazr-formoverlay-errors:not(:empty) { | ||
59 | 35 | min-width: 250px; | ||
60 | 36 | margin-left: auto; | ||
61 | 37 | margin-right: auto; | ||
62 | 38 | } | ||
63 | 39 | |||
64 | 30 | .yui3-lazr-formoverlay-form table { | 40 | .yui3-lazr-formoverlay-form table { |
65 | 31 | /* This gets rid of the 12px margin-bottom that yui specifies | 41 | /* This gets rid of the 12px margin-bottom that yui specifies |
66 | 32 | * in its base.css. | 42 | * in its base.css. |
67 | 33 | 43 | ||
68 | === modified file 'lib/lp/app/javascript/formoverlay/formoverlay.js' | |||
69 | --- lib/lp/app/javascript/formoverlay/formoverlay.js 2012-06-26 00:15:11 +0000 | |||
70 | +++ lib/lp/app/javascript/formoverlay/formoverlay.js 2012-07-27 01:21:21 +0000 | |||
71 | @@ -492,18 +492,21 @@ | |||
72 | 492 | * @method showError | 492 | * @method showError |
73 | 493 | */ | 493 | */ |
74 | 494 | showError: function(error_msgs){ | 494 | showError: function(error_msgs){ |
75 | 495 | var error_content; | ||
76 | 495 | if (Y.Lang.isString(error_msgs)) { | 496 | if (Y.Lang.isString(error_msgs)) { |
78 | 496 | error_msgs = [error_msgs]; | 497 | error_content = Y.Node.create('<p></p>').set('text', error_msgs); |
79 | 498 | } else { | ||
80 | 499 | error_content = Y.Node.create( | ||
81 | 500 | '<p>The following errors were encountered:</p><ul></ul>'); | ||
82 | 501 | var error_list = error_content.one('ul'); | ||
83 | 502 | var li_nodes = new Y.NodeList([]); | ||
84 | 503 | Y.each(error_msgs, function(error_msg){ | ||
85 | 504 | li_nodes.push( | ||
86 | 505 | Y.Node.create('<li></li>').set('text', error_msg)); | ||
87 | 506 | }); | ||
88 | 507 | error_list.append(li_nodes); | ||
89 | 497 | } | 508 | } |
99 | 498 | var error_html = "The following errors were encountered: <ul>"; | 509 | this.error_node.appendChild(error_content); |
91 | 499 | Y.each(error_msgs, function(error_msg){ | ||
92 | 500 | // XXX noodles 2009-02-13 bug=342212. We need to decide on | ||
93 | 501 | // or provide our own escapeHTML() helper. | ||
94 | 502 | error_html += "<li>" + error_msg.replace(/<([^>]+)>/g,'') + | ||
95 | 503 | "</li>"; | ||
96 | 504 | }); | ||
97 | 505 | error_html += "</ul>"; | ||
98 | 506 | this.error_node.set('innerHTML', error_html); | ||
100 | 507 | }, | 510 | }, |
101 | 508 | 511 | ||
102 | 509 | /** | 512 | /** |
103 | @@ -512,7 +515,7 @@ | |||
104 | 512 | * @method clearError | 515 | * @method clearError |
105 | 513 | */ | 516 | */ |
106 | 514 | clearError: function(){ | 517 | clearError: function(){ |
108 | 515 | this.error_node.set('innerHTML', ''); | 518 | this.error_node.empty(true); |
109 | 516 | }, | 519 | }, |
110 | 517 | 520 | ||
111 | 518 | /** | 521 | /** |
112 | 519 | 522 | ||
113 | === modified file 'lib/lp/bugs/javascript/duplicates.js' | |||
114 | --- lib/lp/bugs/javascript/duplicates.js 2012-07-25 00:31:04 +0000 | |||
115 | +++ lib/lp/bugs/javascript/duplicates.js 2012-07-27 01:21:21 +0000 | |||
116 | @@ -13,10 +13,10 @@ | |||
117 | 13 | // Overlay related vars. | 13 | // Overlay related vars. |
118 | 14 | var submit_button_html = | 14 | var submit_button_html = |
119 | 15 | '<button type="submit" name="field.actions.change" ' + | 15 | '<button type="submit" name="field.actions.change" ' + |
121 | 16 | 'value="Change" class="lazr-pos lazr-btn" >OK</button>'; | 16 | '>Save Duplicate</button>'; |
122 | 17 | var cancel_button_html = | 17 | var cancel_button_html = |
123 | 18 | '<button type="button" name="field.actions.cancel" ' + | 18 | '<button type="button" name="field.actions.cancel" ' + |
125 | 19 | 'class="lazr-neg lazr-btn" >Cancel</button>'; | 19 | '>Cancel</button>'; |
126 | 20 | 20 | ||
127 | 21 | /** | 21 | /** |
128 | 22 | * Manage the process of marking a bug as a duplicate. | 22 | * Manage the process of marking a bug as a duplicate. |
129 | @@ -24,6 +24,8 @@ | |||
130 | 24 | */ | 24 | */ |
131 | 25 | namespace.MarkBugDuplicate = Y.Base.create("markBugDupeWidget", Y.Widget, [], { | 25 | namespace.MarkBugDuplicate = Y.Base.create("markBugDupeWidget", Y.Widget, [], { |
132 | 26 | initializer: function(cfg) { | 26 | initializer: function(cfg) { |
133 | 27 | this.lp_client = new Y.lp.client.Launchpad(cfg); | ||
134 | 28 | this.io_provider = Y.lp.client.get_configured_io_provider(cfg); | ||
135 | 27 | var update_dupe_link = cfg.update_dupe_link; | 29 | var update_dupe_link = cfg.update_dupe_link; |
136 | 28 | if (update_dupe_link) { | 30 | if (update_dupe_link) { |
137 | 29 | // Add a class denoting the link as js-action link. | 31 | // Add a class denoting the link as js-action link. |
138 | @@ -55,19 +57,20 @@ | |||
139 | 55 | '</p>'; | 57 | '</p>'; |
140 | 56 | } | 58 | } |
141 | 57 | 59 | ||
143 | 58 | this.duplicate_form_overlay = new Y.lazr.FormOverlay({ | 60 | this.duplicate_form = new Y.lazr.FormOverlay({ |
144 | 59 | headerContent: '<h2>Mark bug report as duplicate</h2>', | 61 | headerContent: '<h2>Mark bug report as duplicate</h2>', |
145 | 60 | form_header: form_header, | 62 | form_header: form_header, |
146 | 61 | form_submit_button: Y.Node.create(submit_button_html), | 63 | form_submit_button: Y.Node.create(submit_button_html), |
147 | 62 | form_cancel_button: Y.Node.create(cancel_button_html), | 64 | form_cancel_button: Y.Node.create(cancel_button_html), |
148 | 63 | centered: true, | 65 | centered: true, |
149 | 66 | // The form submit first searches for the bug. | ||
150 | 64 | form_submit_callback: | 67 | form_submit_callback: |
152 | 65 | Y.bind(this._update_bug_duplicate, this), | 68 | Y.bind(this._find_duplicate_bug, this), |
153 | 66 | visible: false, | 69 | visible: false, |
155 | 67 | io_provider: this.get('io_provider') | 70 | io_provider: this.io_provider |
156 | 68 | }); | 71 | }); |
159 | 69 | this.duplicate_form_overlay.render('#duplicate-form-container'); | 72 | this.duplicate_form.render('#duplicate-form-container'); |
160 | 70 | this.duplicate_form_overlay.loadFormContentAndRender( | 73 | this.duplicate_form.loadFormContentAndRender( |
161 | 71 | mark_dupe_form_url); | 74 | mark_dupe_form_url); |
162 | 72 | }, | 75 | }, |
163 | 73 | 76 | ||
164 | @@ -82,12 +85,251 @@ | |||
165 | 82 | update_dupe_link.on('click', function(e) { | 85 | update_dupe_link.on('click', function(e) { |
166 | 83 | // Only go ahead if we have received the form content by the | 86 | // Only go ahead if we have received the form content by the |
167 | 84 | // time the user clicks: | 87 | // time the user clicks: |
169 | 85 | if (that.duplicate_form_overlay) { | 88 | if (that.duplicate_form) { |
170 | 86 | e.halt(); | 89 | e.halt(); |
172 | 87 | that.duplicate_form_overlay.show(); | 90 | that.duplicate_form.show(); |
173 | 88 | Y.DOM.byId('field.duplicateof').focus(); | 91 | Y.DOM.byId('field.duplicateof').focus(); |
174 | 89 | } | 92 | } |
175 | 90 | }); | 93 | }); |
176 | 94 | // When the dupe form overlay is hidden, we need to reset the form. | ||
177 | 95 | this.duplicate_form.after('visibleChange', function() { | ||
178 | 96 | if (!this.get('visible')) { | ||
179 | 97 | that._hide_confirm_node(); | ||
180 | 98 | } | ||
181 | 99 | }); | ||
182 | 100 | }, | ||
183 | 101 | |||
184 | 102 | /** | ||
185 | 103 | * Show a spinner next to the specified node. | ||
186 | 104 | * | ||
187 | 105 | * @method _show_bug_spinner | ||
188 | 106 | * @private | ||
189 | 107 | */ | ||
190 | 108 | _show_bug_spinner: function(node) { | ||
191 | 109 | if( node === null) { | ||
192 | 110 | return; | ||
193 | 111 | } | ||
194 | 112 | var spinner_node = Y.Node.create( | ||
195 | 113 | '<img class="spinner" src="/@@/spinner" alt="Searching..." />'); | ||
196 | 114 | node.insert(spinner_node, 'after'); | ||
197 | 115 | }, | ||
198 | 116 | |||
199 | 117 | /** | ||
200 | 118 | * Hide the spinner next to the specified node. | ||
201 | 119 | * | ||
202 | 120 | * @method _hide_bug_spinner | ||
203 | 121 | * @private | ||
204 | 122 | */ | ||
205 | 123 | _hide_bug_spinner: function(node) { | ||
206 | 124 | if( node === null) { | ||
207 | 125 | return; | ||
208 | 126 | } | ||
209 | 127 | var spinner = node.get('parentNode').one('.spinner'); | ||
210 | 128 | if (spinner !== null) { | ||
211 | 129 | spinner.remove(); | ||
212 | 130 | } | ||
213 | 131 | }, | ||
214 | 132 | |||
215 | 133 | /** | ||
216 | 134 | * Look up the selected bug and get the user to confirm that it is the one | ||
217 | 135 | * they want. | ||
218 | 136 | * | ||
219 | 137 | * @param data | ||
220 | 138 | * @private | ||
221 | 139 | */ | ||
222 | 140 | _find_duplicate_bug: function(data) { | ||
223 | 141 | var new_dup_id = Y.Lang.trim(data['field.duplicateof'][0]); | ||
224 | 142 | // If there's no bug data entered then we are deleting the duplicate | ||
225 | 143 | // link. | ||
226 | 144 | if (new_dup_id === '') { | ||
227 | 145 | this._update_bug_duplicate(new_dup_id); | ||
228 | 146 | return; | ||
229 | 147 | } | ||
230 | 148 | var that = this; | ||
231 | 149 | var qs_data | ||
232 | 150 | = Y.lp.client.append_qs("", "ws.accept", "application.json"); | ||
233 | 151 | |||
234 | 152 | var bug_field = this.duplicate_form.form_node | ||
235 | 153 | .one('[id="field.duplicateof"]'); | ||
236 | 154 | var config = { | ||
237 | 155 | on: { | ||
238 | 156 | start: function() { | ||
239 | 157 | that._show_bug_spinner(bug_field); | ||
240 | 158 | that.duplicate_form.clearError(); | ||
241 | 159 | }, | ||
242 | 160 | end: function() { | ||
243 | 161 | that._hide_bug_spinner(bug_field); | ||
244 | 162 | }, | ||
245 | 163 | success: function(id, response) { | ||
246 | 164 | if (response.responseText === '') { | ||
247 | 165 | return; | ||
248 | 166 | } | ||
249 | 167 | var bug_data = Y.JSON.parse(response.responseText); | ||
250 | 168 | that._confirm_selected_bug(bug_data); | ||
251 | 169 | }, | ||
252 | 170 | failure: function(id, response) { | ||
253 | 171 | var error_msg; | ||
254 | 172 | if (response.status === 404) { | ||
255 | 173 | error_msg = new_dup_id + | ||
256 | 174 | ' is not a valid bug number.'; | ||
257 | 175 | } else { | ||
258 | 176 | error_msg = response.responseText; | ||
259 | 177 | } | ||
260 | 178 | that.duplicate_form.showError(error_msg); | ||
261 | 179 | } | ||
262 | 180 | }, | ||
263 | 181 | data: qs_data | ||
264 | 182 | }; | ||
265 | 183 | var uri | ||
266 | 184 | = Y.lp.client.get_absolute_uri("/api/devel/bugs/" + new_dup_id); | ||
267 | 185 | this.io_provider.io(uri, config); | ||
268 | 186 | }, | ||
269 | 187 | |||
270 | 188 | // Template for rendering the bug details. | ||
271 | 189 | _bug_details_template: function() { | ||
272 | 190 | return [ | ||
273 | 191 | '<div id="client-listing">', | ||
274 | 192 | ' <div class="buglisting-col1">', | ||
275 | 193 | ' <div class="importance {{importance_class}}">', | ||
276 | 194 | ' {{importance}}', | ||
277 | 195 | ' </div>', | ||
278 | 196 | ' <div class="status {{status_class}}">', | ||
279 | 197 | ' {{status}}', | ||
280 | 198 | ' </div>', | ||
281 | 199 | ' <div class="buginfo-extra">', | ||
282 | 200 | ' <div class="information_type">', | ||
283 | 201 | ' {{information_type}}', | ||
284 | 202 | ' </div>', | ||
285 | 203 | ' </div>', | ||
286 | 204 | ' </div>', | ||
287 | 205 | ' <div class="buglisting-col2" style="max-width: 60em;">', | ||
288 | 206 | ' <span class="bugnumber">#{{id}}</span>', | ||
289 | 207 | ' <a href="{{bug_url}}" class="bugtitle">{{bug_summary}}</a>', | ||
290 | 208 | ' <div class="buginfo-extra">', | ||
291 | 209 | ' <p class="ellipsis line-block" style="max-width: 70em;">', | ||
292 | 210 | ' {{description}}</p>', | ||
293 | 211 | ' </div>', | ||
294 | 212 | ' </div>', | ||
295 | 213 | '</div>' | ||
296 | 214 | ].join(' '); | ||
297 | 215 | }, | ||
298 | 216 | |||
299 | 217 | // Template for the bug confirmation form. | ||
300 | 218 | _bug_confirmation_form_template: function() { | ||
301 | 219 | return [ | ||
302 | 220 | '<div class="confirmation-node yui3-lazr-formoverlay-form" ', | ||
303 | 221 | 'style="margin-top: 6px;">', | ||
304 | 222 | '{{> bug_details}}', | ||
305 | 223 | '<div class="yui3-lazr-formoverlay-errors"></div>', | ||
306 | 224 | '<div class="extra-form-buttons">', | ||
307 | 225 | ' <button class="yes_button" type="button">', | ||
308 | 226 | ' Save Duplicate</button>', | ||
309 | 227 | ' <button class="no_button" type="button">Search Again</button>', | ||
310 | 228 | ' <button class="cancel_button" type="button">Cancel</button>', | ||
311 | 229 | '</div>', | ||
312 | 230 | '</div>'].join(''); | ||
313 | 231 | }, | ||
314 | 232 | |||
315 | 233 | /** | ||
316 | 234 | * Ask the user to confirm the chosen bug is the one they want. | ||
317 | 235 | * @param bug_data | ||
318 | 236 | * @private | ||
319 | 237 | */ | ||
320 | 238 | _confirm_selected_bug: function(bug_data) { | ||
321 | 239 | // TODO - get real data from the server | ||
322 | 240 | bug_data.importance = 'High'; | ||
323 | 241 | bug_data.importance_class = 'importanceHIGH'; | ||
324 | 242 | bug_data.status = 'Triaged'; | ||
325 | 243 | bug_data.status_class = 'statusTRIAGED'; | ||
326 | 244 | bug_data.bug_summary = bug_data.title; | ||
327 | 245 | bug_data.bug_url = bug_data.web_link; | ||
328 | 246 | |||
329 | 247 | var bug_id = bug_data.id; | ||
330 | 248 | var html = Y.lp.mustache.to_html( | ||
331 | 249 | this._bug_confirmation_form_template(), bug_data, | ||
332 | 250 | {bug_details: this._bug_details_template()}); | ||
333 | 251 | var confirm_node = Y.Node.create(html); | ||
334 | 252 | this._show_confirm_node(confirm_node); | ||
335 | 253 | var that = this; | ||
336 | 254 | confirm_node.one(".yes_button") | ||
337 | 255 | .on('click', function(e) { | ||
338 | 256 | e.halt(); | ||
339 | 257 | that._update_bug_duplicate(bug_id); | ||
340 | 258 | }); | ||
341 | 259 | |||
342 | 260 | confirm_node.one(".no_button") | ||
343 | 261 | .on('click', function(e) { | ||
344 | 262 | e.halt(); | ||
345 | 263 | that._hide_confirm_node(confirm_node); | ||
346 | 264 | }); | ||
347 | 265 | confirm_node.one(".cancel_button") | ||
348 | 266 | .on('click', function(e) { | ||
349 | 267 | e.halt(); | ||
350 | 268 | that.duplicate_form.hide(); | ||
351 | 269 | }); | ||
352 | 270 | }, | ||
353 | 271 | |||
354 | 272 | // Centre the duplicate form along the x axis without changing y position. | ||
355 | 273 | _xaxis_centre: function() { | ||
356 | 274 | var viewport = Y.DOM.viewportRegion(); | ||
357 | 275 | var new_x = (viewport.right + viewport.left)/2 - | ||
358 | 276 | this.duplicate_form.get('contentBox').get('offsetWidth')/2; | ||
359 | 277 | this.duplicate_form.move([new_x, this.duplicate_form._getY()]); | ||
360 | 278 | |||
361 | 279 | }, | ||
362 | 280 | |||
363 | 281 | /** Show the bug selection confirmation node. | ||
364 | 282 | * @method _show_confirm_node | ||
365 | 283 | * @private | ||
366 | 284 | */ | ||
367 | 285 | _show_confirm_node: function(confirmation_node) { | ||
368 | 286 | this.duplicate_form.form_header_node | ||
369 | 287 | .insert(confirmation_node, 'after'); | ||
370 | 288 | this.confirmation_node = confirmation_node; | ||
371 | 289 | this._xaxis_centre(); | ||
372 | 290 | this._fade_in(confirmation_node, this.duplicate_form.form_node); | ||
373 | 291 | }, | ||
374 | 292 | |||
375 | 293 | /** Hide the bug selection confirmation node. | ||
376 | 294 | * @method _hide_confirm_node | ||
377 | 295 | * @private | ||
378 | 296 | */ | ||
379 | 297 | _hide_confirm_node: function() { | ||
380 | 298 | this.duplicate_form.form_node.removeClass('hidden'); | ||
381 | 299 | if (Y.Lang.isValue(this.confirmation_node)) { | ||
382 | 300 | this._fade_in( | ||
383 | 301 | this.duplicate_form.form_node, this.confirmation_node); | ||
384 | 302 | this._xaxis_centre(); | ||
385 | 303 | this.confirmation_node.remove(); | ||
386 | 304 | this.confirmation_node = null; | ||
387 | 305 | } | ||
388 | 306 | }, | ||
389 | 307 | |||
390 | 308 | // Animate the display of content. | ||
391 | 309 | _fade_in: function(content_node, old_content, use_animation) { | ||
392 | 310 | content_node.removeClass('hidden'); | ||
393 | 311 | if (old_content === null) { | ||
394 | 312 | content_node.removeClass('transparent'); | ||
395 | 313 | content_node.setStyle('opacity', 1); | ||
396 | 314 | content_node.show(); | ||
397 | 315 | return; | ||
398 | 316 | } | ||
399 | 317 | old_content.addClass('hidden'); | ||
400 | 318 | if (!Y.Lang.isValue(use_animation)) { | ||
401 | 319 | use_animation = this.get('use_animation'); | ||
402 | 320 | } | ||
403 | 321 | if (!use_animation) { | ||
404 | 322 | old_content.setStyle('opacity', 1); | ||
405 | 323 | return; | ||
406 | 324 | } | ||
407 | 325 | content_node.addClass('transparent'); | ||
408 | 326 | content_node.setStyle('opacity', 0); | ||
409 | 327 | var fade_in = new Y.Anim({ | ||
410 | 328 | node: content_node, | ||
411 | 329 | to: {opacity: 1}, | ||
412 | 330 | duration: 0.8 | ||
413 | 331 | }); | ||
414 | 332 | fade_in.run(); | ||
415 | 91 | }, | 333 | }, |
416 | 92 | 334 | ||
417 | 93 | /** | 335 | /** |
418 | @@ -101,6 +343,7 @@ | |||
419 | 101 | */ | 343 | */ |
420 | 102 | _update_bug_duplicate_success: function(updated_entry, new_dup_url, | 344 | _update_bug_duplicate_success: function(updated_entry, new_dup_url, |
421 | 103 | new_dup_id) { | 345 | new_dup_id) { |
422 | 346 | this.duplicate_form.hide(); | ||
423 | 104 | updated_entry.set('duplicate_of_link', new_dup_url); | 347 | updated_entry.set('duplicate_of_link', new_dup_url); |
424 | 105 | this.set('lp_bug_entry', updated_entry); | 348 | this.set('lp_bug_entry', updated_entry); |
425 | 106 | 349 | ||
426 | @@ -129,16 +372,20 @@ | |||
427 | 129 | dupe_span.one('a').set('href', update_dup_url); | 372 | dupe_span.one('a').set('href', update_dup_url); |
428 | 130 | this._hide_comment_on_duplicate_warning(); | 373 | this._hide_comment_on_duplicate_warning(); |
429 | 131 | } | 374 | } |
430 | 375 | var anim_duration = 1; | ||
431 | 376 | if (!this.get('anim_duration')) { | ||
432 | 377 | anim_duration = 0; | ||
433 | 378 | } | ||
434 | 132 | Y.lp.anim.green_flash({ | 379 | Y.lp.anim.green_flash({ |
435 | 133 | node: dupe_span, | 380 | node: dupe_span, |
437 | 134 | duration: this.get('anim_duration') | 381 | duration: anim_duration |
438 | 135 | }).run(); | 382 | }).run(); |
439 | 136 | // ensure the new link is hooked up correctly: | 383 | // ensure the new link is hooked up correctly: |
440 | 137 | var that = this; | 384 | var that = this; |
441 | 138 | dupe_span.one('a').on( | 385 | dupe_span.one('a').on( |
442 | 139 | 'click', function(e){ | 386 | 'click', function(e){ |
443 | 140 | e.preventDefault(); | 387 | e.preventDefault(); |
445 | 141 | that.duplicate_form_overlay.show(); | 388 | that.duplicate_form.show(); |
446 | 142 | Y.DOM.byId('field.duplicateof').focus(); | 389 | Y.DOM.byId('field.duplicateof').focus(); |
447 | 143 | }); | 390 | }); |
448 | 144 | }, | 391 | }, |
449 | @@ -156,23 +403,26 @@ | |||
450 | 156 | // Reset the lp_bug_entry.duplicate_of_link as it wasn't | 403 | // Reset the lp_bug_entry.duplicate_of_link as it wasn't |
451 | 157 | // updated. | 404 | // updated. |
452 | 158 | this.get('lp_bug_entry').set('duplicate_of_link', old_dup_url); | 405 | this.get('lp_bug_entry').set('duplicate_of_link', old_dup_url); |
453 | 406 | var error_msg = response.responseText; | ||
454 | 159 | if (response.status === 400) { | 407 | if (response.status === 400) { |
459 | 160 | this.duplicate_form_overlay.showError( | 408 | var error_info = response.responseText.split('\n'); |
460 | 161 | new_dup_id + ' is not a valid bug number or nickname.'); | 409 | error_msg = error_info.slice(1).join(' '); |
457 | 162 | } else { | ||
458 | 163 | this.duplicate_form_overlay.showError(response.responseText); | ||
461 | 164 | } | 410 | } |
463 | 165 | this.duplicate_form_overlay.show(); | 411 | this.confirmation_node.one('.yui3-lazr-formoverlay-errors') |
464 | 412 | .setContent(error_msg); | ||
465 | 413 | this.confirmation_node.one(".yes_button").addClass('hidden'); | ||
466 | 414 | this._xaxis_centre(); | ||
467 | 415 | |||
468 | 166 | }, | 416 | }, |
469 | 167 | 417 | ||
470 | 168 | /** | 418 | /** |
471 | 169 | * Update the bug duplicate via the LP API | 419 | * Update the bug duplicate via the LP API |
472 | 170 | * | 420 | * |
473 | 171 | * @method _update_bug_duplicate | 421 | * @method _update_bug_duplicate |
475 | 172 | * @param data | 422 | * @param new_dup_id |
476 | 173 | * @private | 423 | * @private |
477 | 174 | */ | 424 | */ |
479 | 175 | _update_bug_duplicate: function(data) { | 425 | _update_bug_duplicate: function(new_dup_id) { |
480 | 176 | // XXX noodles 2009-03-17 bug=336866 It seems the etag | 426 | // XXX noodles 2009-03-17 bug=336866 It seems the etag |
481 | 177 | // returned by lp_save() is incorrect. Remove it for now | 427 | // returned by lp_save() is incorrect. Remove it for now |
482 | 178 | // so that the second save does not result in a '412 | 428 | // so that the second save does not result in a '412 |
483 | @@ -186,11 +436,7 @@ | |||
484 | 186 | var lp_bug_entry = this.get('lp_bug_entry'); | 436 | var lp_bug_entry = this.get('lp_bug_entry'); |
485 | 187 | lp_bug_entry.removeAttr('http_etag'); | 437 | lp_bug_entry.removeAttr('http_etag'); |
486 | 188 | 438 | ||
487 | 189 | // Hide the formoverlay: | ||
488 | 190 | this.duplicate_form_overlay.hide(); | ||
489 | 191 | |||
490 | 192 | var new_dup_url = null; | 439 | var new_dup_url = null; |
491 | 193 | var new_dup_id = Y.Lang.trim(data['field.duplicateof'][0]); | ||
492 | 194 | if (new_dup_id !== '') { | 440 | if (new_dup_id !== '') { |
493 | 195 | var self_link = lp_bug_entry.get('self_link'); | 441 | var self_link = lp_bug_entry.get('self_link'); |
494 | 196 | var last_slash_index = self_link.lastIndexOf('/'); | 442 | var last_slash_index = self_link.lastIndexOf('/'); |
495 | @@ -201,14 +447,20 @@ | |||
496 | 201 | 447 | ||
497 | 202 | var dupe_span = this.get('dupe_span'); | 448 | var dupe_span = this.get('dupe_span'); |
498 | 203 | var that = this; | 449 | var that = this; |
499 | 450 | var submit_btn = null; | ||
500 | 451 | if (Y.Lang.isValue(this.confirmation_node)) { | ||
501 | 452 | submit_btn = this.confirmation_node.one(".yes_button"); | ||
502 | 453 | } | ||
503 | 204 | var config = { | 454 | var config = { |
504 | 205 | on: { | 455 | on: { |
505 | 206 | start: function() { | 456 | start: function() { |
506 | 207 | dupe_span.removeClass('sprite bug-dupe'); | 457 | dupe_span.removeClass('sprite bug-dupe'); |
507 | 208 | dupe_span.addClass('update-in-progress-message'); | 458 | dupe_span.addClass('update-in-progress-message'); |
508 | 459 | that._show_bug_spinner(submit_btn); | ||
509 | 209 | }, | 460 | }, |
510 | 210 | end: function() { | 461 | end: function() { |
511 | 211 | dupe_span.removeClass('update-in-progress-message'); | 462 | dupe_span.removeClass('update-in-progress-message'); |
512 | 463 | that._hide_bug_spinner(submit_btn); | ||
513 | 212 | }, | 464 | }, |
514 | 213 | success: function(updated_entry) { | 465 | success: function(updated_entry) { |
515 | 214 | that._update_bug_duplicate_success( | 466 | that._update_bug_duplicate_success( |
516 | @@ -233,7 +485,7 @@ | |||
517 | 233 | */ | 485 | */ |
518 | 234 | _show_comment_on_duplicate_warning: function() { | 486 | _show_comment_on_duplicate_warning: function() { |
519 | 235 | var duplicate_warning = Y.one('#warning-comment-on-duplicate'); | 487 | var duplicate_warning = Y.one('#warning-comment-on-duplicate'); |
521 | 236 | if (duplicate_warning === null) { | 488 | if (!Y.Lang.isValue(duplicate_warning)) { |
522 | 237 | var container = Y.one('#add-comment-form'); | 489 | var container = Y.one('#add-comment-form'); |
523 | 238 | var first_node = container.get('firstChild'); | 490 | var first_node = container.get('firstChild'); |
524 | 239 | duplicate_warning = Y.Node.create( | 491 | duplicate_warning = Y.Node.create( |
525 | @@ -286,16 +538,13 @@ | |||
526 | 286 | } | 538 | } |
527 | 287 | }, | 539 | }, |
528 | 288 | // Override for testing. | 540 | // Override for testing. |
534 | 289 | io_provider: { | 541 | use_animation: { |
535 | 290 | }, | 542 | value: true |
531 | 291 | // Override for testing. | ||
532 | 292 | anim_duration: { | ||
533 | 293 | value: 1 | ||
536 | 294 | } | 543 | } |
537 | 295 | } | 544 | } |
538 | 296 | }); | 545 | }); |
539 | 297 | 546 | ||
540 | 298 | }, "0.1", {"requires": [ | 547 | }, "0.1", {"requires": [ |
541 | 299 | "base", "io", "oop", "node", "event", "json", "lazr.formoverlay", | 548 | "base", "io", "oop", "node", "event", "json", "lazr.formoverlay", |
543 | 300 | "lazr.effects", "lp.app.widgets.expander", | 549 | "lazr.effects", "lp.app.widgets.expander", "lp.mustache", |
544 | 301 | "lp.app.formwidgets.resizing_textarea", "plugin"]}); | 550 | "lp.app.formwidgets.resizing_textarea", "plugin"]}); |
545 | 302 | 551 | ||
546 | === modified file 'lib/lp/bugs/javascript/tests/test_duplicates.html' | |||
547 | --- lib/lp/bugs/javascript/tests/test_duplicates.html 2012-07-25 00:31:04 +0000 | |||
548 | +++ lib/lp/bugs/javascript/tests/test_duplicates.html 2012-07-27 01:21:21 +0000 | |||
549 | @@ -27,6 +27,7 @@ | |||
550 | 27 | <link rel="stylesheet" href="../../../app/javascript/testing/test.css" /> | 27 | <link rel="stylesheet" href="../../../app/javascript/testing/test.css" /> |
551 | 28 | 28 | ||
552 | 29 | <!-- Dependencies --> | 29 | <!-- Dependencies --> |
553 | 30 | <script type="text/javascript" src="../../../../../build/js/lp/app/mustache.js"></script> | ||
554 | 30 | <script type="text/javascript" src="../../../../../build/js/lp/app/anim/anim.js"></script> | 31 | <script type="text/javascript" src="../../../../../build/js/lp/app/anim/anim.js"></script> |
555 | 31 | <script type="text/javascript" src="../../../../../build/js/lp/app/client.js"></script> | 32 | <script type="text/javascript" src="../../../../../build/js/lp/app/client.js"></script> |
556 | 32 | <script type="text/javascript" src="../../../../../build/js/lp/app/errors.js"></script> | 33 | <script type="text/javascript" src="../../../../../build/js/lp/app/errors.js"></script> |
557 | 33 | 34 | ||
558 | === modified file 'lib/lp/bugs/javascript/tests/test_duplicates.js' | |||
559 | --- lib/lp/bugs/javascript/tests/test_duplicates.js 2012-07-25 12:13:38 +0000 | |||
560 | +++ lib/lp/bugs/javascript/tests/test_duplicates.js 2012-07-27 01:21:21 +0000 | |||
561 | @@ -51,7 +51,7 @@ | |||
562 | 51 | var widget = new Y.lp.bugs.duplicates.MarkBugDuplicate({ | 51 | var widget = new Y.lp.bugs.duplicates.MarkBugDuplicate({ |
563 | 52 | srcNode: '#duplicate-actions', | 52 | srcNode: '#duplicate-actions', |
564 | 53 | lp_bug_entry: this.lp_bug_entry, | 53 | lp_bug_entry: this.lp_bug_entry, |
566 | 54 | anim_duration: 0, | 54 | use_animation: false, |
567 | 55 | io_provider: this.mockio | 55 | io_provider: this.mockio |
568 | 56 | }); | 56 | }); |
569 | 57 | widget.render(); | 57 | widget.render(); |
570 | @@ -97,8 +97,8 @@ | |||
571 | 97 | Y.Assert.areEqual('http://foo/+duplicate', url); | 97 | Y.Assert.areEqual('http://foo/+duplicate', url); |
572 | 98 | }, | 98 | }, |
573 | 99 | 99 | ||
576 | 100 | // The duplicate entry form renders and submits the expected data. | 100 | // The search form renders and submits the expected data. |
577 | 101 | _assert_duplicate_form_submission: function(bug_id) { | 101 | _assert_search_form_submission: function(bug_id) { |
578 | 102 | var form = Y.one('#duplicate-form-container'); | 102 | var form = Y.one('#duplicate-form-container'); |
579 | 103 | Y.Assert.isTrue( | 103 | Y.Assert.isTrue( |
580 | 104 | form.one('div.pretty-overlay-window') | 104 | form.one('div.pretty-overlay-window') |
581 | @@ -108,15 +108,123 @@ | |||
582 | 108 | Y.one('#duplicate-form-container div.pretty-overlay-window') | 108 | Y.one('#duplicate-form-container div.pretty-overlay-window') |
583 | 109 | .hasClass('yui3-lazr-formoverlay-hidden')); | 109 | .hasClass('yui3-lazr-formoverlay-hidden')); |
584 | 110 | Y.DOM.byId('field.duplicateof').value = bug_id; | 110 | Y.DOM.byId('field.duplicateof').value = bug_id; |
590 | 111 | form.one('.lazr-pos').simulate('click'); | 111 | form.one('[name="field.actions.change"]').simulate('click'); |
591 | 112 | Y.Assert.areEqual( | 112 | var expected_url = '/api/devel/bugs/1'; |
587 | 113 | '/api/devel/bugs/1', | ||
588 | 114 | this.mockio.last_request.url); | ||
589 | 115 | var expected_link = '{}'; | ||
592 | 116 | if (bug_id !== '') { | 113 | if (bug_id !== '') { |
594 | 117 | expected_link = | 114 | expected_url = 'file:///api/devel/bugs/' + bug_id; |
595 | 115 | } | ||
596 | 116 | Y.Assert.areEqual(expected_url, this.mockio.last_request.url); | ||
597 | 117 | }, | ||
598 | 118 | |||
599 | 119 | // The bug entry form is visible and the confirmation form is invisible | ||
600 | 120 | // or visa versa. | ||
601 | 121 | _assert_form_state: function(confirm_form_visible) { | ||
602 | 122 | Y.Assert.areEqual( | ||
603 | 123 | confirm_form_visible, | ||
604 | 124 | Y.one('#duplicate-form-container form') | ||
605 | 125 | .hasClass('hidden')); | ||
606 | 126 | var bug_info = Y.one('#duplicate-form-container ' + | ||
607 | 127 | '.confirmation-node #client-listing'); | ||
608 | 128 | if (confirm_form_visible) { | ||
609 | 129 | Y.Assert.isNotNull(bug_info); | ||
610 | 130 | } else { | ||
611 | 131 | Y.Assert.isNull(bug_info); | ||
612 | 132 | } | ||
613 | 133 | }, | ||
614 | 134 | |||
615 | 135 | // Invoke a successful search operation and check the form state. | ||
616 | 136 | _assert_search_form_success: function(bug_id) { | ||
617 | 137 | var expected_updated_entry = { | ||
618 | 138 | id: bug_id, | ||
619 | 139 | uri: 'api/devel/bugs/' + bug_id, | ||
620 | 140 | duplicate_of_link: 'api/devel/bugs/' + bug_id, | ||
621 | 141 | self_link: 'api/devel/bugs/' + bug_id}; | ||
622 | 142 | this.mockio.last_request.successJSON(expected_updated_entry); | ||
623 | 143 | this._assert_form_state(true); | ||
624 | 144 | }, | ||
625 | 145 | |||
626 | 146 | // A successful search for a bug displays the confirmation form. | ||
627 | 147 | test_initial_bug_search_success: function() { | ||
628 | 148 | this.widget = this._createWidget(false); | ||
629 | 149 | this._assert_search_form_submission(3); | ||
630 | 150 | this._assert_search_form_success(3); | ||
631 | 151 | }, | ||
632 | 152 | |||
633 | 153 | // After a successful search, hitting the Search Again button takes us | ||
634 | 154 | // back to the bug details entry form. | ||
635 | 155 | test_initial_bug_search_try_again: function() { | ||
636 | 156 | this.widget = this._createWidget(false); | ||
637 | 157 | this._assert_search_form_submission(3); | ||
638 | 158 | this._assert_search_form_success(3); | ||
639 | 159 | Y.one('#duplicate-form-container .no_button') | ||
640 | 160 | .simulate('click'); | ||
641 | 161 | this._assert_form_state(false); | ||
642 | 162 | }, | ||
643 | 163 | |||
644 | 164 | // After a successful search, hitting the Select bug button initiates | ||
645 | 165 | // the mark as duplicate operation. | ||
646 | 166 | test_bug_search_select_bug: function() { | ||
647 | 167 | this.widget = this._createWidget(false); | ||
648 | 168 | this._assert_search_form_submission(3); | ||
649 | 169 | this._assert_search_form_success(3); | ||
650 | 170 | var update_bug_duplicate_called = false; | ||
651 | 171 | this.widget._update_bug_duplicate = function(bug_id) { | ||
652 | 172 | update_bug_duplicate_called = true; | ||
653 | 173 | Y.Assert.areEqual(3, bug_id); | ||
654 | 174 | }; | ||
655 | 175 | Y.one('#duplicate-form-container .yes_button') | ||
656 | 176 | .simulate('click'); | ||
657 | 177 | this._assert_form_state(true); | ||
658 | 178 | Y.Assert.isTrue(update_bug_duplicate_called); | ||
659 | 179 | }, | ||
660 | 180 | |||
661 | 181 | // The specified error message is displayed. | ||
662 | 182 | _assert_error_display: function(message) { | ||
663 | 183 | var selector | ||
664 | 184 | = '#duplicate-form-container div.pretty-overlay-window'; | ||
665 | 185 | Y.Assert.isFalse( | ||
666 | 186 | Y.one(selector).hasClass('yui3-lazr-formoverlay-hidden')); | ||
667 | 187 | var error_msg = Y.one('.yui3-lazr-formoverlay-errors p'); | ||
668 | 188 | Y.Assert.areEqual(message, error_msg.get('text')); | ||
669 | 189 | }, | ||
670 | 190 | |||
671 | 191 | // The error is displayed as expected when the initial bug search | ||
672 | 192 | // fails with a generic error. | ||
673 | 193 | test_initial_bug_search_generic_failure: function() { | ||
674 | 194 | this.widget = this._createWidget(false); | ||
675 | 195 | this._assert_search_form_submission(3); | ||
676 | 196 | var response = { | ||
677 | 197 | status: 500, | ||
678 | 198 | responseText: 'An error occurred' | ||
679 | 199 | }; | ||
680 | 200 | this.mockio.respond(response); | ||
681 | 201 | this._assert_error_display('An error occurred'); | ||
682 | 202 | }, | ||
683 | 203 | |||
684 | 204 | // The error is displayed as expected when the initial bug search | ||
685 | 205 | // fails with a 404 (invalid/not found bug id). | ||
686 | 206 | test_initial_bug_search_invalid_bug_failure: function() { | ||
687 | 207 | this.widget = this._createWidget(false); | ||
688 | 208 | this._assert_search_form_submission(3); | ||
689 | 209 | var response = { | ||
690 | 210 | status: 404, | ||
691 | 211 | responseText: 'An error occurred' | ||
692 | 212 | }; | ||
693 | 213 | this.mockio.respond(response); | ||
694 | 214 | this._assert_error_display('3 is not a valid bug number.'); | ||
695 | 215 | }, | ||
696 | 216 | |||
697 | 217 | // The duplicate entry form renders and submits the expected data. | ||
698 | 218 | _assert_confirmation_form_submission: function(bug_id) { | ||
699 | 219 | this._assert_search_form_submission(bug_id); | ||
700 | 220 | this._assert_search_form_success(bug_id); | ||
701 | 221 | Y.one('#duplicate-form-container .yes_button') | ||
702 | 222 | .simulate('click'); | ||
703 | 223 | this._assert_form_state(true); | ||
704 | 224 | Y.Assert.areEqual( | ||
705 | 225 | '/api/devel/bugs/1', this.mockio.last_request.url); | ||
706 | 226 | var expected_link = | ||
707 | 118 | '{"duplicate_of_link":"api/devel/bugs/' + bug_id + '"}'; | 227 | '{"duplicate_of_link":"api/devel/bugs/' + bug_id + '"}'; |
708 | 119 | } | ||
709 | 120 | Y.Assert.areEqual( | 228 | Y.Assert.areEqual( |
710 | 121 | expected_link, this.mockio.last_request.config.data); | 229 | expected_link, this.mockio.last_request.config.data); |
711 | 122 | }, | 230 | }, |
712 | @@ -124,6 +232,7 @@ | |||
713 | 124 | // Submitting a bug dupe works as expected. | 232 | // Submitting a bug dupe works as expected. |
714 | 125 | test_duplicate_form_submission_success: function() { | 233 | test_duplicate_form_submission_success: function() { |
715 | 126 | this.widget = this._createWidget(false); | 234 | this.widget = this._createWidget(false); |
716 | 235 | this._assert_confirmation_form_submission(3); | ||
717 | 127 | var success_called = false; | 236 | var success_called = false; |
718 | 128 | this.widget._update_bug_duplicate_success = | 237 | this.widget._update_bug_duplicate_success = |
719 | 129 | function(updated_entry, new_dup_url, new_dup_id) { | 238 | function(updated_entry, new_dup_url, new_dup_id) { |
720 | @@ -134,7 +243,6 @@ | |||
721 | 134 | Y.Assert.areEqual(3, new_dup_id); | 243 | Y.Assert.areEqual(3, new_dup_id); |
722 | 135 | success_called = true; | 244 | success_called = true; |
723 | 136 | }; | 245 | }; |
724 | 137 | this._assert_duplicate_form_submission(3); | ||
725 | 138 | var expected_updated_entry = { | 246 | var expected_updated_entry = { |
726 | 139 | uri: 'api/devel/bugs/1', | 247 | uri: 'api/devel/bugs/1', |
727 | 140 | duplicate_of_link: 'api/devel/bugs/3', | 248 | duplicate_of_link: 'api/devel/bugs/3', |
728 | @@ -146,6 +254,7 @@ | |||
729 | 146 | // A submission failure is handled as expected. | 254 | // A submission failure is handled as expected. |
730 | 147 | test_duplicate_form_submission_failure: function() { | 255 | test_duplicate_form_submission_failure: function() { |
731 | 148 | this.widget = this._createWidget(false); | 256 | this.widget = this._createWidget(false); |
732 | 257 | this._assert_confirmation_form_submission(3); | ||
733 | 149 | var failure_called = false; | 258 | var failure_called = false; |
734 | 150 | this.widget._update_bug_duplicate_failure = | 259 | this.widget._update_bug_duplicate_failure = |
735 | 151 | function(response, old_dup_url, new_dup_id) { | 260 | function(response, old_dup_url, new_dup_id) { |
736 | @@ -155,7 +264,6 @@ | |||
737 | 155 | Y.Assert.areEqual(3, new_dup_id); | 264 | Y.Assert.areEqual(3, new_dup_id); |
738 | 156 | failure_called = true; | 265 | failure_called = true; |
739 | 157 | }; | 266 | }; |
740 | 158 | this._assert_duplicate_form_submission(3); | ||
741 | 159 | this.mockio.respond({ | 267 | this.mockio.respond({ |
742 | 160 | status: 400, | 268 | status: 400, |
743 | 161 | responseText: 'There was an error', | 269 | responseText: 'There was an error', |
744 | @@ -165,7 +273,8 @@ | |||
745 | 165 | 273 | ||
746 | 166 | // Submitting a dupe removal request works as expected. | 274 | // Submitting a dupe removal request works as expected. |
747 | 167 | test_duplicate_form_submission_remove_dupe: function() { | 275 | test_duplicate_form_submission_remove_dupe: function() { |
749 | 168 | this.widget = this._createWidget(true); | 276 | this.widget = this._createWidget(false); |
750 | 277 | this._assert_search_form_submission(''); | ||
751 | 169 | var success_called = false; | 278 | var success_called = false; |
752 | 170 | this.widget._update_bug_duplicate_success = | 279 | this.widget._update_bug_duplicate_success = |
753 | 171 | function(updated_entry, new_dup_url, new_dup_id) { | 280 | function(updated_entry, new_dup_url, new_dup_id) { |
754 | @@ -174,7 +283,6 @@ | |||
755 | 174 | Y.Assert.areEqual('', new_dup_id); | 283 | Y.Assert.areEqual('', new_dup_id); |
756 | 175 | success_called = true; | 284 | success_called = true; |
757 | 176 | }; | 285 | }; |
758 | 177 | this._assert_duplicate_form_submission(''); | ||
759 | 178 | var expected_updated_entry = | 286 | var expected_updated_entry = |
760 | 179 | '{"duplicate_of_link":""}'; | 287 | '{"duplicate_of_link":""}'; |
761 | 180 | this.mockio.success({ | 288 | this.mockio.success({ |
762 | @@ -221,48 +329,6 @@ | |||
763 | 221 | Y.one('#mark-duplicate-text .menu-link-mark-dupe')); | 329 | Y.one('#mark-duplicate-text .menu-link-mark-dupe')); |
764 | 222 | // Test the duplicate warning message is gone. | 330 | // Test the duplicate warning message is gone. |
765 | 223 | Y.Assert.isNull(Y.one('#warning-comment-on-duplicate')); | 331 | Y.Assert.isNull(Y.one('#warning-comment-on-duplicate')); |
766 | 224 | }, | ||
767 | 225 | |||
768 | 226 | // The remove bug duplicate error function works as expected for | ||
769 | 227 | // generic errors. | ||
770 | 228 | test_update_bug_duplicate_generic_failure: function() { | ||
771 | 229 | this.widget = this._createWidget(false); | ||
772 | 230 | var data = { | ||
773 | 231 | self_link: 'api/devel/bugs/1'}; | ||
774 | 232 | var new_bug_entry = new Y.lp.client.Entry( | ||
775 | 233 | this.lp_client, data, data.self_link); | ||
776 | 234 | var response = { | ||
777 | 235 | status: 500, | ||
778 | 236 | responseText: 'An error occurred' | ||
779 | 237 | }; | ||
780 | 238 | this.widget._update_bug_duplicate_failure(response, null, 3); | ||
781 | 239 | Y.Assert.isFalse( | ||
782 | 240 | Y.one('#duplicate-form-container div.pretty-overlay-window') | ||
783 | 241 | .hasClass('yui3-lazr-formoverlay-hidden')); | ||
784 | 242 | var error_msg = Y.one('.yui3-lazr-formoverlay-errors ul li'); | ||
785 | 243 | Y.Assert.areEqual('An error occurred', error_msg.get('text')); | ||
786 | 244 | }, | ||
787 | 245 | |||
788 | 246 | // The remove bug duplicate error function works as expected for | ||
789 | 247 | // invalid bug errors. | ||
790 | 248 | test_update_bug_duplicate_invalid_bug_failure: function() { | ||
791 | 249 | this.widget = this._createWidget(false); | ||
792 | 250 | var data = { | ||
793 | 251 | self_link: 'api/devel/bugs/1'}; | ||
794 | 252 | var new_bug_entry = new Y.lp.client.Entry( | ||
795 | 253 | this.lp_client, data, data.self_link); | ||
796 | 254 | var response = { | ||
797 | 255 | status: 400, | ||
798 | 256 | responseText: 'An error occurred' | ||
799 | 257 | }; | ||
800 | 258 | this.widget._update_bug_duplicate_failure(response, null, 3); | ||
801 | 259 | Y.Assert.isFalse( | ||
802 | 260 | Y.one('#duplicate-form-container div.pretty-overlay-window') | ||
803 | 261 | .hasClass('yui3-lazr-formoverlay-hidden')); | ||
804 | 262 | var error_msg = Y.one('.yui3-lazr-formoverlay-errors ul li'); | ||
805 | 263 | Y.Assert.areEqual( | ||
806 | 264 | '3 is not a valid bug number or nickname.', | ||
807 | 265 | error_msg.get('text')); | ||
808 | 266 | } | 332 | } |
809 | 267 | })); | 333 | })); |
810 | 268 | 334 |
I think this is a good start. It is must better than what users are seeing now. I have a few remarks.
> === modified file 'lib/lp/ app/javascript/ formoverlay/ formoverlay. js' app/javascript/ formoverlay/ formoverlay. js 2012-06-26 00:15:11 +0000 app/javascript/ formoverlay/ formoverlay. js 2012-07-26 11:02:24 +0000 error_msgs) { isString( error_msgs) ) { create( '<p></p> ').set( 'text', error_msgs); </p><ul> </ul>') ; one('ul' ); error_msg) { appendChild( create( '<li></ li>').set( 'text', error_msg));
> --- lib/lp/
> +++ lib/lp/
> @@ -492,18 +492,19 @@
> * @method showError
> */
> showError: function(
> + var error_content;
> if (Y.Lang.
> - error_msgs = [error_msgs];
> + error_content = Y.Node.
> + } else {
> + error_content = Y.Node.create(
> + '<p>The following errors were encountered:
> + var error_list = error_content.
> + Y.each(error_msgs, function(
> + error_list.
> + Y.Node.
> + });
This iteration is expensive. repeated appends also call repeated refreshes
You can build the nodes individually, or pass a string to update the UI once.
var li_nodes = new Y.NodeList([]);
Y. each(error_ msgs, function( error_msg) {
li_nodes. push(
Y. Node.create( '<li></ li>').set( 'text', error_msg));
error_ list.append( li_nodes) ;
});
> === modified file 'lib/lp/ bugs/javascript /duplicates. js' bugs/javascript /duplicates. js 2012-07-25 00:31:04 +0000 bugs/javascript /duplicates. js 2012-07-26 11:02:24 +0000 actions. change" ' + actions. cancel" ' +
> --- lib/lp/
> +++ lib/lp/
> @@ -13,10 +13,10 @@
> // Overlay related vars.
> var submit_button_html =
> '<button type="submit" name="field.
> - 'value="Change" class="lazr-pos lazr-btn" >OK</button>';
> + '>OK</button>';
> var cancel_button_html =
> '<button type="button" name="field.
> - 'class="lazr-neg lazr-btn" >Cancel</button>';
> + '>Cancel</button>';
Thank you for removing the ambiguous and tiny icons. "Ok" is a poor
action term that Launchpad and other UI guidelines avoid. "Save" or
"Change" are better generic terms. If we have a separate action/link to
remove/clear duplicate, then this action would be named "Save
Duplicate".
> @@ -82,12 +85,250 @@ dupe_link. on('click' , function(e) { _form_overlay) { _form) { form_overlay. show(); form.show( ); 'field. duplicateof' ).focus( );
> update_
> // Only go ahead if we have received the form content by the
> // time the user clicks:
> - if (that.duplicate
> + if (that.duplicate
> e.halt();
> - that.duplicate_
> + that.duplicate_
> Y.DOM.byId(
> }
> });
> + // We the due form overlay is hidden, we need to reset the form.
Maybe s/We/When/;
> + // Template for rendering the bug details. template: function() {
> + _bug_details_
> + return [
> + '<div id="client-listing" style="max-width: 75em;">',
This looks too large. 60 is the max for English and that is what we
use else-where in Lp. Wh...