launchpad-reviewers team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

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

Requested reviews:
  William Grant (wgrant): code

For more details, see:
https://code.launchpad.net/~cprov/launchpad/ic-view/+merge/218540

Recover Chris CSS changes from inline comments (conflict fixing) and also suppress inline-comment show/hide checkbox rendering when the feature flag is disabled.
-- 
https://code.launchpad.net/~cprov/launchpad/ic-view/+merge/218540
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

=== modified file 'lib/lp/app/javascript/inlineedit/assets/skins/sam/editor-skin.css'
--- lib/lp/app/javascript/inlineedit/assets/skins/sam/editor-skin.css	2012-08-23 19:56:15 +0000
+++ lib/lp/app/javascript/inlineedit/assets/skins/sam/editor-skin.css	2014-05-07 03:05:13 +0000
@@ -204,3 +204,52 @@
   background: url('edit.png') top left no-repeat;
   padding: 2px 0 0 18px;
 }
+
+tr.ict-header {
+  margin: 20px 0 0 0;
+}
+td.ic-header {
+  border: 1px solid rgb(221, 221, 221);
+  border-right: 0;
+  padding: .5em 12px;
+  font-family: Ubuntu,"Bitstream Vera Sans","DejaVu Sans",Tahoma,sans-serif;
+  font-size: 12px;
+}
+td.inline-comment {
+  border: 1px solid rgb(221, 221, 221);
+  border-right: 0;
+  margin: 0 0 20px 0;
+  line-height: 25px;
+  background-color: rgb(248, 248, 255);
+  font-family: Ubuntu,"Bitstream Vera Sans","DejaVu Sans",Tahoma,sans-serif;
+  font-size: 12px;
+  padding: 0 12px 0;
+  word-break: break-word;
+}
+
+.inline-comment>div { line-height: 25px; }
+.inline-comment .yui3-ieditor { padding-right:0 !important }
+.inline-comment .yui3-ieditor > yui3-ieditor-input { width: auto!important }
+.inline-comment .yui3-ieditor-input {
+  width: auto!important;
+  margin-right: 0;
+  margin-bottom: 5px;
+  margin-top: -6px!important;
+  overflow: hidden;
+}
+.inline-comment .yui3-ieditor-content {top: 5px; left: -5px;}
+.inline-comment .yui3-ieditor-input textarea {
+  width: 92%;
+  max-width: 92%;
+  border-top: 1px solid rgb(214, 214, 214);
+  float: left;
+}
+.inline-comment .yui3-ieditor-input .bg-top-label { margin-right: 10px; }
+.inline-comment .bg-top-label {
+  margin-right: -7px!important;
+  background-image: none!important;
+  margin-top: 18px!important;
+}
+tr.ict-content .yui3-ieditor-multiline .yui3-ieditor-btns .bg-top-label button.lazr-btn { text-indent: -9999px; }
+.inline-comment .bg-top-label button.lazr-btn { text-indent: -9999px; }
+.diff-content .boardCommentBody { padding: 0!important }

=== modified file 'lib/lp/code/javascript/branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-04-12 00:46:58 +0000
+++ lib/lp/code/javascript/branchmergeproposal.inlinecomments.js	2014-05-07 03:05:13 +0000
@@ -50,6 +50,7 @@
                 handling_request = false;
                 if (namespace.inlinecomments[line_number] === undefined) {
                     header_row.remove(true);
+                    content_row.remove(true);
                 }
                 return;
             }
@@ -84,7 +85,8 @@
 namespace.create_row = function(comment) {
     var ident = comment.line_number + '-draft';
     var headerrow = Y.Node.create(
-        '<tr><td class="line-no"></td><td></td></tr>').addClass('ict-header');
+        '<tr><td class="line-no"></td><td class="ic-header"></td></tr>')
+        .addClass('ict-header');
     var headerspan;
     var newrow;
     if (Y.Lang.isUndefined(comment.person)) {
@@ -92,7 +94,9 @@
         headerspan = Y.Node.create('<span></span>').set(
             'text', 'Draft comment');
         headerrow.set('id', 'ict-' + ident + '-header');
-        newrow = Y.Node.create('<tr><td class="line-no"></td><td><div>' +
+        newrow = Y.Node.create(
+            '<tr><td class="line-no"></td>' +
+            '<td class="inline-comment"><div>' +
             '<span class="yui3-editable_text-text"></span>' +
             '<div class="yui3-editable_text-trigger"></div>' +
             '</div></td></tr>').set('id', 'ict-' + ident);
@@ -125,7 +129,8 @@
             new Date(comment.date), {format: '%Y-%m-%d'});
         headerspan.one('span').set('text', fmt_date);
         newrow = Y.Node.create(
-            '<tr><td class="line-no"></td><td><span></span></td></tr>');
+            '<tr><td class="line-no"></td>' +
+            '<td class="inline-comment"><span></span></td></tr>');
         newrow.one('span').set('text', comment.text);
     }
     headerrow.one('td:last-child').appendChild(headerspan);
@@ -234,6 +239,9 @@
 Y.extend(InlineCommentToggle, Y.Widget, {
 
     renderUI: function() {
+        if (LP.cache.inline_diff_comments !== true) {
+            return;
+        }
         var ui = Y.Node.create('<li><label>' +
             '<input type="checkbox" checked="checked" id="show-ic"/>' +
             '&nbsp;Show inline comments</label></li>');

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-04-14 15:08:49 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.inlinecomments.js	2014-05-07 03:05:13 +0000
@@ -152,6 +152,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() {
@@ -254,8 +266,9 @@
         test_diff_nav_feature_flag_disabled: function() {
             // When rendered with the corresponding feature-flag disabled,
             // Diff Navigator only updates the diff content with the latest
-            // previewdiff and does not create the navigator (<select>) nor
-            // fetches the inline comments.
+            // previewdiff and does not create the navigator (<select>),
+            // does not fetch the inline comments nor render the
+            // InlineToggle (show/hide inline comments) widget.
             LP.cache.inline_diff_comments = false;
             this.diffnav.render();
             mockio = this.diffnav.get('lp_client').io_provider;
@@ -265,6 +278,7 @@
                 mockio.last_request.url);
             this.reply_diffcontent();
             Y.Assert.isNull(this.diffnav.get('previewdiff_id'));
+            Y.Assert.isNull(Y.one('.diff-content').one('#show-ic'));
         },
 
         test_diff_nav_rendering: function() {