Draft comments initial implementation

This implements basic functionality for draft CRUD operations.

There are a few things that are TBD:
+ Layout edge cases within the diff view.
+ Reply/Done actions in threads.
+ Not allowing the user to add drafts if logged out.
+ I’m sure a few more things...

Feature: Issue 3649
Change-Id: Ia7419eecee5d5b20e73e17241990d7a7ffede0e8

lib/js/BUCK

@@ -104,6 +104,21 @@ bower_component(
   sha1 = 'f94a3a3d847842c49def41e27da42c7c94f8d7c7',
 )
 
+bower_component(
+  name = 'iron-autogrow-textarea',
+  package = 'polymerelements/iron-autogrow-textarea',
+  version = '1.0.10',
+  deps = [
+    ':iron-behaviors',
+    ':iron-flex-layout',
+    ':iron-form-element-behavior',
+    ':iron-validatable-behavior',
+    ':polymer',
+  ],
+  license = 'polymer',
+  sha1 = 'd368240e60a4b02ffc731ad8f45f3c8bbf47e9bd',
+)
+
 bower_component(
   name = 'iron-behaviors',
   package = 'polymerelements/iron-behaviors',
@@ -150,6 +165,15 @@ bower_component(
   sha1 = '3ca2fbbf3b56d95677663f78304262dee68753c3',
 )
 
+bower_component(
+  name = 'iron-form-element-behavior',
+  package = 'polymerelements/iron-form-element-behavior',
+  version = '1.0.6',
+  deps = [':polymer'],
+  license = 'polymer',
+  sha1 = '8d9e6530edc1b99bec1a5c34853911fba3701220',
+)
+
 bower_component(
   name = 'iron-input',
   package = 'polymerelements/iron-input',
@@ -206,7 +230,6 @@ bower_component(
   name = 'iron-test-helpers',
   package = 'polymerelements/iron-test-helpers',
   version = '1.0.6',
-  semver = '~1.0.6',
   deps = [':polymer'],
   license = 'DO_NOT_DISTRIBUTE',
   sha1 = 'c0f7c7f010ca3c63fb08ae0d9462e400380cde2c',

polygerrit-ui/.gitignore

@@ -1,5 +1,6 @@
 node_modules
 npm-debug.log
 dist
+bower.json
 bower_components
 .tmp

polygerrit-ui/BUCK

@@ -5,6 +5,7 @@ bower_components(
   deps = [
     '//lib/js:iron-a11y-keys-behavior',
     '//lib/js:iron-ajax',
+    '//lib/js:iron-autogrow-textarea',
     '//lib/js:iron-dropdown',
     '//lib/js:iron-input',
     '//lib/js:page',

polygerrit-ui/app/elements/gr-diff-comment-thread.html

@@ -27,7 +27,12 @@ limitations under the License.
       }
     </style>
     <template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment">
-      <gr-diff-comment comment="[[comment]]"></gr-diff-comment>
+      <gr-diff-comment
+          comment="{{comment}}"
+          change-num="[[changeNum]]"
+          patch-num="[[patchNum]]"
+          draft="[[comment.__draft]]"
+          editing="[[!comment.message]]"></gr-diff-comment>
     </template>
   </template>
   <script>
@@ -44,17 +49,31 @@ limitations under the License.
        * @event gr-diff-comment-thread-height-changed
        */
 
+      /**
+       * Fired when the thread should be discarded.
+       *
+       * @event gr-diff-comment-thread-discard
+       */
+
       properties: {
+        changeNum: String,
         comments: {
           type: Array,
+          value: function() { return []; },
           observer: '_commentsChanged',
         },
+        patchNum: String,
+
         _orderedComments: Array,
       },
 
       ready: function() {
         this.addEventListener('gr-diff-comment-height-changed',
             this._handleCommentHeightChange.bind(this));
+        this.addEventListener('gr-diff-comment-reply',
+            this._handleCommentReply.bind(this));
+        this.addEventListener('gr-diff-comment-discard',
+            this._handleCommentDiscard.bind(this));
       },
 
       _commentsChanged: function(comments) {
@@ -62,7 +81,6 @@ limitations under the License.
       },
 
       _sortedComments: function(comments) {
-        var comments = comments || [];
         comments.sort(function(c1, c2) {
           return util.parseDate(c1.updated) - util.parseDate(c2.updated);
         });
@@ -105,6 +123,37 @@ limitations under the License.
             {height: this.offsetHeight});
       },
 
+      _handleCommentReply: function(e) {
+        console.log('should add reply...')
+      },
+
+      _handleCommentDiscard: function(e) {
+        var diffCommentEl = e.target;
+        var idx = this._indexOf(diffCommentEl.comment, this.comments);
+        if (idx == -1) {
+          throw Error('Cannot find comment ' +
+              JSON.stringify(diffCommentEl.comment));
+        }
+        this.comments.splice(idx, 1);
+        this._commentsChanged(this.comments);
+        if (this.comments.length == 0 && this.parentNode) {
+          this.parentNode.removeChild(this);
+        }
+        this.fire('gr-diff-comment-thread-height-changed',
+            {height: this.offsetHeight});
+      },
+
+      _indexOf: function(comment, arr) {
+        for (var i = 0; i < arr.length; i++) {
+          var c = arr[i];
+          if ((c.__draftID != null && c.__draftID == comment.__draftID) ||
+              (c.id != null && c.id == comment.id)) {
+            return i;
+          }
+        }
+        return -1;
+      },
+
     });
   })();
   </script>

polygerrit-ui/app/elements/gr-diff-comment.html

@@ -15,6 +15,9 @@ limitations under the License.
 -->
 
 <link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="../bower_components/polymer/polymer.html">
+<link rel="import" href="../bower_components/iron-autogrow-textarea/iron-autogrow-textarea.html">
+<link rel="import" href="../bower_components/iron-ajax/iron-request.html">
 <link rel="import" href="gr-date-formatter.html">
 
 <dom-module id="gr-diff-comment">
@@ -24,6 +27,12 @@ limitations under the License.
         border: 1px solid #ddd;
         display: block;
       }
+      :host([disabled]) {
+        pointer-events: none;
+      }
+      :host([disabled]) .container {
+        opacity: .5;
+      }
       .header,
       .message,
       .actions {
@@ -31,32 +40,113 @@ limitations under the License.
       }
       .header {
         background-color: #eee;
+        display: flex;
         font-family: 'Open Sans', sans-serif;
       }
-      gr-date-formatter {
+      .headerLeft {
-        float: right;
+        flex: 1;
+      }
+      .authorName,
+      .draftLabel {
+        font-weight: bold;
+      }
+      .draftLabel {
+        color: #999;
+        display: none;
+      }
+      .date {
+        justify-content: flex-end;
         margin-left: 5px;
       }
-      .authorName {
+      a.date:link,
-        font-weight: bold;
+      a.date:visited {
+        color: #666;
+        text-decoration: none;
+      }
+      a.date:hover {
+        text-decoration: underline;
       }
       .message {
         white-space: pre-wrap;
       }
       .actions {
-        /** TODO: remove once the actions actually do something. **/
+        display: flex;
-        display: none;
         padding-top: 0;
       }
+      .action {
+        margin-right: 1em;
+      }
+      .action[disabled] {
+        opacity: .5;
+        pointer-events: none;
+      }
+      .danger {
+        display: flex;
+        flex: 1;
+        justify-content: flex-end;
+      }
+      .editMessage {
+        display: none;
+        margin: .5em .7em;
+        width: calc(100% - 1.4em - 2px);
+      }
+      .danger .action {
+        margin-right: 0;
+      }
+      .container:not(.draft) .actions :not(.reply):not(.done) {
+        display: none;
+      }
+      .draft .reply,
+      .draft .done {
+        display: none;
+      }
+      .draft .draftLabel {
+        display: inline;
+      }
+      .draft:not(.editing) .save,
+      .draft:not(.editing) .cancel {
+        display: none;
+      }
+      .editing .message,
+      .editing .reply,
+      .editing .done,
+      .editing .edit {
+        display: none;
+      }
+      .editing .editMessage {
+        display: block;
+      }
     </style>
+    <div class="container" id="container">
       <div class="header" id="header">
+        <div class="headerLeft">
           <span class="authorName">[[comment.author.name]]</span>
-      <gr-date-formatter date-str="[[comment.updated]]"></gr-date-formatter>
+          <span class="draftLabel">DRAFT</span>
         </div>
+        <a class="date" href$="[[_computeLinkToComment(comment)]]" on-tap="_handleLinkTap">
+          <gr-date-formatter date-str="[[comment.updated]]"></gr-date-formatter>
+        </a>
+      </div>
+      <iron-autogrow-textarea
+          id="editTextarea"
+          class="editMessage"
+          disabled="{{disabled}}"
+          rows="4"
+          max-rows="4"
+          bind-value="{{_editDraft}}"></iron-autogrow-textarea>
       <div class="message">[[comment.message]]</div>
       <div class="actions">
-      <a class="reply" href="#" on-tap="_handleReply">Reply</a>
+        <a class="action reply" href="#" on-tap="_handleReply">Reply</a>
-      <a class="done" href="#" on-tap="_handleDone">Done</a>
+        <a class="action done" href="#" on-tap="_handleDone">Done</a>
+        <a class="action edit" href="#" on-tap="_handleEdit">Edit</a>
+        <a class="action save" href="#"
+            disabled$="[[_computeSaveDisabled(_editDraft)]]"
+            on-tap="_handleSave">Save</a>
+        <a class="action cancel" href="#" on-tap="_handleCancel">Cancel</a>
+        <div class="danger">
+          <a class="action discard" href="#" on-tap="_handleDiscard">Discard</a>
+        </div>
+      </div>
     </div>
   </template>
   <script>
@@ -84,17 +174,81 @@ limitations under the License.
        * @event gr-diff-comment-done
        */
 
+      /**
+       * Fired when this comment is discarded.
+       *
+       * @event gr-diff-comment-discard
+       */
+
       properties: {
-        comment: Object,
+        changeNum: String,
+        comment: {
+          type: Object,
+          notify: true,
+        },
+        disabled: {
+          type: Boolean,
+          value: false,
+          reflectToAttribute: true,
+        },
         draft: {
           type: Boolean,
           value: false,
+          observer: '_draftChanged',
         },
+        editing: {
+          type: Boolean,
+          value: false,
+          observer: '_editingChanged',
+        },
+        patchNum: String,
+
+        _xhrPromise: Object,
+        _editDraft: String,
       },
 
       attached: function() {
+        this._heightChanged();
+      },
+
+      _heightChanged: function() {
+        this.async(function() {
           this.fire('gr-diff-comment-height-changed',
             {height: this.offsetHeight});
+        }.bind(this));
+      },
+
+      _draftChanged: function(draft) {
+        this.$.container.classList.toggle('draft', draft);
+      },
+
+      _editingChanged: function(editing) {
+        this.$.container.classList.toggle('editing', editing);
+        if (editing) {
+          this.async(function() {
+            this.$.editTextarea.textarea.focus();
+          }.bind(this));
+        }
+        this._heightChanged();
+      },
+
+      _computeLinkToComment: function(comment) {
+        return '#' + comment.line;
+      },
+
+      _computeSaveDisabled: function(draft) {
+        return draft == null || draft.trim() == '';
+      },
+
+      _handleLinkTap: function(e) {
+        e.preventDefault();
+        var hash = this._computeLinkToComment(this.comment);
+        // Don't add the hash to the window history if it's already there.
+        // Otherwise you mess up expected back button behavior.
+        if (window.location.hash == hash) { return; }
+        // Change the URL but don’t trigger a nav event. Otherwise it will
+        // reload the page.
+        page.show(window.location.pathname + hash, null, false);
       },
 
       _handleReply: function(e) {
@@ -107,6 +261,89 @@ limitations under the License.
         this.fire('gr-diff-comment-done');
       },
 
+      _handleEdit: function(e) {
+        e.preventDefault();
+        this._editDraft = this.comment.message;
+        this.editing = true;
+      },
+
+      _handleSave: function(e) {
+        e.preventDefault();
+        this.comment.message = this._editDraft;
+        this.disabled = true;
+        var endpoint = this._restEndpoint(this.comment.id);
+        this._send('PUT', endpoint).then(function(req) {
+          this.disabled = false;
+          var comment = req.response;
+          comment.__draft = true;
+          // Maintain the ephemeral draft ID for identification by other
+          // elements.
+          if (this.comment.__draftID) {
+            comment.__draftID = this.comment.__draftID;
+          }
+          this.comment = comment;
+          this.editing = false;
+        }.bind(this)).catch(function(err) {
+          alert('Your draft couldn’t be saved. Check the console and contact ' +
+              'the PolyGerrit team for assistance.');
+          this.disabled = false;
+        }.bind(this));
+      },
+
+      _handleCancel: function(e) {
+        e.preventDefault();
+        if (this.comment.message == null || this.comment.message.length == 0) {
+          this.fire('gr-diff-comment-discard');
+          return;
+        }
+        this._editDraft = this.comment.message;
+        this.editing = false;
+      },
+
+      _handleDiscard: function(e) {
+        e.preventDefault();
+        if (!this.comment.__draft) {
+          throw Error('Cannot discard a non-draft comment.');
+        }
+        this.disabled = true;
+        var commentID = this.comment.id;
+        if (!commentID) {
+          this.fire('gr-diff-comment-discard');
+          return;
+        }
+        this._send('DELETE', this._restEndpoint(commentID)).then(function(req) {
+          this.fire('gr-diff-comment-discard', this.comment);
+        }.bind(this)).catch(function(err) {
+          alert('Your draft couldn’t be deleted. Check the console and ' +
+              'contact the PolyGerrit team for assistance.');
+          this.disabled = false;
+        }.bind(this));
+      },
+
+      _send: function(method, url) {
+        var xhr = document.createElement('iron-request');
+        this._xhrPromise = xhr.send({
+          method: method,
+          headers: {
+            'Content-Type': 'application/json',
+            'X-Gerrit-Auth': util.getCookie('XSRF_TOKEN'),
+          },
+          url: url,
+          body: this.comment,
+          jsonPrefix: ')]}\'',
+        });
+        return this._xhrPromise;
+      },
+
+      _restEndpoint: function(id) {
+        var path = '/changes/' + this.changeNum + '/revisions/' +
+            this.patchNum + '/drafts';
+        if (id) {
+          path += '/' + id;
+        }
+        return path;
+      },
+
     });
   })();
   </script>

polygerrit-ui/app/elements/gr-diff-view.html

@@ -81,10 +81,12 @@ limitations under the License.
       .content {
         position: relative;
       }
-      .lineNum.blank {
+      .lineNum.blank,
+      .threadFiller--redLine {
         border-right: 2px solid #F34D4D;
         margin-right: 3px;
       }
+
       .lineNum:not(.blank) {
         cursor: pointer;
       }
@@ -133,6 +135,16 @@ limitations under the License.
         url="[[_computeFilesPath(_changeNum, _patchNum)]]"
         json-prefix=")]}'"
         on-response="_handleFilesResponse"></iron-ajax>
+    <iron-ajax
+        id="leftDraftsXHR"
+        url="[[_computeDraftsPath(_changeNum, _basePatchNum)]]"
+        json-prefix=")]}'"
+        on-response="_handleLeftDraftsResponse"></iron-ajax>
+    <iron-ajax
+        id="rightDraftsXHR"
+        url="[[_computeDraftsPath(_changeNum, _patchNum)]]"
+        json-prefix=")]}'"
+        on-response="_handleRightDraftsResponse"></iron-ajax>
     <h3>
       <a href$="[[_computeChangePath(_changeNum)]]">[[_changeNum]]</a><span>:</span>
       <span>[[_change.subject]]</span> — <span>[[params.path]]</span>
@@ -184,11 +196,25 @@ limitations under the License.
           type: Array,
           value: function() { return []; },
         },
-        _leftComments: Array,
+        _leftComments: {
+          type: Array,
+          value: function() { return []; },
+        },
+        _leftDrafts: {
+          type: Array,
+          value: function() { return []; },
+        },
         _patchNum: String,
         _path: String,
         _rendered: Boolean,
-        _rightComments: Array,
+        _rightComments: {
+          type: Array,
+          value: function() { return []; },
+        },
+        _rightDrafts: {
+          type: Array,
+          value: function() { return []; },
+        },
       },
 
       listeners: {
@@ -210,21 +236,48 @@ limitations under the License.
           this._patchNum = null;
           this._diff = null;
           this._path = null;
-          this._leftComments = null;
+          this._leftComments = [];
-          this._rightComments = null;
+          this._rightComments = [];
+          this._leftDrafts = [];
+          this._rightDrafts = [];
           this._rendered = false;
           return;
         }
         // Assign the params here since a computed binding relying on
         // `_basePatchNum` won't fire in the case where it's not defined.
         this.$.diffXHR.params = this._diffQueryParams(this._basePatchNum);
-        this.$.diffXHR.generateRequest();
+
+        var requestPromises = [];
+        requestPromises.push(this.$.diffXHR.generateRequest().completes);
 
         if (this._basePatchNum) {
-          this.$.leftCommentsXHR.generateRequest();
+          requestPromises.push(
+              this.$.leftCommentsXHR.generateRequest().completes);
         }
-        this.$.rightCommentsXHR.generateRequest();
+
-        this.$.filesXHR.generateRequest();
+        requestPromises.push(
+            this.$.rightCommentsXHR.generateRequest().completes);
+        requestPromises.push(this.$.filesXHR.generateRequest().completes);
+
+        app.accountReady.then(function() {
+          if (app.loggedIn) {
+            if (this._basePatchNum) {
+              requestPromises.push(
+                  this.$.leftDraftsXHR.generateRequest().completes);
+            }
+            requestPromises.push(
+                this.$.rightDraftsXHR.generateRequest().completes);
+          }
+
+          Promise.all(requestPromises).then(function(requests) {
+            this._renderDiff(this._diff, this._leftComments,
+                this._rightComments, this._leftDrafts, this._rightDrafts);
+          }.bind(this), function(err) {
+            alert('Oops. Something went wrong. Check the console and bug the ' +
+                'PolyGerrit team for assistance.');
+            throw err;
+          });
+        }.bind(this));
       },
 
       _rulerWidthChanged: function(newValue, oldValue) {
@@ -273,6 +326,10 @@ limitations under the License.
         return '/changes/' + changeNum + '/revisions/' + patchNum + '/files';
       },
 
+      _computeDraftsPath: function(changeNum, patchNum) {
+        return '/changes/' + changeNum + '/revisions/' + patchNum + '/drafts';
+      },
+
       _diffQueryParams: function(basePatchNum) {
         var params =  {
           context: 'ALL',
@@ -286,21 +343,65 @@ limitations under the License.
 
       _diffContainerTapHandler: function(e) {
         var el = e.detail.sourceEvent.target;
-        if (el.classList.contains('lineNum')) {
+        // This tap handler only handles line number taps.
-          // TODO: Implement adding draft comments.
+        if (!el.classList.contains('lineNum')) { return; }
+
+        var leftSide = el.parentNode == this.$.leftDiffNumbers;
+        var rightSide = el.parentNode == this.$.rightDiffNumbers;
+        if (leftSide == rightSide) {
+          throw Error('Comment tap event cannot originate from both left and ' +
+              'right side');
         }
+
+        // If a draft or comment is already present at that line, don’t do
+        // anything.
+        var lineNum = el.getAttribute('data-line-num');
+        var patchNum = el.getAttribute('data-patch-num');
+
+        var existingEl = this.$$('gr-diff-comment-thread' +
+            '[data-patch-num="' + patchNum + '"]' +
+            '[data-line-num="' + lineNum + '"]');
+        if (existingEl) {
+          // A comment or draft is already present at this line.
+          return;
+        }
+
+        var tempDraftID = Math.floor(Math.random() * Math.pow(10, 10)) + '';
+        var drafts = [{
+          __draft: true,
+          __draftID: tempDraftID,
+          path: this._path,
+          line: lineNum,
+        }];
+
+        // If the comment is on the left side of a side-by-side diff with the
+        // parent on the left and a patch with patchNum on the right, the patch
+        // number passed to the backend is the right side patchNum when mutating
+        // a draft. The property `side` is used to determine that it should be
+        // on the parent patch, which is inconsistent and why this looks weird.
+        var patchNum = this._patchNum;
+        if (leftSide && this._basePatchNum == null) {
+          drafts[0].side = 'PARENT';
+          patchNum = 'PARENT';
+        }
+
+        this._addThread(drafts, patchNum, lineNum);
       },
 
       _handleLeftCommentsResponse: function(e, req) {
         this._leftComments = e.detail.response[this._path] || [];
-        this._maybeRenderDiff(this._diff, this._leftComments,
-            this._rightComments);
       },
 
       _handleRightCommentsResponse: function(e, req) {
         this._rightComments = e.detail.response[this._path] || [];
-        this._maybeRenderDiff(this._diff, this._leftComments,
+      },
-            this._rightComments);
+
+      _handleLeftDraftsResponse: function(e, req) {
+        this._leftDrafts = e.detail.response[this._path] || [];
+      },
+
+      _handleRightDraftsResponse: function(e, req) {
+        this._rightDrafts = e.detail.response[this._path] || [];
       },
 
       _handleFilesResponse: function(e, req) {
@@ -309,8 +410,6 @@ limitations under the License.
 
       _handleDiffResponse: function(e, req) {
         this._diff = e.detail.response;
-        this._maybeRenderDiff(this._diff, this._leftComments,
-            this._rightComments);
       },
 
       _handleKey: function(e) {
@@ -351,9 +450,17 @@ limitations under the License.
         return 'thread-' + patchNum + '-' + lineNum;
       },
 
-      _renderComments: function(comments, patchNum) {
+      _renderCommentsAndDrafts: function(comments, drafts, patchNum) {
-        // Group the comments by line number. Absense of a line number indicates
+        // Drafts and comments are combined here, with drafts annotated with a
-        // a top-level file comment.
+        // property.
+        var annotatedDrafts = drafts.map(function(d) {
+          d.__draft = true;
+          return d;
+        });
+        comments = comments.concat(annotatedDrafts);
+
+        // Group the comments and drafts by line number. Absence of a line
+        // number indicates a top-level file comment or draft.
         var threads = {};
 
         for (var i = 0; i < comments.length; i++) {
@@ -371,8 +478,16 @@ limitations under the License.
       _addThread: function(comments, patchNum, lineNum) {
         var el = document.createElement('gr-diff-comment-thread');
         el.comments = comments;
+        el.changeNum = this._changeNum;
+        // Assign the element's patchNum to the right side patchNum if the
+        // passed patchNum is 'PARENT' do to the odd behavior of the REST API.
+        // Don't overwrite patchNum since 'PARENT' is used for other properties.
+        el.patchNum = patchNum == 'PARENT' ? this._patchNum : patchNum;
+
         var threadID = this._threadID(patchNum, lineNum);
         el.setAttribute('data-thread-id', threadID);
+        el.setAttribute('data-line-num', lineNum);
+        el.setAttribute('data-patch-num', patchNum);
 
         // Find the element that the thread should be appended after. In the
         // case of a file comment, it will be appended after the first line.
@@ -406,7 +521,7 @@ limitations under the License.
         var els = Polymer.dom(this.root).querySelectorAll(
             '[data-row-num="' + rowNum + '"]');
         for (var i = 0; i < els.length; i++) {
-          // Is this is the side with the comment? Skip if so.
+          // Is this is the column with the comment? Skip if so.
           if (els[i].nextSibling &&
               els[i].nextSibling.tagName == 'GR-DIFF-COMMENT-THREAD') {
             continue;
@@ -414,6 +529,9 @@ limitations under the License.
           var fillerEl = document.createElement('div');
           fillerEl.setAttribute('data-thread-id', threadID);
           fillerEl.classList.add('js-threadFiller');
+          if (els[i].classList.contains('lineNum')) {
+            fillerEl.classList.add('threadFiller--redLine');
+          }
           fillerEl.style.height = e.detail.height + 'px';
           Polymer.dom(els[i].parentNode).insertBefore(
               fillerEl, els[i].nextSibling);
@@ -426,16 +544,14 @@ limitations under the License.
         }
       },
 
-      _maybeRenderDiff: function(diff, leftComments, rightComments) {
+      _renderDiff: function(
+          diff, leftComments, rightComments, leftDrafts, rightDrafts) {
         if (this._rendered) {
           this._clearChildren(this.$.leftDiffNumbers);
           this._clearChildren(this.$.leftDiffContent);
           this._clearChildren(this.$.rightDiffNumbers);
           this._clearChildren(this.$.rightDiffContent);
         }
-        if (!diff || !diff.content) { return; }
-        if (this._basePatchNum && leftComments == null) { return; }
-        if (rightComments == null) { return; }
 
         this.$.diffContainer.classList.toggle('rightOnly',
             diff.change_type == Changes.DiffType.ADDED);
@@ -461,10 +577,12 @@ limitations under the License.
         }
 
         if (leftComments) {
-          this._renderComments(leftComments, this._basePatchNum);
+          this._renderCommentsAndDrafts(leftComments, leftDrafts,
+              this._basePatchNum);
         }
         if (rightComments) {
-          this._renderComments(rightComments, this._patchNum);
+          this._renderCommentsAndDrafts(rightComments, rightDrafts,
+              this._patchNum);
         }
 
         if (this.rulerWidth) {
@@ -561,15 +679,13 @@ limitations under the License.
           el.setAttribute('data-row-num', ctx.rowNum);
         });
 
-        var self = this;
-        if (this._basePatchNum) {
         [leftLineNumEl, leftColEl].forEach(function(el) {
-            el.setAttribute('data-patch-num', self._basePatchNum);
+          el.setAttribute('data-patch-num', this._basePatchNum || 'PARENT');
-          });
+        }.bind(this));
-        }
+
         [rightLineNumEl, rightColEl].forEach(function(el) {
-          el.setAttribute('data-patch-num', self._patchNum);
+          el.setAttribute('data-patch-num', this._patchNum);
-        });
+        }.bind(this));
 
         if (ctx.left.content != null) {
           leftLineNumEl.textContent = ctx.left.lineNum;

polygerrit-ui/app/scripts/util.js

@@ -47,3 +47,18 @@ util.shouldSupressKeyboardShortcut = function(e) {
          target.tagName == 'BUTTON' ||
          target.tagName == 'A';
 };
+
+util.getCookie = function(name) {
+  var key = name + '=';
+  var cookies = document.cookie.split(';');
+  for(var i = 0; i < cookies.length; i++) {
+    var c = cookies[i];
+    while (c.charAt(0) == ' ') {
+      c = c.substring(1);
+    }
+    if (c.indexOf(key) == 0) {
+      return c.substring(key.length, c.length);
+    }
+  }
+  return '';
+};

polygerrit-ui/app/test/gr-diff-comment-test.html

@@ -20,6 +20,8 @@ limitations under the License.
 
 <script src="../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
 <script src="../../bower_components/web-component-tester/browser.js"></script>
+<script src="../../bower_components/page/page.js"></script>
+<script src="../scripts/util.js"></script>
 
 <link rel="import" href="../../bower_components/iron-test-helpers/iron-test-helpers.html">
 <link rel="import" href="../elements/gr-diff-comment.html">
@@ -30,11 +32,27 @@ limitations under the License.
   </template>
 </test-fixture>
 
+<test-fixture id="draft">
+  <template>
+    <gr-diff-comment draft="true"></gr-diff-comment>
+  </template>
+</test-fixture>
+
 <script>
   suite('gr-diff-comment tests', function() {
     var element;
     setup(function() {
       element = fixture('basic');
+      element.comment = {
+        author: {
+          name: 'Mr. Peanutbutter',
+          email: 'tenn1sballchaser@aol.com',
+        },
+        id: 'baf0414d_60047215',
+        line: 5,
+        message: 'is this a crossover episode!?',
+        updated: '2015-12-08 19:48:33.843000000',
+      }
     });
 
     test('proper event fires on reply', function(done) {
@@ -51,5 +69,185 @@ limitations under the License.
       MockInteractions.tap(element.$$('.done'));
     });
 
+    test('clicking on date link does not trigger nav', function() {
+      var showStub = sinon.stub(page, 'show');
+      var dateEl = element.$$('.date');
+      assert.ok(dateEl);
+      MockInteractions.tap(dateEl);
+      var dest = window.location.pathname + '#5';
+      assert(showStub.lastCall.calledWithExactly(dest, null, false),
+          'Should navigate to ' + dest + ' without triggering nav');
+      showStub.restore();
+    });
+  });
+
+  suite('gr-diff-comment draft tests', function() {
+    var element;
+    var server;
+
+    setup(function() {
+      element = fixture('draft');
+      element.changeNum = 42;
+      element.patchNum = 1;
+      element.comment = {
+        __draft: true,
+        __draftID: 'temp_draft_id',
+        path: '/path/to/file',
+        line: 5,
+      };
+
+      server = sinon.fakeServer.create();
+      server.respondWith(
+        'PUT',
+        '/changes/42/revisions/1/drafts',
+        [
+          201,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n{' +
+            '"id": "baf0414d_40572e03",' +
+            '"path": "/path/to/file",' +
+            '"line": 5,' +
+            '"updated": "2015-12-08 21:52:36.177000000",' +
+            '"message": "created!"' +
+          '}'
+        ]
+      );
+
+      server.respondWith(
+        'PUT',
+        /\/changes\/42\/revisions\/1\/drafts\/.+/,
+        [
+          200,
+          { 'Content-Type': 'application/json' },
+          ')]}\'\n{' +
+            '"id": "baf0414d_40572e03",' +
+            '"path": "/path/to/file",' +
+            '"line": 5,' +
+            '"updated": "2015-12-08 21:52:36.177000000",' +
+            '"message": "saved!"' +
+          '}'
+        ]
+      );
+    });
+
+    teardown(function() {
+      server.restore();
+    });
+
+    function isVisible(el) {
+      assert.ok(el);
+      return getComputedStyle(el).getPropertyValue('display') != 'none';
+    }
+
+    test('button visibility states', function() {
+      element.draft = true;
+      assert.isTrue(isVisible(element.$$('.edit')), 'edit is visible');
+      assert.isTrue(isVisible(element.$$('.discard')), 'discard is visible');
+      assert.isFalse(isVisible(element.$$('.save')), 'save is not visible');
+      assert.isFalse(isVisible(element.$$('.cancel')), 'cancel is not visible');
+      assert.isFalse(isVisible(element.$$('.reply')), 'reply is not visible');
+      assert.isFalse(isVisible(element.$$('.done')), 'done is not visible');
+
+      element.editing = true;
+      assert.isFalse(isVisible(element.$$('.edit')), 'edit is not visible')
+      assert.isTrue(isVisible(element.$$('.discard')), 'discard is visible');
+      assert.isTrue(isVisible(element.$$('.save')), 'save is visible');
+      assert.isTrue(isVisible(element.$$('.cancel')), 'cancel is visible');
+      assert.isFalse(isVisible(element.$$('.reply')), 'reply is not visible');
+      assert.isFalse(isVisible(element.$$('.done')), 'done is not visible');
+
+      element.draft = false,
+      element.editing = false;
+      assert.isFalse(isVisible(element.$$('.edit')), 'edit is not visible')
+      assert.isFalse(isVisible(element.$$('.discard')),
+          'discard is not visible');
+      assert.isFalse(isVisible(element.$$('.save')), 'save is not visible');
+      assert.isFalse(isVisible(element.$$('.cancel')), 'cancel is not visible');
+      assert.isTrue(isVisible(element.$$('.reply')), 'edit is visible');
+      assert.isTrue(isVisible(element.$$('.done')), 'edit is visible');
+
+      element.draft = true;
+    });
+
+    test('draft creation/cancelation', function(done) {
+      assert.isFalse(element.editing);
+      MockInteractions.tap(element.$$('.edit'));
+      assert.isTrue(element.editing);
+
+      element._editDraft = '';
+      // Save should be disabled on an empty message.
+      var disabled = element.$$('.save').hasAttribute('disabled');
+      assert.isTrue(disabled, 'save button should be disabled.');
+      element._editDraft == '     ';
+      disabled = element.$$('.save').hasAttribute('disabled');
+      assert.isTrue(disabled, 'save button should be disabled.');
+
+      var numDiscardEvents = 0;
+      element.addEventListener('gr-diff-comment-discard', function(e) {
+        numDiscardEvents++;
+        if (numDiscardEvents == 2) {
+          done();
+        }
+      });
+      MockInteractions.tap(element.$$('.cancel'));
+      MockInteractions.tap(element.$$('.discard'));
+    });
+
+    test('draft saving/editing', function(done) {
+      element.draft = true;
+      MockInteractions.tap(element.$$('.edit'));
+      element._editDraft = 'good news, everyone!';
+      MockInteractions.tap(element.$$('.save'));
+      assert.isTrue(element.disabled,
+          'Element should be disabled when creating draft.');
+
+      server.respond();
+
+      element._xhrPromise.then(function(req) {
+        assert.isFalse(element.disabled,
+            'Element should be enabled when done creating draft.');
+        assert.equal(req.status, 201);
+        assert.equal(req.url, '/changes/42/revisions/1/drafts');
+        assert.equal(req.response.message, 'created!');
+        assert.isFalse(element.editing);
+      }).then(function() {
+        MockInteractions.tap(element.$$('.edit'));
+        element._editDraft = 'You’ll be delivering a package to Chapek 9, a ' +
+            'world where humans are killed on sight.';
+        MockInteractions.tap(element.$$('.save'));
+        assert.isTrue(element.disabled,
+            'Element should be disabled when updating draft.');
+        server.respond();
+
+        element._xhrPromise.then(function(req) {
+          assert.isFalse(element.disabled,
+              'Element should be enabled when done updating draft.');
+          assert.equal(req.status, 200);
+          assert.equal(req.url,
+              '/changes/42/revisions/1/drafts/baf0414d_40572e03');
+          assert.equal(req.response.message, 'saved!');
+          assert.isFalse(element.editing);
+          done();
+        });
+      });
+    });
+
+    test('proper event fires on done', function(done) {
+      element.addEventListener('gr-diff-comment-done', function(e) {
+        done();
+      });
+      MockInteractions.tap(element.$$('.done'));
+    });
+
+    test('clicking on date link does not trigger nav', function() {
+      var showStub = sinon.stub(page, 'show');
+      var dateEl = element.$$('.date');
+      assert.ok(dateEl);
+      MockInteractions.tap(dateEl);
+      var dest = window.location.pathname + '#5';
+      assert(showStub.lastCall.calledWithExactly(dest, null, false),
+          'Should navigate to ' + dest + ' without triggering nav');
+      showStub.restore();
+    });
   });
 </script>

polygerrit-ui/app/test/gr-diff-view-test.html

@@ -45,7 +45,6 @@ limitations under the License.
   </template>
 </test-fixture>
 
-
 <script>
   // Original diff:
   // Left side (side A):
@@ -101,7 +100,8 @@ limitations under the License.
 
     setup(function() {
       element = fixture('basic');
-      element._maybeRenderDiff({content: diffContent}, [], []);
+      element._renderDiff({content: diffContent}, [], [], [], []);
+      flushAsynchronousOperations();
     });
 
     test('ab content is the same for left and right sides', function() {
@@ -174,16 +174,15 @@ limitations under the License.
         assert.equal(els[i].style.left, '100ch');
       }
     });
-
   });
 
-  suite('comments', function() {
+  suite('comments and drafts', function() {
     var element;
 
-    setup(function() {
+    setup(function(done) {
       element = fixture('comments');
       element._patchNum = 1;
-      element._maybeRenderDiff({content: diffContent}, [], [
+      element._renderDiff({content: diffContent}, [], [
           {
             id: 'file_comment',
             message: 'this is a file comment about the meaninglessness of life',
@@ -197,23 +196,18 @@ limitations under the License.
             message: 'MAKE LIFE TAKE THE LEMONS BACK',
             author: {
               name: 'Cave Johnson',
-          },
             }
-      ]);
+          }
+      ], [], []);
+
+      // On WebKit and Gecko, flushAsynchronousOperations isn't enough to allow
+      // the thread filler elements to properly render. Spin the runloop.
+      element.async(function() {
+        done();
+      }, 1);
     });
 
-    test('comment threads are rendered correctly', function(done) {
+    test('comment threads are rendered correctly', function() {
-      // On WebKit and Gecko, flushAsynchronousOperations isn't enough to allow
-      // the thread filler elements to properly render. Wait for the resize
-      // events that trigger their addition and check after the expected number
-      // come in.
-      var numEventsFired = 0;
-      element.addEventListener('gr-diff-comment-thread-height-changed',
-          function() {
-        numEventsFired++;
-        if (numEventsFired < 2) { return; }
-        assert.equal(numEventsFired, 2);
-
       var threadEls = Polymer.dom(element.root).querySelectorAll(
           'gr-diff-comment-thread[data-thread-id="thread-1-8"]');
       assert.equal(threadEls.length, 1);
@@ -227,9 +221,32 @@ limitations under the License.
       fillerEls = Polymer.dom(element.root).querySelectorAll(
           '.js-threadFiller[data-thread-id="thread-1-FILE"]');
       assert.equal(fillerEls.length, 3);
-
-        done();
     });
+
+    test('tapping a line with an existing thread', function() {
+      var threadEls = Polymer.dom(element.root).querySelectorAll(
+          'gr-diff-comment-thread[data-line-num="8"][data-patch-num="1"]');
+      assert.equal(threadEls.length, 1);
+      var lineEl = element.$$(
+          '.lineNum[data-line-num="8"][data-patch-num="1"]');
+      assert.ok(lineEl);
+      MockInteractions.tap(lineEl);
+      threadEls = Polymer.dom(element.root).querySelectorAll(
+          'gr-diff-comment-thread[data-line-num="8"][data-patch-num="1"]');
+      assert.equal(threadEls.length, 1);
+    });
+
+    test('creating a draft', function() {
+      var threadEls = Polymer.dom(element.root).querySelectorAll(
+          'gr-diff-comment-thread[data-line-num="5"][data-patch-num="1"]');
+      assert.equal(threadEls.length, 0);
+      var lineEl = element.$$(
+          '.lineNum[data-line-num="5"][data-patch-num="1"]');
+      assert.ok(lineEl);
+      MockInteractions.tap(lineEl);
+      threadEls = Polymer.dom(element.root).querySelectorAll(
+          'gr-diff-comment-thread[data-line-num="5"][data-patch-num="1"]');
+      assert.equal(threadEls.length, 1);
     });
   });
 
@@ -242,7 +259,7 @@ limitations under the License.
       for (var i = 0; i < 300; i++) {
         longDiffContent[0].ab.push('');
       }
-      element._maybeRenderDiff({content: longDiffContent}, [], []);
+      element._renderDiff({content: longDiffContent}, [], [], [], []);
     });
 
     function isVisibleInWindow(el) {