Ranged comments integration
- gr-file-list recognizes local preferences (for hasRangedComments flag)
- gr-file-list reacts to cursor hotkey only if there is no range
selected (currently always false).
- Remove dead code from GrDiffBuilderSideBySide, GrDiffBuilder,
gr-diff-builder.html
- Bugfix: GrDiffBuilder.prototype.getGroupsByLineRange handles one-line
BOTH code sections correctly. Test updated as well.
- Added utitily methods added to gr-diff-builder.html to reduce
dependency on DOM structure and reduce amount of code copy-pasting:
- renderLineRange, getContentByLine, etc
- For gr-diff.js and gr-diff-comment-thread.js addDraft renamed to
addOrEditDraft because that's what it does.
- For both, addDraft method always creates a draft comment.
- Added support for ranged comments in gr-diff, gr-diff-comment-thread.
- Added mouseenter and mouseout events to gr-comment.js
- Refactored gr-comment.js to reduce code copy-paste, unify event
payload, and to eliminate need of accessing component instance for
patchNum. Tests updated as well.
- Refactored gr-diff.js UI data model update using gr-diff-builder.html
utility methods to make code more readable.
- Added support for creating ranged comments to gr-diff.js.
- gr-selection-action-box now reacts to click and tap to create a
comment.
Change-Id: I01480a4c6f460774a8b2826915702800b3f81d25
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index 7e8779f..8f1b6b6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -20,8 +20,7 @@
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
- GrDiffBuilderSideBySide.prototype.buildSectionElement = function(group,
- opt_beforeSection) {
+ GrDiffBuilderSideBySide.prototype.buildSectionElement = function(group) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
var pairs = group.getSideBySidePairs();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index d9d6c8a..005947c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -77,9 +77,84 @@
return null;
},
+ renderLineRange: function(startLine, endLine, opt_side) {
+ var groups =
+ this._builder.getGroupsByLineRange(startLine, endLine, opt_side);
+ groups.forEach(function(group) {
+ var newElement = this._builder.buildSectionElement(group);
+ var oldElement = group.element;
+
+ // Transfer comment threads from existing section to new one.
+ var threads = Polymer.dom(newElement).querySelectorAll(
+ 'gr-diff-comment-thread');
+ threads.forEach(function(threadEl) {
+ var lineEl = this.getLineElByChild(threadEl, oldElement);
+ if (!lineEl) { // New comment thread.
+ return;
+ }
+ var side = this.getSideByLineEl(lineEl);
+ var line = lineEl.getAttribute('data-value');
+ var oldThreadEl =
+ this.getCommentThreadByLine(line, side, oldElement);
+ threadEl.parentNode.replaceChild(oldThreadEl, threadEl);
+ }, this);
+
+ // Replace old group elements with new ones.
+ group.element.parentNode.replaceChild(newElement, group.element);
+ group.element = newElement;
+ }, this);
+
+ this.async(function() {
+ this.fire('render');
+ }, 1);
+ },
+
+ getContentByLine: function(lineNumber, opt_side, opt_root) {
+ var root = Polymer.dom(opt_root || this.diffElement);
+ var sideSelector = !!opt_side ? ('.' + opt_side) : '';
+ return root.querySelector('td.lineNum[data-value="' + lineNumber +
+ '"]' + sideSelector + ' ~ td.content');
+ },
+
+ getContentByLineEl: function(lineEl) {
+ var root = Polymer.dom(lineEl.parentElement);
+ var side = this.getSideByLineEl(lineEl);
+ var line = lineEl.getAttribute('data-value');
+ return this.getContentByLine(line, side, root);
+ },
+
+ getLineElByNumber: function(lineNumber, opt_side) {
+ var sideSelector = !!opt_side ? ('.' + opt_side) : '';
+ return this.diffElement.querySelector(
+ '.lineNum[data-value="' + lineNumber + '"]' + sideSelector);
+ },
+
+ getContentsByLineRange: function(startLine, endLine, opt_side) {
+ var groups =
+ this._builder.getGroupsByLineRange(startLine, endLine, opt_side);
+ // In each group, search Element for lines in range.
+ return groups.reduce((function(acc, group) {
+ for (var line = startLine; line <= endLine; line++) {
+ var content =
+ this.getContentByLine(line, opt_side, group.element);
+ if (content) {
+ acc.push(content);
+ }
+ }
+ return acc;
+ }).bind(this), []);
+ },
+
+ getCommentThreadByLine: function(lineNumber, opt_side, opt_root) {
+ var root = Polymer.dom(opt_root || this.diffElement);
+ var sideSelector = !!opt_side ? ('.' + opt_side) : ''
+ var content = this.getContentByLine(lineNumber, opt_side, opt_root);
+ return content.querySelector('gr-diff-comment-thread');
+ },
+
getSideByLineEl: function(lineEl) {
return lineEl.classList.contains(GrDiffBuilder.Side.RIGHT) ?
- GrDiffBuilder.Side.RIGHT : GrDiffBuilder.Side.LEFT;
+ GrDiffBuilder.Side.RIGHT : GrDiffBuilder.Side.LEFT;
},
createCommentThread: function(changeNum, patchNum, path, side,
@@ -106,12 +181,12 @@
newGroups.forEach(function(newGroup) {
this._builder.emitGroup(newGroup, sectionEl);
- }.bind(this));
+ }, this);
sectionEl.parentNode.removeChild(sectionEl);
this.async(function() {
this.fire('render');
- }.bind(this), 1);
+ }, 1);
},
_getDiffBuilder: function(diff, comments, prefs) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index f03687c..8acf936 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -65,8 +65,7 @@
}
};
- GrDiffBuilder.prototype.buildSectionElement = function(
- group, opt_beforeSection) {
+ GrDiffBuilder.prototype.buildSectionElement = function(group) {
throw Error('Subclasses must implement buildGroupElement');
};
@@ -88,28 +87,53 @@
}
};
- GrDiffBuilder.prototype.getSectionsByLineRange = function(
+ GrDiffBuilder.prototype.getGroupsByLineRange = function(
startLine, endLine, opt_side) {
- var sections = [];
+ var groups = [];
for (var i = 0; i < this._groups.length; i++) {
var group = this._groups[i];
if (group.lines.length === 0) {
continue;
}
- var groupStartLine;
- var groupEndLine;
- if (opt_side === GrDiffBuilder.Side.LEFT) {
- groupStartLine = group.lines[0].beforeNumber;
- groupEndLine = group.lines[group.lines.length - 1].beforeNumber;
- } else if (opt_side === GrDiffBuilder.Side.RIGHT) {
- groupStartLine = group.lines[0].afterNumber;
- groupEndLine = group.lines[group.lines.length - 1].afterNumber;
+ var groupStartLine = 0;
+ var groupEndLine = 0;
+ switch (group.type) {
+ case GrDiffGroup.Type.BOTH:
+ if (opt_side === GrDiffBuilder.Side.LEFT) {
+ groupStartLine = group.lines[0].beforeNumber;
+ groupEndLine = group.lines[group.lines.length - 1].beforeNumber;
+ } else if (opt_side === GrDiffBuilder.Side.RIGHT) {
+ groupStartLine = group.lines[0].afterNumber;
+ groupEndLine = group.lines[group.lines.length - 1].afterNumber;
+ }
+ break;
+ case GrDiffGroup.Type.DELTA:
+ if (opt_side === GrDiffBuilder.Side.LEFT && group.removes.length) {
+ groupStartLine = group.removes[0].beforeNumber;
+ groupEndLine = group.removes[group.removes.length - 1].beforeNumber;
+ } else if (group.adds.length) {
+ groupStartLine = group.adds[0].afterNumber;
+ groupEndLine = group.adds[group.adds.length - 1].afterNumber;
+ }
+ break;
+ }
+ if (groupStartLine === 0) { // Line was removed or added.
+ groupStartLine = groupEndLine;
+ }
+ if (groupEndLine === 0) { // Line was removed or added.
+ groupEndLine = groupStartLine;
}
if (startLine <= groupEndLine && endLine >= groupStartLine) {
- sections.push(group.element);
+ groups.push(group);
}
}
- return sections;
+ return groups;
+ };
+
+ GrDiffBuilder.prototype.getSectionsByLineRange = function(
+ startLine, endLine, opt_side) {
+ return this.getGroupsByLineRange(startLine, endLine, opt_side).map(
+ function(group) { return group.element; });
};
GrDiffBuilder.prototype._processContent = function(content, groups, context) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index c47e05d..6cef374 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -562,11 +562,16 @@
context: -1
};
content = [
- {ab: []},
- {a: ['all work and no play make andybons a dull boy']},
- {ab: []},
- {b: ['elgoog elgoog elgoog']},
- {ab: []},
+ {
+ a: ['all work and no play make andybons a dull boy'],
+ b: ['elgoog elgoog elgoog']
+ },
+ {
+ ab: [
+ 'Non eram nescius, Brute, cum, quae summis ingeniis ',
+ 'exquisitaque doctrina philosophi Graeco sermone tractavissent',
+ ]
+ },
];
outputEl = document.createElement('out');
builder =
@@ -591,12 +596,23 @@
assert.equal(section.innerHTML, prevInnerHTML);
});
- test('getSectionsByLineRange', function() {
+ test('getSectionsByLineRange one line', function() {
var section = outputEl.querySelector('stub:nth-of-type(2)');
var sections = builder.getSectionsByLineRange(1, 1, 'left');
assert.equal(sections.length, 1);
assert.strictEqual(sections[0], section);
});
+
+ test('getSectionsByLineRange over diff', function() {
+ var section = [
+ outputEl.querySelector('stub:nth-of-type(2)'),
+ outputEl.querySelector('stub:nth-of-type(3)'),
+ ];
+ var sections = builder.getSectionsByLineRange(1, 2, 'left');
+ assert.equal(sections.length, 2);
+ assert.strictEqual(sections[0], section[0]);
+ assert.strictEqual(sections[1], section[1]);
+ });
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
index 7304ac7..8cc784f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
@@ -51,16 +51,19 @@
}.bind(this));
},
- addDraft: function(opt_lineNum) {
+ addOrEditDraft: function(opt_lineNum) {
var lastComment = this.comments[this.comments.length - 1];
if (lastComment && lastComment.__draft) {
var commentEl = this._commentElWithDraftID(
lastComment.id || lastComment.__draftID);
commentEl.editing = true;
- return;
+ } else {
+ this.addDraft(opt_lineNum);
}
+ },
- var draft = this._newDraft(opt_lineNum);
+ addDraft: function(opt_lineNum, opt_range) {
+ var draft = this._newDraft(opt_lineNum, opt_range);
draft.__editing = true;
this.push('comments', draft);
},
@@ -154,7 +157,7 @@
return d;
},
- _newDraft: function(opt_lineNum) {
+ _newDraft: function(opt_lineNum, opt_range) {
var d = {
__draft: true,
__draftID: Math.random().toString(36),
@@ -165,20 +168,28 @@
if (opt_lineNum) {
d.line = opt_lineNum;
}
+ if (opt_range) {
+ d.range = {
+ start_line: opt_range.startLine,
+ start_character: opt_range.startChar,
+ end_line: opt_range.endLine,
+ end_character: opt_range.endChar,
+ };
+ }
return d;
},
_handleCommentDiscard: function(e) {
var diffCommentEl = Polymer.dom(e).rootTarget;
- var idx = this._indexOf(diffCommentEl.comment, this.comments);
+ var comment = diffCommentEl.comment;
+ var idx = this._indexOf(comment, this.comments);
if (idx == -1) {
throw Error('Cannot find comment ' +
JSON.stringify(diffCommentEl.comment));
}
this.splice('comments', idx, 1);
if (this.comments.length == 0) {
- this.fire('thread-discard');
- return;
+ this.fire('thread-discard', {lastComment: comment});
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index 6e7a68a..c3b6233 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -114,7 +114,10 @@
display: block;
}
</style>
- <div class="container" id="container">
+ <div id="container"
+ class="container"
+ on-mouseenter="_handleMouseEnter"
+ on-mouseleave="_handleMouseLeave">
<div class="header" id="header">
<div class="headerLeft">
<span class="authorName">[[comment.author.name]]</span>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
index 7a15754..3baf200 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -50,6 +50,14 @@
* @event comment-update
*/
+ /**
+ * @event comment-mouse-over
+ */
+
+ /**
+ * @event comment-mouse-out
+ */
+
properties: {
changeNum: String,
comment: {
@@ -90,7 +98,7 @@
],
detached: function() {
- this.flushDebouncer('fire-update');
+ this.cancelDebouncer('fire-update');
},
save: function() {
@@ -118,7 +126,7 @@
}
this.comment = comment;
this.editing = false;
- this.fire('comment-save', {comment: this.comment});
+ this._fireSave();
return obj;
}.bind(this));
}.bind(this)).catch(function(err) {
@@ -129,11 +137,29 @@
_commentChanged: function(comment) {
this.editing = !!comment.__editing;
+ if (this.editing) { // It's a new draft/reply, notify.
+ this._fireUpdate();
+ }
+ },
+
+ _getEventPayload: function(opt_mixin) {
+ var payload = {
+ comment: this.comment,
+ patchNum: this.patchNum,
+ };
+ for (var k in opt_mixin) {
+ payload[k] = opt_mixin[k];
+ }
+ return payload;
+ },
+
+ _fireSave: function() {
+ this.fire('comment-save', this._getEventPayload());
},
_fireUpdate: function() {
this.debounce('fire-update', function() {
- this.fire('comment-update', {comment: this.comment});
+ this.fire('comment-update', this._getEventPayload());
}, UPDATE_DEBOUNCE_INTERVAL);
},
@@ -141,7 +167,7 @@
this.$.container.classList.toggle('draft', draft);
},
- _editingChanged: function(editing) {
+ _editingChanged: function(editing, previousValue) {
this.$.container.classList.toggle('editing', editing);
if (editing) {
var textarea = this.$.editTextarea.textarea;
@@ -158,7 +184,10 @@
if (this.comment) {
this.comment.__editing = this.editing;
}
- this._fireUpdate();
+ if (editing != !!previousValue) {
+ // To prevent event firing on comment creation.
+ this._fireUpdate();
+ }
},
_computeLinkToComment: function(comment) {
@@ -216,18 +245,18 @@
_handleReply: function(e) {
this._preventDefaultAndBlur(e);
- this.fire('reply', {comment: this.comment}, {bubbles: false});
+ this.fire('reply', this._getEventPayload(), {bubbles: false});
},
_handleQuote: function(e) {
this._preventDefaultAndBlur(e);
- this.fire('reply', {comment: this.comment, quote: true},
- {bubbles: false});
+ this.fire(
+ 'reply', this._getEventPayload({quote: true}), {bubbles: false});
},
_handleDone: function(e) {
this._preventDefaultAndBlur(e);
- this.fire('done', {comment: this.comment}, {bubbles: false});
+ this.fire('done', this._getEventPayload(), {bubbles: false});
},
_handleEdit: function(e) {
@@ -244,13 +273,17 @@
_handleCancel: function(e) {
this._preventDefaultAndBlur(e);
if (this.comment.message == null || this.comment.message.length == 0) {
- this.fire('comment-discard', {comment: this.comment});
+ this._fireDiscard();
return;
}
this._messageText = this.comment.message;
this.editing = false;
},
+ _fireDiscard: function() {
+ this.fire('comment-discard', this._getEventPayload());
+ },
+
_handleDiscard: function(e) {
this._preventDefaultAndBlur(e);
if (!this.comment.__draft) {
@@ -260,7 +293,7 @@
this.disabled = true;
if (!this.comment.id) {
this.disabled = false;
- this.fire('comment-discard', {comment: this.comment});
+ this._fireDiscard();
return;
}
@@ -269,7 +302,7 @@
this.disabled = false;
if (!response.ok) { return response; }
- this.fire('comment-discard', {comment: this.comment});
+ this._fireDiscard();
}.bind(this)).catch(function(err) {
this.disabled = false;
throw err;
@@ -308,5 +341,13 @@
this.set('comment.message', draft.message);
}
},
+
+ _handleMouseEnter: function(e) {
+ this.fire('comment-mouse-over', this._getEventPayload());
+ },
+
+ _handleMouseLeave: function(e) {
+ this.fire('comment-mouse-out', this._getEventPayload());
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
index d2262ab..de5eb48 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -225,6 +225,7 @@
line: 5,
path: '/path/to/file',
},
+ patchNum: 1,
},
]);
MockInteractions.tap(element.$$('.save'));
@@ -247,6 +248,7 @@
path: '/path/to/file',
updated: '2015-12-08 21:52:36.177000000',
},
+ patchNum: 1,
},
]);
assert.isFalse(element.disabled,
@@ -255,8 +257,8 @@
assert.isFalse(element.editing);
}).then(function() {
MockInteractions.tap(element.$$('.edit'));
- element._messageText = 'You’ll be delivering a package to Chapek 9, a ' +
- 'world where humans are killed on sight.';
+ element._messageText = '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.');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 6eefe07..e3f0c0c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -137,7 +137,6 @@
},
_getDiffPreferences: function() {
- this._localPrefs = this.$.storage.getPreferences();
return this.$.restAPI.getDiffPreferences();
},
@@ -190,10 +189,12 @@
this.$.cursor.moveUp();
break;
case 67: // 'c'
- e.preventDefault();
- var line = this.$.cursor.getTargetLineElement();
- if (line) {
- this.$.diff.addDraftAtLine(line);
+ if (!this.$.diff.isRangeSelected()) {
+ e.preventDefault();
+ var line = this.$.cursor.getTargetLineElement();
+ if (line) {
+ this.$.diff.addDraftAtLine(line);
+ }
}
break;
case 219: // '['
@@ -288,6 +289,7 @@
var promises = [];
+ this._localPrefs = this.$.storage.getPreferences();
promises.push(this._getDiffPreferences().then(function(prefs) {
this._prefs = prefs;
}.bind(this)));
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 8c9b20d..88810c1 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -66,6 +66,7 @@
'comment-discard': '_handleCommentDiscard',
'comment-update': '_handleCommentUpdate',
'comment-save': '_handleCommentSave',
+ 'create-comment': '_handleCreateComment',
},
attached: function() {
@@ -195,33 +196,59 @@
}
},
- _addDraft: function(lineEl, opt_lineNum) {
- var threadEl;
+ _handleCreateComment: function(e) {
+ var range = e.detail.range;
+ var diffSide = e.detail.side;
+ var line = range.endLine;
+ var lineEl = this.$.diffBuilder.getLineElByNumber(line, diffSide);
+ var contentEl = this.$.diffBuilder.getContentByLineEl(lineEl);
+ var patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
+ var side = this._getSideByLineAndContent(lineEl, contentEl);
+ var threadEl = this._getOrCreateThreadAtLine(contentEl, patchNum, side);
- // Does a thread already exist at this line?
- var contentEl = lineEl.nextSibling;
- while (contentEl && !contentEl.classList.contains('content')) {
- contentEl = contentEl.nextSibling;
- }
- if (contentEl.childNodes.length > 0 &&
- contentEl.lastChild.nodeName === 'GR-DIFF-COMMENT-THREAD') {
- threadEl = contentEl.lastChild;
- } else {
- var patchNum = this.patchRange.patchNum;
- var side = 'REVISION';
- if (lineEl.classList.contains(DiffSide.LEFT) ||
- contentEl.classList.contains('remove')) {
- if (this.patchRange.basePatchNum === 'PARENT') {
- side = 'PARENT';
- } else {
- patchNum = this.patchRange.basePatchNum;
- }
- }
+ threadEl.addDraft(line, range);
+ },
+
+ _addDraft: function(lineEl, opt_lineNum) {
+ var line = opt_lineNum || lineEl.getAttribute('data-value');
+ var contentEl = this.$.diffBuilder.getContentByLineEl(lineEl);
+ var patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
+ var side = this._getSideByLineAndContent(lineEl, contentEl);
+ var threadEl = this._getOrCreateThreadAtLine(contentEl, patchNum, side);
+
+ threadEl.addOrEditDraft(opt_lineNum);
+ },
+
+ _getOrCreateThreadAtLine: function(contentEl, patchNum, side) {
+ var threadEl = contentEl.querySelector('gr-diff-comment-thread');
+
+ if (!threadEl) {
threadEl = this.$.diffBuilder.createCommentThread(
this.changeNum, patchNum, this.path, side, this.projectConfig);
contentEl.appendChild(threadEl);
}
- threadEl.addDraft(opt_lineNum);
+
+ return threadEl;
+ },
+
+ _getPatchNumByLineAndContent: function(lineEl, contentEl) {
+ var patchNum = this.patchRange.patchNum;
+ if ((lineEl.classList.contains(DiffSide.LEFT) ||
+ contentEl.classList.contains('remove')) &&
+ this.patchRange.basePatchNum !== 'PARENT') {
+ patchNum = this.patchRange.basePatchNum;
+ }
+ return patchNum;
+ },
+
+ _getSideByLineAndContent: function(lineEl, contentEl) {
+ var side = 'REVISION';
+ if ((lineEl.classList.contains(DiffSide.LEFT) ||
+ contentEl.classList.contains('remove')) &&
+ this.patchRange.basePatchNum === 'PARENT') {
+ side = 'PARENT';
+ }
+ return side;
},
_handleThreadDiscard: function(e) {
@@ -231,7 +258,7 @@
_handleCommentDiscard: function(e) {
var comment = e.detail.comment;
- this._removeComment(comment, e.target.patchNum);
+ this._removeComment(comment, e.detail.patchNum);
},
_removeComment: function(comment, opt_patchNum) {
@@ -250,14 +277,14 @@
_handleCommentSave: function(e) {
var comment = e.detail.comment;
- var side = this._findCommentSide(comment, e.target.patchNum);
+ var side = this._findCommentSide(comment, e.detail.patchNum);
var idx = this._findDraftIndex(comment, side);
this.set(['_comments', side, idx], comment);
},
_handleCommentUpdate: function(e) {
var comment = e.detail.comment;
- var side = this._findCommentSide(comment, e.target.patchNum);
+ var side = this._findCommentSide(comment, e.detail.patchNum);
var idx = this._findCommentIndex(comment, side);
if (idx === -1) {
idx = this._findDraftIndex(comment, side);
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
index f9174ac..6f95789 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.html
@@ -26,6 +26,7 @@
background-color: #fff;
border: 1px solid #000;
border-radius: .5em;
+ cursor: pointer;
padding: .3em;
position: absolute;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
index 0a85096..64e2f4a 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
@@ -17,6 +17,12 @@
Polymer({
is: 'gr-selection-action-box',
+ /**
+ * Fired when the comment creation action was taken (hotkey, click).
+ *
+ * @event create-comment
+ */
+
properties: {
keyEventTarget: {
type: Object,
@@ -41,6 +47,10 @@
Gerrit.KeyboardShortcutBehavior,
],
+ listeners: {
+ 'tap': '_handleTap',
+ },
+
placeAbove: function(el) {
var rect = this._getTargetBoundingRect(el);
var boxRect = this.getBoundingClientRect();
@@ -53,7 +63,7 @@
_getTargetBoundingRect: function(el) {
var rect;
- if (!(el instanceof Element)) {
+ if (el instanceof Text) {
var range = document.createRange();
range.selectNode(el);
rect = range.getBoundingClientRect();
@@ -68,8 +78,16 @@
if (this.shouldSupressKeyboardShortcut(e)) { return; }
if (e.keyCode === 67) { // 'c'
e.preventDefault();
- this.fire('create-comment', {side: this.side, range: this.range});
+ this._fireCreateComment();
};
},
+
+ _handleTap: function() {
+ this._fireCreateComment();
+ },
+
+ _fireCreateComment: function() {
+ this.fire('create-comment', {side: this.side, range: this.range});
+ },
});
})();
