Add long range comment chip
https://imgur.com/a/6pK4RnS
Change-Id: Ib524ee0bd837d8a02c0d4276e7b9ce5593d38874
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
index c788d76..a5c6213 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.ts
@@ -162,17 +162,35 @@
`.range.${strToClassName(threadEl.rootId)}`
);
rangeNodes.forEach(rangeNode => {
- rangeNode.classList.add('rangeHighlight');
- rangeNode.classList.remove('range');
+ rangeNode.classList.add('rangeHoverHighlight');
});
+ const chipNode = threadEl.parentElement?.querySelector(
+ `gr-ranged-comment-chip[threadElRootId="${threadEl.rootId}"]`
+ );
+ if (chipNode) {
+ chipNode.shadowRoot
+ ?.querySelectorAll('.rangeHighlight')
+ .forEach(highlightNode =>
+ highlightNode.classList.add('rangeHoverHighlight')
+ );
+ }
} else {
const rangeNodes = curNode.querySelectorAll(
- `.rangeHighlight.${strToClassName(threadEl.rootId)}`
+ `.rangeHoverHighlight.${strToClassName(threadEl.rootId)}`
);
rangeNodes.forEach(rangeNode => {
- rangeNode.classList.remove('rangeHighlight');
- rangeNode.classList.add('range');
+ rangeNode.classList.remove('rangeHoverHighlight');
});
+ const chipNode = threadEl.parentElement?.querySelector(
+ `gr-ranged-comment-chip[threadElRootId="${threadEl.rootId}"]`
+ );
+ if (chipNode) {
+ chipNode.shadowRoot
+ ?.querySelectorAll('.rangeHoverHighlight')
+ .forEach(highlightNode =>
+ highlightNode.classList.remove('rangeHoverHighlight')
+ );
+ }
}
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
index 038153a..1e5a8d3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-utils.ts
@@ -39,6 +39,10 @@
);
}
+export function isLongCommentRange(range: CommentRange): boolean {
+ return range.end_line - range.start_line > 5;
+}
+
export function getLineNumber(lineEl?: Element | null): LineNumber | null {
if (!lineEl) return null;
const lineNumberStr = lineEl.getAttribute('data-value');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
index 39d645e..661d5d9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.ts
@@ -22,6 +22,7 @@
import '../gr-diff-selection/gr-diff-selection';
import '../gr-syntax-themes/gr-syntax-theme';
import '../gr-ranged-comment-themes/gr-ranged-comment-theme';
+import '../gr-ranged-comment-chip/gr-ranged-comment-chip';
import {PolymerElement} from '@polymer/polymer/polymer-element';
import {dom, EventApi} from '@polymer/polymer/lib/legacy/polymer.dom';
import {GestureEventListeners} from '@polymer/polymer/lib/mixins/gesture-event-listeners';
@@ -34,6 +35,7 @@
getRange,
getSide,
GrDiffThreadElement,
+ isLongCommentRange,
isThreadEl,
rangesEqual,
} from './gr-diff-utils';
@@ -873,6 +875,7 @@
for (const threadEl of addedThreadEls) {
const lineNum = getLine(threadEl);
const commentSide = getSide(threadEl);
+ const range = getRange(threadEl);
if (!commentSide) continue;
const lineEl = this.$.diffBuilder.getLineElByNumber(
lineNum,
@@ -896,6 +899,18 @@
contentEl,
commentSide
);
+
+ const slotAtt = threadEl.getAttribute('slot');
+ if (range && isLongCommentRange(range) && slotAtt) {
+ const longRangeCommentChip = document.createElement(
+ 'gr-ranged-comment-chip'
+ );
+ longRangeCommentChip.range = range;
+ longRangeCommentChip.setAttribute('threadElRootId', threadEl.rootId);
+ longRangeCommentChip.setAttribute('slot', slotAtt);
+ this.insertBefore(longRangeCommentChip, threadEl);
+ }
+
// Create a slot for the thread and attach it to the thread group.
// The Polyfill has some bugs and this only works if the slot is
// attached to the group after the group is attached to the DOM.
@@ -903,7 +918,6 @@
// that is okay because the first matching slot is used and the rest
// are ignored.
const slot = document.createElement('slot') as HTMLSlotElement;
- const slotAtt = threadEl.getAttribute('slot');
if (slotAtt) slot.name = slotAtt;
threadGroupEl.appendChild(slot);
lastEl = threadEl;
@@ -915,6 +929,13 @@
if (lastEl && lastEl.replaceWith) {
lastEl.replaceWith(lastEl);
}
+
+ const removedThreadEls = info.removedNodes.filter(isThreadEl);
+ for (const threadEl of removedThreadEls) {
+ this.querySelector(
+ `gr-ranged-comment-chip[threadElRootId="${threadEl.rootId}"]`
+ )?.remove();
+ }
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
index c5a8db4..da893c9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.js
@@ -19,7 +19,6 @@
import '../../shared/gr-rest-api-interface/gr-rest-api-interface.js';
import {getMockDiffResponse} from '../../../test/mocks/diff-response.js';
import './gr-diff.js';
-import {flush} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {GrDiffBuilderImage} from '../gr-diff-builder/gr-diff-builder-image.js';
import {getComputedStyleValue} from '../../../utils/dom-util.js';
import {_setHiddenScroll} from '../../../scripts/hiddenscroll.js';
@@ -678,6 +677,64 @@
assert.isFalse(element._createComment.called);
});
+ test('adds long range comment chip', async () => {
+ const range = {
+ start_line: 1,
+ end_line: 7,
+ start_character: 0,
+ end_character: 0,
+ };
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('diff-side', 'right');
+ threadEl.setAttribute('line-num', 1);
+ threadEl.setAttribute('range', JSON.stringify(range));
+ threadEl.setAttribute('slot', 'right-1');
+ const content = [{
+ a: [],
+ b: [],
+ }, {
+ ab: Array(8).fill('text'),
+ }];
+ setupSampleDiff({content});
+
+ element.appendChild(threadEl);
+ await flush();
+
+ assert.deepEqual(
+ element.querySelector('gr-ranged-comment-chip').range, range);
+ });
+
+ test('removes long range comment chip when comment is discarded',
+ async () => {
+ const range = {
+ start_line: 1,
+ end_line: 7,
+ start_character: 0,
+ end_character: 0,
+ };
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('diff-side', 'right');
+ threadEl.setAttribute('line-num', 1);
+ threadEl.setAttribute('range', JSON.stringify(range));
+ threadEl.setAttribute('slot', 'right-1');
+ const content = [{
+ a: [],
+ b: [],
+ }, {
+ ab: Array(8).fill('text'),
+ }];
+ setupSampleDiff({content});
+ element.appendChild(threadEl);
+ await flush();
+
+ threadEl.remove();
+ await flush();
+
+ assert.isEmpty(element.querySelectorAll('gr-ranged-comment-chip'));
+ });
+
suite('change in preferences', () => {
setup(() => {
element.diff = {
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip.ts
new file mode 100644
index 0000000..6948283
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip.ts
@@ -0,0 +1,42 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import {customElement, property} from '@polymer/decorators';
+import {CommentRange} from '../../../types/common';
+import {htmlTemplate} from './gr-ranged-comment-chip_html';
+import {PolymerElement} from '@polymer/polymer/polymer-element';
+
+@customElement('gr-ranged-comment-chip')
+export class GrRangedCommentChip extends PolymerElement {
+ static get template() {
+ return htmlTemplate;
+ }
+
+ @property({type: Object})
+ range?: CommentRange;
+
+ _computeRangeLabel(range?: CommentRange): string {
+ if (!range) return '';
+ return `Long comment range ${range.start_line} - ${range.end_line}`;
+ }
+}
+
+declare global {
+ interface HTMLElementTagNameMap {
+ 'gr-ranged-comment-chip': GrRangedCommentChip;
+ }
+}
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_html.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_html.ts
new file mode 100644
index 0000000..62f23d7
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_html.ts
@@ -0,0 +1,50 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import {html} from '@polymer/polymer/lib/utils/html-tag';
+
+export const htmlTemplate = html`
+ <style include="gr-ranged-comment-theme">
+ /* Workaround for empty style block - see https://github.com/Polymer/tools/issues/408 */
+ </style>
+ <style include="shared-styles">
+ .row {
+ color: var(--ranged-comment-chip-text-color);
+ display: flex;
+ font-family: var(--font-family), Roboto;
+ justify-content: flex-end;
+ margin: var(--spacing-xs) 0;
+ }
+ .icon {
+ color: var(--ranged-comment-chip-text-color);
+ height: 16px;
+ width: 16px;
+ }
+ .chip {
+ background-color: var(--ranged-comment-chip-background);
+ border-radius: 24px;
+ margin: var(--spacing-s);
+ padding: var(--spacing-s);
+ padding-right: var(--spacing-m);
+ }
+ </style>
+ <div class="row rangeHighlight">
+ <div class="chip font-normal">
+ <iron-icon class="icon" icon="gr-icons:comment-outline"></iron-icon>
+ [[_computeRangeLabel(range)]]
+ </div>
+ </div>
+`;
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_test.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_test.ts
new file mode 100644
index 0000000..8bce99d
--- /dev/null
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-chip/gr-ranged-comment-chip_test.ts
@@ -0,0 +1,40 @@
+/**
+ * @license
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import '../../../test/common-test-setup-karma';
+import {CommentRange} from '../../../types/common';
+import {GrRangedCommentChip} from './gr-ranged-comment-chip';
+
+suite('gr-ranged-comment-chip tests', () => {
+ let element: GrRangedCommentChip;
+
+ setup(() => {
+ element = fixtureFromElement('gr-ranged-comment-chip').instantiate();
+ });
+
+ test('shows line range', async () => {
+ element.range = {
+ start_line: 2,
+ start_character: 1,
+ end_line: 5,
+ end_character: 3,
+ } as CommentRange;
+ await flush();
+ const textDiv = element.root!.querySelector<HTMLDivElement>('.chip');
+ assert.equal(textDiv!.innerText.trim(), 'Long comment range 2 - 5');
+ });
+});
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
index bb3733f..f33c2ce 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.ts
@@ -29,6 +29,7 @@
} from '@polymer/polymer/interfaces';
import {CommentRange} from '../../../types/common';
import {DiffLayer, DiffLayerListener} from '../../../types/types';
+import {isLongCommentRange} from '../gr-diff/gr-diff-utils';
/**
* Enhanced CommentRange by UI state. Interface for incoming ranges set from the
@@ -49,6 +50,7 @@
*/
interface CommentRangeLineLayer {
hovering: boolean;
+ longRange: boolean;
rootId: string;
start: number;
end: number;
@@ -65,8 +67,9 @@
// Polymer 1 adds # before array's key, while Polymer 2 doesn't
const HOVER_PATH_PATTERN = /^(commentRanges\.#?\d+)\.hovering$/;
-const RANGE_HIGHLIGHT = 'style-scope gr-diff range';
-const HOVER_HIGHLIGHT = 'style-scope gr-diff rangeHighlight';
+const RANGE_BASE_ONLY = 'style-scope gr-diff range';
+const RANGE_HIGHLIGHT = 'style-scope gr-diff range rangeHighlight';
+const HOVER_HIGHLIGHT = 'style-scope gr-diff range rangeHoverHighlight';
@customElement('gr-ranged-comment-layer')
export class GrRangedCommentLayer
@@ -125,8 +128,11 @@
el,
range.start,
range.end - range.start,
- (range.hovering ? HOVER_HIGHLIGHT : RANGE_HIGHLIGHT) +
- ` ${strToClassName(range.rootId)}`
+ (range.hovering
+ ? HOVER_HIGHLIGHT
+ : range.longRange
+ ? RANGE_BASE_ONLY
+ : RANGE_HIGHLIGHT) + ` ${strToClassName(range.rootId)}`
);
}
}
@@ -169,12 +175,13 @@
const value = record.value as CommentRangeLayer[];
this._rangesMap = {left: {}, right: {}};
for (const {side, range, rootId, hovering} of value) {
+ const longRange = isLongCommentRange(range);
this._updateRangesMap({
side,
range,
hovering,
operation: (forLine, start, end, hovering) => {
- forLine.push({start, end, hovering, rootId});
+ forLine.push({start, end, hovering, rootId, longRange});
},
});
}
@@ -228,12 +235,13 @@
indexSplice.index + indexSplice.addedCount
);
for (const {side, range, hovering, rootId} of added) {
+ const longRange = isLongCommentRange(range);
this._updateRangesMap({
side,
range,
hovering,
operation: (forLine, start, end, hovering) => {
- forLine.push({start, end, hovering, rootId});
+ forLine.push({start, end, hovering, rootId, longRange});
},
});
}
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
index 441d585..fad717d 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.js
@@ -68,6 +68,15 @@
},
rootId: 'd',
},
+ {
+ side: 'right',
+ range: {
+ end_character: 1,
+ end_line: 70,
+ start_character: 1,
+ start_line: 60,
+ },
+ },
];
element = basicFixture.instantiate();
@@ -110,7 +119,10 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_a');
+ assert.equal(
+ lastCall.args[3],
+ 'style-scope gr-diff range rangeHighlight generated_a'
+ );
});
test('type=Remove has-comment hovering', () => {
@@ -129,7 +141,8 @@
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
assert.equal(
- lastCall.args[3], 'style-scope gr-diff rangeHighlight generated_a'
+ lastCall.args[3],
+ 'style-scope gr-diff range rangeHoverHighlight generated_a'
);
});
@@ -147,7 +160,10 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_a');
+ assert.equal(
+ lastCall.args[3],
+ 'style-scope gr-diff range rangeHighlight generated_a'
+ );
});
test('type=Both has-comment off side', () => {
@@ -175,7 +191,24 @@
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
- assert.equal(lastCall.args[3], 'style-scope gr-diff range generated_b');
+ assert.equal(
+ lastCall.args[3],
+ 'style-scope gr-diff range rangeHighlight generated_b'
+ );
+ });
+
+ test('long range comment', () => {
+ line.type = GrDiffLineType.ADD;
+ line.afterNumber = 65;
+ el.setAttribute('data-side', 'right');
+
+ element.annotate(el, lineNumberEl, line);
+
+ assert.isTrue(annotateElementStub.called);
+ assert.equal(
+ annotateElementStub.lastCall.args[3],
+ 'style-scope gr-diff range generated_'
+ );
});
});
@@ -281,10 +314,10 @@
assert.equal(element._rangesMap.left[39][0].start, 0);
assert.equal(element._rangesMap.left[39][0].end, 9);
- // The right has two ranged comments, one spanning ll.10-12 and the other
- // on line 100.
+ // The right has four ranged comments: 10-12, 55-55, 60-70, 100-100
const rightKeys = [];
for (let i = 10; i <= 12; i++) { rightKeys.push('' + i); }
+ for (let i = 60; i <= 70; i++) { rightKeys.push('' + i); }
rightKeys.push('55', '100');
assert.deepEqual(Object.keys(element._rangesMap.right).sort(),
rightKeys.sort());
@@ -318,4 +351,3 @@
assert.equal(range.end, line.text.length);
});
});
-
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts b/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
index 70ee196..ee9e8f9 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-themes/gr-ranged-comment-theme.ts
@@ -25,11 +25,11 @@
$_documentContainer.innerHTML = `<dom-module id="gr-ranged-comment-theme">
<template>
<style>
- .range {
+ .rangeHighlight {
background-color: var(--diff-highlight-range-color);
display: inline;
}
- .rangeHighlight {
+ .rangeHoverHighlight {
background-color: var(--diff-highlight-range-hover-color);
display: inline;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
index 1170477..a423d3a 100644
--- a/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
+++ b/polygerrit-ui/app/elements/shared/gr-icons/gr-icons.ts
@@ -63,6 +63,8 @@
<g id="hourglass"><path d="M6 2v6h.01L6 8.01 10 12l-4 4 .01.01H6V22h12v-5.99h-.01L18 16l-4-4 4-3.99-.01-.01H18V2H6z"></path><path d="M0 0h24v24H0V0z" fill="none"></path></g>
<!-- This SVG is a copy from material.io https://material.io/icons/#mode_comment-->
<g id="comment"><path d="M21.99 4c0-1.1-.89-2-1.99-2H4c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h14l4 4-.01-18z"></path><path d="M0 0h24v24H0z" fill="none"></path></g>
+ <!-- This SVG is a copy from material.io https://material.io/resources/icons/?icon=mode_comment&style=outline-->
+ <g id="comment-outline"><path d="M0 0h24v24H0V0z" fill="none"></path><path d="M20 17.17L18.83 16H4V4h16v13.17zM20 2H4c-1.1 0-2 .9-2 2v12c0 1.1.9 2 2 2h14l4 4V4c0-1.1-.9-2-2-2z"></path></g>
<!-- This SVG is a copy from material.io https://material.io/icons/#calendar_today-->
<g id="calendar"><path d="M20 3h-1V1h-2v2H7V1H5v2H4c-1.1 0-2 .9-2 2v16c0 1.1.9 2 2 2h16c1.1 0 2-.9 2-2V5c0-1.1-.9-2-2-2zm0 18H4V8h16v13z"></path><path d="M0 0h24v24H0z" fill="none"></path></g>
<!-- This SVG is a copy from iron-icons https://github.com/PolymerElements/iron-icons/blob/master/iron-icons.js -->
