opendev/gerrit
Code Issues Proposed changes

Merge "Move triggering thread creation to gr-diff-host"

Browse Source
This commit is contained in:
Ole Rehmsen 2018-10-24 14:02:15 +00:00 committed by Gerrit Code Review
commit 9a168fa8d2
8 changed files with 74 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
View File

@ -35,7 +35,7 @@
listeners: {
'comment-mouse-out': '_handleCommentMouseOut',
'comment-mouse-over': '_handleCommentMouseOver',
'create-comment': '_createComment',
'create-range-comment': '_createRangeComment',
},
observers: [
@ -317,7 +317,7 @@
}
},
_createComment(e) {
_createRangeComment(e) {
this._removeActionBox();
},

4
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
View File

@ -215,9 +215,9 @@ limitations under the License.
assert.isFalse(builder.getContentsByLineRange.called);
});
test('on create-comment action box is removed', () => {
test('on create-range-comment action box is removed', () => {
sandbox.stub(element, '_removeActionBox');
element.fire('create-comment', {
element.fire('create-range-comment', {
comment: {
range: {},
},

28
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
View File

@ -177,7 +177,7 @@
},
listeners: {
'draft-interaction': '_handleDraftInteraction',
'create-comment': '_handleCreateComment',
},
observers: [
@ -445,10 +445,34 @@
this.patchRange);
},
_handleDraftInteraction() {
/** @param {CustomEvent} e */
_handleCreateComment(e) {
const {threadGroupEl, lineNum, side, range} = e.detail;
const threadEl = this._getOrCreateThread(threadGroupEl, side, range);
threadEl.addOrEditDraft(lineNum, range);
this.$.reporting.recordDraftInteraction();
},
/**
* Gets or creates a comment thread from a specific thread group.
* May include a range, if the comment is a range comment.
*
* @param {!Object} threadGroupEl
* @param {string} commentSide
* @param {!Object=} range
* @return {!Object}
*/
_getOrCreateThread(threadGroupEl, commentSide, range=undefined) {
let threadEl = threadGroupEl.getThread(commentSide, range);
if (!threadEl) {
threadGroupEl.addNewThread(commentSide, range);
Polymer.dom.flush();
threadEl = threadGroupEl.getThread(commentSide, range);
}
return threadEl;
},
/**
* Take a diff that was loaded with a ignore-whitespace other than
* IGNORE_NONE, and convert delta chunks labeled as common into shared

23
polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
View File

@ -776,6 +776,29 @@ limitations under the License.
});
});
test('_getOrCreateThread', () => {
const threadGroupEl =
document.createElement('gr-diff-comment-thread-group');
const commentSide = 'left';
assert.isOk(element._getOrCreateThread(threadGroupEl,
commentSide));
// Try to fetch a thread with a different range.
range = {
startLine: 1,
startChar: 1,
endLine: 1,
endChar: 3,
};
assert.isOk(element._getOrCreateThread(
threadGroupEl, commentSide, range));
const threadCount = Polymer.dom(threadGroupEl.root).
querySelectorAll('gr-diff-comment-thread').length;
assert.equal(threadCount, 2);
});
suite('_translateChunksToIgnore', () => {
let content;

50
polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
View File

@ -60,9 +60,9 @@
*/
/**
* Fired when a draft is added or edited.
* Fired when a comment is created
*
* @event draft-interaction
* @event create-comment
*/
properties: {
@ -207,7 +207,7 @@
'comment-discard': '_handleCommentDiscard',
'comment-update': '_handleCommentUpdate',
'comment-save': '_handleCommentSave',
'create-comment': '_handleCreateComment',
'create-range-comment': '_handleCreateRangeComment',
},
/** Cancel any remaining diff builder rendering work. */
@ -322,7 +322,7 @@
this._createComment(el, lineNum);
},
_handleCreateComment(e) {
_handleCreateRangeComment(e) {
const range = e.detail.range;
const side = e.detail.side;
const lineNum = range.endLine;
@ -359,23 +359,29 @@
/**
* @param {!Object} lineEl
* @param {number=} opt_lineNum
* @param {string=} opt_side
* @param {!Object=} opt_range
* @param {number=} lineNum
* @param {string=} side
* @param {!Object=} range
*/
_createComment(lineEl, opt_lineNum, opt_side, opt_range) {
this.dispatchEvent(new CustomEvent('draft-interaction', {bubbles: true}));
_createComment(lineEl, lineNum=undefined, side=undefined, range=undefined) {
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
const side = opt_side ||
side = side ||
this._getCommentSideByLineAndContent(lineEl, contentEl);
const patchNum = this._getPatchNumByLineAndContent(lineEl, contentEl);
const isOnParent =
this._getIsParentCommentByLineAndContent(lineEl, contentEl);
const threadGroupEl = this._getOrCreateThreadGroup(contentEl, patchNum,
side, isOnParent);
const threadEl = this._getOrCreateThread(threadGroupEl, side, opt_range);
threadEl.addOrEditDraft(opt_lineNum, opt_range);
this.dispatchEvent(new CustomEvent('create-comment', {
bubbles: true,
detail: {
threadGroupEl,
lineNum,
side,
range,
},
}));
},
_getThreadGroupForLine(contentEl) {
@ -402,26 +408,6 @@
return threadGroupEl;
},
/**
* Gets or creates a comment thread from a specific thread group.
* May include a range, if the comment is a range comment.
*
* @param {!Object} threadGroupEl
* @param {string} commentSide
* @param {!Object=} range
* @return {!Object}
*/
_getOrCreateThread(threadGroupEl, commentSide, range=undefined) {
let threadEl = threadGroupEl.getThread(commentSide, range);
if (!threadEl) {
threadGroupEl.addNewThread(commentSide, range);
Polymer.dom.flush();
threadEl = threadGroupEl.getThread(commentSide, range);
}
return threadEl;
},
/**
* @param {number} patchNum
* @param {boolean} isOnParent

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

@ -298,12 +298,6 @@ limitations under the License.
const commentSide = 'left';
const patchNum = 1;
const side = 'PARENT';
let range = {
startLine: 1,
startChar: 1,
endLine: 1,
endChar: 2,
};
element.changeNum = 123;
element.patchRange = {basePatchNum: 1, patchNum: 2};
@ -322,30 +316,11 @@ limitations under the License.
patchNum, commentSide, side);
assert.isOk(threadGroupEl);
assert.isOk(element._getOrCreateThread(threadGroupEl,
commentSide, side));
// Try to fetch a thread with a different range.
range = {
startLine: 1,
startChar: 1,
endLine: 1,
endChar: 3,
};
assert.isOk(element._getOrCreateThread(
threadGroupEl, commentSide, range));
// The new thread group can be fetched.
assert.isOk(element._getThreadGroupForLine(contentEl));
assert.equal(contentEl.querySelectorAll(
'gr-diff-comment-thread-group').length, 1);
const threadGroup = contentEl.querySelector(
'gr-diff-comment-thread-group');
const threadLength = Polymer.dom(threadGroup.root).
querySelectorAll('gr-diff-comment-thread').length;
assert.equal(threadLength, 2);
});
suite('image diffs', () => {

4
polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box.js
View File

@ -23,7 +23,7 @@
/**
* Fired when the comment creation action was taken (hotkey, click).
*
* @event create-comment
* @event create-range-comment
*/
properties: {
@ -110,7 +110,7 @@
},
_fireCreateComment() {
this.fire('create-comment', {side: this.side, range: this.range});
this.fire('create-range-comment', {side: this.side, range: this.range});
},
});
})();

2
polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
View File

@ -98,7 +98,7 @@ limitations under the License.
element.range = range;
MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c');
assert(element.fire.calledWithExactly(
'create-comment',
'create-range-comment',
{
side,
range,