Merge lp:~stevenk/launchpad/inline-diff-comments-ui into lp:launchpad
- inline-diff-comments-ui
- Merge into devel
Proposed by
Steve Kowalik
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Steve Kowalik | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 16871 | ||||
Proposed branch: | lp:~stevenk/launchpad/inline-diff-comments-ui | ||||
Merge into: | lp:launchpad | ||||
Diff against target: |
438 lines (+332/-7) 10 files modified
lib/canonical/launchpad/icing/style.css (+3/-0) lib/lp/app/browser/stringformatter.py (+1/-1) lib/lp/app/javascript/client.js (+2/-1) lib/lp/app/javascript/inlineedit/editor.js (+6/-2) lib/lp/code/browser/branchmergeproposal.py (+4/-2) lib/lp/code/javascript/branchmergeproposal.inlinecomments.js (+132/-0) lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html (+84/-0) lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js (+91/-0) lib/lp/code/templates/branchmergeproposal-index.pt (+3/-1) lib/lp/services/features/flags.py (+6/-0) |
||||
To merge this branch: | bzr merge lp:~stevenk/launchpad/inline-diff-comments-ui | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
William Grant | code | Approve | |
Review via email:
|
Commit message
Add the fully JavaScript UI for inline comments on merge proposal diffs.
Description of the change
Add the fully JavaScript UI for inline comments for diffs. This is protected by a feature flag, so should be safe to land but will change a little, since the server side bits to populate existing comments, save draft comments, and publish them require DB changes and modeling.
I have more then enough LoC to eat the net-positive cost of this branch.
To post a comment you must log in.
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
William Grant (wgrant) : | # |
review:
Approve
(code)
Revision history for this message
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Samuel Mehrbrodt (sam92) wrote : | # |
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'lib/canonical/launchpad/icing/style.css' | |||
2 | --- lib/canonical/launchpad/icing/style.css 2012-08-09 04:56:41 +0000 | |||
3 | +++ lib/canonical/launchpad/icing/style.css 2013-12-12 06:04:37 +0000 | |||
4 | @@ -628,6 +628,9 @@ | |||
5 | 628 | margin: 0 0 0 0; | 628 | margin: 0 0 0 0; |
6 | 629 | } | 629 | } |
7 | 630 | 630 | ||
8 | 631 | tr.ict-header { | ||
9 | 632 | background-color: #EEEEEE; | ||
10 | 633 | } | ||
11 | 631 | 634 | ||
12 | 632 | /* === Bugs === */ | 635 | /* === Bugs === */ |
13 | 633 | 636 | ||
14 | 634 | 637 | ||
15 | === modified file 'lib/lp/app/browser/stringformatter.py' | |||
16 | --- lib/lp/app/browser/stringformatter.py 2013-10-04 05:49:30 +0000 | |||
17 | +++ lib/lp/app/browser/stringformatter.py 2013-12-12 06:04:37 +0000 | |||
18 | @@ -882,7 +882,7 @@ | |||
19 | 882 | max_format_lines = config.diff.max_format_lines | 882 | max_format_lines = config.diff.max_format_lines |
20 | 883 | header_next = False | 883 | header_next = False |
21 | 884 | for row, line in enumerate(text.splitlines()[:max_format_lines]): | 884 | for row, line in enumerate(text.splitlines()[:max_format_lines]): |
23 | 885 | result.append('<tr>') | 885 | result.append('<tr id="diff-line-%s">' % (row + 1)) |
24 | 886 | result.append('<td class="line-no">%s</td>' % (row + 1)) | 886 | result.append('<td class="line-no">%s</td>' % (row + 1)) |
25 | 887 | if line.startswith('==='): | 887 | if line.startswith('==='): |
26 | 888 | css_class = 'diff-file text' | 888 | css_class = 'diff-file text' |
27 | 889 | 889 | ||
28 | === modified file 'lib/lp/app/javascript/client.js' | |||
29 | --- lib/lp/app/javascript/client.js 2012-09-07 15:13:40 +0000 | |||
30 | +++ lib/lp/app/javascript/client.js 2013-12-12 06:04:37 +0000 | |||
31 | @@ -1179,7 +1179,8 @@ | |||
32 | 1179 | return result; | 1179 | return result; |
33 | 1180 | } else { | 1180 | } else { |
34 | 1181 | if (this.get('use_html')) { | 1181 | if (this.get('use_html')) { |
36 | 1182 | return result.getHTML(attribute).get('innerHTML'); | 1182 | return Y.Node.create( |
37 | 1183 | result.getHTML(attribute).get('innerHTML')); | ||
38 | 1183 | } else { | 1184 | } else { |
39 | 1184 | return result.get(attribute); | 1185 | return result.get(attribute); |
40 | 1185 | } | 1186 | } |
41 | 1186 | 1187 | ||
42 | === modified file 'lib/lp/app/javascript/inlineedit/editor.js' | |||
43 | --- lib/lp/app/javascript/inlineedit/editor.js 2013-03-20 03:41:40 +0000 | |||
44 | +++ lib/lp/app/javascript/inlineedit/editor.js 2013-12-12 06:04:37 +0000 | |||
45 | @@ -1339,9 +1339,13 @@ | |||
46 | 1339 | var text = this.get(TEXT), | 1339 | var text = this.get(TEXT), |
47 | 1340 | val = this.editor.get(VALUE); | 1340 | val = this.editor.get(VALUE); |
48 | 1341 | text.setData(ORIGINAL_ELLIPSIS_TEXT, val); | 1341 | text.setData(ORIGINAL_ELLIPSIS_TEXT, val); |
50 | 1342 | text.set('innerHTML', ''); | 1342 | text.set('text', ''); |
51 | 1343 | if (this.editor.get(MULTILINE)) { | 1343 | if (this.editor.get(MULTILINE)) { |
53 | 1344 | text.set('innerHTML', val); | 1344 | if (Y.Lang.isString(val)) { |
54 | 1345 | text.set('text', val); | ||
55 | 1346 | } else { | ||
56 | 1347 | text.appendChild(val); | ||
57 | 1348 | } | ||
58 | 1345 | } else { | 1349 | } else { |
59 | 1346 | text.appendChild(document.createTextNode(val)); | 1350 | text.appendChild(document.createTextNode(val)); |
60 | 1347 | } | 1351 | } |
61 | 1348 | 1352 | ||
62 | === modified file 'lib/lp/code/browser/branchmergeproposal.py' | |||
63 | --- lib/lp/code/browser/branchmergeproposal.py 2013-04-10 08:09:05 +0000 | |||
64 | +++ lib/lp/code/browser/branchmergeproposal.py 2013-12-12 06:04:37 +0000 | |||
65 | @@ -623,8 +623,10 @@ | |||
66 | 623 | self.context.source_branch.unique_name), | 623 | self.context.source_branch.unique_name), |
67 | 624 | }) | 624 | }) |
68 | 625 | if getFeatureFlag("longpoll.merge_proposals.enabled"): | 625 | if getFeatureFlag("longpoll.merge_proposals.enabled"): |
71 | 626 | cache.objects['merge_proposal_event_key'] = ( | 626 | cache.objects['merge_proposal_event_key'] = subscribe( |
72 | 627 | subscribe(self.context).event_key) | 627 | self.context).event_key |
73 | 628 | if getFeatureFlag("code.inline_diff_comments.enabled"): | ||
74 | 629 | cache.objects['inline_diff_comments'] = True | ||
75 | 628 | 630 | ||
76 | 629 | @action('Claim', name='claim') | 631 | @action('Claim', name='claim') |
77 | 630 | def claim_action(self, action, data): | 632 | def claim_action(self, action, data): |
78 | 631 | 633 | ||
79 | === added file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js' | |||
80 | --- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 1970-01-01 00:00:00 +0000 | |||
81 | +++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2013-12-12 06:04:37 +0000 | |||
82 | @@ -0,0 +1,132 @@ | |||
83 | 1 | /* Copyright 2013 Canonical Ltd. This software is licensed under the | ||
84 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). | ||
85 | 3 | * | ||
86 | 4 | * Code for handling inline comments in diffs. | ||
87 | 5 | * | ||
88 | 6 | * @module lp.code.branchmergeproposal.inlinecomments | ||
89 | 7 | * @requires node | ||
90 | 8 | */ | ||
91 | 9 | |||
92 | 10 | YUI.add('lp.code.branchmergeproposal.inlinecomments', function(Y) { | ||
93 | 11 | |||
94 | 12 | // Grab the namespace in order to be able to expose the connect methods. | ||
95 | 13 | var namespace = Y.namespace('lp.code.branchmergeproposal.inlinecomments'); | ||
96 | 14 | |||
97 | 15 | namespace.add_doubleclick_handler = function() { | ||
98 | 16 | var inlinecomments = {}; | ||
99 | 17 | var rows = Y.one('.diff').all('tr'); | ||
100 | 18 | var handling_request = false; | ||
101 | 19 | handler = function(e) { | ||
102 | 20 | if (handling_request === true) { | ||
103 | 21 | return; | ||
104 | 22 | } | ||
105 | 23 | handling_request = true; | ||
106 | 24 | var linenumberdata = e.currentTarget.one('.line-no'); | ||
107 | 25 | var rownumber = linenumberdata.get('text'); | ||
108 | 26 | var rows = namespace.create_or_return_row(rownumber, null, null, null); | ||
109 | 27 | var headerrow = rows[0]; | ||
110 | 28 | var newrow = rows[1]; | ||
111 | 29 | var widget = new Y.EditableText({ | ||
112 | 30 | contentBox: newrow.one('#inlinecomment-' + rownumber + '-draft'), | ||
113 | 31 | initial_value_override: inlinecomments[rownumber], | ||
114 | 32 | accept_empty: true, | ||
115 | 33 | multiline: true, | ||
116 | 34 | buttons: 'top' | ||
117 | 35 | }); | ||
118 | 36 | widget.render(); | ||
119 | 37 | handle_widget_button = function(saved, comment) { | ||
120 | 38 | if (saved === true) { | ||
121 | 39 | inlinecomments[rownumber] = comment; | ||
122 | 40 | } | ||
123 | 41 | if (comment === '') { | ||
124 | 42 | headerrow.remove(true); | ||
125 | 43 | newrow.remove(true); | ||
126 | 44 | } | ||
127 | 45 | handling_request = false; | ||
128 | 46 | }; | ||
129 | 47 | widget.editor.on('save', function() { | ||
130 | 48 | handle_widget_button(true, this.get('value')); | ||
131 | 49 | }); | ||
132 | 50 | widget.editor.on('cancel', function(e) { | ||
133 | 51 | handle_widget_button(false, this.get('value')); | ||
134 | 52 | }); | ||
135 | 53 | widget._triggerEdit(e); | ||
136 | 54 | }; | ||
137 | 55 | rows.on('dblclick', handler); | ||
138 | 56 | }; | ||
139 | 57 | |||
140 | 58 | namespace.create_or_return_row = function(rownumber, person, comment, date) { | ||
141 | 59 | var suffix = '-draft'; | ||
142 | 60 | var middle = rownumber + suffix; | ||
143 | 61 | var draft = true; | ||
144 | 62 | if (person !== null) { | ||
145 | 63 | draft = false; | ||
146 | 64 | } | ||
147 | 65 | var headerrow = null; | ||
148 | 66 | if (draft === true) { | ||
149 | 67 | headerrow = Y.one('#ict-' + middle + '-header'); | ||
150 | 68 | if (headerrow !== null) { | ||
151 | 69 | return [headerrow, headerrow.next()]; | ||
152 | 70 | } | ||
153 | 71 | } | ||
154 | 72 | headerrow = Y.Node.create( | ||
155 | 73 | '<tr><td colspan="2"></td></tr>').addClass('ict-header'); | ||
156 | 74 | var headerspan; | ||
157 | 75 | if (person !== null && date !== null) { | ||
158 | 76 | headerspan = Y.Node.create( | ||
159 | 77 | '<span>Comment by <a></a> on <span></span></span>'); | ||
160 | 78 | headerspan.one('a').set('href', person.web_link).set( | ||
161 | 79 | 'text', person.display_name + ' (' + person.name + ')'); | ||
162 | 80 | headerspan.one('span').set('text', date); | ||
163 | 81 | } else { | ||
164 | 82 | headerspan = Y.Node.create('<span></span>').set( | ||
165 | 83 | 'text', 'Draft comment.'); | ||
166 | 84 | } | ||
167 | 85 | headerrow.one('td').appendChild(headerspan); | ||
168 | 86 | if (draft === true) { | ||
169 | 87 | headerrow.set('id', 'ict-' + middle + '-header'); | ||
170 | 88 | newrow = Y.Node.create('<tr><td></td><td><div>' + | ||
171 | 89 | '<span class="yui3-editable_text-text"></span>' + | ||
172 | 90 | '<div class="yui3-editable_text-trigger"></div>' + | ||
173 | 91 | '</div></td></tr>').set('id', 'ict-' + middle); | ||
174 | 92 | newrow.one('td>div').set('id', 'inlinecomment-' + middle); | ||
175 | 93 | } else { | ||
176 | 94 | newrow = Y.Node.create('<tr><td></td><td><span></span></td></tr>'); | ||
177 | 95 | newrow.one('span').set('text', comment); | ||
178 | 96 | } | ||
179 | 97 | // We want to have the comments in order after the line, so grab the | ||
180 | 98 | // next row. | ||
181 | 99 | var tr = Y.one('#diff-line-' + (parseInt(rownumber, 10) + 1)); | ||
182 | 100 | if (tr !== null) { | ||
183 | 101 | tr.insert(headerrow, 'before'); | ||
184 | 102 | } else { | ||
185 | 103 | // If this is the last row, grab the last child. | ||
186 | 104 | tr = Y.one('.diff>tbody>tr:last-child'); | ||
187 | 105 | tr.insert(headerrow, 'after'); | ||
188 | 106 | } | ||
189 | 107 | // The header row is the tricky one to place, the comment just goes | ||
190 | 108 | // after it. | ||
191 | 109 | headerrow.insert(newrow, 'after'); | ||
192 | 110 | return [headerrow, newrow]; | ||
193 | 111 | }; | ||
194 | 112 | |||
195 | 113 | namespace.populate_existing_comments = function() { | ||
196 | 114 | var i; | ||
197 | 115 | for (i = 0; i < LP.cache.published_inline_comments.length; i++) { | ||
198 | 116 | var row = LP.cache.published_inline_comments[i]; | ||
199 | 117 | namespace.create_or_return_row( | ||
200 | 118 | row.line, row.person, row.comment, row.date); | ||
201 | 119 | } | ||
202 | 120 | }; | ||
203 | 121 | |||
204 | 122 | namespace.setup_inline_comments = function() { | ||
205 | 123 | if (LP.cache.inline_diff_comments === true) { | ||
206 | 124 | // Add the double-click handler for each row in the diff. This needs | ||
207 | 125 | // to be done first since populating existing published and draft | ||
208 | 126 | // comments may add more rows. | ||
209 | 127 | namespace.add_doubleclick_handler(); | ||
210 | 128 | namespace.populate_existing_comments(); | ||
211 | 129 | } | ||
212 | 130 | }; | ||
213 | 131 | |||
214 | 132 | }, '0.1', {requires: ['event', 'io', 'node', 'widget', 'lp.ui.editor']}); | ||
215 | 0 | 133 | ||
216 | === added file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html' | |||
217 | --- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 1970-01-01 00:00:00 +0000 | |||
218 | +++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.html 2013-12-12 06:04:37 +0000 | |||
219 | @@ -0,0 +1,84 @@ | |||
220 | 1 | <!DOCTYPE html> | ||
221 | 2 | <!-- | ||
222 | 3 | Copyright 2013 Canonical Ltd. This software is licensed under the | ||
223 | 4 | GNU Affero General Public License version 3 (see the file LICENSE). | ||
224 | 5 | --> | ||
225 | 6 | |||
226 | 7 | <html> | ||
227 | 8 | <head> | ||
228 | 9 | <title>code.branchmergeproposal.inlinecomments Tests</title> | ||
229 | 10 | |||
230 | 11 | <!-- YUI and test setup --> | ||
231 | 12 | <script type="text/javascript" | ||
232 | 13 | src="../../../../../build/js/yui/yui/yui.js"> | ||
233 | 14 | </script> | ||
234 | 15 | <link rel="stylesheet" | ||
235 | 16 | href="../../../../../build/js/yui/console/assets/console-core.css" /> | ||
236 | 17 | <link rel="stylesheet" | ||
237 | 18 | href="../../../../../build/js/yui/test-console/assets/skins/sam/test-console.css" /> | ||
238 | 19 | <link rel="stylesheet" | ||
239 | 20 | href="../../../../../build/js/yui/test/assets/skins/sam/test.css" /> | ||
240 | 21 | |||
241 | 22 | <script type="text/javascript" | ||
242 | 23 | src="../../../../../build/js/lp/app/testing/testrunner.js"></script> | ||
243 | 24 | <script type="text/javascript" | ||
244 | 25 | src="../../../../../build/js/lp/app/testing/helpers.js"></script> | ||
245 | 26 | |||
246 | 27 | <link rel="stylesheet" href="../../../app/javascript/testing/test.css" /> | ||
247 | 28 | |||
248 | 29 | <!-- Dependencies --> | ||
249 | 30 | <script type="text/javascript" | ||
250 | 31 | src="../../../../../build/js/lp/app/ui/ui.js"></script> | ||
251 | 32 | <script type="text/javascript" | ||
252 | 33 | src="../../../../../build/js/lp/app/extras/extras.js"></script> | ||
253 | 34 | <script type="text/javascript" | ||
254 | 35 | src="../../../../../build/js/lp/app/anim/anim.js"></script> | ||
255 | 36 | <script type="text/javascript" | ||
256 | 37 | src="../../../../../build/js/lp/app/errors.js"></script> | ||
257 | 38 | <script type="text/javascript" | ||
258 | 39 | src="../../../../../build/js/lp/app/overlay/overlay.js"></script> | ||
259 | 40 | <script type="text/javascript" | ||
260 | 41 | src="../../../../../build/js/lp/app/formoverlay/formoverlay.js"></script> | ||
261 | 42 | <script type="text/javascript" | ||
262 | 43 | src="../../../../../build/js/lp/app/formwidgets/resizing_textarea.js"></script> | ||
263 | 44 | <script type="text/javascript" | ||
264 | 45 | src="../../../../../build/js/lp/app/ellipsis.js"></script> | ||
265 | 46 | <script type="text/javascript" | ||
266 | 47 | src="../../../../../build/js/lp/app/inlineedit/editor.js"></script> | ||
267 | 48 | |||
268 | 49 | <!-- The module under test. --> | ||
269 | 50 | <script type="text/javascript" src="../branchmergeproposal.inlinecomments.js"></script> | ||
270 | 51 | |||
271 | 52 | <!-- The test suite --> | ||
272 | 53 | <script type="text/javascript" src="test_branchmergeproposal.inlinecomments.js"></script> | ||
273 | 54 | |||
274 | 55 | <!-- expected variable --> | ||
275 | 56 | <script type="text/javascript"> | ||
276 | 57 | var LP = { | ||
277 | 58 | cache: {} | ||
278 | 59 | }; | ||
279 | 60 | </script> | ||
280 | 61 | |||
281 | 62 | </head> | ||
282 | 63 | <body class="yui3-skin-sam"> | ||
283 | 64 | <ul id="suites"> | ||
284 | 65 | <li>lp.code.branchmergeproposal.inlinecomments.test</li> | ||
285 | 66 | </ul> | ||
286 | 67 | <table class="diff"> | ||
287 | 68 | <tbody> | ||
288 | 69 | <tr id="diff-line-1"> | ||
289 | 70 | <td class="line-no">1</td> | ||
290 | 71 | <td class="diff-header text">foo bar</td> | ||
291 | 72 | </tr> | ||
292 | 73 | <tr id="diff-line-2"> | ||
293 | 74 | <td class="line-no">2</td> | ||
294 | 75 | <td class="text">baz</td> | ||
295 | 76 | </tr> | ||
296 | 77 | <tr id="diff-line-3"> | ||
297 | 78 | <td class="line-no">3</td> | ||
298 | 79 | <td class="text">quux</td> | ||
299 | 80 | </tr> | ||
300 | 81 | </tbody> | ||
301 | 82 | </table> | ||
302 | 83 | </body> | ||
303 | 84 | </html> | ||
304 | 0 | 85 | ||
305 | === added file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js' | |||
306 | --- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 1970-01-01 00:00:00 +0000 | |||
307 | +++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js 2013-12-12 06:04:37 +0000 | |||
308 | @@ -0,0 +1,91 @@ | |||
309 | 1 | /* Copyright 2013 Canonical Ltd. This software is licensed under the | ||
310 | 2 | * GNU Affero General Public License version 3 (see the file LICENSE). */ | ||
311 | 3 | |||
312 | 4 | YUI.add('lp.code.branchmergeproposal.inlinecomments.test', function (Y) { | ||
313 | 5 | |||
314 | 6 | var module = Y.lp.code.branchmergeproposal.inlinecomments; | ||
315 | 7 | var tests = Y.namespace('lp.code.branchmergeproposal.inlinecomments.test'); | ||
316 | 8 | tests.suite = new Y.Test.Suite('branchmergeproposal.inlinecomments Tests'); | ||
317 | 9 | |||
318 | 10 | tests.suite.add(new Y.Test.Case({ | ||
319 | 11 | name: 'code.branchmergeproposal.inlinecomments_tests', | ||
320 | 12 | |||
321 | 13 | setUp: function () {}, | ||
322 | 14 | tearDown: function () {}, | ||
323 | 15 | |||
324 | 16 | test_library_exists: function () { | ||
325 | 17 | Y.Assert.isObject( | ||
326 | 18 | module, "Could not locate the " + | ||
327 | 19 | "lp.code.branchmergeproposal.inlinecomments module"); | ||
328 | 20 | }, | ||
329 | 21 | |||
330 | 22 | test_populatation: function () { | ||
331 | 23 | var published_inline_comments = [ | ||
332 | 24 | {'line': 2, 'person': {'display_name': 'Sample Person', | ||
333 | 25 | 'name': 'name.12', 'web_link': 'http://launchpad.dev/~name12'}, | ||
334 | 26 | 'comment': 'This is preloaded.', 'date': '2012-08-12 17:45'}]; | ||
335 | 27 | LP.cache.published_inline_comments = published_inline_comments; | ||
336 | 28 | module.populate_existing_comments(); | ||
337 | 29 | var row = Y.one('#diff-line-2').next().next(); | ||
338 | 30 | Y.Assert.areEqual('This is preloaded.', row.get('text')); | ||
339 | 31 | }, | ||
340 | 32 | |||
341 | 33 | test_handler_normal: function() { | ||
342 | 34 | module.add_doubleclick_handler(); | ||
343 | 35 | var line_2 = Y.one('#diff-line-2'); | ||
344 | 36 | line_2.simulate('dblclick'); | ||
345 | 37 | var comment = 'This is a comment.'; | ||
346 | 38 | Y.one('.yui3-ieditor-input>textarea').set('value', comment); | ||
347 | 39 | Y.one('.lazr-pos').simulate('click'); | ||
348 | 40 | Y.Assert.areEqual( | ||
349 | 41 | comment, Y.one('.yui3-editable_text-text').get('text')); | ||
350 | 42 | }, | ||
351 | 43 | |||
352 | 44 | test_handler_cancel: function() { | ||
353 | 45 | var line_2 = Y.one('#diff-line-2'); | ||
354 | 46 | line_2.simulate('dblclick'); | ||
355 | 47 | var comment = 'Cancelling test.'; | ||
356 | 48 | Y.one('.yui3-ieditor-input>textarea').set('value', comment); | ||
357 | 49 | Y.one('.lazr-pos').simulate('click'); | ||
358 | 50 | line_2.simulate('dblclick'); | ||
359 | 51 | Y.one('.yui3-ieditor-input>textarea').set('value', 'Foobar!'); | ||
360 | 52 | Y.one('.lazr-neg').simulate('click'); | ||
361 | 53 | Y.Assert.areEqual( | ||
362 | 54 | comment, Y.one('.yui3-editable_text-text').get('text')); | ||
363 | 55 | }, | ||
364 | 56 | |||
365 | 57 | test_handler_cancel_immediately: function() { | ||
366 | 58 | var line_1 = Y.one('#diff-line-1'); | ||
367 | 59 | line_1.simulate('dblclick'); | ||
368 | 60 | Y.one('.lazr-neg').simulate('click'); | ||
369 | 61 | Y.Assert.isNull(Y.one('#ict-1-draft-header')); | ||
370 | 62 | }, | ||
371 | 63 | |||
372 | 64 | test_feature_flag_off: function() { | ||
373 | 65 | var called = false; | ||
374 | 66 | add_doubleclick_handler = function() { | ||
375 | 67 | called = true; | ||
376 | 68 | }; | ||
377 | 69 | module.add_doubleclick_handler = add_doubleclick_handler; | ||
378 | 70 | module.setup_inline_comments(); | ||
379 | 71 | Y.Assert.isFalse(called); | ||
380 | 72 | }, | ||
381 | 73 | |||
382 | 74 | test_feature_flag: function() { | ||
383 | 75 | LP.cache.published_inline_comments = []; | ||
384 | 76 | var called = false; | ||
385 | 77 | add_doubleclick_handler = function() { | ||
386 | 78 | called = true; | ||
387 | 79 | }; | ||
388 | 80 | module.add_doubleclick_handler = add_doubleclick_handler; | ||
389 | 81 | window.LP.cache.inline_diff_comments = true; | ||
390 | 82 | module.setup_inline_comments(); | ||
391 | 83 | Y.Assert.isTrue(called); | ||
392 | 84 | } | ||
393 | 85 | |||
394 | 86 | })); | ||
395 | 87 | |||
396 | 88 | }, '0.1', { | ||
397 | 89 | requires: ['node-event-simulate', 'test', 'lp.testing.helpers', 'console', | ||
398 | 90 | 'lp.code.branchmergeproposal.inlinecomments'] | ||
399 | 91 | }); | ||
400 | 0 | 92 | ||
401 | === modified file 'lib/lp/code/templates/branchmergeproposal-index.pt' | |||
402 | --- lib/lp/code/templates/branchmergeproposal-index.pt 2012-06-28 14:19:28 +0000 | |||
403 | +++ lib/lp/code/templates/branchmergeproposal-index.pt 2013-12-12 06:04:37 +0000 | |||
404 | @@ -197,7 +197,8 @@ | |||
405 | 197 | LPJS.use('io-base', 'lp.code.branchmergeproposal.reviewcomment', | 197 | LPJS.use('io-base', 'lp.code.branchmergeproposal.reviewcomment', |
406 | 198 | 'lp.code.branchmergeproposal.status', 'lp.app.comment', | 198 | 'lp.code.branchmergeproposal.status', 'lp.app.comment', |
407 | 199 | 'lp.code.branchmergeproposal.updater', 'lp.app.widgets.expander', | 199 | 'lp.code.branchmergeproposal.updater', 'lp.app.widgets.expander', |
409 | 200 | 'lp.code.branch.revisionexpander', function(Y) { | 200 | 'lp.code.branch.revisionexpander', |
410 | 201 | 'lp.code.branchmergeproposal.inlinecomments', function(Y) { | ||
411 | 201 | 202 | ||
412 | 202 | Y.on('load', function() { | 203 | Y.on('load', function() { |
413 | 203 | var logged_in = LP.links['me'] !== undefined; | 204 | var logged_in = LP.links['me'] !== undefined; |
414 | @@ -237,6 +238,7 @@ | |||
415 | 237 | '.expander-content', | 238 | '.expander-content', |
416 | 238 | false, | 239 | false, |
417 | 239 | Y.lp.code.branch.revisionexpander.bmp_diff_loader); | 240 | Y.lp.code.branch.revisionexpander.bmp_diff_loader); |
418 | 241 | Y.lp.code.branchmergeproposal.inlinecomments.setup_inline_comments(); | ||
419 | 240 | }); | 242 | }); |
420 | 241 | }); | 243 | }); |
421 | 242 | --> | 244 | --> |
422 | 243 | 245 | ||
423 | === modified file 'lib/lp/services/features/flags.py' | |||
424 | --- lib/lp/services/features/flags.py 2013-08-08 08:49:04 +0000 | |||
425 | +++ lib/lp/services/features/flags.py 2013-12-12 06:04:37 +0000 | |||
426 | @@ -221,6 +221,12 @@ | |||
427 | 221 | 'disabled', | 221 | 'disabled', |
428 | 222 | '', | 222 | '', |
429 | 223 | ''), | 223 | ''), |
430 | 224 | ('code.inline_diff_comments.enabled', | ||
431 | 225 | 'space delimited', | ||
432 | 226 | 'Names of projects have inline diff comments enabled.', | ||
433 | 227 | '', | ||
434 | 228 | '', | ||
435 | 229 | ''), | ||
436 | 224 | ]) | 230 | ]) |
437 | 225 | 231 | ||
438 | 226 | # The set of all flag names that are documented. | 232 | # The set of all flag names that are documented. |
Hi,
are there any plans to enable this feature on launchpad.net?
Would be really helpful.