Gr-diff retrofit: diff section state.
Fixing UI data pipe line, re-rendering:
- store section DOM Elements in gr-diff-builder
- find diff section by line range
- store collapsed lines as gr-diff-group
- update gr-diff-builder's data model on expanding collapsed lines
- .right/.left CSS classes applied to td.lineNum instead of .content
Feature: Issue 3910
Change-Id: Ia0cb78aa00cfd910d186a37631fe71a89204a843
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index 01aaa9c..3de3ae4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -278,7 +278,7 @@
}
for (i = 0;
- i < splice.removed && splicee.removed.length;
+ i < splice.removed && splice.removed.length;
i++) {
this.unlisten(splice.removed[i], 'render', 'handleDiffUpdate');
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
index bf93112..7e8779f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-side-by-side.js
@@ -20,7 +20,7 @@
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
- GrDiffBuilderSideBySide.prototype.emitGroup = function(group,
+ GrDiffBuilderSideBySide.prototype.buildSectionElement = function(group,
opt_beforeSection) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
@@ -29,7 +29,7 @@
sectionEl.appendChild(this._createRow(sectionEl, pairs[i].left,
pairs[i].right));
}
- this._outputEl.insertBefore(sectionEl, opt_beforeSection);
+ return sectionEl;
};
GrDiffBuilderSideBySide.prototype._createRow = function(section, leftLine,
@@ -48,13 +48,14 @@
GrDiffBuilderSideBySide.prototype._appendPair = function(section, row, line,
lineNumber, side) {
- row.appendChild(this._createLineEl(line, lineNumber, line.type, side));
+ var lineEl = this._createLineEl(line, lineNumber, line.type, side);
+ lineEl.classList.add(side);
+ row.appendChild(lineEl);
var action = this._createContextControl(section, line);
if (action) {
row.appendChild(action);
} else {
var textEl = this._createTextEl(line);
- textEl.classList.add(side);
var threadEl = this._commentThreadForLine(line, side);
if (threadEl) {
textEl.appendChild(threadEl);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
index 86340bd..2f1aac6 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder-unified.js
@@ -20,23 +20,26 @@
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderUnified.prototype.constructor = GrDiffBuilderUnified;
- GrDiffBuilderUnified.prototype.emitGroup = function(group,
- opt_beforeSection) {
+ GrDiffBuilderUnified.prototype.buildSectionElement = function(group) {
var sectionEl = this._createElement('tbody', 'section');
sectionEl.classList.add(group.type);
for (var i = 0; i < group.lines.length; ++i) {
sectionEl.appendChild(this._createRow(sectionEl, group.lines[i]));
}
- this._outputEl.insertBefore(sectionEl, opt_beforeSection);
+ return sectionEl;
};
GrDiffBuilderUnified.prototype._createRow = function(section, line) {
var row = this._createElement('tr', line.type);
- row.appendChild(this._createLineEl(line, line.beforeNumber,
- GrDiffLine.Type.REMOVE));
- row.appendChild(this._createLineEl(line, line.afterNumber,
- GrDiffLine.Type.ADD));
+ var 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');
var action = this._createContextControl(section, line);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
index f55037c..d2a7c25 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder.js
@@ -57,8 +57,51 @@
}
};
+ GrDiffBuilder.prototype.buildSectionElement = function(
+ group, opt_beforeSection) {
+ throw Error('Subclasses must implement buildGroupElement');
+ };
+
GrDiffBuilder.prototype.emitGroup = function(group, opt_beforeSection) {
- throw Error('Subclasses must implement emitGroup');
+ var element = this.buildSectionElement(group);
+ this._outputEl.insertBefore(element, opt_beforeSection);
+ group.element = element;
+ };
+
+ GrDiffBuilder.prototype.renderSection = function(element) {
+ for (var i = 0; i < this._groups.length; i++) {
+ var group = this._groups[i];
+ if (group.element === element) {
+ var newElement = this.buildSectionElement(group);
+ group.element.parentElement.replaceChild(newElement, group.element);
+ group.element = newElement;
+ break;
+ }
+ }
+ };
+
+ GrDiffBuilder.prototype.getSectionsByLineRange = function(
+ startLine, endLine, opt_side) {
+ var sections = [];
+ for (var i = 0; i < this._groups.length; i++) {
+ var group = this._groups[i];
+ if (group.lines.length === 0) {
+ continue;
+ }
+ var groupStartLine;
+ var groupEndLine;
+ if (opt_side === GrDiffBuilder.Side.LEFT) {
+ groupStartLine = group.lines[0].beforeNumber;
+ groupEndLine = group.lines[group.lines.length - 1].beforeNumber;
+ } else if (opt_side === GrDiffBuilder.Side.RIGHT) {
+ groupStartLine = group.lines[0].afterNumber;
+ groupEndLine = group.lines[group.lines.length - 1].afterNumber;
+ }
+ if (startLine <= groupEndLine && endLine >= groupStartLine) {
+ sections.push(group.element);
+ }
+ }
+ return sections;
};
GrDiffBuilder.prototype._processContent = function(content, groups, context) {
@@ -183,6 +226,7 @@
currentChunk.ab.push(chunk[j]);
}
}
+ // != instead of !== because we want to cover both undefined and null.
if (currentChunk.ab != null && currentChunk.ab.length > 0) {
result.push(currentChunk);
}
@@ -261,7 +305,8 @@
}
var ctxLine = new GrDiffLine(GrDiffLine.Type.CONTEXT_CONTROL);
- ctxLine.contextLines = hiddenLines;
+ ctxLine.contextGroup =
+ new GrDiffGroup(GrDiffGroup.Type.BOTH, hiddenLines);
groups.push(new GrDiffGroup(GrDiffGroup.Type.CONTEXT_CONTROL,
[ctxLine]));
@@ -311,13 +356,14 @@
};
GrDiffBuilder.prototype._createContextControl = function(section, line) {
- if (!line.contextLines.length) {
+ if (!line.contextGroup || !line.contextGroup.lines.length) {
return null;
}
+ var contextLines = line.contextGroup.lines;
var td = this._createElement('td');
var button = this._createElement('gr-button', 'showContext');
button.setAttribute('link', true);
- var commonLines = line.contextLines.length;
+ var commonLines = contextLines.length;
var text = 'Show ' + commonLines + ' common line';
if (commonLines > 1) {
text += 's';
@@ -326,7 +372,7 @@
button.textContent = text;
button.addEventListener('tap', function(e) {
e.detail = {
- group: new GrDiffGroup(GrDiffGroup.Type.BOTH, line.contextLines),
+ group: line.contextGroup,
section: section,
};
// Let it bubble up the DOM tree.
@@ -383,7 +429,7 @@
}
var patchNum = this._comments.meta.patchRange.patchNum;
- var side = 'REVISION';
+ var side = comments[0].side || 'REVISION';
if (line.type === GrDiffLine.Type.REMOVE ||
opt_side === GrDiffBuilder.Side.LEFT) {
if (this._comments.meta.patchRange.basePatchNum === 'PARENT') {
@@ -413,7 +459,7 @@
} else if (line.type === GrDiffLine.Type.CONTEXT_CONTROL) {
td.classList.add('contextLineNum');
td.setAttribute('data-value', '@@');
- } else if (line.type === GrDiffLine.Type.BOTH || line.type == type) {
+ } else if (line.type === GrDiffLine.Type.BOTH || line.type === type) {
td.classList.add('lineNum');
td.setAttribute('data-value', number);
}
@@ -476,18 +522,18 @@
// Tags don't count as characters
while (index < html.length &&
- html.charCodeAt(index) == GrDiffBuilder.LESS_THAN_CODE) {
+ html.charCodeAt(index) === GrDiffBuilder.LESS_THAN_CODE) {
while (index < html.length &&
- html.charCodeAt(index) != GrDiffBuilder.GREATER_THAN_CODE) {
+ html.charCodeAt(index) !== GrDiffBuilder.GREATER_THAN_CODE) {
index++;
}
index++; // skip the ">" itself
}
// An HTML entity (e.g., <) counts as one character.
if (index < html.length &&
- html.charCodeAt(index) == GrDiffBuilder.AMPERSAND_CODE) {
+ html.charCodeAt(index) === GrDiffBuilder.AMPERSAND_CODE) {
while (index < html.length &&
- html.charCodeAt(index) != GrDiffBuilder.SEMICOLON_CODE) {
+ html.charCodeAt(index) !== GrDiffBuilder.SEMICOLON_CODE) {
index++;
}
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
index 22b9072..4392de0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-builder_test.html
@@ -144,8 +144,9 @@
assert.equal(groups[0].lines[0].afterNumber, GrDiffLine.FILE);
assert.equal(groups[1].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[1].lines[0].contextLines.length, 90);
- groups[1].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[1].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[1].lines[0].contextGroup.lines.length, 90);
+ groups[1].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[0].ab[0]);
});
@@ -179,8 +180,9 @@
});
assert.equal(groups[7].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[7].lines[0].contextLines.length, 90);
- groups[7].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[7].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[7].lines[0].contextGroup.lines.length, 90);
+ groups[7].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[4].ab[0]);
});
@@ -215,8 +217,9 @@
});
assert.equal(groups[3].type, GrDiffGroup.Type.CONTEXT_CONTROL);
- assert.equal(groups[3].lines[0].contextLines.length, 30);
- groups[3].lines[0].contextLines.forEach(function(l) {
+ assert.instanceOf(groups[3].lines[0].contextGroup, GrDiffGroup);
+ assert.equal(groups[3].lines[0].contextGroup.lines.length, 30);
+ groups[3].lines[0].contextGroup.lines.forEach(function(l) {
assert.equal(l.text, content[1].ab[0]);
});
@@ -517,5 +520,54 @@
}
]);
});
+
+ suite('rendering', function() {
+ var content;
+ var outputEl;
+
+ setup(function() {
+ var prefs = {
+ line_length: 10,
+ show_tabs: true,
+ tab_size: 4,
+ context: -1
+ };
+ content = [
+ {ab: []},
+ {a: ['all work and no play make andybons a dull boy']},
+ {ab: []},
+ {b: ['elgoog elgoog elgoog']},
+ {ab: []},
+ ];
+ outputEl = document.createElement('out');
+ builder =
+ new GrDiffBuilder(
+ {content: content}, {left: [], right: []}, prefs, outputEl);
+ builder.buildSectionElement = function(group) {
+ var section = document.createElement('stub');
+ section.textContent = group.lines.reduce(function(acc, line) {
+ return acc + line.text;
+ }, '');
+ return section;
+ };
+ builder.emitDiff();
+ });
+
+ test('renderSection', function() {
+ var section = outputEl.querySelector('stub:nth-of-type(2)');
+ var prevInnerHTML = section.innerHTML;
+ section.innerHTML = 'wiped';
+ builder.renderSection(section);
+ section = outputEl.querySelector('stub:nth-of-type(2)');
+ assert.equal(section.innerHTML, prevInnerHTML);
+ });
+
+ test('getSectionsByLineRange', function() {
+ var section = outputEl.querySelector('stub:nth-of-type(2)');
+ var sections = builder.getSectionsByLineRange(1, 1, 'left');
+ assert.equal(sections.length, 1);
+ assert.strictEqual(sections[0], section);
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
index 750f7da..7c7c508 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-group.js
@@ -25,6 +25,8 @@
}
}
+ GrDiffGroup.prototype.element = null;
+
GrDiffGroup.Type = {
BOTH: 'both',
CONTEXT_CONTROL: 'contextControl',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
index ea00a3d..4acde0c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
@@ -16,13 +16,14 @@
function GrDiffLine(type) {
this.type = type;
- this.contextLines = [];
this.highlights = [];
}
+ GrDiffLine.prototype.afterNumber = 0;
+
GrDiffLine.prototype.beforeNumber = 0;
- GrDiffLine.prototype.afterNumber = 0;
+ GrDiffLine.prototype.contextGroup = null;
GrDiffLine.prototype.text = '';
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index ea7111a..d813933 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -215,7 +215,7 @@
} else {
var patchNum = this.patchRange.patchNum;
var side = 'REVISION';
- if (contentEl.classList.contains(DiffSide.LEFT) ||
+ if (lineEl.classList.contains(DiffSide.LEFT) ||
contentEl.classList.contains('remove')) {
if (this.patchRange.basePatchNum === 'PARENT') {
side = 'PARENT';
@@ -364,6 +364,13 @@
},
_showContext: function(group, sectionEl) {
+ var groups = this._builder._groups;
+ // TODO(viktard): Polyfill findIndex for IE10.
+ var contextIndex = groups.findIndex(function(group) {
+ return group.element == sectionEl;
+ });
+ groups[contextIndex] = group;
+
this._builder.emitGroup(group, sectionEl);
sectionEl.parentNode.removeChild(sectionEl);
