Merge lp:~wallyworld/launchpad/improve-dupe-bug-ui-227310 into lp:launchpad
- improve-dupe-bug-ui-227310
- Merge into devel
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | no longer in the source branch. |
Merged at revision: | 15784 |
Proposed branch: | lp:~wallyworld/launchpad/improve-dupe-bug-ui-227310 |
Merge into: | lp:launchpad |
Diff against target: |
821 lines (+325/-103) 14 files modified
lib/canonical/launchpad/icing/css/components/bug_picker.css (+8/-0) lib/canonical/launchpad/icing/css/components/yui_picker.css (+2/-1) lib/canonical/launchpad/icing/css/modifiers.css (+9/-0) lib/canonical/launchpad/icing/style.css (+24/-0) lib/lp/app/javascript/picker/picker.js (+3/-3) lib/lp/bugs/javascript/bug_picker.js (+31/-24) lib/lp/bugs/javascript/bugtask_index.js (+1/-1) lib/lp/bugs/javascript/duplicates.js (+191/-55) lib/lp/bugs/javascript/tests/test_bug_picker.js (+2/-2) lib/lp/bugs/javascript/tests/test_duplicates.html (+6/-2) lib/lp/bugs/javascript/tests/test_duplicates.js (+6/-0) lib/lp/bugs/templates/bug-portlet-actions.pt (+11/-4) lib/lp/bugs/templates/bugtask-index.pt (+25/-0) lib/lp/code/javascript/branch.bugspeclinks.js (+6/-11) |
To merge this branch: | bzr merge lp:~wallyworld/launchpad/improve-dupe-bug-ui-227310 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+118738@code.launchpad.net |
Commit message
Display an information message about a bug duplicate at the top of the bug tasks table, and add remove icons.
Description of the change
== Implementation ==
This branch improves the display of bug duplicate information. It renders an informational message at the top of the bug tasks table, along with an edit and remove icon. It also adds a remove icon to the duplicate portlet. So the dupe can be removed directly from the remove icon or from the picker.
As a driveby, the bug picker event names were corrected.
The method of rendering the in-progress spinner for the bug picker was changed to use a css approach instead of explicitly adding and removing an extra spinner node. This was necessary to fit with using the additional remove icons. There was already a button.spinner style defined (not sure where it is used but it looks horrible) so I created a new button.
== Demo ==
Screenshot of the picker showing the new spinner icon for the for button and the message display:
http://
== Tests ==
Update test_duplicates yui tests to check the rendering of the new informational message.
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Ian Booth (wallyworld) wrote : | # |
Thanks for reviewing.
>
> IE 8 (and 7) does not support :not(). I think you need to write this like this:
> span.update-
> button.
>
Ok, thanks. I struggled to get this right. I also recently added :not()
to formoverlay-
> I think the inline style on 402 is wrong. "style="max-width: 60em;" is only used when we know there is no side bar. sibling elements are 45ems. Why does this element allowed to extend beyond their length?
>
The intent here was to limited the horizontal width of the message so
that it didn't expand out to totally fill the entire line if the browser
window were wide. ie long horizontal text is harder to read so I thought
we needed to have "Blah blah blah....".
> I see this pattern:
> + ' class="sprite remove action-icon remove-
> 415+ ' Remove</a>',
> which puts leading white-space between the start of the anchor and the text. Even though the links are action-icon, parts of the underline can appear in some browsers, and quite likely for users that increase the text size. Maybe this is safer
> + ' class="sprite remove action-icon remove-
> 415+ 'Remove</a>',
>
Bah, typo. Thanks.
> TAL process its instructions in a specific order, define, then condition. This also means that define is always called on the element. Even though you wrote the condition first, the define was already executed:
> 570+ tal:condition=
> 571+ tal:define=
> Since we know the order we could write this:
> 570+ tal:define=
> 571+ tal:condition=
>
>
Good pickup. I missed this, thanks.
Curtis Hovey (sinzui) wrote : | # |
You might want to verify that the line really does extend with the style. Our css thinks *all* paragraphs are 45ems wide.
Ian Booth (wallyworld) wrote : | # |
> >
> > IE 8 (and 7) does not support :not(). I think you need to write this like
> this:
> > span.update-
> > button.
> >
>
> Ok, thanks. I struggled to get this right. I also recently added :not()
> to formoverlay-
>
The above won't work because it only covers <span> and not <a>
This works:
.update-
background: url(/@@/spinner) center left no-repeat;
padding-left: 18px;
color: #666;
}
button.
background: inherit;
padding-left: inherit;
color: inherit;
}
button.
background: url("/@@/spinner") center left no-repeat;
content: "\00A0";
margin-left: 6px;
padding-right: 12px;
}
The spinner which replaces the anchor sprite appears a few pixels too low so I also need this:
a.update-
margin-top: -4px;
}
If you are ok with this I'll use this approach.
Preview Diff
1 | === modified file 'lib/canonical/launchpad/icing/css/components/bug_picker.css' |
2 | --- lib/canonical/launchpad/icing/css/components/bug_picker.css 2012-08-03 12:47:11 +0000 |
3 | +++ lib/canonical/launchpad/icing/css/components/bug_picker.css 2012-08-09 04:59:21 +0000 |
4 | @@ -6,3 +6,11 @@ |
5 | .yui3-bugpickerwidget .search-header { |
6 | margin-top: 6px; |
7 | } |
8 | + |
9 | +.yui3-bugpickerwidget { |
10 | + max-width: 80%; |
11 | + } |
12 | + |
13 | +.yui3-bugpickerwidget #client-listing p { |
14 | + margin-bottom: 0 |
15 | + } |
16 | |
17 | === modified file 'lib/canonical/launchpad/icing/css/components/yui_picker.css' |
18 | --- lib/canonical/launchpad/icing/css/components/yui_picker.css 2011-11-18 05:15:52 +0000 |
19 | +++ lib/canonical/launchpad/icing/css/components/yui_picker.css 2012-08-09 04:59:21 +0000 |
20 | @@ -1,5 +1,6 @@ |
21 | .yui3-picker { |
22 | - width: 40%; |
23 | + min-width: 40%; |
24 | + max-width: 40%; |
25 | } |
26 | |
27 | .yui3-picker-results div.badge { |
28 | |
29 | === modified file 'lib/canonical/launchpad/icing/css/modifiers.css' |
30 | --- lib/canonical/launchpad/icing/css/modifiers.css 2012-07-26 00:07:27 +0000 |
31 | +++ lib/canonical/launchpad/icing/css/modifiers.css 2012-08-09 04:59:21 +0000 |
32 | @@ -113,6 +113,7 @@ |
33 | } |
34 | |
35 | .ellipsis { |
36 | + display: inline-block; |
37 | white-space: nowrap; |
38 | overflow: hidden; |
39 | text-overflow: ellipsis; |
40 | @@ -120,6 +121,10 @@ |
41 | -ms-text-overflow: ellipsis; |
42 | } |
43 | |
44 | +.ellipsis.wide { |
45 | + max-width: 60em; |
46 | + } |
47 | + |
48 | .exception { |
49 | color: #cc0000; |
50 | } |
51 | @@ -213,6 +218,10 @@ |
52 | margin-left: 0px; |
53 | } |
54 | |
55 | +.action-icon.standalone { |
56 | + margin-top: -6px; |
57 | + } |
58 | + |
59 | th .action-icon { |
60 | height: 12px; |
61 | } |
62 | |
63 | === modified file 'lib/canonical/launchpad/icing/style.css' |
64 | --- lib/canonical/launchpad/icing/style.css 2012-07-05 00:47:31 +0000 |
65 | +++ lib/canonical/launchpad/icing/style.css 2012-08-09 04:59:21 +0000 |
66 | @@ -393,6 +393,30 @@ |
67 | color: #666; |
68 | } |
69 | |
70 | +a.action-icon.standalone.update-in-progress-message { |
71 | + display: inline-block; |
72 | + margin-left: inherit; |
73 | + margin-top: -10px; |
74 | + width: 1px; |
75 | + height: 16px; |
76 | + vertical-align: text-top; |
77 | + text-indent: 3px; |
78 | + overflow: hidden; |
79 | +} |
80 | + |
81 | +button.update-in-progress-message { |
82 | + background: buttonface none; |
83 | + padding-left: inherit; |
84 | + color: inherit; |
85 | +} |
86 | + |
87 | +button.update-in-progress-message:after { |
88 | + background: url("/@@/spinner") center left no-repeat; |
89 | + content: "\00A0"; |
90 | + margin-left: 6px; |
91 | + padding-right: 12px; |
92 | +} |
93 | + |
94 | a.update-retry { |
95 | padding-left: 1em; |
96 | } |
97 | |
98 | === modified file 'lib/lp/app/javascript/picker/picker.js' |
99 | --- lib/lp/app/javascript/picker/picker.js 2012-08-02 20:40:57 +0000 |
100 | +++ lib/lp/app/javascript/picker/picker.js 2012-08-09 04:59:21 +0000 |
101 | @@ -143,12 +143,12 @@ |
102 | this.subscribe('cancel', this._defaultCancel); |
103 | |
104 | if ( this.get('picker_activator') ) { |
105 | - var element = Y.one(this.get('picker_activator')); |
106 | - element.on('click', function(e) { |
107 | + var elements = Y.all(this.get('picker_activator')); |
108 | + elements.on('click', function(e) { |
109 | e.halt(); |
110 | this.show(); |
111 | }, this); |
112 | - element.addClass(this.get('picker_activator_css_class')); |
113 | + elements.addClass(this.get('picker_activator_css_class')); |
114 | } |
115 | |
116 | if (!Y.Lang.isUndefined(cfg)) { |
117 | |
118 | === modified file 'lib/lp/bugs/javascript/bug_picker.js' |
119 | --- lib/lp/bugs/javascript/bug_picker.js 2012-08-07 09:28:09 +0000 |
120 | +++ lib/lp/bugs/javascript/bug_picker.js 2012-08-09 04:59:21 +0000 |
121 | @@ -55,6 +55,15 @@ |
122 | return this._remove_link_html(); |
123 | }, |
124 | |
125 | + // Centre the picker along the x axis without changing y position. |
126 | + _xaxis_centre: function() { |
127 | + var viewport = Y.DOM.viewportRegion(); |
128 | + var new_x = (viewport.right + viewport.left)/2 - |
129 | + this.get('contentBox').get('offsetWidth')/2; |
130 | + this.move([new_x, this._getY()]); |
131 | + |
132 | + }, |
133 | + |
134 | renderUI: function() { |
135 | Y.lazr.picker.Picker.prototype.renderUI.apply(this, arguments); |
136 | var search_header = Y.Node.create(this._bug_search_header()); |
137 | @@ -70,7 +79,7 @@ |
138 | if (Y.Lang.isValue(this.remove_link)) { |
139 | this.remove_link.on('click', function(e) { |
140 | e.halt(); |
141 | - that.fire(namespace.BugPicker.REMOVE_DUPLICATE); |
142 | + that.fire(namespace.BugPicker.REMOVE); |
143 | }); |
144 | } |
145 | this.after('visibleChange', function() { |
146 | @@ -88,30 +97,26 @@ |
147 | }, |
148 | |
149 | /** |
150 | - * Show a spinner next to the specified node. |
151 | + * Show a spinner for the specified node. |
152 | * |
153 | * @method _show_bug_spinner |
154 | * @param node |
155 | * @protected |
156 | */ |
157 | _show_bug_spinner: function(node) { |
158 | - if( !Y.Lang.isValue(node)) { |
159 | - return null; |
160 | + if( Y.Lang.isValue(node)) { |
161 | + node.addClass('update-in-progress-message'); |
162 | } |
163 | - var spinner_node = Y.Node.create( |
164 | - '<img class="spinner" src="/@@/spinner" alt="Searching..." />'); |
165 | - node.insert(spinner_node, 'after'); |
166 | - return spinner_node; |
167 | }, |
168 | |
169 | /** |
170 | - * Hide the specified spinner. |
171 | - * @param spinner |
172 | + * Hide the spinner for the specified node. |
173 | + * @param node |
174 | * @protected |
175 | */ |
176 | - _hide_bug_spinner: function(spinner) { |
177 | - if( Y.Lang.isValue(spinner)) { |
178 | - spinner.remove(true); |
179 | + _hide_bug_spinner: function(node) { |
180 | + if( Y.Lang.isValue(node)) { |
181 | + node.removeClass('update-in-progress-message'); |
182 | } |
183 | }, |
184 | |
185 | @@ -182,13 +187,13 @@ |
186 | ' </div>', |
187 | ' </div>', |
188 | ' </div>', |
189 | - ' <div class="buglisting-col2" style="max-width: 60em;">', |
190 | - ' <a href="{{bug_url}}" class="bugtitle sprite new-window">', |
191 | - ' <span class="bugnumber">#{{id}}</span> {{bug_summary}}</a>', |
192 | - ' <div class="buginfo-extra">', |
193 | - ' <p class="ellipsis line-block">', |
194 | - ' {{description}}</p>', |
195 | - ' </div>', |
196 | + ' <div class="buglisting-col2">', |
197 | + ' <a href="{{bug_url}}" class="bugtitle sprite new-window" ', |
198 | + ' style="padding-top: 3px">', |
199 | + ' <p class="ellipsis"><span class="bugnumber">#{{id}}</span>', |
200 | + ' {{bug_summary}}</p></a>', |
201 | + ' <div class="buginfo-extra">', |
202 | + ' <p class="ellipsis">{{description}}</p></div>', |
203 | ' </div>', |
204 | '</div></td></tr>', |
205 | '{{> private_warning}}', |
206 | @@ -248,8 +253,7 @@ |
207 | this.save_button |
208 | .on('click', function(e) { |
209 | e.halt(); |
210 | - that.fire( |
211 | - namespace.BugPicker.SAVE_DUPLICATE, bug_data); |
212 | + that.fire(namespace.BugPicker.SAVE, bug_data); |
213 | }); |
214 | }, |
215 | |
216 | @@ -266,6 +270,8 @@ |
217 | } else { |
218 | this.save_button.detachAll(); |
219 | } |
220 | + this.save_button.focus(); |
221 | + this._xaxis_centre(); |
222 | var use_animation = this.get('use_animation'); |
223 | if (!use_animation) { |
224 | this.bug_details_node = new_bug_details_node; |
225 | @@ -300,6 +306,7 @@ |
226 | this.save_button.remove(true); |
227 | this.save_button = null; |
228 | } |
229 | + this._xaxis_centre(); |
230 | } |
231 | }, { |
232 | ATTRS: { |
233 | @@ -366,8 +373,8 @@ |
234 | }); |
235 | |
236 | // Events |
237 | -namespace.BugPicker.SAVE_DUPLICATE = 'save'; |
238 | -namespace.BugPicker.REMOVE_DUPLICATE = 'remove'; |
239 | +namespace.BugPicker.SAVE = 'save'; |
240 | +namespace.BugPicker.REMOVE = 'remove'; |
241 | |
242 | }, "0.1", {"requires": [ |
243 | "base", "io", "oop", "node", "event", "json", |
244 | |
245 | === modified file 'lib/lp/bugs/javascript/bugtask_index.js' |
246 | --- lib/lp/bugs/javascript/bugtask_index.js 2012-08-07 09:28:09 +0000 |
247 | +++ lib/lp/bugs/javascript/bugtask_index.js 2012-08-09 04:59:21 +0000 |
248 | @@ -41,7 +41,7 @@ |
249 | setup_client_and_bug(); |
250 | |
251 | var config = { |
252 | - picker_activator: '.menu-link-mark-dupe, #change_duplicate_bug' |
253 | + picker_activator: '.menu-link-mark-dupe, .change_duplicate_bug' |
254 | }; |
255 | var dup_widget = new Y.lp.bugs.duplicates.DuplicateBugPicker(config); |
256 | dup_widget.render(); |
257 | |
258 | === modified file 'lib/lp/bugs/javascript/duplicates.js' |
259 | --- lib/lp/bugs/javascript/duplicates.js 2012-08-08 00:45:37 +0000 |
260 | +++ lib/lp/bugs/javascript/duplicates.js 2012-08-09 04:59:21 +0000 |
261 | @@ -25,20 +25,40 @@ |
262 | superclass.prototype.bindUI.apply(this, arguments); |
263 | var that = this; |
264 | this.subscribe( |
265 | - Y.lp.bugs.bug_picker.BugPicker.SAVE_DUPLICATE, function(e) { |
266 | - e.preventDefault(); |
267 | + Y.lp.bugs.bug_picker.BugPicker.SAVE, function(e) { |
268 | + e.halt(); |
269 | that.set('progress', 100); |
270 | var bug_data = e.details[0]; |
271 | var bug_id = bug_data.id; |
272 | var bug_title = bug_data.bug_summary; |
273 | - that._submit_bug(bug_id, bug_title, this.save_button); |
274 | + that._submit_bug(bug_id, bug_title, that.save_button); |
275 | }); |
276 | this.subscribe( |
277 | - Y.lp.bugs.bug_picker.BugPicker.REMOVE_DUPLICATE, function(e) { |
278 | - e.preventDefault(); |
279 | + Y.lp.bugs.bug_picker.BugPicker.REMOVE, function(e) { |
280 | + e.halt(); |
281 | that.set('progress', 100); |
282 | - that._submit_bug('', null, this.remove_link); |
283 | + that._submit_bug('', null, that.remove_link); |
284 | }); |
285 | + this._connect_links(); |
286 | + }, |
287 | + |
288 | + // Wire up the edit and remove links. |
289 | + _connect_links: function() { |
290 | + Y.all( |
291 | + '.change-duplicate-bug, .menu-link-mark-dupe, ' + |
292 | + '.remove-duplicate-bug').detachAll(); |
293 | + var that = this; |
294 | + Y.all( |
295 | + '.remove-duplicate-bug').on('click', function(e) { |
296 | + e.halt(); |
297 | + that._submit_bug('', null, e.currentTarget); |
298 | + }); |
299 | + Y.all( |
300 | + '.change-duplicate-bug, .menu-link-mark-dupe').on('click', |
301 | + function(e) { |
302 | + e.halt(); |
303 | + that.show(); |
304 | + }); |
305 | }, |
306 | |
307 | _syncResultsUI: function() { |
308 | @@ -121,7 +141,7 @@ |
309 | }, |
310 | |
311 | // A common error handler for XHR operations. |
312 | - _error_handler: function() { |
313 | + _error_handler: function(widget) { |
314 | var that = this; |
315 | var error_handler = new Y.lp.client.ErrorHandler(); |
316 | error_handler.handleError = function(id, response) { |
317 | @@ -145,6 +165,19 @@ |
318 | error_handler.showError = function(error_msg) { |
319 | that.set('error', error_msg); |
320 | }; |
321 | + error_handler.clearProgressUI = function() { |
322 | + that._hide_bug_spinner(widget); |
323 | + var dupe_span = that.get('dupe_span'); |
324 | + dupe_span.removeClass('update-in-progress-message'); |
325 | + Y.all('.remove-duplicate-bug').each(function(node) { |
326 | + node.removeClass('update-in-progress-message'); |
327 | + node.addClass('remove'); |
328 | + }); |
329 | + if (Y.Lang.isValue(this.remove_link)) { |
330 | + this.remove_link.removeClass('update-in-progress-message'); |
331 | + this.remove_link.addClass('remove'); |
332 | + } |
333 | + }; |
334 | return error_handler; |
335 | }, |
336 | |
337 | @@ -155,6 +188,38 @@ |
338 | Y.lp.bugs.bugtask_index.setup_bugtask_table(); |
339 | }, |
340 | |
341 | + // Create the duplicate edit anchor. |
342 | + _dupe_edit_link: function(link_id, url, dup_id) { |
343 | + var template = [ |
344 | + '<a id="{link_id}" ', |
345 | + 'title="Edit or remove linked duplicate bug" ', |
346 | + 'href={url} ', |
347 | + 'class="sprite edit action-icon change-duplicate-bug"', |
348 | + 'style="margin-left: 0">Edit</a>', |
349 | + '<span id="mark-duplicate-text">', |
350 | + 'Duplicate of <a href="/bugs/{dup_id}">bug #{dup_id}</a>', |
351 | + '</span>'].join(""); |
352 | + return Y.Lang.substitute(template, { |
353 | + link_id: link_id, |
354 | + url: url, |
355 | + dup_id: dup_id |
356 | + }); |
357 | + }, |
358 | + |
359 | + // Create the duplicate removal anchor. |
360 | + _dupe_remove_link: function(link_id, url) { |
361 | + var template = [ |
362 | + '<a id="{link_id}" ', |
363 | + 'title="Edit or remove linked duplicate bug" ', |
364 | + 'href={url} ', |
365 | + 'class="sprite remove action-icon remove-duplicate-bug"', |
366 | + 'style="float: right;">Remove</a>'].join(""); |
367 | + return Y.Lang.substitute(template, { |
368 | + link_id: link_id, |
369 | + url: url |
370 | + }); |
371 | + }, |
372 | + |
373 | /** |
374 | * Bug was successfully marked as a duplicate, update the UI. |
375 | * |
376 | @@ -168,62 +233,83 @@ |
377 | _submit_bug_success: function(response, new_dup_url, |
378 | new_dup_id, new_dupe_title) { |
379 | this._performDefaultSave(); |
380 | - |
381 | // Render the new bug tasks table. |
382 | LP.cache.bug.duplicate_of_link = new_dup_url; |
383 | this._render_bugtask_table(response.responseText); |
384 | |
385 | - // Render the new dupe portlet. |
386 | - var dupe_span = this.get('dupe_span').ancestor('li'); |
387 | - var update_dup_url = dupe_span.one('a').get('href'); |
388 | - var edit_link; |
389 | if (Y.Lang.isValue(new_dup_url)) { |
390 | - dupe_span.removeClass('sprite bug-dupe'); |
391 | - dupe_span.setContent([ |
392 | - '<a id="change_duplicate_bug" ', |
393 | - 'title="Edit or remove linked duplicate bug" ', |
394 | - 'class="sprite edit action-icon"', |
395 | - 'style="margin-left: 0">Edit</a>', |
396 | - '<span id="mark-duplicate-text">', |
397 | - 'Duplicate of <a>bug #</a></span>'].join("")); |
398 | - edit_link = dupe_span.one('#change_duplicate_bug'); |
399 | - edit_link.set('href', update_dup_url); |
400 | - dupe_span.all('a').item(1) |
401 | - .set('href', '/bugs/' + new_dup_id) |
402 | - .appendChild(document.createTextNode(new_dup_id)); |
403 | - var duplicatesNode = this.get('duplicatesNode'); |
404 | - if (Y.Lang.isValue(duplicatesNode)) { |
405 | - duplicatesNode.remove(true); |
406 | + this._render_dupe_information(new_dup_id, new_dupe_title); |
407 | + } else { |
408 | + Y.all('.remove-duplicate-bug').each(function(node) { |
409 | + node.removeClass('update-in-progress-message'); |
410 | + node.addClass('remove'); |
411 | + }); |
412 | + if (Y.Lang.isValue(this.remove_link)) { |
413 | + this.remove_link.removeClass('update-in-progress-message'); |
414 | + this.remove_link.addClass('remove'); |
415 | } |
416 | - this._show_comment_on_duplicate_warning( |
417 | - new_dup_id, new_dupe_title); |
418 | - } else { |
419 | - dupe_span.addClass('sprite bug-dupe'); |
420 | - dupe_span.setContent([ |
421 | - '<span id="mark-duplicate-text">', |
422 | - '<a class="menu-link-mark-dupe js-action">', |
423 | - 'Mark as duplicate</a></span>'].join("")); |
424 | - edit_link = dupe_span.one('a'); |
425 | - edit_link.set('href', update_dup_url); |
426 | - this._hide_comment_on_duplicate_warning(); |
427 | - } |
428 | - dupe_span = this.get('portletNode').one('#mark-duplicate-text'); |
429 | - this.set('dupe_span', dupe_span); |
430 | + this._remove_dupe_information(); |
431 | + } |
432 | + var dupe_portlet_node = |
433 | + this.get('portletNode').one('#mark-duplicate-text'); |
434 | + this.set('dupe_portlet_node', dupe_portlet_node); |
435 | + this._connect_links(); |
436 | + }, |
437 | + |
438 | + // Render the new duplicate information. |
439 | + _render_dupe_information: function(new_dup_id, new_dupe_title) { |
440 | + var dupe_portlet_node = this.get('dupe_span').ancestor('li'); |
441 | + var update_dup_url = dupe_portlet_node.one('a').get('href'); |
442 | + dupe_portlet_node.empty(true); |
443 | + dupe_portlet_node.removeClass('sprite bug-dupe'); |
444 | + var edit_link = this._dupe_edit_link( |
445 | + 'change-duplicate-bug', update_dup_url, new_dup_id); |
446 | + dupe_portlet_node.appendChild(edit_link); |
447 | + var remove_link = this._dupe_remove_link( |
448 | + 'remove-duplicate-bug', update_dup_url); |
449 | + dupe_portlet_node.appendChild(remove_link); |
450 | + var duplicatesNode = this.get('duplicatesNode'); |
451 | + if (Y.Lang.isValue(duplicatesNode)) { |
452 | + duplicatesNode.remove(true); |
453 | + } |
454 | + this._show_comment_on_duplicate_warning(new_dup_id, new_dupe_title); |
455 | + this._show_bugtasks_duplicate_message(new_dup_id, new_dupe_title); |
456 | var anim_duration = 1; |
457 | if (!this.get('use_animation')) { |
458 | anim_duration = 0; |
459 | } |
460 | Y.lp.anim.green_flash({ |
461 | - node: dupe_span, |
462 | + node: '.bug-duplicate-details', |
463 | duration: anim_duration |
464 | }).run(); |
465 | - // ensure the new link is hooked up correctly: |
466 | + }, |
467 | + |
468 | + // Remove the old duplicate information. |
469 | + _remove_dupe_information: function() { |
470 | + var dupe_portlet_node = this.get('dupe_span').ancestor('li'); |
471 | + var update_dup_url = dupe_portlet_node.one('a').get('href'); |
472 | + dupe_portlet_node.addClass('sprite bug-dupe'); |
473 | + dupe_portlet_node.setContent([ |
474 | + '<span id="mark-duplicate-text">', |
475 | + '<a class="menu-link-mark-dupe js-action">', |
476 | + 'Mark as duplicate</a></span>'].join("")); |
477 | + var edit_link = dupe_portlet_node.one('a'); |
478 | + edit_link.set('href', update_dup_url); |
479 | + this._hide_comment_on_duplicate_warning(); |
480 | var that = this; |
481 | - edit_link.on( |
482 | - 'click', function(e){ |
483 | - e.preventDefault(); |
484 | - that.show(); |
485 | + var hide_dupe_message = function() { |
486 | + that._hide_bugtasks_duplicate_message(); |
487 | + }; |
488 | + if (!this.get('use_animation')) { |
489 | + hide_dupe_message(); |
490 | + return; |
491 | + } |
492 | + var anim = Y.lp.anim.green_flash({ |
493 | + node: '.bug-duplicate-details', |
494 | + duration: 1 |
495 | }); |
496 | + anim.on('end', hide_dupe_message); |
497 | + anim.run(); |
498 | }, |
499 | |
500 | /** |
501 | @@ -237,11 +323,11 @@ |
502 | */ |
503 | _submit_bug: function(new_dup_id, new_dupe_title, widget) { |
504 | var dupe_span = this.get('dupe_span'); |
505 | - var that = this; |
506 | var new_dup_url = null; |
507 | |
508 | var qs; |
509 | - if (new_dup_id !== '') { |
510 | + var setting_dupe = new_dup_id !== ''; |
511 | + if (setting_dupe) { |
512 | var bug_link = LP.cache.bug.self_link; |
513 | var last_slash_index = bug_link.lastIndexOf('/'); |
514 | new_dup_url = bug_link.slice(0, last_slash_index+1) + new_dup_id; |
515 | @@ -253,23 +339,32 @@ |
516 | '', 'field.actions.remove', 'Remove Duplicate'); |
517 | } |
518 | |
519 | + var that = this; |
520 | var spinner = null; |
521 | - var error_handler = this._error_handler(); |
522 | + var error_handler = this._error_handler(widget); |
523 | var submit_url = LP.cache.context.web_link + '/+duplicate'; |
524 | var y_config = { |
525 | method: "POST", |
526 | headers: {'Accept': 'application/json; application/xhtml'}, |
527 | on: { |
528 | start: function() { |
529 | + var dupe_span = that.get('dupe_span'); |
530 | dupe_span.removeClass('sprite bug-dupe'); |
531 | dupe_span.addClass('update-in-progress-message'); |
532 | + if (!setting_dupe) { |
533 | + Y.all('.remove-duplicate-bug').each(function(node) { |
534 | + node.removeClass('remove'); |
535 | + node.addClass('update-in-progress-message'); |
536 | + }); |
537 | + if (Y.Lang.isValue(that.remove_link)) { |
538 | + that.remove_link.removeClass('remove'); |
539 | + that.remove_link |
540 | + .addClass('update-in-progress-message'); |
541 | + } |
542 | + } |
543 | that.set('error', null); |
544 | spinner = that._show_bug_spinner(widget); |
545 | }, |
546 | - end: function() { |
547 | - dupe_span.removeClass('update-in-progress-message'); |
548 | - that._hide_bug_spinner(spinner); |
549 | - }, |
550 | success: function(id, response) { |
551 | that._submit_bug_success( |
552 | response, new_dup_url, new_dup_id, new_dupe_title); |
553 | @@ -282,6 +377,47 @@ |
554 | io_provider.io(submit_url, y_config); |
555 | }, |
556 | |
557 | + // Create the informational message to go at the top of the bug tasks |
558 | + // table. |
559 | + _duplicate_bug_info_message: function(dup_id, dup_title) { |
560 | + var info_template = [ |
561 | + '<span class="bug-duplicate-details ellipsis wide">', |
562 | + '<span class="sprite info"></span>', |
563 | + 'This bug report is a duplicate of: ', |
564 | + '<a href="/bugs/{dup_id}">Bug #{dup_id} {dup_title}</a></span>', |
565 | + '<a id="change-duplicate-bug-bugtasks"', |
566 | + ' href="+duplicate"', |
567 | + ' title="Edit or remove linked duplicate bug"', |
568 | + ' class="sprite edit action-icon standalone ', |
569 | + ' change-duplicate-bug">Edit</a>', |
570 | + '<a id="remove-duplicate-bug-bugtasks"', |
571 | + ' href="+duplicate"', |
572 | + ' title="Remove linked duplicate bug"', |
573 | + ' class="sprite remove action-icon standalone ', |
574 | + ' remove-duplicate-bug">Remove</a>'].join(" "); |
575 | + return Y.Lang.substitute(info_template, { |
576 | + dup_id: dup_id, |
577 | + dup_title: dup_title |
578 | + }); |
579 | + }, |
580 | + |
581 | + // Render the duplicate message at the top of the bug tasks table. |
582 | + _show_bugtasks_duplicate_message: function(dup_id, dup_title) { |
583 | + var dupe_info = Y.one("#bug-is-duplicate"); |
584 | + if (Y.Lang.isValue(dupe_info)) { |
585 | + dupe_info.setContent(Y.Node.create( |
586 | + this._duplicate_bug_info_message(dup_id, dup_title))); |
587 | + } |
588 | + }, |
589 | + |
590 | + // Hide the duplicate message at the top of the bug tasks table. |
591 | + _hide_bugtasks_duplicate_message: function() { |
592 | + var dupe_info = Y.one("#bug-is-duplicate"); |
593 | + if (Y.Lang.isValue(dupe_info)) { |
594 | + dupe_info.empty(); |
595 | + } |
596 | + }, |
597 | + |
598 | /* |
599 | * Ensure that a warning about adding a comment to a duplicate bug |
600 | * is displayed. |
601 | |
602 | === modified file 'lib/lp/bugs/javascript/tests/test_bug_picker.js' |
603 | --- lib/lp/bugs/javascript/tests/test_bug_picker.js 2012-08-07 05:00:15 +0000 |
604 | +++ lib/lp/bugs/javascript/tests/test_bug_picker.js 2012-08-09 04:59:21 +0000 |
605 | @@ -125,7 +125,7 @@ |
606 | this._assert_search_form_success(3); |
607 | var save_bug_called = false; |
608 | this.widget.subscribe( |
609 | - Y.lp.bugs.bug_picker.BugPicker.SAVE_DUPLICATE, |
610 | + Y.lp.bugs.bug_picker.BugPicker.SAVE, |
611 | function(e) { |
612 | e.preventDefault(); |
613 | var bug_data = e.details[0]; |
614 | @@ -173,7 +173,7 @@ |
615 | this.widget = this._createWidget(); |
616 | var remove_bug_called = false; |
617 | this.widget.subscribe( |
618 | - Y.lp.bugs.bug_picker.BugPicker.REMOVE_DUPLICATE, |
619 | + Y.lp.bugs.bug_picker.BugPicker.REMOVE, |
620 | function(e) { |
621 | e.preventDefault(); |
622 | remove_bug_called = true; |
623 | |
624 | === modified file 'lib/lp/bugs/javascript/tests/test_duplicates.html' |
625 | --- lib/lp/bugs/javascript/tests/test_duplicates.html 2012-08-08 00:45:37 +0000 |
626 | +++ lib/lp/bugs/javascript/tests/test_duplicates.html 2012-08-09 04:59:21 +0000 |
627 | @@ -83,6 +83,7 @@ |
628 | <div id="warning-comment-on-duplicate"></div> |
629 | <div id="add-comment-form"></div> |
630 | <div id="portlet-duplicates"></div> |
631 | + <div id="bug-is-duplicate"></div> |
632 | </script> |
633 | <script type="text/x-template" id="existing-duplicate"> |
634 | <table id="affected-software"></table> |
635 | @@ -90,9 +91,9 @@ |
636 | <div><ul id="duplicate-actions"> |
637 | <li> |
638 | <span id="mark-duplicate-text"> |
639 | - <a class="sprite edit action-icon" |
640 | + <a class="sprite edit action-icon bug-duplicate-details" |
641 | title="Edit or remove linked duplicate bug" |
642 | - id="change_duplicate_bug" |
643 | + id="change-duplicate-bug" |
644 | href="http://foo/+duplicate">Edit</a> |
645 | Duplicate of <a href="http://foo/bugs/1234">bug #1234 |
646 | </a> |
647 | @@ -102,6 +103,9 @@ |
648 | <div id="duplicate-form-container"></div> |
649 | <div id="warning-comment-on-duplicate"></div> |
650 | <div id="add-comment-form"></div> |
651 | + <div id="bug-is-duplicate"> |
652 | + <p class="bug-duplicate-details">A dupe</p> |
653 | + </div> |
654 | </script> |
655 | </body> |
656 | </html> |
657 | |
658 | === modified file 'lib/lp/bugs/javascript/tests/test_duplicates.js' |
659 | --- lib/lp/bugs/javascript/tests/test_duplicates.js 2012-08-08 00:45:37 +0000 |
660 | +++ lib/lp/bugs/javascript/tests/test_duplicates.js 2012-08-09 04:59:21 +0000 |
661 | @@ -241,6 +241,9 @@ |
662 | 'Comment here only if you think the duplicate status ' + |
663 | 'is wrong.', |
664 | dupe_warning.get('text').trim()); |
665 | + // The duplicate info message |
666 | + Y.Assert.isNotNull( |
667 | + Y.one('#bug-is-duplicate span.bug-duplicate-details')); |
668 | // Any previously listed duplicates are removed. |
669 | Y.Assert.isNull(Y.one('#portlet-duplicates')); |
670 | // The bug dupe table is updated. |
671 | @@ -263,6 +266,9 @@ |
672 | Y.one('#mark-duplicate-text .menu-link-mark-dupe')); |
673 | // Test the duplicate warning message is gone. |
674 | Y.Assert.isNull(Y.one('#warning-comment-on-duplicate')); |
675 | + // The duplicate info message is gone. |
676 | + Y.Assert.isNull( |
677 | + Y.one('#bug-is-duplicate p.bug-duplicate-details')); |
678 | // The bug dupe table is updated. |
679 | Y.Assert.areEqual( |
680 | 'Bug tasks', Y.one('#affected-software').get('text')); |
681 | |
682 | === modified file 'lib/lp/bugs/templates/bug-portlet-actions.pt' |
683 | --- lib/lp/bugs/templates/bug-portlet-actions.pt 2012-07-30 23:36:26 +0000 |
684 | +++ lib/lp/bugs/templates/bug-portlet-actions.pt 2012-08-09 04:59:21 +0000 |
685 | @@ -23,12 +23,12 @@ |
686 | tal:define="duplicateof context/duplicateof" |
687 | tal:condition="duplicateof" |
688 | > |
689 | - <li><a |
690 | + <li class="bug-duplicate-details"><a |
691 | tal:define="link context_menu/markduplicate" |
692 | tal:condition="python: link.enabled" |
693 | - id="change_duplicate_bug" |
694 | + id="change-duplicate-bug" |
695 | title="Edit or remove linked duplicate bug" |
696 | - class="sprite edit" |
697 | + class="sprite edit change-duplicate-bug" |
698 | tal:attributes="href link/url"></a><span id="mark-duplicate-text">Duplicate of |
699 | <a |
700 | tal:condition="duplicateof/required:launchpad.View" |
701 | @@ -39,7 +39,14 @@ |
702 | >bug #42</a> |
703 | <span |
704 | tal:condition="not:duplicateof/required:launchpad.View" |
705 | - tal:replace="string:a private bug" /></span></li> |
706 | + tal:replace="string:a private bug" /></span> |
707 | + <a id="remove-duplicate-bug" |
708 | + href="+duplicate" |
709 | + title="Remove linked duplicate bug" |
710 | + class="sprite remove action-icon remove-duplicate-bug" |
711 | + style="float: right;">Remove</a> |
712 | + |
713 | + </li> |
714 | </tal:block> |
715 | <li |
716 | tal:define="link context_menu/createquestion" |
717 | |
718 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' |
719 | --- lib/lp/bugs/templates/bugtask-index.pt 2012-07-25 21:49:38 +0000 |
720 | +++ lib/lp/bugs/templates/bugtask-index.pt 2012-08-09 04:59:21 +0000 |
721 | @@ -97,6 +97,31 @@ |
722 | (<a href="https://help.launchpad.net/BugExpiry">find out why</a>) |
723 | </p> |
724 | |
725 | + <div id="bug-is-duplicate"> |
726 | + <tal:dupe-info |
727 | + condition="duplicateof|nothing" |
728 | + define="duplicateof context/bug/duplicateof"> |
729 | + <span class="bug-duplicate-details ellipsis wide"> |
730 | + <span class="sprite info"></span> |
731 | + This bug report is a duplicate of: |
732 | + <a |
733 | + tal:condition="duplicateof/required:launchpad.View" |
734 | + tal:attributes="href duplicateof/fmt:url; title |
735 | + duplicateof/title; |
736 | + id string:duplicate-of-warning-link-bugtasks;" |
737 | + tal:content="string:Bug #${duplicateof/id}: ${duplicateof/title}." |
738 | + >bug #42</a> |
739 | + </span> |
740 | + <a id="change-duplicate-bug-bugtasks" |
741 | + href="+duplicate" |
742 | + title="Edit or remove linked duplicate bug" |
743 | + class="sprite edit action-icon standalone change-duplicate-bug">Edit</a> |
744 | + <a id="remove-duplicate-bug-bugtasks" |
745 | + href="+duplicate" |
746 | + title="Remove linked duplicate bug" |
747 | + class="sprite remove action-icon standalone remove-duplicate-bug">Remove</a> |
748 | + </tal:dupe-info> |
749 | + </div> |
750 | <p id="bug-is-question" |
751 | tal:condition="context/bug/getQuestionCreatedFromBug" |
752 | tal:define="question context/bug/getQuestionCreatedFromBug"> |
753 | |
754 | === modified file 'lib/lp/code/javascript/branch.bugspeclinks.js' |
755 | --- lib/lp/code/javascript/branch.bugspeclinks.js 2012-08-07 09:28:09 +0000 |
756 | +++ lib/lp/code/javascript/branch.bugspeclinks.js 2012-08-09 04:59:21 +0000 |
757 | @@ -51,8 +51,8 @@ |
758 | superclass.prototype.bindUI.apply(this, arguments); |
759 | var that = this; |
760 | this.subscribe( |
761 | - Y.lp.bugs.bug_picker.BugPicker.SAVE_DUPLICATE, function(e) { |
762 | - e.preventDefault(); |
763 | + Y.lp.bugs.bug_picker.BugPicker.SAVE, function(e) { |
764 | + e.halt(); |
765 | that.set('progress', 100); |
766 | var bug_data = e.details[0]; |
767 | var bug_id = bug_data.id; |
768 | @@ -118,11 +118,10 @@ |
769 | } |
770 | var bug_link = |
771 | Y.lp.client.get_absolute_uri("/api/devel/bugs/" + bug_id); |
772 | - var spinner; |
773 | var that = this; |
774 | var error_handler = new Y.lp.client.ErrorHandler(); |
775 | error_handler.clearProgressUI = function() { |
776 | - that._hide_bug_spinner(spinner); |
777 | + that._hide_bug_spinner(widget); |
778 | that._hide_temporary_spinner(); |
779 | }; |
780 | error_handler.showError = function(error_msg) { |
781 | @@ -132,11 +131,11 @@ |
782 | on: { |
783 | start: function() { |
784 | that.set('error', null); |
785 | - spinner = that._show_bug_spinner(widget); |
786 | + that._show_bug_spinner(widget); |
787 | that._show_temporary_spinner(); |
788 | }, |
789 | success: function() { |
790 | - that._update_bug_links(bug_id, spinner); |
791 | + that._update_bug_links(bug_id, widget); |
792 | }, |
793 | failure: error_handler.getFailureHandler() |
794 | }, |
795 | @@ -154,7 +153,7 @@ |
796 | * @param widget |
797 | * @private |
798 | */ |
799 | - _update_bug_links: function(bug_id, spinner) { |
800 | + _update_bug_links: function(bug_id, widget) { |
801 | var error_handler = new Y.lp.client.ErrorHandler(); |
802 | error_handler.showError = function(error_msg) { |
803 | that.set('error', error_msg); |
804 | @@ -163,9 +162,6 @@ |
805 | this.lp_client.io_provider.io('++bug-links', { |
806 | on: { |
807 | end: function() { |
808 | - if (spinner !== null) { |
809 | - spinner.remove(true); |
810 | - } |
811 | that._hide_temporary_spinner(); |
812 | }, |
813 | success: function(id, response) { |
814 | @@ -273,7 +269,6 @@ |
815 | * Destroy the temporary "Linking..." text. |
816 | */ |
817 | _hide_temporary_spinner: function() { |
818 | - |
819 | var temp_spinner = Y.one('#temp-spinner'); |
820 | var spinner_parent = temp_spinner.get('parentNode'); |
821 | spinner_parent.removeChild(temp_spinner); |
IE 8 (and 7) does not support :not(). I think you need to write this like this: update- in-progress- message {} update- in-progress- message: after {}
span.
button.
I think the inline style on 402 is wrong. "style="max-width: 60em;" is only used when we know there is no side bar. sibling elements are 45ems. Why does this element allowed to extend beyond their length?
I see this pattern: duplicate- bug">', duplicate- bug">',
+ ' class="sprite remove action-icon remove-
415+ ' Remove</a>',
which puts leading white-space between the start of the anchor and the text. Even though the links are action-icon, parts of the underline can appear in some browsers, and quite likely for users that increase the text size. Maybe this is safer
+ ' class="sprite remove action-icon remove-
415+ 'Remove</a>',
TAL process its instructions in a specific order, define, then condition. This also means that define is always called on the element. Even though you wrote the condition first, the define was already executed: "context/ bug/duplicateof |nothing" "duplicateof context/ bug/duplicateof "> "duplicateof context/ bug/duplicateof " "duplicateof| nothing" >
570+ tal:condition=
571+ tal:define=
Since we know the order we could write this:
570+ tal:define=
571+ tal:condition=