Add dropdown for selecting diff view mode to file list
Setting persists for the duration of viewing a change. When a user
selects a new change, their default view mode is set.
Bug: Issue 4670
Change-Id: I4eade0e42c13bf2b3079ef38fb07239f98a243d8
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 a4b0a9b..5925725 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
@@ -298,7 +298,8 @@
drafts="[[_diffDrafts]]"
revisions="[[_change.revisions]]"
projectConfig="[[_projectConfig]]"
- selected-index="{{viewState.selectedFileIndex}}"></gr-file-list>
+ selected-index="{{viewState.selectedFileIndex}}"
+ diff-view-mode="{{viewState.diffMode}}"></gr-file-list>
</section>
<gr-messages-list id="messageList"
change-num="[[_changeNum]]"
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 87e03dc..a297cb2 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
@@ -147,9 +147,18 @@
/
<gr-button link on-tap="_collapseAllDiffs">Hide diffs</gr-button>
/
+ <select
+ id="modeSelect"
+ is="gr-select"
+ bind-value="{{diffViewMode}}"
+ on-change="_handleDropdownChange">
+ <option value="SIDE_BY_SIDE">Side By Side</option>
+ <option value="UNIFIED_DIFF">Unified</option>
+ </select>
+ /
<label>
Diff against
- <select on-change="_handlePatchChange">
+ <select id="patchChange" on-change="_handlePatchChange">
<option value="PARENT">Base</option>
<template is="dom-repeat" items="[[_computePatchSets(revisions, patchRange.*)]]" as="patchNum">
<option
@@ -210,7 +219,7 @@
path="[[file.__path]]"
prefs="[[_diffPrefs]]"
project-config="[[projectConfig]]"
- view-mode="[[_userPrefs.diff_view]]"></gr-diff>
+ view-mode="[[_diffMode]]"></gr-diff>
</template>
<gr-button
class="fileListButton"
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 a562338..874cc89 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
@@ -16,6 +16,11 @@
var COMMIT_MESSAGE_PATH = '/COMMIT_MSG';
+ var DiffViewMode = {
+ SIDE_BY_SIDE: 'SIDE_BY_SIDE',
+ UNIFIED: 'UNIFIED_DIFF',
+ };
+
Polymer({
is: 'gr-file-list',
@@ -36,7 +41,10 @@
value: function() { return document.body; },
},
change: Object,
-
+ diffViewMode: {
+ type: String,
+ notify: true,
+ },
_files: {
type: Array,
observer: '_filesChanged',
@@ -67,6 +75,10 @@
type: Array,
computed: '_computeFilesShown(_numFilesShown, _files.*)',
},
+ _diffMode: {
+ type: String,
+ computed: '_getDiffViewMode(diffViewMode, _userPrefs)',
+ },
},
behaviors: [
@@ -100,8 +112,17 @@
this._diffPrefs = prefs;
}.bind(this)));
+ // Initialize with user's diff mode preference. Default to
+ // SIDE_BY_SIDE in the meantime.
+ var setDiffViewMode = this.diffViewMode === null;
+ if (setDiffViewMode) {
+ this.set('diffViewMode', DiffViewMode.SIDE_BY_SIDE);
+ }
promises.push(this._getPreferences().then(function(prefs) {
this._userPrefs = prefs;
+ if (setDiffViewMode) {
+ this.set('diffViewMode', prefs.diff_view);
+ }
}.bind(this)));
},
@@ -474,5 +495,32 @@
_showAllFiles: function() {
this._numFilesShown = this._files.length;
},
+
+ /**
+ * _getDiffViewMode: Get the diff view (side-by-side or unified) based on
+ * the current state.
+ *
+ * The expected behavior is to use the mode specified in the user's
+ * preferences unless they have manually chosen the alternative view. If the
+ * user navigates up to the change view, it should clear this choice and
+ * revert to the preference the next time a diff is viewed.
+ *
+ * Use side-by-side if the user is not logged in.
+ *
+ * @return {String}
+ */
+ _getDiffViewMode: function() {
+ if (this.diffViewMode) {
+ return this.diffViewMode;
+ } else if (this._userPrefs && this._userPrefs.diff_view) {
+ return this.diffViewMode = this._userPrefs.diff_view;
+ }
+
+ return DiffViewMode.SIDE_BY_SIDE;
+ },
+
+ _handleDropdownChange: function(e) {
+ e.target.blur();
+ },
});
})();
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 502597c..3402395 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
@@ -32,20 +32,36 @@
</template>
</test-fixture>
+<test-fixture id="blank">
+ <template>
+ <div></div>
+ </template>
+</test-fixture>
+
<script>
suite('gr-file-list tests', function() {
var element;
+ var sandbox;
setup(function() {
+ sandbox = sinon.sandbox.create();
stub('gr-rest-api-interface', {
getLoggedIn: function() { return Promise.resolve(true); },
getPreferences: function() { return Promise.resolve({}); },
+ fetchJSON: function() { return Promise.resolve({}); },
+ });
+ stub('gr-date-formatter', {
+ _loadTimeFormat: function() { return Promise.resolve(''); }
});
element = fixture('basic');
});
+ teardown(function() {
+ sandbox.restore();
+ });
+
test('get file list', function(done) {
- var getChangeFilesStub = sinon.stub(element.$.restAPI, 'getChangeFiles',
+ var getChangeFilesStub = sandbox.stub(element.$.restAPI, 'getChangeFiles',
function() {
return Promise.resolve({
'/COMMIT_MSG': {lines_inserted: 9},
@@ -97,12 +113,15 @@
});
test('toggle left diff via shortcut', function() {
- var toggleLeftDiffStub = sinon.stub();
- sinon.stub(element, 'diffs', {get: function() {
+ var toggleLeftDiffStub = sandbox.stub();
+ // Property getter cannot be stubbed w/ sandbox due to a bug in Sinon.
+ // https://github.com/sinonjs/sinon/issues/781
+ var diffsStub = sinon.stub(element, 'diffs', {get: function() {
return [{toggleLeftDiff: toggleLeftDiffStub}];
}});
MockInteractions.pressAndReleaseKeyOn(element, 65, 'shift'); // 'A'
assert.isTrue(toggleLeftDiffStub.calledOnce);
+ diffsStub.restore();
});
test('keyboard shortcuts', function() {
@@ -117,7 +136,7 @@
assert.equal(element.selectedIndex, 1);
MockInteractions.pressAndReleaseKeyOn(element, 74); // 'J'
- var showStub = sinon.stub(page, 'show');
+ var showStub = sandbox.stub(page, 'show');
assert.equal(element.selectedIndex, 2);
MockInteractions.pressAndReleaseKeyOn(element, 13); // 'ENTER'
assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
@@ -241,7 +260,7 @@
assert.isFalse(fileAdded.checked);
assert.isTrue(myFile.checked);
- var saveStub = sinon.stub(element, '_saveReviewedState',
+ var saveStub = sandbox.stub(element, '_saveReviewedState',
function() { return Promise.resolve(); });
MockInteractions.tap(commitMsg);
@@ -272,7 +291,7 @@
});
test('diff against dropdown', function(done) {
- var showStub = sinon.stub(page, 'show');
+ var showStub = sandbox.stub(page, 'show');
element.changeNum = '42';
element.patchRange = {
basePatchNum: 'PARENT',
@@ -284,7 +303,7 @@
rev3: {_number: 3},
};
flush(function() {
- var selectEl = element.$$('select');
+ var selectEl = element.$.patchChange;
assert.equal(selectEl.value, 'PARENT');
assert.isTrue(element.$$('option[value="3"]').hasAttribute('disabled'));
selectEl.addEventListener('change', function() {
@@ -313,7 +332,7 @@
var fileRows =
Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
// Prevent diff from making API call.
- var diffStub = sinon.stub(element.diffs[0], 'reload');
+ var diffStub = sandbox.stub(element.diffs[0], 'reload');
var showHideCheck = fileRows[0].querySelector(
'input.show-hide[type="checkbox"]');
assert.isTrue(showHideCheck.checked);
@@ -339,5 +358,50 @@
element.$$('a').getAttribute('href'),
'/c/42/2/foo+bar/my%252Bfile.txt%2525');
});
+
+ test('diff mode correctly toggles the diffs', function() {
+ element._files = [
+ {__path: 'myfile.txt', __expanded: false},
+ ];
+ element.changeNum = '42';
+ element.patchRange = {
+ basePatchNum: 'PARENT',
+ patchNum: '2',
+ };
+ element.selectedIndex = 0;
+ flushAsynchronousOperations();
+ var select = element.$.modeSelect;
+ var diffDisplay = element.diffs[0];
+ element._userPrefs = {diff_view: 'SIDE_BY_SIDE'};
+ assert.equal(element._getDiffViewMode(), 'SIDE_BY_SIDE');
+ assert.equal(element.diffViewMode, 'SIDE_BY_SIDE');
+ assert.equal(diffDisplay.viewMode, 'SIDE_BY_SIDE');
+ element.set('diffViewMode', 'UNIFIED_DIFF');
+ assert.equal(element._getDiffViewMode(), 'UNIFIED_DIFF');
+ assert.equal(diffDisplay.viewMode, 'UNIFIED_DIFF');
+ });
+
+ test('diff mode selector initializes from preferences', function() {
+ var resolvePrefs;
+ var prefsPromise = new Promise(function(resolve) {
+ resolvePrefs = resolve;
+ });
+ sandbox.stub(element, '_getPreferences').returns(prefsPromise);
+
+ // Attach a new gr-file-list so we can intercept the preferences fetch.
+ var view = document.createElement('gr-file-list');
+ var select = view.$.modeSelect;
+ fixture('blank').appendChild(view);
+ flushAsynchronousOperations();
+
+ // At this point the diff mode doesn't yet have the user's preference.
+ assert.equal(select.value, 'SIDE_BY_SIDE');
+
+ // Receive the overriding preference.
+ resolvePrefs({diff_view: 'UNIFIED'});
+ flushAsynchronousOperations();
+ assert.equal(select.value, 'SIDE_BY_SIDE');
+ document.getElementById('blank').restore();
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 6c73e08..03609c0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -104,7 +104,6 @@
this._setReviewed(true);
}
}.bind(this));
-
if (this.changeViewState.diffMode === null) {
// Initialize with user's diff mode preference. Default to
// SIDE_BY_SIDE in the meantime.
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 1eb3f95..566c6a1 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
@@ -457,7 +457,6 @@
// We will simulate a user change of the selected mode.
var newMode = 'UNIFIED_DIFF';
-
// Set the actual value of the select, and simulate the change event.
select.value = newMode;
element.fire('change', {}, {node: select});
