ux: add hidden comments indicators plus style tweaks

* add hidden comments indicator in diff lines
* make no new line at end of file marker red
* fixes bug where show/hide comments was not changing message

refs  #4311

rhodecode/public/css/code-block.less

@@ -877,11 +877,17 b' input.filediff-collapse-state {'
             .show-comment-button {
                 display: inline;
             }
-            .show-comment-button {
+            .hide-comment-button {
                 display: none;
             }
         }
     }
+
+    .hide-line-comments {
+        .inline-comments {
+            display: none;
+        }
+    }
     .inline-comments {
         border-radius: @border-radius;
         background: @grey6;
@@ -1037,6 +1043,9 b' table.cb {'
                 white-space: pre-wrap;
                 font-family: @font-family-monospace;
                 word-break: break-word;
+                .nonl {
+                    color: @color5;
+                }
             }
 
             &> button.cb-comment-box-opener {
@@ -1061,6 +1070,22 b' table.cb {'
             }
         }
 
+        &.cb-data {
+            text-align: right;
+            width: 30px;
+            font-family: @font-family-monospace;
+
+            .icon-comment {
+                cursor: pointer;
+            }
+            &.cb-line-selected > div {
+                display: block;
+                background: @comment-highlight-color !important;
+                line-height: @cb-line-height;
+                color: rgba(0, 0, 0, 0.3);
+            }
+        }
+
         &.cb-lineno {
             padding: 0;
             width: 50px;

rhodecode/public/js/src/rhodecode.js

@@ -292,6 +292,7 b' function scrollToElement(element, percen'
                 $('.cb-line-selected').removeClass('cb-line-selected');
                 var td = $(this).parent();
                 td.addClass('cb-line-selected'); // line number td
+                td.prev().addClass('cb-line-selected'); // line data td
                 td.next().addClass('cb-line-selected'); // line content td
 
                 // Replace URL without jumping to it if browser supports.
@@ -475,6 +476,7 b' function scrollToElement(element, percen'
             }
             $.each(highlightable_line_tds, function (i, $td) {
                 $td.addClass('cb-line-selected'); // line number td
+                $td.prev().addClass('cb-line-selected'); // line data
                 $td.next().addClass('cb-line-selected'); // line content
             });
 

rhodecode/public/js/src/rhodecode/comments.js

@@ -670,3 +670,228 b' var CommentForm = (function() {'
 
     return CommentForm;
 })();
+
+var CommentsController = function() { /* comments controller */
+  var self = this;
+
+  this.cancelComment = function(node) {
+      var $node = $(node);
+      var $td = $node.closest('td');
+      $node.closest('.comment-inline-form').removeClass('comment-inline-form-open');
+      return false;
+  }
+  this.getLineNumber = function(node) {
+      var $node = $(node);
+      return $node.closest('td').attr('data-line-number');
+  }
+  this.scrollToComment = function(node, offset) {
+    if (!node) {
+      node = $('.comment-selected');
+      if (!node.length) {
+        node = $('comment-current')
+      }
+    }
+    $comment = $(node).closest('.comment-current');
+    $comments = $('.comment-current');
+
+    $('.comment-selected').removeClass('comment-selected');
+
+    var nextIdx = $('.comment-current').index($comment) + offset;
+    if (nextIdx >= $comments.length) {
+      nextIdx = 0;
+    }
+    var $next = $('.comment-current').eq(nextIdx);
+    var $cb = $next.closest('.cb');
+    $cb.removeClass('cb-collapsed')
+
+    var $filediffCollapseState = $cb.closest('.filediff').prev();
+    $filediffCollapseState.prop('checked', false);
+    $next.addClass('comment-selected');
+    scrollToElement($next);
+    return false;
+  }
+  this.nextComment = function(node) {
+    return self.scrollToComment(node, 1);
+  }
+  this.prevComment = function(node) {
+    return self.scrollToComment(node, -1);
+  }
+  this.deleteComment = function(node) {
+      if (!confirm(_gettext('Delete this comment?'))) {
+        return false;
+      }
+      var $node = $(node);
+      var $td = $node.closest('td');
+      var $comment = $node.closest('.comment');
+      var comment_id = $comment.attr('data-comment-id');
+      var url = AJAX_COMMENT_DELETE_URL.replace('__COMMENT_ID__', comment_id);
+      var postData = {
+        '_method': 'delete',
+        'csrf_token': CSRF_TOKEN
+      };
+
+      $comment.addClass('comment-deleting');
+      $comment.hide('fast');
+
+      var success = function(response) {
+        $comment.remove();
+        return false;
+      };
+      var failure = function(data, textStatus, xhr) {
+        alert("error processing request: " + textStatus);
+        $comment.show('fast');
+        $comment.removeClass('comment-deleting');
+        return false;
+      };
+      ajaxPOST(url, postData, success, failure);
+  }
+  this.toggleComments = function(node, show) {
+    var $filediff = $(node).closest('.filediff');
+    if (show === true) {
+      $filediff.removeClass('hide-comments');
+    } else if (show === false) {
+      $filediff.find('.hide-line-comments').removeClass('hide-line-comments');
+      $filediff.addClass('hide-comments');
+    } else {
+      $filediff.find('.hide-line-comments').removeClass('hide-line-comments');
+      $filediff.toggleClass('hide-comments');
+    }
+    return false;
+  }
+  this.toggleLineComments = function(node) {
+    self.toggleComments(node, true);
+    var $node = $(node);
+    $node.closest('tr').toggleClass('hide-line-comments');
+  }
+  this.createComment = function(node) {
+      var $node = $(node);
+      var $td = $node.closest('td');
+      var $form = $td.find('.comment-inline-form');
+
+      if (!$form.length) {
+          var tmpl = $('#cb-comment-inline-form-template').html();
+          var $filediff = $node.closest('.filediff');
+          $filediff.removeClass('hide-comments');
+          var f_path = $filediff.attr('data-f-path');
+          var lineno = self.getLineNumber(node);
+          tmpl = tmpl.format(f_path, lineno);
+          $form = $(tmpl);
+
+          var $comments = $td.find('.inline-comments');
+          if (!$comments.length) {
+            $comments = $(
+              $('#cb-comments-inline-container-template').html());
+            $td.append($comments);
+          }
+
+          $td.find('.cb-comment-add-button').before($form);
+
+          var pullRequestId = templateContext.pull_request_data.pull_request_id;
+          var commitId = templateContext.commit_data.commit_id;
+          var _form = $form[0];
+          var commentForm = new CommentForm(_form, commitId, pullRequestId, lineno, false);
+          var cm = commentForm.getCmInstance();
+
+          // set a CUSTOM submit handler for inline comments.
+          commentForm.setHandleFormSubmit(function(o) {
+            var text = commentForm.cm.getValue();
+
+            if (text === "") {
+              return;
+            }
+
+            if (lineno === undefined) {
+              alert('missing line !');
+              return;
+            }
+            if (f_path === undefined) {
+              alert('missing file path !');
+              return;
+            }
+
+            var excludeCancelBtn = false;
+            var submitEvent = true;
+            commentForm.setActionButtonsDisabled(true, excludeCancelBtn, submitEvent);
+            commentForm.cm.setOption("readOnly", true);
+            var postData = {
+                'text': text,
+                'f_path': f_path,
+                'line': lineno,
+                'csrf_token': CSRF_TOKEN
+            };
+            var submitSuccessCallback = function(json_data) {
+              $form.remove();
+              console.log(json_data)
+              try {
+                var html = json_data.rendered_text;
+                var lineno = json_data.line_no;
+                var target_id = json_data.target_id;
+
+                $comments.find('.cb-comment-add-button').before(html);
+                console.log(lineno, target_id, $comments);
+
+              } catch (e) {
+                console.error(e);
+              }
+
+
+              // re trigger the linkification of next/prev navigation
+              linkifyComments($('.inline-comment-injected'));
+              timeagoActivate();
+              bindDeleteCommentButtons();
+              commentForm.setActionButtonsDisabled(false);
+
+            };
+            var submitFailCallback = function(){
+                commentForm.resetCommentFormState(text)
+            };
+            commentForm.submitAjaxPOST(
+                commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback);
+          });
+
+          setTimeout(function() {
+              // callbacks
+              if (cm !== undefined) {
+                  cm.focus();
+              }
+          }, 10);
+
+            $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({
+                form: _form,
+                parent: $td[0],
+                lineno: lineno,
+                f_path: f_path}
+            );
+      }
+
+      $form.addClass('comment-inline-form-open');
+  }
+
+  this.renderInlineComments = function(file_comments) {
+    show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
+
+    for (var i = 0; i < file_comments.length; i++) {
+      var box = file_comments[i];
+
+      var target_id = $(box).attr('target_id');
+
+      // actually comments with line numbers
+      var comments = box.children;
+
+      for (var j = 0; j < comments.length; j++) {
+        var data = {
+          'rendered_text': comments[j].outerHTML,
+          'line_no': $(comments[j]).attr('line'),
+          'target_id': target_id
+        };
+      }
+    }
+
+    // since order of injection is random, we're now re-iterating
+    // from correct order and filling in links
+    linkifyComments($('.inline-comment-injected'));
+    bindDeleteCommentButtons();
+    firefoxAnchorFix();
+  };
+
+}
  No newline at end of file

rhodecode/templates/base/root.html

@@ -115,217 +115,6 b" c.template_context['visual']['default_re"
               }
             };
 
-
-Rhodecode = (function() {
-    function _Rhodecode() {
-        this.comments = new (function() { /* comments controller */
-            var self = this;
-
-            this.cancelComment = function(node) {
-                var $node = $(node);
-                var $td = $node.closest('td');
-                $node.closest('.comment-inline-form').removeClass('comment-inline-form-open');
-                return false;
-            }
-            this.getLineNumber = function(node) {
-                var $node = $(node);
-                return $node.closest('td').attr('data-line-number');
-            }
-            this.scrollToComment = function(node, offset) {
-              if (!node) {
-                node = $('.comment-selected');
-                if (!node.length) {
-                  node = $('comment-current')
-                }
-              }
-              $comment = $(node).closest('.comment-current');
-              $comments = $('.comment-current');
-
-              $('.comment-selected').removeClass('comment-selected');
-
-              var nextIdx = $('.comment-current').index($comment) + offset;
-              if (nextIdx >= $comments.length) {
-                nextIdx = 0;
-              }
-              var $next = $('.comment-current').eq(nextIdx);
-              var $cb = $next.closest('.cb');
-              $cb.removeClass('cb-collapsed')
-
-              var $filediffCollapseState = $cb.closest('.filediff').prev();
-              $filediffCollapseState.prop('checked', false);
-              $next.addClass('comment-selected');
-              scrollToElement($next);
-              return false;
-            }
-            this.nextComment = function(node) {
-              return self.scrollToComment(node, 1);
-            }
-            this.prevComment = function(node) {
-              return self.scrollToComment(node, -1);
-            }
-            this.deleteComment = function(node) {
-                if (!confirm(_gettext('Delete this comment?'))) {
-                  return false;
-                }
-                var $node = $(node);
-                var $td = $node.closest('td');
-                var $comment = $node.closest('.comment');
-                var comment_id = $comment.attr('data-comment-id');
-                var url = AJAX_COMMENT_DELETE_URL.replace('__COMMENT_ID__', comment_id);
-                var postData = {
-                  '_method': 'delete',
-                  'csrf_token': CSRF_TOKEN
-                };
-
-                $comment.addClass('comment-deleting');
-                $comment.hide('fast');
-
-                var success = function(response) {
-                  $comment.remove();
-                  return false;
-                };
-                var failure = function(data, textStatus, xhr) {
-                  alert("error processing request: " + textStatus);
-                  $comment.show('fast');
-                  $comment.removeClass('comment-deleting');
-                  return false;
-                };
-                ajaxPOST(url, postData, success, failure);
-            }
-            this.createComment = function(node) {
-                var $node = $(node);
-                var $td = $node.closest('td');
-                var $form = $td.find('.comment-inline-form');
-
-                if (!$form.length) {
-                    var tmpl = $('#cb-comment-inline-form-template').html();
-                    var f_path = $node.closest('.filediff').attr('data-f-path');
-                    var lineno = self.getLineNumber(node);
-                    tmpl = tmpl.format(f_path, lineno);
-                    $form = $(tmpl);
-
-                    var $comments = $td.find('.inline-comments');
-                    if (!$comments.length) {
-                      $comments = $(
-                        $('#cb-comments-inline-container-template').html());
-                      $td.append($comments);
-                    }
-
-                    $td.find('.cb-comment-add-button').before($form);
-
-                    var pullRequestId = templateContext.pull_request_data.pull_request_id;
-                    var commitId = templateContext.commit_data.commit_id;
-                    var _form = $form[0];
-                    var commentForm = new CommentForm(_form, commitId, pullRequestId, lineno, false);
-                    var cm = commentForm.getCmInstance();
-
-                    // set a CUSTOM submit handler for inline comments.
-                    commentForm.setHandleFormSubmit(function(o) {
-                      var text = commentForm.cm.getValue();
-
-                      if (text === "") {
-                        return;
-                      }
-
-                      if (lineno === undefined) {
-                        alert('missing line !');
-                        return;
-                      }
-                      if (f_path === undefined) {
-                        alert('missing file path !');
-                        return;
-                      }
-
-                      var excludeCancelBtn = false;
-                      var submitEvent = true;
-                      commentForm.setActionButtonsDisabled(true, excludeCancelBtn, submitEvent);
-                      commentForm.cm.setOption("readOnly", true);
-                      var postData = {
-                          'text': text,
-                          'f_path': f_path,
-                          'line': lineno,
-                          'csrf_token': CSRF_TOKEN
-                      };
-                      var submitSuccessCallback = function(json_data) {
-                        $form.remove();
-                        console.log(json_data)
-                        try {
-                          var html = json_data.rendered_text;
-                          var lineno = json_data.line_no;
-                          var target_id = json_data.target_id;
-
-                          $comments.find('.cb-comment-add-button').before(html);
-                          console.log(lineno, target_id, $comments);
-
-                        } catch (e) {
-                          console.error(e);
-                        }
-
-
-                        // re trigger the linkification of next/prev navigation
-                        linkifyComments($('.inline-comment-injected'));
-                        timeagoActivate();
-                        bindDeleteCommentButtons();
-                        commentForm.setActionButtonsDisabled(false);
-
-                      };
-                      var submitFailCallback = function(){
-                          commentForm.resetCommentFormState(text)
-                      };
-                      commentForm.submitAjaxPOST(
-                          commentForm.submitUrl, postData, submitSuccessCallback, submitFailCallback);
-                    });
-
-                    setTimeout(function() {
-                        // callbacks
-                        if (cm !== undefined) {
-                            cm.focus();
-                        }
-                    }, 10);
-
-                      $.Topic('/ui/plugins/code/comment_form_built').prepareOrPublish({
-                          form: _form,
-                          parent: $td[0],
-                          lineno: lineno,
-                          f_path: f_path}
-                      );
-                }
-
-                $form.addClass('comment-inline-form-open');
-            }
-
-            this.renderInlineComments = function(file_comments) {
-              show_add_button = typeof show_add_button !== 'undefined' ? show_add_button : true;
-
-              for (var i = 0; i < file_comments.length; i++) {
-                var box = file_comments[i];
-
-                var target_id = $(box).attr('target_id');
-
-                // actually comments with line numbers
-                var comments = box.children;
-
-                for (var j = 0; j < comments.length; j++) {
-                  var data = {
-                    'rendered_text': comments[j].outerHTML,
-                    'line_no': $(comments[j]).attr('line'),
-                    'target_id': target_id
-                  };
-                }
-              }
-
-              // since order of injection is random, we're now re-iterating
-              // from correct order and filling in links
-              linkifyComments($('.inline-comment-injected'));
-              bindDeleteCommentButtons();
-              firefoxAnchorFix();
-            };
-
-        })();
-    }
-    return new _Rhodecode();
-})();
-
             </script>
             <%include file="/base/plugins_base.html"/>
             <!--[if lt IE 9]>
@@ -342,6 +131,13 b' Rhodecode = (function() {'
             ${self.js_extra()}
 
             <script type="text/javascript">
+            Rhodecode = (function() {
+                function _Rhodecode() {
+                  this.comments = new CommentsController();
+                }
+                return new _Rhodecode();
+            })();
+
             $(document).ready(function(){
               show_more_event();
               timeagoActivate();

rhodecode/templates/codeblocks/diffs.html

@@ -192,7 +192,7 b' collapse_all = len(diffset.files) > coll'
         %endif
         %if over_lines_changed_limit:
                 <tr class="cb-warning cb-collapser">
-                    <td class="cb-text" ${c.diffmode == 'unified' and 'colspan=3' or 'colspan=4'}>
+                    <td class="cb-text" ${c.diffmode == 'unified' and 'colspan=4' or 'colspan=4'}>
                         ${_('This diff has been collapsed as it changes many lines, (%i lines changed)' % lines_changed)}
                         <a href="#" class="cb-expand"
                            onclick="$(this).closest('table').removeClass('cb-collapsed'); return false;">${_('Show them')}
@@ -205,20 +205,20 b' collapse_all = len(diffset.files) > coll'
         %endif
         %if filediff.patch['is_limited_diff']:
                 <tr class="cb-warning cb-collapser">
-                    <td class="cb-text" ${c.diffmode == 'unified' and 'colspan=3' or 'colspan=4'}>
+                    <td class="cb-text" ${c.diffmode == 'unified' and 'colspan=4' or 'colspan=4'}>
                         ${_('The requested commit is too big and content was truncated.')} <a href="${link_for(fulldiff=1)}" onclick="return confirm('${_("Showing a big diff might take some time and resources, continue?")}')">${_('Show full diff')}</a>
                     </td>
                 </tr>
         %endif
         %for hunk in filediff.hunks:
                 <tr class="cb-hunk">
-                    <td ${c.diffmode == 'unified' and 'colspan=2' or ''}>
+                    <td ${c.diffmode == 'unified' and 'colspan=3' or ''}>
                         ## TODO: dan: add ajax loading of more context here
                         ## <a href="#">
                             <i class="icon-more"></i>
                         ## </a>
                     </td>
-                    <td ${c.diffmode == 'sideside' and 'colspan=3' or ''}>
+                    <td ${c.diffmode == 'sideside' and 'colspan=5' or ''}>
                         @@
                         -${hunk.source_start},${hunk.source_length}
                         +${hunk.target_start},${hunk.target_length}
@@ -377,7 +377,7 b' from rhodecode.lib.diffs import NEW_FILE'
 
 
         %if use_comments:
-        <a href="#" onclick="$(this).closest('.filediff').toggleClass('hide-comments'); return false;">
+        <a href="#" onclick="return Rhodecode.comments.toggleComments(this);">
             <span class="show-comment-button">${_('Show comments')}</span><span class="hide-comment-button">${_('Hide comments')}</span>
         </a>
         %endif
@@ -410,6 +410,15 b' from rhodecode.lib.diffs import NEW_FILE'
         new_line_anchor = diff_line_anchor(hunk.filediff.target_file_path, line.modified.lineno, 'n')
     %>
     <tr class="cb-line">
+        <td class="cb-data ${action_class(line.original.action)}"
+            data-line-number="${line.original.lineno}"
+            >
+            <div>
+            %if line.original.comments:
+            <i class="icon-comment" onclick="return Rhodecode.comments.toggleLineComments(this)"></i>
+            %endif
+            </div>
+        </td>
         <td class="cb-lineno ${action_class(line.original.action)}"
             data-line-number="${line.original.lineno}"
             %if old_line_anchor:
@@ -431,6 +440,15 b' from rhodecode.lib.diffs import NEW_FILE'
             ${inline_comments_container(line.original.comments)}
             %endif
         </td>
+        <td class="cb-data ${action_class(line.modified.action)}"
+            data-line-number="${line.modified.lineno}"
+            >
+            <div>
+            %if line.modified.comments:
+            <i class="icon-comment" onclick="return Rhodecode.comments.toggleLineComments(this)"></i>
+            %endif
+            </div>
+        </td>
         <td class="cb-lineno ${action_class(line.modified.action)}"
             data-line-number="${line.modified.lineno}"
             %if new_line_anchor:
@@ -467,6 +485,13 b' from rhodecode.lib.diffs import NEW_FILE'
         new_line_anchor = diff_line_anchor(hunk.filediff.target_file_path, new_line_no, 'n')
     %>
     <tr class="cb-line">
+        <td class="cb-data ${action_class(action)}">
+            <div>
+            %if comments:
+            <i class="icon-comment" onclick="return Rhodecode.comments.toggleLineComments(this)"></i>
+            %endif
+            </div>
+        </td>
         <td class="cb-lineno ${action_class(action)}"
             data-line-number="${old_line_no}"
             %if old_line_anchor:
@@ -506,7 +531,7 b' from rhodecode.lib.diffs import NEW_FILE'
 <button
     class="btn btn-small btn-primary cb-comment-box-opener"
     onclick="return Rhodecode.comments.createComment(this)"
->+</button>
+><span>+</span></button>
 </%def>
 
 <%def name="render_diffset_menu()">