launchpad-reviewers team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

Celso Providelo has proposed merging lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cprov/launchpad/ic-ui-prettify/+merge/214872

Styling diff inline-comments.

Also minor JS bug-fixing + renaming. 
-- 
https://code.launchpad.net/~cprov/launchpad/ic-ui-prettify/+merge/214872
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cprov/launchpad/ic-ui-prettify into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/style.css'
--- lib/canonical/launchpad/icing/style.css	2013-09-05 03:57:28 +0000
+++ lib/canonical/launchpad/icing/style.css	2014-04-09 04:05:39 +0000
@@ -628,8 +628,40 @@
   margin: 0 0 0 0;
 }
 
-tr.ict-header {
-    background-color: #EEEEEE;
+/* === Branch Merge Proposal Inline Comments === */
+
+tr.ict-header h4 {
+    font-weight: bold;
+}
+
+tr.ict-header td.text {
+    background-color: #AACCEE;
+    margin: 4px;
+    padding-left: 4px;
+}
+
+tr.ict-content td.text {
+    background-color: #AACCEE;
+    margin: 4px;
+    padding-left: 4px;
+}
+
+tr.ict-content div.inline-content {
+    margin: 4px;
+    width: 95% !important;
+}
+
+tr.ict-content .yui3-ieditor {
+    padding-right: 0px;
+    width: 100% !important
+}
+
+tr.ict-content .yui3-ieditor-multiline .yui3-ieditor-btns .bg-top-label {
+    background-image: none;
+}
+
+tr.ict-content .yui3-ieditor-input textarea {
+    border: 1px solid;
 }
 
 /* === Bugs === */

=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-04-04 05:23:48 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-04-09 04:05:39 +0000
@@ -50,6 +50,7 @@
                 handling_request = false;
                 if (namespace.inlinecomments[line_number] === undefined) {
                     header_row.remove(true);
+                    content_row.remove(true);
                 }
                 return;
             }
@@ -84,27 +85,32 @@
 namespace.create_row = function(comment) {
     var ident = comment.line_number + '-draft';
     var headerrow = Y.Node.create(
-        '<tr><td colspan="2"></td></tr>').addClass('ict-header');
-    var headerspan;
-    var newrow;
+        '<tr><td class="line-no"></td><td class="text"></td></tr>')
+        .addClass('ict-header');
+    var headercontent;
+    var commentrow;
     if (Y.Lang.isUndefined(comment.person)) {
         // Creates a draft inline comment area.
-        headerspan = Y.Node.create('<span></span>').set(
-            'text', 'Draft comment.');
+        headercontent = Y.Node.create('<h4>Draft comment</h4>');
         headerrow.set('id', 'ict-' + ident + '-header');
-        newrow = Y.Node.create('<tr><td></td><td><div>' +
+        commentrow = Y.Node.create(
+            '<tr class="ict-content"><td class="line-no"></td>'+
+            '<td class="text">' +
+            '<div class="inline-content">' + 
+            '<div class="yui3-editable_text-trigger"></div>' +
             '<span class="yui3-editable_text-text"></span>' +
-            '<div class="yui3-editable_text-trigger"></div>' +
-            '</div></td></tr>').set('id', 'ict-' + ident);
-        newrow.one('td>div').set('id', 'inlinecomment-' + ident);
+            '</div></td></tr>')
+            .set('id', 'ict-' + ident);
+        commentrow.one('td>div').set('id', 'inlinecomment-' + ident);
         if (!Y.Lang.isUndefined(comment.text)) {
             namespace.inlinecomments[comment.line_number] = comment.text;
-            newrow.one('span').set('text', comment.text);
+            commentrow.one('span').set('text', comment.text);
         }
+        
     } else {
         // Creates a published inline comment area.
-        headerspan = Y.Node.create(
-            '<span>Comment by <a></a> <span></span></span>');
+        headercontent = Y.Node.create(
+            '<h4>Comment by <a></a> <span></span></h4>');
         // Wrap resources coming from LP.cache, as they come from
         // the LP API (updates during the page life-span). This way
         // the handling code can be unified.
@@ -115,18 +121,25 @@
         var header_content = (
             comment.person.get('display_name') +
             ' (' + comment.person.get('name') + ')');
-        headerspan.one('a').set('href', comment.person.get('web_link')).set(
-            'text', header_content);
+        var author_link = headercontent.one('a')
+            .set('href', comment.person.get('web_link'))
+            .set('text', header_content);
         // XXX cprov 20140226: the date should be formatted according to
         // the user locale/timezone similar to fmt:displaydate.
         // We should leverage Y.Intl features.
         var fmt_date = 'on ' + Y.Date.format(
             new Date(comment.date), {format: '%Y-%m-%d'});
-        headerspan.one('span').set('text', fmt_date);
-        newrow = Y.Node.create('<tr><td></td><td><span></span></td></tr>');
-        newrow.one('span').set('text', comment.text);
+        headercontent.one('span').set('text', fmt_date);
+        commentrow = Y.Node.create(
+            '<tr><td class="line-no"></td><td class="text">' +
+            '<div class="inline-content">' + 
+            '<span></span>' +
+            '</div>' +
+            '</td></tr>')
+            .addClass('ict-content');
+        commentrow.one('span').set('text', comment.text);
     }
-    headerrow.one('td').appendChild(headerspan);
+    headerrow.one('td.text').appendChild(headercontent);
 
     // We want to have the comments in order after the line.
     var tr = Y.one('#diff-line-' + (parseInt(comment.line_number, 10) + 1));
@@ -137,7 +150,7 @@
         tr = Y.one('.diff>tbody>tr:last-child');
         tr.insert(headerrow, 'after');
     }
-    headerrow.insert(newrow, 'after');
+    headerrow.insert(commentrow, 'after');
 
     return headerrow;
 };

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-04-04 05:23:48 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-04-09 04:05:39 +0000
@@ -90,7 +90,7 @@
 
             // Draft comment is displayed.
             header = Y.one('#diff-line-3').next();
-            Y.Assert.areEqual('Draft comment.', header.get('text'));
+            Y.Assert.areEqual('Draft comment', header.get('text'));
             text = header.next();
             Y.Assert.areEqual('Zoing!', text.get('text'));
         },
@@ -117,7 +117,7 @@
             // LP was hit and a comment was created.
             Y.Assert.areEqual(1, mockio.requests.length);
             Y.Assert.areEqual(
-                'Draft comment.', Y.one('#ict-1-draft-header').get('text'));
+                'Draft comment', Y.one('#ict-1-draft-header').get('text'));
             Y.Assert.areEqual(
                 'Go!', ic_area.one('.yui3-editable_text-text').get('text'));
 
@@ -145,6 +145,18 @@
             Y.Assert.areEqual('diff-line-2', line.next().get('id'));
         },
 
+        test_edit_cancelling: function() {
+            // Cancelling a inline comment attempt on a row with
+            // no comments does not leave an empty row behind.
+            module.add_doubleclick_handler();
+            var line  = Y.one('#diff-line-1');
+            line.simulate('dblclick');
+            ic_area = line.next().next();
+            ic_area.one('.yui3-ieditor-input>textarea').set('value', '');
+            ic_area.one('.lazr-neg').simulate('click');
+            Y.Assert.areEqual('diff-line-2', line.next().get('id'));
+        },
+
         test_feature_flag_off: function() {
             var called = false;
             add_doubleclick_handler = function() {