Move gr-diff-comment-thread factory to diff-host
This is now only used from there, so it no longer needs to be globally
accessible on Gerrit.
Change-Id: I6facd64ece360f5b8ddf7b801621e86bd5f39697
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 33e2a19..6db5411 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
@@ -19,6 +19,7 @@
<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
+<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
<link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
<link rel="import" href="../gr-syntax-layer/gr-syntax-layer.html">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
index e7641cd..0321826 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js
@@ -17,58 +17,6 @@
(function() {
'use strict';
- window.Gerrit = window.Gerrit || {};
-
- // This method will eventually move to gr-diff-host (so that gr-diff and it's
- // descendants, including gr-diff-comment-thread-group, do not depend on a
- // specific comment thread implementation, but can instead be used with other
- // comment widgets). I cannot move it there yet, because it is still called
- // from this file, and this file cannot depend on gr-diff-host. I decided to
- // make it a function on the global Gerrit namespace, so that
- // 1) I can move the call-side in the next change without moving this code,
- // and thereby reduce the number of moving parts per commit.
- // 2) To already now cut the ties to the this object - if it was an element
- // method, I would probably want to use isOnParent etc. from `this`, and
- // thus be required to change the code when I move it to gr-diff host.
- // Making it a free function first requires me to catch any references to
- // `this` and instead pass those in as parameter, which then allows me
- // to move it later without any other changes, which makes the diff
- // easier to read.
- /**
- * @param {Object} thread
- * @param {number} parentIndex
- * @param {number} changeNum
- * @param {string} path
- * @param {string} projectName
- */
- window.Gerrit.createThreadElement = function(
- thread, parentIndex, changeNum, path, projectName) {
- const threadEl = document.createElement('gr-diff-comment-thread');
- threadEl.comments = thread.comments;
- threadEl.commentSide = thread.commentSide;
- threadEl.isOnParent = !!thread.isOnParent;
- threadEl.parentIndex = parentIndex;
- threadEl.changeNum = changeNum;
- threadEl.patchNum = thread.patchNum;
- threadEl.lineNum = thread.lineNum;
- const rootIdChangedListener = changeEvent => {
- thread.rootId = changeEvent.detail.value;
- };
- threadEl.addEventListener('root-id-changed', rootIdChangedListener);
- threadEl.path = path;
- threadEl.projectName = projectName;
- threadEl.range = thread.range;
- const threadDiscardListener = e => {
- const threadEl = /** @type {!Node} */ (e.currentTarget);
- const parent = Polymer.dom(threadEl).parentNode;
- threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
- threadEl.removeEventListener('thread-discard', threadDiscardListener);
- Polymer.dom(parent).removeChild(threadEl);
- };
- threadEl.addEventListener('thread-discard', threadDiscardListener);
- return threadEl;
- };
-
Polymer({
is: 'gr-diff-comment-thread-group',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
index d1150ed..8219d1b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html
@@ -36,38 +36,14 @@
<script>
suite('gr-diff-comment-thread-group tests', () => {
- let element;
let sandbox;
setup(() => {
sandbox = sinon.sandbox.create();
- element = fixture('basic');
});
teardown(() => {
sandbox.restore();
});
-
- test('thread-discard handling', () => {
- const threads = [
- {comments: [{id: 4711}]},
- {comments: [{id: 42}]},
- ];
- const threadEls = threads.map(thread => Gerrit.createThreadElement(
- thread, 1, 2, 'some/path', 'Some Project'));
- assert.equal(threadEls.length, 2);
- assert.equal(threadEls[0].rootId, 4711);
- assert.equal(threadEls[1].rootId, 42);
- for (const threadEl of threadEls) {
- Polymer.dom(element).appendChild(threadEl);
- }
-
- threadEls[0].dispatchEvent(
- new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
- const attachedThreads = element.queryAllEffectiveChildren(
- 'gr-diff-comment-thread');
- assert.equal(attachedThreads.length, 1);
- assert.equal(attachedThreads[0].rootId, 42);
- });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index 028e44c..d335e7a 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -20,7 +20,6 @@
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
-<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
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 2b91060..5e4a3fd 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
@@ -599,15 +599,36 @@
},
_createThreadElement(thread) {
- const threadEl = Gerrit.createThreadElement(
- thread, this._parentIndex, this.changeNum, this.path,
- this.projectName);
+ const threadEl = document.createElement('gr-diff-comment-thread');
threadEl.className = 'comment-thread';
- threadEl.addEventListener('thread-discard', e => {
+ threadEl.comments = thread.comments;
+ threadEl.commentSide = thread.commentSide;
+ threadEl.isOnParent = !!thread.isOnParent;
+ threadEl.parentIndex = this._parentIndex;
+ threadEl.changeNum = this.changeNum;
+ threadEl.patchNum = thread.patchNum;
+ threadEl.lineNum = thread.lineNum;
+ const rootIdChangedListener = changeEvent => {
+ thread.rootId = changeEvent.detail.value;
+ };
+ threadEl.addEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.path = this.path;
+ threadEl.projectName = this.projectName;
+ threadEl.range = thread.range;
+ const threadDiscardListener = e => {
+ const threadEl = /** @type {!Node} */ (e.currentTarget);
+
+ const parent = Polymer.dom(threadEl).parentNode;
+ Polymer.dom(parent).removeChild(threadEl);
+
const i = this._threadEls.findIndex(
threadEl => threadEl.rootId == e.detail.rootId);
this._threadEls.splice(i, 1);
- });
+
+ threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
+ threadEl.removeEventListener('thread-discard', threadDiscardListener);
+ };
+ threadEl.addEventListener('thread-discard', threadDiscardListener);
return threadEl;
},
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 790a71f..c7ee1c2 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
@@ -55,6 +55,32 @@
sandbox.restore();
});
+ test('thread-discard handling', () => {
+ const threads = [
+ {comments: [{id: 4711}]},
+ {comments: [{id: 42}]},
+ ];
+ element._parentIndex = 1;
+ element.changeNum = '2';
+ element.path = 'some/path';
+ element.projectName = 'Some project';
+ const threadEls = threads.map(
+ thread => element._createThreadElement(thread));
+ assert.equal(threadEls.length, 2);
+ assert.equal(threadEls[0].rootId, 4711);
+ assert.equal(threadEls[1].rootId, 42);
+ for (const threadEl of threadEls) {
+ Polymer.dom(element).appendChild(threadEl);
+ }
+
+ threadEls[0].dispatchEvent(
+ new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
+ const attachedThreads = element.queryAllEffectiveChildren(
+ 'gr-diff-comment-thread');
+ assert.equal(attachedThreads.length, 1);
+ assert.equal(attachedThreads[0].rootId, 42);
+ });
+
test('reload() cancels before network resolves', () => {
const cancelStub = sandbox.stub(element.$.diff, 'cancel');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index e5854011..b258455 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -20,6 +20,7 @@
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
+<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
<link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
<link rel="import" href="../gr-diff-selection/gr-diff-selection.html">
<link rel="import" href="../gr-syntax-themes/gr-syntax-theme.html">
