Move triggering thread creation to gr-diff-host
This is a step towards allowing code using gr-diff pick their own
comment widget. The threads are still created by
gr-diff-comment-thread-group in this commit - only the triggering is
moved. But that will change in a future CL.
Change-Id: If42b07df11422b69e7e1421c15e923d208012b47
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index cee3cad..4fe4ab08 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -35,7 +35,7 @@
listeners: {
'comment-mouse-out': '_handleCommentMouseOut',
'comment-mouse-over': '_handleCommentMouseOver',
- 'create-comment': '_createComment',
+ 'create-range-comment': '_createRangeComment',
},
observers: [
@@ -311,7 +311,7 @@
}
},
- _createComment(e) {
+ _createRangeComment(e) {
this._removeActionBox();
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index 7b19338..8c758fe 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -215,9 +215,9 @@
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: {},
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 6f61fb9..19fb308 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -177,7 +177,7 @@
},
listeners: {
- 'draft-interaction': '_handleDraftInteraction',
+ 'create-comment': '_handleCreateComment',
},
observers: [
@@ -445,11 +445,35 @@
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
* chunks.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index f83253e..3374686 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -776,6 +776,29 @@
});
});
+ 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;
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 003e024..69ac283 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -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) {
@@ -403,26 +409,6 @@
},
/**
- * 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
* @param {!string} commentSide
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index aa28713..07584c7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -298,12 +298,6 @@
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 @@
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', () => {
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 6349ab6..0f84877 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
@@ -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});
},
});
})();
diff --git a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
index 19155e4..4f1065a 100644
--- a/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-selection-action-box/gr-selection-action-box_test.html
@@ -98,7 +98,7 @@
element.range = range;
MockInteractions.pressAndReleaseKeyOn(document.body, 67, null, 'c');
assert(element.fire.calledWithExactly(
- 'create-comment',
+ 'create-range-comment',
{
side,
range,
