Fix the js error on commentRanges change event
The change record on array mutation is not using the number indicate the index, but using it as the key, so can not retrieve the item by the number, use `this.get` as recommended from the doc. https://polymer-library.polymer-project.org/1.0/docs/devguide/observers Bug: Issue 11375 Change-Id: I04da93aa7c01ec40bddc91d2400bceac101f7c62
This commit is contained in:
@@ -373,7 +373,10 @@
|
||||
});
|
||||
this.splice('_commentRanges', i, 1);
|
||||
}
|
||||
this.push('_commentRanges', ...addedCommentRanges);
|
||||
|
||||
if (addedCommentRanges && addedCommentRanges.length) {
|
||||
this.push('_commentRanges', ...addedCommentRanges);
|
||||
}
|
||||
},
|
||||
|
||||
/**
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
'use strict';
|
||||
|
||||
// Polymer 1 adds # before array's key, while Polymer 2 doesn't
|
||||
const HOVER_PATH_PATTERN = /^commentRanges\.\#?(\d+)\.hovering$/;
|
||||
const HOVER_PATH_PATTERN = /^(commentRanges\.\#?\d+)\.hovering$/;
|
||||
|
||||
const RANGE_HIGHLIGHT = 'style-scope gr-diff range';
|
||||
const HOVER_HIGHLIGHT = 'style-scope gr-diff rangeHighlight';
|
||||
@@ -131,8 +131,10 @@
|
||||
// If the change only changed the `hovering` property of a comment.
|
||||
const match = record.path.match(HOVER_PATH_PATTERN);
|
||||
if (match) {
|
||||
const commentRangesIndex = match[1];
|
||||
const {side, range, hovering} = this.commentRanges[commentRangesIndex];
|
||||
// The #number indicates the key of that item in the array
|
||||
// not the index, especially in polymer 1.
|
||||
const {side, range, hovering} = this.get(match[1]);
|
||||
|
||||
this._updateRangesMap(
|
||||
side, range, hovering, (forLine, start, end, hovering) => {
|
||||
const index = forLine.findIndex(lineRange =>
|
||||
|
||||
@@ -209,6 +209,7 @@ limitations under the License.
|
||||
test('_handleCommentRangesChange hovering', () => {
|
||||
const notifyStub = sinon.stub();
|
||||
element.addListener(notifyStub);
|
||||
const updateRangesMapSpy = sandbox.spy(element, '_updateRangesMap');
|
||||
|
||||
element.set(['commentRanges', 1, 'hovering'], true);
|
||||
|
||||
@@ -217,6 +218,8 @@ limitations under the License.
|
||||
assert.equal(lastCall.args[0], 10);
|
||||
assert.equal(lastCall.args[1], 12);
|
||||
assert.equal(lastCall.args[2], 'right');
|
||||
|
||||
assert.isTrue(updateRangesMapSpy.called);
|
||||
});
|
||||
|
||||
test('_handleCommentRangesChange splice out', () => {
|
||||
@@ -253,6 +256,31 @@ limitations under the License.
|
||||
assert.equal(lastCall.args[2], 'left');
|
||||
});
|
||||
|
||||
test('_handleCommentRangesChange mixed actions', () => {
|
||||
const notifyStub = sinon.stub();
|
||||
element.addListener(notifyStub);
|
||||
const updateRangesMapSpy = sandbox.spy(element, '_updateRangesMap');
|
||||
|
||||
element.set(['commentRanges', 1, 'hovering'], true);
|
||||
assert.isTrue(updateRangesMapSpy.callCount === 1);
|
||||
element.splice('commentRanges', 1, 1);
|
||||
assert.isTrue(updateRangesMapSpy.callCount === 2);
|
||||
element.splice('commentRanges', 1, 1);
|
||||
assert.isTrue(updateRangesMapSpy.callCount === 3);
|
||||
element.splice('commentRanges', 1, 0, {
|
||||
side: 'left',
|
||||
range: {
|
||||
end_character: 15,
|
||||
end_line: 275,
|
||||
start_character: 5,
|
||||
start_line: 250,
|
||||
},
|
||||
});
|
||||
assert.isTrue(updateRangesMapSpy.callCount === 4);
|
||||
element.set(['commentRanges', 2, 'hovering'], true);
|
||||
assert.isTrue(updateRangesMapSpy.callCount === 5);
|
||||
});
|
||||
|
||||
test('_computeCommentMap creates maps correctly', () => {
|
||||
// There is only one ranged comment on the left, but it spans ll.36-39.
|
||||
const leftKeys = [];
|
||||
|
||||
Reference in New Issue
Block a user