opendev/gerrit
Code Issues Proposed changes

Implement comment display/draft authoring

Browse Source 
Some additional cleanup of event names and fixing of the bubbles
property not being placed in the right place.

TODO in a follow-up change:
+ Comments are not considered in context, yet. So if there is a
  comment made in a span of common lines that are hidden, it will
  be there underneath the context control but wont show the code.

Feature: Issue 3649
Change-Id: I8117b243a7d8d529e0c1cea155a4b6b436c0c768
This commit is contained in:
Andrew Bonventre
2015-12-20 19:13:45 -05:00
committed by Dave Borowitz
parent c0a5aeb28e
commit 1b3d9536eb
11 changed files with 758 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/gr-ajax.html
View File

@@ -39,16 +39,12 @@ limitations under the License.
/**
* Fired when a response is received.
* This event does not have a gr- prefix in order to maintain a similar
* API to iron-ajax.
*
* @event response
*/
/**
* Fired when an error is received.
* This event does not have a gr- prefix in order to maintain a similar
* API to iron-ajax.
*
* @event error
*/

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

@@ -25,15 +25,31 @@ limitations under the License.
max-width: 50em;
white-space: normal;
}
gr-diff-comment {
border-left: 1px solid #ddd;
}
gr-diff-comment:first-of-type {
border-top: 1px solid #ddd;
}
gr-diff-comment:last-of-type {
border-bottom: 1px solid #ddd;
}
</style>
<template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment">
<gr-diff-comment
comment="{{comment}}"
change-num="[[changeNum]]"
patch-num="[[patchNum]]"
draft="[[comment.__draft]]"
editing="[[!comment.message]]"></gr-diff-comment>
</template>
<div id="container">
<template id="commentList" is="dom-repeat" items="{{_orderedComments}}" as="comment">
<gr-diff-comment
comment="{{comment}}"
change-num="[[changeNum]]"
patch-num="[[patchNum]]"
draft="[[comment.__draft]]"
editing="[[!comment.message]]"
show-actions="[[showActions]]"
on-height-change="_handleCommentHeightChange"
on-reply="_handleCommentReply"
on-discard="_handleCommentDiscard"
on-done="_handleCommentDone"></gr-diff-comment>
</template>
</div>
</template>
<script>
(function() {
@@ -46,13 +62,13 @@ limitations under the License.
/**
* Fired when the height of the thread changes.
*
* @event gr-diff-comment-thread-height-changed
* @event height-change
*/
/**
* Fired when the thread should be discarded.
*
* @event gr-diff-comment-thread-discard
* @event discard
*/
properties: {
@@ -63,17 +79,15 @@ limitations under the License.
observer: '_commentsChanged',
},
patchNum: String,
path: String,
showActions: Boolean,
_lastHeight: Number,
_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));
get naturalHeight() {
return this.$.container.offsetHeight;
},
_commentsChanged: function(comments) {
@@ -82,7 +96,9 @@ limitations under the License.
_sortedComments: function(comments) {
comments.sort(function(c1, c2) {
return util.parseDate(c1.updated) - util.parseDate(c2.updated);
var c1Date = c1.__date || util.parseDate(c1.updated);
var c2Date = c2.__date || util.parseDate(c2.updated);
return c1Date - c2Date;
});
var commentIDToReplies = {};
@@ -116,19 +132,60 @@ limitations under the License.
},
_handleCommentHeightChange: function(e) {
// TODO: This fires for each comment on initialization. Optimize to only
// fire the top level "thread height has changed" event once during
// initial DOM stamp.
this.fire('gr-diff-comment-thread-height-changed',
{height: this.offsetHeight});
e.stopPropagation();
this._heightChanged();
},
_handleCommentReply: function(e) {
console.log('should add reply...')
var comment = e.detail.comment;
this.push('comments',
this._newReply(comment.id, comment.line, this.path));
this.async(this._heightChanged.bind(this), 1);
},
_handleCommentDone: function(e) {
var comment = e.detail.comment;
var reply = this._newReply(comment.id, comment.line, this.path, 'Done');
this.push('comments', reply);
// Allow the reply to render in the dom-repeat.
this.async(function() {
var commentEls =
Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
for (var i = 0; i < commentEls.length; i++) {
if (commentEls[i].comment.__draftID == reply.__draftID) {
commentEls[i].save();
return;
}
}
this._heightChanged();
}.bind(this), 1);
},
_newReply: function(inReplyTo, line, path, opt_message) {
// TODO(andybons): These ad-hoc properties should be masked by a
// lightweight model class:
// https://code.google.com/p/gerrit/issues/detail?id=3729
var c = {
__draft: true,
__draftID: Math.random().toString(36),
__date: new Date(),
line: line,
path: path,
in_reply_to: inReplyTo,
};
if (opt_message != null) {
c.message = opt_message;
}
return c;
},
_handleCommentDiscard: function(e) {
var diffCommentEl = e.target;
// TODO(andybons): In Shadow DOM, the event bubbles up, while in Shady
// DOM, it respects the bubbles property.
// https://github.com/Polymer/polymer/issues/3226
e.stopPropagation();
var diffCommentEl = Polymer.dom(e).rootTarget;
var idx = this._indexOf(diffCommentEl.comment, this.comments);
if (idx == -1) {
throw Error('Cannot find comment ' +
@@ -136,11 +193,19 @@ limitations under the License.
}
this.comments.splice(idx, 1);
this._commentsChanged(this.comments);
if (this.comments.length == 0 && this.parentNode) {
this.parentNode.removeChild(this);
if (this.comments.length == 0) {
this.fire('discard', null, {bubbles: false});
return;
}
this.fire('gr-diff-comment-thread-height-changed',
{height: this.offsetHeight});
this.async(this._heightChanged.bind(this), 1);
},
_heightChanged: function() {
var height = this.$.container.offsetHeight;
if (height == this._lastHeight) { return; }
this.fire('height-change', {height: height}, {bubbles: false});
this._lastHeight = height;
},
_indexOf: function(comment, arr) {

76
polygerrit-ui/app/elements/gr-diff-comment.html
View File

@@ -23,7 +23,7 @@ limitations under the License.
<template>
<style>
:host {
border: 1px solid #ddd;
background-color: #ffd;
display: block;
}
:host([disabled]) {
@@ -38,8 +38,8 @@ limitations under the License.
padding: .5em .7em;
}
.header {
background-color: #eee;
display: flex;
padding-bottom: 0;
font-family: 'Open Sans', sans-serif;
}
.headerLeft {
@@ -113,6 +113,7 @@ limitations under the License.
display: none;
}
.editing .editMessage {
background-color: #fff;
display: block;
}
</style>
@@ -134,7 +135,7 @@ limitations under the License.
max-rows="4"
bind-value="{{_editDraft}}"></iron-autogrow-textarea>
<div class="message">[[comment.message]]</div>
<div class="actions">
<div class="actions" hidden$="[[!showActions]]">
<a class="action reply" href="#" on-tap="_handleReply">Reply</a>
<a class="action done" href="#" on-tap="_handleDone">Done</a>
<a class="action edit" href="#" on-tap="_handleEdit">Edit</a>
@@ -158,25 +159,25 @@ limitations under the License.
/**
* Fired when the height of the comment changes.
*
* @event gr-diff-comment-height-changed
* @event height-change
*/
/**
* Fired when the Reply action is triggered.
*
* @event gr-diff-comment-reply
* @event reply
*/
/**
* Fired when the Done action is triggered.
*
* @event gr-diff-comment-done
* @event done
*/
/**
* Fired when this comment is discarded.
*
* @event gr-diff-comment-discard
* @event discard
*/
properties: {
@@ -201,19 +202,46 @@ limitations under the License.
observer: '_editingChanged',
},
patchNum: String,
showActions: Boolean,
_xhrPromise: Object, // Used for testing.
_editDraft: String,
},
ready: function() {
this._editDraft = (this.comment && this.comment.message) || '';
},
attached: function() {
this._heightChanged();
},
save: function() {
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));
},
_heightChanged: function() {
this.async(function() {
this.fire('gr-diff-comment-height-changed',
{height: this.offsetHeight});
this.fire('height-change', {height: this.offsetHeight},
{bubbles: false});
}.bind(this));
},
@@ -252,12 +280,12 @@ limitations under the License.
_handleReply: function(e) {
e.preventDefault();
this.fire('gr-diff-comment-reply');
this.fire('reply', {comment: this.comment}, {bubbles: false});
},
_handleDone: function(e) {
e.preventDefault();
this.fire('gr-diff-comment-done');
this.fire('done', {comment: this.comment}, {bubbles: false});
},
_handleEdit: function(e) {
@@ -268,31 +296,13 @@ limitations under the License.
_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));
this.save();
},
_handleCancel: function(e) {
e.preventDefault();
if (this.comment.message == null || this.comment.message.length == 0) {
this.fire('gr-diff-comment-discard');
this.fire('discard', null, {bubbles: false});
return;
}
this._editDraft = this.comment.message;
@@ -307,11 +317,11 @@ limitations under the License.
this.disabled = true;
var commentID = this.comment.id;
if (!commentID) {
this.fire('gr-diff-comment-discard');
this.fire('discard', null, {bubbles: false});
return;
}
this._send('DELETE', this._restEndpoint(commentID)).then(function(req) {
this.fire('gr-diff-comment-discard', this.comment);
this.fire('discard', null, {bubbles: false});
}.bind(this)).catch(function(err) {
alert('Your draft couldn’t be deleted. Check the console and ' +
'contact the PolyGerrit team for assistance.');

181
polygerrit-ui/app/elements/gr-diff-side.html
View File

@@ -15,6 +15,7 @@ limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="gr-diff-comment-thread.html">
<dom-module id="gr-diff-side">
<template>
@@ -111,6 +112,24 @@ limitations under the License.
* @event expand-context
*/
/**
* Fired when a thread's height is changed.
*
* @event thread-height-change
*/
/**
* Fired when a draft should be added.
*
* @event add-draft
*/
/**
* Fired when a thread is removed.
*
* @event remove-thread
*/
properties: {
canComment: {
type: Boolean,
@@ -125,6 +144,9 @@ limitations under the License.
type: Number,
observer: '_widthChanged',
},
changeNum: String,
patchNum: String,
path: String,
_lineFeedHTML: {
type: String,
@@ -143,6 +165,33 @@ limitations under the License.
},
},
listeners: {
'tap': '_tapHandler',
},
rowInserted: function(index) {
this.renderLineIndexRange(index, index);
this._updateDOMIndices();
},
rowRemoved: function(index) {
var removedEls = Polymer.dom(this.root).querySelectorAll(
'[data-index="' + index + '"]');
for (var i = 0; i < removedEls.length; i++) {
removedEls[i].parentNode.removeChild(removedEls[i]);
}
this._updateDOMIndices();
},
rowUpdated: function(index) {
var removedEls = Polymer.dom(this.root).querySelectorAll(
'[data-index="' + index + '"]');
for (var i = 0; i < removedEls.length; i++) {
removedEls[i].parentNode.removeChild(removedEls[i]);
}
this.renderLineIndexRange(index, index);
},
scrollToLine: function(lineNum) {
if (isNaN(lineNum) || lineNum < 1) { return; }
@@ -176,6 +225,70 @@ limitations under the License.
}
},
getRowHeight: function(index) {
var row = this.content[index];
// Filler elements should not be taken into account when determining
// height calculations.
if (row.type == 'FILLER') {
return 0;
}
if (row.height != null) {
return row.height;
}
var selector = '[data-index="' + index + '"]'
var els = Polymer.dom(this.root).querySelectorAll(selector);
if (els.length != 2) {
throw Error('Rows should only consist of two elements');
}
return Math.max(els[0].offsetHeight, els[1].offsetHeight);
},
getRowNaturalHeight: function(index) {
var contentEl = this.$$('.content [data-index="' + index + '"]');
return contentEl.naturalHeight || contentEl.offsetHeight;
},
setRowNaturalHeight: function(index) {
var lineEl = this.$$('.numbers [data-index="' + index + '"]');
var contentEl = this.$$('.content [data-index="' + index + '"]');
contentEl.style.height = null;
var height = contentEl.offsetHeight;
lineEl.style.height = height + 'px';
this.content[index].height = height;
return height;
},
setRowHeight: function(index, height) {
var selector = '[data-index="' + index + '"]';
var els = Polymer.dom(this.root).querySelectorAll(selector);
for (var i = 0; i < els.length; i++) {
els[i].style.height = height + 'px';
}
this.content[index].height = height;
},
_updateDOMIndices: function() {
// There is no way to select elements with a data-index greater than a
// given value. For now, just update all DOM elements.
var lineEls = Polymer.dom(this.root).querySelectorAll(
'.numbers [data-index]');
var contentEls = Polymer.dom(this.root).querySelectorAll(
'.content [data-index]');
if (lineEls.length != contentEls.length) {
throw Error(
'There must be the same number of line and content elements');
}
var index = 0;
for (var i = 0; i < this.content.length; i++) {
if (this.content[i].hidden) { continue; }
lineEls[index].setAttribute('data-index', i);
contentEls[index].setAttribute('data-index', i);
index++;
}
},
_widthChanged: function(width) {
this.$.content.style.width = width + 'ch';
},
@@ -190,6 +303,21 @@ limitations under the License.
return 'container' + (canComment ? ' canComment' : '');
},
_tapHandler: function(e) {
var lineEl = Polymer.dom(e).rootTarget;
if (!this.canComment || !lineEl.classList.contains('lineNum')) {
return;
}
e.preventDefault();
var index = parseInt(lineEl.getAttribute('data-index'), 10);
var line = parseInt(lineEl.getAttribute('data-line-num'), 10);
this.fire('add-draft', {
index: index,
line: line
}, {bubbles: false});
},
_clearChildren: function(el) {
while (el.firstChild) {
el.removeChild(el.firstChild);
@@ -198,7 +326,7 @@ limitations under the License.
_handleContextControlClick: function(context, e) {
e.preventDefault();
this.fire('expand-context', {bubbles: false, context: context});
this.fire('expand-context', {context: context}, {bubbles: false});
},
_render: function(diff, startIndex, endIndex) {
@@ -207,6 +335,14 @@ limitations under the License.
if (endIndex != diff.length - 1) {
beforeLineEl = this.$$('.numbers [data-index="' + endIndex + '"]');
beforeContentEl = this.$$('.content [data-index="' + endIndex + '"]');
if (!beforeLineEl && !beforeContentEl) {
// `endIndex` may be present within the model, but not in the DOM.
// Insert it before its successive element.
beforeLineEl = this.$$(
'.numbers [data-index="' + (endIndex + 1) + '"]');
beforeContentEl = this.$$(
'.content [data-index="' + (endIndex + 1) + '"]');
}
}
for (var i = startIndex; i <= endIndex; i++) {
@@ -223,10 +359,53 @@ limitations under the License.
this._renderContextControl(diff[i], i, beforeLineEl,
beforeContentEl);
break;
case 'COMMENT_THREAD':
this._renderCommentThread(diff[i], i, beforeLineEl,
beforeContentEl);
break;
}
}
},
_handleCommentThreadHeightChange: function(e) {
var threadEl = Polymer.dom(e).rootTarget;
var index = parseInt(threadEl.getAttribute('data-index'), 10);
this.content[index].height = e.detail.height;
var lineEl = this.$$('.numbers [data-index="' + index + '"]');
lineEl.style.height = e.detail.height + 'px';
this.fire('thread-height-change', {
index: index,
height: e.detail.height,
}, {bubbles: false});
},
_handleCommentThreadDiscard: function(e) {
var threadEl = Polymer.dom(e).rootTarget;
var index = parseInt(threadEl.getAttribute('data-index'), 10);
this.fire('remove-thread', {index: index}, {bubbles: false});
},
_renderCommentThread: function(thread, index, beforeLineEl,
beforeContentEl) {
var lineEl = this._createElement('div', 'commentThread');
lineEl.classList.add('filler');
lineEl.setAttribute('data-index', index);
var threadEl = document.createElement('gr-diff-comment-thread');
threadEl.addEventListener('height-change',
this._handleCommentThreadHeightChange.bind(this));
threadEl.addEventListener('discard',
this._handleCommentThreadDiscard.bind(this));
threadEl.setAttribute('data-index', index);
threadEl.changeNum = this.changeNum;
threadEl.patchNum = this.patchNum;
threadEl.path = this.path;
threadEl.comments = thread.comments;
threadEl.showActions = this.canComment;
this.$.numbers.insertBefore(lineEl, beforeLineEl);
this.$.content.insertBefore(threadEl, beforeContentEl);
},
_renderContextControl: function(control, index, beforeLineEl,
beforeContentEl) {
var lineEl = this._createElement('div', 'contextControl');

170
polygerrit-ui/app/elements/gr-diff.html
View File

@@ -84,15 +84,27 @@ limitations under the License.
<div class="diffContainer">
<gr-diff-side id="leftDiff"
change-num="[[changeNum]]"
patch-num="[[patchRange.basePatchNum]]"
path="[[path]]"
content="{{_diff.leftSide}}"
width="[[sideWidth]]"
can-comment="[[_loggedIn]]"
on-expand-context="_handleExpandContext"></gr-diff-side>
on-expand-context="_handleExpandContext"
on-thread-height-change="_handleThreadHeightChange"
on-add-draft="_handleAddDraft"
on-remove-thread="_handleRemoveThread"></gr-diff-side>
<gr-diff-side id="rightDiff"
change-num="[[changeNum]]"
patch-num="[[patchRange.patchNum]]"
path="[[path]]"
content="{{_diff.rightSide}}"
width="[[sideWidth]]"
can-comment="[[_loggedIn]]"
on-expand-context="_handleExpandContext"></gr-diff-side>
on-expand-context="_handleExpandContext"
on-thread-height-change="_handleThreadHeightChange"
on-add-draft="_handleAddDraft"
on-remove-thread="_handleRemoveThread"></gr-diff-side>
</div>
</template>
<script>
@@ -126,15 +138,29 @@ limitations under the License.
type: Number,
value: 80,
},
_comments: Array,
_context: {
type: Number,
value: 10,
observer: '_contextChanged',
},
_baseComments: Array,
_comments: Array,
_drafts: Array,
_baseDrafts: Array,
/**
* Base (left side) comments and drafts grouped by line number.
*/
_groupedBaseComments: {
type: Object,
value: function() { return {}; },
},
/**
* Comments and drafts (right side) grouped by line number.
*/
_groupedComments: {
type: Object,
value: function() { return {}; },
},
_diffResponse: Object,
_diff: {
type: Object,
@@ -191,11 +217,12 @@ limitations under the License.
},
_render: function() {
this._groupCommentsAndDrafts();
this._processContent();
// Allow for the initial rendering to complete before firing the event.
this.async(function() {
this.fire('render', {bubbles: false});
this.fire('render', null, {bubbles: false});
}.bind(this), 1);
},
@@ -274,10 +301,11 @@ limitations under the License.
},
_handleContextSelectChange: function(e) {
if (e.target.value == 'ALL') {
var selectEl = Polymer.dom(e).rootTarget;
if (selectEl.value == 'ALL') {
this._context = Infinity;
} else {
this._context = parseInt(e.target.value, 10);
this._context = parseInt(selectEl.value, 10);
}
this._render();
},
@@ -303,6 +331,84 @@ limitations under the License.
this.$.rightDiff.renderLineIndexRange(ctx.start, ctx.end);
},
_handleThreadHeightChange: function(e) {
var index = e.detail.index;
var diffEl = Polymer.dom(e).rootTarget;
var otherSide = diffEl == this.$.leftDiff ?
this.$.rightDiff : this.$.leftDiff;
var threadHeight = e.detail.height;
var otherSideHeight;
if (otherSide.content[index].type == 'COMMENT_THREAD') {
otherSideHeight = otherSide.getRowNaturalHeight(index);
} else {
otherSideHeight = otherSide.getRowHeight(index);
}
var maxHeight = Math.max(threadHeight, otherSideHeight);
this.$.leftDiff.setRowHeight(index, maxHeight);
this.$.rightDiff.setRowHeight(index, maxHeight);
},
_handleAddDraft: function(e) {
var insertIndex = e.detail.index + 1;
var diffEl = Polymer.dom(e).rootTarget;
var content = diffEl.content;
if (content[insertIndex] &&
content[insertIndex].type == 'COMMENT_THREAD') {
// A thread is already here. Do nothing.
return;
}
var comment = {
type: 'COMMENT_THREAD',
comments: [{
__draft: true,
__draftID: Math.random().toString(36),
line: e.detail.line,
path: this.path,
}]
};
if (content[insertIndex] &&
content[insertIndex].type == 'FILLER') {
content[insertIndex] = comment;
diffEl.rowUpdated(insertIndex);
} else {
content.splice(insertIndex, 0, comment);
diffEl.rowInserted(insertIndex);
}
var otherSide = diffEl == this.$.leftDiff ?
this.$.rightDiff : this.$.leftDiff;
if (otherSide.content[insertIndex] == null ||
otherSide.content[insertIndex].type != 'COMMENT_THREAD') {
otherSide.content.splice(insertIndex, 0, {
type: 'FILLER',
});
otherSide.rowInserted(insertIndex);
}
},
_handleRemoveThread: function(e) {
var diffEl = Polymer.dom(e).rootTarget;
var otherSide = diffEl == this.$.leftDiff ?
this.$.rightDiff : this.$.leftDiff;
var index = e.detail.index;
if (otherSide.content[index].type == 'FILLER') {
otherSide.content.splice(index, 1);
otherSide.rowRemoved(index);
diffEl.content.splice(index, 1);
diffEl.rowRemoved(index);
} else if (otherSide.content[index].type == 'COMMENT_THREAD') {
diffEl.content[index] = {type: 'FILLER'};
diffEl.rowUpdated(index);
var height = otherSide.setRowNaturalHeight(index);
diffEl.setRowHeight(index, height);
} else {
throw Error('A thread cannot be opposite anything but filler or ' +
'another thread');
}
},
_processContent: function() {
var leftSide = [];
var rightSide = [];
@@ -329,12 +435,38 @@ limitations under the License.
ctx.diffChunkIndex = i;
this._addDiffChunk(ctx, content[i], leftSide, rightSide);
}
this._diff = {
leftSide: leftSide,
rightSide: rightSide,
};
},
_groupCommentsAndDrafts: function() {
this._baseDrafts.forEach(function(d) { d.__draft = true; });
this._drafts.forEach(function(d) { d.__draft = true; });
var allLeft = this._baseComments.concat(this._baseDrafts);
var allRight = this._comments.concat(this._drafts);
var leftByLine = {};
var rightByLine = {};
var mapFunc = function(byLine) {
return function(c) {
// File comments/drafts are grouped with line 1 for now.
var line = c.line || 1;
if (byLine[line] == null) {
byLine[line] = [];
}
byLine[line].push(c);
}
};
allLeft.forEach(mapFunc(leftByLine));
allRight.forEach(mapFunc(rightByLine));
this._groupedBaseComments = leftByLine;
this._groupedComments = rightByLine;
},
_addContextControl: function(ctx, leftSide, rightSide) {
var numLinesHidden = ctx.lastNumLinesHidden;
var leftStart = leftSide.length - numLinesHidden;
@@ -390,6 +522,8 @@ limitations under the License.
numLines: numLines,
lineNum: ++ctx.right.lineNum,
});
this._addCommentsIfPresent(ctx, leftSide, rightSide);
}
if (ctx.lastNumLinesHidden > 0) {
this._addContextControl(ctx, leftSide, rightSide);
@@ -456,9 +590,33 @@ limitations under the License.
numLines: maxNumLines,
});
}
this._addCommentsIfPresent(ctx, leftSide, rightSide);
}
},
_addCommentsIfPresent: function(ctx, leftSide, rightSide) {
var leftComments = this._groupedBaseComments[ctx.left.lineNum];
var rightComments = this._groupedComments[ctx.right.lineNum];
if (leftComments) {
leftSide.push({
type: 'COMMENT_THREAD',
comments: leftComments,
});
}
if (rightComments) {
rightSide.push({
type: 'COMMENT_THREAD',
comments: rightComments,
});
}
if (leftComments && !rightComments) {
rightSide.push({ type: 'FILLER' });
} else if (!leftComments && rightComments) {
leftSide.push({ type: 'FILLER' });
}
delete(this._groupedBaseComments[ctx.left.lineNum]);
delete(this._groupedComments[ctx.right.lineNum]);
},
// The `highlights` array consists of a list of <skip length, mark length>
// pairs, where the skip length is the number of characters between the

5
polygerrit-ui/app/elements/gr-file-list.html
View File

@@ -115,10 +115,7 @@ limitations under the License.
},
_computeCommentsString: function(comments, patchNum, path) {
var patchComments = (comments[path] || []).filter(function(c) {
return c.patch_set == patchNum;
});
var num = patchComments.length;
var num = (comments[path] || []).length;
if (num == 0) { return ''; }
if (num == 1) { return '1 comment'; }
if (num > 1) { return num + ' comments'; };

2
polygerrit-ui/app/elements/gr-reply-dropdown.html
View File

@@ -289,7 +289,7 @@ limitations under the License.
}
this.disabled = true;
this._send(obj).then(function(req) {
this.fire('send', {bubbles: false});
this.fire('send', null, {bubbles: false});
this._draft = '';
this.disabled = false;
this.$.dropdown.close();

20
polygerrit-ui/app/test/gr-diff-comment-test.html
View File

@@ -52,18 +52,18 @@ limitations under the License.
line: 5,
message: 'is this a crossover episode!?',
updated: '2015-12-08 19:48:33.843000000',
}
};
});
test('proper event fires on reply', function(done) {
element.addEventListener('gr-diff-comment-reply', function(e) {
element.addEventListener('reply', function(e) {
done();
});
MockInteractions.tap(element.$$('.reply'));
});
test('proper event fires on done', function(done) {
element.addEventListener('gr-diff-comment-done', function(e) {
element.addEventListener('done', function(e) {
done();
});
MockInteractions.tap(element.$$('.done'));
@@ -140,6 +140,11 @@ limitations under the License.
}
test('button visibility states', function() {
element.showActions = false;
assert.isTrue(element.$$('.actions').hasAttribute('hidden'));
element.showActions = true;
assert.isFalse(element.$$('.actions').hasAttribute('hidden'));
element.draft = true;
assert.isTrue(isVisible(element.$$('.edit')), 'edit is visible');
assert.isTrue(isVisible(element.$$('.discard')), 'discard is visible');
@@ -183,7 +188,7 @@ limitations under the License.
assert.isTrue(disabled, 'save button should be disabled.');
var numDiscardEvents = 0;
element.addEventListener('gr-diff-comment-discard', function(e) {
element.addEventListener('discard', function(e) {
numDiscardEvents++;
if (numDiscardEvents == 2) {
done();
@@ -232,13 +237,6 @@ limitations under the License.
});
});
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');

112
polygerrit-ui/app/test/gr-diff-comment-thread-test.html
View File

@@ -31,6 +31,12 @@ limitations under the License.
</template>
</test-fixture>
<test-fixture id="withComment">
<template>
<gr-diff-comment-thread></gr-diff-comment-thread>
</template>
</test-fixture>
<script>
suite('gr-diff-comment-thread tests', function() {
var element;
@@ -111,6 +117,112 @@ limitations under the License.
}
]);
});
});
suite('comment action tests', function() {
var element;
var server;
setup(function() {
element = fixture('withComment');
element.comments = [{
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',
}];
flushAsynchronousOperations();
server = sinon.fakeServer.create();
// Eat any requests made by elements in this suite.
server.respondWith(
'PUT',
'/changes/41/1/drafts',
[
201,
{ 'Content-Type': 'application/json' },
')]}\'\n' + JSON.stringify({
id: "7afa4931_de3d65bd",
path: '/path/to/file.txt',
line: 5,
in_reply_to: 'baf0414d_60047215',
updated: '2015-12-21 02:01:10.850000000',
message: 'Done'
}),
]
);
server.respondWith(
'DELETE',
'/changes/41/1/drafts/baf0414d_60047215',
[
204,
{},
'',
]
);
});
test('reply', function(done) {
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
commentEl.addEventListener('reply', function() {
var drafts = element.comments.filter(function(c) {
return c.__draft == true;
});
assert.equal(drafts.length, 1);
assert.notOk(drafts[0].message, 'message should be empty');
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
commentEl.fire('reply', {comment: commentEl.comment}, {bubbles: false});
});
test('done', function(done) {
element.changeNum = '42';
element.patchNum = '1';
var commentEl = element.$$('gr-diff-comment');
assert.ok(commentEl);
commentEl.addEventListener('done', function() {
server.respond();
var drafts = element.comments.filter(function(c) {
return c.__draft == true;
});
assert.equal(drafts.length, 1);
assert.equal(drafts[0].message, 'Done');
assert.equal(drafts[0].in_reply_to, 'baf0414d_60047215');
done();
});
commentEl.fire('done', {comment: commentEl.comment}, {bubbles: false});
});
test('discard', function(done) {
element.changeNum = '42';
element.patchNum = '1';
element.comments.push(element._newReply(
element.comments[0].id,
element.comments[0].line,
element.comments[0].path,
'it’s pronouced jiff, not giff'));
element._commentsChanged(element.comments);
flushAsynchronousOperations();
var draftEl =
Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
assert.ok(draftEl);
draftEl.addEventListener('discard', function() {
server.respond();
var drafts = element.comments.filter(function(c) {
return c.__draft == true;
});
assert.equal(drafts.length, 0);
done();
});
draftEl.fire('discard', null, {bubbles: false});
});
});
</script>

21
polygerrit-ui/app/test/gr-diff-side-test.html
View File

@@ -183,5 +183,26 @@ limitations under the License.
// Nine lines should now be present in the DOM.
assert.equal(codeLineEls.length, 9 * 2);
});
test('tap line to add a draft', function() {
var numAddDraftEvents = 0;
var fireMock = sinon.stub(element, 'fire', function(eventName) {
if (eventName == 'add-draft') {
numAddDraftEvents++;
}
});
element.content = [{type: 'CODE', content: '<!DOCTYPE html>'}];
element.canComment = false;
flushAsynchronousOperations();
var lineEl = element.$$('.lineNum');
assert.ok(lineEl);
MockInteractions.tap(lineEl);
assert.equal(numAddDraftEvents, 0);
element.canComment = true;
MockInteractions.tap(lineEl);
assert.equal(numAddDraftEvents, 1);
});
});
</script>

136
polygerrit-ui/app/test/gr-diff-test.html
View File

@@ -33,6 +33,12 @@ limitations under the License.
</template>
</test-fixture>
<test-fixture id="comments">
<template>
<gr-diff></gr-diff>
</template>
</test-fixture>
<script>
suite('gr-diff tests', function() {
var element;
@@ -54,7 +60,39 @@ limitations under the License.
JSON.stringify({
change_type: 'MODIFIED',
content: [
{ab: ['doin some codez and stuffs']},
{
ab: [
'<!DOCTYPE html>',
'<meta charset="utf-8">',
'<title>My great page</title>',
'<style>',
' *,',
' *:before,',
' *:after {',
' box-sizing: border-box;',
' }',
'</style>',
'<header>',
]
},
{
a: [
' Welcome ',
' to the wooorld of tomorrow!',
],
b: [
' Hello, world!',
],
},
{
ab: [
'</header>',
'<body>',
'Leela: This is the only place the ship can’t hear us, so ',
'everyone pretend to shower.',
'Fry: Same as every day. Got it.',
]
},
]
}),
]
@@ -113,7 +151,7 @@ limitations under the License.
email: 'dborowitz@google.com',
},
id: '001a2067_f30f3048',
line: 17,
line: 12,
message: 'What on earth are you thinking, here?',
updated: '2015-12-12 02:51:37.973000000',
},
@@ -138,7 +176,7 @@ limitations under the License.
},
id: 'a0407443_30dfe8fb',
in_reply_to: '001a2067_f30f3048',
line: 17,
line: 12,
message: '¯\\_(ツ)_/¯',
updated: '2015-12-12 18:50:21.627000000',
},
@@ -186,6 +224,96 @@ limitations under the License.
});
});
test('comment rendering', function(done) {
element._context = Infinity;
element._loggedIn = true;
element.patchRange = {
basePatchNum: 1,
patchNum: 2,
};
server.respond();
// Allow events to fire and the threads to render.
element.async(function() {
var leftThreadEls =
Polymer.dom(element.$.leftDiff.root).querySelectorAll(
'gr-diff-comment-thread');
assert.equal(leftThreadEls.length, 1);
assert.equal(leftThreadEls[0].comments.length, 1);
var rightThreadEls =
Polymer.dom(element.$.rightDiff.root).querySelectorAll(
'gr-diff-comment-thread');
assert.equal(rightThreadEls.length, 1);
assert.equal(rightThreadEls[0].comments.length, 2);
var index = leftThreadEls[0].getAttribute('data-index');
var leftFillerEls =
Polymer.dom(element.$.leftDiff.root).querySelectorAll(
'.commentThread.filler[data-index="' + index + '"]');
assert.equal(leftFillerEls.length, 1);
var rightFillerEls =
Polymer.dom(element.$.rightDiff.root).querySelectorAll(
'[data-index="' + index + '"]');
assert.equal(rightFillerEls.length, 2);
for (var i = 0; i < rightFillerEls.length; i++) {
assert.isTrue(rightFillerEls[i].classList.contains('filler'));
}
var originalHeight = rightFillerEls[0].offsetHeight;
assert.equal(rightFillerEls[1].offsetHeight, originalHeight);
assert.equal(leftThreadEls[0].offsetHeight, originalHeight);
assert.equal(leftFillerEls[0].offsetHeight, originalHeight);
// Create a comment on the opposite side of the first comment.
var rightLineEL = element.$.rightDiff.$$(
'.lineNum[data-index="' + (index - 1) + '"');
assert.ok(rightLineEL);
MockInteractions.tap(rightLineEL);
element.async(function() {
var newThreadEls =
Polymer.dom(element.$.rightDiff.root).querySelectorAll(
'[data-index="' + index + '"]');
assert.equal(newThreadEls.length, 2);
for (var i = 0; i < newThreadEls.length; i++) {
assert.isTrue(
newThreadEls[i].classList.contains('commentThread') ||
newThreadEls[i].tagName == 'GR-DIFF-COMMENT-THREAD');
}
var newHeight = newThreadEls[0].offsetHeight
assert.equal(newThreadEls[1].offsetHeight, newHeight);
assert.equal(leftFillerEls[0].offsetHeight, newHeight);
assert.equal(leftThreadEls[0].offsetHeight, newHeight);
// The editing mode height of the right comment will be greater than
// the non-editing mode height of the left comment.
assert.isAbove(newHeight, originalHeight);
// Discard the right thread and ensure the left comment heights are
// back to their original values.
newThreadEls[1].addEventListener('discard', function() {
rightFillerEls =
Polymer.dom(element.$.rightDiff.root).querySelectorAll(
'[data-index="' + index + '"]');
assert.equal(rightFillerEls.length, 2);
for (var i = 0; i < rightFillerEls.length; i++) {
assert.isTrue(rightFillerEls[i].classList.contains('filler'));
}
var originalHeight = rightFillerEls[0].offsetHeight;
assert.equal(rightFillerEls[1].offsetHeight, originalHeight);
assert.equal(leftThreadEls[0].offsetHeight, originalHeight);
assert.equal(leftFillerEls[0].offsetHeight, originalHeight);
done()
});
var commentEl = newThreadEls[1].$$('gr-diff-comment');
commentEl.fire('discard', null, {bubbles: false});
}, 1);
}, 1);
}),
test('intraline normalization', function() {
// The content and highlights are in the format returned by the Gerrit
// REST API.
@@ -341,4 +469,6 @@ limitations under the License.
assert.equal(leftContext.end, 19);
});
});
</script>