Merge "Add diff preferences to change view"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
index 776ace2..2d57392 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -19,6 +19,7 @@
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html">
<link rel="import" href="../../../behaviors/rest-client-behavior/rest-client-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../diff/gr-diff-preferences/gr-diff-preferences.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
<link rel="import" href="../../shared/gr-select/gr-select.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
@@ -78,6 +79,9 @@
font-size: 1.2em;
font-weight: bold;
}
+ .prefsButton {
+ float: right;
+ }
gr-change-star {
margin-right: .25em;
vertical-align: -.425em;
@@ -193,6 +197,9 @@
height: 0;
margin-bottom: 1em;
}
+ #diffPrefsContainer {
+ margin: auto 0 auto auto;
+ }
.patchInfo-header-wrapper {
width: 100%;
}
@@ -461,9 +468,17 @@
read-only="[[_descriptionReadOnly]]"
on-changed="_handleDescriptionChanged"></gr-editable-label>
</span>
+ <span id="diffPrefsContainer"
+ hidden$="[[_computePrefsButtonHidden(_diffPrefs, _loggedIn)]]"
+ hidden>
+ <gr-button link
+ class="prefsButton desktop"
+ on-tap="_handlePrefsTap">Diff Preferences</gr-button>
+ </span>
</div>
</div>
<gr-file-list id="fileList"
+ diff-prefs="{{_diffPrefs}}"
change="[[_change]]"
change-num="[[_changeNum]]"
patch-range="{{_patchRange}}"
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
index f140296..ea835e7 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -73,6 +73,7 @@
type: Object,
value: function() { return document.body; },
},
+ _diffPrefs: Object,
_numFilesShown: {
type: Number,
observer: '_numFilesShownChanged',
@@ -190,6 +191,7 @@
'u': '_handleUKey',
'x': '_handleXKey',
'z': '_handleZKey',
+ ',': '_handleCommaKey',
},
attached: function() {
@@ -225,6 +227,10 @@
}
},
+ _computePrefsButtonHidden: function(prefs, loggedIn) {
+ return !loggedIn || !prefs;
+ },
+
_handleEditCommitMessage: function(e) {
this._editingCommitMessage = true;
this.$.commitMessageEditor.focusTextarea();
@@ -273,6 +279,11 @@
return false;
},
+ _handlePrefsTap: function(e) {
+ e.preventDefault();
+ this.$.fileList.openDiffPrefs();
+ },
+
_handleCommentSave: function(e) {
if (!e.target.comment.__draft) { return; }
@@ -800,6 +811,14 @@
this.$.messageList.handleExpandCollapse(false);
},
+ _handleCommaKey: function(e) {
+ if (this.shouldSuppressKeyboardShortcut(e) ||
+ this.modifierPressed(e)) { return; }
+
+ e.preventDefault();
+ this.$.fileList.openDiffPrefs();
+ },
+
_determinePageBack: function() {
// Default backPage to '/' if user came to change view page
// via an email link, etc.
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
index 06fb387..5e932f8d 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -157,6 +157,45 @@
MockInteractions.pressAndReleaseKeyOn(element, 68, null, 'd');
assert.isTrue(stub.called);
});
+
+ test(', should open diff preferences', function() {
+ var stub = sandbox.stub(element.$.fileList.$.diffPreferences, 'open');
+ MockInteractions.pressAndReleaseKeyOn(element, 188, null, ',');
+ assert.isTrue(stub.called);
+ });
+ });
+
+ test('Diff preferences hidden when no prefs or logged out',
+ function() {
+ element._loggedIn = false;
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = true;
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = false;
+ element._diffPrefs = {'font_size': '12'};
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = true;
+ flushAsynchronousOperations();
+ assert.isFalse(element.$.diffPrefsContainer.hidden);
+ });
+
+ test('prefsButton opens gr-diff-preferences', function() {
+ var handlePrefsTapSpy = sandbox.spy(element, '_handlePrefsTap');
+ var overlayOpenStub = sandbox.stub(element.$.fileList,
+ 'openDiffPrefs');
+ var prefsButton = Polymer.dom(element.root).querySelectorAll(
+ '.prefsButton')[0];
+
+ MockInteractions.tap(prefsButton);
+
+ assert.isTrue(handlePrefsTapSpy.called);
+ assert.isTrue(overlayOpenStub.called);
});
test('_computeDescriptionReadOnly', function() {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 6f8568c..f3e1092 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -328,9 +328,10 @@
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
path="[[file.__path]]"
- prefs="[[_diffPrefs]]"
+ prefs="[[diffPrefs]]"
project-config="[[projectConfig]]"
on-line-selected="_onLineSelected"
+ no-render-on-prefs-change
view-mode="[[_getDiffViewMode(diffViewMode, _userPrefs)]]"></gr-diff>
</template>
</template>
@@ -384,6 +385,10 @@
[[_computeShowAllText(_files)]]
</gr-button><!--
--></gr-tooltip-content>
+ <gr-diff-preferences
+ id="diffPreferences"
+ prefs="{{diffPrefs}}"
+ local-prefs="{{_localPrefs}}"></gr-diff-preferences>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
<gr-diff-cursor id="diffCursor"></gr-diff-cursor>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 705a8f1..30ccb76 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -54,6 +54,7 @@
diffViewMode: {
type: String,
notify: true,
+ observer: '_updateDiffPreferences',
},
_files: {
type: Array,
@@ -69,7 +70,11 @@
value: function() { return []; },
},
_diffAgainst: String,
- _diffPrefs: Object,
+ diffPrefs: {
+ type: Object,
+ notify: true,
+ observer: '_updateDiffPreferences',
+ },
_userPrefs: Object,
_localPrefs: Object,
_showInlineDiffs: Boolean,
@@ -158,7 +163,7 @@
this._localPrefs = this.$.storage.getPreferences();
promises.push(this._getDiffPreferences().then(function(prefs) {
- this._diffPrefs = prefs;
+ this.diffPrefs = prefs;
}.bind(this)));
promises.push(this._getPreferences().then(function(prefs) {
@@ -173,6 +178,10 @@
return Polymer.dom(this.root).querySelectorAll('gr-diff');
},
+ openDiffPrefs: function() {
+ this.$.diffPreferences.open();
+ },
+
_calculatePatchChange: function(files) {
var filesNoCommitMsg = files.filter(function(files) {
return files.__path !== '/COMMIT_MSG';
@@ -232,6 +241,19 @@
this._patchRangeStr(patchRange), true));
},
+ _updateDiffPreferences: function() {
+ if (!this.diffs.length) { return; }
+ // Re-render all expanded diffs sequentially.
+ var timerName = 'Update ' + this._expandedFilePaths.length +
+ ' diffs with new prefs';
+ this._renderInOrder(this._expandedFilePaths, this.diffs,
+ this._expandedFilePaths.length)
+ .then(function() {
+ this.$.reporting.timeEnd(timerName);
+ this.$.diffCursor.handleDiffUpdate();
+ }.bind(this));
+ },
+
_forEachDiff: function(fn) {
var diffs = this.diffs;
for (var i = 0; i < diffs.length; i++) {
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index a34463b..d07f364 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -699,6 +699,7 @@
basePatchNum: 'PARENT',
patchNum: '2',
};
+ sandbox.spy(element, '_updateDiffPreferences');
element.$.fileCursor.setCursorAtIndex(0);
flushAsynchronousOperations();
@@ -714,6 +715,7 @@
assert.equal(diffDisplay.viewMode, 'SIDE_BY_SIDE');
element.set('diffViewMode', 'UNIFIED_DIFF');
assert.equal(diffDisplay.viewMode, 'UNIFIED_DIFF');
+ assert.isTrue(element._updateDiffPreferences.called);
});
test('diff mode selector initializes from preferences', function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 214454a..c407829 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -405,7 +405,6 @@
td.classList.add(line.type);
var html = this._escapeHTML(text);
html = this._addTabWrappers(html, this._prefs.tab_size);
-
if (!this._prefs.line_wrapping &&
this._textLength(text, this._prefs.tab_size) >
this._prefs.line_length) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
index a3ff3d1..93d939d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.html
@@ -264,7 +264,8 @@
<option value="SIDE_BY_SIDE">Side By Side</option>
<option value="UNIFIED_DIFF">Unified</option>
</select>
- <span hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]">
+ <span id="diffPrefsContainer"
+ hidden$="[[_computePrefsButtonHidden(_prefs, _loggedIn)]]" hidden>
<span class="preferences desktop">
<span
hidden$="[[_computeModeSelectHidden(_isImageDiff)]]">/</span>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
index 166a3d3..663cd3b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
@@ -252,6 +252,38 @@
'Should navigate to /c/42/1');
});
+ test('Diff preferences hidden when no prefs or logged out',
+ function() {
+ element._loggedIn = false;
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = true;
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = false;
+ element._prefs = {'font_size': '12'};
+ flushAsynchronousOperations();
+ assert.isTrue(element.$.diffPrefsContainer.hidden);
+
+ element._loggedIn = true;
+ flushAsynchronousOperations();
+ assert.isFalse(element.$.diffPrefsContainer.hidden);
+ });
+
+ test('prefsButton opens gr-diff-preferences', function() {
+ var handlePrefsTapSpy = sandbox.spy(element, '_handlePrefsTap');
+ var overlayOpenStub = sandbox.stub(element.$.diffPreferences,
+ 'open');
+ var prefsButton = Polymer.dom(element.root).querySelector('.prefsButton');
+
+ MockInteractions.tap(prefsButton);
+
+ assert.isTrue(handlePrefsTapSpy.called);
+ assert.isTrue(overlayOpenStub.called);
+ });
+
test('go up to change via kb without change loaded', function() {
element._changeNum = '42';
element._patchRange = {
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 cd1931a..1d766b86 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -74,6 +74,7 @@
type: Boolean,
reflectToAttribute: true,
},
+ noRenderOnPrefsChange: Boolean,
_loggedIn: {
type: Boolean,
value: false,
@@ -438,7 +439,7 @@
this.updateStyles();
- if (this._diff && this._comments) {
+ if (this._diff && this._comments && !this.noRenderOnPrefsChange) {
this._renderDiffTable();
}
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index 5868f77..2a150ff 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -840,6 +840,59 @@
});
});
+ suite('change in preferences', function() {
+ setup(function() {
+ element._diff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg', lines: 66},
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ diff_header: [],
+ intraline_status: 'OK',
+ change_type: 'MODIFIED',
+ content: [{skip: 66}],
+ };
+ element._comments = {
+ meta: {
+ changeNum: '42',
+ patchRange: {
+ basePatchNum: 'PARENT',
+ patchNum: 3,
+ },
+ path: '/path/to/foo',
+ projectConfig: {foo: 'bar'},
+ },
+ left: [
+ {id: 'bc1', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bc2', side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd1', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ {id: 'bd2', __draft: true, side: 'PARENT', __commentSide: 'left'},
+ ],
+ right: [
+ {id: 'c1', __commentSide: 'right'},
+ {id: 'c2', __commentSide: 'right'},
+ {id: 'd1', __draft: true, __commentSide: 'right'},
+ {id: 'd2', __draft: true, __commentSide: 'right'},
+ ],
+ };
+ });
+
+ test('change in preferences re-renders diff', function() {
+ sandbox.stub(element, '_renderDiffTable');
+ element.prefs = {};
+ element.prefs = {time_format: 'HHMM_12'};
+ assert.isTrue(element._renderDiffTable.called);
+ });
+
+ test('change in preferences does not re-renders diff with ' +
+ 'noRenderOnPrefsChange', function() {
+ sandbox.stub(element, '_renderDiffTable');
+ element.noRenderOnPrefsChange = true;
+ element.prefs = {};
+ element.prefs = {time_format: 'HHMM_12'};
+ assert.isFalse(element._renderDiffTable.called);
+ });
+ });
+
suite('handle comment-update', function() {
setup(function() {
