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 -->
diff --git a/polygerrit-ui/app/styles/themes/app-theme.ts b/polygerrit-ui/app/styles/themes/app-theme.ts index 73dfe5c..1f03f7b 100644 --- a/polygerrit-ui/app/styles/themes/app-theme.ts +++ b/polygerrit-ui/app/styles/themes/app-theme.ts
@@ -182,6 +182,8 @@ --light-remove-highlight-color: #ffebee; --coverage-covered: #e0f2f1; --coverage-not-covered: #ffd1a4; + --ranged-comment-chip-background: #b06000; + --ranged-comment-chip-text-color: #feefe3; /* syntax colors */ --syntax-attr-color: #219;
diff --git a/polygerrit-ui/app/styles/themes/dark-theme.ts b/polygerrit-ui/app/styles/themes/dark-theme.ts index c7c8eb1..0b82a61 100644 --- a/polygerrit-ui/app/styles/themes/dark-theme.ts +++ b/polygerrit-ui/app/styles/themes/dark-theme.ts
@@ -133,6 +133,8 @@ --light-remove-highlight-color: #320404; --coverage-covered: #112826; --coverage-not-covered: #6b3600; + --ranged-comment-chip-background: #e8f0fe; + --ranged-comment-chip-text-color: #174ea6; /* syntax colors */ --syntax-attr-color: #80cbbf;