Implement diff view context

Only show N lines of context with a control to set the user's
preferred context length.

TODO in follow-up change: persist the context preference.

Feature: Issue 3675
Change-Id: I7e98651459c80d4894c495f68b6f26f201b587ac
This commit is contained in:
Andrew Bonventre 2015-12-18 19:52:28 -05:00
parent 9231301e82
commit 92f672f5cc
4 changed files with 376 additions and 52 deletions

View File

@ -65,8 +65,23 @@ limitations under the License.
/* >> character */
content: '\00BB';
}
.filler {
background: #eee;
.numbers .filler {
background-color: #eee;
}
.contextControl {
background-color: #fef;
}
.contextControl a:link,
.contextControl a:visited {
display: block;
text-decoration: none;
}
.numbers .contextControl {
padding: 0 .75em;
text-align: right;
}
.content .contextControl {
text-align: center;
}
</style>
<div class$="[[_computeContainerClass(canComment)]]">
@ -90,6 +105,12 @@ limitations under the License.
Polymer({
is: 'gr-diff-side',
/**
* Fired when an expand context control is clicked.
*
* @event expand-context
*/
properties: {
canComment: {
type: Boolean,
@ -98,7 +119,7 @@ limitations under the License.
content: {
type: Array,
notify: true,
observer: '_render',
observer: '_contentChanged',
},
width: {
type: Number,
@ -143,10 +164,28 @@ limitations under the License.
window.scrollTo(0, top - (window.innerHeight / 3) - el.offsetHeight);
},
renderLineIndexRange: function(startIndex, endIndex) {
this._render(this.content, startIndex, endIndex);
},
hideElementsWithIndex: function(index) {
var els = Polymer.dom(this.root).querySelectorAll(
'[data-index="' + index + '"]');
for (var i = 0; i < els.length; i++) {
els[i].setAttribute('hidden', true);
}
},
_widthChanged: function(width) {
this.$.content.style.width = width + 'ch';
},
_contentChanged: function(diff) {
this._clearChildren(this.$.numbers);
this._clearChildren(this.$.content);
this._render(diff, 0, diff.length - 1);
},
_computeContainerClass: function(canComment) {
return 'container' + (canComment ? ' canComment' : '');
},
@ -157,24 +196,61 @@ limitations under the License.
}
},
_render: function(diff) {
this._clearChildren(this.$.numbers);
this._clearChildren(this.$.content);
for (var i = 0; i < diff.length; i++) {
_handleContextControlClick: function(context, e) {
e.preventDefault();
this.fire('expand-context', {bubbles: false, context: context});
},
_render: function(diff, startIndex, endIndex) {
var beforeLineEl;
var beforeContentEl;
if (endIndex != diff.length - 1) {
beforeLineEl = this.$$('.numbers [data-index="' + endIndex + '"]');
beforeContentEl = this.$$('.content [data-index="' + endIndex + '"]');
}
for (var i = startIndex; i <= endIndex; i++) {
if (diff[i].hidden) { continue; }
switch (diff[i].type) {
case 'CODE':
this._renderCode(diff[i]);
this._renderCode(diff[i], i, beforeLineEl, beforeContentEl);
break;
case 'FILLER':
this._renderFiller(diff[i]);
this._renderFiller(diff[i], i, beforeLineEl, beforeContentEl);
break;
case 'CONTEXT_CONTROL':
this._renderContextControl(diff[i], i, beforeLineEl,
beforeContentEl);
break;
}
}
},
_renderFiller: function(filler) {
_renderContextControl: function(control, index, beforeLineEl,
beforeContentEl) {
var lineEl = this._createElement('div', 'contextControl');
lineEl.setAttribute('data-index', index);
lineEl.textContent = '@@';
var contentEl = this._createElement('div', 'contextControl');
contentEl.setAttribute('data-index', index);
var a = this._createElement('a');
a.href = '#';
a.textContent = 'Show ' + control.numLines + ' common ' +
(control.numLines == 1 ? 'line' : 'lines') + '...';
a.addEventListener('click',
this._handleContextControlClick.bind(this, control));
contentEl.appendChild(a);
this.$.numbers.insertBefore(lineEl, beforeLineEl);
this.$.content.insertBefore(contentEl, beforeContentEl);
},
_renderFiller: function(filler, index, beforeLineEl, beforeContentEl) {
var lineFillerEl = this._createElement('div', 'filler');
lineFillerEl.setAttribute('data-index', index);
var fillerEl = this._createElement('div', 'filler');
fillerEl.setAttribute('data-index', index);
var numLines = filler.numLines || 1;
lineFillerEl.textContent = '\n'.repeat(numLines);
@ -182,18 +258,22 @@ limitations under the License.
var newlineEl = this._createElement('span', 'br');
fillerEl.appendChild(newlineEl);
}
this.$.numbers.appendChild(lineFillerEl);
this.$.content.appendChild(fillerEl);
this.$.numbers.insertBefore(lineFillerEl, beforeLineEl);
this.$.content.insertBefore(fillerEl, beforeContentEl);
},
_renderCode: function(code) {
_renderCode: function(code, index, beforeLineEl,
beforeContentEl) {
var lineNumEl = this._createElement('div', 'lineNum');
lineNumEl.setAttribute('data-line-num', code.lineNum);
lineNumEl.setAttribute('data-index', index);
var numLines = code.numLines || 1;
lineNumEl.textContent = code.lineNum + '\n'.repeat(numLines);
var contentEl = this._createElement('div', 'code');
contentEl.setAttribute('data-line-num', code.lineNum);
contentEl.setAttribute('data-index', index);
if (code.highlight) {
contentEl.classList.add(code.intraline.length > 0 ?
@ -218,8 +298,8 @@ limitations under the License.
contentEl.innerHTML = html;
}
this.$.numbers.appendChild(lineNumEl);
this.$.content.appendChild(contentEl);
this.$.numbers.insertBefore(lineNumEl, beforeLineEl);
this.$.content.insertBefore(contentEl, beforeContentEl);
},
// Advance `index` by the appropriate number of characters that would
@ -321,7 +401,10 @@ limitations under the License.
// Since the Polymer DOM utility functions (which would do this
// automatically) are not being used for performance reasons, this is
// done manually.
el.classList.add('style-scope', 'gr-diff-side', className);
el.classList.add('style-scope', 'gr-diff-side');
if (!!className) {
el.classList.add(className);
}
return el;
},
});

View File

@ -23,9 +23,14 @@ limitations under the License.
<dom-module id="gr-diff">
<template>
<style>
gr-patch-range-select {
.header {
display: flex;
margin: 0 var(--default-horizontal-margin) .75em;
}
.contextControl {
flex: 1;
text-align: right;
}
.diffContainer {
border-bottom: 1px solid #eee;
border-top: 1px solid #eee;
@ -34,12 +39,12 @@ limitations under the License.
white-space: pre;
}
gr-diff-side:first-of-type {
--light-highlight-color: #ffecec;
--dark-highlight-color: #faa;
--light-highlight-color: #fee;
--dark-highlight-color: #ffd4d4;
}
gr-diff-side:last-of-type {
--light-highlight-color: #eaffea;
--dark-highlight-color: #9f9;
--light-highlight-color: #efe;
--dark-highlight-color: #d4ffd4;
border-right: 1px solid #ddd;
}
</style>
@ -56,20 +61,37 @@ limitations under the License.
<gr-ajax id="draftsXHR"
url="[[_computeDraftsPath(changeNum, patchRange.patchNum)]]"></gr-ajax>
<gr-patch-range-select
path="[[path]]"
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
available-patches="[[availablePatches]]"></gr-patch-range-select>
<div class="header">
<gr-patch-range-select
path="[[path]]"
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
available-patches="[[availablePatches]]"></gr-patch-range-select>
<div class="contextControl">
Context:
<select id="contextSelect" on-change="_handleContextSelectChange">
<option value="3">3 lines</option>
<option value="10">10 lines</option>
<option value="25">25 lines</option>
<option value="50">50 lines</option>
<option value="75">75 lines</option>
<option value="100">100 lines</option>
<option value="ALL">Whole file</option>
</select>
</div>
</div>
<div class="diffContainer">
<gr-diff-side id="leftDiff"
content="{{_diff.leftSide}}"
width="[[sideWidth]]"
can-comment="[[_loggedIn]]"></gr-diff-side>
can-comment="[[_loggedIn]]"
on-expand-context="_handleExpandContext"></gr-diff-side>
<gr-diff-side id="rightDiff"
content="{{_diff.rightSide}}"
width="[[sideWidth]]"
can-comment="[[_loggedIn]]"></gr-diff-side>
can-comment="[[_loggedIn]]"
on-expand-context="_handleExpandContext"></gr-diff-side>
</div>
</template>
<script>
@ -104,6 +126,11 @@ limitations under the License.
value: 80,
},
_comments: Array,
_context: {
type: Number,
value: 10,
observer: '_contextChanged',
},
_baseComments: Array,
_drafts: Array,
_baseDrafts: Array,
@ -134,6 +161,14 @@ limitations under the License.
this.$.rightDiff.scrollToLine(lineNum);
},
_contextChanged: function(context) {
if (context == Infinity) {
this.$.contextSelect.value = 'ALL';
} else {
this.$.contextSelect.value = context;
}
},
_diffOptionsChanged: function(changeNum, patchRange, path) {
if (!this.auto) { return; }
@ -145,7 +180,7 @@ limitations under the License.
app.accountReady.then(function() {
promises.push(this._getCommentsAndDrafts(basePatchNum, app.loggedIn));
this._diffRequestsPromise = Promise.all(promises).then(function() {
this._allDataReceived();
this._render();
}.bind(this)).catch(function(err) {
alert('Oops. Something went wrong. Check the console and bug the ' +
'PolyGerrit team for assistance.');
@ -154,7 +189,7 @@ limitations under the License.
}.bind(this));
},
_allDataReceived: function() {
_render: function() {
this._processContent();
// Allow for the initial rendering to complete before firing the event.
@ -237,11 +272,43 @@ limitations under the License.
return params;
},
_handleContextSelectChange: function(e) {
if (e.target.value == 'ALL') {
this._context = Infinity;
} else {
this._context = parseInt(e.target.value, 10);
}
this._render();
},
_handleExpandContext: function(e) {
var ctx = e.detail.context;
var contextControlIndex = -1;
for (var i = ctx.start; i <= ctx.end; i++) {
this._diff.leftSide[i].hidden = false;
this._diff.rightSide[i].hidden = false;
if (this._diff.leftSide[i].type == 'CONTEXT_CONTROL' &&
this._diff.rightSide[i].type == 'CONTEXT_CONTROL') {
contextControlIndex = i;
}
}
this._diff.leftSide[contextControlIndex].hidden = true;
this._diff.rightSide[contextControlIndex].hidden = true;
this.$.leftDiff.hideElementsWithIndex(contextControlIndex);
this.$.rightDiff.hideElementsWithIndex(contextControlIndex);
this.$.leftDiff.renderLineIndexRange(ctx.start, ctx.end);
this.$.rightDiff.renderLineIndexRange(ctx.start, ctx.end);
},
_processContent: function() {
var leftSide = [];
var rightSide = [];
var initialLineNum = 0 + (this._diffResponse.content.skip || 0);
var ctx = {
hidingLines: false,
lastNumLinesHidden: 0,
left: {
lineNum: initialLineNum,
},
@ -249,9 +316,17 @@ limitations under the License.
lineNum: initialLineNum,
}
};
for (var i = 0; i < this._diffResponse.content.length; i++) {
this._addDiffChunk(ctx, this._diffResponse.content[i], leftSide,
rightSide);
var content = this._diffResponse.content;
for (var i = 0; i < content.length; i++) {
if (i == 0) {
ctx.skipRange = [0, this._context];
} else if (i == content.length - 1) {
ctx.skipRange = [this._context, 0];
} else {
ctx.skipRange = [this._context, this._context];
}
ctx.diffChunkIndex = i;
this._addDiffChunk(ctx, content[i], leftSide, rightSide);
}
this._diff = {
leftSide: leftSide,
@ -259,24 +334,71 @@ limitations under the License.
};
},
_addContextControl: function(ctx, leftSide, rightSide) {
var numLinesHidden = ctx.lastNumLinesHidden;
var leftStart = leftSide.length - numLinesHidden;
var leftEnd = leftSide.length;
var rightStart = rightSide.length - numLinesHidden;
var rightEnd = rightSide.length;
if (leftStart != rightStart || leftEnd != rightEnd) {
throw Error(
'Left and right ranges for context control should be equal:' +
'Left: [' + leftStart + ', ' + leftEnd + '] ' +
'Right: ['+ rightStart + ', ' + rightEnd + ']');
}
var obj = {
type: 'CONTEXT_CONTROL',
numLines: numLinesHidden,
start: leftStart,
end: leftEnd,
};
// NOTE: Be careful, here. This object is meant to be immutable. If the
// object is altered within one side's array it will reflect the
// alterations in another.
leftSide.push(obj);
rightSide.push(obj);
},
_addCommonDiffChunk: function(ctx, chunk, leftSide, rightSide) {
for (var i = 0; i < chunk.ab.length; i++) {
var numLines = Math.ceil(chunk.ab[i].length / this.sideWidth);
var hidden = i >= ctx.skipRange[0] &&
i < chunk.ab.length - ctx.skipRange[1];
if (ctx.hidingLines && hidden == false) {
// No longer hiding lines. Add a context control.
this._addContextControl(ctx, leftSide, rightSide);
ctx.lastNumLinesHidden = 0;
}
ctx.hidingLines = hidden;
if (hidden) {
ctx.lastNumLinesHidden++;
}
// Blank lines within a diff content array indicate a newline.
leftSide.push({
type: 'CODE',
hidden: hidden,
content: chunk.ab[i] || '\n',
numLines: numLines,
lineNum: ++ctx.left.lineNum,
});
rightSide.push({
type: 'CODE',
hidden: hidden,
content: chunk.ab[i] || '\n',
numLines: numLines,
lineNum: ++ctx.right.lineNum,
});
}
if (ctx.lastNumLinesHidden > 0) {
this._addContextControl(ctx, leftSide, rightSide);
}
},
_addDiffChunk: function(ctx, chunk, leftSide, rightSide) {
if (chunk.ab) {
for (var i = 0; i < chunk.ab.length; i++) {
var numLines = Math.ceil(chunk.ab[i].length / this.sideWidth);
// Blank lines within a diff content array indicate a newline.
leftSide.push({
type: 'CODE',
content: chunk.ab[i] || '\n',
numLines: numLines,
lineNum: ++ctx.left.lineNum,
});
rightSide.push({
type: 'CODE',
content: chunk.ab[i] || '\n',
numLines: numLines,
lineNum: ++ctx.right.lineNum,
});
}
this._addCommonDiffChunk(ctx, chunk, leftSide, rightSide);
return;
}
var leftHighlights = [];

View File

@ -65,7 +65,7 @@ limitations under the License.
intraline: [],
});
}
element._render(content);
element.content = content;
window.scrollTo(0, 0);
element.scrollToLine(-12849);
@ -113,7 +113,7 @@ limitations under the License.
highlight: false,
intraline: [],
}];
element._render(content);
element.content = content;
var lineEl = element.$$('.numbers .lineNum[data-line-num="1"]');
assert.ok(lineEl);
@ -129,7 +129,7 @@ limitations under the License.
highlight: false,
intraline: [],
}];
element._render(content);
element.content = content;
lineEl = element.$$('.numbers .lineNum[data-line-num="1"]');
assert.ok(lineEl);
@ -140,5 +140,48 @@ limitations under the License.
'A'.repeat(20) + element._lineFeedHTML);
});
test('diff context', function() {
var content = [
{type: 'CODE', hidden: true, content: '<!DOCTYPE html>'},
{type: 'CODE', hidden: true, content: '<meta charset="utf-8">'},
{type: 'CODE', hidden: true, content: '<title>My great page</title>'},
{type: 'CODE', hidden: true, content: '<style>'},
{type: 'CODE', hidden: true, content: ' *,'},
{type: 'CODE', hidden: true, content: ' *:before,'},
{type: 'CODE', hidden: true, content: ' *:after {'},
{type: 'CODE', hidden: true, content: ' box-sizing: border-box;'},
{type: 'CONTEXT_CONTROL', numLines: 8, start: 0, end: 8},
{type: 'CODE', hidden: false, content: ' }'},
];
element.content = content;
// Only the context elements and the following code line elements should
// be present in the DOM.
var contextEls =
Polymer.dom(element.root).querySelectorAll('.contextControl');
assert.equal(contextEls.length, 2);
var codeLineEls =
Polymer.dom(element.root).querySelectorAll('.lineNum, .code');
assert.equal(codeLineEls.length, 2);
for (var i = 0; i <= 8; i++) {
element.content[i].hidden = false;
}
element.renderLineIndexRange(0, 8);
element.hideElementsWithIndex(8);
contextEls =
Polymer.dom(element.root).querySelectorAll('.contextControl');
for (var i = 0; i < contextEls.length; i++) {
assert.isTrue(contextEls[i].hasAttribute('hidden'));
}
codeLineEls =
Polymer.dom(element.root).querySelectorAll('.lineNum, .code');
// Nine lines should now be present in the DOM.
assert.equal(codeLineEls.length, 9 * 2);
});
});
</script>

View File

@ -264,5 +264,81 @@ limitations under the License.
}
]);
});
test('context', function() {
element._context = 3;
element._diffResponse = {
content: [
{
ab: [
'<!DOCTYPE html>',
'<meta charset="utf-8">',
'<title>My great page</title>',
'<style>',
' *,',
' *:before,',
' *:after {',
' box-sizing: border-box;',
' }',
'</style>',
'<header>',
]
},
{
a: [
' Welcome ',
' to the wooorld of tomorrow!',
],
b: [
' Hello, world!',
],
},
{
ab: [
'</header>',
'<body>',
'Leela: This is the only place the ship cant hear us, so ',
'everyone pretend to shower.',
'Fry: Same as every day. Got it.',
]
},
]
};
element._processContent();
// First eight lines should be hidden on both sides.
for (var i = 0; i < 8; i++) {
assert.isTrue(element._diff.leftSide[i].hidden);
assert.isTrue(element._diff.rightSide[i].hidden);
}
// A context control should be at index 8 on both sides.
var leftContext = element._diff.leftSide[8];
var rightContext = element._diff.rightSide[8];
assert.deepEqual(leftContext, rightContext);
assert.equal(leftContext.numLines, 8);
assert.equal(leftContext.start, 0);
assert.equal(leftContext.end, 8);
// Line indices 9-16 should be shown.
for (var i = 9; i <= 16; i++) {
// notOk (falsy) because the `hidden` attribute may not be present.
assert.notOk(element._diff.leftSide[i].hidden);
assert.notOk(element._diff.rightSide[i].hidden);
}
// Lines at indices 17 and 18 should be hidden.
assert.isTrue(element._diff.leftSide[17].hidden);
assert.isTrue(element._diff.rightSide[17].hidden);
assert.isTrue(element._diff.leftSide[18].hidden);
assert.isTrue(element._diff.rightSide[18].hidden);
// Context control at index 19.
leftContext = element._diff.leftSide[19];
rightContext = element._diff.rightSide[19];
assert.deepEqual(leftContext, rightContext);
assert.equal(leftContext.numLines, 2);
assert.equal(leftContext.start, 17);
assert.equal(leftContext.end, 19);
});
});
</script>