Merge lp:~wallyworld/launchpad/improve-dupe-bug-ui-227310 into lp:launchpad

Proposed by Ian Booth
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
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.update-in-progress-message style to match the existing non button update-in-progress-message style. This style displays a spinner on the right hand side of the button. So now the bug picker widget can simply apply the same update-in-progress-message style to the clicked element (button or anchor) and the spinner displays nicely.

== Demo ==

Screenshot of the picker showing the new spinner icon for the for button and the message display:

http://people.canonical.com/~ianb/bug-dupe-message.png

== 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/launchpad/icing/style.css
  lib/lp/app/javascript/picker/picker.js
  lib/lp/bugs/javascript/bug_picker.js
  lib/lp/bugs/javascript/bugtask_index.js
  lib/lp/bugs/javascript/duplicates.js
  lib/lp/bugs/javascript/tests/test_bug_picker.js
  lib/lp/bugs/javascript/tests/test_duplicates.html
  lib/lp/bugs/javascript/tests/test_duplicates.js
  lib/lp/bugs/templates/bug-portlet-actions.pt
  lib/lp/bugs/templates/bugtask-index.pt
  lib/lp/code/javascript/branch.bugspeclinks.js

To post a comment you must log in.
Revision history for this message
Curtis Hovey (sinzui) wrote :

IE 8 (and 7) does not support :not(). I think you need to write this like this:
    span.update-in-progress-message {}
    button.update-in-progress-message:after {}

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:
     + ' class="sprite remove action-icon remove-duplicate-bug">',
  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-duplicate-bug">',
  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:
  570+ tal:condition="context/bug/duplicateof|nothing"
  571+ tal:define="duplicateof context/bug/duplicateof">
Since we know the order we could write this:
  570+ tal:define="duplicateof context/bug/duplicateof"
  571+ tal:condition="duplicateof|nothing">

review: Approve (code)
Revision history for this message
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-in-progress-message {}
> button.update-in-progress-message:after {}
>

Ok, thanks. I struggled to get this right. I also recently added :not()
to formoverlay-core.css so will have to revisit that to try and fix it.

> 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-duplicate-bug">',
> 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-duplicate-bug">',
> 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="context/bug/duplicateof|nothing"
> 571+ tal:define="duplicateof context/bug/duplicateof">
> Since we know the order we could write this:
> 570+ tal:define="duplicateof context/bug/duplicateof"
> 571+ tal:condition="duplicateof|nothing">
>
>

Good pickup. I missed this, thanks.

Revision history for this message
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.

Revision history for this message
Ian Booth (wallyworld) wrote :

> >
> > IE 8 (and 7) does not support :not(). I think you need to write this like
> this:
> > span.update-in-progress-message {}
> > button.update-in-progress-message:after {}
> >
>
> Ok, thanks. I struggled to get this right. I also recently added :not()
> to formoverlay-core.css so will have to revisit that to try and fix it.
>

The above won't work because it only covers <span> and not <a>
This works:

.update-in-progress-message {
    background: url(/@@/spinner) center left no-repeat;
    padding-left: 18px;
    color: #666;
}

button.update-in-progress-message {
    background: inherit;
    padding-left: inherit;
    color: inherit;
}

button.update-in-progress-message:after {
    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-in-progress-message {
    margin-top: -4px;
}

If you are ok with this I'll use this approach.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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>&nbsp;{{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+ ' &nbsp;{{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:&nbsp;',
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:&nbsp;
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);