opendev/gerrit
Code Issues Proposed changes

Gr-diff retrofit: diff section state.

Browse Source 
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
This commit is contained in:
Viktar Donich
2016-05-23 15:31:21 -07:00
committed by Andrew Bonventre
parent 7ad28920e9
commit 679fa309c2
8 changed files with 144 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -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');
}

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

@@ -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);

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

@@ -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);

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

@@ -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., &lt;) 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++;
}
}

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

@@ -144,8 +144,9 @@ limitations under the License.
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 @@ limitations under the License.
});
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 @@ limitations under the License.
});
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 @@ limitations under the License.
}
]);
});
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>

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

@@ -25,6 +25,8 @@
}
}
GrDiffGroup.prototype.element = null;
GrDiffGroup.Type = {
BOTH: 'both',
CONTEXT_CONTROL: 'contextControl',

5
polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
View File

@@ -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 = '';

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

@@ -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);