opendev/gerrit
Code Issues Proposed changes

Move comment and thread elements to shared

Browse Source 
Even before the refactorings, the comment and thread elements were also used
from views outside of diff/. Now they are now completely independent from
gr-diff and descendants, and can move into their own folder and drop the -diff
from their name.

The dependency left from diff/gr-diff-host onto comment[-thread], makes sense
since gr-diff-host is the Gerrit wrapper for gr-diff with gr-comment[-thread].

Change-Id: I5076428da980198989edc605e5edc0e4d66529dd
This commit is contained in:
Ole Rehmsen 2018-11-19 21:45:41 +01:00
parent 71e09f25a8
commit 9b94616a54
16 changed files with 60 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
View File

@ -91,7 +91,7 @@ iron-a11y-keys-behavior's keyBindings attribute to implement the binding in the
element. An example of this is in comment threads. A diff view supports actions
on comment threads, but there may be zero or many comment threads attached at
any given point. So the shortcut is declared as doc-only by the diff view and
by gr-app, and actually implemented by gr-diff-comment-thread.
by gr-app, and actually implemented by gr-comment-thread.
NOTE: doc-only shortcuts will not be customizable in the same way that other
shortcuts are.

2
polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
View File

@ -219,7 +219,7 @@
[this.Shortcut.TOGGLE_FILE_REVIEWED]: '_handleToggleFileReviewed',
[this.Shortcut.TOGGLE_LEFT_PANE]: '_handleToggleLeftPane',
// Final two are actually handled by gr-diff-comment-thread.
// Final two are actually handled by gr-comment-thread.
[this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
[this.Shortcut.COLLAPSE_ALL_COMMENT_THREADS]: null,
};

14
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
View File

@ -18,7 +18,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../bower_components/paper-toggle-button/paper-toggle-button.html">
<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../diff/gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../../shared/gr-comment-thread/gr-comment-thread.html">
<dom-module id="gr-thread-list">
<template>
@ -28,7 +28,7 @@ limitations under the License.
min-height: 20rem;
padding: 1rem;
}
gr-diff-comment-thread {
gr-comment-thread {
display: block;
margin-bottom: .5rem;
max-width: 80ch;
@ -54,9 +54,9 @@ limitations under the License.
display: flex;
margin-right: 1rem;
}
.draftsOnly:not(.unresolvedOnly) gr-diff-comment-thread[has-draft],
.unresolvedOnly:not(.draftsOnly) gr-diff-comment-thread[unresolved],
.draftsOnly.unresolvedOnly gr-diff-comment-thread[has-draft][unresolved] {
.draftsOnly:not(.unresolvedOnly) gr-comment-thread[has-draft],
.unresolvedOnly:not(.draftsOnly) gr-comment-thread[unresolved],
.draftsOnly.unresolvedOnly gr-comment-thread[has-draft][unresolved] {
display: block
}
</style>
@ -82,7 +82,7 @@ limitations under the License.
as="thread"
initial-count="5"
target-framerate="60">
<gr-diff-comment-thread
<gr-comment-thread
show-file-path
change-num="[[changeNum]]"
comments="[[thread.comments]]"
@ -94,7 +94,7 @@ limitations under the License.
path="[[thread.path]]"
root-id="{{thread.rootId}}"
on-thread-changed="_handleCommentsChanged"
on-thread-discard="_handleThreadDiscard"></gr-diff-comment-thread>
on-thread-discard="_handleThreadDiscard"></gr-comment-thread>
</template>
</div>
</template>

10
polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
View File

@ -171,7 +171,7 @@ limitations under the License.
];
flushAsynchronousOperations();
threadElements = Polymer.dom(element.root)
.querySelectorAll('gr-diff-comment-thread');
.querySelectorAll('gr-comment-thread');
});
teardown(() => {
@ -188,7 +188,7 @@ limitations under the License.
test('there are five threads by default', () => {
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-diff-comment-thread').length, 5);
.querySelectorAll('gr-comment-thread').length, 5);
});
test('_computeSortedThreads', () => {
@ -231,14 +231,14 @@ limitations under the License.
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-diff-comment-thread').length, 3);
.querySelectorAll('gr-comment-thread').length, 3);
});
test('toggle drafts only shows threads with draft comments', () => {
MockInteractions.tap(element.$.draftToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-diff-comment-thread').length, 2);
.querySelectorAll('gr-comment-thread').length, 2);
});
test('toggle drafts and unresolved only shows threads with drafts and ' +
@ -247,7 +247,7 @@ limitations under the License.
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
.querySelectorAll('gr-diff-comment-thread').length, 2);
.querySelectorAll('gr-comment-thread').length, 2);
});
test('modification events are consumed and displatched', () => {

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

@ -19,7 +19,7 @@ limitations under the License.
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<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="../../shared/gr-comment-thread/gr-comment-thread.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">

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

@ -601,7 +601,7 @@
},
_createThreadElement(thread) {
const threadEl = document.createElement('gr-diff-comment-thread');
const threadEl = document.createElement('gr-comment-thread');
threadEl.className = 'comment-thread';
threadEl.slot = `${thread.commentSide}-${thread.lineNum}`;
threadEl.comments = thread.comments;

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

@ -256,7 +256,7 @@ limitations under the License.
threadEls[0].dispatchEvent(
new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
const attachedThreads = element.queryAllEffectiveChildren(
'gr-diff-comment-thread');
'gr-comment-thread');
assert.equal(attachedThreads.length, 1);
assert.equal(attachedThreads[0].rootId, 42);
});
@ -1130,7 +1130,7 @@ limitations under the License.
commentSide, undefined, false));
let threads = Polymer.dom(element.$.diff)
.queryDistributedElements('gr-diff-comment-thread');
.queryDistributedElements('gr-comment-thread');
assert.equal(threads.length, 1);
assert.equal(threads[0].commentSide, commentSide);
@ -1151,7 +1151,7 @@ limitations under the License.
'3', 1, commentSide, range, true));
threads = Polymer.dom(element.$.diff)
.queryDistributedElements('gr-diff-comment-thread');
.queryDistributedElements('gr-comment-thread');
assert.equal(threads.length, 2);
assert.equal(threads[1].commentSide, commentSide);

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

@ -83,7 +83,7 @@
targetClasses.push(SelectionClass.BLAME);
} else {
const commentSelected =
this._elementDescendedFromClass(e.target, 'gr-diff-comment');
this._elementDescendedFromClass(e.target, 'gr-comment');
const side = this.diffBuilder.getSideByLineEl(lineEl);
targetClasses.push(side === 'left' ?

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

@ -221,7 +221,7 @@
[this.Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
[this.Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
// Final two are actually handled by gr-diff-comment-thread.
// Final two are actually handled by gr-comment-thread.
[this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
[this.Shortcut.COLLAPSE_ALL_COMMENT_THREADS]: null,
};

10
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html → polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
View File

@ -22,9 +22,9 @@ limitations under the License.
<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="../../shared/gr-storage/gr-storage.html">
<link rel="import" href="../gr-diff-comment/gr-diff-comment.html">
<link rel="import" href="../gr-comment/gr-comment.html">
<dom-module id="gr-diff-comment-thread">
<dom-module id="gr-comment-thread">
<template>
<style include="shared-styles">
gr-button {
@ -72,7 +72,7 @@ limitations under the License.
<div id="container" class$="[[_computeHostClass(unresolved)]]">
<template id="commentList" is="dom-repeat" items="[[_orderedComments]]"
as="comment">
<gr-diff-comment
<gr-comment
comment="{{comment}}"
robot-button-disabled="[[_hideActions(_showActions, _lastComment)]]"
change-num="[[changeNum]]"
@ -85,7 +85,7 @@ limitations under the License.
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
on-comment-discard="_handleCommentDiscard"
on-comment-save="_handleCommentSavedOrDiscarded"></gr-diff-comment>
on-comment-save="_handleCommentSavedOrDiscarded"></gr-comment>
</template>
<div id="commentInfoContainer"
hidden$="[[_hideActions(_showActions, _lastComment)]]">
@ -122,5 +122,5 @@ limitations under the License.
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
</template>
<script src="gr-diff-comment-thread.js"></script>
<script src="gr-comment-thread.js"></script>
</dom-module>

12
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js → polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
View File

@ -21,7 +21,7 @@
const NEWLINE_PATTERN = /\n/g;
Polymer({
is: 'gr-diff-comment-thread',
is: 'gr-comment-thread',
/**
* Fired when the thread should be discarded.
@ -36,7 +36,7 @@
*/
/**
* gr-diff-comment-thread exposes the following attributes that allow a
* gr-comment-thread exposes the following attributes that allow a
* diff widget like gr-diff to show the thread in the right location:
*
* line-num:
@ -219,7 +219,7 @@
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
// Don’t preventDefault in this case because it will render the event
// useless for other handlers (other gr-diff-comment-thread elements).
// useless for other handlers (other gr-comment-thread elements).
if (e.detail.keyboardEvent.shiftKey) {
this._expandCollapseComments(true);
} else {
@ -230,7 +230,7 @@
_expandCollapseComments(actionIsCollapse) {
const comments =
Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
Polymer.dom(this.root).querySelectorAll('gr-comment');
for (const comment of comments) {
comment.collapsed = actionIsCollapse;
}
@ -283,7 +283,7 @@
parent.range);
// If there is currently a comment in an editing state, add an attribute
// so that the gr-diff-comment knows not to populate the draft text.
// so that the gr-comment knows not to populate the draft text.
for (let i = 0; i < this.comments.length; i++) {
if (this.comments[i].__editing) {
reply.__otherEditing = true;
@ -350,7 +350,7 @@
},
_commentElWithDraftID(id) {
const els = Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
const els = Polymer.dom(this.root).querySelectorAll('gr-comment');
for (const el of els) {
if (el.comment.id === id || el.comment.__draftID === id) {
return el;

34
polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html → polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
View File

@ -17,31 +17,31 @@ limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-diff-comment-thread</title>
<title>gr-comment-thread</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
<script src="../../../scripts/util.js"></script>
<link rel="import" href="gr-diff-comment-thread.html">
<link rel="import" href="gr-comment-thread.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<gr-diff-comment-thread></gr-diff-comment-thread>
<gr-comment-thread></gr-comment-thread>
</template>
</test-fixture>
<test-fixture id="withComment">
<template>
<gr-diff-comment-thread></gr-diff-comment-thread>
<gr-comment-thread></gr-comment-thread>
</template>
</test-fixture>
<script>
suite('gr-diff-comment-thread tests', () => {
suite('gr-comment-thread tests', () => {
let element;
let sandbox;
@ -248,7 +248,7 @@ limitations under the License.
});
test('reply', () => {
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
const reportStub = sandbox.stub(element.$.reporting,
'recordDraftInteraction');
assert.ok(commentEl);
@ -267,7 +267,7 @@ limitations under the License.
});
test('quote reply', () => {
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
const reportStub = sandbox.stub(element.$.reporting,
'recordDraftInteraction');
assert.ok(commentEl);
@ -300,7 +300,7 @@ limitations under the License.
}];
flushAsynchronousOperations();
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const quoteBtn = element.$.quoteBtn;
@ -323,7 +323,7 @@ limitations under the License.
element.changeNum = '42';
element.patchNum = '1';
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const ackBtn = element.$.ackBtn;
@ -346,7 +346,7 @@ limitations under the License.
'recordDraftInteraction');
element.changeNum = '42';
element.patchNum = '1';
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const doneBtn = element.$.doneBtn;
@ -368,12 +368,12 @@ limitations under the License.
element.changeNum = '42';
element.patchNum = '1';
element.path = '/path/to/file.txt';
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
element.$$('gr-diff-comment')._fireSave();
element.$$('gr-comment')._fireSave();
flush(() => {
assert.isTrue(saveOrDiscardStub.called);
@ -389,7 +389,7 @@ limitations under the License.
test('please fix', done => {
element.changeNum = '42';
element.patchNum = '1';
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
commentEl.addEventListener('create-fix-comment', () => {
const drafts = element._orderedComments.filter(c => {
@ -420,7 +420,7 @@ limitations under the License.
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl =
Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
Polymer.dom(element.root).querySelectorAll('gr-comment')[1];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
const drafts = element.comments.filter(c => {
@ -452,7 +452,7 @@ limitations under the License.
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl =
Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[0];
Polymer.dom(element.root).querySelectorAll('gr-comment')[0];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
assert.equal(element.comments.length, 0);
@ -534,7 +534,7 @@ limitations under the License.
flushAsynchronousOperations();
const draftEl =
Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
Polymer.dom(element.root).querySelectorAll('gr-comment')[1];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
assert.isFalse(storageStub.called);
@ -546,7 +546,7 @@ limitations under the License.
});
test('comment-update', () => {
const commentEl = element.$$('gr-diff-comment');
const commentEl = element.$$('gr-comment');
const updatedComment = {
id: element.comments[0].id,
foo: 'bar',

4
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html → polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
View File

@ -35,7 +35,7 @@ limitations under the License.
<link rel="import" href="../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html">
<script src="../../../scripts/rootElement.js"></script>
<dom-module id="gr-diff-comment">
<dom-module id="gr-comment">
<template>
<style include="shared-styles">
:host {
@ -384,5 +384,5 @@ limitations under the License.
<gr-storage id="storage"></gr-storage>
<gr-reporting id="reporting"></gr-reporting>
</template>
<script src="gr-diff-comment.js"></script>
<script src="gr-comment.js"></script>
</dom-module>

2
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js → polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
View File

@ -32,7 +32,7 @@
const FILE = 'FILE';
Polymer({
is: 'gr-diff-comment',
is: 'gr-comment',
/**
* Fired when the create fix comment action is triggered.

12
polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html → polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
View File

@ -17,7 +17,7 @@ limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-diff-comment</title>
<title>gr-comment</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
@ -25,19 +25,19 @@ limitations under the License.
<script src="../../../bower_components/page/page.js"></script>
<script src="../../../scripts/util.js"></script>
<link rel="import" href="gr-diff-comment.html">
<link rel="import" href="gr-comment.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
<gr-diff-comment></gr-diff-comment>
<gr-comment></gr-comment>
</template>
</test-fixture>
<test-fixture id="draft">
<template>
<gr-diff-comment draft="true"></gr-diff-comment>
<gr-comment draft="true"></gr-comment>
</template>
</test-fixture>
@ -48,7 +48,7 @@ limitations under the License.
return getComputedStyle(el).getPropertyValue('display') !== 'none';
}
suite('gr-diff-comment tests', () => {
suite('gr-comment tests', () => {
let element;
let sandbox;
setup(() => {
@ -343,7 +343,7 @@ limitations under the License.
});
});
suite('gr-diff-comment draft tests', () => {
suite('gr-comment draft tests', () => {
let element;
let sandbox;

4
polygerrit-ui/app/test/index.html
View File

@ -103,8 +103,6 @@ limitations under the License.
'core/gr-smart-search/gr-smart-search_test.html',
'diff/gr-comment-api/gr-comment-api_test.html',
'diff/gr-diff-builder/gr-diff-builder_test.html',
'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
'diff/gr-diff-comment/gr-diff-comment_test.html',
'diff/gr-diff-cursor/gr-diff-cursor_test.html',
'diff/gr-diff-highlight/gr-annotation_test.html',
'diff/gr-diff-highlight/gr-diff-highlight_test.html',
@ -157,6 +155,8 @@ limitations under the License.
'shared/gr-button/gr-button_test.html',
'shared/gr-change-star/gr-change-star_test.html',
'shared/gr-change-status/gr-change-status_test.html',
'shared/gr-comment-thread/gr-comment-thread_test.html',
'shared/gr-comment/gr-comment_test.html',
'shared/gr-copy-clipboard/gr-copy-clipboard_test.html',
'shared/gr-cursor-manager/gr-cursor-manager_test.html',
'shared/gr-date-formatter/gr-date-formatter_test.html',