Merge lp:~blr/launchpad/inline-comment-delint into lp:launchpad

Proposed by Kit Randel
Status: Merged
Approved by: Kit Randel
Approved revision: no longer in the source branch.
Merged at revision: 17311
Proposed branch: lp:~blr/launchpad/inline-comment-delint
Merge into: lp:launchpad
Diff against target: 233 lines (+38/-34)
1 file modified
lib/lp/code/javascript/branchmergeproposal.inlinecomments.js (+38/-34)
To merge this branch: bzr merge lp:~blr/launchpad/inline-comment-delint
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+243322@code.launchpad.net

Description of the change

A delinting pass over the inline comment code I've been looking at recently.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-11-27 22:27:28 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js 2014-12-01 19:25:34 +0000
@@ -43,7 +43,7 @@
43namespace.flush_drafts_to_server = function() {43namespace.flush_drafts_to_server = function() {
44 var config = {44 var config = {
45 on: {45 on: {
46 success: function () {46 success: function() {
47 Y.fire('inlinecomment.UPDATED');47 Y.fire('inlinecomment.UPDATED');
48 }48 }
49 },49 },
@@ -89,10 +89,10 @@
89 });89 });
90 widget.editor.on('keydown', function(e) {90 widget.editor.on('keydown', function(e) {
91 var enterKeyCode = 13;91 var enterKeyCode = 13;
92 if (e.domEvent.ctrlKey === true && 92 if (e.domEvent.ctrlKey === true &&
93 e.domEvent.button === enterKeyCode) {93 e.domEvent.button === enterKeyCode) {
94 this.save();94 this.save();
95 } 95 }
96 });96 });
97 widget.editor.on('cancel', function(e) {97 widget.editor.on('cancel', function(e) {
98 handling_request = false;98 handling_request = false;
@@ -111,7 +111,6 @@
111 .append('<div class="editorShortcutTip">' +111 .append('<div class="editorShortcutTip">' +
112 '[Esc] Cancel, [Ctrl+Enter] Save' +112 '[Esc] Cancel, [Ctrl+Enter] Save' +
113 '</div>');113 '</div>');
114
115 };114 };
116 var delete_handler = function(e) {115 var delete_handler = function(e) {
117 var line_number = namespace.find_line_number(e.currentTarget);116 var line_number = namespace.find_line_number(e.currentTarget);
@@ -143,11 +142,14 @@
143142
144namespace.create_row = function(comment) {143namespace.create_row = function(comment) {
145 // Find or create the container for this line's comments.144 // Find or create the container for this line's comments.
146 var comments_tr = Y.one('#comments-diff-line-' + comment.line_number);145 var comments_tr = Y.one('#comments-diff-line-' +
146 comment.line_number),
147 comment_date;
148
147 if (comments_tr === null) {149 if (comments_tr === null) {
148 var comments_tr = Y.Node.create(150 comments_tr = Y.Node.create(
149 '<tr class="inline-comments">'151 '<tr class="inline-comments">' +
150 + '<td colspan="2"><div></div></td></tr>')152 '<td colspan="2"><div></div></td></tr>')
151 .set('id', 'comments-diff-line-' + comment.line_number);153 .set('id', 'comments-diff-line-' + comment.line_number);
152 Y.one('#diff-line-' + comment.line_number)154 Y.one('#diff-line-' + comment.line_number)
153 .insert(comments_tr, 'after');155 .insert(comments_tr, 'after');
@@ -181,7 +183,7 @@
181 // Wrap resources coming from LP.cache, as they come from183 // Wrap resources coming from LP.cache, as they come from
182 // the LP API (updates during the page life-span). This way184 // the LP API (updates during the page life-span). This way
183 // the handling code can be unified.185 // the handling code can be unified.
184 if (LP.links['me'] !== undefined) {186 if (LP.links.me !== undefined) {
185 newrow.append('<div class="boardCommentFooter"></div>');187 newrow.append('<div class="boardCommentFooter"></div>');
186 newrow.one('.boardCommentFooter')188 newrow.one('.boardCommentFooter')
187 .append('<a class="js-action draft-edit">Reply</a>');189 .append('<a class="js-action draft-edit">Reply</a>');
@@ -198,10 +200,10 @@
198 // XXX cprov 20140226: the date should be formatted according to200 // XXX cprov 20140226: the date should be formatted according to
199 // the user locale/timezone similar to fmt:displaydate.201 // the user locale/timezone similar to fmt:displaydate.
200 // We should leverage Y.Intl features.202 // We should leverage Y.Intl features.
201 if (typeof comment.date === "string") {203 if (typeof comment.date === 'string') {
202 var comment_date = Y.lp.app.date.parse_date(comment.date);204 comment_date = Y.lp.app.date.parse_date(comment.date);
203 } else {205 } else {
204 var comment_date = comment.date;206 comment_date = comment.date;
205 }207 }
206 var date = Y.lp.app.date.approximatedate(comment_date);208 var date = Y.lp.app.date.approximatedate(comment_date);
207 headerspan.one('span').set('text', date);209 headerspan.one('span').set('text', date);
@@ -228,11 +230,11 @@
228namespace.populate_comments = function() {230namespace.populate_comments = function() {
229 var config_published = {231 var config_published = {
230 on: {232 on: {
231 success: function (comments) {233 success: function(comments) {
232 // Display published inline comments.234 // Display published inline comments.
233 // [{'line_number': <lineno>, 'person': <IPerson>,235 // [{'line_number': <lineno>, 'person': <IPerson>,
234 // 'text': <comment>, 'date': <timestamp>}, ...]236 // 'text': <comment>, 'date': <timestamp>}, ...]
235 comments.forEach( function (comment) {237 comments.forEach(function(comment) {
236 namespace.create_row(comment);238 namespace.create_row(comment);
237 });239 });
238 Y.fire('inlinecomment.UPDATED');240 Y.fire('inlinecomment.UPDATED');
@@ -249,14 +251,14 @@
249namespace.populate_drafts = function() {251namespace.populate_drafts = function() {
250 var config_draft = {252 var config_draft = {
251 on: {253 on: {
252 success: function (drafts) {254 success: function(drafts) {
253 namespace.inlinecomments = {};255 namespace.inlinecomments = {};
254 if (drafts === null) {256 if (drafts === null) {
255 return;257 return;
256 }258 }
257 // Display draft inline comments:259 // Display draft inline comments:
258 // {'<line_number>':''<comment>', ...})260 // {'<line_number>':''<comment>', ...})
259 Object.keys(drafts).forEach( function (line_number) {261 Object.keys(drafts).forEach(function(line_number) {
260 var comment = {262 var comment = {
261 'line_number': line_number,263 'line_number': line_number,
262 'text': drafts[line_number]264 'text': drafts[line_number]
@@ -284,7 +286,7 @@
284 // loaded for anonymous requests.286 // loaded for anonymous requests.
285 // In fact, if loading draft is attempted is will fail due to287 // In fact, if loading draft is attempted is will fail due to
286 // the LP permission checks.288 // the LP permission checks.
287 if (LP.links['me'] !== undefined) {289 if (LP.links.me !== undefined) {
288 // Add the double-click handler for each row in the diff. This needs290 // Add the double-click handler for each row in the diff. This needs
289 // to be done first since populating existing published and draft291 // to be done first since populating existing published and draft
290 // comments may add more rows.292 // comments may add more rows.
@@ -295,7 +297,7 @@
295};297};
296298
297299
298var PublishDrafts = function () {300var PublishDrafts = function() {
299 PublishDrafts.superclass.constructor.apply(this, arguments);301 PublishDrafts.superclass.constructor.apply(this, arguments);
300};302};
301303
@@ -320,29 +322,31 @@
320 * @method syncUI322 * @method syncUI
321 */323 */
322 syncUI: function() {324 syncUI: function() {
325 var n_drafts = Y.all('.draft').size(),
326 rc_scroller = Y.one('.review-comment-scroller'),
327 scroller_text = '';
328
323 this.get('contentBox').empty();329 this.get('contentBox').empty();
324 var n_drafts = Y.all('.draft').size();
325 var rc_scroller = Y.one('.review-comment-scroller');
326 if (rc_scroller !== null) {330 if (rc_scroller !== null) {
327 if (n_drafts > 0) {331 if (n_drafts > 0) {
328 var scroller_text = 'Return to save comment'332 scroller_text = 'Return to save comment';
329 if (n_drafts > 1) scroller_text += 's';333 if (n_drafts > 1) { scroller_text += 's'; }
330 rc_scroller.set('text', scroller_text);334 rc_scroller.set('text', scroller_text);
331 } else {335 } else {
332 rc_scroller.set('text', 'Return to add comment');336 rc_scroller.set('text', 'Return to add comment');
333 }337 }
334 }338 }
335 if (n_drafts == 0) {339 if (n_drafts === 0) {
336 Y.fire('CodeReviewComment.SET_DISABLED', true);340 Y.fire('CodeReviewComment.SET_DISABLED', true);
337 return;341 return;
338 }342 }
339 var text = ' Include ' + n_drafts + ' diff comment';343 var text = ' Include ' + n_drafts + ' diff comment';
340 if (n_drafts > 1) text += 's';344 if (n_drafts > 1) { text += 's'; }
341 var checkbox = Y.Node.create(345 var checkbox = Y.Node.create(
342 '<input id="field.publish_inline_comments"' +346 '<input id="field.publish_inline_comments"' +
343 'name="field.publish_inline_comments"' +347 'name="field.publish_inline_comments"' +
344 'type="checkbox" class="checkboxType"' +348 'type="checkbox" class="checkboxType"' +
345 'checked=""></input>')349 'checked=""></input>');
346 var label = Y.Node.create(350 var label = Y.Node.create(
347 '<label for="field.publish_inline_comments" />')351 '<label for="field.publish_inline_comments" />')
348 .set('text', text);352 .set('text', text);
@@ -367,7 +371,7 @@
367namespace.PublishDrafts = PublishDrafts;371namespace.PublishDrafts = PublishDrafts;
368372
369373
370var InlineCommentToggle = function () {374var InlineCommentToggle = function() {
371 InlineCommentToggle.superclass.constructor.apply(this, arguments);375 InlineCommentToggle.superclass.constructor.apply(this, arguments);
372};376};
373377
@@ -408,7 +412,7 @@
408 if (display) {412 if (display) {
409 css_display = 'table-row';413 css_display = 'table-row';
410 }414 }
411 Y.all('.inline-comments').each( function() {415 Y.all('.inline-comments').each(function() {
412 this.setStyle('display', css_display);416 this.setStyle('display', css_display);
413 this.next().setStyle('display', css_display);417 this.next().setStyle('display', css_display);
414 });418 });
@@ -469,12 +473,12 @@
469 * @method initializer473 * @method initializer
470 * @protected474 * @protected
471 */475 */
472 initializer: function(cfg){476 initializer: function(cfg) {
473 // If we have not been provided with a Launchpad Client, then477 // If we have not been provided with a Launchpad Client, then
474 // create one now:478 // create one now:
475 if (null === this.get("lp_client")){479 if (null === this.get('lp_client')) {
476 // Create our own instance of the LP client.480 // Create our own instance of the LP client.
477 this.set("lp_client", new Y.lp.client.Launchpad());481 this.set('lp_client', new Y.lp.client.Launchpad());
478 }482 }
479 },483 },
480484
@@ -534,7 +538,7 @@
534 _connectScrollers: function() {538 _connectScrollers: function() {
535 var self = this;539 var self = this;
536 var rc = Y.lp.code.branchmergeproposal.reviewcomment;540 var rc = Y.lp.code.branchmergeproposal.reviewcomment;
537 Y.all('td[data-previewdiff-id]').each( function(td) {541 Y.all('td[data-previewdiff-id]').each(function(td) {
538 // Comments from superseded MPs should be ignored.542 // Comments from superseded MPs should be ignored.
539 if (td.getData('from-superseded') === 'True') {543 if (td.getData('from-superseded') === 'True') {
540 return;544 return;
@@ -561,7 +565,7 @@
561 return;565 return;
562 }566 }
563 rc_scroller = Y.Node.create('<a href="">Return to add comment</a>')567 rc_scroller = Y.Node.create('<a href="">Return to add comment</a>')
564 .addClass('review-comment-scroller')568 .addClass('review-comment-scroller');
565 this.get('srcNode').append(rc_scroller);569 this.get('srcNode').append(rc_scroller);
566 rc.link_scroller(rc_scroller, '#add-comment-form', function() {570 rc.link_scroller(rc_scroller, '#add-comment-form', function() {
567 if (Y.all('.draft').size() > 0) {571 if (Y.all('.draft').size() > 0) {
@@ -583,7 +587,7 @@
583 var container = this.get('srcNode').one('.diff-content');587 var container = this.get('srcNode').one('.diff-content');
584 var config = {588 var config = {
585 on: {589 on: {
586 success: function (diff_html) {590 success: function(diff_html) {
587 container.set('innerHTML', diff_html);591 container.set('innerHTML', diff_html);
588 self.set_status_updated();592 self.set_status_updated();
589 },593 },
@@ -642,7 +646,7 @@
642646
643 var pub_drafts_anchor = Y.one('[id="field.review_type"]');647 var pub_drafts_anchor = Y.one('[id="field.review_type"]');
644 if (pub_drafts_anchor !== null) {648 if (pub_drafts_anchor !== null) {
645 var pub_drafts_container = Y.one('.publish_drafts_container')649 var pub_drafts_container = Y.one('.publish_drafts_container');
646 if (pub_drafts_container === null) {650 if (pub_drafts_container === null) {
647 pub_drafts_container = Y.Node.create(651 pub_drafts_container = Y.Node.create(
648 '<div class="publish_drafts_container">');652 '<div class="publish_drafts_container">');