mozreview: add inline-comments js, html, css (bug 1115707); r?davidwalsh,smacleod draft
authorbyron jones <glob@mozilla.com>
Thu, 25 Aug 2016 14:12:33 +0800
changeset 171 4807d7726280de5addae0fd594b14585fb562155
parent 151 1151bd237e980c858499e632e4d94d919eb06d00
child 172 44d3d7eb2f3ed329e574fead70480e42b8cd35ae
push idunknown
push userunknown
push dateunknown
reviewersdavidwalsh, smacleod
bugs1115707
mozreview: add inline-comments js, html, css (bug 1115707); r?davidwalsh,smacleod Add core support for showing comments inline on the diff view. MozReview-Commit-ID: HWeg6ZAEt8f
reviewboard/reviewboard/static/rb/css/pages/diffviewer_mozreview.less
reviewboard/reviewboard/static/rb/js/views/inlineCommentView_mozreview.js
reviewboard/reviewboard/staticbundles.py
reviewboard/reviewboard/templates/diffviewer/inline_comments.html
--- a/reviewboard/reviewboard/static/rb/css/pages/diffviewer_mozreview.less
+++ b/reviewboard/reviewboard/static/rb/css/pages/diffviewer_mozreview.less
@@ -21,25 +21,30 @@
 }
 
 
 .sidebyside {
   @linenum-padding-right: 4px;
   @moved-border-width: 4px;
 
   background: white;
-  border-collapse: collapse;
-  padding: 2px;
   width: 100%;
   overflow: hidden;
 
-  // This fixes an annoying 1px bright line between the bottom of the
-  // side-by-side diff view and the bottom border of the roundrect it's
-  // contained within.
-  margin-bottom: -1px;
+  // MozReview: don't use border-collapse:collapse, as this prevents borders
+  // from being rendered in some circumstances, and is the cause of the
+  // "annoying 1px bright line" that upstream uses margin-bottom:-1px to
+  // remove.
+  border-collapse: separate;
+  border-spacing: 0px;
+  padding: 0px;
+
+  &.loading {
+    padding: 0px !important;
+  }
 
   &.diff-error {
     td {
       background: @diff-error-color;
       padding: 1em;
 
       a {
         color: blue;
@@ -828,9 +833,104 @@
   color: @paginate-text-color;
   font-weight: bold;
 }
 
 #pagination2 {
   margin-top: 1em;
 }
 
+
+/****************************************************************************
+ * Inline Comments
+ ****************************************************************************/
+@inline-comments-border: #888;
+@inline-comments-border-hover: #000;
+@inline-comments-background: #f5deb3;
+@inline-comments-text: #555;
+
+.inlineCommentRow {
+  .inlineComments {
+    border-top: 1px solid @inline-comments-border;
+    border-bottom: 1px solid @inline-comments-border;
+
+    &.hovered {
+      border-top: 1px solid @inline-comments-border-hover;
+      border-bottom: 1px solid @inline-comments-border-hover;
+    }
+
+    .inlineComment {
+      background: @inline-comments-background;
+      border-right: 0;
+      cursor: pointer;
+      font-size: 9pt;
+      padding: 2px;
+
+      &.draft {
+        font-style: italic;
+      }
+
+      .meta  {
+        display: inline-block;
+        overflow: hidden;
+        vertical-align: bottom;
+
+        .rb-icon {
+          opacity: 0.75;
+          vertical-align: bottom;
+        }
+      }
+
+      .comment {
+        color: @inline-comments-text;
+        display: inline-block;
+        margin-left: 4px;
+        overflow: hidden;
+        text-overflow: ellipsis;
+        vertical-align: bottom;
+        white-space: nowrap;
+        width: 250px;  // calculated on window.resize
+
+        p {
+          display: inline;
+        }
+      }
+    }
+  }
+}
+
+.sidebyside {
+  tbody {
+    tr.inlineCommentRow {
+      th, td {
+        cursor: default;
+        padding: 0;
+        border-color: @inline-comments-border;
+      }
+    }
+
+    &.delete, &.insert, &.replace {
+      tr {
+        th {
+          background: @diff-linenum-background-color;
+        }
+        td.col1 {
+          background: @inline-comments-background;
+        }
+      }
+    }
+
+    tr {
+      &.selected-draft * { background: @diff-selected-color; }
+    }
+    &.delete {
+      tr.selected-draft * { background: @diff-delete-selected-color; }
+    }
+    &.insert {
+      tr.selected-draft * { background: @diff-insert-selected-color; }
+    }
+    &.replace {
+      tr.selected-draft * { background: @diff-replace-selected-color; }
+    }
+  }
+}
+
 // vim: set et ts=2 sw=2:
new file mode 100644
--- /dev/null
+++ b/reviewboard/reviewboard/static/rb/js/views/inlineCommentView_mozreview.js
@@ -0,0 +1,265 @@
+RB.InlineCommentView = RB.AbstractCommentBlockView.extend({
+    tagName: 'tr',
+    className: 'inlineCommentRow',
+
+    initialize: function() {
+        var view = this;
+        this.commentTemplate = _.template($('#inlineComments-comment').html());
+        this.rowTemplate = null;
+        this.draftTemplate = null;
+        this.$beginRow = null;
+        this.$endRow = null;
+        this.isDraft = false;
+        this.contextRows = [];
+        _.bindAll(this, '_updateSize');
+
+        // Detect when chunks are expanded/collapsed to update visible rows.
+        $.each(RB.PageManager.getPage()._diffReviewableViews, function() {
+            this.on('chunkExpansionChanged', function() {
+                view._updateContextRows();
+                view._updateSize();
+            });
+        });
+
+        // Update icon when issue statuses change.
+        RB.PageManager.getPage().commentIssueManager.on('issueStatusUpdated',
+            function(comment) {
+                $('#c' + comment.id + ' .rb-icon')[0].className =
+                    view._commentIconClass(
+                        comment.get('issueOpened'), comment.get('issueStatus'));
+            });
+
+        // Update rendered comments when draft is added.
+        this.model.get('review').on('saved', function() {
+            view.updateComments();
+        });
+
+        // Don't recalculate the sizes on _every_ window.resize event.
+        this.lazyUpdateSize = _.debounce(this._updateSize, 100);
+        $(window).on('resize', this.lazyUpdateSize);
+    },
+
+    renderContent: function() {
+        this.updateComments();
+        this.lazyUpdateSize();
+    },
+
+    updateComments: function() {
+        var view = this;
+        var commentHtml = [];
+
+        // Cannot render until we know the scope.
+        if (!this.$endRow) {
+            return;
+        }
+
+        // Ensure we notice when a draft is updated or destroyed.
+        if (this.model.get('draftComment')) {
+            if (!this.isDraft) {
+                this.isDraft = true;
+                this.draftTemplate = _.template($('#inlineComments-draft').html());
+                this.model.get('draftComment').on('saved', function() {
+                    view.updateComments();
+                });
+                this.model.get('draftComment').on('destroy', function() {
+                    view.updateComments();
+                });
+            }
+        } else {
+            this.isDraft = false;
+        }
+
+        // Collate comments.
+        var comments = [];
+        _.each(this.model.get('serializedComments'), function(comment) {
+            var obj = { comment: comment, replies: [], depth: 0 };
+            if (comment.reply_to_id) {
+                var reply_to_id = comment.reply_to_id;
+                for (var i = 0, l = comments.length; i < l; i++) {
+                    if (comments[i].comment.comment_id === reply_to_id) {
+                        obj.depth = 1;
+                        comments[i].replies.push(obj);
+                        break;
+                    }
+                }
+            } else {
+                comments.push(obj);
+            }
+        });
+
+        // Generate html.
+        function appendCommentHtml(root) {
+            _.each(root, function(element) {
+                var comment = element.comment;
+                var hasReplies = element.replies.length !== 0;
+                commentHtml.push(view.commentTemplate({
+                    id: comment.comment_id,
+                    user_name: comment.user.username,
+                    comment_html: comment.html,
+                    depth: element.depth,
+                    icon_class: view._commentIconClass(
+                        comment.issue_opened, comment.issue_status)
+                }));
+                if (hasReplies) {
+                    appendCommentHtml(element.replies);
+                }
+            });
+        }
+        appendCommentHtml(comments);
+
+        if (this.isDraft) {
+            commentHtml.push(view.draftTemplate({
+                user_name: RB.UserSession.instance.get('username')
+            }));
+        }
+
+        // Add to DOM.
+        this.$el
+            .html(this.rowTemplate({ commentHtml: commentHtml.join('') }))
+            .prop('id', this.cid);
+
+        // Keep draft comment text in sync.
+        if (this.isDraft) {
+            this.$el.find('.draft .comment')
+                .bindProperty('html', this.model.get('draftComment'), 'html', {
+                    elementToModel: false
+                });
+        }
+
+        // Hook up click and hover events.
+        this.$el
+            .click(function() {
+                view.trigger('clicked');
+            })
+            .hover(
+                function() {
+                    _.each(view.contextRows, function($row) {
+                        $row.addClass('selected');
+                    });
+                    view.$el.find('.inlineComments').addClass('hovered');
+                },
+                function() {
+                    _.each(view.contextRows, function($row) {
+                        $row.removeClass('selected');
+                    });
+                    view.$el.find('.inlineComments').removeClass('hovered');
+                }
+            );
+    },
+
+    remove: function() {
+        Backbone.View.prototype.remove.call(this);
+        $(window).off('resize', this.lazyUpdateSize);
+    },
+
+    setRows: function($beginRow, $endRow) {
+        var view = this;
+        this.$beginRow = $beginRow;
+        this.$endRow = $endRow;
+        this.rowTemplate = this.$endRow.children('th').length === 1
+            ? _.template($('#inlineComments-row-1col').html())
+            : _.template($('#inlineComments-row-2col').html());
+
+        this._updateContextRows();
+        this.updateComments();
+
+        // Highlight selected rows on drafts during creation.
+        if (this.isDraft && !this.model.get('draftComment').get('id')) {
+            _.each(view.contextRows, function($row) {
+                $row.addClass('selected-draft');
+            });
+        }
+    },
+
+    _commentIconClass: function(issueOpened, issueStatus) {
+        return 'rb-icon ' +
+            (issueOpened
+                ? 'rb-icon-issue-' + issueStatus
+                : 'rb-icon-datagrid-comment');
+    },
+
+    positionCommentDlg: function(commentDlg) {
+        commentDlg.$el.css({
+            left: $(document).scrollLeft() +
+                    ($(window).width() - commentDlg.$el.width()) / 2,
+            top: this.$endRow.offset().top + this.$endRow.height()
+        });
+    },
+
+    _updateContextRows: function () {
+        if (!(this.$beginRow && this.$endRow)) {
+            return;
+        }
+        // Keep track of the rows this comment refers to.
+        var beginLineNum = parseInt(this.$beginRow.attr('line'), 10);
+        var endLineNum = parseInt(this.$endRow.attr('line'), 10);
+        this.contextRows = [];
+        for (var lineNum = beginLineNum; lineNum <= endLineNum; lineNum++) {
+            var $tr = $('tr[line="' + lineNum + '"]');
+            if ($tr.length) {
+                this.contextRows.push($tr);
+            }
+        }
+    },
+
+    _updateSize: function() {
+        if (!(this.$beginRow && this.$endRow)) {
+            return;
+        }
+        // Show as much comment context as possible.
+        // We can't accurately measure this as the comment is within the
+        // column we'd need to measure.  table/4 results in a reasonable
+        // result without causing the column size to change.
+        this.$el
+            .find('.inlineComment .comment')
+            .width(this.$el.parents('table').width() / 4);
+    },
+
+    dispose: function() {
+        _.each(this.contextRows, function($row) {
+            $row.removeClass('selected-draft');
+        });
+        this.remove();
+    },
+
+    render: function() {
+        this.renderContent();
+        this.model.on('change:draftComment', this._onDraftCommentChanged, this);
+        this._onDraftCommentChanged();
+        return this;
+    },
+
+    notify: function() {
+        // Ignore requests to show a notification bubble.
+    },
+
+    _onDraftCommentChanged: function() {
+        var view = this,
+            $el = this.$el,
+            comment = this.model.get('draftComment');
+
+        if (!comment) {
+            $el.removeClass('draft');
+            return;
+        }
+
+        comment.on('destroy', function() {
+            // Discard the comment block if empty.
+            if (this.model.isEmpty()) {
+                $el.fadeOut(150, function() { view.dispose(); });
+            } else {
+                $el.removeClass('draft');
+            }
+        }, this);
+
+        comment.on('saved', function() {
+            _.each(view.contextRows, function($row) {
+                $row.removeClass('selected-draft');
+            });
+            view.updateComments();
+            RB.DraftReviewBannerView.instance.show();
+        }, this);
+
+        $el.addClass('draft');
+    }
+});
--- a/reviewboard/reviewboard/staticbundles.py
+++ b/reviewboard/reviewboard/staticbundles.py
@@ -230,16 +230,17 @@ PIPELINE_JS = dict({
             'rb/js/views/reviewReplyDraftBannerView.js',
             'rb/js/views/reviewReplyEditorView.js',
             'rb/js/views/reviewRequestEditorView.js',
             'rb/js/views/screenshotThumbnailView.js',
             'rb/js/views/imageReviewableView.js',
             'rb/js/views/dummyReviewableView.js',
             'rb/js/views/textBasedCommentBlockView.js',
             'rb/js/views/textBasedReviewableView.js',
+            'rb/js/views/inlineCommentView_mozreview.js',
             'rb/js/views/textCommentRowSelector_mozreview.js',
             'rb/js/views/markdownReviewableView.js',
             'rb/js/views/uploadDiffView.js',
             'rb/js/views/updateDiffView.js',
             'rb/js/diffviewer/models/diffCommentBlockModel.js',
             'rb/js/diffviewer/models/diffCommentsHintModel.js',
             'rb/js/diffviewer/models/diffFileModel.js',
             'rb/js/diffviewer/models/diffReviewableModel.js',
new file mode 100644
--- /dev/null
+++ b/reviewboard/reviewboard/templates/diffviewer/inline_comments.html
@@ -0,0 +1,32 @@
+
+<script type="text/template" id="inlineComments-row-1col">
+  <th></th>
+  <td class="inlineComments"><%= commentHtml %></td>
+</script>
+
+<script type="text/template" id="inlineComments-row-2col">
+  <th></th>
+  <td class="col1"></td>
+  <th></th>
+  <td class="inlineComments"><%= commentHtml %></td>
+</script>
+
+<script type="text/template" id="inlineComments-comment">
+  <div class="inlineComment" id="c<%- id %>" style="padding-left: <%- 2 + depth * 8 %>px">
+    <div class="meta">
+      <span class="<%- icon_class %>"></span>
+      <span class="user"><%- user_name %></span>
+    </div>
+    <div class="comment"><%= comment_html %></div>
+  </div>
+</script>
+
+<script type="text/template" id="inlineComments-draft">
+  <div class="inlineComment draft">
+    <div class="meta">
+      <span class="rb-icon rb-icon-datagrid-comment-draft"></span>
+      <span class="user"><%- user_name %></span>
+    </div>
+    <div class="comment"></div>
+  </div>
+</script>