opendev/gerrit
Code Issues Proposed changes

Add a preferences pane to the diff view

Browse Source 
This includes to start:
+ Context
+ Column width
+ Tab size
+ Show tab indicators

Bug: Issue 3841
Change-Id: I3c8292648fd87ac2680d0eb22ce8f9e78d24429e
This commit is contained in:
Andrew Bonventre
2016-01-26 16:30:02 -05:00
parent 55b51ce27a
commit 33543b7ed8
13 changed files with 460 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
polygerrit-ui/BUCK
View File

@@ -7,6 +7,7 @@ bower_components(
'//lib/js:iron-autogrow-textarea',
'//lib/js:iron-dropdown',
'//lib/js:iron-input',
'//lib/js:iron-overlay-behavior',
'//lib/js:iron-selector',
'//lib/js:page',
'//lib/js:polymer',

12
polygerrit-ui/app/elements/gr-app.html
View File

@@ -228,9 +228,21 @@ limitations under the License.
} else {
// These defaults should match the defaults in
// gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java
// NOTE: There are some settings that don't apply to PolyGerrit
// (Render mode being at least one of them).
this._diffPreferences = {
auto_hide_diff_table_header: true,
context: 10,
cursor_blink_rate: 0,
ignore_whitespace: 'IGNORE_NONE',
intraline_difference: true,
line_length: 100,
show_line_endings: true,
show_tabs: true,
show_whitespace_errors: true,
syntax_highlighting: true,
tab_size: 8,
theme: 'DEFAULT',
};
}
},

1
polygerrit-ui/app/elements/gr-diff-comment-thread.html
View File

@@ -22,7 +22,6 @@ limitations under the License.
<style>
:host {
display: block;
max-width: 50em;
white-space: normal;
}
gr-diff-comment {

187
polygerrit-ui/app/elements/gr-diff-preferences.html Normal file
View File

@@ -0,0 +1,187 @@
<!--
Copyright (C) 2016 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../bower_components/iron-input/iron-input.html">
<dom-module id="gr-diff-preferences">
<template>
<style>
:host {
display: block;
}
:host[disabled] {
opacity: .5;
pointer-events: none;
}
button,
input,
select {
font: inherit;
}
input[type="number"] {
width: 4em;
}
button {
background-color: #f1f2f3;
border: 1px solid #aaa;
border-radius: 2px;
cursor: pointer;
font: inherit;
padding: .2em .75em;
}
button.primary {
background: #448aff;
border: none;
color: #fff;
}
.header,
.actions {
padding: 1em 1.5em;
}
.header {
border-bottom: 1px solid #ddd;
font-weight: bold;
}
.mainContainer {
padding: 1em 0;
}
.pref {
align-items: center;
display: flex;
padding: .35em 1.5em;
width: 20em;
}
.pref:hover {
background-color: #ebf5fb;
}
.pref label {
cursor: pointer;
flex: 1;
}
.actions {
border-top: 1px solid #ddd;
display: flex;
}
.cancel {
display: flex;
flex: 1;
justify-content: flex-end;
}
</style>
<div class="header">
Diff View Preferences
</div>
<div class="mainContainer">
<div class="pref">
<label for="contextSelect">Context</label>
<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="-1">Whole file</option>
</select>
</div>
<div class="pref">
<label for="columnsInput">Columns</label>
<input is="iron-input" type="number" id="columnsInput"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{prefs.line_length}}">
</div>
<div class="pref">
<label for="tabSizeInput">Tab width</label>
<input is="iron-input" type="number" id="tabSizeInput"
prevent-invalid-input
allowed-pattern="[0-9]"
bind-value="{{prefs.tab_size}}">
</div>
<div class="pref">
<label for="showTabsInput">Show tabs</label>
<input is="iron-input" type="checkbox" id="showTabsInput"
on-tap="_handleShowTabsTap">
</div>
</div>
<div class="actions">
<button class="primary" on-tap="_handleSave">Save</button>
<div class="cancel">
<button on-tap="_handleCancel">Cancel</button>
</div>
</div>
</template>
<script>
(function() {
'use strict';
Polymer({
is: 'gr-diff-preferences',
/**
* Fired when the user presses the save button.
*
* @event save
*/
/**
* Fired when the user presses the cancel button.
*
* @event cancel
*/
properties: {
prefs: {
type: Object,
notify: true,
value: function() { return {}; },
},
disabled: {
type: Boolean,
value: false,
reflectToAttribute: true,
},
},
observers: [ '_prefsChanged(prefs.*)' ],
_prefsChanged: function(changeRecord) {
var prefs = changeRecord.base;
this.$.contextSelect.value = prefs.context;
this.$.showTabsInput.checked = prefs.show_tabs;
},
_handleContextSelectChange: function(e) {
var selectEl = Polymer.dom(e).rootTarget;
this.set('prefs.context', parseInt(selectEl.value, 10));
},
_handleShowTabsTap: function(e) {
this.set('prefs.show_tabs', Polymer.dom(e).rootTarget.checked);
},
_handleSave: function() {
this.fire('save', null, {bubble: false});
},
_handleCancel: function() {
this.fire('cancel', null, {bubble: false});
},
});
})();
</script>
</dom-module>

40
polygerrit-ui/app/elements/gr-diff-side.html
View File

@@ -24,9 +24,6 @@ limitations under the License.
.container {
display: flex;
}
.content {
width: 80ch;
}
.lineNum:before,
.code:before {
/* To ensure the height is non-zero in these elements, a
@@ -62,7 +59,10 @@ limitations under the License.
/* Line feed */
content: '\A';
}
.tab:before {
.tab {
display: inline-block;
}
.tab.withIndicator:before {
color: #C62828;
/* >> character */
content: '\00BB';
@@ -141,11 +141,10 @@ limitations under the License.
notify: true,
observer: '_contentChanged',
},
width: {
type: Number,
observer: '_widthChanged',
prefs: {
type: Object,
value: function() { return {}; },
},
tabWidth: Number,
changeNum: String,
patchNum: String,
path: String,
@@ -175,6 +174,8 @@ limitations under the License.
'tap': '_tapHandler',
},
observers: [ '_prefsChanged(prefs.*)' ],
rowInserted: function(index) {
this.renderLineIndexRange(index, index);
this._updateDOMIndices();
@@ -295,8 +296,9 @@ limitations under the License.
}
},
_widthChanged: function(width) {
this.$.content.style.width = width + 'ch';
_prefsChanged: function(changeRecord) {
var prefs = changeRecord.base;
this.$.content.style.width = prefs.line_length + 'ch';
},
_projectConfigChanged: function(projectConfig) {
@@ -481,7 +483,7 @@ limitations under the License.
if (numLines > 1) {
html = this._addNewLines(code.content, html, numLines);
}
html = this._addTabIndicators(code.content, html);
html = this._addTabWrappers(code.content, html);
// If the html is equivalent to the text then it didn't get highlighted
// or escaped. Use textContent which is faster than innerHTML.
@@ -559,12 +561,12 @@ limitations under the License.
var indices = [];
var numChars = 0;
for (var i = 0; i < content.length; i++) {
if (numChars > 0 && numChars % this.width == 0) {
if (numChars > 0 && numChars % this.prefs.line_length == 0) {
indices.push(htmlIndex);
}
htmlIndex = this._advanceChar(html, htmlIndex)
if (content[i] == '\t') {
numChars += this.tabWidth;
numChars += this.prefs.tab_size;
} else {
numChars++;
}
@@ -587,9 +589,15 @@ limitations under the License.
return result;
},
_addTabIndicators: function(content, html) {
return html.replace(TAB_REGEX,
'<span class="style-scope gr-diff-side tab">\t</span>');
_addTabWrappers: function(content, html) {
// TODO(andybons): CSS tab-size is not supported in IE.
// Force this to be a number to prevent arbitrary injection.
var tabSize = +this.prefs.tab_size;
var htmlStr = '<span class="style-scope gr-diff-side tab ' +
(this.prefs.show_tabs ? 'withIndicator" ' : '" ') +
'style="tab-size:' + tabSize + ';' +
'-moz-tab-size:' + tabSize + ';">\t</span>';
return html.replace(TAB_REGEX, htmlStr);
},
_createElement: function(tagName, className) {

3
polygerrit-ui/app/elements/gr-diff-view.html
View File

@@ -199,6 +199,9 @@ limitations under the License.
page.show(this._computeChangePath(this._changeNum));
}
break;
case 188: // ','
this.$.diff.showDiffPreferences();
break;
}
},

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

@@ -16,7 +16,9 @@ limitations under the License.
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="gr-ajax.html">
<link rel="import" href="gr-diff-preferences.html">
<link rel="import" href="gr-diff-side.html">
<link rel="import" href="gr-overlay.html">
<link rel="import" href="gr-patch-range-select.html">
<link rel="import" href="gr-request.html">
@@ -31,7 +33,7 @@ limitations under the License.
display: flex;
margin: 0 var(--default-horizontal-margin) .75em;
}
.contextControl {
.prefsButton {
flex: 1;
text-align: right;
}
@@ -74,19 +76,18 @@ limitations under the License.
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
available-patches="[[availablePatches]]"></gr-patch-range-select>
<div class="contextControl" hidden$="[[!prefs.context]]" hidden>
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="-1">Whole file</option>
</select>
</div>
<a class="prefsButton"
href="#"
on-tap="_handlePrefsTap"
hidden$="[[!prefs]]"
hidden>Diff View Preferences</a>
</div>
<gr-overlay id="prefsOverlay" with-backdrop>
<gr-diff-preferences
prefs="{{prefs}}"
on-save="_handlePrefsSave"
on-cancel="_handlePrefsCancel"></gr-diff-preferences>
</gr-overlay>
<div class="diffContainer">
<gr-diff-side id="leftDiff"
@@ -94,8 +95,7 @@ limitations under the License.
patch-num="[[patchRange.basePatchNum]]"
path="[[path]]"
content="{{_diff.leftSide}}"
width="[[sideWidth]]"
tab-width="[[prefs.tab_size]]"
prefs="[[prefs]]"
can-comment="[[_loggedIn]]"
project-config="[[projectConfig]]"
on-expand-context="_handleExpandContext"
@@ -107,8 +107,7 @@ limitations under the License.
patch-num="[[patchRange.patchNum]]"
path="[[path]]"
content="{{_diff.rightSide}}"
width="[[sideWidth]]"
tab-width="[[prefs.tab_size]]"
prefs="[[prefs]]"
can-comment="[[_loggedIn]]"
project-config="[[projectConfig]]"
on-expand-context="_handleExpandContext"
@@ -141,10 +140,6 @@ limitations under the License.
*/
patchRange: Object,
path: String,
sideWidth: {
type: Number,
value: 80,
},
prefs: {
type: Object,
notify: true,
@@ -195,6 +190,8 @@ limitations under the License.
type: Boolean,
value: true,
},
_savedPrefs: Object,
_diffRequestsPromise: Object, // Used for testing.
_diffPreferencesPromise: Object, // Used for testing.
},
@@ -235,16 +232,15 @@ limitations under the License.
}.bind(this));
},
showDiffPreferences: function() {
this.$.prefsOverlay.open();
},
_prefsChanged: function(changeRecord) {
var prefs = changeRecord.base;
this.$.contextSelect.value = prefs.context;
if (this._initialRenderComplete) {
this._render();
}
this._resolvePrefsReady(prefs);
this._resolvePrefsReady(changeRecord.base);
},
_render: function() {
@@ -333,15 +329,31 @@ limitations under the License.
return params;
},
_handleContextSelectChange: function(e) {
var selectEl = Polymer.dom(e).rootTarget;
this.set('prefs.context', parseInt(selectEl.value, 10));
_handlePrefsTap: function(e) {
e.preventDefault();
// TODO(andybons): This is not supported in IE. Implement a polyfill.
// NOTE: Object.assign is NOT automatically a deep copy. If prefs adds
// an object as a value, it must be marked enumerable.
this._savedPrefs = Object.assign({}, this.prefs);
this.$.prefsOverlay.open();
},
_handlePrefsSave: function(e) {
e.stopPropagation();
var el = Polymer.dom(e).rootTarget;
el.disabled = true;
app.accountReady.then(function() {
if (!this._loggedIn) { return; }
this._diffPreferencesPromise =
this._saveDiffPreferences().catch(function(err) {
if (!this._loggedIn) {
el.disabled = false;
this.$.prefsOverlay.close();
return;
}
this._saveDiffPreferences().then(function() {
this.$.prefsOverlay.close();
el.disabled = false;
}.bind(this)).catch(function(err) {
el.disabled = false;
alert('Oops. Something went wrong. Check the console and bug the ' +
'PolyGerrit team for assistance.');
throw err;
@@ -359,6 +371,12 @@ limitations under the License.
return this._diffPreferencesPromise;
},
_handlePrefsCancel: function(e) {
e.stopPropagation();
this.prefs = this._savedPrefs;
this.$.prefsOverlay.close();
},
_handleExpandContext: function(e) {
var ctx = e.detail.context;
var contextControlIndex = -1;
@@ -606,7 +624,7 @@ limitations under the License.
_addCommonDiffChunk: function(ctx, chunk, leftSide, rightSide) {
for (var i = 0; i < chunk.ab.length; i++) {
var numLines = Math.ceil(
this._visibleLineLength(chunk.ab[i]) / this.sideWidth);
this._visibleLineLength(chunk.ab[i]) / this.prefs.line_length);
var hidden = i >= ctx.skipRange[0] &&
i < chunk.ab.length - ctx.skipRange[1];
if (ctx.hidingLines && hidden == false) {
@@ -802,8 +820,8 @@ limitations under the License.
_maxLinesSpanned: function(left, right) {
return Math.max(
Math.ceil(this._visibleLineLength(left) / this.sideWidth),
Math.ceil(this._visibleLineLength(right) / this.sideWidth));
Math.ceil(this._visibleLineLength(left) / this.prefs.line_length),
Math.ceil(this._visibleLineLength(right) / this.prefs.line_length));
},
});

40
polygerrit-ui/app/elements/gr-overlay.html Normal file
View File

@@ -0,0 +1,40 @@
<!--
Copyright (C) 2016 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<link rel="import" href="../bower_components/polymer/polymer.html">
<link rel="import" href="../bower_components/iron-overlay-behavior/iron-overlay-behavior.html">
<dom-module id="gr-overlay">
<style>
:host {
background: #fff;
box-shadow: rgba(0, 0, 0, 0.3) 0 1px 3px;
}
</style>
<template>
<content></content>
</template>
<script>
(function() {
'use strict';
Polymer({
is: 'gr-overlay',
behaviors: [ Polymer.IronOverlayBehavior ],
});
})();
</script>
</dom-module>

75
polygerrit-ui/app/test/gr-diff-preferences-test.html Normal file
View File

@@ -0,0 +1,75 @@
<!DOCTYPE html>
<!--
Copyright (C) 2016 The Android Open Source Project
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
<title>gr-diff-preferences</title>
<script src="../bower_components/webcomponentsjs/webcomponents.min.js"></script>
<script src="../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../bower_components/iron-test-helpers/iron-test-helpers.html">
<link rel="import" href="../elements/gr-diff-preferences.html">
<test-fixture id="basic">
<template>
<gr-diff-preferences></gr-diff-preferences>
</template>
</test-fixture>
<script>
suite('gr-diff-preferences tests', function() {
var element;
setup(function() {
element = fixture('basic');
});
test('model changes', function() {
element.prefs = {
context: 10,
line_length: 100,
show_tabs: true,
tab_size: 8,
};
element.$.contextSelect.value = '50';
element.fire('change', {}, {node: element.$.contextSelect});
element.$.columnsInput.bindValue = 80;
element.$.tabSizeInput.bindValue = 4;
MockInteractions.tap(element.$.showTabsInput);
assert.equal(element.prefs.context, 50);
assert.equal(element.prefs.line_length, 80);
assert.equal(element.prefs.tab_size, 4);
assert.isFalse(element.prefs.show_tabs);
});
test('events', function(done) {
var savePromise = new Promise(function(resolve) {
element.addEventListener('save', function() { resolve(); });
});
var cancelPromise = new Promise(function(resolve) {
element.addEventListener('cancel', function() { resolve(); });
})
Promise.all([savePromise, cancelPromise]).then(function() {
done();
});
MockInteractions.tap(element.$$('button.primary'));
MockInteractions.tap(element.$$('.cancel button'));
});
});
</script>

58
polygerrit-ui/app/test/gr-diff-side-test.html
View File

@@ -104,8 +104,10 @@ limitations under the License.
});
test('newlines', function() {
element.width = 80;
element.tabWidth = 4;
element.prefs = {
line_length: 80,
tab_size: 4,
};
element.content = [{
type: 'CODE',
@@ -138,6 +140,14 @@ limitations under the License.
assert.equal(contentEl.innerHTML,
'A'.repeat(80) + element._lineFeedHTML +
'A'.repeat(20) + element._lineFeedHTML);
});
test('tabs', function(done) {
element.prefs = {
line_length: 80,
tab_size: 4,
show_tabs: true,
};
element.content = [{
type: 'CODE',
@@ -148,15 +158,47 @@ limitations under the License.
intraline: [],
}];
lineEl = element.$$('.numbers .lineNum[data-line-num="1"]');
var lineEl = element.$$('.numbers .lineNum[data-line-num="1"]');
assert.ok(lineEl);
var contentEl = element.$$('.content .code[data-line-num="1"]');
assert.ok(contentEl);
var spanEl = contentEl.childNodes[1];
assert.equal(spanEl.tagName, 'SPAN');
assert.isTrue(spanEl.classList.contains(
'style-scope', 'gr-diff-side', 'tab', 'withIndicator'));
element.prefs.show_tabs = false;
element.content = [{
type: 'CODE',
content: 'A'.repeat(50) + '\t' + 'A'.repeat(50),
numLines: 2,
lineNum: 1,
highlight: false,
intraline: [],
}];
contentEl = element.$$('.content .code[data-line-num="1"]');
assert.ok(contentEl);
assert.equal(contentEl.innerHTML,
'A'.repeat(50) +
'<span class="style-scope gr-diff-side tab">\t</span>' +
'A'.repeat(26) + element._lineFeedHTML +
'A'.repeat(24) + element._lineFeedHTML);
var spanEl = contentEl.childNodes[1];
assert.equal(spanEl.tagName, 'SPAN');
assert.isTrue(spanEl.classList.contains(
'style-scope', 'gr-diff-side', 'tab'));
var alertStub = sinon.stub(window, 'alert');
element.prefs.tab_size =
'"><img src="/" onerror="alert(1);"><span class="';
element.content = [{
type: 'CODE',
content: '\t',
numLines: 1,
lineNum: 1,
highlight: false,
intraline: [],
}];
element.async(function() {
assert.isFalse(alertStub.called);
alertStub.restore();
done();
}, 100); // Allow some time for the img error event to fire.
});
test('diff context', function() {

14
polygerrit-ui/app/test/gr-diff-test.html
View File

@@ -481,18 +481,18 @@ limitations under the License.
assert.equal(leftContext.end, 19);
});
test('save context', function(done) {
test('save prefs', function(done) {
element._loggedIn = false;
var contextSelectEl = element.$.contextSelect;
assert.ok(contextSelectEl);
contextSelectEl.value = '50';
element.fire('change', {}, {node: contextSelectEl});
element.prefs = {
tab_size: 4,
context: 50,
};
element.fire('save', {}, {node: element.$$('gr-diff-preferences')});
assert.isTrue(element._diffPreferencesPromise == null);
element._loggedIn = true;
contextSelectEl.value = '25';
element.fire('change', {}, {node: contextSelectEl});
element.fire('save', {}, {node: element.$$('gr-diff-preferences')});
server.respond();
element._diffPreferencesPromise.then(function(req) {

5
polygerrit-ui/app/test/gr-diff-view-test.html
View File

@@ -78,6 +78,11 @@ limitations under the License.
assert(showStub.lastCall.calledWithExactly('/c/42'),
'Should navigate to /c/42');
var showPrefsStub = sinon.stub(element.$.diff, 'showDiffPreferences');
MockInteractions.pressAndReleaseKeyOn(element, 188); // ','
assert(showPrefsStub.calledOnce);
showPrefsStub.restore();
showStub.restore();
});

1
polygerrit-ui/app/test/index.html
View File

@@ -35,6 +35,7 @@ limitations under the License.
'gr-date-formatter-test.html',
'gr-diff-comment-test.html',
'gr-diff-comment-thread-test.html',
'gr-diff-preferences-test.html',
'gr-diff-side-test.html',
'gr-diff-test.html',
'gr-diff-view-test.html',