opendev/gerrit
Code Issues Proposed changes

Refactors annotation layer

Browse Source 
Formerly, the annotation layer interface provided the GrAnnotation
library as a parameter to the `annotate` method. This was so the layer
would not necessarily need to import the library at the module level
and instead could use it as a utility toolbox.

With this change, the library is no-longer part of the interface and the
layers are now expected to import it at the module layer (if they have
a use for it).

Change-Id: I49b96c67ec724708c2861ab6be3ce27a53cc1b05
This commit is contained in:
Wyatt Allen
2016-07-25 22:46:20 -07:00
parent a008aa9428
commit 5c5f00fb13
8 changed files with 37 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -286,7 +286,7 @@ limitations under the License.
// Take a DIV.contentText element and a line object with intraline
// differences to highlight and apply them to the element as
// annotations.
annotate: function(el, line, GrAnnotation) {
annotate: function(el, line) {
var HL_CLASS = 'style-scope gr-diff intraline';
line.highlights.forEach(function(highlight) {
// The start and end indices could be the same if a highlight is

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

@@ -393,7 +393,7 @@
'lightHighlight' : 'darkHighlight');
this.layers.forEach(function(layer) {
layer.annotate(contentText, line, GrAnnotation);
layer.annotate(contentText, line);
});
td.appendChild(contentText);

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

@@ -280,7 +280,7 @@ limitations under the License.
highlights: [],
};
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
// The content is unchanged.
assert.isFalse(annotateElementSpy.called);
@@ -303,7 +303,7 @@ limitations under the License.
var str3 = slice(str, 18, 22);
var str4 = slice(str, 22);
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 5);
@@ -335,7 +335,7 @@ limitations under the License.
var str0 = slice(str, 0, 28);
var str1 = slice(str, 28);
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 2);
@@ -355,7 +355,7 @@ limitations under the License.
],
};
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
assert.isFalse(annotateElementSpy.called);
assert.equal(el.childNodes.length, 1);
@@ -376,7 +376,7 @@ limitations under the License.
var str1 = slice(str, 6, 12);
var str2 = slice(str, 12);
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 3);
@@ -406,7 +406,7 @@ limitations under the License.
var str0 = slice(str, 0, 6);
var str1 = slice(str, 6);
layer.annotate(el, line, GrAnnotation);
layer.annotate(el, line);
assert.isTrue(annotateElementSpy.called);
assert.equal(el.childNodes.length, 2);

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

@@ -16,5 +16,6 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<dom-module id="gr-ranged-comment-layer">
<script src="../gr-diff-highlight/gr-annotation.js"></script>
<script src="gr-ranged-comment-layer.js"></script>
</dom-module>

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

@@ -44,9 +44,8 @@
* @param {HTMLElement} el The DIV.contentText element to apply the
* annotation to.
* @param {GrDiffLine} line The line object.
* @param {Object} GrAnnotation The annotation library.
*/
annotate: function(el, line, GrAnnotation) {
annotate: function(el, line) {
var ranges = [];
if (line.type === GrDiffLine.Type.REMOVE || (
line.type === GrDiffLine.Type.BOTH &&

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

@@ -90,25 +90,31 @@ limitations under the License.
});
suite('annotate', function() {
var GrAnnotation;
var sandbox;
var el;
var line;
var annotateElementStub;
setup(function() {
GrAnnotation = {annotateElement: sinon.stub()};
sandbox = sinon.sandbox.create();
annotateElementStub = sandbox.stub(GrAnnotation, 'annotateElement');
el = document.createElement('div');
el.setAttribute('data-side', 'left');
line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.text = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit,';
});
teardown(function() {
sandbox.restore();
});
test('type=Remove no-comment', function() {
line.type = GrDiffLine.Type.REMOVE;
line.beforeNumber = 40;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isFalse(GrAnnotation.annotateElement.called);
assert.isFalse(annotateElementStub.called);
});
test('type=Remove has-comment', function() {
@@ -117,10 +123,10 @@ limitations under the License.
var expectedStart = 6;
var expectedLength = line.text.length - expectedStart;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isTrue(GrAnnotation.annotateElement.called);
var lastCall = GrAnnotation.annotateElement.lastCall;
assert.isTrue(annotateElementStub.called);
var lastCall = annotateElementStub.lastCall;
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
@@ -135,10 +141,10 @@ limitations under the License.
var expectedStart = 6;
var expectedLength = line.text.length - expectedStart;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isTrue(GrAnnotation.annotateElement.called);
var lastCall = GrAnnotation.annotateElement.lastCall;
assert.isTrue(annotateElementStub.called);
var lastCall = annotateElementStub.lastCall;
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
@@ -152,10 +158,10 @@ limitations under the License.
var expectedStart = 6;
var expectedLength = line.text.length - expectedStart;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isTrue(GrAnnotation.annotateElement.called);
var lastCall = GrAnnotation.annotateElement.lastCall;
assert.isTrue(annotateElementStub.called);
var lastCall = annotateElementStub.lastCall;
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);
@@ -170,9 +176,9 @@ limitations under the License.
var expectedStart = 6;
var expectedLength = line.text.length - expectedStart;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isFalse(GrAnnotation.annotateElement.called);
assert.isFalse(annotateElementStub.called);
});
test('type=Add has-comment', function() {
@@ -183,10 +189,10 @@ limitations under the License.
var expectedStart = 0;
var expectedLength = 22;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isTrue(GrAnnotation.annotateElement.called);
var lastCall = GrAnnotation.annotateElement.lastCall;
assert.isTrue(annotateElementStub.called);
var lastCall = annotateElementStub.lastCall;
assert.equal(lastCall.args[0], el);
assert.equal(lastCall.args[1], expectedStart);
assert.equal(lastCall.args[2], expectedLength);

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

@@ -73,7 +73,7 @@
* @param {!HTMLElement} el
* @param {!GrDiffLine} line
*/
annotate: function(el, line, GrAnnotation) {
annotate: function(el, line) {
// Determine the side.
var side;
if (line.type === GrDiffLine.Type.REMOVE || (

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

@@ -55,7 +55,7 @@ limitations under the License.
var line = new GrDiffLine(GrDiffLine.Type.REMOVE);
line.beforeNumber = 12;
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isFalse(annotationSpy.called);
});
@@ -77,7 +77,7 @@ limitations under the License.
className: className,
}];
element.annotate(el, line, GrAnnotation);
element.annotate(el, line);
assert.isTrue(annotationSpy.called);
assert.equal(annotationSpy.lastCall.args[0], el);