mozreview: draw attention to review comments (bug 1101611) r?mdoglio draft
authorbyron jones <glob@mozilla.com>
Tue, 23 Feb 2016 15:34:50 +0800
changeset 7319 c2e6d518193ecceb78b59ef33861dedbe50c2431
parent 7318 e668e1a9894407f051d33281a41bae50cdba511c
push id659
push userbjones@mozilla.com
push dateThu, 25 Feb 2016 07:01:50 +0000
reviewersmdoglio
bugs1101611
mozreview: draw attention to review comments (bug 1101611) r?mdoglio Review comments appear only as a tiny number in the left margin. This makes them easy to overlook, especially when viewing side-by-side diffs where your focus is on the right column. This adds a dotted line across the diff view to draw attention to the comments. MozReview-Commit-ID: EohHRRTY9pB
pylib/mozreview/mozreview/static/mozreview/css/viewdiff.less
pylib/mozreview/mozreview/static/mozreview/js/init_filediffreviewer.js
--- a/pylib/mozreview/mozreview/static/mozreview/css/viewdiff.less
+++ b/pylib/mozreview/mozreview/static/mozreview/css/viewdiff.less
@@ -1,8 +1,10 @@
+@import (reference) "@{STATIC_ROOT}rb/css/defs.less";
+
 button.diff-file-btn {
     position: absolute;
     background-color: #DDD;
     font-weight: bold;
     text-align: right;
     top: 7px;
     right: 7px;
     padding: 5px;
@@ -37,16 +39,18 @@ button.diff-file-btn {
 }
 
 .unindent {
     color: #aaaaaa;
     background-color: #ffe0e5;
     outline: 1px solid #ffe0e5;
 }
 
+/* add comment icon which follows cursor */
+
 .sidebyside {
     tbody {
         tr {
             &:hover {
                 pre:before {
                     content: '';
                     display: relative;
                     box-shadow: 1px 1px 3px black;
@@ -66,9 +70,18 @@ button.diff-file-btn {
                 padding-right: 20px;
             }
         }
     }
 }
 
 span.ghost-commentflag {
     display: none !important;
-}
\ No newline at end of file
+}
+
+/* make review comments more noticable */
+
+.comment-block-container {
+  border-top: 2px dotted @comment-flag-color;
+}
+.comment-block-container-draft {
+  border-top: 2px dotted @comment-flag-draft-color;
+}
--- a/pylib/mozreview/mozreview/static/mozreview/js/init_filediffreviewer.js
+++ b/pylib/mozreview/mozreview/static/mozreview/js/init_filediffreviewer.js
@@ -1,11 +1,49 @@
 $(document).ready(function() {
   var page = RB.PageManager.getPage();
 
+  // Adjust the styling of comment blocks to draw attention to their
+  // existance.  Unfortunately the template is opaque to us, so we
+  // have to resort to MutationObserver shenanigans.
+  try {
+    var observer = new MutationObserver(function(mutations) {
+      mutations.forEach(function(mutation) {
+        var $target = $(mutation.target);
+        if ($target.hasClass('diff-box')) {
+          // initial page layout
+          var $flags = $target.find('.commentflag');
+          $flags
+            .filter(':not(.draft)')
+            .parents('tr')
+            .addClass('comment-block-container');
+          $flags
+            .filter('.draft')
+            .parents('tr')
+            .addClass('comment-block-container-draft');
+        } else if ($target.prop('nodeName') === 'TH') {
+          // comment added/removed
+          var $tr = $target.parent('tr');
+          $tr
+            .removeClass('comment-block-container')
+            .removeClass('comment-block-container-draft');
+          if ($target.find('.commentflag').length) {
+            $tr.addClass($target.find('.commentflag.draft').length ?
+                         'comment-block-container-draft' :
+                         'comment-block-container');
+          }
+        }
+      });
+    });
+    observer.observe(document.querySelector('#diffs'),
+                     { childList: true, subtree: true });
+  } catch(e) {
+    // we don't care if this fails
+  }
+
   var FileDiffReviewerData = $('#file-diff-reviewer-data')
                              .data('file-diff-reviewer');
   var fileDiffReviewerModels = FileDiffReviewerData.map(function(item){
     return new RB.FileDiffReviewerModel(item);
   });
   var fileDiffReviewerCollection = new RB.FileDiffReviewerCollection(
     fileDiffReviewerModels
   );