opendev/gerrit
Code Issues Proposed changes

Comment events wiring for ranged comments

Browse Source 
Listen and update diff on comment events:
- call appropriate methods to apply comment ranges on comment creation
- re-render diff on and thread comment discard
- apply highlight on comment mouse over
- remove highlight on comment mouse out
- tests for all above

Feature: Issue 3910
Change-Id: I501ddcd063407777355b9c887118fcae53dcb5f1
This commit is contained in:
Viktar Donich
2016-06-17 16:53:45 -07:00
parent d9b1f53a6a
commit 3d6f8d695b
2 changed files with 235 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

87
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
View File

@@ -16,6 +16,8 @@
// Astral code point as per https://mathiasbynens.be/notes/javascript-unicode
var REGEX_ASTRAL_SYMBOL = /[\uD800-\uDBFF][\uDC00-\uDFFF]/;
var RANGE_HIGHLIGHT = 'range';
var HOVER_HIGHLIGHT = 'rangeHighlight';
Polymer({
is: 'gr-diff-highlight',
@@ -28,12 +30,15 @@
},
loggedIn: Boolean,
_cachedDiffBuilder: Object,
_diffElement: Object,
_enabledListeners: {
type: Object,
value: function() {
return {
'down': '_handleDown',
'comment-discard': '_handleCommentDiscard',
'comment-mouse-out': '_handleCommentMouseOut',
'comment-mouse-over': '_handleCommentMouseOver',
'create-comment': '_createComment',
'thread-discard': '_handleThreadDiscard',
};
},
},
@@ -66,13 +71,75 @@
return !!this.$$('gr-selection-action-box');
},
_handleDown: function(e) {
var actionBox = this.$$('gr-selection-action-box');
if (actionBox && !actionBox.contains(e.target)) {
this._removeActionBox();
_handleThreadDiscard: function(e) {
var comment = e.detail.lastComment;
// Comment Element was removed from DOM already.
if (comment.range) {
this._renderCommentRange(comment, e.target);
}
},
_handleCommentDiscard: function(e) {
var comment = e.detail.comment;
if (comment.range) {
this._renderCommentRange(comment, e.target);
}
},
_handleCommentMouseOver: function(e) {
var comment = e.detail.comment;
var range = comment.range;
if (!range) {
return;
}
var lineEl = this.diffBuilder.getLineElByChild(e.target);
var side = this.diffBuilder.getSideByLineEl(lineEl);
this._applyRangedHighlight(
HOVER_HIGHLIGHT, range.start_line, range.start_character,
range.end_line, range.end_character, side);
},
_handleCommentMouseOut: function(e) {
var comment = e.detail.comment;
var range = comment.range;
if (!range) {
return;
}
var lineEl = this.diffBuilder.getLineElByChild(e.target);
var side = this.diffBuilder.getSideByLineEl(lineEl);
var contentEls = this.diffBuilder.getContentsByLineRange(
range.start_line, range.end_line, side);
contentEls.forEach(function(content) {
Polymer.dom(content).querySelectorAll('.' + HOVER_HIGHLIGHT).forEach(
function(el) {
el.classList.remove(HOVER_HIGHLIGHT);
el.classList.add(RANGE_HIGHLIGHT);
});
}, this);
},
_renderCommentRange: function(comment, el) {
var lineEl = this.diffBuilder.getLineElByChild(el);
if (!lineEl) {
return;
}
var side = this.diffBuilder.getSideByLineEl(lineEl);
this._rerenderByLines(
comment.range.start_line, comment.range.end_line, side);
},
_createComment: function(e) {
this._removeActionBox();
var side = e.detail.side;
var range = e.detail.range;
if (!range) {
return;
}
this._applyRangedHighlight(
RANGE_HIGHLIGHT, range.startLine, range.startChar,
range.endLine, range.endChar, side);
},
_removeActionBox: function() {
var actionBox = this.$$('gr-selection-action-box');
if (actionBox) {
@@ -122,7 +189,7 @@
}
return length;
} else {
// DOM API for textConten.length is broken for Unicode:
// DOM API for textContent.length is broken for Unicode:
// https://mathiasbynens.be/notes/javascript-unicode
return node.textContent.replace(REGEX_ASTRAL_SYMBOL, '_').length;
}
@@ -374,5 +441,11 @@
}, this);
}
},
_rerenderByLines: function(startLine, endLine, opt_side) {
this.async(function() {
this.diffBuilder.renderLineRange(startLine, endLine, opt_side);
}, 1);
},
});
})();

162
polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
View File

@@ -92,14 +92,162 @@ limitations under the License.
sandbox.restore();
});
test('handles down only when enabled ', function() {
sinon.stub(element, '_handleDown');
MockInteractions.down(element);
assert.isFalse(element._handleDown.called);
test('_enabledListeners', function() {
var listeners = element._enabledListeners;
for (var eventName in listeners) {
sandbox.stub(element, listeners[eventName]);
}
// Enable all the listeners.
element.enabled = true;
MockInteractions.down(element);
assert.isTrue(element._handleDown.called);
element._handleDown.restore();
for (var eventName in listeners) {
var methodName = listeners[eventName];
var stub = element[methodName];
element.fire(eventName);
assert.isTrue(stub.called);
stub.reset();
}
// Disable all the listeners.
element.enabled = false;
for (var eventName in listeners) {
var methodName = listeners[eventName];
var stub = element[methodName];
element.fire(eventName);
assert.isFalse(stub.called);
}
});
suite('comment events', function() {
var builder;
setup(function() {
builder = {
getContentsByLineRange: sandbox.stub().returns([]),
getLineElByChild: sandbox.stub().returns({}),
getSideByLineEl: sandbox.stub().returns('other-side'),
renderLineRange: sandbox.stub(),
};
element._cachedDiffBuilder = builder;
element.enabled = true;
});
test('ignores thread discard for line comment', function(done) {
element.fire('thread-discard', {lastComment: {}});
flush(function() {
assert.isFalse(builder.renderLineRange.called);
done();
});
});
test('ignores comment discard for line comment', function(done) {
element.fire('comment-discard', {comment: {}});
flush(function() {
assert.isFalse(builder.renderLineRange.called);
done();
});
});
test('renders lines in comment range on thread discard', function(done) {
element.fire('thread-discard', {
lastComment: {
range: {
start_line: 10,
end_line: 24,
},
},
});
flush(function() {
assert.isTrue(
builder.renderLineRange.calledWithExactly(10, 24, 'other-side'));
done();
});
});
test('renders lines in comment range on comment discard', function(done) {
element.fire('comment-discard', {
comment: {
range: {
start_line: 10,
end_line: 24,
},
},
});
flush(function() {
assert.isTrue(
builder.renderLineRange.calledWithExactly(10, 24, 'other-side'));
done();
});
});
test('comment-mouse-over from line comments is ignored', function() {
sandbox.stub(element, '_applyRangedHighlight');
element.fire('comment-mouse-over', {comment: {}});
assert.isFalse(element._applyRangedHighlight.called);
});
test('comment-mouse-out from line comments is ignored', function() {
element.fire('comment-mouse-over', {comment: {}});
assert.isFalse(builder.getContentsByLineRange.called);
});
test('on comment-mouse-out highlight classes are removed', function() {
var testEl = fixture('highlighted');
builder.getContentsByLineRange.returns([testEl]);
element.fire('comment-mouse-out', {
comment: {
range: {
start_line: 3,
start_character: 14,
end_line: 10,
end_character: 24,
}
}});
assert.isTrue(builder.getContentsByLineRange.calledWithExactly(
3, 10, 'other-side'));
assert.equal(0, testEl.querySelectorAll('.rangeHighlight').length);
assert.equal(2, testEl.querySelectorAll('.range').length);
});
test('on comment-mouse-over range is highlighted', function() {
sandbox.stub(element, '_applyRangedHighlight');
element.fire('comment-mouse-over', {
comment: {
range: {
start_line: 3,
start_character: 14,
end_line: 10,
end_character: 24,
},
}});
assert.isTrue(element._applyRangedHighlight.calledWithExactly(
'rangeHighlight', 3, 14, 10, 24, 'other-side'));
});
test('on create-comment range is highlighted', function() {
sandbox.stub(element, '_applyRangedHighlight');
element.fire('create-comment', {
range: {
startLine: 3,
startChar: 14,
endLine: 10,
endChar: 24,
},
side: 'some-side',
});
assert.isTrue(element._applyRangedHighlight.calledWithExactly(
'range', 3, 14, 10, 24, 'some-side'));
});
test('on create-comment action box is removed', function() {
sandbox.stub(element, '_applyRangedHighlight');
sandbox.stub(element, '_removeActionBox');
element.fire('create-comment', {
comment: {
range: {},
},
});
assert.isTrue(element._removeActionBox.called);
});
});
test('apply multiline highlight', function() {