Merge lp:~abentley/launchpad/reuse-refactor into lp:launchpad
- reuse-refactor
- Merge into devel
Proposed by
Aaron Bentley
Status: | Merged |
---|---|
Merged at revision: | not available |
Proposed branch: | lp:~abentley/launchpad/reuse-refactor |
Merge into: | lp:launchpad |
Diff against target: |
668 lines 7 files modified
lib/canonical/launchpad/interfaces/_schema_circular_imports.py (+2/-0) lib/canonical/launchpad/javascript/bugs/bugtask-index.js (+15/-155) lib/canonical/launchpad/javascript/lp/comment.js (+191/-0) lib/canonical/launchpad/javascript/lp/errors.js (+81/-0) lib/lp/bugs/templates/bugtask-index.pt (+14/-0) lib/lp/code/interfaces/branchmergeproposal.py (+6/-4) lib/lp/code/stories/webservice/xx-branchmergeproposal.txt (+4/-2) |
To merge this branch: | bzr merge lp:~abentley/launchpad/reuse-refactor |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Brad Crittenden (community) | code | Approve | |
Review via email: mp+13283@code.launchpad.net |
Commit message
Description of the change
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote : | # |
Revision history for this message
Brad Crittenden (bac) wrote : | # |
Aaron this is a great change. Thanks for moving the comment and error functionality to a place they can be re-used.
The only tiny nit I have is at line 645 of the diff where you have an in-line comment, which our coding standards discourage.
review:
Approve
(code)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py' | |||
2 | --- lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-09-09 11:28:03 +0000 | |||
3 | +++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py 2009-10-14 13:58:19 +0000 | |||
4 | @@ -100,6 +100,8 @@ | |||
5 | 100 | IBranchMergeProposal['createComment'].queryTaggedValue( | 100 | IBranchMergeProposal['createComment'].queryTaggedValue( |
6 | 101 | LAZR_WEBSERVICE_EXPORTED)['params']['parent'].schema = \ | 101 | LAZR_WEBSERVICE_EXPORTED)['params']['parent'].schema = \ |
7 | 102 | ICodeReviewComment | 102 | ICodeReviewComment |
8 | 103 | patch_entry_return_type( | ||
9 | 104 | IBranchMergeProposal, 'createComment', ICodeReviewComment) | ||
10 | 103 | IBranchMergeProposal['all_comments'].value_type.schema = ICodeReviewComment | 105 | IBranchMergeProposal['all_comments'].value_type.schema = ICodeReviewComment |
11 | 104 | IBranchMergeProposal['nominateReviewer'].queryTaggedValue( | 106 | IBranchMergeProposal['nominateReviewer'].queryTaggedValue( |
12 | 105 | LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = ICodeReviewVoteReference | 107 | LAZR_WEBSERVICE_EXPORTED)['return_type'].schema = ICodeReviewVoteReference |
13 | 106 | 108 | ||
14 | === modified file 'lib/canonical/launchpad/javascript/bugs/bugtask-index.js' | |||
15 | --- lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-06 21:58:10 +0000 | |||
16 | +++ lib/canonical/launchpad/javascript/bugs/bugtask-index.js 2009-10-14 13:58:19 +0000 | |||
17 | @@ -83,7 +83,7 @@ | |||
18 | 83 | } | 83 | } |
19 | 84 | }; | 84 | }; |
20 | 85 | error_handler.showError = function(error_msg) { | 85 | error_handler.showError = function(error_msg) { |
22 | 86 | display_error(Y.get('.menu-link-addsubscriber'), error_msg); | 86 | Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
23 | 87 | }; | 87 | }; |
24 | 88 | 88 | ||
25 | 89 | var config = { | 89 | var config = { |
26 | @@ -210,12 +210,11 @@ | |||
27 | 210 | }); | 210 | }); |
28 | 211 | privacy_link.addClass('js-action'); | 211 | privacy_link.addClass('js-action'); |
29 | 212 | } | 212 | } |
30 | 213 | create_error_overlay(); | ||
31 | 214 | setup_inline_commenting(); | ||
32 | 215 | setup_add_attachment(); | 213 | setup_add_attachment(); |
33 | 216 | }, window); | 214 | }, window); |
34 | 217 | }; | 215 | }; |
35 | 218 | 216 | ||
36 | 217 | |||
37 | 219 | /* | 218 | /* |
38 | 220 | * Clear the subscribe someone else picker. | 219 | * Clear the subscribe someone else picker. |
39 | 221 | * | 220 | * |
40 | @@ -313,7 +312,6 @@ | |||
41 | 313 | 312 | ||
42 | 314 | setup_unsubscribe_icon_handlers(subscription); | 313 | setup_unsubscribe_icon_handlers(subscription); |
43 | 315 | setup_subscribe_someone_else_handler(subscription); | 314 | setup_subscribe_someone_else_handler(subscription); |
44 | 316 | create_error_overlay(); | ||
45 | 317 | } | 315 | } |
46 | 318 | 316 | ||
47 | 319 | /* | 317 | /* |
48 | @@ -568,64 +566,6 @@ | |||
49 | 568 | lp_bug_entry.lp_save(config); | 566 | lp_bug_entry.lp_save(config); |
50 | 569 | }; | 567 | }; |
51 | 570 | 568 | ||
52 | 571 | /* | ||
53 | 572 | * Create the form overlay to use when encountering errors. | ||
54 | 573 | * | ||
55 | 574 | * @method create_error_overlay | ||
56 | 575 | */ | ||
57 | 576 | function create_error_overlay() { | ||
58 | 577 | if (error_overlay === undefined) { | ||
59 | 578 | error_overlay = new Y.lazr.FormOverlay({ | ||
60 | 579 | headerContent: '<h2>Error</h2>', | ||
61 | 580 | form_header: '', | ||
62 | 581 | form_content: '', | ||
63 | 582 | form_submit_button: Y.Node.create( | ||
64 | 583 | '<button style="display:none"></button>'), | ||
65 | 584 | form_cancel_button: cancel_form_button(), | ||
66 | 585 | centered: true, | ||
67 | 586 | visible: false | ||
68 | 587 | }); | ||
69 | 588 | error_overlay.render(); | ||
70 | 589 | } | ||
71 | 590 | } | ||
72 | 591 | |||
73 | 592 | /* | ||
74 | 593 | * Create a form button for canceling an error form | ||
75 | 594 | * that won't reload the page on submit. | ||
76 | 595 | * | ||
77 | 596 | * @method cancel_form_button | ||
78 | 597 | * @return button {Node} The form's cancel button. | ||
79 | 598 | */ | ||
80 | 599 | function cancel_form_button() { | ||
81 | 600 | var button = Y.Node.create('<button>OK</button>'); | ||
82 | 601 | button.on('click', function(e) { | ||
83 | 602 | e.preventDefault(); | ||
84 | 603 | error_overlay.hide(); | ||
85 | 604 | }); | ||
86 | 605 | return button; | ||
87 | 606 | } | ||
88 | 607 | |||
89 | 608 | /* | ||
90 | 609 | * Take an error message and display in an overlay (creating it if necessary). | ||
91 | 610 | * | ||
92 | 611 | * @method display_error | ||
93 | 612 | * @param flash_node {Node} The node to red flash. | ||
94 | 613 | * @param msg {String} The message to display. | ||
95 | 614 | */ | ||
96 | 615 | function display_error(flash_node, msg) { | ||
97 | 616 | create_error_overlay(); | ||
98 | 617 | if (flash_node) { | ||
99 | 618 | var anim = Y.lazr.anim.red_flash({ node: flash_node }); | ||
100 | 619 | anim.on('end', function(e) { | ||
101 | 620 | error_overlay.showError(msg); | ||
102 | 621 | error_overlay.show(); | ||
103 | 622 | }); | ||
104 | 623 | anim.run(); | ||
105 | 624 | } else { | ||
106 | 625 | error_overlay.showError(msg); | ||
107 | 626 | error_overlay.show(); | ||
108 | 627 | } | ||
109 | 628 | } | ||
110 | 629 | 569 | ||
111 | 630 | /* | 570 | /* |
112 | 631 | * Traverse the DOM of a given remove icon to find | 571 | * Traverse the DOM of a given remove icon to find |
113 | @@ -652,6 +592,7 @@ | |||
114 | 652 | return user_uri; | 592 | return user_uri; |
115 | 653 | } | 593 | } |
116 | 654 | 594 | ||
117 | 595 | |||
118 | 655 | /* | 596 | /* |
119 | 656 | * Build the HTML for a user link for the subscribers list. | 597 | * Build the HTML for a user link for the subscribers list. |
120 | 657 | * | 598 | * |
121 | @@ -1014,7 +955,7 @@ | |||
122 | 1014 | }; | 955 | }; |
123 | 1015 | error_handler.showError = function (error_msg) { | 956 | error_handler.showError = function (error_msg) { |
124 | 1016 | var flash_node = Y.get('.' + person.get('css_name')); | 957 | var flash_node = Y.get('.' + person.get('css_name')); |
126 | 1017 | display_error(flash_node, error_msg); | 958 | Y.lp.display_error(flash_node, error_msg); |
127 | 1018 | 959 | ||
128 | 1019 | }; | 960 | }; |
129 | 1020 | 961 | ||
130 | @@ -1089,7 +1030,7 @@ | |||
131 | 1089 | subscription.disable_spinner(); | 1030 | subscription.disable_spinner(); |
132 | 1090 | }; | 1031 | }; |
133 | 1091 | error_handler.showError = function (error_msg) { | 1032 | error_handler.showError = function (error_msg) { |
135 | 1092 | display_error(subscription_link, error_msg); | 1033 | Y.lp.display_error(subscription_link, error_msg); |
136 | 1093 | }; | 1034 | }; |
137 | 1094 | 1035 | ||
138 | 1095 | var config = { | 1036 | var config = { |
139 | @@ -1145,7 +1086,7 @@ | |||
140 | 1145 | subscription.disable_spinner(); | 1086 | subscription.disable_spinner(); |
141 | 1146 | }; | 1087 | }; |
142 | 1147 | error_handler.showError = function (error_msg) { | 1088 | error_handler.showError = function (error_msg) { |
144 | 1148 | display_error(subscription_link, error_msg); | 1089 | Y.lp.display_error(subscription_link, error_msg); |
145 | 1149 | }; | 1090 | }; |
146 | 1150 | 1091 | ||
147 | 1151 | var config = { | 1092 | var config = { |
148 | @@ -1257,7 +1198,7 @@ | |||
149 | 1257 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' | 1198 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' |
150 | 1258 | }); | 1199 | }); |
151 | 1259 | status_choice_edit.showError = function(err) { | 1200 | status_choice_edit.showError = function(err) { |
153 | 1260 | display_error(null, err); | 1201 | Y.lp.display_error(null, err); |
154 | 1261 | }; | 1202 | }; |
155 | 1262 | status_choice_edit.on('save', function(e) { | 1203 | status_choice_edit.on('save', function(e) { |
156 | 1263 | var cb = status_choice_edit.get('contentBox'); | 1204 | var cb = status_choice_edit.get('contentBox'); |
157 | @@ -1289,7 +1230,7 @@ | |||
158 | 1289 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' | 1230 | backgroundColor: tr.hasClass('highlight') ? '#FFFF99' : '#FFFFFF' |
159 | 1290 | }); | 1231 | }); |
160 | 1291 | importance_choice_edit.showError = function(err) { | 1232 | importance_choice_edit.showError = function(err) { |
162 | 1292 | display_error(null, err); | 1233 | Y.lp.display_error(null, err); |
163 | 1293 | }; | 1234 | }; |
164 | 1294 | importance_choice_edit.on('save', function(e) { | 1235 | importance_choice_edit.on('save', function(e) { |
165 | 1295 | var cb = importance_choice_edit.get('contentBox'); | 1236 | var cb = importance_choice_edit.get('contentBox'); |
166 | @@ -1324,7 +1265,7 @@ | |||
167 | 1324 | clickable_content: false | 1265 | clickable_content: false |
168 | 1325 | }); | 1266 | }); |
169 | 1326 | milestone_choice_edit.showError = function(err) { | 1267 | milestone_choice_edit.showError = function(err) { |
171 | 1327 | display_error(null, err); | 1268 | Y.lp.display_error(null, err); |
172 | 1328 | }; | 1269 | }; |
173 | 1329 | milestone_choice_edit.plug({ | 1270 | milestone_choice_edit.plug({ |
174 | 1330 | fn: Y.lp.client.plugins.PATCHPlugin, cfg: { | 1271 | fn: Y.lp.client.plugins.PATCHPlugin, cfg: { |
175 | @@ -1378,7 +1319,7 @@ | |||
176 | 1378 | "header": "Change assignee", | 1319 | "header": "Change assignee", |
177 | 1379 | "remove_button_text": "Remove assignee", | 1320 | "remove_button_text": "Remove assignee", |
178 | 1380 | "null_display_value": "Unassigned"}); | 1321 | "null_display_value": "Unassigned"}); |
180 | 1381 | assignee_picker.render() | 1322 | assignee_picker.render(); |
181 | 1382 | } | 1323 | } |
182 | 1383 | }; | 1324 | }; |
183 | 1384 | 1325 | ||
184 | @@ -1486,7 +1427,7 @@ | |||
185 | 1486 | }, | 1427 | }, |
186 | 1487 | 1428 | ||
187 | 1488 | showError: function(err) { | 1429 | showError: function(err) { |
189 | 1489 | display_error(null, err); | 1430 | Y.lp.display_error(null, err); |
190 | 1490 | }, | 1431 | }, |
191 | 1491 | 1432 | ||
192 | 1492 | syncUI: function() { | 1433 | syncUI: function() { |
193 | @@ -1549,7 +1490,7 @@ | |||
194 | 1549 | function check_can_be_unsubscribed(subscription) { | 1490 | function check_can_be_unsubscribed(subscription) { |
195 | 1550 | var error_handler = new LP.client.ErrorHandler(); | 1491 | var error_handler = new LP.client.ErrorHandler(); |
196 | 1551 | error_handler.showError = function (error_msg) { | 1492 | error_handler.showError = function (error_msg) { |
198 | 1552 | display_error(Y.get('.menu-link-addsubscriber'), error_msg); | 1493 | Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
199 | 1553 | }; | 1494 | }; |
200 | 1554 | 1495 | ||
201 | 1555 | var config = { | 1496 | var config = { |
202 | @@ -1618,7 +1559,7 @@ | |||
203 | 1618 | 1559 | ||
204 | 1619 | var error_handler = new LP.client.ErrorHandler(); | 1560 | var error_handler = new LP.client.ErrorHandler(); |
205 | 1620 | error_handler.showError = function(error_msg) { | 1561 | error_handler.showError = function(error_msg) { |
207 | 1621 | display_error(Y.get('.menu-link-addsubscriber'), error_msg); | 1562 | Y.lp.display_error(Y.get('.menu-link-addsubscriber'), error_msg); |
208 | 1622 | }; | 1563 | }; |
209 | 1623 | 1564 | ||
210 | 1624 | if (subscription.is_already_subscribed()) { | 1565 | if (subscription.is_already_subscribed()) { |
211 | @@ -1631,88 +1572,6 @@ | |||
212 | 1631 | } | 1572 | } |
213 | 1632 | 1573 | ||
214 | 1633 | /* | 1574 | /* |
215 | 1634 | * Set up and handle submitting a comment inline. | ||
216 | 1635 | * | ||
217 | 1636 | * @method setup_inline_commenting | ||
218 | 1637 | */ | ||
219 | 1638 | function setup_inline_commenting() { | ||
220 | 1639 | var submit_button = Y.get('[id="field.actions.save"]'); | ||
221 | 1640 | var progress_message = Y.Node.create( | ||
222 | 1641 | '<span class="update-in-progress-message">Saving...</span>'); | ||
223 | 1642 | var comment_input = Y.get('[id="field.comment"]'); | ||
224 | 1643 | |||
225 | 1644 | var error_handler = new LP.client.ErrorHandler(); | ||
226 | 1645 | error_handler.clearProgressUI = function () { | ||
227 | 1646 | clearProgressUI(); | ||
228 | 1647 | }; | ||
229 | 1648 | error_handler.showError = function (error_msg) { | ||
230 | 1649 | display_error(submit_button, error_msg); | ||
231 | 1650 | }; | ||
232 | 1651 | |||
233 | 1652 | function clearProgressUI() { | ||
234 | 1653 | progress_message.get('parentNode').replaceChild( | ||
235 | 1654 | submit_button, progress_message); | ||
236 | 1655 | comment_input.removeAttribute('disabled'); | ||
237 | 1656 | } | ||
238 | 1657 | |||
239 | 1658 | function hide_or_show_submit_button(e) { | ||
240 | 1659 | if (comment_input.get('value') === '') { | ||
241 | 1660 | submit_button.set('disabled', true); | ||
242 | 1661 | } | ||
243 | 1662 | else { | ||
244 | 1663 | submit_button.set('disabled', false); | ||
245 | 1664 | } | ||
246 | 1665 | } | ||
247 | 1666 | /* Hook up hide_or_show_submit_button on both keyup and mouseup to | ||
248 | 1667 | handle both entering text with the keyboard, and pasting with the | ||
249 | 1668 | mouse. */ | ||
250 | 1669 | comment_input.on('keyup', hide_or_show_submit_button); | ||
251 | 1670 | comment_input.on('mouseup', hide_or_show_submit_button); | ||
252 | 1671 | hide_or_show_submit_button(null); | ||
253 | 1672 | submit_button.addClass('js-action'); | ||
254 | 1673 | submit_button.setStyle('display', 'inline'); | ||
255 | 1674 | submit_button.on('click', function(e) { | ||
256 | 1675 | e.halt(); | ||
257 | 1676 | var comment_text = comment_input.get('value'); | ||
258 | 1677 | /* Don't try to add an empty comment. */ | ||
259 | 1678 | if (trim(comment_text) === '') { | ||
260 | 1679 | return; | ||
261 | 1680 | } | ||
262 | 1681 | var config = { | ||
263 | 1682 | on: { | ||
264 | 1683 | success: function(message_entry) { | ||
265 | 1684 | var config = { | ||
266 | 1685 | on: { | ||
267 | 1686 | success: function(message_html) { | ||
268 | 1687 | var fieldset = Y.get('#add-comment-form'); | ||
269 | 1688 | var legend = Y.get('#add-comment-form legend'); | ||
270 | 1689 | var comment = Y.Node.create(message_html); | ||
271 | 1690 | fieldset.get('parentNode').insertBefore( | ||
272 | 1691 | comment, fieldset); | ||
273 | 1692 | clearProgressUI(); | ||
274 | 1693 | comment_input.set('value', ''); | ||
275 | 1694 | Y.lazr.anim.green_flash({node: comment}).run(); | ||
276 | 1695 | } | ||
277 | 1696 | }, | ||
278 | 1697 | accept: LP.client.XHTML | ||
279 | 1698 | }; | ||
280 | 1699 | lp_client.get(message_entry.get('self_link'), config); | ||
281 | 1700 | }, | ||
282 | 1701 | failure: error_handler.getFailureHandler() | ||
283 | 1702 | }, | ||
284 | 1703 | parameters: { | ||
285 | 1704 | content: comment_input.get('value') | ||
286 | 1705 | } | ||
287 | 1706 | }; | ||
288 | 1707 | comment_input.set('disabled', 'true'); | ||
289 | 1708 | submit_button.get('parentNode').replaceChild( | ||
290 | 1709 | progress_message, submit_button); | ||
291 | 1710 | lp_client.named_post( | ||
292 | 1711 | bug_repr.self_link, 'newMessage', config); | ||
293 | 1712 | }); | ||
294 | 1713 | } | ||
295 | 1714 | |||
296 | 1715 | /* | ||
297 | 1716 | * Click handling to pass comment text to the attachment | 1575 | * Click handling to pass comment text to the attachment |
298 | 1717 | * page if there is a comment. | 1576 | * page if there is a comment. |
299 | 1718 | * | 1577 | * |
300 | @@ -1734,5 +1593,6 @@ | |||
301 | 1734 | }, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute', | 1593 | }, '0.1', {requires: ['base', 'oop', 'node', 'event', 'io-base', 'substitute', |
302 | 1735 | 'widget-position-ext', 'lazr.formoverlay', 'lazr.anim', | 1594 | 'widget-position-ext', 'lazr.formoverlay', 'lazr.anim', |
303 | 1736 | 'lazr.base', 'lazr.overlay', 'lazr.choiceedit', | 1595 | 'lazr.base', 'lazr.overlay', 'lazr.choiceedit', |
305 | 1737 | 'lp.picker', 'lp.client.plugins', 'lp.subscriber']}); | 1596 | 'lp.picker', 'lp.client.plugins', 'lp.subscriber', |
306 | 1597 | 'lp.errors']}); | ||
307 | 1738 | 1598 | ||
308 | 1739 | 1599 | ||
309 | === added file 'lib/canonical/launchpad/javascript/lp/comment.js' | |||
310 | --- lib/canonical/launchpad/javascript/lp/comment.js 1970-01-01 00:00:00 +0000 | |||
311 | +++ lib/canonical/launchpad/javascript/lp/comment.js 2009-10-14 13:58:19 +0000 | |||
312 | @@ -0,0 +1,191 @@ | |||
313 | 1 | YUI.add('lp.comment', function(Y) { | ||
314 | 2 | |||
315 | 3 | Y.lp = Y.namespace('lp'); | ||
316 | 4 | |||
317 | 5 | var Comment = function () { | ||
318 | 6 | Comment.superclass.constructor.apply(this, arguments); | ||
319 | 7 | }; | ||
320 | 8 | |||
321 | 9 | |||
322 | 10 | Comment.NAME = 'comment'; | ||
323 | 11 | |||
324 | 12 | Comment.ATTRS = { | ||
325 | 13 | }; | ||
326 | 14 | Y.extend(Comment, Y.Widget, { | ||
327 | 15 | |||
328 | 16 | /** | ||
329 | 17 | * Initialize the Comment | ||
330 | 18 | * | ||
331 | 19 | * @method initializer | ||
332 | 20 | */ | ||
333 | 21 | initializer: function() { | ||
334 | 22 | this.submit_button = this.get_submit(); | ||
335 | 23 | this.comment_input = Y.get('[id="field.comment"]'); | ||
336 | 24 | this.lp_client = new LP.client.Launchpad(); | ||
337 | 25 | this.error_handler = new LP.client.ErrorHandler(); | ||
338 | 26 | this.error_handler.clearProgressUI = bind(this.clearProgressUI, this); | ||
339 | 27 | this.error_handler.showError = bind(function (error_msg) { | ||
340 | 28 | Y.lp.display_error(this.submit_button, error_msg); | ||
341 | 29 | }, this); | ||
342 | 30 | this.progress_message = Y.Node.create( | ||
343 | 31 | '<span class="update-in-progress-message">Saving...</span>'); | ||
344 | 32 | }, | ||
345 | 33 | |||
346 | 34 | /** | ||
347 | 35 | * Return the Submit button. | ||
348 | 36 | * | ||
349 | 37 | * This is provided so that it can be overridden in subclasses. | ||
350 | 38 | * | ||
351 | 39 | * @method get_submit | ||
352 | 40 | */ | ||
353 | 41 | get_submit: function(){ | ||
354 | 42 | return Y.get('[id="field.actions.save"]'); | ||
355 | 43 | }, | ||
356 | 44 | /** | ||
357 | 45 | * Implementation of Widget.renderUI. | ||
358 | 46 | * | ||
359 | 47 | * This redisplays the submit button, in case it has been hidden by | ||
360 | 48 | * the web page. | ||
361 | 49 | * | ||
362 | 50 | * @method renderUI | ||
363 | 51 | */ | ||
364 | 52 | renderUI: function() { | ||
365 | 53 | this.submit_button.addClass('js-action'); | ||
366 | 54 | this.submit_button.setStyle('display', 'inline'); | ||
367 | 55 | }, | ||
368 | 56 | /** | ||
369 | 57 | * Ensure that the widget's values are suitable for submission. | ||
370 | 58 | * | ||
371 | 59 | * The contents of the comment field must contain at least one | ||
372 | 60 | * non-whitespace character. | ||
373 | 61 | * | ||
374 | 62 | * @method validate | ||
375 | 63 | */ | ||
376 | 64 | validate: function() { | ||
377 | 65 | return trim(this.comment_input.get('value')) !== ''; | ||
378 | 66 | }, | ||
379 | 67 | /** | ||
380 | 68 | * Make the widget enabled or disabled. | ||
381 | 69 | * | ||
382 | 70 | * @method set_disabled | ||
383 | 71 | * @param disabled A boolean, true if the widget is disabled. | ||
384 | 72 | */ | ||
385 | 73 | set_disabled: function(disabled){ | ||
386 | 74 | this.comment_input.set('disabled', disabled); | ||
387 | 75 | }, | ||
388 | 76 | /** | ||
389 | 77 | * Add the widget's comment as a new comment, updating the display. | ||
390 | 78 | * | ||
391 | 79 | * @method add_comment | ||
392 | 80 | * @param e An event | ||
393 | 81 | */ | ||
394 | 82 | add_comment: function(e){ | ||
395 | 83 | e.halt(); | ||
396 | 84 | /* Don't try to add an empty comment. */ | ||
397 | 85 | if (!this.validate()) { | ||
398 | 86 | return; | ||
399 | 87 | } | ||
400 | 88 | this.set_disabled(true); | ||
401 | 89 | this.submit_button.get('parentNode').replaceChild( | ||
402 | 90 | this.progress_message, this.submit_button); | ||
403 | 91 | this.post_comment(bind(function(message_entry) { | ||
404 | 92 | this.get_comment_HTML( | ||
405 | 93 | message_entry, bind(this.insert_comment_HTML, this)); | ||
406 | 94 | }, this)); | ||
407 | 95 | }, | ||
408 | 96 | /** | ||
409 | 97 | * Post the comment to the Launchpad API | ||
410 | 98 | * | ||
411 | 99 | * @method post_comment | ||
412 | 100 | * @param callback A callable to call if the post is successful. | ||
413 | 101 | */ | ||
414 | 102 | post_comment: function(callback) { | ||
415 | 103 | var config = { | ||
416 | 104 | on: { | ||
417 | 105 | success: callback, | ||
418 | 106 | failure: this.error_handler.getFailureHandler() | ||
419 | 107 | }, | ||
420 | 108 | parameters: {content: this.comment_input.get('value')} | ||
421 | 109 | }; | ||
422 | 110 | this.lp_client.named_post( | ||
423 | 111 | LP.client.cache.bug.self_link, 'newMessage', config); | ||
424 | 112 | }, | ||
425 | 113 | /** | ||
426 | 114 | * Retrieve the HTML of the specified message entry. | ||
427 | 115 | * | ||
428 | 116 | * @method get_comment_HTML | ||
429 | 117 | * @param message_entry The comment to get the HTML for. | ||
430 | 118 | * @param callback On success, call this with the HTML of the comment. | ||
431 | 119 | */ | ||
432 | 120 | get_comment_HTML: function(message_entry, callback){ | ||
433 | 121 | var config = { | ||
434 | 122 | on: { | ||
435 | 123 | success: callback | ||
436 | 124 | }, | ||
437 | 125 | accept: LP.client.XHTML | ||
438 | 126 | }; | ||
439 | 127 | this.lp_client.get(message_entry.get('self_link'), config); | ||
440 | 128 | }, | ||
441 | 129 | /** | ||
442 | 130 | * Insert the specified HTML into the page. | ||
443 | 131 | * | ||
444 | 132 | * @method insert_comment_HTML | ||
445 | 133 | * @param message_html The HTML of the comment to insert. | ||
446 | 134 | */ | ||
447 | 135 | insert_comment_HTML: function(message_html) { | ||
448 | 136 | var fieldset = Y.get('#add-comment-form'); | ||
449 | 137 | var comment = Y.Node.create(message_html); | ||
450 | 138 | fieldset.get('parentNode').insertBefore(comment, fieldset); | ||
451 | 139 | this.reset_contents(); | ||
452 | 140 | Y.lazr.anim.green_flash({node: comment}).run(); | ||
453 | 141 | }, | ||
454 | 142 | /** | ||
455 | 143 | * Reset the widget to a blank state. | ||
456 | 144 | * | ||
457 | 145 | * @method reset_contents | ||
458 | 146 | */ | ||
459 | 147 | reset_contents: function() { | ||
460 | 148 | this.clearProgressUI(); | ||
461 | 149 | this.comment_input.set('value', ''); | ||
462 | 150 | this.syncUI(); | ||
463 | 151 | }, | ||
464 | 152 | /** | ||
465 | 153 | * Stop indicating that a submission is in progress. | ||
466 | 154 | * | ||
467 | 155 | * @method clearProgressUI | ||
468 | 156 | */ | ||
469 | 157 | clearProgressUI: function(){ | ||
470 | 158 | this.progress_message.get('parentNode').replaceChild( | ||
471 | 159 | this.submit_button, this.progress_message); | ||
472 | 160 | this.set_disabled(false); | ||
473 | 161 | }, | ||
474 | 162 | /** | ||
475 | 163 | * Implementation of Widget.bindUI: Bind events to methods. | ||
476 | 164 | * | ||
477 | 165 | * Key and mouse presses (e.g. mouse paste) call syncUI, in case the submit | ||
478 | 166 | * button needs to be updated. Clicking on the submit button invokes | ||
479 | 167 | * add_comment. | ||
480 | 168 | * | ||
481 | 169 | * @method bindUI | ||
482 | 170 | */ | ||
483 | 171 | bindUI: function(){ | ||
484 | 172 | this.comment_input.on('keyup', bind(this.syncUI, this)); | ||
485 | 173 | this.comment_input.on('mouseup', bind(this.syncUI, this)); | ||
486 | 174 | this.submit_button.on('click', bind(this.add_comment, this)); | ||
487 | 175 | }, | ||
488 | 176 | /** | ||
489 | 177 | * Implementation of Widget.syncUI: Update appearance according to state. | ||
490 | 178 | * | ||
491 | 179 | * This just updates the submit button. | ||
492 | 180 | * | ||
493 | 181 | * @method syncUI | ||
494 | 182 | */ | ||
495 | 183 | syncUI: function(){ | ||
496 | 184 | this.submit_button.set('disabled', !this.validate()); | ||
497 | 185 | } | ||
498 | 186 | }); | ||
499 | 187 | |||
500 | 188 | Y.lp.Comment = Comment; | ||
501 | 189 | |||
502 | 190 | |||
503 | 191 | }, '0.1' ,{requires:['oop', 'widget', 'lp.client.plugins', 'lp.errors']}); | ||
504 | 0 | 192 | ||
505 | === added file 'lib/canonical/launchpad/javascript/lp/errors.js' | |||
506 | --- lib/canonical/launchpad/javascript/lp/errors.js 1970-01-01 00:00:00 +0000 | |||
507 | +++ lib/canonical/launchpad/javascript/lp/errors.js 2009-10-14 13:58:19 +0000 | |||
508 | @@ -0,0 +1,81 @@ | |||
509 | 1 | YUI.add('lp.errors', function(Y) { | ||
510 | 2 | |||
511 | 3 | Y.lp = Y.namespace('lp'); | ||
512 | 4 | |||
513 | 5 | /* | ||
514 | 6 | * Create a form button for canceling an error form | ||
515 | 7 | * that won't reload the page on submit. | ||
516 | 8 | * | ||
517 | 9 | * @method cancel_form_button | ||
518 | 10 | * @return button {Node} The form's cancel button. | ||
519 | 11 | */ | ||
520 | 12 | var cancel_form_button = function() { | ||
521 | 13 | var button = Y.Node.create('<button>OK</button>'); | ||
522 | 14 | button.on('click', function(e) { | ||
523 | 15 | e.preventDefault(); | ||
524 | 16 | error_overlay.hide(); | ||
525 | 17 | }); | ||
526 | 18 | return button; | ||
527 | 19 | }; | ||
528 | 20 | |||
529 | 21 | |||
530 | 22 | var error_overlay; | ||
531 | 23 | /* | ||
532 | 24 | * Create the form overlay to use when encountering errors. | ||
533 | 25 | * | ||
534 | 26 | * @method create_error_overlay | ||
535 | 27 | */ | ||
536 | 28 | var create_error_overlay = function() { | ||
537 | 29 | if (error_overlay === undefined) { | ||
538 | 30 | error_overlay = new Y.lazr.FormOverlay({ | ||
539 | 31 | headerContent: '<h2>Error</h2>', | ||
540 | 32 | form_header: '', | ||
541 | 33 | form_content: '', | ||
542 | 34 | form_submit_button: Y.Node.create( | ||
543 | 35 | '<button style="display:none"></button>'), | ||
544 | 36 | form_cancel_button: cancel_form_button(), | ||
545 | 37 | centered: true, | ||
546 | 38 | visible: false | ||
547 | 39 | }); | ||
548 | 40 | error_overlay.render(); | ||
549 | 41 | } | ||
550 | 42 | }; | ||
551 | 43 | |||
552 | 44 | /** | ||
553 | 45 | * Run a callback, optionally flashing a specified node red beforehand. | ||
554 | 46 | * | ||
555 | 47 | * If the supplied node evaluates false, the callback is invoked immediately. | ||
556 | 48 | * | ||
557 | 49 | * @method maybe_red_flash | ||
558 | 50 | * @param flash_node The node to flash red, or null for no flash. | ||
559 | 51 | * @param callback The callback to invoke. | ||
560 | 52 | */ | ||
561 | 53 | var maybe_red_flash = function(flash_node, callback) | ||
562 | 54 | { | ||
563 | 55 | if (flash_node) { | ||
564 | 56 | var anim = Y.lazr.anim.red_flash({ node: flash_node }); | ||
565 | 57 | anim.on('end', callback); | ||
566 | 58 | anim.run(); | ||
567 | 59 | } else { | ||
568 | 60 | callback(); | ||
569 | 61 | } | ||
570 | 62 | }; | ||
571 | 63 | |||
572 | 64 | |||
573 | 65 | /* | ||
574 | 66 | * Take an error message and display in an overlay (creating it if necessary). | ||
575 | 67 | * | ||
576 | 68 | * @method display_error | ||
577 | 69 | * @param flash_node {Node} The node to red flash. | ||
578 | 70 | * @param msg {String} The message to display. | ||
579 | 71 | */ | ||
580 | 72 | var display_error = function(flash_node, msg) { | ||
581 | 73 | create_error_overlay(); | ||
582 | 74 | maybe_red_flash(flash_node, function(){ | ||
583 | 75 | error_overlay.showError(msg); | ||
584 | 76 | error_overlay.show(); | ||
585 | 77 | }); | ||
586 | 78 | }; | ||
587 | 79 | |||
588 | 80 | Y.lp.display_error = display_error; | ||
589 | 81 | }, '0.1', {requires:['lazr.formoverlay']}); | ||
590 | 0 | 82 | ||
591 | === modified file 'lib/lp/bugs/templates/bugtask-index.pt' | |||
592 | --- lib/lp/bugs/templates/bugtask-index.pt 2009-09-22 18:05:41 +0000 | |||
593 | +++ lib/lp/bugs/templates/bugtask-index.pt 2009-10-14 13:58:19 +0000 | |||
594 | @@ -25,6 +25,14 @@ | |||
595 | 25 | tal:define="lp_js string:${icingroot}/build" | 25 | tal:define="lp_js string:${icingroot}/build" |
596 | 26 | tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js"> | 26 | tal:attributes="src string:${lp_js}/bugs/bug_tags_entry.js"> |
597 | 27 | </script> | 27 | </script> |
598 | 28 | <script type="text/javascript" | ||
599 | 29 | tal:define="lp_js string:${icingroot}/build" | ||
600 | 30 | tal:attributes="src string:${lp_js}/lp/comment.js"> | ||
601 | 31 | </script> | ||
602 | 32 | <script type="text/javascript" | ||
603 | 33 | tal:define="lp_js string:${icingroot}/build" | ||
604 | 34 | tal:attributes="src string:${lp_js}/lp/errors.js"> | ||
605 | 35 | </script> | ||
606 | 28 | </tal:devmode> | 36 | </tal:devmode> |
607 | 29 | <script type="text/javascript"> | 37 | <script type="text/javascript"> |
608 | 30 | YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) { | 38 | YUI().use('base', 'node', 'oop', 'event', 'bugs.bugtask_index', function(Y) { |
609 | @@ -282,6 +290,12 @@ | |||
610 | 282 | var button = document.getElementById('field.actions.save'); | 290 | var button = document.getElementById('field.actions.save'); |
611 | 283 | button.style.display = 'none'; | 291 | button.style.display = 'none'; |
612 | 284 | </script> | 292 | </script> |
613 | 293 | <script type="text/javascript"> | ||
614 | 294 | YUI().use('lp.comment', function(Y) { | ||
615 | 295 | var comment = new Y.lp.Comment(); | ||
616 | 296 | comment.render(); | ||
617 | 297 | }) | ||
618 | 298 | </script> | ||
619 | 285 | </div> | 299 | </div> |
620 | 286 | <tal:attachment-link | 300 | <tal:attachment-link |
621 | 287 | define="add_attachment_link context_menu/addcomment" | 301 | define="add_attachment_link context_menu/addcomment" |
622 | 288 | 302 | ||
623 | === modified file 'lib/lp/code/interfaces/branchmergeproposal.py' | |||
624 | --- lib/lp/code/interfaces/branchmergeproposal.py 2009-10-01 13:25:12 +0000 | |||
625 | +++ lib/lp/code/interfaces/branchmergeproposal.py 2009-10-14 13:58:19 +0000 | |||
626 | @@ -41,9 +41,10 @@ | |||
627 | 41 | from canonical.launchpad.webapp.interfaces import ITableBatchNavigator | 41 | from canonical.launchpad.webapp.interfaces import ITableBatchNavigator |
628 | 42 | from lazr.restful.fields import CollectionField, Reference | 42 | from lazr.restful.fields import CollectionField, Reference |
629 | 43 | from lazr.restful.declarations import ( | 43 | from lazr.restful.declarations import ( |
633 | 44 | call_with, export_as_webservice_entry, export_read_operation, | 44 | call_with, export_as_webservice_entry, export_factory_operation, |
634 | 45 | export_write_operation, exported, operation_parameters, | 45 | export_read_operation, export_write_operation, exported, |
635 | 46 | operation_returns_entry, rename_parameters_as, REQUEST_USER) | 46 | operation_parameters, operation_returns_entry, rename_parameters_as, |
636 | 47 | REQUEST_USER) | ||
637 | 47 | 48 | ||
638 | 48 | 49 | ||
639 | 49 | class InvalidBranchMergeProposal(Exception): | 50 | class InvalidBranchMergeProposal(Exception): |
640 | @@ -454,7 +455,8 @@ | |||
641 | 454 | vote=Choice(vocabulary=CodeReviewVote), review_type=Text(), | 455 | vote=Choice(vocabulary=CodeReviewVote), review_type=Text(), |
642 | 455 | parent=Reference(schema=Interface)) | 456 | parent=Reference(schema=Interface)) |
643 | 456 | @call_with(owner=REQUEST_USER) | 457 | @call_with(owner=REQUEST_USER) |
645 | 457 | @export_write_operation() | 458 | # ICodeReviewComment supplied as Interface to avoid circular imports. |
646 | 459 | @export_factory_operation(Interface, []) | ||
647 | 458 | def createComment(owner, subject, content=None, vote=None, | 460 | def createComment(owner, subject, content=None, vote=None, |
648 | 459 | review_type=None, parent=None): | 461 | review_type=None, parent=None): |
649 | 460 | """Create an ICodeReviewComment associated with this merge proposal. | 462 | """Create an ICodeReviewComment associated with this merge proposal. |
650 | 461 | 463 | ||
651 | === modified file 'lib/lp/code/stories/webservice/xx-branchmergeproposal.txt' | |||
652 | --- lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-12 09:40:17 +0000 | |||
653 | +++ lib/lp/code/stories/webservice/xx-branchmergeproposal.txt 2009-10-14 13:58:19 +0000 | |||
654 | @@ -146,10 +146,12 @@ | |||
655 | 146 | 146 | ||
656 | 147 | Now the code review should be made. | 147 | Now the code review should be made. |
657 | 148 | 148 | ||
659 | 149 | >>> comment = reviewer_webservice.named_post( | 149 | >>> comment_result = reviewer_webservice.named_post( |
660 | 150 | ... merge_proposal['self_link'], 'createComment', | 150 | ... merge_proposal['self_link'], 'createComment', |
661 | 151 | ... subject='Great work', content='This is great work', | 151 | ... subject='Great work', content='This is great work', |
663 | 152 | ... vote=CodeReviewVote.APPROVE.title, review_type='code').jsonBody() | 152 | ... vote=CodeReviewVote.APPROVE.title, review_type='code') |
664 | 153 | >>> comment_link = comment_result.getHeader('Location') | ||
665 | 154 | >>> comment = reviewer_webservice.get(comment_link).jsonBody() | ||
666 | 153 | >>> pprint_entry(comment) | 155 | >>> pprint_entry(comment) |
667 | 154 | branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1' | 156 | branch_merge_proposal_link: u'http://.../~source/fooix/fix-it/+merge/1' |
668 | 155 | id: 3 | 157 | id: 3 |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
= Summary =
Refactor the inline bug comment code for reuse, specifically by branch
merge proposals.
== Proposed fix ==
Extract the error handling code from bugtask-index.js into errors.js.
Extract the comment handling code into comment.js. Reorganize as a Widget.
== Pre-implementation notes ==
Preimplementation was with thumper.
== Implementation details == posal.createCom ment is factory_ operation. This ensures that its return value
As a drive-by, the export of IBranchMergePro
changed to export_
is usable.
== Tests == BugsWindmillLay er
bin/test -v --layer=
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files: code/interfaces /branchmergepro posal.py /launchpad/ javascript/ bugs/bugtask- index.js /launchpad/ javascript/ lp/errors. js /launchpad/ interfaces/ _schema_ circular_ imports. py bugs/templates/ bugtask- index.pt /launchpad/ javascript/ lp/comment. js
lib/lp/
lib/canonical
lib/canonical
lib/canonical
lib/lp/
lib/canonical
== JSLint notices == abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ lp/comment. js'.
jslint: No problem found in
'/home/
jslint: No problem found in abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ lp/errors. js'.
'/home/
jslint: No problem found in abentley/ launchpad/ inline- comment/ lib/canonical/ launchpad/ javascript/ bugs/bugtask- index.js' .
'/home/
jslint: 3 files to lint.
== Pylint notices ==
lib/lp/ code/interfaces /branchmergepro posal.py .event' (No module named fields' (No module named declarations' (No module
27: [F0401] Unable to import 'lazr.lifecycle
lifecycle)
42: [F0401] Unable to import 'lazr.restful.
restful)
43: [F0401] Unable to import 'lazr.restful.
named restful)
lib/canonical/ launchpad/ interfaces/ _schema_ circular_ imports. py declarations' (No module enigmail. mozdev. org
18: [F0401] Unable to import 'lazr.restful.
named restful)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://
iEYEARECAAYFAkr UklMACgkQ0F+ nu1YWqI0TIgCfcs mvIb+qgNSDBGRnD yciBJvh FvxrnRxj4Miho4v OzpQF912z
h3gAnin+
=+Dh5
-----END PGP SIGNATURE-----