opendev/gerrit
Code Issues Proposed changes

Allow annotation layers to modify the line number element

Browse Source 
Change-Id: I45195834b560eca3cbe0d4f365cdc1e071935674
This commit is contained in:
brohlfs
2019-01-23 10:38:37 +01:00
parent a5d9e07917
commit 1ec643f764
15 changed files with 194 additions and 84 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/README.md
View File

@@ -123,7 +123,9 @@ For interactively working on a single test file, do the following:
./polygerrit-ui/run-server.sh ./polygerrit-ui/run-server.sh
``` ```
Then visit http://localhost:8081/elements/foo/bar_test.html Then visit http://localhost:8081/elements/foo/bar_test.html and check "Disable
cache" in the "Network" tab of Chrome's dev tools, so code changes are picked
up on "reload".
To run Chrome tests in headless mode: To run Chrome tests in headless mode:

8
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
View File

@@ -89,14 +89,14 @@
GrDiffBuilderSideBySide.prototype._appendPair = function(section, row, line, GrDiffBuilderSideBySide.prototype._appendPair = function(section, row, line,
lineNumber, side) { lineNumber, side) {
const lineEl = this._createLineEl(line, lineNumber, line.type, side); const lineNumberEl = this._createLineEl(line, lineNumber, line.type, side);
lineEl.classList.add(side); lineNumberEl.classList.add(side);
row.appendChild(lineEl); row.appendChild(lineNumberEl);
const action = this._createContextControl(section, line); const action = this._createContextControl(section, line);
if (action) { if (action) {
row.appendChild(action); row.appendChild(action);
} else { } else {
const textEl = this._createTextEl(line, side); const textEl = this._createTextEl(lineNumberEl, line, side);
row.appendChild(textEl); row.appendChild(textEl);
} }
}; };

22
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
View File

@@ -68,24 +68,24 @@
GrDiffBuilderUnified.prototype._createRow = function(section, line) { GrDiffBuilderUnified.prototype._createRow = function(section, line) {
const row = this._createElement('tr', line.type); const row = this._createElement('tr', line.type);
row.appendChild(this._createBlameCell(line));
let lineEl = this._createLineEl(line, line.beforeNumber,
GrDiffLine.Type.REMOVE);
lineEl.classList.add('left');
row.appendChild(lineEl);
lineEl = this._createLineEl(line, line.afterNumber,
GrDiffLine.Type.ADD);
lineEl.classList.add('right');
row.appendChild(lineEl);
row.classList.add('diff-row', 'unified'); row.classList.add('diff-row', 'unified');
row.tabIndex = -1; row.tabIndex = -1;
row.appendChild(this._createBlameCell(line));
let lineNumberEl = this._createLineEl(line, line.beforeNumber,
GrDiffLine.Type.REMOVE);
lineNumberEl.classList.add('left');
row.appendChild(lineNumberEl);
lineNumberEl = this._createLineEl(line, line.afterNumber,
GrDiffLine.Type.ADD);
lineNumberEl.classList.add('right');
row.appendChild(lineNumberEl);
const action = this._createContextControl(section, line); const action = this._createContextControl(section, line);
if (action) { if (action) {
row.appendChild(action); row.appendChild(action);
} else { } else {
const textEl = this._createTextEl(line); const textEl = this._createTextEl(lineNumberEl, line);
row.appendChild(textEl); row.appendChild(textEl);
} }
return row; return row;

12
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
View File

@@ -331,7 +331,7 @@ limitations under the License.
// Take a DIV.contentText element and a line object with intraline // Take a DIV.contentText element and a line object with intraline
// differences to highlight and apply them to the element as // differences to highlight and apply them to the element as
// annotations. // annotations.
annotate(el, line) { annotate(contentEl, lineNumberEl, line) {
const HL_CLASS = 'style-scope gr-diff intraline'; const HL_CLASS = 'style-scope gr-diff intraline';
for (const highlight of line.highlights) { for (const highlight of line.highlights) {
// The start and end indices could be the same if a highlight is // The start and end indices could be the same if a highlight is
@@ -345,7 +345,7 @@ limitations under the License.
highlight.endIndex; highlight.endIndex;
GrAnnotation.annotateElement( GrAnnotation.annotateElement(
el, contentEl,
highlight.startIndex, highlight.startIndex,
endIndex - highlight.startIndex, endIndex - highlight.startIndex,
HL_CLASS); HL_CLASS);
@@ -357,7 +357,7 @@ limitations under the License.
_createTabIndicatorLayer() { _createTabIndicatorLayer() {
const show = () => this._showTabs; const show = () => this._showTabs;
return { return {
annotate(el, line) { annotate(contentEl, lineNumberEl, line) {
// If visible tabs are disabled, do nothing. // If visible tabs are disabled, do nothing.
if (!show()) { return; } if (!show()) { return; }
@@ -368,7 +368,7 @@ limitations under the License.
// Skip forward by the length of the content // Skip forward by the length of the content
pos += split[i].length; pos += split[i].length;
GrAnnotation.annotateElement(el, pos, 1, GrAnnotation.annotateElement(contentEl, pos, 1,
'style-scope gr-diff tab-indicator'); 'style-scope gr-diff tab-indicator');
// Skip forward by one tab character. // Skip forward by one tab character.
@@ -384,7 +384,7 @@ limitations under the License.
}.bind(this); }.bind(this);
return { return {
annotate(el, line) { annotate(contentEl, lineNumberEl, line) {
if (!show()) { return; } if (!show()) { return; }
const match = line.text.match(TRAILING_WHITESPACE_PATTERN); const match = line.text.match(TRAILING_WHITESPACE_PATTERN);
@@ -394,7 +394,7 @@ limitations under the License.
const index = GrAnnotation.getStringLength( const index = GrAnnotation.getStringLength(
line.text.substr(0, match.index)); line.text.substr(0, match.index));
const length = GrAnnotation.getStringLength(match[0]); const length = GrAnnotation.getStringLength(match[0]);
GrAnnotation.annotateElement(el, index, length, GrAnnotation.annotateElement(contentEl, index, length,
'style-scope gr-diff trailing-whitespace'); 'style-scope gr-diff trailing-whitespace');
} }
}, },

22
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
View File

@@ -218,7 +218,9 @@
// if lines are collapsed and not visible on the page yet. // if lines are collapsed and not visible on the page yet.
continue; continue;
} }
el.parentElement.replaceChild(this._createTextEl(line, side).firstChild, const lineNumberEl = this._getLineNumberEl(el, side);
el.parentElement.replaceChild(
this._createTextEl(lineNumberEl, line, side).firstChild,
el); el);
} }
}; };
@@ -343,7 +345,8 @@
return td; return td;
}; };
GrDiffBuilder.prototype._createTextEl = function(line, opt_side) { GrDiffBuilder.prototype._createTextEl = function(
lineNumberEl, line, opt_side) {
const td = this._createElement('td'); const td = this._createElement('td');
if (line.type !== GrDiffLine.Type.BLANK) { if (line.type !== GrDiffLine.Type.BLANK) {
td.classList.add('content'); td.classList.add('content');
@@ -360,7 +363,7 @@
} }
for (const layer of this.layers) { for (const layer of this.layers) {
layer.annotate(contentText, line); layer.annotate(contentText, lineNumberEl, line);
} }
td.appendChild(contentText); td.appendChild(contentText);
@@ -594,5 +597,18 @@
return blameTd; return blameTd;
}; };
/**
* Finds the line number element given the content element by walking up the
* DOM tree to the diff row and then querying for a .lineNum element on the
* requested side.
*
* TODO(brohlfs): Consolidate this with getLineEl... methods in html file.
*/
GrDiffBuilder.prototype._getLineNumberEl = function(content, side) {
let row = content;
while (row && !row.classList.contains('diff-row')) row = row.parentElement;
return row ? row.querySelector('.lineNum.' + side) : null;
};
window.GrDiffBuilder = GrDiffBuilder; window.GrDiffBuilder = GrDiffBuilder;
})(window, GrDiffGroup, GrDiffLine); })(window, GrDiffGroup, GrDiffLine);

101
polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
View File

@@ -163,7 +163,7 @@ limitations under the License.
const text = 'a'.repeat(51); const text = 'a'.repeat(51);
const line = {text, highlights: []}; const line = {text, highlights: []};
const result = builder._createTextEl(line).firstChild.innerHTML; const result = builder._createTextEl(undefined, line).firstChild.innerHTML;
assert.equal(result, text); assert.equal(result, text);
}); });
@@ -173,14 +173,14 @@ limitations under the License.
const line = {text, highlights: []}; const line = {text, highlights: []};
const expected = 'a'.repeat(50) + LINE_FEED_HTML + 'a'; const expected = 'a'.repeat(50) + LINE_FEED_HTML + 'a';
const result = builder._createTextEl(line).firstChild.innerHTML; const result = builder._createTextEl(undefined, line).firstChild.innerHTML;
assert.equal(result, expected); assert.equal(result, expected);
}); });
test('_createTextEl linewrap with tabs', () => { test('_createTextEl linewrap with tabs', () => {
const text = '\t'.repeat(7) + '!'; const text = '\t'.repeat(7) + '!';
const line = {text, highlights: []}; const line = {text, highlights: []};
const el = builder._createTextEl(line); const el = builder._createTextEl(undefined, line);
assert.equal(el.innerText, text); assert.equal(el.innerText, text);
// With line length 10 and tab size 2, there should be a line break // With line length 10 and tab size 2, there should be a line break
// after every two tabs. // after every two tabs.
@@ -306,6 +306,7 @@ limitations under the License.
let str; let str;
let annotateElementSpy; let annotateElementSpy;
let layer; let layer;
const lineNumberEl = document.createElement('td');
function slice(str, start, end) { function slice(str, start, end) {
return Array.from(str).slice(start, end).join(''); return Array.from(str).slice(start, end).join('');
@@ -325,7 +326,7 @@ limitations under the License.
highlights: [], highlights: [],
}; };
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
// The content is unchanged. // The content is unchanged.
assert.isFalse(annotateElementSpy.called); assert.isFalse(annotateElementSpy.called);
@@ -348,7 +349,7 @@ limitations under the License.
const str3 = slice(str, 18, 22); const str3 = slice(str, 18, 22);
const str4 = slice(str, 22); const str4 = slice(str, 22);
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementSpy.called); assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 5); assert.equal(el.childNodes.length, 5);
@@ -380,7 +381,7 @@ limitations under the License.
const str0 = slice(str, 0, 28); const str0 = slice(str, 0, 28);
const str1 = slice(str, 28); const str1 = slice(str, 28);
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementSpy.called); assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 2); assert.equal(el.childNodes.length, 2);
@@ -400,7 +401,7 @@ limitations under the License.
], ],
}; };
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementSpy.called); assert.isFalse(annotateElementSpy.called);
assert.equal(el.childNodes.length, 1); assert.equal(el.childNodes.length, 1);
@@ -421,7 +422,7 @@ limitations under the License.
const str1 = slice(str, 6, 12); const str1 = slice(str, 6, 12);
const str2 = slice(str, 12); const str2 = slice(str, 12);
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementSpy.called); assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 3); assert.equal(el.childNodes.length, 3);
@@ -451,7 +452,7 @@ limitations under the License.
const str0 = slice(str, 0, 6); const str0 = slice(str, 0, 6);
const str1 = slice(str, 6); const str1 = slice(str, 6);
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementSpy.called); assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 2); assert.equal(el.childNodes.length, 2);
@@ -467,6 +468,7 @@ limitations under the License.
suite('tab indicators', () => { suite('tab indicators', () => {
let element; let element;
let layer; let layer;
const lineNumberEl = document.createElement('td');
setup(() => { setup(() => {
element = fixture('basic'); element = fixture('basic');
@@ -480,7 +482,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -493,7 +495,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -506,7 +508,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.equal(annotateElementStub.callCount, 1); assert.equal(annotateElementStub.callCount, 1);
const args = annotateElementStub.getCalls()[0].args; const args = annotateElementStub.getCalls()[0].args;
@@ -526,7 +528,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -539,7 +541,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.equal(annotateElementStub.callCount, 2); assert.equal(annotateElementStub.callCount, 2);
@@ -564,7 +566,7 @@ limitations under the License.
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.equal(annotateElementStub.callCount, 1); assert.equal(annotateElementStub.callCount, 1);
const args = annotateElementStub.getCalls()[0].args; const args = annotateElementStub.getCalls()[0].args;
@@ -606,6 +608,7 @@ limitations under the License.
suite('trailing whitespace', () => { suite('trailing whitespace', () => {
let element; let element;
let layer; let layer;
const lineNumberEl = document.createElement('td');
setup(() => { setup(() => {
element = fixture('basic'); element = fixture('basic');
@@ -618,7 +621,7 @@ limitations under the License.
const el = document.createElement('div'); const el = document.createElement('div');
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -629,7 +632,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -640,7 +643,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
assert.equal(annotateElementStub.lastCall.args[1], 11); assert.equal(annotateElementStub.lastCall.args[1], 11);
assert.equal(annotateElementStub.lastCall.args[2], 3); assert.equal(annotateElementStub.lastCall.args[2], 3);
@@ -653,7 +656,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
assert.equal(annotateElementStub.lastCall.args[1], 11); assert.equal(annotateElementStub.lastCall.args[1], 11);
assert.equal(annotateElementStub.lastCall.args[2], 3); assert.equal(annotateElementStub.lastCall.args[2], 3);
@@ -666,7 +669,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
assert.equal(annotateElementStub.lastCall.args[1], 11); assert.equal(annotateElementStub.lastCall.args[1], 11);
assert.equal(annotateElementStub.lastCall.args[2], 3); assert.equal(annotateElementStub.lastCall.args[2], 3);
@@ -679,7 +682,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
assert.equal(annotateElementStub.lastCall.args[1], 1); assert.equal(annotateElementStub.lastCall.args[1], 1);
assert.equal(annotateElementStub.lastCall.args[2], 1); assert.equal(annotateElementStub.lastCall.args[2], 1);
@@ -693,7 +696,7 @@ limitations under the License.
el.textContent = str; el.textContent = str;
const annotateElementStub = const annotateElementStub =
sandbox.stub(GrAnnotation, 'annotateElement'); sandbox.stub(GrAnnotation, 'annotateElement');
layer.annotate(el, line); layer.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
}); });
@@ -964,7 +967,7 @@ limitations under the License.
assert.equal(spy.callCount, count); assert.equal(spy.callCount, count);
spy.getCalls().forEach((call, i) => { spy.getCalls().forEach((call, i) => {
assert.equal(call.args[0].beforeNumber, start + i); assert.equal(call.args[1].beforeNumber, start + i);
}); });
}); });
@@ -975,9 +978,11 @@ limitations under the License.
(s, e, d, lines, elements) => { (s, e, d, lines, elements) => {
// Add a line and a corresponding element. // Add a line and a corresponding element.
lines.push(new GrDiffLine(GrDiffLine.Type.BOTH)); lines.push(new GrDiffLine(GrDiffLine.Type.BOTH));
const parEl = document.createElement('div'); const tr = document.createElement('tr');
const td = document.createElement('td');
const el = document.createElement('div'); const el = document.createElement('div');
parEl.appendChild(el); tr.appendChild(td);
td.appendChild(el);
elements.push(el); elements.push(el);
// Add 2 lines without corresponding elements. // Add 2 lines without corresponding elements.
@@ -991,6 +996,52 @@ limitations under the License.
assert.equal(spy.callCount, 1); assert.equal(spy.callCount, 1);
}); });
test('_getLineNumberEl side-by-side left', () => {
const contentEl = builder.getContentByLine(5, 'left',
element.$.diffTable);
const lineNumberEl = builder._getLineNumberEl(contentEl, 'left');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('left'));
});
test('_getLineNumberEl side-by-side right', () => {
const contentEl = builder.getContentByLine(5, 'right',
element.$.diffTable);
const lineNumberEl = builder._getLineNumberEl(contentEl, 'right');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('right'));
});
test('_getLineNumberEl unified left', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const contentEl = builder.getContentByLine(5, 'left',
element.$.diffTable);
const lineNumberEl = builder._getLineNumberEl(contentEl, 'left');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('left'));
done();
});
});
test('_getLineNumberEl unified right', done => {
// Re-render as unified:
element.viewMode = 'UNIFIED_DIFF';
element.render(keyLocations, prefs).then(() => {
builder = element._builder;
const contentEl = builder.getContentByLine(5, 'right',
element.$.diffTable);
const lineNumberEl = builder._getLineNumberEl(contentEl, 'right');
assert.isTrue(lineNumberEl.classList.contains('lineNum'));
assert.isTrue(lineNumberEl.classList.contains('right'));
done();
});
});
test('_getNextContentOnSide side-by-side left', () => { test('_getNextContentOnSide side-by-side left', () => {
const startElem = builder.getContentByLine(5, 'left', const startElem = builder.getContentByLine(5, 'left',
element.$.diffTable); element.$.diffTable);

3
polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
View File

@@ -51,9 +51,10 @@
* Layer method to add annotations to a line. * Layer method to add annotations to a line.
* @param {!HTMLElement} el The DIV.contentText element to apply the * @param {!HTMLElement} el The DIV.contentText element to apply the
* annotation to. * annotation to.
* @param {!HTMLElement} lineNumberEl
* @param {!Object} line The line object. (GrDiffLine) * @param {!Object} line The line object. (GrDiffLine)
*/ */
annotate(el, line) { annotate(el, lineNumberEl, line) {
let ranges = []; let ranges = [];
if (line.type === GrDiffLine.Type.REMOVE || ( if (line.type === GrDiffLine.Type.REMOVE || (
line.type === GrDiffLine.Type.BOTH && line.type === GrDiffLine.Type.BOTH &&

13
polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer_test.html
View File

@@ -93,6 +93,7 @@ limitations under the License.
let el; let el;
let line; let line;
let annotateElementStub; let annotateElementStub;
const lineNumberEl = document.createElement('td');
setup(() => { setup(() => {
sandbox = sinon.sandbox.create(); sandbox = sinon.sandbox.create();
@@ -111,7 +112,7 @@ limitations under the License.
line.type = GrDiffLine.Type.REMOVE; line.type = GrDiffLine.Type.REMOVE;
line.beforeNumber = 40; line.beforeNumber = 40;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -122,7 +123,7 @@ limitations under the License.
const expectedStart = 6; const expectedStart = 6;
const expectedLength = line.text.length - expectedStart; const expectedLength = line.text.length - expectedStart;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
const lastCall = annotateElementStub.lastCall; const lastCall = annotateElementStub.lastCall;
@@ -140,7 +141,7 @@ limitations under the License.
const expectedStart = 6; const expectedStart = 6;
const expectedLength = line.text.length - expectedStart; const expectedLength = line.text.length - expectedStart;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
const lastCall = annotateElementStub.lastCall; const lastCall = annotateElementStub.lastCall;
@@ -157,7 +158,7 @@ limitations under the License.
const expectedStart = 6; const expectedStart = 6;
const expectedLength = line.text.length - expectedStart; const expectedLength = line.text.length - expectedStart;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
const lastCall = annotateElementStub.lastCall; const lastCall = annotateElementStub.lastCall;
@@ -172,7 +173,7 @@ limitations under the License.
line.beforeNumber = 36; line.beforeNumber = 36;
el.setAttribute('data-side', 'right'); el.setAttribute('data-side', 'right');
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isFalse(annotateElementStub.called); assert.isFalse(annotateElementStub.called);
}); });
@@ -185,7 +186,7 @@ limitations under the License.
const expectedStart = 0; const expectedStart = 0;
const expectedLength = 22; const expectedLength = 22;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isTrue(annotateElementStub.called); assert.isTrue(annotateElementStub.called);
const lastCall = annotateElementStub.lastCall; const lastCall = annotateElementStub.lastCall;

3
polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js
View File

@@ -163,9 +163,10 @@
* Annotation layer method to add syntax annotations to the given element * Annotation layer method to add syntax annotations to the given element
* for the given line. * for the given line.
* @param {!HTMLElement} el * @param {!HTMLElement} el
* @param {!HTMLElement} lineNumberEl
* @param {!Object} line (GrDiffLine) * @param {!Object} line (GrDiffLine)
*/ */
annotate(el, line) { annotate(el, lineNumberEl, line) {
if (!this.enabled) { return; } if (!this.enabled) { return; }
// Determine the side. // Determine the side.

7
polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer_test.html
View File

@@ -38,6 +38,7 @@ limitations under the License.
let sandbox; let sandbox;
let diff; let diff;
let element; let element;
const lineNumberEl = document.createElement('td');
function getMockHLJS() { function getMockHLJS() {
const html = '<span class="gr-diff gr-syntax gr-syntax-string">' + const html = '<span class="gr-diff gr-syntax gr-syntax-string">' +
@@ -77,7 +78,7 @@ limitations under the License.
const line = new GrDiffLine(GrDiffLine.Type.REMOVE); const line = new GrDiffLine(GrDiffLine.Type.REMOVE);
line.beforeNumber = 12; line.beforeNumber = 12;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isFalse(annotationSpy.called); assert.isFalse(annotationSpy.called);
}); });
@@ -99,7 +100,7 @@ limitations under the License.
className, className,
}]; }];
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isTrue(annotationSpy.called); assert.isTrue(annotationSpy.called);
assert.equal(annotationSpy.lastCall.args[0], el); assert.equal(annotationSpy.lastCall.args[0], el);
@@ -127,7 +128,7 @@ limitations under the License.
}]; }];
element.enabled = false; element.enabled = false;
element.annotate(el, line); element.annotate(el, lineNumberEl, line);
assert.isFalse(annotationSpy.called); assert.isFalse(annotationSpy.called);
}); });

46
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context.js
View File

@@ -19,15 +19,19 @@
/** /**
* Used to create a context for GrAnnotationActionsInterface. * Used to create a context for GrAnnotationActionsInterface.
* @param {HTMLElement} el The DIV.contentText element to apply the * @param {HTMLElement} contentEl The DIV.contentText element of the line
* annotation to using annotateRange. * content to apply the annotation to using annotateRange.
* @param {HTMLElement} lineNumberEl The TD element of the line number to
* apply the annotation to using annotateLineNumber.
* @param {GrDiffLine} line The line object. * @param {GrDiffLine} line The line object.
* @param {String} path The file path (eg: /COMMIT_MSG'). * @param {string} path The file path (eg: /COMMIT_MSG').
* @param {String} changeNum The Gerrit change number. * @param {string} changeNum The Gerrit change number.
* @param {String} patchNum The Gerrit patch number. * @param {string} patchNum The Gerrit patch number.
*/ */
function GrAnnotationActionsContext(el, line, path, changeNum, patchNum) { function GrAnnotationActionsContext(
this._el = el; contentEl, lineNumberEl, line, path, changeNum, patchNum) {
this._contentEl = contentEl;
this._lineNumberEl = lineNumberEl;
this.line = line; this.line = line;
this.path = path; this.path = path;
@@ -36,16 +40,28 @@
} }
/** /**
* Method to add annotations to a line. * Method to add annotations to a content line.
* @param {Number} start The line number where the update starts. * @param {number} offset The char offset where the update starts.
* @param {Number} end The line number where the update ends. * @param {number} length The number of chars that the update covers.
* @param {String} cssClass The name of a CSS class created using Gerrit.css. * @param {string} cssClass The name of a CSS class created using Gerrit.css.
* @param {String} side The side of the update. ('left' or 'right') * @param {string} side The side of the update. ('left' or 'right')
*/ */
GrAnnotationActionsContext.prototype.annotateRange = function( GrAnnotationActionsContext.prototype.annotateRange = function(
start, end, cssClass, side) { offset, length, cssClass, side) {
if (this._el.getAttribute('data-side') == side) { if (this._contentEl && this._contentEl.getAttribute('data-side') == side) {
GrAnnotation.annotateElement(this._el, start, end, cssClass); GrAnnotation.annotateElement(this._contentEl, offset, length, cssClass);
}
};
/**
* Method to add a CSS class to the line number TD element.
* @param {string} cssClass The name of a CSS class created using Gerrit.css.
* @param {string} side The side of the update. ('left' or 'right')
*/
GrAnnotationActionsContext.prototype.annotateLineNumber = function(
cssClass, side) {
if (this._lineNumberEl && this._lineNumberEl.classList.contains(side)) {
this._lineNumberEl.classList.add(cssClass);
} }
}; };

17
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-context_test.html
View File

@@ -39,6 +39,7 @@ limitations under the License.
let instance; let instance;
let sandbox; let sandbox;
let el; let el;
let lineNumberEl;
setup(() => { setup(() => {
sandbox = sinon.sandbox.create(); sandbox = sinon.sandbox.create();
@@ -47,8 +48,10 @@ limitations under the License.
el = document.createElement('div'); el = document.createElement('div');
el.textContent = str; el.textContent = str;
el.setAttribute('data-side', 'right'); el.setAttribute('data-side', 'right');
lineNumberEl = document.createElement('td');
lineNumberEl.classList.add('right');
instance = new GrAnnotationActionsContext( instance = new GrAnnotationActionsContext(
el, line, 'dummy/path', '123', '1'); el, lineNumberEl, line, 'dummy/path', '123', '1');
}); });
teardown(() => { teardown(() => {
@@ -74,5 +77,17 @@ limitations under the License.
assert.equal(args[2], end); assert.equal(args[2], end);
assert.equal(args[3], cssClass); assert.equal(args[3], cssClass);
}); });
test('test annotateLineNumber', () => {
const cssClass = Gerrit.css('background-color: #000000');
// Assert that css class is *not* applied when side is different.
instance.annotateLineNumber(cssClass, 'left');
assert.isFalse(lineNumberEl.classList.contains(cssClass));
// Assert that css class is applied when side is the same.
instance.annotateLineNumber(cssClass, 'right');
assert.isTrue(lineNumberEl.classList.contains(cssClass));
});
}); });
</script> </script>

16
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api.js
View File

@@ -32,8 +32,9 @@
/** /**
* Register a function to call to apply annotations. Plugins should use * Register a function to call to apply annotations. Plugins should use
* GrAnnotationActionsContext.annotateRange to apply a CSS class to a range * GrAnnotationActionsContext.annotateRange and
* within a line. * GrAnnotationActionsContext.annotateLineNumber to apply a CSS class to the
* line content or the line number.
* @param {function(GrAnnotationActionsContext)} addLayerFunc The function * @param {function(GrAnnotationActionsContext)} addLayerFunc The function
* that will be called when the AnnotationLayer is ready to annotate. * that will be called when the AnnotationLayer is ready to annotate.
*/ */
@@ -158,13 +159,16 @@
/** /**
* Layer method to add annotations to a line. * Layer method to add annotations to a line.
* @param {HTMLElement} el The DIV.contentText element to apply the * @param {HTMLElement} contentEl The DIV.contentText element of the line
* annotation to. * content to apply the annotation to using annotateRange.
* @param {HTMLElement} lineNumberEl The TD element of the line number to
* apply the annotation to using annotateLineNumber.
* @param {GrDiffLine} line The line object. * @param {GrDiffLine} line The line object.
*/ */
AnnotationLayer.prototype.annotate = function(el, line) { AnnotationLayer.prototype.annotate = function(contentEl, lineNumberEl, line) {
const annotationActionsContext = new GrAnnotationActionsContext( const annotationActionsContext = new GrAnnotationActionsContext(
el, line, this._path, this._changeNum, this._patchNum); contentEl, lineNumberEl, line, this._path, this._changeNum,
this._patchNum);
this._addLayerFunc(annotationActionsContext); this._addLayerFunc(annotationActionsContext);
}; };

3
polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-annotation-actions-js-api_test.html
View File

@@ -71,7 +71,8 @@ limitations under the License.
const annotationLayer = annotationActions.getLayer( const annotationLayer = annotationActions.getLayer(
'/dummy/path', changeNum, patchNum); '/dummy/path', changeNum, patchNum);
annotationLayer.annotate(el, line); const lineNumberEl = document.createElement('td');
annotationLayer.annotate(el, lineNumberEl, line);
assert.isTrue(testLayerFuncCalled); assert.isTrue(testLayerFuncCalled);
}); });

1
polygerrit-ui/app/samples/coverage-plugin.html
View File

@@ -50,6 +50,7 @@
const linesMissingCoverage = coverageData[path].linesMissingCoverage; const linesMissingCoverage = coverageData[path].linesMissingCoverage;
if (linesMissingCoverage.includes(line.afterNumber)) { if (linesMissingCoverage.includes(line.afterNumber)) {
context.annotateRange(0, line.text.length, cssClass, 'right'); context.annotateRange(0, line.text.length, cssClass, 'right');
context.annotateLineNumber(cssClass, 'right');
} }
} }
}).enableToggleCheckbox('Display Coverage', checkbox => { }).enableToggleCheckbox('Display Coverage', checkbox => {